From 22fd99bd704da33c6916a0c41301eb02557d70fb Mon Sep 17 00:00:00 2001
From: CyB3RC0nN0R <kske@outlook.de>
Date: Thu, 25 Jun 2020 17:00:41 +0200
Subject: [PATCH] Fix notifying the sender about a message delivery

This addresses bugs in two instances of delivery notification:
* the sender is online -> no event was sent
* the sender comes online later -> wrong status (SENT) was sent
---
 .../processors/LoginCredentialProcessor.java  | 11 ++++-
 .../server/processors/MessageProcessor.java   | 47 +++++++++++--------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/src/main/java/envoy/server/processors/LoginCredentialProcessor.java b/src/main/java/envoy/server/processors/LoginCredentialProcessor.java
index 9877f36..ad7addd 100755
--- a/src/main/java/envoy/server/processors/LoginCredentialProcessor.java
+++ b/src/main/java/envoy/server/processors/LoginCredentialProcessor.java
@@ -109,14 +109,21 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
 
 		final var pendingMessages = PersistenceManager.getInstance().getPendingMessages(user);
 		logger.fine("Sending " + pendingMessages.size() + " pending messages to " + user + "...");
+
 		for (var msg : pendingMessages) {
 			final var msgCommon = msg.toCommon();
 			if (msg.getStatus() == MessageStatus.SENT) {
+
+				// Send the message
 				writeProxy.write(socketID, msgCommon);
 				msg.received();
-				if (connectionManager.isOnline(msg.getSender().getID()))
-					writeProxy.write(connectionManager.getSocketID(msg.getSender().getID()), new MessageStatusChange(msgCommon));
 				PersistenceManager.getInstance().updateMessage(msg);
+
+				// Notify the sender about the delivery
+				if (connectionManager.isOnline(msg.getSender().getID())) {
+					msgCommon.nextStatus();
+					writeProxy.write(connectionManager.getSocketID(msg.getSender().getID()), new MessageStatusChange(msgCommon));
+				}
 			} else writeProxy.write(socketID, new MessageStatusChange(msgCommon));
 		}
 	}
diff --git a/src/main/java/envoy/server/processors/MessageProcessor.java b/src/main/java/envoy/server/processors/MessageProcessor.java
index c143d42..1f9b540 100755
--- a/src/main/java/envoy/server/processors/MessageProcessor.java
+++ b/src/main/java/envoy/server/processors/MessageProcessor.java
@@ -1,21 +1,22 @@
 package envoy.server.processors;
 
 import java.io.IOException;
-import java.time.LocalDateTime;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import javax.persistence.EntityExistsException;
 
 import envoy.data.Message;
 import envoy.data.Message.MessageStatus;
+import envoy.event.MessageStatusChange;
 import envoy.server.data.PersistenceManager;
 import envoy.server.net.ConnectionManager;
 import envoy.server.net.ObjectWriteProxy;
 import envoy.util.EnvoyLog;
 
 /**
- * This {@link ObjectProcessor} handles incoming {@link Message}s.<br>
- * <br>
+ * This {@link ObjectProcessor} handles incoming {@link Message}s.
+ * <p>
  * Project: <strong>envoy-server-standalone</strong><br>
  * File: <strong>MessageProcessor.java</strong><br>
  * Created: <strong>30.12.2019</strong><br>
@@ -26,6 +27,8 @@ import envoy.util.EnvoyLog;
  */
 public class MessageProcessor implements ObjectProcessor<Message> {
 
+	private static final PersistenceManager	persistenceManager	= PersistenceManager.getInstance();
+	private static final ConnectionManager	connectionManager	= ConnectionManager.getInstance();
 	private static final Logger logger = EnvoyLog.getLogger(MessageProcessor.class);
 
 	@Override
@@ -35,26 +38,32 @@ public class MessageProcessor implements ObjectProcessor<Message> {
 			return;
 		}
 		message.nextStatus();
-		ConnectionManager connectionManager = ConnectionManager.getInstance();
 
-		sendToUser(connectionManager, message, writeProxy);
+		// Convert to server message
+		final var serverMessage = new envoy.server.data.Message(message);
+
 		try {
-			PersistenceManager.getInstance().addMessage(new envoy.server.data.Message(message));
-		} catch (EntityExistsException e) {
-			logger.warning("Received a message with an id that already exists");
-		}
-	}
 
-	private void sendToUser(ConnectionManager connectionManager, Message message, ObjectWriteProxy writeProxy) {
-		if (connectionManager.isOnline(message.getRecipientID())) try {
-			// If recipient is online, send the message directly
-			writeProxy.write(connectionManager.getSocketID(message.getRecipientID()), message);
-			// Update the message status to RECEIVED
-			message.setReceivedDate(LocalDateTime.now());
-			message.nextStatus();
+			// Persist the message
+			persistenceManager.addMessage(serverMessage);
+
+			// Send the message to the recipient if online
+			if (connectionManager.isOnline(message.getRecipientID())) {
+				writeProxy.write(connectionManager.getSocketID(message.getRecipientID()), message);
+
+				// Increment status
+				message.nextStatus();
+				serverMessage.received();
+				persistenceManager.updateMessage(serverMessage);
+
+				// Notify the sender about the delivery
+				// Note that the exact time stamp might differ slightly
+				writeProxy.write(socketID, new MessageStatusChange(message));
+			}
+		} catch (EntityExistsException e) {
+			logger.log(Level.WARNING, "Received " + message + " with an ID that already exists!");
 		} catch (IOException e) {
-			logger.warning("Recipient online. Failed to send message" + message.getID());
-			e.printStackTrace();
+			logger.log(Level.WARNING, "Failed to deliver " + message + ":", e);
 		}
 	}