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.
This commit is contained in:
committed by
Jannis Mattheis
parent
0ebeeec35e
commit
8d4a331bba
@@ -55,10 +55,10 @@ import com.github.gotify.log.Log;
|
|||||||
import com.github.gotify.log.LogsActivity;
|
import com.github.gotify.log.LogsActivity;
|
||||||
import com.github.gotify.login.LoginActivity;
|
import com.github.gotify.login.LoginActivity;
|
||||||
import com.github.gotify.messages.provider.ApplicationHolder;
|
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.MessageFacade;
|
||||||
import com.github.gotify.messages.provider.MessageState;
|
import com.github.gotify.messages.provider.MessageState;
|
||||||
import com.github.gotify.messages.provider.MessageWithImage;
|
import com.github.gotify.messages.provider.MessageWithImage;
|
||||||
import com.github.gotify.messages.provider.PositionPair;
|
|
||||||
import com.github.gotify.service.WebSocketService;
|
import com.github.gotify.service.WebSocketService;
|
||||||
import com.google.android.material.navigation.NavigationView;
|
import com.google.android.material.navigation.NavigationView;
|
||||||
import com.google.android.material.snackbar.BaseTransientBottomBar;
|
import com.google.android.material.snackbar.BaseTransientBottomBar;
|
||||||
@@ -374,15 +374,15 @@ public class MessagesActivity extends AppCompatActivity
|
|||||||
}
|
}
|
||||||
|
|
||||||
private void undoDelete() {
|
private void undoDelete() {
|
||||||
PositionPair positionPair = messages.undoDeleteLocal();
|
MessageDeletion deletion = messages.undoDeleteLocal();
|
||||||
|
|
||||||
if (positionPair != null) {
|
if (deletion != null) {
|
||||||
ListMessageAdapter adapter = (ListMessageAdapter) messagesView.getAdapter();
|
ListMessageAdapter adapter = (ListMessageAdapter) messagesView.getAdapter();
|
||||||
adapter.setItems(messages.get(appId));
|
adapter.setItems(messages.get(appId));
|
||||||
int insertPosition =
|
int insertPosition =
|
||||||
appId == MessageState.ALL_MESSAGES
|
appId == MessageState.ALL_MESSAGES
|
||||||
? positionPair.getAllPosition()
|
? deletion.getAllPosition()
|
||||||
: positionPair.getAppPosition();
|
: deletion.getAppPosition();
|
||||||
adapter.notifyItemInserted(insertPosition);
|
adapter.notifyItemInserted(insertPosition);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,10 +1,14 @@
|
|||||||
package com.github.gotify.messages.provider;
|
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 allPosition;
|
||||||
private final int appPosition;
|
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.allPosition = allPosition;
|
||||||
this.appPosition = appPosition;
|
this.appPosition = appPosition;
|
||||||
}
|
}
|
||||||
@@ -16,4 +20,8 @@ public final class PositionPair {
|
|||||||
public int getAppPosition() {
|
public int getAppPosition() {
|
||||||
return appPosition;
|
return appPosition;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public Message getMessage() {
|
||||||
|
return message;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -10,7 +10,6 @@ public class MessageFacade {
|
|||||||
private final MessageRequester requester;
|
private final MessageRequester requester;
|
||||||
private final MessageStateHolder state;
|
private final MessageStateHolder state;
|
||||||
private final MessageImageCombiner combiner;
|
private final MessageImageCombiner combiner;
|
||||||
private Message messagePendingDeletion;
|
|
||||||
|
|
||||||
public MessageFacade(MessageApi api, ApplicationHolder applicationHolder) {
|
public MessageFacade(MessageApi api, ApplicationHolder applicationHolder) {
|
||||||
this.applicationHolder = applicationHolder;
|
this.applicationHolder = applicationHolder;
|
||||||
@@ -34,13 +33,6 @@ public class MessageFacade {
|
|||||||
if (state.hasNext || !state.loaded) {
|
if (state.hasNext || !state.loaded) {
|
||||||
PagedMessages pagedMessages = requester.loadMore(state);
|
PagedMessages pagedMessages = requester.loadMore(state);
|
||||||
this.state.newMessages(appId, pagedMessages);
|
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);
|
return get(appId);
|
||||||
}
|
}
|
||||||
@@ -61,23 +53,21 @@ public class MessageFacade {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public synchronized void deleteLocal(Message message) {
|
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
|
// If there is already a deletion pending, that one should be executed before scheduling the
|
||||||
// next deletion.
|
// next deletion.
|
||||||
if (messagePendingDeletion != null) {
|
if (this.state.deletionPending()) commitDelete();
|
||||||
commitDelete();
|
this.state.deleteMessage(message);
|
||||||
}
|
|
||||||
messagePendingDeletion = message;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public synchronized void commitDelete() {
|
public synchronized void commitDelete() {
|
||||||
this.requester.asyncRemoveMessage(messagePendingDeletion);
|
if (this.state.deletionPending()) {
|
||||||
messagePendingDeletion = null;
|
MessageDeletion deletion = this.state.purgePendingDeletion();
|
||||||
|
this.requester.asyncRemoveMessage(deletion.getMessage());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public synchronized PositionPair undoDeleteLocal() {
|
public synchronized MessageDeletion undoDeleteLocal() {
|
||||||
messagePendingDeletion = null;
|
return this.state.undoPendingDeletion();
|
||||||
return this.state.undoLastRemoveMessage();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public boolean deleteAll(Integer appId) {
|
public boolean deleteAll(Integer appId) {
|
||||||
|
|||||||
@@ -9,9 +9,7 @@ class MessageStateHolder {
|
|||||||
private int lastReceivedMessage = -1;
|
private int lastReceivedMessage = -1;
|
||||||
private Map<Integer, MessageState> states = new HashMap<>();
|
private Map<Integer, MessageState> states = new HashMap<>();
|
||||||
|
|
||||||
private Message lastRemovedMessage;
|
private MessageDeletion pendingDeletion = null;
|
||||||
private int lastRemovedAllPosition;
|
|
||||||
private int lastRemovedAppPosition;
|
|
||||||
|
|
||||||
synchronized void clear() {
|
synchronized void clear() {
|
||||||
states = new HashMap<>();
|
states = new HashMap<>();
|
||||||
@@ -31,11 +29,24 @@ class MessageStateHolder {
|
|||||||
state.nextSince = pagedMessages.getPaging().getSince();
|
state.nextSince = pagedMessages.getPaging().getSince();
|
||||||
state.appId = appId;
|
state.appId = appId;
|
||||||
states.put(appId, state);
|
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) {
|
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);
|
addMessage(message, 0, 0);
|
||||||
lastReceivedMessage = message.getId();
|
lastReceivedMessage = message.getId();
|
||||||
|
|
||||||
|
if (deletion != null) deleteMessage(deletion.getMessage());
|
||||||
}
|
}
|
||||||
|
|
||||||
synchronized MessageState state(Integer appId) {
|
synchronized MessageState state(Integer appId) {
|
||||||
@@ -66,40 +77,48 @@ class MessageStateHolder {
|
|||||||
return lastReceivedMessage;
|
return lastReceivedMessage;
|
||||||
}
|
}
|
||||||
|
|
||||||
synchronized void removeMessage(Message message) {
|
synchronized void deleteMessage(Message message) {
|
||||||
MessageState allMessages = state(MessageState.ALL_MESSAGES);
|
MessageState allMessages = state(MessageState.ALL_MESSAGES);
|
||||||
MessageState appMessages = state(message.getAppid());
|
MessageState appMessages = state(message.getAppid());
|
||||||
|
|
||||||
|
int pendingDeletedAllPosition = -1;
|
||||||
|
int pendingDeletedAppPosition = -1;
|
||||||
|
|
||||||
if (allMessages.loaded) {
|
if (allMessages.loaded) {
|
||||||
int allPosition = allMessages.messages.indexOf(message);
|
int allPosition = allMessages.messages.indexOf(message);
|
||||||
allMessages.messages.remove(allPosition);
|
allMessages.messages.remove(allPosition);
|
||||||
lastRemovedAllPosition = allPosition;
|
pendingDeletedAllPosition = allPosition;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (appMessages.loaded) {
|
if (appMessages.loaded) {
|
||||||
int appPosition = appMessages.messages.indexOf(message);
|
int appPosition = appMessages.messages.indexOf(message);
|
||||||
appMessages.messages.remove(appPosition);
|
appMessages.messages.remove(appPosition);
|
||||||
lastRemovedAppPosition = appPosition;
|
pendingDeletedAppPosition = appPosition;
|
||||||
}
|
}
|
||||||
|
|
||||||
lastRemovedMessage = message;
|
pendingDeletion =
|
||||||
|
new MessageDeletion(message, pendingDeletedAllPosition, pendingDeletedAppPosition);
|
||||||
}
|
}
|
||||||
|
|
||||||
synchronized PositionPair undoLastRemoveMessage() {
|
synchronized MessageDeletion undoPendingDeletion() {
|
||||||
PositionPair result = null;
|
if (pendingDeletion != null)
|
||||||
|
addMessage(
|
||||||
if (lastRemovedMessage != null) {
|
pendingDeletion.getMessage(),
|
||||||
addMessage(lastRemovedMessage, lastRemovedAllPosition, lastRemovedAppPosition);
|
pendingDeletion.getAllPosition(),
|
||||||
result = new PositionPair(lastRemovedAllPosition, lastRemovedAppPosition);
|
pendingDeletion.getAppPosition());
|
||||||
|
return purgePendingDeletion();
|
||||||
lastRemovedMessage = null;
|
|
||||||
lastRemovedAllPosition = -1;
|
|
||||||
lastRemovedAppPosition = -1;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
synchronized MessageDeletion purgePendingDeletion() {
|
||||||
|
MessageDeletion result = pendingDeletion;
|
||||||
|
pendingDeletion = null;
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
boolean deletionPending() {
|
||||||
|
return pendingDeletion != null;
|
||||||
|
}
|
||||||
|
|
||||||
private void addMessage(Message message, int allPosition, int appPosition) {
|
private void addMessage(Message message, int allPosition, int appPosition) {
|
||||||
MessageState allMessages = state(MessageState.ALL_MESSAGES);
|
MessageState allMessages = state(MessageState.ALL_MESSAGES);
|
||||||
MessageState appMessages = state(message.getAppid());
|
MessageState appMessages = state(message.getAppid());
|
||||||
|
|||||||
Reference in New Issue
Block a user