From 9a5cf4ea13ac62e68ae913b43e43742dea1bbf14 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 3 Jul 2025 10:02:57 -0700 Subject: [PATCH 01/12] Issue 52657: LKSM: We shouldn't allow creating sample names that differ only in case. --- .../labkey/api/exp/api/ExperimentService.java | 2 + .../labkey/experiment/ExpDataIterators.java | 75 ++++++++++++++++++- .../api/ExpDataClassDataTableImpl.java | 5 ++ .../experiment/api/ExperimentServiceImpl.java | 11 +++ .../api/SampleTypeUpdateServiceDI.java | 6 ++ 5 files changed, 98 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 5e369154827..83e01403208 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -1155,6 +1155,8 @@ List getExpProtocolsWithParameterValue( Map> getDomainMetrics(); + boolean checkDuplicateName(@NotNull String newName, @NotNull TableInfo tableInfo); + class XarExportOptions { String _lsidRelativizer = LSID_OPTION_FOLDER_RELATIVE; diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 1bdb4385312..582ab33c544 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2370,9 +2370,15 @@ else if (isMergeOrUpdate) step2c = LoggingDataIterator.wrap(new ExpDataIterators.SampleStatusCheckIteratorBuilder(step2b, _container)); } + DataIteratorBuilder step2d = step2c; + if (canUpdateNames && !dontUpdate.contains("name")) + { + step2d = LoggingDataIterator.wrap(new ExpDataIterators.DuplicateNameCheckIteratorBuilder(step2c, _propertiesTable)); + } + // Insert into exp.data then the provisioned table // Use embargo data iterator to ensure rows are committed before being sent along Issue 26082 (row at a time, reselect rowid) - DataIteratorBuilder step3 = LoggingDataIterator.wrap(new TableInsertDataIteratorBuilder(step2c, _expTable, _container) + DataIteratorBuilder step3 = LoggingDataIterator.wrap(new TableInsertDataIteratorBuilder(step2d, _expTable, _container) .setKeyColumns(keyColumns) .setDontUpdate(dontUpdate) .setAddlSkipColumns(_excludedColumns) @@ -3155,6 +3161,73 @@ public static void incrementCounts(Map currentCounts, Map map = DataIteratorUtil.createColumnNameMap(di); + _nameCol = map.get(NAME_FIELD); + } + + @Override + public boolean next() throws BatchValidationException + { + boolean hasNext = super.next(); + if (!hasNext) + return false; + + if (_context.getErrors().hasErrors()) + return hasNext; + + if (_nameCol == null) + return hasNext; + + Object nameObj = get(_nameCol); + if (nameObj == null) + return hasNext; + + String newName = String.valueOf(nameObj); + if (StringUtils.isEmpty(newName)) + return hasNext; + + Map existingValues = getExistingRecord(); + if (existingValues != null && !existingValues.isEmpty() && existingValues.get(NAME_FIELD).equals(newName)) + return hasNext; + + if (ExperimentService.get().checkDuplicateName(newName, _tableInfo)) + _context.getErrors().addRowError(new ValidationException(String.format("Name '%s' already exist.", newName))); + + return hasNext; + } + } + public static class SampleStatusCheckIteratorBuilder implements DataIteratorBuilder { private final DataIteratorBuilder _in; diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 90408498d8b..bef3d49adeb 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1323,6 +1323,11 @@ protected Map _update(User user, Container c, Map rowStripped = new CaseInsensitiveHashMap<>(); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index c0e44cad601..d40c537d93e 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9161,6 +9161,17 @@ public Map> getDomainMetrics() return metrics; } + @Override + public boolean checkDuplicateName(@NotNull String newName, @NotNull TableInfo tableInfo) + { + SQLFragment dataRowSQL = new SQLFragment("SELECT name FROM ") + .append(tableInfo) + .append(" WHERE LOWER(name) = LOWER(?) AND name <> ?") + .add(newName) + .add(newName); + return new SqlSelector(ExperimentService.get().getSchema(), dataRowSQL).exists(); + } + private Map getImportTemplatesMetrics() { DbSchema dbSchema = CoreSchema.getInstance().getSchema(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 0e12b71b23b..1b1ee775654 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -712,6 +712,12 @@ protected Map _update(User user, Container c, Map Date: Thu, 3 Jul 2025 10:51:17 -0700 Subject: [PATCH 02/12] Deadlock as result of aliquot rollup calculation and search indexer. --- .../src/org/labkey/experiment/api/SampleTypeServiceImpl.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 1a6f7a2bad8..90c2e1e7aaa 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -118,6 +118,7 @@ import java.util.Optional; import java.util.Set; import java.util.SortedSet; +import java.util.TreeMap; import java.util.TreeSet; import java.util.function.Function; import java.util.function.Predicate; @@ -1699,7 +1700,7 @@ private Map> getSampleAliquotCounts(Collection> sampleAliquotCounts = new HashMap<>(); + Map> sampleAliquotCounts = new TreeMap<>(); // Order sample by rowId to reduce probability of deadlock with search indexer try (ResultSet rs = new SqlSelector(dbSchema, sql).getResultSet()) { while (rs.next()) @@ -1760,7 +1761,7 @@ SELECT RootMaterialRowId as rootRowId, COUNT(*) as aliquotCount } dialect.appendInClauseSql(sql, sampleIds); - Map> sampleAliquotCounts = new HashMap<>(); + Map> sampleAliquotCounts = new TreeMap<>(); // Order by rowId to reduce deadlock with search indexer try (ResultSet rs = new SqlSelector(dbSchema, sql).getResultSet()) { while (rs.next()) From f840a102b691fcabf03080c119f6f75a74e4ac92 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 3 Jul 2025 12:54:47 -0700 Subject: [PATCH 03/12] fix sql server --- .../src/org/labkey/experiment/api/ExperimentServiceImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index d40c537d93e..317f9c0d8e1 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9169,6 +9169,9 @@ public boolean checkDuplicateName(@NotNull String newName, @NotNull TableInfo ta .append(" WHERE LOWER(name) = LOWER(?) AND name <> ?") .add(newName) .add(newName); + if (tableInfo.getSqlDialect().isSqlServer()) + dataRowSQL.append(" COLLATE Latin1_General_BIN"); // force case sensitive comparison + return new SqlSelector(ExperimentService.get().getSchema(), dataRowSQL).exists(); } From 0171a1a62ab77560ca9430d130449bc3e90974fa Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 4 Jul 2025 15:19:31 -0700 Subject: [PATCH 04/12] allow renaming to case insensitive name --- api/src/org/labkey/api/exp/api/ExperimentService.java | 2 -- .../src/org/labkey/experiment/ExpDataIterators.java | 9 +++++++-- .../experiment/api/ExpDataClassDataTableImpl.java | 4 ++-- .../labkey/experiment/api/ExperimentServiceImpl.java | 10 +++++++--- .../experiment/api/SampleTypeUpdateServiceDI.java | 4 ++-- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 83e01403208..5e369154827 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -1155,8 +1155,6 @@ List getExpProtocolsWithParameterValue( Map> getDomainMetrics(); - boolean checkDuplicateName(@NotNull String newName, @NotNull TableInfo tableInfo); - class XarExportOptions { String _lsidRelativizer = LSID_OPTION_FOLDER_RELATIVE; diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 582ab33c544..c330f912b92 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -3221,8 +3221,13 @@ public boolean next() throws BatchValidationException if (existingValues != null && !existingValues.isEmpty() && existingValues.get(NAME_FIELD).equals(newName)) return hasNext; - if (ExperimentService.get().checkDuplicateName(newName, _tableInfo)) - _context.getErrors().addRowError(new ValidationException(String.format("Name '%s' already exist.", newName))); + if (ExperimentServiceImpl.get().isNameAllowed(newName, _tableInfo)) + { + String error = String.format("The name '%s' already exists.", newName); + if (_context.getInsertOption().allowUpdate) + error = String.format("The name '%s' could not be resolved. Please check the casing of the provided name.", newName); + _context.getErrors().addRowError(new ValidationException(error)); + } return hasNext; } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index bef3d49adeb..c20f37912e4 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1325,8 +1325,8 @@ protected Map _update(User user, Container c, Map> getDomainMetrics() return metrics; } - @Override - public boolean checkDuplicateName(@NotNull String newName, @NotNull TableInfo tableInfo) + public boolean isNameAllowed(@NotNull String newName, @NotNull TableInfo tableInfo) + { + return checkDuplicateName(newName, newName, tableInfo); + } + + public boolean checkDuplicateName(@NotNull String oldName, @NotNull String newName, @NotNull TableInfo tableInfo) { SQLFragment dataRowSQL = new SQLFragment("SELECT name FROM ") .append(tableInfo) .append(" WHERE LOWER(name) = LOWER(?) AND name <> ?") .add(newName) - .add(newName); + .add(oldName); if (tableInfo.getSqlDialect().isSqlServer()) dataRowSQL.append(" COLLATE Latin1_General_BIN"); // force case sensitive comparison diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 1b1ee775654..0ef7e99f68b 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -714,8 +714,8 @@ protected Map _update(User user, Container c, Map Date: Sun, 6 Jul 2025 17:33:49 -0700 Subject: [PATCH 05/12] allow renaming to case insensitive name --- .../labkey/experiment/ExpDataIterators.java | 36 ++++++++++++++++--- .../api/ExpDataClassDataTableImpl.java | 7 ++-- .../experiment/api/ExperimentServiceImpl.java | 23 ++++++++---- .../api/SampleTypeUpdateServiceDI.java | 7 ++-- 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index c330f912b92..a0d3293154c 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2373,7 +2373,7 @@ else if (isMergeOrUpdate) DataIteratorBuilder step2d = step2c; if (canUpdateNames && !dontUpdate.contains("name")) { - step2d = LoggingDataIterator.wrap(new ExpDataIterators.DuplicateNameCheckIteratorBuilder(step2c, _propertiesTable)); + step2d = LoggingDataIterator.wrap(new ExpDataIterators.DuplicateNameCheckIteratorBuilder(step2c, isUpdateUsingLsid, _propertiesTable)); } // Insert into exp.data then the provisioned table @@ -3165,10 +3165,12 @@ public static class DuplicateNameCheckIteratorBuilder implements DataIteratorBui { private final DataIteratorBuilder _in; private final TableInfo _tableInfo; + private final boolean _isUpdateUsingLsid; - public DuplicateNameCheckIteratorBuilder(@NotNull DataIteratorBuilder in, TableInfo tableInfo) + public DuplicateNameCheckIteratorBuilder(@NotNull DataIteratorBuilder in, boolean isUpdateUsingLsid, TableInfo tableInfo) { _in = in; + _isUpdateUsingLsid = isUpdateUsingLsid; _tableInfo = tableInfo; } @@ -3176,7 +3178,7 @@ public DuplicateNameCheckIteratorBuilder(@NotNull DataIteratorBuilder in, TableI public DataIterator getDataIterator(DataIteratorContext context) { DataIterator pre = _in.getDataIterator(context); - return LoggingDataIterator.wrap(new DuplicateNameCheckDataIterator(pre, context, _tableInfo)); + return LoggingDataIterator.wrap(new DuplicateNameCheckDataIterator(pre, context, _isUpdateUsingLsid, _tableInfo)); } } @@ -3185,15 +3187,19 @@ public static class DuplicateNameCheckDataIterator extends WrapperDataIterator final static String NAME_FIELD = "name"; private final DataIteratorContext _context; private final Integer _nameCol; + private final Integer _lsidCol; private final TableInfo _tableInfo; + private final boolean _isUpdateUsingLsid; - protected DuplicateNameCheckDataIterator(DataIterator di, DataIteratorContext context, TableInfo tableInfo) + protected DuplicateNameCheckDataIterator(DataIterator di, DataIteratorContext context, boolean isUpdateUsingLsid, TableInfo tableInfo) { super(di); _context = context; _tableInfo = tableInfo; + _isUpdateUsingLsid = isUpdateUsingLsid; Map map = DataIteratorUtil.createColumnNameMap(di); _nameCol = map.get(NAME_FIELD); + _lsidCol = map.get("lsid"); } @Override @@ -3221,7 +3227,27 @@ public boolean next() throws BatchValidationException if (existingValues != null && !existingValues.isEmpty() && existingValues.get(NAME_FIELD).equals(newName)) return hasNext; - if (ExperimentServiceImpl.get().isNameAllowed(newName, _tableInfo)) + boolean isNameValid = true; + if (!_context.getInsertOption().allowUpdate) // insert new + { + isNameValid = ExperimentServiceImpl.get().isValidNewOrExistingName(newName, _tableInfo, false); + } + else if (_isUpdateUsingLsid && _lsidCol != null) // update using rowId is not yet supported for DIB + { + Object lsidObj = get(_lsidCol); + if (lsidObj != null) + { + String lsid = String.valueOf(lsidObj); + if (!StringUtils.isEmpty(lsid) && !ExperimentServiceImpl.get().canRename(lsid, newName, _tableInfo)) + isNameValid = false; + } + } + else if (_context.getInsertOption().mergeRows) // merge + { + isNameValid = ExperimentServiceImpl.get().isValidNewOrExistingName(newName, _tableInfo,true); + } + + if (!isNameValid) { String error = String.format("The name '%s' already exists.", newName); if (_context.getInsertOption().allowUpdate) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index c20f37912e4..5e93115692d 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1323,11 +1323,8 @@ protected Map _update(User user, Container c, Map rowStripped = new CaseInsensitiveHashMap<>(); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 8d47ee34a79..f553f528c06 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9161,22 +9161,33 @@ public Map> getDomainMetrics() return metrics; } - public boolean isNameAllowed(@NotNull String newName, @NotNull TableInfo tableInfo) + public boolean isValidNewOrExistingName(String newOrExistingName, @NotNull TableInfo tableInfo, boolean allowExisting) { - return checkDuplicateName(newName, newName, tableInfo); + SQLFragment dataRowSQL = new SQLFragment("SELECT name FROM ") + .append(tableInfo) + .append(" WHERE LOWER(name) = LOWER(?) AND name <> ?") + .add(newOrExistingName); + if (allowExisting) // // exclude existing name for merge + { + dataRowSQL.append(" AND name <> ?").add(newOrExistingName); + if (tableInfo.getSqlDialect().isSqlServer()) + dataRowSQL.append(" COLLATE Latin1_General_BIN"); // force case sensitive comparison for <> operator to exclude existing name + } + + return !(new SqlSelector(ExperimentService.get().getSchema(), dataRowSQL).exists()); } - public boolean checkDuplicateName(@NotNull String oldName, @NotNull String newName, @NotNull TableInfo tableInfo) + public boolean canRename(@NotNull String lsid, @NotNull String newName, @NotNull TableInfo tableInfo) { SQLFragment dataRowSQL = new SQLFragment("SELECT name FROM ") .append(tableInfo) - .append(" WHERE LOWER(name) = LOWER(?) AND name <> ?") + .append(" WHERE LOWER(name) = LOWER(?) AND lsid <> ?") .add(newName) - .add(oldName); + .add(lsid); if (tableInfo.getSqlDialect().isSqlServer()) dataRowSQL.append(" COLLATE Latin1_General_BIN"); // force case sensitive comparison - return new SqlSelector(ExperimentService.get().getSchema(), dataRowSQL).exists(); + return !(new SqlSelector(ExperimentService.get().getSchema(), dataRowSQL).exists()); } private Map getImportTemplatesMetrics() diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 0ef7e99f68b..1de8d8504fe 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -712,11 +712,8 @@ protected Map _update(User user, Container c, Map Date: Sun, 6 Jul 2025 20:31:49 -0700 Subject: [PATCH 06/12] allow renaming to case insensitive name - tests --- experiment/src/org/labkey/experiment/ExpDataIterators.java | 2 +- .../org/labkey/experiment/api/ExpDataClassDataTableImpl.java | 2 +- .../src/org/labkey/experiment/api/ExperimentServiceImpl.java | 2 +- .../org/labkey/experiment/api/SampleTypeUpdateServiceDI.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index a0d3293154c..8b0bebbf85e 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -3250,7 +3250,7 @@ else if (_context.getInsertOption().mergeRows) // merge if (!isNameValid) { String error = String.format("The name '%s' already exists.", newName); - if (_context.getInsertOption().allowUpdate) + if (_context.getInsertOption().mergeRows) error = String.format("The name '%s' could not be resolved. Please check the casing of the provided name.", newName); _context.getErrors().addRowError(new ValidationException(error)); } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 5e93115692d..abee86500f8 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1323,7 +1323,7 @@ protected Map _update(User user, Container c, Map ?") + .append(" WHERE LOWER(name) = LOWER(?)") .add(newOrExistingName); if (allowExisting) // // exclude existing name for merge { diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 1de8d8504fe..2dff25314fa 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -712,7 +712,7 @@ protected Map _update(User user, Container c, Map Date: Mon, 7 Jul 2025 16:41:14 -0700 Subject: [PATCH 07/12] add test coverage --- .../test/integration/DataClassCrud.ispec.ts | 148 +++++++++++++++++- 1 file changed, 146 insertions(+), 2 deletions(-) diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index e8875f28ff9..1f7986edfd3 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -357,11 +357,11 @@ describe('Duplicate IDs', () => { }] }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { errorResp = JSON.parse(result.text); - expect(errorResp.exception.indexOf('duplicate key') > -1).toBeTruthy(); + expect(errorResp.exception.indexOf('already exists') > -1).toBeTruthy(); }); // import errorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nduplicateShouldFail\tbad\nduplicateShouldFail\tbad", dataType, "IMPORT", topFolderOptions, editorUserOptions); - expect(errorResp.text.indexOf('duplicate key') > -1).toBeTruthy(); + expect(errorResp.text.indexOf('already exists') > -1).toBeTruthy(); // merge const duplicateKeyErrorPrefix = 'Duplicate key provided: '; @@ -433,4 +433,148 @@ describe('Duplicate IDs', () => { expect(caseInsensitive(dataResults[1], 'description')).toBe('created'); }); + + it("Issue 52657: We shouldn't allow creating data names that differ only in case.", async () => { + const dataType = "Type Case Sensitive"; + const createPayload = { + kind: 'DataClass', + domainDesign: { name: dataType, fields: [{ name: 'Prop' }] }, + options: { + name: dataType, + nameExpression: 'Src-${Prop}' + } + }; + + await server.post('property', 'createDomain', createPayload, + {...topFolderOptions, ...designerReaderOptions}).expect(successfulResponse); + + const NAME_EXIST_MSG = "The name '%%' already exists."; + const data1 = 'Src-case-dAta1'; + const data2 = 'Src-case-dAta2'; + + let insertRows = [{ + name: data1, + },{ + name: data2, + }]; + const dataRows = await ExperimentCRUDUtils.insertRows(server, insertRows, 'exp.data', dataType, topFolderOptions, editorUserOptions); + const data1RowId = caseInsensitive(dataRows[0], 'rowId'); + const data1Lsid = caseInsensitive(dataRows[0], 'lsid'); + const data2RowId = caseInsensitive(dataRows[1], 'rowId'); + const data2Lsid = caseInsensitive(dataRows[1], 'lsid'); + + let expectedError = NAME_EXIST_MSG.replace('%%', 'Src-case-data1'); + // import + let errorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nSrc-case-data1\tbad\nSrc-case-data2\tbad", dataType, "IMPORT", topFolderOptions, editorUserOptions); + expect(errorResp.text).toContain(expectedError); + + // merge + let mergeError = 'The name \'Src-case-data1\' could not be resolved. Please check the casing of the provided name.'; + errorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nSrc-case-data1\tbad\nSrc-case-data2\tbad", dataType, "MERGE", topFolderOptions, editorUserOptions); + expect(errorResp.text).toContain(mergeError); + + // insert + await server.post('query', 'insertRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-data1', + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(expectedError); + }); + + // insert using naming expression to create case-insensitive name + await server.post('query', 'insertRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + prop: 'case-data1', + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(expectedError); + }); + + // renaming data to another data's name, using rowId + await server.post('query', 'updateRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-dAta2', + rowId: data1RowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'Src-case-dAta2')); + }); + + // renaming data to another data's case-insensitive name, using rowId + await server.post('query', 'updateRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-data2', + rowId: data1RowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'Src-case-data2')); + }); + + // renaming data to another data's case-insensitive name, using lsid. Currently can only be done using api + await server.post('query', 'updateRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-data2', + lsid: data1Lsid + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'Src-case-data2')); + }); + + // swap names (fail) + await server.post('query', 'updateRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-data2', + lsid: data1Lsid + }, { + name: 'Src-case-data1', + lsid: data2Lsid + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'Src-case-data2')); + }); + + await server.post('query', 'updateRows', { + schemaName: 'exp.data', + queryName: dataType, + rows: [{ + name: 'Src-case-data2', + rowId: data1RowId + }, { + name: 'Src-case-data1', + rowId: data2RowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'Src-case-data2')); + }); + + // renaming data to its case-insensitive name, using rowId + let results = await ExperimentCRUDUtils.updateRows(server, [{name: 'SRC-CASE-data1', rowId: data1RowId}], 'exp.data', dataType, topFolderOptions, editorUserOptions); + expect(caseInsensitive(results[0], 'Name')).toBe('SRC-CASE-data1'); + + // renaming data to its case-insensitive name, using lsid + results = await ExperimentCRUDUtils.updateRows(server, [{name: 'src-case-DATA1', lsid: data1Lsid}], 'exp.data', dataType, topFolderOptions, editorUserOptions); + expect(caseInsensitive(results[0], 'Name')).toBe('src-case-DATA1'); + + }); + }); \ No newline at end of file From c63481251c4aeac0621565531358f49545637723 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 7 Jul 2025 20:36:00 -0700 Subject: [PATCH 08/12] update test --- .../org/labkey/experiment/api/UniqueValueCounterTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/UniqueValueCounterTestCase.java b/experiment/src/org/labkey/experiment/api/UniqueValueCounterTestCase.java index 464dff92606..06408cc00de 100644 --- a/experiment/src/org/labkey/experiment/api/UniqueValueCounterTestCase.java +++ b/experiment/src/org/labkey/experiment/api/UniqueValueCounterTestCase.java @@ -176,7 +176,7 @@ sampTypeName, null, props, emptyList(), svc.insertRows(user, c, rows, errors, null, null); assertTrue(errors.hasErrors()); assertTrue("Expected duplicate key violation: " + errors.getMessage(), - errors.getMessage().contains("duplicate key")); + errors.getMessage().contains("already exists")); // NOTE: This test case doesn't repro for SampleType because the CoerceDataIterator is run before the CounterDataIteratorBuilder and will include null values for any missing columns From df3aa348879cdbf88915a17d64b36a3cddaf958f Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 8 Jul 2025 10:05:25 -0700 Subject: [PATCH 09/12] update duplicate key msg in tests --- .../src/client/test/integration/SampleTypeCrud.ispec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index a6ce27c54ba..e1a50e115c9 100644 --- a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts +++ b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts @@ -715,7 +715,7 @@ describe('Aliquot crud', () => { importText = "Description\tAliquotedFrom\n"; importText += aliquotDesc + "\t" + absentRootSample + "\n"; let resp = await ExperimentCRUDUtils.importSample(server, importText, SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, "IMPORT", topFolderOptions, editorUserOptions); - expect(resp.text.indexOf("Aliquot parent 'Absent_Root' not found.") > -1).toBeTruthy(); + expect(resp.text).toContain("Aliquot parent 'Absent_Root' not found."); const invalidRootSample = "Not_This_Root"; await ExperimentCRUDUtils.insertSamples(server, [{name: invalidRootSample}], SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, topFolderOptions, editorUserOptions) @@ -723,11 +723,11 @@ describe('Aliquot crud', () => { importText += aliquot01 + "\t" + aliquotDesc + "\t" + invalidRootSample + "\n"; // Validate that if the AliquotedFrom field has an invalid value the import fails. resp = await ExperimentCRUDUtils.importSample(server, importText, SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, "IMPORT", topFolderOptions, editorUserOptions); - expect(resp.text.indexOf("duplicate key") > -1).toBeTruthy(); + expect(resp.text).toContain("already exists"); // Validate that the AliquotedFrom field of an aliquot cannot be updated. resp = await ExperimentCRUDUtils.importSample(server, importText, SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, "MERGE", topFolderOptions, editorUserOptions); - expect(resp.text.indexOf("Aliquot parents cannot be updated for sample testInvalidImportCasesParent1-1.") > -1).toBeTruthy(); + expect(resp.text).toContain("Aliquot parents cannot be updated for sample testInvalidImportCasesParent1-1."); // AliquotedFrom is ignored for UPDATE option resp = await ExperimentCRUDUtils.importSample(server, importText, SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, "UPDATE", topFolderOptions, editorUserOptions); @@ -746,7 +746,7 @@ describe('Aliquot crud', () => { importText = "Name\tAliquotedFrom\n"; importText += invalidRootSample + "\t" + parentSampleName + "\n"; resp = await ExperimentCRUDUtils.importSample(server, importText, SAMPLE_ALIQUOT_IMPORT_TYPE_NAME, "MERGE", topFolderOptions, editorUserOptions); - expect(resp.text.indexOf("Unable to change sample to aliquot Not_This_Root.") > -1).toBeTruthy(); + expect(resp.text).toContain("Unable to change sample to aliquot Not_This_Root."); }); /** From ca89839e9fc61b35ca6a29f83639f990a86452f9 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 8 Jul 2025 16:50:03 -0700 Subject: [PATCH 10/12] Update error msg. Update tests --- .../api/query/AbstractQueryUpdateService.java | 13 +- .../test/integration/SampleTypeCrud.ispec.ts | 134 +++++++++++++++++- .../labkey/experiment/ExpDataIterators.java | 2 +- .../experiment/api/ExpSampleTypeTestCase.jsp | 4 +- .../api/SampleTypeUpdateServiceDI.java | 2 +- 5 files changed, 147 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 4a1ce0f0744..f4963067409 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -200,7 +200,9 @@ public Map> getExistingRows(User user, Container co Map> result = new LinkedHashMap<>(); for (Map.Entry> key : keys.entrySet()) { - Map row = getRow(user, container, key.getValue(), verifyNoCrossFolderData); + Map keyValues = key.getValue(); + Map row = getRow(user, container, keyValues, verifyNoCrossFolderData); + boolean hasValidExisting = false; if (row != null) { result.put(key.getKey(), row); @@ -212,9 +214,14 @@ public Map> getExistingRows(User user, Container co if (!container.getId().equals(dataContainer)) throw new InvalidKeyException("Data doesn't belong to folder '" + container.getName() + "': " + key.getValue().values()); } + // sql server will return case-insensitive match + if (verifyExisting) + hasValidExisting = !keyValues.containsKey("Name") || keyValues.get("Name").equals(row.get("Name")); } - else if (verifyExisting) - throw new InvalidKeyException("Data not found for " + key.getValue().values()); + + if (verifyExisting && !hasValidExisting) + throw new InvalidKeyException("Data not found: " + (keyValues.get("Name") != null ? keyValues.get("Name") : keyValues.values()) + "."); + } return result; } diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index e1a50e115c9..fc216383297 100644 --- a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts +++ b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts @@ -317,7 +317,7 @@ describe('Import with update / merge', () => { it ("Issue 52922: Blank sample id in the file are getting ignored in update from file", async () => { const BLANK_KEY_UPDATE_ERROR = 'Name value not provided on row '; const BLANK_KEY_MERGE_ERROR_NO_EXPRESSION = 'SampleID or Name is required for sample on row'; - const BOGUS_KEY_UPDATE_ERROR = 'Sample does not exist: bogus.'; + const BOGUS_KEY_UPDATE_ERROR = 'Sample not found: bogus.'; const CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR = "Sample does not belong to "; const dataType = SAMPLE_ALIQUOT_IMPORT_NO_NAME_PATTERN_NAME; @@ -951,3 +951,135 @@ describe('Aliquot crud', () => { }); +describe('Duplicate IDs', () => { + it("Issue 52657: We shouldn't allow creating sample names that differ only in case.", async () => { + const sampleTypeName = 'Type Case Sensitive'; + let field = { name: 'case', rangeURI: 'http://www.w3.org/2001/XMLSchema#string'}; + const domainPayload = { + kind: 'SampleSet', + domainDesign: { name: sampleTypeName, fields: [{ name: 'Name' }, field]}, + options: { + name: sampleTypeName, + nameExpression: 'S-${case}' + } + }; + await server.post('property', 'createDomain', domainPayload, {...topFolderOptions, ...designerReaderOptions}).expect(successfulResponse); + + const NAME_EXIST_MSG = "The name '%%' already exists."; + const sample1 = 'S-case-sAmple1'; + const sample2 = 'S-case-sAmple2'; + + let insertRows = [{ + name: sample1, + },{ + name: sample2, + }]; + const sampleRows = await ExperimentCRUDUtils.insertSamples(server, insertRows, sampleTypeName, topFolderOptions, editorUserOptions); + const sample1RowId = caseInsensitive(sampleRows[0], 'rowId'); + const sample1Lsid = caseInsensitive(sampleRows[0], 'lsid'); + const sample2RowId = caseInsensitive(sampleRows[1], 'rowId'); + const sample2Lsid = caseInsensitive(sampleRows[1], 'lsid'); + + let expectedError = NAME_EXIST_MSG.replace('%%', 'S-case-sample1'); + // import + let errorResp = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\nS-case-sample1\tbad\ns-case-sample2\tbad", sampleTypeName, "IMPORT", topFolderOptions, editorUserOptions); + expect(errorResp.text).toContain(expectedError); + + // merge + let mergeError = 'The name \'S-case-sample1\' could not be resolved. Please check the casing of the provided name.'; + errorResp = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\nS-case-sample1\tbad\ns-case-sample2\tbad", sampleTypeName, "MERGE", topFolderOptions, editorUserOptions); + expect(errorResp.text).toContain(mergeError); + + // insert + await server.post('query', 'insertRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + name: 'S-case-sample1', + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(expectedError); + }); + + // insert using naming expression to create case-insensitive name + await server.post('query', 'insertRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + case: 'case-sample1', + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(expectedError); + }); + + // renaming sample to another sample's case-insensitive name, using rowId + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + name: 'S-case-sample2', + rowId: sample1RowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'S-case-sample2')); + }); + + // renaming sample to another sample's case-insensitive name, using lsid. Currently can only be done using api + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + name: 'S-case-sample2', + lsid: sample1Lsid + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'S-case-sample2')); + }); + + // swap names (fail) + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + name: 'S-case-sample2', + lsid: sample1Lsid + }, { + name: 'S-case-sample1', + lsid: sample2Lsid + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'S-case-sample2')); + }); + + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: sampleTypeName, + rows: [{ + name: 'S-case-sample2', + rowId: sample1RowId + }, { + name: 'S-case-sample1', + rowId: sample2RowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toBe(NAME_EXIST_MSG.replace('%%', 'S-case-sample2')); + }); + + // renaming current sample to case-insensitive name, using rowId + let results = await ExperimentCRUDUtils.updateSamples(server, [{name: 'S-CASE-sample1', rowId: sample1RowId}], sampleTypeName, topFolderOptions, editorUserOptions); + expect(caseInsensitive(results[0], 'Name')).toBe('S-CASE-sample1'); + + // renaming current sample to case-insensitive name, using lsid + results = await ExperimentCRUDUtils.updateSamples(server, [{name: 's-case-SAMPLE1', lsid: sample1Lsid}], sampleTypeName, topFolderOptions, editorUserOptions); + expect(caseInsensitive(results[0], 'Name')).toBe('s-case-SAMPLE1'); + + }); + +}) + diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 8b0bebbf85e..d3dd7c615a9 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -3065,7 +3065,7 @@ private void writeRowsToFile(TypeData typeData) } if (!notFoundIds.isEmpty()) { - _context.getErrors().addRowError(new ValidationException((_isSamples ? "Samples" : "Data") + " not found for " + StringUtils.join(notFoundIds, ", "))); + _context.getErrors().addRowError(new ValidationException((_isSamples ? "Sample" + (notFoundIds.size() > 1 ? "s" : "") : "Data") + " not found: " + StringUtils.join(notFoundIds, ", ") + ".")); return; } diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index f34bfb8a7a9..b357e2b0c08 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -1271,7 +1271,7 @@ public void testInsertOptionUpdate() throws Exception qus.loadRows(user, c, MapDataIterator.of(rowsToUpdate), context, null); assertTrue(context.getErrors().hasErrors()); String msg = !context.getErrors().getRowErrors().isEmpty() ? context.getErrors().getRowErrors().get(0).toString() : "no message"; - assertTrue(msg.contains("Sample does not exist: S-1-absent.")); + assertTrue(msg.contains("Sample not found: S-1-absent.")); context = new DataIteratorContext(); context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); @@ -1282,7 +1282,7 @@ public void testInsertOptionUpdate() throws Exception qus.loadRows(user, c, MapDataIterator.of(rowsToUpdate), context, null); assertTrue(context.getErrors().hasErrors()); msg = !context.getErrors().getRowErrors().isEmpty() ? context.getErrors().getRowErrors().get(0).toString() : "no message"; - assertTrue(msg.contains("Sample does not exist: S-1-absent.")); + assertTrue(msg.contains("Sample not found: S-1-absent.")); // AliquotedFrom is supplied but doesn't match the current aliquot status / parents should get ignored rowsToUpdate = new ArrayList<>(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 2dff25314fa..8be3f3b6ecb 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1261,7 +1261,7 @@ else if (name != null && materialSourceId != null) } if (verifyExisting && !allKeys.isEmpty()) - throw new InvalidKeyException("Sample does not exist: " + allKeys.iterator().next() + "."); + throw new InvalidKeyException("Sample not found: " + allKeys.iterator().next() + "."); // if contains domain fields, check for aliquot specific fields if (!queryTableInfo.getName().equalsIgnoreCase("material")) From b82c533dca6286490e515e6055ac74590f4863f5 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 9 Jul 2025 11:34:08 -0700 Subject: [PATCH 11/12] update test --- experiment/src/client/test/integration/DataClassCrud.ispec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index 1f7986edfd3..31cd3362e1b 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -233,7 +233,7 @@ describe('Import with update / merge', () => { it ("Issue 52922: Blank sample id in the file are getting ignored in update from file", async () => { const BLANK_KEY_UPDATE_ERROR_NO_EXPRESSION = 'Missing value for required property: Name'; const BLANK_KEY_UPDATE_ERROR_WITH_EXPRESSION = 'Name value not provided on row '; - const BOGUS_KEY_UPDATE_ERROR = 'Data not found for '; + const BOGUS_KEY_UPDATE_ERROR = 'Data not found: '; const CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR = "Data doesn't belong to folder "; const dataType = "NoExpressionNameRequired52922"; From 1e676e33a761a885be6f57edd8beca2831fc8e10 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 10 Jul 2025 09:25:52 -0700 Subject: [PATCH 12/12] update comment --- api/src/org/labkey/api/query/AbstractQueryUpdateService.java | 2 +- .../src/org/labkey/experiment/api/ExperimentServiceImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index f4963067409..3dbaa08b6cb 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -214,7 +214,7 @@ public Map> getExistingRows(User user, Container co if (!container.getId().equals(dataContainer)) throw new InvalidKeyException("Data doesn't belong to folder '" + container.getName() + "': " + key.getValue().values()); } - // sql server will return case-insensitive match + // sql server will return case-insensitive match, check for exact match using equals if (verifyExisting) hasValidExisting = !keyValues.containsKey("Name") || keyValues.get("Name").equals(row.get("Name")); } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 515ef05945e..54f00f276d2 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9143,7 +9143,7 @@ public boolean isValidNewOrExistingName(String newOrExistingName, @NotNull Table .append(tableInfo) .append(" WHERE LOWER(name) = LOWER(?)") .add(newOrExistingName); - if (allowExisting) // // exclude existing name for merge + if (allowExisting) // // allow existing name for merge { dataRowSQL.append(" AND name <> ?").add(newOrExistingName); if (tableInfo.getSqlDialect().isSqlServer())