Merge pull request #660 from jnelsonjava/adherence-duplicate-key

BRIDGE-3331 Fixing duplicate key error in adherence reporting
Sage-Bionetworks · May 3, 2023 · 9a79810 · 9a79810
2 parents d663e1a + b61c849
commit 9a79810
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 2 deletions.
diff --git a/src/main/java/org/sagebionetworks/bridge/models/schedules2/adherence/AdherenceState.java b/src/main/java/org/sagebionetworks/bridge/models/schedules2/adherence/AdherenceState.java
@@ -54,8 +54,12 @@ public AdherenceState(AdherenceState.Builder builder) {
         daysSinceEventByEventId = new HashMap<>();
         eventTimestampByEventId = new HashMap<>();
         streamsByStreamKey = new HashMap<>();
-        adherenceByGuid = builder.adherenceRecords.stream()
-                .collect(toMap(AdherenceRecord::getInstanceGuid, (a) -> a));
+
+        adherenceByGuid = new HashMap<>();
+
+        for (AdherenceRecord adherenceRecord : adherenceRecords) {
+            adherenceByGuid.put(adherenceRecord.getInstanceGuid(), adherenceRecord);
+        }
 
         for (StudyActivityEvent event : builder.events) {
             DateTime eventTimestamp = event.getTimestamp().withZone(zone);

diff --git a/src/test/java/org/sagebionetworks/bridge/models/schedules2/adherence/AdherenceStateTest.java b/src/test/java/org/sagebionetworks/bridge/models/schedules2/adherence/AdherenceStateTest.java
@@ -6,6 +6,7 @@
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertSame;
+import static org.testng.Assert.fail;
 
 import java.util.List;
 
@@ -215,4 +216,46 @@ public void toBuilder () {
         assertSame(copy.getAdherenceRecords(), state.getAdherenceRecords());
         assertSame(copy.getStudyStartEventId(), "event1");
     }
+
+    // BRIDGE-3331 testing bug where persistent windows led to a duplicate key error
+    // when collecting adherence records with duplicate instance GUIDs into guid into a map.
+    // Persistent time windows are ignored in adherence, so we can safely bypass the exception.
+    @Test
+    public void persistentWindowDuplicateKey() {
+        StudyActivityEvent e1 = new StudyActivityEvent.Builder()
+                .withEventId("event1")
+                .withTimestamp(EVENT_TS1)
+                .build();
+
+        StudyActivityEvent e2 = new StudyActivityEvent.Builder()
+                .withEventId("event2")
+                .withTimestamp(EVENT_TS2)
+                .build();
+
+        List<StudyActivityEvent> events = ImmutableList.of(e1, e2);
+
+        AdherenceRecord record1 = new AdherenceRecord();
+        record1.setInstanceGuid("ar1");
+        record1.setEventTimestamp(EVENT_TS1);
+
+        AdherenceRecord record2 = new AdherenceRecord();
+        record2.setInstanceGuid("ar1");
+        record2.setEventTimestamp(EVENT_TS2);
+
+        List<AdherenceRecord> adherenceRecords = ImmutableList.of(record1, record2);
+
+        // If there bug is present, there will be an exception thrown during the state build.
+        try {
+            AdherenceState adherenceState = new AdherenceState.Builder()
+                    .withMetadata(metadata)
+                    .withEvents(events)
+                    .withAdherenceRecords(adherenceRecords)
+                    .withNow(NOW)
+                    .withClientTimeZone(TEST_CLIENT_TIME_ZONE)
+                    .withStudyStartEventId("event1")
+                    .build();
+        } catch (IllegalStateException e) {
+            fail("AdherenceState failed to build due to duplicate adherence record instanceGuids.");
+        }
+    }
 }