fix(trading): abort item exchange when persistence fails

RoomTrade previously caught SQLException during ownership updates but continued into the in-memory inventory and credit transfer path. That could desynchronize or duplicate trade results if the database batch failed while the live session still completed the exchange. Keep item owner mutations after the successful batch, return offered items on failed completion, and add a contract test that prevents SQL failures from falling through to the transfer path.

Emulator/src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java

@@ -58,7 +58,7 @@ public class RoomTrade {
     public synchronized void offerItem(Habbo habbo, HabboItem item) {
         RoomTradeUser user = this.getRoomTradeUserForHabbo(habbo);
 
-        if (user.getItems().contains(item))
+        if (user == null || item == null || user.getItems().contains(item))
             return;
 
         habbo.getInventory().getItemsComponent().removeHabboItem(item);
@@ -71,6 +71,9 @@ public class RoomTrade {
     public synchronized void offerMultipleItems(Habbo habbo, THashSet<HabboItem> items) {
         RoomTradeUser user = this.getRoomTradeUserForHabbo(habbo);
 
+        if (user == null || items == null)
+            return;
+
         for (HabboItem item : items) {
             if (!user.getItems().contains(item)) {
                 habbo.getInventory().getItemsComponent().removeHabboItem(item);
@@ -85,7 +88,7 @@ public class RoomTrade {
     public synchronized void removeItem(Habbo habbo, HabboItem item) {
         RoomTradeUser user = this.getRoomTradeUserForHabbo(habbo);
 
-        if (!user.getItems().contains(item))
+        if (user == null || item == null || !user.getItems().contains(item))
             return;
 
         habbo.getInventory().getItemsComponent().addItem(item);
@@ -98,6 +101,9 @@ public class RoomTrade {
     public synchronized void accept(Habbo habbo, boolean value) {
         RoomTradeUser user = this.getRoomTradeUserForHabbo(habbo);
 
+        if (user == null)
+            return;
+
         user.setAccepted(value);
 
         this.sendMessageToUsers(new TradeAcceptedComposer(user));
@@ -120,6 +126,9 @@ public class RoomTrade {
 
         RoomTradeUser user = this.getRoomTradeUserForHabbo(habbo);
 
+        if (user == null)
+            return;
+
         user.confirm();
 
         this.sendMessageToUsers(new TradeAcceptedComposer(user));
@@ -134,6 +143,12 @@ public class RoomTrade {
             if (this.tradeItems()) {
                 this.closeWindow();
                 this.sendMessageToUsers(new TradeCompleteComposer());
+            } else {
+                this.returnItems();
+                for (RoomTradeUser roomTradeUser : this.users) {
+                    roomTradeUser.clearItems();
+                }
+                this.closeWindow();
             }
 
             this.room.stopTrade(this);
@@ -188,7 +203,6 @@ public class RoomTrade {
             try (PreparedStatement statement = connection.prepareStatement("UPDATE items SET user_id = ? WHERE id = ? LIMIT 1")) {
                 try (PreparedStatement stmt = connection.prepareStatement("INSERT INTO room_trade_log_items (id, item_id, user_id) VALUES (?, ?, ?)")) {
                     for (HabboItem item : userOne.getItems()) {
-                        item.setUserId(userTwoId);
                         statement.setInt(1, userTwoId);
                         statement.setInt(2, item.getId());
                         statement.addBatch();
@@ -202,7 +216,6 @@ public class RoomTrade {
                     }
 
                     for (HabboItem item : userTwo.getItems()) {
-                        item.setUserId(userOneId);
                         statement.setInt(1, userOneId);
                         statement.setInt(2, item.getId());
                         statement.addBatch();
@@ -224,6 +237,16 @@ public class RoomTrade {
             }
         } catch (SQLException e) {
             LOGGER.error("Caught SQL exception", e);
+            this.sendMessageToUsers(new TradeClosedComposer(userOne.getHabbo().getRoomUnit().getId(), TradeClosedComposer.ITEMS_NOT_FOUND));
+            return false;
+        }
+
+        for (HabboItem item : userOne.getItems()) {
+            item.setUserId(userTwo.getHabbo().getHabboInfo().getId());
+        }
+
+        for (HabboItem item : userTwo.getItems()) {
+            item.setUserId(userOne.getHabbo().getHabboInfo().getId());
         }
 
         THashSet<HabboItem> itemsUserOne = new THashSet<>(userOne.getItems());

Emulator/src/test/java/com/eu/habbo/habbohotel/rooms/RoomTradeSafetyContractTest.java

@@ -0,0 +1,38 @@
+package com.eu.habbo.habbohotel.rooms;
+
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class RoomTradeSafetyContractTest {
+    private static String roomTradeSource() throws Exception {
+        return Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/rooms/RoomTrade.java"));
+    }
+
+    @Test
+    void sqlFailureStopsBeforeInventoryTransfer() throws Exception {
+        String source = roomTradeSource();
+        int catchIndex = source.indexOf("catch (SQLException e)");
+        int inventoryTransferIndex = source.indexOf("THashSet<HabboItem> itemsUserOne");
+
+        assertTrue(catchIndex > -1, "RoomTrade must handle SQL failures explicitly");
+        assertTrue(inventoryTransferIndex > catchIndex, "Inventory transfer should happen after SQL ownership updates");
+        assertTrue(source.substring(catchIndex, inventoryTransferIndex).contains("return false"),
+                "SQL failures must abort the trade before in-memory inventory/credit transfer");
+    }
+
+    @Test
+    void itemOwnersChangeOnlyAfterDatabaseBatchSucceeds() throws Exception {
+        String source = roomTradeSource();
+        int firstOwnerMutation = source.indexOf("item.setUserId(");
+        int batchExecution = source.indexOf("statement.executeBatch();");
+
+        assertTrue(firstOwnerMutation > -1, "RoomTrade should update in-memory item owners after commit");
+        assertTrue(batchExecution > -1, "RoomTrade should persist item owner changes with a batch update");
+        assertTrue(firstOwnerMutation > batchExecution,
+                "In-memory item owners must not change until the database batch has succeeded");
+    }
+}