From e456e486768201b0938a4c12a70887439107191a Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 21 Aug 2026 17:07:26 -0700 Subject: [PATCH] GitHub Issue 1431: Cannot generate samples with long counter in the naming pattern --- .../labkey/api/data/DbSequenceManager.java | 18 ++++++++- .../org/labkey/api/data/NameGenerator.java | 40 +++++++++++++++---- .../test/integration/SampleTypeCrud.ispec.ts | 37 +++++++++++++++++ .../api/ExpIdentifiableBaseImpl.java | 13 ++++-- 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/data/DbSequenceManager.java b/api/src/org/labkey/api/data/DbSequenceManager.java index 3b321af0b15..ae56e07b881 100644 --- a/api/src/org/labkey/api/data/DbSequenceManager.java +++ b/api/src/org/labkey/api/data/DbSequenceManager.java @@ -262,13 +262,13 @@ public static void deleteAll(Container c) } - static int current(DbSequence sequence) + static long current(DbSequence sequence) { TableInfo tinfo = getTableInfo(); SQLFragment sql = new SQLFragment("SELECT Value FROM ").append(tinfo.getSelectName()).append(" WHERE RowId = ?"); sql.add(sequence.getRowId()); - Integer currentValue = executeAndMaybeReturnInteger(tinfo, sql); + Long currentValue = executeAndMaybeReturnLong(tinfo, sql); if (null == currentValue) throw new IllegalStateException("Current value for " + sequence + " was null!"); @@ -411,6 +411,20 @@ private static void execute(TableInfo tinfo, SQLFragment sql) } } + private static @Nullable Long executeAndMaybeReturnLong(TableInfo tinfo, SQLFragment sql) + { + DbScope scope = tinfo.getSchema().getScope(); + + try (Connection conn = scope.getPooledConnection()) + { + return new SqlSelector(scope, conn, sql).getObject(Long.class); + } + catch (SQLException e) + { + throw new RuntimeSQLException(e); + } + } + // Executes in a separate connection that does NOT participate in the current transaction. Always returns an int. private static int executeAndReturnInt(TableInfo tinfo, SQLFragment sql, Level level) diff --git a/api/src/org/labkey/api/data/NameGenerator.java b/api/src/org/labkey/api/data/NameGenerator.java index b7671fe9160..22be6759786 100644 --- a/api/src/org/labkey/api/data/NameGenerator.java +++ b/api/src/org/labkey/api/data/NameGenerator.java @@ -622,7 +622,7 @@ else if (commaIndex > startParen && commaIndex < endParen) { try { - Integer.parseInt(startVal); + Long.parseLong(startVal); } catch (NumberFormatException e) { @@ -1636,7 +1636,7 @@ public static long getCounterStartValue(@Nullable String nameExpression, EntityC { try { - startInd = Integer.valueOf(startIndStr); + startInd = Long.parseLong(startIndStr); } catch (NumberFormatException e) { @@ -1803,7 +1803,7 @@ protected StringExpressionFactory.StringPart parsePart(@NotNull String expressio if (counterMatcher.find()) { String namePrefixExpression = counterMatcher.group(1); - int startInd = 0; + long startInd = 0; String startIndStr = counterMatcher.group(2); String numberFormat = counterMatcher.group(3); String param = counterMatcher.group(4); @@ -1812,7 +1812,7 @@ protected StringExpressionFactory.StringPart parsePart(@NotNull String expressio { try { - startInd = Integer.valueOf(startIndStr); + startInd = Long.parseLong(startIndStr); } catch (NumberFormatException e) { @@ -1974,7 +1974,7 @@ public static class CounterExpressionPart extends StringExpressionFactory.String private final static long WITH_COUNTER_PREVIEW_VALUE = 1; private final String _prefixExpression; - private final Integer _startIndex; + private final long _startIndex; private final FieldKeyStringExpression _parsedNameExpression; @@ -1987,7 +1987,7 @@ public static class CounterExpressionPart extends StringExpressionFactory.String private final String _counterSeqPrefix; - public CounterExpressionPart(String expression, int startIndex, String counterFormatStr, boolean strictIncremental, Container container, Function getNonConflictCountFn, String counterSeqPrefix) + public CounterExpressionPart(String expression, long startIndex, String counterFormatStr, boolean strictIncremental, Container container, Function getNonConflictCountFn, String counterSeqPrefix) { _prefixExpression = expression; _parsedNameExpression = NameGenerationExpression.create(expression, false, NullValueBehavior.ReplaceNullWithBlank, true, container, null, null, false); @@ -2606,7 +2606,7 @@ public void testWithCounter() assertEquals(2, se.getDeepParsedExpression().size()); assertTrue(parsedExpressions.getFirst() instanceof NameGenerator.CounterExpressionPart); NameGenerator.CounterExpressionPart counterPart = (NameGenerator.CounterExpressionPart) parsedExpressions.getFirst(); - assertEquals((Integer) 101, counterPart._startIndex); + assertEquals(101L, counterPart._startIndex); String s = se.eval(m); assertEquals("S100..101", s); @@ -2632,6 +2632,32 @@ public void testWithCounter() } + @Test + public void testWithCounterLongStartValue() + { + // Issue 1431: a start value larger than Integer.MAX_VALUE must not be truncated + Map m = new HashMap<>(); + m.put(ALIQUOTED_FROM_INPUT, aliquotedFrom); + Map fm = toFieldKeyMap(m); + + Container c = JunitUtil.getTestContainer(); + long start = 10_000_000_000L; // > Integer.MAX_VALUE + DbSequenceManager.delete(c, COUNTER_SEQ_PREFIX + (aliquotedFrom + "Z").toLowerCase()); + + String pattern = "${${AliquotedFrom}Z:withCounter(" + start + ")}"; + Pair, List> messages = + NameGenerator.validateWithCounterSyntax(pattern, pattern.indexOf(SubstitutionValue.withCounter.name())); + assertTrue("Long start value should validate", messages.first.isEmpty()); + + FieldKeyStringExpression se = NameGenerationExpression.create( + pattern, false, NullValueBehavior.ReplaceNullWithBlank, true, c, null); + NameGenerator.CounterExpressionPart counterPart = (NameGenerator.CounterExpressionPart) se.getParsedExpression().getFirst(); + assertEquals(start, counterPart._startIndex); + + assertEquals("S100Z" + start, se.eval(m)); + assertEquals("S100Z" + (start + 1), se.eval(fm)); + } + @Test public void testNoMismatchedBraces() { diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index 90b4edf7425..fa35cbd2435 100644 --- a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts +++ b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts @@ -1452,3 +1452,40 @@ describe('Amount/Unit CRUD', () => { }); +describe('Name expression', () => { + it('GitHub Issue 1431: withCounter generate long values', async () => { + const sampleTypeWithCount = 'SampleTypeWithCounterExp'; + + const createPayload = { + kind: 'SampleSet', + domainDesign: { name: sampleTypeWithCount, fields: [{ name: 'Name' }] }, + options: { + name: sampleTypeWithCount, + metricUnit: 'g', + nameExpression: "${WCE-:withCounter(1,'00000')}", + } + }; + await server.post('property', 'createDomain', createPayload, {...topFolderOptions, ...designerReaderOptions}).expect(successfulResponse); + + // insert a sample with a name whose counter suffix exceeds Integer.MAX_VALUE, seeding the counter above the int range + await insertRows(server, [{ name: 'WCE-123456789123456789' }], 'samples', sampleTypeWithCount, topFolderOptions, editorUserOptions); + + // insert another sample without name, verify the generated name continues from the long seed + let inserted = await insertRows(server, [{description: 'withoutCounter with long value'}], 'samples', sampleTypeWithCount, topFolderOptions, editorUserOptions); + let rowId = caseInsensitive(inserted[0], 'rowId'); + let data = await ExperimentCRUDUtils.getSamplesData(server, [rowId], sampleTypeWithCount, 'Name', topFolderOptions, editorUserOptions); + expect(caseInsensitive(data[0], 'name')).toEqual('WCE-123456789123456790'); + + // repeat insert without name, verify name is correct + inserted = await insertRows(server, [{description: 'withoutCounter with long value'}], 'samples', sampleTypeWithCount, topFolderOptions, editorUserOptions); + rowId = caseInsensitive(inserted[0], 'rowId'); + data = await ExperimentCRUDUtils.getSamplesData(server, [rowId], sampleTypeWithCount, 'Name', topFolderOptions, editorUserOptions); + expect(caseInsensitive(data[0], 'name')).toEqual('WCE-123456789123456791'); + + // do import, without name, verify name is correct + await importSample(server, 'Name\tDescription\n\tgenerated via import', sampleTypeWithCount, 'IMPORT', topFolderOptions, editorUserOptions); + const importedSample = await ExperimentCRUDUtils.getSampleDataByName(server, 'WCE-123456789123456792', sampleTypeWithCount, 'Name', topFolderOptions, editorUserOptions); + expect(caseInsensitive(importedSample, 'name')).toEqual('WCE-123456789123456792'); + }) +}) + diff --git a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java index 82539972c83..c19fc6a0ef9 100644 --- a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java @@ -192,11 +192,18 @@ protected Function getMaxCounterWithPrefixFunction(TableInfo table for (String nameSuffix : nameSuffixes) { + // \d+ admits suffixes beyond Long.MAX_VALUE, so skip any that don't fit if (nameSuffix.matches("\\d+")) { - long id = Long.parseLong(nameSuffix); - if (id > max) - max = id; + try + { + long id = Long.parseLong(nameSuffix); + if (id > max) + max = id; + } + catch (NumberFormatException ignored) + { + } } }