From ba7ad450e405b2165ece8a739cabfe84420f1d99 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 22 Jan 2025 21:58:31 +0100 Subject: [PATCH] OF-2437: Introduce CachingPubsubPersistenceProviderTest This new unit test verifies behavior of CachingPubsubPersistenceProvider (which fails, as indicated by some of these test failing). The tests introduced here only cover one (of three) types of operations that are being scheduled by the implementation: - Node creation/changing/removal These two other types of operation still need to be added: - Affiliation creation/update/removal - Subscription creation/update/removal --- .../CachingPubsubPersistenceProvider.java | 4 +- .../CachingPubsubPersistenceProviderTest.java | 393 ++++++++++++++++++ 2 files changed, 396 insertions(+), 1 deletion(-) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProviderTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProvider.java b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProvider.java index 405e337f25..c9ca57bafb 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProvider.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProvider.java @@ -15,6 +15,7 @@ */ package org.jivesoftware.openfire.pubsub; +import com.google.common.annotations.VisibleForTesting; import org.jivesoftware.openfire.cluster.ClusterManager; import org.jivesoftware.openfire.pep.PEPService; import org.jivesoftware.openfire.pubsub.cluster.FlushTask; @@ -82,7 +83,8 @@ public class CachingPubsubPersistenceProvider implements PubSubPersistenceProvid */ private final HashMap itemsPending = new HashMap<>(); - private ConcurrentMap> nodesToProcess = new ConcurrentHashMap<>(); + @VisibleForTesting + final ConcurrentMap> nodesToProcess = new ConcurrentHashMap<>(); /** * Cache name for recently accessed published items. diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProviderTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProviderTest.java new file mode 100644 index 0000000000..99b08baef2 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/CachingPubsubPersistenceProviderTest.java @@ -0,0 +1,393 @@ +/* + * Copyright (C) 2025 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.pubsub; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.xmpp.packet.JID; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Verifies the implementation of {@link CachingPubsubPersistenceProvider} + * + * @author Guus der Kinderen, guus@goodbytes.nl + */ +@ExtendWith(MockitoExtension.class) +public class CachingPubsubPersistenceProviderTest +{ + /** + * Asserts that an invocation of {@link CachingPubsubPersistenceProvider#createNode(Node)} causes a corresponding + * operation to be scheduled. + */ + @Test + public void testCreateNode() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.createNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.CREATE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + /** + * Asserts that an invocation of {@link CachingPubsubPersistenceProvider#updateNode(Node)} causes a corresponding + * operation to be scheduled. + */ + @Test + public void testUpdateNode() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.updateNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.UPDATE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + /** + * Asserts that an invocation of {@link CachingPubsubPersistenceProvider#removeNode(Node)} causes a corresponding + * operation to be scheduled. + */ + @Test + public void testDeleteNode() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.removeNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.REMOVE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#createNode(Node)} attempts to create the + * same node 'twice'. + * + * This tests an erroneous invocation. The design of the caching provider is such that it shouldn't try to 'clean up' + * such misbehavior. Instead, it should pass this through to the delegate, which is expected to generate an + * appropriate error response. + * + * Therefor, this test asserts that for each invocation, a corresponding operation is scheduled. + */ + @Test + public void testCreateNodeTwice() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockOrigNode = Mockito.mock(Node.class); + final Node.UniqueIdentifier nodeId = new Node.UniqueIdentifier("mock-service-1", "mock-node-a"); + Mockito.lenient().when(mockOrigNode.getUniqueIdentifier()).thenReturn(nodeId); + Mockito.lenient().when(mockOrigNode.getCreationDate()).thenReturn(new Date(0)); + final Node mockReplaceNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockReplaceNode.getUniqueIdentifier()).thenReturn(nodeId); + Mockito.lenient().when(mockReplaceNode.getCreationDate()).thenReturn(new Date(1)); + + // Execute system under test. + provider.createNode(mockOrigNode); + provider.createNode(mockReplaceNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(nodeId, id -> new ArrayList<>()); + assertEquals(2, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation firstNodeOperation = pending.get(0); + assertEquals(mockOrigNode.getUniqueIdentifier(), firstNodeOperation.node.getUniqueIdentifier()); + assertEquals(new Date(0), firstNodeOperation.node.getCreationDate()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.CREATE, firstNodeOperation.action); + assertNull(firstNodeOperation.subscription); + assertNull(firstNodeOperation.affiliate); + + final CachingPubsubPersistenceProvider.NodeOperation secondNodeOperation = pending.get(1); + assertEquals(mockOrigNode.getUniqueIdentifier(), secondNodeOperation.node.getUniqueIdentifier()); + assertNotEquals(new Date(0), secondNodeOperation.node.getCreationDate()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.CREATE, secondNodeOperation.action); + assertNull(secondNodeOperation.subscription); + assertNull(secondNodeOperation.affiliate); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#updateNode(Node)} attempts to update the + * same node 'twice'. + * + * Updates of a node are never partial. This allows the caching provider to optimize two sequential updates. + * + * Therefor, this test asserts that after each invocation, only one operation, corresponding to the last update, is + * scheduled. + */ + @Test + public void testUpdateNodeTwice() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockOrigNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockOrigNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + Mockito.lenient().when(mockOrigNode.getCreationDate()).thenReturn(new Date(0)); + final Node mockReplaceNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockReplaceNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + Mockito.lenient().when(mockReplaceNode.getCreationDate()).thenReturn(new Date(1)); + + // Execute system under test. + provider.updateNode(mockOrigNode); + provider.updateNode(mockReplaceNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockOrigNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockOrigNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertNotEquals(new Date(0), nodeOperation.node.getCreationDate()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.UPDATE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#createNode(Node)} and + * {@link CachingPubsubPersistenceProvider#removeNode(Node)} creates and immediately removes a node again. + * + * The caching provider can optimize these two operations, where the 'net effect' of them is to have no operation. + * + * Therefor, this test asserts that after each invocation, no operation is scheduled. + */ + @Test + public void testDeleteNodeAfterCreateNode() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.createNode(mockNode); + provider.removeNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(0, pending.size()); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#removeNode(Node)} and + * {@link CachingPubsubPersistenceProvider#createNode(Node)} removes (a node that is thought to preexist) and then + * recreate a node again. + * + * This test asserts that after both invocations, two corresponding operation are scheduled in that order. + */ + @Test + public void testDeleteNodeDoesNotVoidNewerCreate() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.removeNode(mockNode); + provider.createNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(2, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation firstOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), firstOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.REMOVE, firstOperation.action); + assertNull(firstOperation.subscription); + assertNull(firstOperation.affiliate); + + final CachingPubsubPersistenceProvider.NodeOperation secondOperation = pending.get(1); + assertEquals(mockNode.getUniqueIdentifier(), secondOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.CREATE, secondOperation.action); + assertNull(secondOperation.subscription); + assertNull(secondOperation.affiliate); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#createNode(Node)}, + * {@link CachingPubsubPersistenceProvider#removeNode(Node)} and {@link CachingPubsubPersistenceProvider#createNode(Node)} + * creates, then removes and then recreate a node again. + * + * The caching provider can optimize the first two operations, where the 'net effect' of them is to have no operation. + * + * Therefor, this test asserts that after all invocations, only one operation that represents the last invocation, + * is scheduled. + */ + @Test + public void testDeleteNodeDoesNotVoidNewerCreate2() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.createNode(mockNode); + provider.removeNode(mockNode); + provider.createNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.CREATE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#updateNode(Node)} and + * {@link CachingPubsubPersistenceProvider#removeNode(Node)} first updates (a node that is thought to preexist) and + * then removes a node. + * + * The caching provider can optimize the two operations, as the 'net effect' of them is to have removal. + * + * Therefor, this test asserts that after all invocations, only one operation that represents the last invocation, + * is scheduled. + */ + @Test + public void testDeleteNodeReplacesOlderUpdate() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.updateNode(mockNode); + provider.removeNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(1, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation nodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), nodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.REMOVE, nodeOperation.action); + assertNull(nodeOperation.subscription); + assertNull(nodeOperation.affiliate); + } + + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#createNode(Node)}, + * {@link CachingPubsubPersistenceProvider#updateNode(Node)} and {@link CachingPubsubPersistenceProvider#removeNode(Node)} + * a node is created, updated and immediately removes a node again. + * + * The caching provider can optimize these three operations, where the 'net effect' of them is to have no operation. + * + * Therefor, this test asserts that after each invocation, no operation is scheduled. + */ + @Test + public void testDeleteNodeVoidsOlderCreateWithUpdate() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.createNode(mockNode); + provider.updateNode(mockNode); + provider.removeNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(0, pending.size()); + } + + /** + * Executes a test that, through {@link CachingPubsubPersistenceProvider#removeNode(Node)} attempts to remove the + * same node 'twice'. + * + * This tests an erroneous invocation. The design of the caching provider is such that it shouldn't try to 'clean up' + * such misbehavior. Instead, it should pass this through to the delegate, which is expected to generate an + * appropriate error response. + * + * Therefor, this test asserts that for each invocation, a corresponding operation is scheduled. + */ + @Test + public void testDeleteNodeTwice() throws Exception + { + // Setup test fixture. + final CachingPubsubPersistenceProvider provider = new CachingPubsubPersistenceProvider(); + final Node mockNode = Mockito.mock(Node.class); + Mockito.lenient().when(mockNode.getUniqueIdentifier()).thenReturn(new Node.UniqueIdentifier("mock-service-1", "mock-node-a")); + + // Execute system under test. + provider.removeNode(mockNode); + provider.removeNode(mockNode); + + // Verify results. + final List pending = provider.nodesToProcess.computeIfAbsent(mockNode.getUniqueIdentifier(), id -> new ArrayList<>()); + assertEquals(2, pending.size()); + + final CachingPubsubPersistenceProvider.NodeOperation firstNodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), firstNodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.REMOVE, firstNodeOperation.action); + assertNull(firstNodeOperation.subscription); + assertNull(firstNodeOperation.affiliate); + + final CachingPubsubPersistenceProvider.NodeOperation secondNodeOperation = pending.get(0); + assertEquals(mockNode.getUniqueIdentifier(), secondNodeOperation.node.getUniqueIdentifier()); + assertEquals(CachingPubsubPersistenceProvider.NodeOperation.Action.REMOVE, secondNodeOperation.action); + assertNull(secondNodeOperation.subscription); + assertNull(secondNodeOperation.affiliate); + } +}