Skip to content

Commit 524367a

Browse files
committed
DeltaTrackingMap.wrap
1 parent 198ac8b commit 524367a

2 files changed

Lines changed: 108 additions & 6 deletions

File tree

api/src/org/labkey/api/collections/DeltaTrackingMap.java

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.ArrayList;
88
import java.util.Collection;
99
import java.util.Collections;
10+
import java.util.HashSet;
1011
import java.util.LinkedHashMap;
1112
import java.util.Map;
1213
import java.util.Set;
@@ -24,7 +25,7 @@
2425
* Map<String, Object> baseRow = new CaseInsensitiveHashMap<>();
2526
* baseRow.put("ColumnA", "Value1");
2627
*
27-
* DeltaTrackingMap<Object> trackedRow = new DeltaTrackingMap<>(baseRow);
28+
* DeltaTrackingMap<Object> trackedRow = DeltaTrackingMap.wrap(baseRow);
2829
*
2930
* // Updating an existing key (zero tracking overhead)
3031
* trackedRow.put("ColumnA", "NewValue");
@@ -58,6 +59,11 @@ public DeltaTrackingMap(Map<String, V> delegate)
5859
this.delegate = delegate;
5960
}
6061

62+
protected Set<String> newTrackingSet()
63+
{
64+
return new HashSet<>();
65+
}
66+
6167
@Override
6268
public V put(String key, V value)
6369
{
@@ -72,7 +78,7 @@ public V put(String key, V value)
7278
else
7379
{
7480
if (added == null)
75-
added = Sets.newCaseInsensitiveHashSet();
81+
added = newTrackingSet();
7682
added.add(key);
7783
}
7884
}
@@ -94,7 +100,7 @@ public V remove(Object key)
94100
else
95101
{
96102
if (removed == null)
97-
removed = Sets.newCaseInsensitiveHashSet();
103+
removed = newTrackingSet();
98104
removed.add(strKey);
99105
}
100106
}
@@ -202,6 +208,41 @@ public V get(Object key)
202208
return Collections.unmodifiableSet(delegate.entrySet());
203209
}
204210

211+
/**
212+
* Returns a {@link CaseInsensitive} wrapper when the delegate implements
213+
* {@link CaseInsensitiveCollection}, and a plain {@link DeltaTrackingMap} otherwise.
214+
* Prefer this over calling a constructor directly when the case-sensitivity of the
215+
* delegate is not known at compile time.
216+
*/
217+
public static <V> DeltaTrackingMap<V> wrap(Map<String, V> delegate)
218+
{
219+
if (delegate instanceof CaseInsensitiveCollection)
220+
return new DeltaTrackingMap.CaseInsensitive<>(delegate);
221+
return new DeltaTrackingMap<>(delegate);
222+
}
223+
224+
/**
225+
* A case-insensitive variant of {@link DeltaTrackingMap} that also implements
226+
* {@link CaseInsensitiveCollection}. Use this when wrapping a case-insensitive delegate such
227+
* as {@link CaseInsensitiveHashMap} so that downstream code relying on
228+
* {@code instanceof CaseInsensitiveCollection} continues to work correctly.
229+
*
230+
* @param <V> the type of mapped values
231+
*/
232+
public static class CaseInsensitive<V> extends DeltaTrackingMap<V> implements CaseInsensitiveCollection
233+
{
234+
public CaseInsensitive(Map<String, V> delegate)
235+
{
236+
super(delegate);
237+
}
238+
239+
@Override
240+
protected Set<String> newTrackingSet()
241+
{
242+
return new CaseInsensitiveHashSet();
243+
}
244+
}
245+
205246
public static class TestCase extends Assert
206247
{
207248
private static DeltaTrackingMap<String> createTracker()
@@ -210,7 +251,7 @@ private static DeltaTrackingMap<String> createTracker()
210251
baseMap.put("ExistingKey1", "Value1");
211252
baseMap.put("ExistingKey2", "Value2");
212253
baseMap.put("ExistingKey3", "Value3");
213-
return new DeltaTrackingMap<>(baseMap);
254+
return new DeltaTrackingMap.CaseInsensitive<>(baseMap);
214255
}
215256

216257
@Test
@@ -436,6 +477,58 @@ public void testNonStringRemoval()
436477
assertFalse("Removing a non-existent, non-string key should do nothing", map.hasStructuralChanges());
437478
}
438479

480+
@Test
481+
public void testCaseSensitiveDelegateTracking()
482+
{
483+
// A case-sensitive delegate must use case-sensitive tracking sets so that
484+
// differently cased variants of a key are treated as independent entries.
485+
Map<String, String> baseMap = new LinkedHashMap<>();
486+
baseMap.put("Key", "Value");
487+
DeltaTrackingMap<String> map = new DeltaTrackingMap<>(baseMap);
488+
489+
// "key" is a brand-new key in a case-sensitive map — must be tracked as an addition
490+
map.put("key", "lowerValue");
491+
assertTrue(map.hasStructuralChanges());
492+
assertEquals(1, map.getAddedKeys().size());
493+
assertTrue("Added 'key'", map.getAddedKeys().contains("key"));
494+
assertFalse("'Key' was not added", map.getAddedKeys().contains("Key"));
495+
496+
// Removing "Key" (original casing) must NOT cancel the tracking of the added "key"
497+
map.remove("Key");
498+
assertEquals("'key' (added) and 'Key' (removed) are independent", 1, map.getAddedKeys().size());
499+
assertTrue(map.getAddedKeys().contains("key"));
500+
assertEquals(1, map.getRemovedKeys().size());
501+
assertTrue(map.getRemovedKeys().contains("Key"));
502+
assertFalse("'key' was not removed", map.getRemovedKeys().contains("key"));
503+
}
504+
505+
@Test
506+
public void testEntrySetValueUpdate()
507+
{
508+
DeltaTrackingMap<String> map = createTracker();
509+
510+
// Get the entry for "ExistingKey1" and update its value directly
511+
Map.Entry<String, String> found = null;
512+
for (Map.Entry<String, String> e : map.entrySet())
513+
{
514+
if ("ExistingKey1".equals(e.getKey()))
515+
{
516+
found = e;
517+
break;
518+
}
519+
}
520+
assertNotNull(found);
521+
522+
String oldValue = found.setValue("UpdatedViaEntry");
523+
assertEquals("Value1", oldValue);
524+
assertEquals("UpdatedViaEntry", map.get("ExistingKey1"));
525+
526+
// entry.setValue() is a value-only mutation; no key was added or removed
527+
assertFalse("entry.setValue() on an existing key must not flag structural changes", map.hasStructuralChanges());
528+
assertTrue(map.getAddedKeys().isEmpty());
529+
assertTrue(map.getRemovedKeys().isEmpty());
530+
}
531+
439532
@Test
440533
public void testWithLinkedHashMap()
441534
{
@@ -480,7 +573,16 @@ public void testWithLinkedHashMap()
480573
assertEquals(3, map.size());
481574
assertFalse(map.containsKey("SecondKey"));
482575
assertTrue(map.containsKey("secondkey"));
483-
assertFalse("Tracker lost the state because case-insensitive matching cancelled out the add/remove", map.hasStructuralChanges());
576+
577+
// With a case-sensitive delegate, "secondkey" (added) and "SecondKey" (removed)
578+
// are different keys and must be tracked independently — no cancellation.
579+
assertTrue("Case-sensitive delegate: add and remove of differently-cased keys must not cancel", map.hasStructuralChanges());
580+
assertEquals(1, map.getAddedKeys().size());
581+
assertTrue(map.getAddedKeys().contains("secondkey"));
582+
assertFalse(map.getAddedKeys().contains("SecondKey"));
583+
assertEquals(1, map.getRemovedKeys().size());
584+
assertTrue(map.getRemovedKeys().contains("SecondKey"));
585+
assertFalse(map.getRemovedKeys().contains("secondkey"));
484586
}
485587
}
486588
}

api/src/org/labkey/api/data/AbstractTableInfo.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2009,7 +2009,7 @@ public void fireRowTrigger(
20092009

20102010
if (newRow != null && manageColumns)
20112011
{
2012-
trackedRow = new DeltaTrackingMap<>(newRow);
2012+
trackedRow = DeltaTrackingMap.wrap(newRow);
20132013
newRowTracked = trackedRow;
20142014
}
20152015

0 commit comments

Comments
 (0)