fix(pubsub): always update lastPublished when same item is overwritten (XEP-0060 §7.1.2)

Root cause of PubSubExtIntegrationTest failure:
setLastPublishedItem() only updated the in-memory lastPublished cache
when the new item's creation date was *strictly after* the existing one.
When a publisher re-publishes an item with the same ItemID in rapid
succession (within the same millisecond, common in integration tests),
both items share the same CacheFactory.getClusterTime() value.
The after() check returns false, so lastPublished is NOT updated.
The persistence layer correctly performs an SQL UPDATE, but
getPublishedItem() short-circuits by returning the stale in-memory
lastPublished instead of going to the database, causing the test
assertion ("item equals first, not second") to fail.

Fix: add a third update condition — always update when the incoming
item has the same unique identifier (same node + same ItemID) as the
current lastPublished. Overwrites are always reflected in the cache
regardless of timestamp resolution.

Adds 3 regression tests in LeafNodeTest covering same-ID/same-time
overwrite, same-time different-ID non-overwrite, and newer-time
different-ID update.

Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com>
(cherry picked from commit ddfc525)

Author: 2 people

xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java

@@ -184,7 +184,17 @@ protected void deletingNode() {
 
     public synchronized void setLastPublishedItem(PublishedItem item)
     {
-        if ((lastPublished == null) || (item != null) && item.getCreationDate().after(lastPublished.getCreationDate())) {
+        // Always update when:
+        // 1. There is no last-published item yet (initial state).
+        // 2. The incoming item overwrites the current last-published item (same unique identifier).
+        //    XEP-0060 §7.1.2 requires the server to replace an existing item with the same ID.
+        //    Even if both items share the same creation-date (e.g. published in the same millisecond),
+        //    the in-memory cache must reflect the new payload so that getPublishedItem() does not
+        //    serve the stale first item.
+        // 3. The incoming item is strictly newer than the current last-published item.
+        if (item != null && (lastPublished == null
+                || lastPublished.getUniqueIdentifier().equals(item.getUniqueIdentifier())
+                || item.getCreationDate().after(lastPublished.getCreationDate()))) {
             Log.trace("Set last published item to: {}", item.getID());
             lastPublished = item;
         }

xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020-2023 Ignite Realtime Foundation. All rights reserved.
+ * Copyright (C) 2020-2026 Ignite Realtime Foundation. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@
 import org.junit.jupiter.api.Test;
 import org.xmpp.packet.JID;
 
+import java.util.Date;
+
 import static org.junit.jupiter.api.Assertions.*;
 
 /**
@@ -29,6 +31,17 @@
  */
 public class LeafNodeTest
 {
+    /** Creates a minimal LeafNode for use in tests. */
+    private static LeafNode createTestLeafNode() {
+        final DefaultNodeConfiguration config = new DefaultNodeConfiguration(true);
+        return new LeafNode(
+            new PubSubService.UniqueIdentifier("test-service-id"),
+            null,
+            "test-node-id",
+            new JID("unit-test@example.org"),
+            config);
+    }
+
     @Test
     public void testSerialization() throws Exception
     {
@@ -59,4 +72,93 @@ public void testSerialization() throws Exception
         assertTrue( result instanceof LeafNode );
         assertEquals( input, result );
     }
+
+    // =========================================================================
+    // setLastPublishedItem / getPublishedItem — XEP-0060 §7.1.2 compliance
+    //
+    // When a publisher publishes an item with the same ItemID as a previously
+    // published item, the server MUST overwrite the old item with the new one.
+    // The in-memory 'lastPublished' cache must reflect the new item immediately,
+    // even if both items share the same creation-date timestamp.
+    // =========================================================================
+
+    /**
+     * Regression test for XEP-0060 §7.1.2: publishing a second item with the same
+     * ItemID must update the in-memory {@code lastPublished} reference, even when
+     * the two items share the same creation-date (published within the same
+     * millisecond, as is common in fast sequential integration tests).
+     *
+     * <p>Before the fix, {@code setLastPublishedItem} only updated {@code lastPublished}
+     * when {@code item.getCreationDate().after(lastPublished.getCreationDate())} returned
+     * {@code true}. If both items had the same timestamp the old item remained cached,
+     * causing {@code getPublishedItem()} to serve the stale first-published payload.
+     */
+    @Test
+    public void testSetLastPublishedItem_SameIdSameTimestamp_UpdatesCache() {
+        final LeafNode node = createTestLeafNode();
+        final JID publisher = new JID("user@example.org");
+        final Date sharedTimestamp = new Date(1_000_000L);
+
+        // First publish: item1 with itemID="shared-id", payload conceptually "payload-1".
+        final PublishedItem item1 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp);
+        node.setLastPublishedItem(item1);
+
+        // Sanity: lastPublished should now be item1.
+        assertSame(item1, node.getLastPublishedItem(),
+            "After first publish, lastPublished must be item1");
+
+        // Second publish: item2 with SAME itemID and SAME timestamp (typical in fast tests).
+        final PublishedItem item2 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp);
+        node.setLastPublishedItem(item2);
+
+        // Assert: lastPublished must be updated to item2 (the overwriting item).
+        // Before the fix this assertion would fail because the creation-date guard
+        // prevented the update when both items shared the same timestamp.
+        assertSame(item2, node.getLastPublishedItem(),
+            "XEP-0060 §7.1.2: re-publishing with the same ItemID must update lastPublished " +
+            "even when both items have the same creation-date");
+    }
+
+    /**
+     * Complementary test: publishing an item with a different ItemID but the same
+     * timestamp must NOT overwrite {@code lastPublished}. Only items with newer
+     * timestamps (or the same ID) should replace it.
+     */
+    @Test
+    public void testSetLastPublishedItem_DifferentIdSameTimestamp_DoesNotOverwrite() {
+        final LeafNode node = createTestLeafNode();
+        final JID publisher = new JID("user@example.org");
+        final Date sharedTimestamp = new Date(1_000_000L);
+
+        final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", sharedTimestamp);
+        node.setLastPublishedItem(item1);
+
+        // A different item with a different ID but the same timestamp.
+        final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", sharedTimestamp);
+        node.setLastPublishedItem(item2);
+
+        // item1 was the last published (by time) and has a different ID,
+        // so it should remain as lastPublished (item2 is not newer).
+        assertSame(item1, node.getLastPublishedItem(),
+            "A different item published at the same timestamp must not displace lastPublished");
+    }
+
+    /**
+     * Sanity test: publishing a newer item (strictly later timestamp) with a
+     * different ID must update {@code lastPublished} as before.
+     */
+    @Test
+    public void testSetLastPublishedItem_DifferentIdNewerTimestamp_UpdatesCache() {
+        final LeafNode node = createTestLeafNode();
+        final JID publisher = new JID("user@example.org");
+
+        final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", new Date(1_000L));
+        node.setLastPublishedItem(item1);
+
+        final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", new Date(2_000L));
+        node.setLastPublishedItem(item2);
+
+        assertSame(item2, node.getLastPublishedItem(),
+            "A newer item (later timestamp, different ID) must update lastPublished");
+    }
 }