diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 4a1ce0f0744..3dbaa08b6cb 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, check for exact match using equals + 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/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index e8875f28ff9..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"; @@ -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 diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index a6ce27c54ba..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; @@ -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."); }); /** @@ -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 1bdb4385312..d3dd7c615a9 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, isUpdateUsingLsid, _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) @@ -3059,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; } @@ -3155,6 +3161,104 @@ public static void incrementCounts(Map currentCounts, Map map = DataIteratorUtil.createColumnNameMap(di); + _nameCol = map.get(NAME_FIELD); + _lsidCol = map.get("lsid"); + } + + @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; + + 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().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)); + } + + 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..abee86500f8 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1323,6 +1323,8 @@ protected Map _update(User user, Container c, Map rowStripped = new CaseInsensitiveHashMap<>(); diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index 37c86a68589..561d53013fa 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -1241,7 +1241,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); @@ -1252,7 +1252,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/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index a7ac6304235..54f00f276d2 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9137,6 +9137,35 @@ public Map> getDomainMetrics() return metrics; } + public boolean isValidNewOrExistingName(String newOrExistingName, @NotNull TableInfo tableInfo, boolean allowExisting) + { + SQLFragment dataRowSQL = new SQLFragment("SELECT name FROM ") + .append(tableInfo) + .append(" WHERE LOWER(name) = LOWER(?)") + .add(newOrExistingName); + if (allowExisting) // // allow 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 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 lsid <> ?") + .add(newName) + .add(lsid); + if (tableInfo.getSqlDialect().isSqlServer()) + dataRowSQL.append(" COLLATE Latin1_General_BIN"); // force case sensitive comparison + + 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/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 8068ef0714a..10abe5ad1e6 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -136,6 +136,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; @@ -1717,7 +1718,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()) @@ -1778,7 +1779,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()) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index b8ff3fd8425..d6cfb67af62 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -712,6 +712,9 @@ protected Map _update(User user, Container c, Map