From fc437f5e64139e20afc72e9ebdf26f7d912c3cf3 Mon Sep 17 00:00:00 2001 From: Richard Ogin Date: Thu, 3 Jul 2025 22:11:02 -0500 Subject: [PATCH 1/2] Add DestinationSetTest.java This adds several tests for the DestinationSet.removeAllExcept(Object) method, and confirms a bug exists. Signed-off-by: Richard Ogin Issue: https://github.com/nextgenhealthcare/connect/issues/5875 --- .../server/userutil/DestinationSetTest.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 server/test/com/mirth/connect/server/userutil/DestinationSetTest.java diff --git a/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java b/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java new file mode 100644 index 0000000000..5295ca8c45 --- /dev/null +++ b/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2025 Richard Ogin + +package com.mirth.connect.server.userutil; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.junit.Test; + +import com.mirth.connect.donkey.model.message.ConnectorMessage; +import com.mirth.connect.donkey.server.Constants; +import com.mirth.connect.userutil.ImmutableConnectorMessage; + +public class DestinationSetTest { + private Set createMetadataIds() { + Set metaDataIds = new HashSet<>(); + metaDataIds.add(1); + metaDataIds.add(3); + metaDataIds.add(5); + metaDataIds.add(7); + metaDataIds.add(9); + + return metaDataIds; + } + + private Map createDestinationIdMap() { + Map destinationIdMap = new HashMap<>(); + destinationIdMap.put("One", 1); + destinationIdMap.put("Three", 3); + destinationIdMap.put("Five", 5); + destinationIdMap.put("Seven", 7); + destinationIdMap.put("Nine", 9); + + return destinationIdMap; + } + + private ImmutableConnectorMessage createMessage(Map destinationIdMap, Set metadataIds) { + ConnectorMessage cm = new ConnectorMessage(); + + if(metadataIds != null) { + cm.getSourceMap().put(Constants.DESTINATION_SET_KEY, metadataIds); + } + + if(destinationIdMap != null) { + return new ImmutableConnectorMessage(cm,true, destinationIdMap); + } else { + return new ImmutableConnectorMessage(cm, true); + } + } + + @Test + public void test_removeAllExceptObject_withSourceMap_removeAllForMetadataIdWhichDoesNotExist() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.removeAllExcept("I_DONT_EXIST")); + assertTrue(metaDataIds.isEmpty()); + } + + @Test + public void test_removeAllExceptObject_withSourceMap_removeForMatchingMetadataId() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.removeAllExcept(3)); + assertTrue(metaDataIds.size() == 1); + } + + @Test + public void test_removeAllExceptObject_withSourceMap_removeForMatchingConnectorName() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.removeAllExcept("Seven")); + assertTrue(metaDataIds.size() == 1); + } + + @Test + public void test_removeAllExceptObject_noSourceMap_noRemovalForMetadataIdWhichDoesNotExist() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + + assertFalse(destinationSet.removeAllExcept("I_DONT_EXIST")); + } + + @Test + public void test_removeAllExceptObject_noSourceMap_noRemovalForMatchingMetadataId() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + + assertFalse(destinationSet.removeAllExcept(3)); + } + + @Test + public void test_removeAllExceptObject_noSourceMap_noRemovalForMatchingConnectorName() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + + assertFalse(destinationSet.removeAllExcept("Seven")); + } +} From e686d615a6751b30a2de34ef527edc5af98bcbe8 Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Fri, 4 Jul 2025 14:04:10 -0400 Subject: [PATCH 2/2] Fix DestinationSet.removeAllExcept(Object) and add additional tests The DestinationSet.removeAllExcept(Object) method takes a destination metadata id or destination name. If it's passed a reference to a destination which does not exist, then it should remove all entries from the set. This was working correctly if a number was passed to the method which did not refer to a destination which remained in the set, but not for a name. This change ensures that when a destination name does not appear in the destination set, that all destinations are removed. Additonal tests were also added to DestinationSetTest for greater coverage. Signed-off-by: Tony Germano Issue: https://github.com/nextgenhealthcare/connect/issues/5875 Reported-by: Mike Hokanson --- .../server/userutil/DestinationSet.java | 8 +- .../server/userutil/DestinationSetTest.java | 128 +++++++++++++++++- 2 files changed, 129 insertions(+), 7 deletions(-) diff --git a/server/src/com/mirth/connect/server/userutil/DestinationSet.java b/server/src/com/mirth/connect/server/userutil/DestinationSet.java index 6b19594705..738d4c350f 100644 --- a/server/src/com/mirth/connect/server/userutil/DestinationSet.java +++ b/server/src/com/mirth/connect/server/userutil/DestinationSet.java @@ -101,9 +101,9 @@ public boolean removeAllExcept(Object metaDataIdOrConnectorName) { if (metaDataIds != null) { Integer metaDataId = convertToMetaDataId(metaDataIdOrConnectorName); - if (metaDataId != null) { - return metaDataIds.retainAll(Collections.singleton(metaDataId)); - } + Set set = (metaDataId != null) ? Collections.singleton(metaDataId) : Collections.emptySet(); + + return metaDataIds.retainAll(set); } return false; @@ -165,4 +165,4 @@ private Integer convertToMetaDataId(Object metaDataIdOrConnectorName) { return null; } -} \ No newline at end of file +} diff --git a/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java b/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java index 5295ca8c45..25f390bf3d 100644 --- a/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java +++ b/server/test/com/mirth/connect/server/userutil/DestinationSetTest.java @@ -1,11 +1,14 @@ // SPDX-License-Identifier: MPL-2.0 // SPDX-FileCopyrightText: 2025 Richard Ogin +// SPDX-FileCopyrightText: 2025 Tony Germano package com.mirth.connect.server.userutil; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -43,17 +46,136 @@ private Map createDestinationIdMap() { private ImmutableConnectorMessage createMessage(Map destinationIdMap, Set metadataIds) { ConnectorMessage cm = new ConnectorMessage(); - if(metadataIds != null) { + if (metadataIds != null) { cm.getSourceMap().put(Constants.DESTINATION_SET_KEY, metadataIds); } - if(destinationIdMap != null) { - return new ImmutableConnectorMessage(cm,true, destinationIdMap); + if (destinationIdMap != null) { + return new ImmutableConnectorMessage(cm, true, destinationIdMap); } else { return new ImmutableConnectorMessage(cm, true); } } + // --- Tests for remove(Object) --- + @Test + public void test_removeObject_withSourceMap_removeForMatchingMetadataId() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.remove(3)); + assertFalse(metaDataIds.contains(3)); + assertTrue(metaDataIds.size() == 4); + } + + @Test + public void test_removeObject_withSourceMap_removeForMatchingConnectorName() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.remove("Five")); + assertFalse(metaDataIds.contains(5)); + assertTrue(metaDataIds.size() == 4); + } + + @Test + public void test_removeObject_withSourceMap_noRemovalForNonExistentName() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertFalse(destinationSet.remove("I_DONT_EXIST")); + assertTrue(metaDataIds.size() == 5); + } + + @Test + public void test_removeObject_noSourceMap_noRemoval() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + assertFalse(destinationSet.remove(3)); + } + + // --- Tests for remove(Collection) --- + @Test + public void test_removeCollection_withSourceMap_removeForMatchingItems() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + Collection toRemove = Arrays.asList(1, "Three", 99); // "99" does not exist + assertTrue(destinationSet.remove(toRemove)); + assertFalse(metaDataIds.contains(1)); + assertFalse(metaDataIds.contains(3)); + assertTrue(metaDataIds.size() == 3); + } + + @Test + public void test_removeCollection_withSourceMap_noRemovalForNonExistentItems() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + Collection toRemove = Arrays.asList(100, "OneHundred"); + assertFalse(destinationSet.remove(toRemove)); + assertTrue(metaDataIds.size() == 5); + } + + @Test + public void test_removeCollection_noSourceMap_noRemoval() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + assertFalse(destinationSet.remove(Arrays.asList(1, "Three"))); + } + + // --- Tests for removeAll() --- + @Test + public void test_removeAll_withSourceMap_removesAll() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertTrue(destinationSet.removeAll()); + assertTrue(metaDataIds.isEmpty()); + } + + @Test + public void test_removeAll_withSourceMap_returnsFalseWhenAlreadyEmpty() throws Exception { + Set metaDataIds = new HashSet<>(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + assertFalse(destinationSet.removeAll()); + } + + @Test + public void test_removeAll_noSourceMap_noRemoval() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + assertFalse(destinationSet.removeAll()); + } + + // --- Tests for removeAllExcept(Collection) --- + @Test + public void test_removeAllExceptCollection_withSourceMap_retainsMatchingItems() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + Collection toRetain = Arrays.asList(1, "Five", 99); // "99" does not exist + assertTrue(destinationSet.removeAllExcept(toRetain)); + assertTrue(metaDataIds.size() == 2); + assertTrue(metaDataIds.contains(1)); + assertTrue(metaDataIds.contains(5)); + } + + @Test + public void test_removeAllExceptCollection_withSourceMap_removesAllForNonExistentItems() throws Exception { + Set metaDataIds = createMetadataIds(); + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), metaDataIds)); + + Collection toRetain = Arrays.asList(100, "OneHundred"); + assertTrue(destinationSet.removeAllExcept(toRetain)); + assertTrue(metaDataIds.isEmpty()); + } + + @Test + public void test_removeAllExceptCollection_noSourceMap_noRemoval() throws Exception { + DestinationSet destinationSet = new DestinationSet(createMessage(createDestinationIdMap(), null)); + assertFalse(destinationSet.removeAllExcept(Arrays.asList(1, "Five"))); + } + + // --- Tests for removeAllExcept(Object) --- @Test public void test_removeAllExceptObject_withSourceMap_removeAllForMetadataIdWhichDoesNotExist() throws Exception { Set metaDataIds = createMetadataIds();