Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions api/src/org/labkey/api/data/DbSequenceManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down Expand Up @@ -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)
Expand Down
40 changes: 33 additions & 7 deletions api/src/org/labkey/api/data/NameGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ else if (commaIndex > startParen && commaIndex < endParen)
{
try
{
Integer.parseInt(startVal);
Long.parseLong(startVal);
}
catch (NumberFormatException e)
{
Expand Down Expand Up @@ -1636,7 +1636,7 @@ public static long getCounterStartValue(@Nullable String nameExpression, EntityC
{
try
{
startInd = Integer.valueOf(startIndStr);
startInd = Long.parseLong(startIndStr);
}
catch (NumberFormatException e)
{
Expand Down Expand Up @@ -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);
Expand All @@ -1812,7 +1812,7 @@ protected StringExpressionFactory.StringPart parsePart(@NotNull String expressio
{
try
{
startInd = Integer.valueOf(startIndStr);
startInd = Long.parseLong(startIndStr);
}
catch (NumberFormatException e)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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<String, Long> getNonConflictCountFn, String counterSeqPrefix)
public CounterExpressionPart(String expression, long startIndex, String counterFormatStr, boolean strictIncremental, Container container, Function<String, Long> getNonConflictCountFn, String counterSeqPrefix)
{
_prefixExpression = expression;
_parsedNameExpression = NameGenerationExpression.create(expression, false, NullValueBehavior.ReplaceNullWithBlank, true, container, null, null, false);
Expand Down Expand Up @@ -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);
Expand All @@ -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<String, Object> m = new HashMap<>();
m.put(ALIQUOTED_FROM_INPUT, aliquotedFrom);
Map<FieldKey, Object> 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<String>, List<String>> 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()
{
Expand Down
37 changes: 37 additions & 0 deletions experiment/src/client/test/integration/SampleTypeCrud.ispec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
})
})

Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,18 @@ protected Function<String, Long> 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)
{
}
}
}

Expand Down