diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index b17fc536299..3d3fe1528bf 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java @@ -220,6 +220,7 @@ public RemapConverter(@NotNull TableInfo targetTable, boolean includeTitleColumn public void setIncludePkLookup(boolean includePkLookup) { _includePkLookup = includePkLookup; + _maps = null; } public ColumnInfo getPkColumn() @@ -227,17 +228,6 @@ public ColumnInfo getPkColumn() return _targetTable.getPkColumns().getFirst(); } - private Pair> pkLookupMap() - { - if (!_includePkLookup) - return null; - - if (_pkColumnLookupMap == null) - _pkColumnLookupMap = Pair.of(getPkColumn(), new HashMap<>()); - - return _pkColumnLookupMap; - } - private List>> getMaps() { if (_maps == null) @@ -281,6 +271,11 @@ public ColumnInfo getPkColumn() _titleColumnLookupMap = Triple.of(pkCol, titleColumn, new ArrayListValuedHashMap()); } } + + if (_includePkLookup) + { + _pkColumnLookupMap = Pair.of(pkCol, new HashMap<>()); + } } return _maps; } @@ -296,10 +291,9 @@ public Object mappedValue(Object k) List>> maps = getMaps(); - Pair> pkLookupMap = pkLookupMap(); - if (pkLookupMap != null) + if (_pkColumnLookupMap != null) { - Object v = fetch(pkLookupMap, k); + Object v = fetch(_pkColumnLookupMap, k); if (v != null) return v; } @@ -2239,97 +2233,6 @@ public void convertRemapTest() throws Exception } - /** Lookup fixture with one text alternate key (Value) over an integer pk (RowId); Ordinal is unique but not text, so it yields no map. */ - private EnumTableInfo remapLookupTable() - { - var core = QueryService.get().getUserSchema(TestContext.get().getUser(), JunitUtil.getTestContainer(), "core"); - return new EnumTableInfo<>(LookupValues.class, core, "fake enum", true); - } - - @Test - public void remapCacheSurvivesPkLookupToggle() - { - RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, true); - - // RemappingConvertColumn flips this before every row, so it must not discard what earlier rows resolved - converter.setIncludePkLookup(false); - - List>> maps = converter.getMaps(); - assertEquals("expected one alternate-key map, on the Value column", 1, maps.size()); - - // Seed keys no enum value can supply, so anything but a cache hit resolves to null - MultiValuedMap cache = maps.getFirst().getRight(); - Integer seeded = 42; - cache.put("seeded-hit", seeded); - cache.put("seeded-miss", converter.MISS); - - assertEquals(seeded, converter.mappedValue("seeded-hit")); - assertNull(converter.mappedValue("seeded-miss")); - - for (int i = 0; i < 3; i++) - { - converter.setIncludePkLookup(true); - converter.setIncludePkLookup(false); - } - - assertSame("toggling includePkLookup discarded the cached lookups", maps, converter.getMaps()); - assertEquals("resolved value was discarded, so every row re-queries it", seeded, converter.mappedValue("seeded-hit")); - assertNull("MISS marker was discarded, so every row re-queries the absent value", converter.mappedValue("seeded-miss")); - } - - @Test - public void remapResolutionIsStableAcrossPkLookupToggle() - { - RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, true); - converter.setIncludePkLookup(false); - - Object resolved = converter.mappedValue(LookupValues.Two.name()); - assertNotNull("expected " + LookupValues.Two + " to resolve by alternate key", resolved); - - converter.setIncludePkLookup(true); - converter.setIncludePkLookup(false); - - assertEquals(resolved, converter.mappedValue(LookupValues.Two.name())); - } - - @Test - public void remapAlternateKeyWinsWhenPkLookupIsOff() - { - RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, true); - - // Seed the two maps to disagree on one key, so the resolved value says which map was consulted - Integer key = 7; - Integer pkResolution = 7; - Integer akResolution = 99; - Map pkCache = converter.pkLookupMap().getValue(); - MultiValuedMap akCache = converter.getMaps().getFirst().getRight(); - pkCache.put(key, pkResolution); - akCache.put(key, akResolution); - - assertEquals("pk lookup should take precedence while includePkLookup is on", pkResolution, converter.mappedValue(key)); - - // The pk map survives the toggle, so this also pins that it is not consulted while the flag is off - converter.setIncludePkLookup(false); - assertEquals("alternate key should resolve while includePkLookup is off", akResolution, converter.mappedValue(key)); - } - - @Test - public void remapPkLookupMapIsRetained() - { - RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, false); - assertNull("pk lookup map should not exist while includePkLookup is off", converter.pkLookupMap()); - - converter.setIncludePkLookup(true); - Pair> pkMap = converter.pkLookupMap(); - assertNotNull(pkMap); - - converter.setIncludePkLookup(false); - assertNull(converter.pkLookupMap()); - - converter.setIncludePkLookup(true); - assertSame("pk lookup map was rebuilt rather than retained", pkMap, converter.pkLookupMap()); - } - @Test public void getFileRootSubstitutedFilePathTest() {