Contact Deletion #97

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

View File

@ -21,13 +21,12 @@ import envoy.event.MessageStatusChange;
*/ */
public class Chat implements Serializable { public class Chat implements Serializable {
protected final Contact recipient;
protected transient ObservableList<Message> messages = FXCollections.observableArrayList(); protected transient ObservableList<Message> messages = FXCollections.observableArrayList();
protected int unreadAmount; protected int unreadAmount;
protected boolean disabled;
private boolean disabled; protected final Contact recipient;
kske marked this conversation as resolved
Review

Why is this private even though we permit subclassing here?

Why is this `private` even though we permit subclassing here?
Review

Because we don't need to access this in subclasses. Subclasses themselves should never disable themselves.

Because we don't need to access this in subclasses. Subclasses themselves should never disable themselves.
Review

There is a public setter though. The chat can literally be disabled from anywhere.

According to Meyer's open-closed principle (the O in SOLID), software entities should open for extension, while closed for modification. To me, this implies that subclasses should be able to access this value without the use of a getter.

There is a public setter though. The chat can literally be disabled from anywhere. According to Meyer's open-closed principle (the O in SOLID), software entities should open for extension, while closed for modification. To me, this implies that subclasses should be able to access this value without the use of a getter.
Review

The problem I have with this is that in this case it is literally intended that they are modified from outside and that they do not modify themselves.

The problem I have with this is that in this case it is literally intended that they are modified from outside and that they **do not modify themselves**.
/** /**
* Stores the last time an {@link envoy.event.IsTyping} event has been sent. * Stores the last time an {@link envoy.event.IsTyping} event has been sent.
@ -58,7 +57,13 @@ public class Chat implements Serializable {
@Override @Override
public String toString() { public String toString() {
return String.format("%s[recipient=%s,messages=%d, disabled=%b]", getClass().getSimpleName(), recipient, messages.size(), disabled); return String.format(
"%s[recipient=%s,messages=%d,disabled=%b]",
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.
getClass().getSimpleName(),
recipient,
messages.size(),
disabled
);
} }
/** /**
@ -94,7 +99,9 @@ public class Chat implements Serializable {
public void read(WriteProxy writeProxy) { public void read(WriteProxy writeProxy) {
for (int i = messages.size() - 1; i >= 0; --i) { for (int i = messages.size() - 1; i >= 0; --i) {
final var m = messages.get(i); final var m = messages.get(i);
if (m.getSenderID() == recipient.getID()) if (m.getStatus() == MessageStatus.READ) break; if (m.getSenderID() == recipient.getID())
if (m.getStatus() == MessageStatus.READ)
break;
else { else {
m.setStatus(MessageStatus.READ); m.setStatus(MessageStatus.READ);
writeProxy.writeMessageStatusChange(new MessageStatusChange(m)); writeProxy.writeMessageStatusChange(new MessageStatusChange(m));
@ -174,10 +181,10 @@ public class Chat implements Serializable {
public void lastWritingEventWasNow() { lastWritingEvent = System.currentTimeMillis(); } public void lastWritingEventWasNow() { lastWritingEvent = System.currentTimeMillis(); }
/** /**
* Determines whether messages can be sent in this chat. Should be true i.e. for * Determines whether messages can be sent in this chat. Should be {@code true}
* chats whose recipient deleted this client as a contact. * i.e. for chats whose recipient deleted this client as a contact.
* *
* @return whether this {@code Chat} has been disabled * @return whether this chat has been disabled
* @since Envoy Client v0.3-beta * @since Envoy Client v0.3-beta
*/ */
delvh marked this conversation as resolved
Review

Doesn't sound very conclusive to me.

Doesn't sound very conclusive to me.
public boolean isDisabled() { return disabled; } public boolean isDisabled() { return disabled; }
@ -186,7 +193,7 @@ public class Chat implements Serializable {
* Determines whether messages can be sent in this chat. Should be true i.e. for * Determines whether messages can be sent in this chat. Should be true i.e. for
* chats whose recipient deleted this client as a contact. * chats whose recipient deleted this client as a contact.
* *
* @param disabled the disabled to set * @param disabled whether this chat should be disabled
* @since Envoy Client v0.3-beta * @since Envoy Client v0.3-beta
*/ */
public void setDisabled(boolean disabled) { this.disabled = disabled; } public void setDisabled(boolean disabled) { this.disabled = disabled; }

View File

@ -44,7 +44,6 @@ public final class LocalDB implements EventListener {
private CacheMap cacheMap = new CacheMap(); private CacheMap cacheMap = new CacheMap();
private String authToken; private String authToken;
private boolean contactsChanged; 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;
@ -144,16 +143,29 @@ public final class LocalDB implements EventListener {
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 // Some chats have changed and should not be overwritten by the saved values
if (changedChats != null) { if (contactsChanged) {
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...)
changedChats.forEach(chat -> { final var contacts = user.getContacts();
final var chatIndex = chats.indexOf(chat);
if (chatIndex == -1) return; // Mark chats as disabled if a contact is no longer in this users contact list
else chats.set(chatIndex, chat); final var changedUserChats = chats.stream()
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.
.filter(not(chat -> contacts.contains(chat.getRecipient())))
.peek(chat -> { chat.setDisabled(true); logger.log(Level.INFO, String.format("Deleted chat with %s.", chat.getRecipient())); });
// Also update groups with a different member count
final var changedGroupChats = contacts.stream().filter(Group.class::isInstance).flatMap(group -> {
final var potentialChat = getChat(group.getID());
if (potentialChat.isEmpty()) return Stream.empty();
final var chat = potentialChat.get();
if (group.getContacts().size() != chat.getRecipient().getContacts().size()) {
logger.log(Level.INFO, "Removed one (or more) members from " + group);
return Stream.of(chat);
} else return Stream.empty();
}); });
Stream.concat(changedUserChats, changedGroupChats).forEach(chat -> chats.set(chats.indexOf(chat), chat));
// loadUserData can get called two (or more?) times during application lifecycle // loadUserData can get called two (or more?) times during application lifecycle
changedChats = null; contactsChanged = false;
} }
cacheMap = (CacheMap) in.readObject(); cacheMap = (CacheMap) in.readObject();
lastSync = (Instant) in.readObject(); lastSync = (Instant) in.readObject();
@ -213,7 +225,7 @@ public final class LocalDB implements EventListener {
* @throws IOException if the saving process failed * @throws IOException if the saving process failed
* @since Envoy Client v0.3-alpha * @since Envoy Client v0.3-alpha
*/ */
@Event(eventType = EnvoyCloseEvent.class, priority = 1000) @Event(eventType = EnvoyCloseEvent.class, priority = 500)
private synchronized void save() { private synchronized void save() {
EnvoyLog.getLogger(LocalDB.class).log(Level.FINER, "Saving local database..."); EnvoyLog.getLogger(LocalDB.class).log(Level.FINER, "Saving local database...");
@ -235,35 +247,34 @@ public final class LocalDB implements EventListener {
} }
} }
@Event(priority = 150) @Event(priority = 500)
private void onMessage(Message msg) { if (msg.getStatus() == MessageStatus.SENT) msg.nextStatus(); } private void onMessage(Message msg) { if (msg.getStatus() == MessageStatus.SENT) msg.nextStatus(); }
@Event(priority = 150) @Event(priority = 500)
private void onGroupMessage(GroupMessage msg) { private void onGroupMessage(GroupMessage msg) {
// TODO: Cancel event once EventBus is updated // TODO: Cancel event once EventBus is updated
if (msg.getStatus() == MessageStatus.WAITING || msg.getStatus() == MessageStatus.READ) if (msg.getStatus() == MessageStatus.WAITING || msg.getStatus() == MessageStatus.READ)
logger.warning("The groupMessage has the unexpected status " + msg.getStatus()); logger.warning("The groupMessage has the unexpected status " + msg.getStatus());
} }
@Event(priority = 150) @Event(priority = 500)
private void onMessageStatusChange(MessageStatusChange evt) { getMessage(evt.getID()).ifPresent(msg -> msg.setStatus(evt.get())); } private void onMessageStatusChange(MessageStatusChange evt) { getMessage(evt.getID()).ifPresent(msg -> msg.setStatus(evt.get())); }
@Event(priority = 150) @Event(priority = 500)
private void onGroupMessageStatusChange(GroupMessageStatusChange evt) { private void onGroupMessageStatusChange(GroupMessageStatusChange evt) {
this.<GroupMessage>getMessage(evt.getID()).ifPresent(msg -> msg.getMemberStatuses().replace(evt.getMemberID(), evt.get())); this.<GroupMessage>getMessage(evt.getID()).ifPresent(msg -> msg.getMemberStatuses().replace(evt.getMemberID(), evt.get()));
} }
delvh marked this conversation as resolved Outdated
Outdated
Review

As users only contains users that have locally logged in, this is unnecessary.

As `users` only contains users that have locally logged in, this is unnecessary.
@Event(priority = 150) @Event(priority = 500)
private void onUserStatusChange(UserStatusChange evt) { private void onUserStatusChange(UserStatusChange evt) {
getChat(evt.getID()).map(Chat::getRecipient).map(User.class::cast).ifPresent(u -> u.setStatus(evt.get())); getChat(evt.getID()).map(Chat::getRecipient).map(User.class::cast).ifPresent(u -> u.setStatus(evt.get()));
} }
@Event(priority = 200) @Event(priority = 500)
private void onUserOperation(UserOperation operation) { private void onUserOperation(UserOperation operation) {
final var eventUser = operation.get(); final var eventUser = operation.get();
switch (operation.getOperationType()) { switch (operation.getOperationType()) {
case ADD: case ADD:
getUsers().put(eventUser.getName(), eventUser);
Platform.runLater(() -> chats.add(0, new Chat(eventUser))); Platform.runLater(() -> chats.add(0, new Chat(eventUser)));
break; break;
case REMOVE: case REMOVE:
@ -283,10 +294,10 @@ public final class LocalDB implements EventListener {
else Platform.runLater(() -> chats.add(new GroupChat(user, newGroup))); else Platform.runLater(() -> chats.add(new GroupChat(user, newGroup)));
} }
@Event(priority = 150) @Event(priority = 500)
private void onGroupResize(GroupResize evt) { getChat(evt.getGroupID()).map(Chat::getRecipient).map(Group.class::cast).ifPresent(evt::apply); } private void onGroupResize(GroupResize evt) { getChat(evt.getGroupID()).map(Chat::getRecipient).map(Group.class::cast).ifPresent(evt::apply); }
@Event(priority = 150) @Event(priority = 500)
private void onNameChange(NameChange evt) { private void onNameChange(NameChange evt) {
chats.stream().map(Chat::getRecipient).filter(c -> c.getID() == evt.getID()).findAny().ifPresent(c -> c.setName(evt.get())); chats.stream().map(Chat::getRecipient).filter(c -> c.getID() == evt.getID()).findAny().ifPresent(c -> c.setName(evt.get()));
} }
@ -305,7 +316,7 @@ public final class LocalDB implements EventListener {
* *
* @since Envoy Client v0.2-beta * @since Envoy Client v0.2-beta
*/ */
@Event(eventType = Logout.class, priority = 100) @Event(eventType = Logout.class, priority = 50)
private void onLogout() { private void onLogout() {
autoSaver.cancel(); autoSaver.cancel();
autoSaveRestart = true; autoSaveRestart = true;
@ -340,7 +351,10 @@ 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) contactsChanged = true; } private void onContactsChangedSinceLastLogin() { contactsChanged = true; }
@Event(priority = 500)
private void onContactDisabled(ContactDisabled event) { getChat(event.get().getID()).ifPresent(chat -> chat.setDisabled(true)); }
/** /**
* @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
@ -369,39 +383,6 @@ public final class LocalDB implements EventListener {
*/ */
public Optional<Chat> getChat(long recipientID) { return chats.stream().filter(c -> c.getRecipient().getID() == recipientID).findAny(); } public Optional<Chat> getChat(long recipientID) { return chats.stream().filter(c -> c.getRecipient().getID() == recipientID).findAny(); }
/**
* Sets the given user as base of this {@code LocalDB}.
* Additionally compares his contacts with the ones returned by the server, in
* case they might have been deleted.
*
* @param user the user to set
* @since Envoy Client v0.2-alpha
*/
public void setUserAndMergeContacts(User user) {
this.user = user;
if (contactsChanged) {
final var contacts = user.getContacts();
// Mark chats as disabled if a contact is no longer in this users contact list
final var changedUserChats = chats.stream()
.filter(not(chat -> contacts.contains(chat.getRecipient())))
.peek(chat -> { chat.setDisabled(true); logger.log(Level.INFO, String.format("Marked %s as blocked.", chat.getRecipient())); });
// Also update groups with a different member count
final var changedGroupChats = contacts.stream().filter(Group.class::isInstance).flatMap(group -> {
final var potentialChat = getChat(group.getID());
if (potentialChat.isEmpty()) return Stream.empty();
final var chat = potentialChat.get();
if (group.getContacts().size() != chat.getRecipient().getContacts().size()) {
logger.log(Level.INFO, "Removed one (or more) members from " + group);
return Stream.of(chat);
} else return Stream.empty();
});
changedChats = Stream.concat(changedUserChats, changedGroupChats);
}
}
/** /**
* @return all saved {@link Chat} objects that list the client user as the * @return all saved {@link Chat} objects that list the client user as the
* sender * sender
delvh marked this conversation as resolved Outdated
Outdated
Review

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

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

View File

@ -68,7 +68,7 @@ public final class Settings implements EventListener {
* @throws IOException if an error occurs while saving the themes * @throws IOException if an error occurs while saving the themes
* @since Envoy Client v0.2-alpha * @since Envoy Client v0.2-alpha
*/ */
@Event(eventType = EnvoyCloseEvent.class, priority = 900) @Event(eventType = EnvoyCloseEvent.class)
private void save() { private void save() {
EnvoyLog.getLogger(Settings.class).log(Level.INFO, "Saving settings..."); EnvoyLog.getLogger(Settings.class).log(Level.INFO, "Saving settings...");

View File

@ -6,6 +6,7 @@ import envoy.client.data.Context;
import envoy.client.helper.ShutdownHelper; import envoy.client.helper.ShutdownHelper;
import envoy.client.ui.SceneContext.SceneInfo; import envoy.client.ui.SceneContext.SceneInfo;
import envoy.client.util.UserUtil; import envoy.client.util.UserUtil;
import envoy.data.User.UserStatus;
/** /**
* Envoy-specific implementation of the keyboard-shortcut interaction offered by * Envoy-specific implementation of the keyboard-shortcut interaction offered by
@ -40,5 +41,25 @@ public class EnvoyShortcutConfig {
() -> Context.getInstance().getSceneContext().load(SceneInfo.SETTINGS_SCENE), () -> Context.getInstance().getSceneContext().load(SceneInfo.SETTINGS_SCENE),
SceneInfo.SETTINGS_SCENE, SceneInfo.SETTINGS_SCENE,
SceneInfo.LOGIN_SCENE); SceneInfo.LOGIN_SCENE);
// Add option to change to status away
instance.addForNotExcluded(new KeyCodeCombination(KeyCode.A, KeyCombination.CONTROL_DOWN, KeyCombination.SHIFT_DOWN),
() -> UserUtil.changeStatus(UserStatus.AWAY),
SceneInfo.LOGIN_SCENE);
// Add option to change to status busy
instance.addForNotExcluded(new KeyCodeCombination(KeyCode.B, KeyCombination.CONTROL_DOWN, KeyCombination.SHIFT_DOWN),
() -> UserUtil.changeStatus(UserStatus.BUSY),
SceneInfo.LOGIN_SCENE);
// Add option to change to status offline
instance.addForNotExcluded(new KeyCodeCombination(KeyCode.F, KeyCombination.CONTROL_DOWN, KeyCombination.SHIFT_DOWN),
() -> UserUtil.changeStatus(UserStatus.OFFLINE),
SceneInfo.LOGIN_SCENE);
// Add option to change to status online
instance.addForNotExcluded(new KeyCodeCombination(KeyCode.N, KeyCombination.CONTROL_DOWN, KeyCombination.SHIFT_DOWN),
() -> UserUtil.changeStatus(UserStatus.ONLINE),
SceneInfo.LOGIN_SCENE);
} }
} }

View File

@ -0,0 +1,21 @@
package envoy.client.event;
import envoy.data.Contact;
import envoy.event.Event;
/**
* Signifies that the chat of a contact should be disabled.
*
* @author Leon Hofmeister
* @since Envoy Client v0.3-beta
*/
public class ContactDisabled extends Event<Contact> {
private static final long serialVersionUID = 1L;
/**
* @param contact the contact that should be disabled
* @since Envoy Client v0.3-beta
*/
public ContactDisabled(Contact contact) { super(contact); }
}

View File

@ -9,7 +9,6 @@ import envoy.client.data.*;
import envoy.client.event.EnvoyCloseEvent; import envoy.client.event.EnvoyCloseEvent;
import envoy.data.*; import envoy.data.*;
import envoy.event.*; import envoy.event.*;
import envoy.event.contact.ContactsChangedSinceLastLogin;
import envoy.util.*; import envoy.util.*;
import dev.kske.eventbus.*; import dev.kske.eventbus.*;
@ -30,7 +29,6 @@ public final class Client implements EventListener, Closeable {
private Socket socket; private Socket socket;
private Receiver receiver; private Receiver receiver;
private boolean online; private boolean online;
private boolean isHandShake;
// Asynchronously initialized during handshake // Asynchronously initialized during handshake
delvh marked this conversation as resolved Outdated
Outdated
Review

This should be called handshake, also it should be volatile and thus moved down into the line with rejected.

This should be called `handshake`, also it should be `volatile` and thus moved down into the line with `rejected`.
private volatile User sender; private volatile User sender;
@ -63,7 +61,7 @@ public final class Client implements EventListener, Closeable {
*/ */
public void performHandshake(LoginCredentials credentials, CacheMap cacheMap) throws TimeoutException, IOException, InterruptedException { public void performHandshake(LoginCredentials credentials, CacheMap cacheMap) throws TimeoutException, IOException, InterruptedException {
if (online) throw new IllegalStateException("Handshake has already been performed successfully"); if (online) throw new IllegalStateException("Handshake has already been performed successfully");
isHandShake = true; rejected = false;
// Establish TCP connection // Establish TCP connection
logger.log(Level.FINER, String.format("Attempting connection to server %s:%d...", config.getServer(), config.getPort())); logger.log(Level.FINER, String.format("Attempting connection to server %s:%d...", config.getServer(), config.getPort()));
@ -78,8 +76,6 @@ public final class Client implements EventListener, Closeable {
receiver.registerProcessor(User.class, sender -> this.sender = sender); receiver.registerProcessor(User.class, sender -> this.sender = sender);
receiver.registerProcessors(cacheMap.getMap()); receiver.registerProcessors(cacheMap.getMap());
rejected = false;
// Start receiver // Start receiver
receiver.start(); receiver.start();
@ -95,19 +91,17 @@ public final class Client implements EventListener, Closeable {
if (rejected) { if (rejected) {
socket.close(); socket.close();
receiver.removeAllProcessors(); receiver.removeAllProcessors();
isHandShake = false;
return; return;
} }
if (System.currentTimeMillis() - start > 5000) { if (System.currentTimeMillis() - start > 5000) {
isHandShake = false; rejected = true;
throw new TimeoutException("Did not log in after 5 seconds"); throw new TimeoutException("Did not log in after 5 seconds");
} }
Thread.sleep(500); Thread.sleep(500);
} }
delvh marked this conversation as resolved Outdated
Outdated
Review

Maybe set rejected here as well?

Maybe set `rejected` here as well?
online = true; online = true;
isHandShake = false;
logger.log(Level.INFO, "Handshake completed."); logger.log(Level.INFO, "Handshake completed.");
} }
@ -182,23 +176,10 @@ public final class Client implements EventListener, Closeable {
} }
@Event(eventType = HandshakeRejection.class, priority = 1000) @Event(eventType = HandshakeRejection.class, priority = 1000)
private void onHandshakeRejection() { private void onHandshakeRejection() { rejected = true; }
rejected = true;
isHandShake = false;
}
@Event(eventType = ContactsChangedSinceLastLogin.class, priority = 1000)
private void onContactsChangedSinceLastLogin() {
if (!isHandShake) {
logger.log(Level.WARNING, "Received a request to check contacts while not in the handshake");
// TODO: consume event once event consumption is implemented
return;
}
}
@Override @Override
@Event(eventType = EnvoyCloseEvent.class, priority = 800) @Event(eventType = EnvoyCloseEvent.class, priority = 50)
public void close() { public void close() {
if (online) { if (online) {
logger.log(Level.INFO, "Closing connection..."); logger.log(Level.INFO, "Closing connection...");

View File

@ -179,7 +179,7 @@ public final class Startup extends Application {
// Set client user in local database // Set client user in local database
final var user = client.getSender(); final var user = client.getSender();
localDB.setUserAndMergeContacts(user); localDB.setUser(user);
// Initialize chats in local database // Initialize chats in local database
try { try {

View File

@ -1,13 +1,12 @@
package envoy.client.ui.control; package envoy.client.ui.control;
import javafx.geometry.*; import javafx.geometry.*;
import javafx.scene.control.*; import javafx.scene.control.Label;
import javafx.scene.image.Image; import javafx.scene.image.Image;
import javafx.scene.layout.*; import javafx.scene.layout.*;
import envoy.client.data.*; import envoy.client.data.*;
import envoy.client.util.*; import envoy.client.util.IconUtil;
import envoy.data.User;
/** /**
* Displays a chat using a contact control for the recipient and a label for the * Displays a chat using a contact control for the recipient and a label for the
@ -17,7 +16,7 @@ import envoy.data.User;
* @author Leon Hofmeister * @author Leon Hofmeister
* @since Envoy Client v0.1-beta * @since Envoy Client v0.1-beta
*/ */
public final class ChatControl extends Label { public final class ChatControl extends HBox {
Outdated
Review

Please move all of what you have done here to a new AbstractListCell, enabling you to properly display the context menu and the background color.

Please move all of what you have done here to a new `AbstractListCell`, enabling you to properly display the context menu and the background color.
private static final Image userIcon = IconUtil.loadIconThemeSensitive("user_icon", 32), private static final Image userIcon = IconUtil.loadIconThemeSensitive("user_icon", 32),
groupIcon = IconUtil.loadIconThemeSensitive("group_icon", 32); groupIcon = IconUtil.loadIconThemeSensitive("group_icon", 32);
@ -32,35 +31,25 @@ public final class ChatControl extends Label {
setAlignment(Pos.CENTER_LEFT); setAlignment(Pos.CENTER_LEFT);
setPadding(new Insets(0, 0, 3, 0)); setPadding(new Insets(0, 0, 3, 0));
final var menu = new ContextMenu();
final var removeMI = new MenuItem();
removeMI
.setText(chat.isDisabled() ? "Delete " : chat.getRecipient() instanceof User ? "Block " : "Leave group " + chat.getRecipient().getName());
removeMI.setOnAction(chat.isDisabled() ? e -> UserUtil.deleteContact(chat.getRecipient()) : e -> UserUtil.blockContact(chat.getRecipient()));
menu.getItems().add(removeMI);
setContextMenu(menu);
final var display = new HBox();
// Profile picture // Profile picture
final var contactProfilePic = new ProfilePicImageView(chat instanceof GroupChat ? groupIcon : userIcon, 32); final var contactProfilePic = new ProfilePicImageView(chat instanceof GroupChat ? groupIcon : userIcon, 32);
display.getChildren().add(contactProfilePic); getChildren().add(contactProfilePic);
// Spacing // Spacing
final var leftSpacing = new Region(); final var leftSpacing = new Region();
leftSpacing.setPrefSize(8, 0); leftSpacing.setPrefSize(8, 0);
leftSpacing.setMinSize(8, 0); leftSpacing.setMinSize(8, 0);
leftSpacing.setMaxSize(8, 0); leftSpacing.setMaxSize(8, 0);
display.getChildren().add(leftSpacing); getChildren().add(leftSpacing);
// Contact control // Contact control
display.getChildren().add(new ContactControl(chat.getRecipient())); getChildren().add(new ContactControl(chat.getRecipient()));
// Unread messages // Unread messages
if (chat.getUnreadAmount() != 0) { if (chat.getUnreadAmount() != 0) {
final var spacing = new Region(); final var spacing = new Region();
HBox.setHgrow(spacing, Priority.ALWAYS); setHgrow(spacing, Priority.ALWAYS);
display.getChildren().add(spacing); getChildren().add(spacing);
final var unreadMessagesLabel = new Label(Integer.toString(chat.getUnreadAmount())); final var unreadMessagesLabel = new Label(Integer.toString(chat.getUnreadAmount()));
unreadMessagesLabel.setMinSize(15, 15); unreadMessagesLabel.setMinSize(15, 15);
final var vbox = new VBox(); final var vbox = new VBox();
@ -68,12 +57,8 @@ public final class ChatControl extends Label {
unreadMessagesLabel.setAlignment(Pos.CENTER); unreadMessagesLabel.setAlignment(Pos.CENTER);
unreadMessagesLabel.getStyleClass().add("unread-messages-amount"); unreadMessagesLabel.getStyleClass().add("unread-messages-amount");
vbox.getChildren().add(unreadMessagesLabel); vbox.getChildren().add(unreadMessagesLabel);
display.getChildren().add(vbox); getChildren().add(vbox);
} }
getStyleClass().add("list-element");
setGraphic(display);
// Set background depending on whether it is disabled or not
getStyleClass().add(chat.isDisabled() ? "disabled-chat" : "list-element");
} }
} }

View File

@ -162,7 +162,7 @@ public final class ChatScene implements EventListener, Restorable {
// Initialize message and user rendering // Initialize message and user rendering
messageList.setCellFactory(MessageListCell::new); messageList.setCellFactory(MessageListCell::new);
chatList.setCellFactory(new ListCellFactory<>(ChatControl::new)); chatList.setCellFactory(ChatListCell::new);
// JavaFX provides an internal way of populating the context menu of a text // JavaFX provides an internal way of populating the context menu of a text
// area. // area.
@ -268,22 +268,21 @@ 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
delvh marked this conversation as resolved Outdated
Outdated
Review

You have a typo here, it's called 'dependent'.

You have a typo here, it's called 'dependent'.
if (operation.getOperationType().equals(ElementOperation.REMOVE)) Platform.runLater(() -> disableChat(operation.get(), true)); // All ADD dependent logic resides in LocalDB
if (operation.getOperationType().equals(ElementOperation.REMOVE)) Platform.runLater(() -> disableChat(new ContactDisabled(operation.get())));
} }
@Event @Event
private void onGroupResize(GroupResize resize) { private void onGroupResize(GroupResize resize) {
delvh marked this conversation as resolved
Review

Why not use Optional#ifPresent here?

Why not use `Optional#ifPresent` here?
final var chatFound = localDB.getChat(resize.getGroupID()); final var chatFound = localDB.getChat(resize.getGroupID());
if (chatFound.isEmpty()) return; chatFound.ifPresent(chat -> Platform.runLater(() -> {
Platform.runLater(() -> {
chatList.refresh(); chatList.refresh();
// Update the top-bar status label if all conditions apply // Update the top-bar status label if all conditions apply
if (currentChat != null && currentChat.getRecipient().equals(chatFound.get().getRecipient())) if (currentChat != null && currentChat.getRecipient().equals(chat.getRecipient())) topBarStatusLabel
topBarStatusLabel.setText(chatFound.get().getRecipient().getContacts().size() + " member" .setText(chat.getRecipient().getContacts().size() + " member" + (currentChat.getRecipient().getContacts().size() != 1 ? "s" : ""));
+ (currentChat.getRecipient().getContacts().size() != 1 ? "s" : "")); }));
});
} }
@Event(eventType = NoAttachments.class) @Event(eventType = NoAttachments.class)
@ -331,7 +330,7 @@ public final class ChatScene implements EventListener, Restorable {
*/ */
@FXML @FXML
private void chatListClicked() { private void chatListClicked() {
if (currentChat != null && chatList.getSelectionModel().isEmpty()) return; if (chatList.getSelectionModel().isEmpty()) return;
final var chat = chatList.getSelectionModel().getSelectedItem(); final var chat = chatList.getSelectionModel().getSelectedItem();
delvh marked this conversation as resolved
Review

currentChat != null is unnecessary here.

`currentChat != null` is unnecessary here.
Review

Let's hope I' ll stay unable to reproduce what I once have produced...

Let's hope I' ll stay unable to reproduce what I once have produced...
if (chat == null) return; if (chat == null) return;
@ -376,12 +375,12 @@ public final class ChatScene implements EventListener, Restorable {
if (currentChat != null) { if (currentChat != null) {
topBarContactLabel.setText(currentChat.getRecipient().getName()); topBarContactLabel.setText(currentChat.getRecipient().getName());
topBarContactLabel.setVisible(true); topBarContactLabel.setVisible(true);
topBarStatusLabel.setVisible(true);
if (currentChat.getRecipient() instanceof User) { if (currentChat.getRecipient() instanceof User) {
final var status = ((User) currentChat.getRecipient()).getStatus().toString(); final var status = ((User) currentChat.getRecipient()).getStatus().toString();
topBarStatusLabel.setText(status); topBarStatusLabel.setText(status);
topBarStatusLabel.getStyleClass().clear(); topBarStatusLabel.getStyleClass().clear();
topBarStatusLabel.getStyleClass().add(status.toLowerCase()); topBarStatusLabel.getStyleClass().add(status.toLowerCase());
topBarStatusLabel.setVisible(true);
recipientProfilePic.setImage(IconUtil.loadIconThemeSensitive("user_icon", 43)); recipientProfilePic.setImage(IconUtil.loadIconThemeSensitive("user_icon", 43));
delvh marked this conversation as resolved Outdated
Outdated
Review

This is unnecessary (your words).

This is unnecessary (your words).
} else { } else {
topBarStatusLabel.setText(currentChat.getRecipient().getContacts().size() + " member" topBarStatusLabel.setText(currentChat.getRecipient().getContacts().size() + " member"
@ -395,11 +394,7 @@ public final class ChatScene implements EventListener, Restorable {
clip.setArcHeight(43); clip.setArcHeight(43);
clip.setArcWidth(43); clip.setArcWidth(43);
recipientProfilePic.setClip(clip); recipientProfilePic.setClip(clip);
messageList.getStyleClass().clear();
messageSearchButton.setVisible(true); messageSearchButton.setVisible(true);
// Change the background of the message list if the chat is disabled
if (currentChat.isDisabled()) messageList.getStyleClass().add("disabled-chat");
} }
} }
@ -749,18 +744,17 @@ public final class ChatScene implements EventListener, Restorable {
* Redesigns the UI when the {@link Chat} of the given contact has been marked * Redesigns the UI when the {@link Chat} of the given contact has been marked
* as disabled. * as disabled.
* *
* @param recipient the contact whose chat got disabled * @param event the contact whose chat got disabled
* @param refreshChatList whether to refresh the chatList
* @since Envoy Client v0.3-beta * @since Envoy Client v0.3-beta
*/ */
public void disableChat(Contact recipient, boolean refreshChatList) { @Event
if (refreshChatList) { public void disableChat(ContactDisabled event) {
chatList.refresh(); chatList.refresh();
final var recipient = event.get();
// Decrement member count for groups // Decrement member count for groups
if (recipient instanceof Group) if (recipient instanceof Group)
topBarStatusLabel.setText(recipient.getContacts().size() + " member" + (recipient.getContacts().size() != 1 ? "s" : "")); topBarStatusLabel.setText(recipient.getContacts().size() + " member" + (recipient.getContacts().size() != 1 ? "s" : ""));
}
if (currentChat != null && currentChat.getRecipient().equals(recipient)) { if (currentChat != null && currentChat.getRecipient().equals(recipient)) {
messageTextArea.setDisable(true); messageTextArea.setDisable(true);
voiceButton.setDisable(true); voiceButton.setDisable(true);
@ -792,11 +786,7 @@ public final class ChatScene implements EventListener, Restorable {
remainingChars.setVisible(false); remainingChars.setVisible(false);
pendingAttachment = null; pendingAttachment = null;
recipientProfilePic.setImage(null); recipientProfilePic.setImage(null);
if (recorder.isRecording()) recorder.cancel();
// It is a valid case that cancel can cause a NullPointerException
try {
recorder.cancel();
} catch (final NullPointerException e) {}
} }
@FXML @FXML

View File

@ -33,6 +33,7 @@ public abstract class AbstractListCell<T, U extends Node> extends ListCell<T> {
setGraphic(renderItem(item)); setGraphic(renderItem(item));
} else { } else {
setGraphic(null); setGraphic(null);
setCursor(Cursor.DEFAULT);
} }
} }

View File

@ -0,0 +1,46 @@
package envoy.client.ui.listcell;
import javafx.scene.control.*;
import envoy.client.data.*;
import envoy.client.net.Client;
import envoy.client.ui.control.ChatControl;
import envoy.client.util.UserUtil;
import envoy.data.User;
/**
* A list cell containing chats represented as chat controls.
*
* @author Leon Hofmeister
* @since Envoy Client v0.3-beta
*/
public class ChatListCell extends AbstractListCell<Chat, ChatControl> {
private static final Client client = Context.getInstance().getClient();
/**
* @param listView the list view inside of which the cell will be displayed
* @since Envoy Client v0.3-beta
*/
public ChatListCell(ListView<? extends Chat> listView) { super(listView); }
@Override
protected ChatControl renderItem(Chat chat) {
if (client.isOnline()) {
final var menu = new ContextMenu();
final var removeMI = new MenuItem();
removeMI.setText(
chat.isDisabled() ? "Delete " : chat.getRecipient() instanceof User ? "Block " : "Leave group " + chat.getRecipient().getName());
removeMI.setOnAction(
chat.isDisabled() ? e -> UserUtil.deleteContact(chat.getRecipient()) : e -> UserUtil.disableContact(chat.getRecipient()));
menu.getItems().add(removeMI);
setContextMenu(menu);
} else setContextMenu(null);
// TODO: replace with icon in ChatControl
final var chatControl = new ChatControl(chat);
if (chat.isDisabled()) chatControl.getStyleClass().add("disabled-chat");
else chatControl.getStyleClass().remove("disabled-chat");
return chatControl;
}
}

View File

@ -5,7 +5,7 @@ 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;
import envoy.client.data.*; import envoy.client.data.Context;
import envoy.client.event.*; import envoy.client.event.*;
import envoy.client.helper.*; import envoy.client.helper.*;
import envoy.client.ui.SceneContext.SceneInfo; import envoy.client.ui.SceneContext.SceneInfo;
@ -70,31 +70,24 @@ public final class UserUtil {
} }
/** /**
* Removes the given contact. Should not be confused with the method that is * Removes the given contact.
* called when the server reports that a contact has been deleted while the user
* was offline.
* *
* @param block the contact that should be removed * @param block the contact that should be removed
* @since Envoy Client v0.3-beta * @since Envoy Client v0.3-beta
* @see LocalDB#setUserAndMergeContacts(envoy.data.User)
*/ */
public static void blockContact(Contact block) { public static void disableContact(Contact block) {
if (!context.getClient().isOnline() || block == null) return; if (!context.getClient().isOnline() || block == null) return;
else { else {
final var alert = new Alert(AlertType.CONFIRMATION); final var alert = new Alert(AlertType.CONFIRMATION);
alert.setContentText("Are you sure you want to " + (block instanceof User ? "block " : "leave group ") + block.getName() + "?"); alert.setContentText("Are you sure you want to " + (block instanceof User ? "block " : "leave group ") + block.getName() + "?");
AlertHelper.confirmAction(alert, () -> { AlertHelper.confirmAction(alert, () -> {
final var isUser = block instanceof User;
context.getClient() context.getClient()
.send(block instanceof User ? new UserOperation((User) block, ElementOperation.REMOVE) .send(isUser ? new UserOperation((User) block, ElementOperation.REMOVE)
: new GroupResize(context.getLocalDB().getUser(), (Group) block, ElementOperation.REMOVE)); : new GroupResize(context.getLocalDB().getUser(), (Group) block, ElementOperation.REMOVE));
context.getLocalDB().getChat(block.getID()).ifPresent(chat -> chat.setDisabled(true)); if (!isUser) block.getContacts().remove(context.getLocalDB().getUser());
if (block instanceof User) logger.log(Level.INFO, "A user was blocked."); EventBus.getInstance().dispatch(new ContactDisabled(block));
else { logger.log(Level.INFO, isUser ? "A user was blocked." : "The user left a group.");
block.getContacts().remove(context.getLocalDB().getUser());
logger.log(Level.INFO, "The user left a group.");
}
final var controller = context.getSceneContext().getController();
if (controller instanceof ChatScene) ((ChatScene) controller).disableChat(block, true);
}); });
} }
} }

View File

@ -147,7 +147,7 @@
} }
.disabled-chat { .disabled-chat {
-fx-background-color: #F04000; -fx-background-color: #0000FF;
} }
#quick-select-list .scroll-bar:horizontal{ #quick-select-list .scroll-bar:horizontal{

View File

@ -105,7 +105,7 @@ public final class SerializationUtils {
file.createNewFile(); file.createNewFile();
} }
try (ObjectOutputStream out = new ObjectOutputStream(new FileOutputStream(file))) { try (ObjectOutputStream out = new ObjectOutputStream(new FileOutputStream(file))) {
for (final var obj : objs) for (var obj : objs)
delvh marked this conversation as resolved Outdated
Outdated
Review

Please reset this class to develop.

Please reset this class to `develop`.
out.writeObject(obj); out.writeObject(obj);
} }
} }
@ -119,7 +119,7 @@ public final class SerializationUtils {
* @since Envoy Common v0.2-alpha * @since Envoy Common v0.2-alpha
*/ */
public static byte[] writeToByteArray(Object obj) throws IOException { public static byte[] writeToByteArray(Object obj) throws IOException {
final var baos = new ByteArrayOutputStream(); ByteArrayOutputStream baos = new ByteArrayOutputStream();
try (ObjectOutputStream oout = new ObjectOutputStream(baos)) { try (ObjectOutputStream oout = new ObjectOutputStream(baos)) {
oout.writeObject(obj); oout.writeObject(obj);
} }
@ -137,10 +137,10 @@ public final class SerializationUtils {
*/ */
public static void writeBytesWithLength(Object obj, OutputStream out) throws IOException { public static void writeBytesWithLength(Object obj, OutputStream out) throws IOException {
// Serialize object to byte array // Serialize object to byte array
final var objBytes = writeToByteArray(obj); byte[] objBytes = writeToByteArray(obj);
// Get length of byte array in bytes // Get length of byte array in bytes
final var objLen = intToBytes(objBytes.length); byte[] objLen = intToBytes(objBytes.length);
// Write length and byte array // Write length and byte array
out.write(objLen); out.write(objLen);