Contact Deletion #97

Merged
delvh merged 6 commits from f/contact-deletion into develop 2020-10-19 18:09:21 +02:00
6 changed files with 45 additions and 21 deletions
Showing only changes of commit a91181b4dd - Show all commits

View File

@ -57,7 +57,9 @@ public class Chat implements Serializable {
} }
@Override @Override
public String toString() { return String.format("%s[recipient=%s,messages=%d]", getClass().getSimpleName(), recipient, messages.size()); } public String toString() {
return String.format("%s[recipient=%s,messages=%d, disabled=%b]", getClass().getSimpleName(), recipient, messages.size(), disabled);
delvh marked this conversation as resolved Outdated
Outdated
Review

There is an unnecessary space in the template.

There is an unnecessary space in the template.
}
/** /**
* Generates a hash code based on the recipient. * Generates a hash code based on the recipient.

View File

@ -1,11 +1,14 @@
package envoy.client.data; package envoy.client.data;
import static java.util.function.Predicate.not;
import java.io.*; import java.io.*;
import java.nio.channels.*; import java.nio.channels.*;
import java.nio.file.StandardOpenOption; import java.nio.file.StandardOpenOption;
import java.time.Instant; import java.time.Instant;
import java.util.*; import java.util.*;
import java.util.logging.*; import java.util.logging.*;
import java.util.stream.Stream;
import javafx.application.Platform; import javafx.application.Platform;
import javafx.collections.*; import javafx.collections.*;
@ -35,12 +38,13 @@ public final class LocalDB implements EventListener {
// Data // Data
private User user; private User user;
private Map<String, User> users = Collections.synchronizedMap(new HashMap<>()); private Map<String, User> users = Collections.synchronizedMap(new HashMap<>());
private ObservableList<Chat> chats = FXCollections.observableArrayList(); private ObservableList<Chat> chats = FXCollections.observableArrayList();
private IDGenerator idGenerator; private IDGenerator idGenerator;
private CacheMap cacheMap = new CacheMap(); private CacheMap cacheMap = new CacheMap();
private String authToken; private String authToken;
private Set<? extends Contact> originalContacts = Set.of(); private boolean contactsChanged;
private Stream<Chat> changedChats;
delvh marked this conversation as resolved Outdated
Outdated
Review

Integrate this into loadUserData as discussed.

Integrate this into `loadUserData` as discussed.
// Auto save timer // Auto save timer
private Timer autoSaver; private Timer autoSaver;
@ -138,7 +142,19 @@ public final class LocalDB implements EventListener {
if (user == null) throw new IllegalStateException("Client user is null, cannot initialize user storage"); if (user == null) throw new IllegalStateException("Client user is null, cannot initialize user storage");
userFile = new File(dbDir, user.getID() + ".db"); userFile = new File(dbDir, user.getID() + ".db");
try (var in = new ObjectInputStream(new FileInputStream(userFile))) { try (var in = new ObjectInputStream(new FileInputStream(userFile))) {
chats = FXCollections.observableList((List<Chat>) in.readObject()); chats = FXCollections.observableList((List<Chat>) in.readObject());
// Some chats have changed and should not be overridden by the saved values
delvh marked this conversation as resolved Outdated
Outdated
Review

I think you mean 'overwritten'.

I think you mean 'overwritten'.
Outdated
Review

See here.

See [here](https://www.dict.cc/?s=%C3%BCberschrieben).
Outdated
Review

Past tense of override.

Past tense of override.
Outdated
Review

I know what the past tense of override is.

What you want to prevent here is the destruction of old data by the recording new data over it, which is also known as overwriting.

Look here for a comparison of both terms.

I know what the past tense of override is. What you want to prevent here is the destruction of old data by the recording new data over it, which is also known as overwriting. Look [here](https://wikidiff.com/overwrite/override) for a comparison of both terms.
Outdated
Review

Still, both are applicable here. My preference is overridden, yours is overwritten.
Let's give @DieGurke the vote on what stays in. (How great that we have an odd amount of members, I don't want to imagine disagreements with an even amount of members...)

Still, both are applicable here. My preference is `overridden`, yours is `overwritten`. Let's give @DieGurke the vote on what stays in. (How great that we have an odd amount of members, I don't want to imagine disagreements with an even amount of members...)
if (changedChats != null) {
changedChats.forEach(chat -> {
final var chatIndex = chats.indexOf(chat);
if (chatIndex == -1) return;
kske marked this conversation as resolved Outdated
Outdated
Review

Can this even happen? If it can, should it be logged?

Can this even happen? If it can, should it be logged?
Outdated
Review

While it could theoretically occur, even if the user deleted a group while being offline, it should not happen.
Removing it now.

While it could theoretically occur, even if the user deleted a group while being offline, it should not happen. Removing it now.
else chats.set(chatIndex, chat);
});
// loadUserData can get called two (or more?) times during application lifecycle
changedChats = null;
}
cacheMap = (CacheMap) in.readObject(); cacheMap = (CacheMap) in.readObject();
lastSync = (Instant) in.readObject(); lastSync = (Instant) in.readObject();
} finally { } finally {
@ -324,7 +340,7 @@ public final class LocalDB implements EventListener {
private void onOwnStatusChange(OwnStatusChange statusChange) { user.setStatus(statusChange.get()); } private void onOwnStatusChange(OwnStatusChange statusChange) { user.setStatus(statusChange.get()); }
@Event(eventType = ContactsChangedSinceLastLogin.class, priority = 500) @Event(eventType = ContactsChangedSinceLastLogin.class, priority = 500)
private void onContactsChangedSinceLastLogin() { if (user != null) originalContacts = user.getContacts(); } private void onContactsChangedSinceLastLogin() { if (user != null) contactsChanged = true; }
delvh marked this conversation as resolved Outdated
Outdated
Review

Please document why this is necessary, or at least give some conclusive message to the user.

Please document why this is necessary, or at least give some conclusive message to the user.
/** /**
* @return a {@code Map<String, User>} of all users stored locally with their * @return a {@code Map<String, User>} of all users stored locally with their
@ -363,13 +379,12 @@ public final class LocalDB implements EventListener {
*/ */
public void setUserAndMergeContacts(User user) { public void setUserAndMergeContacts(User user) {
this.user = user; this.user = user;
if (originalContacts.isEmpty()) return; if (contactsChanged)
else {
// mark all chats of deleted contacts // Mark chats as disabled if a contact is no longer in this users contact list
originalContacts.removeAll(user.getContacts()); changedChats = chats.stream()
originalContacts.forEach(contact -> getChat(contact.getID()).ifPresent(chat -> chat.setDisabled(true))); .filter(not(chat -> user.getContacts().contains(chat.getRecipient())))
} .peek(chat -> { chat.setDisabled(true); logger.log(Level.INFO, String.format("Marked %s as blocked.", chat.getRecipient())); });
} }
delvh marked this conversation as resolved Outdated
Outdated
Review

Maybe log 'Disabled chat ...' here instead.

Maybe log 'Disabled chat ...' here instead.
/** /**

View File

@ -272,8 +272,7 @@ public final class ChatScene implements EventListener, Restorable {
@Event @Event
private void onUserOperation(UserOperation operation) { private void onUserOperation(UserOperation operation) {
// All ADD dependant logic resides in LocalDB // All ADD dependant logic resides in LocalDB
if (operation.getOperationType().equals(ElementOperation.REMOVE)) if (operation.getOperationType().equals(ElementOperation.REMOVE)) Platform.runLater(() -> disableChat(operation.get(), true));
if (currentChat != null && currentChat.getRecipient().equals(operation.get())) Platform.runLater(this::resetState);
} }
delvh marked this conversation as resolved
Review

Why not use Optional#ifPresent here?

Why not use `Optional#ifPresent` here?
@Event @Event

View File

@ -124,5 +124,8 @@ public class ContactSearchTab implements EventListener {
} }
@FXML @FXML
private void backButtonClicked() { eventBus.dispatch(new BackEvent()); } private void backButtonClicked() {
searchBar.setText("");
eventBus.dispatch(new BackEvent());
}
} }

View File

@ -82,7 +82,7 @@ public class GroupCreationTab implements EventListener {
.map(User.class::cast) .map(User.class::cast)
.collect(Collectors.toList())); .collect(Collectors.toList()));
resizeQuickSelectSpace(0); resizeQuickSelectSpace(0);
quickSelectList.addEventFilter(MouseEvent.MOUSE_PRESSED, evt -> evt.consume()); quickSelectList.addEventFilter(MouseEvent.MOUSE_PRESSED, MouseEvent::consume);
} }
/** /**
@ -238,7 +238,7 @@ public class GroupCreationTab implements EventListener {
Platform.runLater(() -> { Platform.runLater(() -> {
switch (operation.getOperationType()) { switch (operation.getOperationType()) {
case ADD: case ADD:
userList.getItems().add((User) operation.get()); userList.getItems().add(operation.get());
break; break;
case REMOVE: case REMOVE:
userList.getItems().removeIf(operation.get()::equals); userList.getItems().removeIf(operation.get()::equals);

View File

@ -1,6 +1,6 @@
package envoy.client.util; package envoy.client.util;
import java.util.logging.Level; import java.util.logging.*;
import javafx.scene.control.Alert; import javafx.scene.control.Alert;
import javafx.scene.control.Alert.AlertType; import javafx.scene.control.Alert.AlertType;
@ -26,7 +26,8 @@ import dev.kske.eventbus.EventBus;
*/ */
public final class UserUtil { public final class UserUtil {
private static final Context context = Context.getInstance(); private static final Context context = Context.getInstance();
private static final Logger logger = EnvoyLog.getLogger(UserUtil.class);
private UserUtil() {} private UserUtil() {}
@ -46,6 +47,7 @@ public final class UserUtil {
EventBus.getInstance().dispatch(new EnvoyCloseEvent()); EventBus.getInstance().dispatch(new EnvoyCloseEvent());
EventBus.getInstance().dispatch(new Logout()); EventBus.getInstance().dispatch(new Logout());
context.getSceneContext().load(SceneInfo.LOGIN_SCENE); context.getSceneContext().load(SceneInfo.LOGIN_SCENE);
logger.log(Level.INFO, "A logout occurred.");
}); });
} }
@ -63,6 +65,7 @@ public final class UserUtil {
else { else {
EventBus.getInstance().dispatch(new OwnStatusChange(newStatus)); EventBus.getInstance().dispatch(new OwnStatusChange(newStatus));
if (context.getClient().isOnline()) context.getClient().send(new UserStatusChange(context.getLocalDB().getUser().getID(), newStatus)); if (context.getClient().isOnline()) context.getClient().send(new UserStatusChange(context.getLocalDB().getUser().getID(), newStatus));
logger.log(Level.INFO, "A manual status change occurred.");
} }
} }
@ -87,6 +90,7 @@ public final class UserUtil {
context.getLocalDB().getChat(block.getID()).ifPresent(chat -> chat.setDisabled(true)); context.getLocalDB().getChat(block.getID()).ifPresent(chat -> chat.setDisabled(true));
final var controller = context.getSceneContext().getController(); final var controller = context.getSceneContext().getController();
if (controller instanceof ChatScene) ((ChatScene) controller).disableChat(block, true); if (controller instanceof ChatScene) ((ChatScene) controller).disableChat(block, true);
logger.log(Level.INFO, "A contact was blocked.");
}); });
} }
} }
@ -108,6 +112,7 @@ public final class UserUtil {
context.getLocalDB().getChats().removeIf(chat -> chat.getRecipient().equals(delete)); context.getLocalDB().getChats().removeIf(chat -> chat.getRecipient().equals(delete));
if (context.getSceneContext().getController() instanceof ChatScene) if (context.getSceneContext().getController() instanceof ChatScene)
((ChatScene) context.getSceneContext().getController()).resetState(); ((ChatScene) context.getSceneContext().getController()).resetState();
logger.log(Level.INFO, "A contact with all his messages was deleted.");
}); });
} }
} }