From 8d4a331bbaece1b9ebc17caeca9873647c51ca77 Mon Sep 17 00:00:00 2001 From: leopoldsedev Date: Sat, 15 Feb 2020 19:12:35 +0100 Subject: [PATCH] Rework undo deletion logic. Previously the MessageFacade and MessageStateHolder both had their own state of the last deleted message, which was redundant. Now only MessageStateHolder governs the state of a pending deletion and MessageFacade handles commiting the deletion to the server. --- .../gotify/messages/MessagesActivity.java | 10 ++-- ...PositionPair.java => MessageDeletion.java} | 12 +++- .../messages/provider/MessageFacade.java | 26 +++------ .../messages/provider/MessageStateHolder.java | 55 +++++++++++++------ 4 files changed, 60 insertions(+), 43 deletions(-) rename app/src/main/java/com/github/gotify/messages/provider/{PositionPair.java => MessageDeletion.java} (53%) diff --git a/app/src/main/java/com/github/gotify/messages/MessagesActivity.java b/app/src/main/java/com/github/gotify/messages/MessagesActivity.java index 930a6a8..dba3a1b 100644 --- a/app/src/main/java/com/github/gotify/messages/MessagesActivity.java +++ b/app/src/main/java/com/github/gotify/messages/MessagesActivity.java @@ -55,10 +55,10 @@ import com.github.gotify.log.Log; import com.github.gotify.log.LogsActivity; import com.github.gotify.login.LoginActivity; import com.github.gotify.messages.provider.ApplicationHolder; +import com.github.gotify.messages.provider.MessageDeletion; import com.github.gotify.messages.provider.MessageFacade; import com.github.gotify.messages.provider.MessageState; import com.github.gotify.messages.provider.MessageWithImage; -import com.github.gotify.messages.provider.PositionPair; import com.github.gotify.service.WebSocketService; import com.google.android.material.navigation.NavigationView; import com.google.android.material.snackbar.BaseTransientBottomBar; @@ -374,15 +374,15 @@ public class MessagesActivity extends AppCompatActivity } private void undoDelete() { - PositionPair positionPair = messages.undoDeleteLocal(); + MessageDeletion deletion = messages.undoDeleteLocal(); - if (positionPair != null) { + if (deletion != null) { ListMessageAdapter adapter = (ListMessageAdapter) messagesView.getAdapter(); adapter.setItems(messages.get(appId)); int insertPosition = appId == MessageState.ALL_MESSAGES - ? positionPair.getAllPosition() - : positionPair.getAppPosition(); + ? deletion.getAllPosition() + : deletion.getAppPosition(); adapter.notifyItemInserted(insertPosition); } } diff --git a/app/src/main/java/com/github/gotify/messages/provider/PositionPair.java b/app/src/main/java/com/github/gotify/messages/provider/MessageDeletion.java similarity index 53% rename from app/src/main/java/com/github/gotify/messages/provider/PositionPair.java rename to app/src/main/java/com/github/gotify/messages/provider/MessageDeletion.java index 2f9fe03..471f77b 100644 --- a/app/src/main/java/com/github/gotify/messages/provider/PositionPair.java +++ b/app/src/main/java/com/github/gotify/messages/provider/MessageDeletion.java @@ -1,10 +1,14 @@ package com.github.gotify.messages.provider; -public final class PositionPair { +import com.github.gotify.client.model.Message; + +public final class MessageDeletion { + private final Message message; private final int allPosition; private final int appPosition; - public PositionPair(int allPosition, int appPosition) { + public MessageDeletion(Message message, int allPosition, int appPosition) { + this.message = message; this.allPosition = allPosition; this.appPosition = appPosition; } @@ -16,4 +20,8 @@ public final class PositionPair { public int getAppPosition() { return appPosition; } + + public Message getMessage() { + return message; + } } diff --git a/app/src/main/java/com/github/gotify/messages/provider/MessageFacade.java b/app/src/main/java/com/github/gotify/messages/provider/MessageFacade.java index d829141..4f0b93c 100644 --- a/app/src/main/java/com/github/gotify/messages/provider/MessageFacade.java +++ b/app/src/main/java/com/github/gotify/messages/provider/MessageFacade.java @@ -10,7 +10,6 @@ public class MessageFacade { private final MessageRequester requester; private final MessageStateHolder state; private final MessageImageCombiner combiner; - private Message messagePendingDeletion; public MessageFacade(MessageApi api, ApplicationHolder applicationHolder) { this.applicationHolder = applicationHolder; @@ -34,13 +33,6 @@ public class MessageFacade { if (state.hasNext || !state.loaded) { PagedMessages pagedMessages = requester.loadMore(state); this.state.newMessages(appId, pagedMessages); - - // If there is a message with pending removal, it should not reappear in the list when - // reloading. Thus, it needs to be removed from the local list again after loading new - // messages. - if (messagePendingDeletion != null) { - this.state.removeMessage(messagePendingDeletion); - } } return get(appId); } @@ -61,23 +53,21 @@ public class MessageFacade { } public synchronized void deleteLocal(Message message) { - this.state.removeMessage(message); // If there is already a deletion pending, that one should be executed before scheduling the // next deletion. - if (messagePendingDeletion != null) { - commitDelete(); - } - messagePendingDeletion = message; + if (this.state.deletionPending()) commitDelete(); + this.state.deleteMessage(message); } public synchronized void commitDelete() { - this.requester.asyncRemoveMessage(messagePendingDeletion); - messagePendingDeletion = null; + if (this.state.deletionPending()) { + MessageDeletion deletion = this.state.purgePendingDeletion(); + this.requester.asyncRemoveMessage(deletion.getMessage()); + } } - public synchronized PositionPair undoDeleteLocal() { - messagePendingDeletion = null; - return this.state.undoLastRemoveMessage(); + public synchronized MessageDeletion undoDeleteLocal() { + return this.state.undoPendingDeletion(); } public boolean deleteAll(Integer appId) { diff --git a/app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java b/app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java index 6cc050d..e465661 100644 --- a/app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java +++ b/app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java @@ -9,9 +9,7 @@ class MessageStateHolder { private int lastReceivedMessage = -1; private Map states = new HashMap<>(); - private Message lastRemovedMessage; - private int lastRemovedAllPosition; - private int lastRemovedAppPosition; + private MessageDeletion pendingDeletion = null; synchronized void clear() { states = new HashMap<>(); @@ -31,11 +29,24 @@ class MessageStateHolder { state.nextSince = pagedMessages.getPaging().getSince(); state.appId = appId; states.put(appId, state); + + // If there is a message with pending deletion, it should not reappear in the list in case + // it is added again. + if (deletionPending()) { + deleteMessage(pendingDeletion.getMessage()); + } } synchronized void newMessage(Message message) { + // If there is a message with pending deletion, its indices are going to change. To keep + // them consistent the deletion is undone first and redone again after adding the new + // message. + MessageDeletion deletion = undoPendingDeletion(); + addMessage(message, 0, 0); lastReceivedMessage = message.getId(); + + if (deletion != null) deleteMessage(deletion.getMessage()); } synchronized MessageState state(Integer appId) { @@ -66,40 +77,48 @@ class MessageStateHolder { return lastReceivedMessage; } - synchronized void removeMessage(Message message) { + synchronized void deleteMessage(Message message) { MessageState allMessages = state(MessageState.ALL_MESSAGES); MessageState appMessages = state(message.getAppid()); + int pendingDeletedAllPosition = -1; + int pendingDeletedAppPosition = -1; + if (allMessages.loaded) { int allPosition = allMessages.messages.indexOf(message); allMessages.messages.remove(allPosition); - lastRemovedAllPosition = allPosition; + pendingDeletedAllPosition = allPosition; } if (appMessages.loaded) { int appPosition = appMessages.messages.indexOf(message); appMessages.messages.remove(appPosition); - lastRemovedAppPosition = appPosition; + pendingDeletedAppPosition = appPosition; } - lastRemovedMessage = message; + pendingDeletion = + new MessageDeletion(message, pendingDeletedAllPosition, pendingDeletedAppPosition); } - synchronized PositionPair undoLastRemoveMessage() { - PositionPair result = null; - - if (lastRemovedMessage != null) { - addMessage(lastRemovedMessage, lastRemovedAllPosition, lastRemovedAppPosition); - result = new PositionPair(lastRemovedAllPosition, lastRemovedAppPosition); - - lastRemovedMessage = null; - lastRemovedAllPosition = -1; - lastRemovedAppPosition = -1; - } + synchronized MessageDeletion undoPendingDeletion() { + if (pendingDeletion != null) + addMessage( + pendingDeletion.getMessage(), + pendingDeletion.getAllPosition(), + pendingDeletion.getAppPosition()); + return purgePendingDeletion(); + } + synchronized MessageDeletion purgePendingDeletion() { + MessageDeletion result = pendingDeletion; + pendingDeletion = null; return result; } + boolean deletionPending() { + return pendingDeletion != null; + } + private void addMessage(Message message, int allPosition, int appPosition) { MessageState allMessages = state(MessageState.ALL_MESSAGES); MessageState appMessages = state(message.getAppid());