diff --git a/api/src/org/labkey/api/dataiterator/DataClassUpdateAddColumnsDataIterator.java b/api/src/org/labkey/api/dataiterator/DataClassUpdateAddColumnsDataIterator.java index 8d930cf16a1..dba5988baae 100644 --- a/api/src/org/labkey/api/dataiterator/DataClassUpdateAddColumnsDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DataClassUpdateAddColumnsDataIterator.java @@ -125,7 +125,7 @@ protected void prefetchExisting() throws BatchValidationException } if (!notFoundKeys.isEmpty()) - _context.getErrors().addRowError(new ValidationException("Data not found for " + notFoundKeys)); + _context.getErrors().addRowError(new ValidationException("Data does not exist in " + _targetContainer.getName() + ": " + notFoundKeys + ".")); resetAfterBatch(); } diff --git a/api/src/org/labkey/api/dataiterator/DataIteratorContext.java b/api/src/org/labkey/api/dataiterator/DataIteratorContext.java index 12713d9605b..6107285824a 100644 --- a/api/src/org/labkey/api/dataiterator/DataIteratorContext.java +++ b/api/src/org/labkey/api/dataiterator/DataIteratorContext.java @@ -54,7 +54,6 @@ public class DataIteratorContext LookupResolutionType _lookupResolutionType = LookupResolutionType.primaryKey; QueryImportPipelineJob _backgroundJob = null; boolean _crossTypeImport = false; - boolean _crossFolderImport = false; boolean _allowCreateStorage = false; boolean _useTransactionAuditCache = false; private final Set _passThroughBuiltInColumnNames = new CaseInsensitiveHashSet(); @@ -202,16 +201,6 @@ public void setCrossTypeImport(boolean crossTypeImport) _crossTypeImport = crossTypeImport; } - public boolean isCrossFolderImport() - { - return _crossFolderImport; - } - - public void setCrossFolderImport(boolean crossFolderImport) - { - _crossFolderImport = crossFolderImport; - } - public boolean isAllowCreateStorage() { return _allowCreateStorage; diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index 9e489a5165a..719ab8e16cb 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -311,7 +311,6 @@ public enum Params crossTypeImport, allowCreateStorage, importLookupByAlternateKey, // deprecated. Prefer lookupResolutionType - crossFolderImport, useTransactionAuditCache, lookupResolutionType, auditDetails, @@ -331,7 +330,6 @@ protected Map getOptionParamsMap() _optionParamsMap.put(Params.importIdentity, Boolean.valueOf(getParam(Params.importIdentity))); _optionParamsMap.put(Params.crossTypeImport, Boolean.valueOf(getParam(Params.crossTypeImport))); _optionParamsMap.put(Params.allowCreateStorage, Boolean.valueOf(getParam(Params.allowCreateStorage))); - _optionParamsMap.put(Params.crossFolderImport, Boolean.valueOf(getParam(Params.crossFolderImport))); _optionParamsMap.put(Params.useTransactionAuditCache, Boolean.valueOf(getParam(Params.useTransactionAuditCache))); } return _optionParamsMap; @@ -345,8 +343,6 @@ protected Set getTransactionImportParams(String insertOption, boolean us importParams.add("backgroundImport"); if (Boolean.valueOf(getParam(Params.crossTypeImport))) importParams.add(Params.crossTypeImport.name()); - if (Boolean.valueOf(getParam(Params.crossFolderImport))) - importParams.add(Params.crossFolderImport.name()); if (Boolean.valueOf(getParam(Params.useTransactionAuditCache))) importParams.add(Params.useTransactionAuditCache.name()); if (Boolean.valueOf(getParam(Params.allowCreateStorage))) @@ -852,7 +848,6 @@ public static DataIteratorContext createDataIteratorContext(QueryUpdateService.I boolean importIdentity = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.importIdentity, false); boolean crossTypeImport = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.crossTypeImport, false); boolean allowCreateStorage = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.allowCreateStorage, false); - boolean crossFolderImport = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.crossFolderImport, false); boolean useTransactionAuditCache = optionParamsMap.getOrDefault(Params.useTransactionAuditCache, false); DataIteratorContext context = new DataIteratorContext(errors); @@ -874,7 +869,6 @@ public static DataIteratorContext createDataIteratorContext(QueryUpdateService.I context.setSupportAutoIncrementKey(true); } context.setCrossTypeImport(crossTypeImport); - context.setCrossFolderImport(crossFolderImport && container != null && container.hasProductFolders()); context.setAllowCreateStorage(allowCreateStorage); context.setUseTransactionAuditCache(useTransactionAuditCache); context.setLogger(logger); diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 19453164ec4..7b90296a4ec 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -215,21 +215,15 @@ public Map> getExistingRows(User user, Container co if (StringUtils.isEmpty(dataContainer)) dataContainer = (String) row.get("folder"); if (!container.getId().equals(dataContainer)) - throw new InvalidKeyException("Data does not belong to folder '" + container.getName() + "': " + key.getValue().values()); + throw new InvalidKeyException("Data does not exist in " + container.getName() + ": " + key.getValue().toString() + "."); } } else if (verifyExisting) - throw new InvalidKeyException("Data not found for " + key.getValue().values()); + throw new InvalidKeyException("Data does not exist in " + container.getName() + ": " + key.getValue().toString() + "."); } return result; } - @Override - public boolean hasExistingRowsInOtherContainers(Container container, Map> keys) - { - return false; - } - public static TransactionAuditProvider.TransactionAuditEvent createTransactionAuditEvent(Container container, QueryService.AuditAction auditAction) { return createTransactionAuditEvent(container, auditAction, null); @@ -408,7 +402,7 @@ protected int _importRowsUsingDIB(User user, Container container, DataIteratorBu preImportDIBValidation(in, null); - boolean skipTriggers = context.getConfigParameterBoolean(ConfigParameters.SkipTriggers) || context.isCrossTypeImport() || context.isCrossFolderImport(); + boolean skipTriggers = context.getConfigParameterBoolean(ConfigParameters.SkipTriggers) || context.isCrossTypeImport(); boolean hasTableScript = hasTableScript(container); TriggerDataBuilderHelper helper = new TriggerDataBuilderHelper(getQueryTable(), container, user, extraScriptContext, context.getInsertOption().useImportAliases); if (!skipTriggers) @@ -1299,7 +1293,7 @@ static FileLike checkFileUnderRoot(Container container, FileLike file) throws Ex protected void _addSummaryAuditEvent(Container container, User user, DataIteratorContext context, int count) { - if (!context.isCrossTypeImport() && !context.isCrossFolderImport()) // audit handled at table level + if (!context.isCrossTypeImport()) // audit handled at table level { AuditBehaviorType auditType = (AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior); String auditUserComment = (String) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment); diff --git a/api/src/org/labkey/api/query/CacheClearingQueryUpdateService.java b/api/src/org/labkey/api/query/CacheClearingQueryUpdateService.java index bdb40f298c2..e1d56949fd8 100644 --- a/api/src/org/labkey/api/query/CacheClearingQueryUpdateService.java +++ b/api/src/org/labkey/api/query/CacheClearingQueryUpdateService.java @@ -52,14 +52,6 @@ public List> getRows(User user, Container container, List> keys) - { - var ret = _service.hasExistingRowsInOtherContainers(container, keys); - clearCache(); - return ret; - } - @Override public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, Set columns) throws InvalidKeyException, QueryUpdateServiceException, SQLException diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index a2b32cffcb3..01d5f07ebfd 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -65,7 +65,6 @@ import org.labkey.vfs.FileLike; import org.springframework.web.multipart.MultipartFile; -import java.io.IOException; import java.nio.file.Path; import java.sql.SQLException; import java.util.ArrayList; @@ -915,25 +914,6 @@ protected boolean isAttachmentProperty(String name) return false; } - protected void configureCrossFolderImport(DataIteratorBuilder rows, DataIteratorContext context) throws IOException - { - if (!context.getInsertOption().updateOnly && context.isCrossFolderImport() && rows instanceof DataLoader dataLoader) - { - boolean hasContainerField = false; - for (ColumnDescriptor columnDescriptor : dataLoader.getColumns()) - { - String fieldName = columnDescriptor.getColumnName(); - if (fieldName.equalsIgnoreCase("Container") || fieldName.equalsIgnoreCase("Folder")) - { - hasContainerField = true; - break; - } - } - if (!hasContainerField) - context.setCrossFolderImport(false); - } - } - public static @Nullable String getKeyColumnAliasForUpdate(TableInfo tableInfo, @NotNull Map columnNameMap) { // Currently, SampleUpdateAddColumnsDataIterator and DataClassUpdateAddColumnsDataIterator is being called before a translator is invoked to diff --git a/api/src/org/labkey/api/query/QueryUpdateService.java b/api/src/org/labkey/api/query/QueryUpdateService.java index 09c9e4f6495..e3dd53bc50d 100644 --- a/api/src/org/labkey/api/query/QueryUpdateService.java +++ b/api/src/org/labkey/api/query/QueryUpdateService.java @@ -149,8 +149,6 @@ List> getRows(User user, Container container, List> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) throws InvalidKeyException, QueryUpdateServiceException, SQLException; - boolean hasExistingRowsInOtherContainers(Container container, Map> keys); - /** * Inserts or merges the given values into the source table of this query. * The operation to be performed and import behavior is configured by the context parameter. diff --git a/experiment/package-lock.json b/experiment/package-lock.json index 75f67f2cab1..6bebfb8666c 100644 --- a/experiment/package-lock.json +++ b/experiment/package-lock.json @@ -12,7 +12,7 @@ }, "devDependencies": { "@labkey/build": "9.1.4", - "@labkey/test": "1.13.2", + "@labkey/test": "1.14.0-fb-dropCrossFolder.1", "@types/jest": "30.0.0", "@types/react": "18.3.27", "@types/react-dom": "18.3.7", @@ -3692,9 +3692,9 @@ } }, "node_modules/@labkey/test": { - "version": "1.13.2", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/test/-/@labkey/test-1.13.2.tgz", - "integrity": "sha512-6ZsumxiRt5QTQv78/g0uUA4P5LTuvgbO2+xQszOYMCLBLhA0yOICaxWc8NLJkDKLPRJB5cnn03PDiNzK3LjAog==", + "version": "1.14.0-fb-dropCrossFolder.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/test/-/@labkey/test-1.14.0-fb-dropCrossFolder.1.tgz", + "integrity": "sha512-7bKNtycwklcH5OC3PraxcWTn+ZIiBqRMd395pUgyAl2f0wus24s3SRkekYWlC3xwFKYYkt6MPCfmWf2v0Q/ETQ==", "dev": true, "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { diff --git a/experiment/package.json b/experiment/package.json index d4208d502a0..a71dac1472f 100644 --- a/experiment/package.json +++ b/experiment/package.json @@ -17,7 +17,7 @@ }, "devDependencies": { "@labkey/build": "9.1.4", - "@labkey/test": "1.13.2", + "@labkey/test": "1.14.0-fb-dropCrossFolder.1", "@types/jest": "30.0.0", "@types/react": "18.3.27", "@types/react-dom": "18.3.7", diff --git a/experiment/src/client/test/integration/AssayDesignCrud.ispec.ts b/experiment/src/client/test/integration/AssayDesignCrud.ispec.ts index ea6322154a8..2ff3c55733f 100644 --- a/experiment/src/client/test/integration/AssayDesignCrud.ispec.ts +++ b/experiment/src/client/test/integration/AssayDesignCrud.ispec.ts @@ -1,4 +1,4 @@ -import { hookServer, RequestOptions, selectRandomN, successfulResponse } from '@labkey/test'; +import { hookServer, RequestOptions, selectRandomN, successfulResponse, testSeed } from '@labkey/test'; import { deleteAssayDesign, @@ -12,7 +12,8 @@ import { ASSAY_DESIGNER_ROLE } from '@labkey/components'; // @ts-expect-error process is not available in a browser environment const server = hookServer(process.env); -const PROJECT_NAME = 'DataClassCrudJestProject'; +const PROJECT_NAME = 'AssayDesignCrudJestProject'; +console.log(`[AssayDesignCrud] Random seed: ${testSeed} (rerun with: TEST_SEED=${testSeed})`); let readerUser, readerUserOptions; let editorUser, editorUserOptions; diff --git a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts index a240785336a..be78f5671fc 100644 --- a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts +++ b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts @@ -1,4 +1,4 @@ -import { hookServer, IntegrationTestServer, RequestOptions, successfulResponse } from '@labkey/test'; +import { hookServer, IntegrationTestServer, RequestOptions, successfulResponse, testSeed } from '@labkey/test'; import mock from 'mock-fs'; import { @@ -27,6 +27,9 @@ import { // @ts-expect-error process is not available in a browser environment const server = hookServer(process.env); const PROJECT_NAME = 'ArrayImportRunActionTest Project'; + +console.log(`[ArrayImportRunAction] Random seed: ${testSeed} (rerun with: TEST_SEED=${testSeed})`); + const BATCH_FILE_FIELD_NAME = 'batchFileField'; const BATCH_FILE_FIELD_TWO_NAME = 'batchFile2Field'; const RUN_FILE_FIELD_NAME = 'runFileField'; diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index 353ca67141a..c3071ddf9d1 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -4,7 +4,8 @@ import { getEscapedNameExpression, hookServer, RequestOptions, - successfulResponse + successfulResponse, + testSeed } from '@labkey/test'; import mock from 'mock-fs'; import { @@ -22,6 +23,7 @@ import { import { caseInsensitive, DATA_CLASS_DESIGNER_ROLE } from '@labkey/components'; const server = hookServer(process.env); const PROJECT_NAME = 'DataClassCrudJestProject'; +console.log(`[DataClassCrud] Random seed: ${testSeed} (rerun with: TEST_SEED=${testSeed})`); let readerUser, readerUserOptions; let editorUser, editorUserOptions; @@ -278,8 +280,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 DUPLICATE_KEY_ERROR = 'duplicate key value'; + const BOGUS_KEY_UPDATE_ERROR = 'Data does not exist in '; const dataType = "NoExpressionNameRequired52922"; const createPayload = { @@ -349,9 +350,9 @@ describe('Import with update / merge', () => { successResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\n\tisBlank", dataTypeWithExpression, "MERGE", subfolder1Options, editorUserOptions); expect(successResp.text.indexOf('"success" : true') > -1).toBeTruthy(); - // cross folder update not supported when folder type is "Collaboration" + // cross folder update not supported let crossFolderErrorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nData1\tNotblank\n\tisBlank", dataTypeWithExpression, "MERGE", subfolder1Options, editorUserOptions); - expect(crossFolderErrorResp.text.indexOf(DUPLICATE_KEY_ERROR) > -1).toBeTruthy(); + expect(crossFolderErrorResp.text.indexOf(BOGUS_KEY_UPDATE_ERROR) > -1).toBeTruthy(); crossFolderErrorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nData1\tNotblank", dataTypeWithExpression, "UPDATE", subfolder1Options, editorUserOptions); expect(crossFolderErrorResp.text.indexOf(BOGUS_KEY_UPDATE_ERROR) > -1).toBeTruthy(); @@ -1140,12 +1141,12 @@ describe('Data CRUD', () => { }] }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { const errorResp = JSON.parse(result.text); - expect(errorResp['exception']).toContain('Data not found for [' + row3RowId + ']'); + expect(errorResp['exception']).toContain('Data does not exist in ' + PROJECT_NAME + ': {RowId=' + row3RowId + '}.'); }); // using update from file, verify update using rowId for data that doesn't exist on this dataclass should fail. errorResp = await ExperimentCRUDUtils.importData(server, "RowId\tDescription\n" + row3RowId + "\tupdate\n", emptyDataClass, "UPDATE", topFolderOptions, editorUserOptions); - expect(errorResp.text).toContain('Data not found for [' + row3RowId + ']'); + expect(errorResp.text).toContain('Data does not exist in ' + PROJECT_NAME + ': {RowId=' + row3RowId + '}.'); }); diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index 30f3f9d2e82..3019f8466a2 100644 --- a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts +++ b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts @@ -1,4 +1,11 @@ -import { ExperimentCRUDUtils, hookServer, RequestOptions, selectRandomN, successfulResponse } from '@labkey/test'; +import { + ExperimentCRUDUtils, + hookServer, + RequestOptions, + selectRandomN, + successfulResponse, + testSeed +} from '@labkey/test'; import mock from 'mock-fs'; import { checkDomainName, @@ -15,6 +22,8 @@ const { importSample, insertRows } = ExperimentCRUDUtils; const server = hookServer(process.env); const PROJECT_NAME = 'SampleTypeCrudJestProject'; +console.log(`[SampleTypeCrud] Random seed: ${testSeed} (rerun with: TEST_SEED=${testSeed})`); + const SAMPLE_ALIQUOT_IMPORT_TYPE_NAME = "SampleType_Aliquots_Import"; const SAMPLE_ALIQUOT_REQ_IMPORT_TYPE_NAME = "Aliquot_Import_RequiredProp"; const SAMPLE_ALIQUOT_IMPORT_NO_NAME_PATTERN_NAME = "SampleType_Aliquots_Import_NoExpression"; @@ -318,8 +327,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'; const BLANK_KEY_MERGE_ERROR_NO_EXPRESSION = 'SampleID or Name is required for sample'; - const BOGUS_KEY_UPDATE_ERROR = 'Sample does not exist: bogus.'; - const CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR = "Sample does not belong to "; + const BOGUS_KEY_UPDATE_ERROR = 'Sample does not exist in '; const dataType = SAMPLE_ALIQUOT_IMPORT_NO_NAME_PATTERN_NAME; const dataName = "Data1"; @@ -369,11 +377,11 @@ describe('Import with update / merge', () => { successResp = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\n\tisBlank", dataTypeWithExpression, "MERGE", subfolder1Options, editorUserOptions); expect(successResp.text.indexOf('"success" : true') > -1).toBeTruthy(); - // cross folder update not supported when folder type is "Collaboration" + // cross folder update not supported let crossFolderUpdateErrorResp = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\nData1\tNotblank", dataTypeWithExpression, "UPDATE", subfolder1Options, editorUserOptions); - expect(crossFolderUpdateErrorResp.text.indexOf(CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR) > -1).toBeTruthy(); + expect(crossFolderUpdateErrorResp.text.indexOf(BOGUS_KEY_UPDATE_ERROR) > -1).toBeTruthy(); let crossFolderMergeErrorResp = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\nData1\tNotblank\n\tisBlank", dataTypeWithExpression, "MERGE", subfolder1Options, editorUserOptions); - expect(crossFolderMergeErrorResp.text.indexOf(CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR) > -1).toBeTruthy(); + expect(crossFolderMergeErrorResp.text.indexOf(BOGUS_KEY_UPDATE_ERROR) > -1).toBeTruthy(); // bogus name bogusKeyProvidedError = await ExperimentCRUDUtils.importSample(server, "Name\tDescription\nbogus\tisBogus", dataTypeWithExpression, "UPDATE", topFolderOptions, editorUserOptions); @@ -487,7 +495,7 @@ describe('Import with update / merge', () => { // Assert // Verify that these rows are not updated expect(resp.body.success).toEqual(false); - expect(resp.body.exception).toContain('Sample does not exist: (RowId)'); + expect(resp.body.exception).toContain('Sample does not exist in ' + PROJECT_NAME + ': (RowId)'); }) }); diff --git a/experiment/src/client/test/integration/utils.ts b/experiment/src/client/test/integration/utils.ts index 6acf022954c..77c98dfde04 100644 --- a/experiment/src/client/test/integration/utils.ts +++ b/experiment/src/client/test/integration/utils.ts @@ -2,6 +2,7 @@ import { ExperimentCRUDUtils, generateFieldName, IntegrationTestServer, + random, RequestOptions, selectRandomN, successfulResponse, @@ -632,7 +633,7 @@ export async function verifyRequiredLineageInsertUpdate(server: IntegrationTestS const dataType = "withRequired" + (isParentSample ? 'SampleParent' : 'DataParent'); let childDomainId = -1, childDomainURI = ''; - const useLowerCase = Math.random() < 0.5; + const useLowerCase = random() < 0.5; // test both lower case and upper case prefix const parentInput = (isParentSample ? (useLowerCase ? 'materialInputs/' : 'MaterialInputs/') : (useLowerCase ? 'dataInputs/' : 'DataInputs/')) + parentDataType; await server.post('property', 'createDomain', { diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index f5f81746a2c..e1ad4da2af8 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -15,7 +15,6 @@ */ package org.labkey.experiment; -import org.apache.commons.beanutils.ConversionException; import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; @@ -36,12 +35,10 @@ import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; -import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CounterDefinition; import org.labkey.api.data.DbScope; import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.ImportAliasable; -import org.labkey.api.data.JdbcType; import org.labkey.api.data.MultiChoice; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.RemapCache; @@ -88,7 +85,6 @@ import org.labkey.api.exp.api.SimpleRunRecord; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.PropertyService; -import org.labkey.api.exp.query.AbstractExpSchema; import org.labkey.api.exp.query.DataClassUserSchema; import org.labkey.api.exp.query.ExpDataTable; import org.labkey.api.exp.query.ExpSchema; @@ -111,13 +107,9 @@ import org.labkey.api.reader.DataLoader; import org.labkey.api.reader.TabLoader; import org.labkey.api.security.User; -import org.labkey.api.security.permissions.InsertPermission; -import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.study.publish.StudyPublishService; -import org.labkey.api.usageMetrics.SimpleMetricsService; import org.labkey.api.util.DateUtil; import org.labkey.api.util.FileUtil; -import org.labkey.api.util.GUID; import org.labkey.api.util.Pair; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.logging.LogHelper; @@ -159,7 +151,6 @@ import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; import static org.labkey.api.data.CompareType.IN; @@ -2653,10 +2644,8 @@ public boolean next() throws BatchValidationException public static class MultiDataTypeCrossProjectDataIterator extends WrapperDataIterator { - private static final String INVALID_FOLDER_MESSAGE = "Import or update of data in folder %s from folder %s is not allowed. Verify the folder exists, you have proper permissions, and data from that folder is visible here."; private static final Set IGNORED_FIELD_NAMES = Set.of("lsid", "genid"); private static final Set SAMPLE_TYPE_FIELD_NAMES = Set.of("SampleType", "Sample Type"); - private static final Set CONTAINER_FIELD_NAMES = Set.of("Container", "Folder"); private static final int BATCH_SIZE = 1000; record TypeData( Container container, @@ -2673,27 +2662,21 @@ record TypeData( private final DataIteratorContext _context; private final boolean _isCrossType; - private final boolean _isCrossFolder; private final boolean _isSamples; private final ExpObject _dataType; private final Container _container; private final User _user; private Integer _typeColIndex = null; private String _typeColName = null; - private Integer _folderColIndex = null; // want to process the sample types in the order given in the original file, unless we have dependencies private final Map> _typeFolderDataMap = new TreeMap<>(); private final Map> _orderDependencies = new HashMap<>(); private final int _dataIdIndex; - private final FieldKey _dataKey; - private final boolean _dataKeyIsNumeric; private final Map> _idsPerType = new HashMap<>(); private final Map> _parentIdsPerType = new HashMap<>(); - private final Map _containerMap = new CaseInsensitiveHashMap<>(); - private final boolean _isCrossFolderUpdate; private final TSVWriter _tsvWriter; - private MultiDataTypeCrossProjectDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, boolean isCrossType, boolean isCrossFolder, ExpObject dataType, boolean isSamples) + private MultiDataTypeCrossProjectDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, boolean isCrossType, ExpObject dataType, boolean isSamples) { super(di); _context = context; @@ -2702,14 +2685,11 @@ private MultiDataTypeCrossProjectDataIterator(DataIterator di, DataIteratorConte _dataType = dataType; _user = user; _isCrossType = isCrossType; - _isCrossFolder = isCrossFolder; Map map = DataIteratorUtil.createColumnNameMap(di); // Determine the dataId column { int index; - FieldKey dataKey; - boolean isNumeric; var foundId = RowId.namesAndLabels().stream() .filter(map::containsKey) @@ -2718,19 +2698,13 @@ private MultiDataTypeCrossProjectDataIterator(DataIterator di, DataIteratorConte if (foundId.isPresent()) { index = map.get(foundId.get()); - dataKey = RowId.fieldKey(); - isNumeric = true; } else { index = map.getOrDefault(Name.name(), -1); - dataKey = Name.fieldKey(); - isNumeric = false; } _dataIdIndex = index; - _dataKey = dataKey; - _dataKeyIsNumeric = isNumeric; } _tsvWriter = new TSVWriter() // Used to quote values with newline/tabs/quotes @@ -2743,8 +2717,6 @@ protected int write() }; _tsvWriter.setAdditionalQuotedChars(TSVWriter.BACKSLASH_CHAR_STRING); - _isCrossFolderUpdate = isCrossFolder && context.getInsertOption().updateOnly; - if (_isCrossType && _isSamples) //cross type only supported for samples { SAMPLE_TYPE_FIELD_NAMES.forEach(name -> { @@ -2759,51 +2731,6 @@ protected int write() if (_typeColIndex == null) _context.getErrors().addRowError(new ValidationException("Could not determine sample type. Please provide a 'Sample Type' column in the data.")); } - - if (_isCrossFolder) - { - CONTAINER_FIELD_NAMES.forEach(name -> { - if (map.get(name) != null) - { - if (_folderColIndex != null) - _context.getErrors().addRowError(new ValidationException("Only one of [" + CONTAINER_FIELD_NAMES.stream().sorted().collect(Collectors.joining(", ")) + "] allowed for import.")); - _folderColIndex = map.get(name); - } - }); - - if (_folderColIndex != null || _isCrossFolderUpdate) - { - ContainerFilter cf; - if (container.isProductFoldersEnabled()) - { - // Note that this is slightly different from our treatment of lookups: - // - when in a project, we allow import or update to all subfolders, - // - when in a folder, we only allow references to data up the folder tree - if (container.isProject()) - cf = new ContainerFilter.AllInProjectPlusShared(container, user); - else - cf = new ContainerFilter.CurrentPlusProjectAndShared(container, user); - } - else - cf = ContainerFilter.current(container, user); - - Collection validContainerIds; - if (cf instanceof ContainerFilter.ContainerFilterWithPermission cfp) - validContainerIds = cfp.generateIds(container, context.getInsertOption().allowUpdate ? UpdatePermission.class : InsertPermission.class, null); - else - validContainerIds = cf.getIds(); - - if (validContainerIds != null) - { - for (GUID containerId : validContainerIds) - { - Container validContainer = ContainerManager.getForId(containerId); - _containerMap.put(validContainer.getId(), validContainer); - _containerMap.put(validContainer.getName(), validContainer); // for multi-type import, container column lookup is not yet resolved - } - } - } - } } private int _importSplitFile(TypeData typeData, File splitFile, Container dataContainer, TableInfo dataTable) @@ -2844,32 +2771,6 @@ private int _importPartition(TypeData typeData) if (_context.getErrors().hasErrors()) return 0; - int totalRowCount = 0; - if (_isCrossFolderUpdate && !typeData.folderFiles.keySet().isEmpty()) - { - boolean hasCrossFolderData = typeData.folderFiles.keySet().stream().anyMatch(id -> id != _container.getRowId()); - - if (hasCrossFolderData) - { - for (Map.Entry containerSplitFile : typeData.folderFiles.entrySet()) - { - Container splitContainer = ContainerManager.getForRowId(containerSplitFile.getKey()); - AbstractExpSchema schema = _isSamples ? new SamplesSchema(_user, splitContainer) : new DataClassUserSchema(splitContainer, _user); - QueryDefinition qDef = schema.getQueryDefForTable(typeData.dataType.getName()); - setContainerFilterForImport(qDef, splitContainer, _user); - TableInfo dataTable = qDef.getTable(schema, new ArrayList<>(), true); - - if (dataTable == null) - { - _context.getErrors().addRowError(new ValidationException("Table for " + (_isSamples ? "sample type" : "dataclass") + " '" + typeData.dataType.getName() + "' not found.")); - return totalRowCount; - } - totalRowCount += _importSplitFile(typeData, containerSplitFile.getValue(), splitContainer, dataTable); - } - return totalRowCount; - } - } - return _importSplitFile(typeData, typeData.dataFile, typeData.container, typeData.tableInfo); } @@ -2887,16 +2788,12 @@ public boolean next() throws BatchValidationException if (!_context.getErrors().hasErrors()) { _context.setCrossTypeImport(false); - _context.setCrossFolderImport(false); _context.putConfigParameter(QueryUpdateService.ConfigParameters.ProcessingPartition, true); - boolean hasCrossFolderImport = false; - // process the individual files for (String key : importOrderKeys) { Map typeFolderData = _typeFolderDataMap.get(key); - hasCrossFolderImport = hasCrossFolderImport || typeFolderData.keySet().size() > 1; for (TypeData typeData : typeFolderData.values()) { writeRowsToFile(typeData); // write the last rows that have been collected since the last write, if any @@ -2905,12 +2802,8 @@ public boolean next() throws BatchValidationException } } - if (_isCrossFolder && !_context.getInsertOption().updateOnly && hasCrossFolderImport) // all updates are cross-folder due to lack of Container column - SimpleMetricsService.get().increment(ExperimentService.MODULE_NAME, _isSamples ? "sampleImport" : "dataClassImport", "multiFolderImport"); - _context.putConfigParameter(QueryUpdateService.ConfigParameters.ProcessingPartition, false); _context.setCrossTypeImport(_isCrossType); - _context.setCrossFolderImport(_isCrossFolder); } return false; @@ -2935,21 +2828,6 @@ public boolean next() throws BatchValidationException _context.getErrors().addRowError(new ValidationException("No value provided for '" + _typeColName + "'.")); else { - // Issue 52626 and Issue 52609 - don't check folders during update - if (_isCrossFolder && _folderColIndex != null && !_context.getInsertOption().updateOnly) - { - String rowFolderId = StringUtils.trim((String) get(_folderColIndex)); - if (!StringUtils.isEmpty(rowFolderId)) - { - targetContainer = _containerMap.get(rowFolderId); - if (targetContainer == null) - { - _context.getErrors().addRowError(new ValidationException(String.format(INVALID_FOLDER_MESSAGE, rowFolderId, _container.getName()))); - return true; - } - } - } - Map typeFolderMap = _typeFolderDataMap.computeIfAbsent(typeName, k -> new LinkedHashMap<>()); typeFolderData = typeFolderMap.get(targetContainer.getId()); if (typeFolderData == null) @@ -3265,23 +3143,6 @@ private void addDataRow(TypeData typeData) { String dataString = data.toString(); _idsPerType.computeIfAbsent(typeData.dataType.getName(), k -> new HashSet<>()).add(dataString); - if (_isCrossFolderUpdate) - { - if (_dataKeyIsNumeric) - { - try - { - typeData.dataIds.add(JdbcType.BIGINT.convert(data)); - } - catch (ConversionException e) - { - _context.getErrors().addRowError(new ValidationException(e.getMessage() + " on row " + get(0), _dataKey.getName())); - return; - } - } - else - typeData.dataIds.add(dataString); - } } // if the data represents a derivation dependency between types, and we're creating ids within the file, @@ -3296,11 +3157,6 @@ private void addDataRow(TypeData typeData) _orderDependencies.computeIfAbsent(typeData.dataType.getName(), i -> new HashSet<>()).add(parentTypeName); } } - else if (index == _dataIdIndex && _isCrossFolderUpdate) - { - // Issue 52922: Samples with blank sample id in the file are getting ignored - throw new IllegalArgumentException(_dataKey.getName() + " value not provided on row " + get(0)); - } }); typeData.dataRows.add(StringUtils.join(dataRow, "\t")); } @@ -3310,90 +3166,6 @@ private void writeRowsToFile(TypeData typeData) if (typeData.dataRows.isEmpty()) return; - // for cross-folder import, write to further partitions - if (_isCrossFolderUpdate) - { - ExpObject dataType = typeData.dataType; - Map> containerRows = new HashMap<>(); - - TableInfo tableInfo; - SimpleFilter filter; - - if (_isSamples) - { - filter = new SimpleFilter(MaterialSourceId.fieldKey(), dataType.getRowId()); - filter.addCondition(_dataKey, typeData.dataIds, CompareType.IN); - tableInfo = ExperimentService.get().getTinfoMaterial(); - } - else - { - filter = new SimpleFilter(FieldKey.fromParts("ClassId"), dataType.getRowId()); - filter.addCondition(_dataKey, typeData.dataIds, CompareType.IN); - tableInfo = ExperimentService.get().getTinfoData(); - } - - Map[] rows = new TableSelector(tableInfo, Set.of(_dataKey.getName(), "container"), filter, null).getMapArray(); - - Set notFoundIds = new HashSet<>(typeData.dataIds); - for (Map row : rows) - { - Object raw = row.get(_dataKey.getName()); - Object identifier = _dataKeyIsNumeric ? asLong(raw) : raw; - notFoundIds.remove(identifier); - String dataContainer = (String) row.get("container"); - // could be updating the same data multiple times in a single import, the import will later be rejected - List dataRowIds = - IntStream.range(0, typeData.dataIds.size()).boxed() - .filter(i -> typeData.dataIds.get(i).equals(identifier)) - .toList(); - containerRows.computeIfAbsent(dataContainer, k -> new ArrayList<>()).addAll(dataRowIds); - } - if (!notFoundIds.isEmpty()) - { - _context.getErrors().addRowError(new ValidationException((_isSamples ? "Samples" : "Data") + " not found for " + StringUtils.join(notFoundIds, ", "))); - return; - } - - for (String containerId : containerRows.keySet()) - { - Container container = _containerMap.get(containerId); - if (container == null) - { - Container folder = ContainerManager.getForId(containerId); - _context.getErrors().addRowError(new ValidationException(String.format(INVALID_FOLDER_MESSAGE, (folder != null ? folder.getName() : containerId), _container.getName()))); - return; - } - - int containerRowId = container.getRowId(); - File splitFile = typeData.folderFiles.get(containerRowId); - - if (splitFile == null) - { - splitFile = writeSplitFile(typeData.dataType.getName(), "~containerSplit~", containerRowId + "-" + typeData.dataFile.getName(), typeData.headerRow); - if (splitFile == null) - return; - typeData.folderFiles.put(containerRowId, splitFile); - } - - List dataRows = new ArrayList<>(); - List dataRowIndexes = containerRows.get(containerId); - Collections.sort(dataRowIndexes); - for (Integer dataRowIndex : dataRowIndexes) - dataRows.add(typeData.dataRows.get(dataRowIndex)); - - try (FileWriter writer = new FileWriter(splitFile, true)) - { - writer.write(StringUtils.join(dataRows, System.lineSeparator())); - writer.write(System.lineSeparator()); // Issue 48442: add a new line to the end so the next written rows start on a new line - } - catch (IOException e) - { - _context.getErrors().addRowError(new ValidationException("Unable to write data for '" + typeData.dataType.getName() + "'.")); - return; - } - } - } - try (FileWriter writer = new FileWriter(typeData.dataFile, true)) { writer.write(StringUtils.join(typeData.dataRows, System.lineSeparator())); @@ -3414,17 +3186,15 @@ public static class MultiDataTypeCrossProjectDataIteratorBuilder implements Data private final Container _container; private final User _user; private final boolean _isCrossType; - private final boolean _isCrossFolder; private final ExpObject _dataType; private final boolean _isSamples; - public MultiDataTypeCrossProjectDataIteratorBuilder(@NotNull User user, @NotNull Container container, @NotNull DataIteratorBuilder in, boolean isCrossType, boolean isCrossFolder, ExpObject dataType, boolean isSamples) + public MultiDataTypeCrossProjectDataIteratorBuilder(@NotNull User user, @NotNull Container container, @NotNull DataIteratorBuilder in, boolean isCrossType, ExpObject dataType, boolean isSamples) { _in = in; _container = container; _user = user; _isCrossType = isCrossType; - _isCrossFolder = isCrossFolder; _dataType = dataType; _isSamples = isSamples; } @@ -3436,7 +3206,7 @@ public DataIterator getDataIterator(DataIteratorContext context) if (di == null) return null; // can happen if context has errors - return LoggingDataIterator.wrap(new MultiDataTypeCrossProjectDataIterator(di, context, _container, _user, _isCrossType, _isCrossFolder, _dataType, _isSamples)); + return LoggingDataIterator.wrap(new MultiDataTypeCrossProjectDataIterator(di, context, _container, _user, _isCrossType, _dataType, _isSamples)); } } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 2c47efa7184..143568a2e5a 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -994,10 +994,11 @@ public DataIterator getDataIterator(DataIteratorContext context) boolean isContainerField = name.equalsIgnoreCase("Container") || name.equalsIgnoreCase("Folder"); if (isContainerField) { - if (isUpdate || !context.isCrossFolderImport()) - drop.add(name); + drop.add(name); + continue; } - else if (ExpDataTable.Column.Name.name().equalsIgnoreCase(name)) + + if (ExpDataTable.Column.Name.name().equalsIgnoreCase(name)) { keysCheck.add(ExpDataTable.Column.Name.name()); } @@ -1227,20 +1228,6 @@ public int importRows(User user, Container container, DataIteratorBuilder rows, return _importRowsUsingDIB(user, container, rows, null, getDataIteratorContext(errors, InsertOption.IMPORT, configParameters), extraScriptContext); } - @Override - public int loadRows(User user, Container container, DataIteratorBuilder rows, DataIteratorContext context, @Nullable Map extraScriptContext) - { - try - { - configureCrossFolderImport(rows, context); - } - catch (IOException e) - { - throw new RuntimeException(e); - } - return super.loadRows(user, container, rows, context, extraScriptContext); - } - @Override public int mergeRows(User user, Container container, DataIteratorBuilder rows, BatchValidationException errors, @Nullable Map configParameters, Map extraScriptContext) { @@ -1365,7 +1352,7 @@ protected Map getRow(User user, Container container, Map> getRows(User user, Container container, List> keys) - throws SQLException + throws SQLException, InvalidKeyException { if (!hasPermission(user, ReadPermission.class)) throw new UnauthorizedException("You do not have permission to read data from this table."); @@ -1402,7 +1389,7 @@ protected Map _select(Container container, Object[] keys) throws throw new IllegalStateException(); } - @Nullable protected Map getDataMap(Map keys, User user, Container container, boolean allowCrossContainer, boolean addInputs) throws SQLException + @Nullable protected Map getDataMap(Map keys, User user, Container container, boolean allowCrossContainer, boolean addInputs) throws SQLException, InvalidKeyException { Long rowId = (Long)JdbcType.BIGINT.convert(keys.get(Column.RowId.name())); String lsid = (String)JdbcType.VARCHAR.convert(keys.get(Column.LSID.name())); @@ -1419,7 +1406,8 @@ protected Map _select(Container container, Object[] keys) throws // as expected to match row inserts and other querySchema data SimpleFilter filter = new SimpleFilter(); if (null != rowId) - filter.addCondition(Column.RowId.fieldKey(), rowId); + filter.addCondition(Column.ClassId.fieldKey(), classId) + .addCondition(Column.RowId.fieldKey(), rowId); else if (null != lsid) filter.addCondition(Column.LSID.fieldKey(), lsid); else @@ -1439,6 +1427,16 @@ else if (null != lsid) } } + if (dataRow == null && allowCrossContainer) + { + // data not found from queryTable but exist in exp.data, which happens when users lack of permission to data's container + if (new TableSelector(ExperimentService.get().getTinfoData(), Collections.singleton(ExpDataTable.Column.RowId.name()), filter, null).exists()) + { + String keyDisplay = name != null ? name : (rowId != null ? "{RowId=" + rowId + "}" : lsid); + throw new InvalidKeyException("Data does not exist in " + container.getName() + ": " + keyDisplay + "."); + } + } + if (!addInputs) return dataRow; @@ -1542,9 +1540,6 @@ protected void preImportDIBValidation(@Nullable DataIteratorBuilder in, @Nullabl @Override public DataIteratorBuilder createImportDIB(User user, Container container, DataIteratorBuilder data, DataIteratorContext context) { - if (context.isCrossFolderImport()) - return new ExpDataIterators.MultiDataTypeCrossProjectDataIteratorBuilder(user, container, data, context.isCrossTypeImport(), context.isCrossFolderImport(), _dataClass, false); - StandardDataIteratorBuilder standard = StandardDataIteratorBuilder.forInsert(getQueryTable(), data, container, user, context); DataIteratorBuilder dib = ((UpdateableTableInfo)getQueryTable()).persistRows(standard, context); dib = AttachmentDataIterator.getAttachmentDataIteratorBuilder(getQueryTable(), dib, user, context.getInsertOption().batch ? getAttachmentDirectory() : null, @@ -1579,32 +1574,5 @@ protected AttachmentParentFactory getAttachmentParentFactory() return (entityId, c) -> new ExpDataClassAttachmentParent(c, Lsid.parse(entityId)); } - @Override - public boolean hasExistingRowsInOtherContainers(Container container, Map> keys) - { - Integer dataClassId = null; - Set dataNames = new HashSet<>(); - for (Map.Entry> keyMap : keys.entrySet()) - { - Object oName = keyMap.getValue().get("Name"); - - if (oName != null) - dataNames.add((String) oName); - - if (dataClassId == null) - { - Object oClassId = keyMap.getValue().get("ClassId"); - if (oClassId != null) - dataClassId = _converter.convert(Integer.class, oClassId); - } - - } - - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ClassId"), dataClassId); - filter.addCondition(FieldKey.fromParts("Name"), dataNames, CompareType.IN); - filter.addCondition(FieldKey.fromParts("Container"), container, CompareType.NEQ); - - return new TableSelector(ExperimentService.get().getTinfoData(), filter, null).exists(); - } } } diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index 892a1295e10..573540e7b27 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -1343,7 +1343,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().getFirst().toString() : "no message"; - assertTrue(msg.contains("Sample does not exist: S-1-absent.")); + assertTrue(msg.contains("Sample does not exist in") && msg.contains("S-1-absent.")); context = new DataIteratorContext(); context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); @@ -1354,7 +1354,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().getFirst().toString() : "no message"; - assertTrue(msg.contains("Sample does not exist: S-1-absent.")); + assertTrue(msg.contains("Sample does not exist in") && msg.contains("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 13774614ac3..61a4be9b3f4 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -96,9 +96,11 @@ import org.labkey.api.query.ValidationException; import org.labkey.api.reader.ColumnDescriptor; import org.labkey.api.reader.DataLoader; +import org.labkey.api.security.ElevatedUser; import org.labkey.api.security.User; import org.labkey.api.security.permissions.MoveEntitiesPermission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.usageMetrics.SimpleMetricsService; @@ -398,8 +400,8 @@ protected Map extractProvidedAmountsAndUnits(@NotNull Map getMaterialMap(Map keys, boolean use return new TableSelector(getQueryTable(), filter, null).getMap(); } - @Override - public boolean hasExistingRowsInOtherContainers(Container container, Map> keys) - { - Long sampleTypeId = null; - Set sampleNames = new HashSet<>(); - for (Map.Entry> keyMap : keys.entrySet()) - { - String name = getMaterialName(keyMap.getValue()); - - if (name != null) - sampleNames.add(name); - - if (sampleTypeId == null) - sampleTypeId = getMaterialSourceId(keyMap.getValue()); - } - - SimpleFilter filter = new SimpleFilter(MaterialSourceId.fieldKey(), sampleTypeId); - filter.addCondition(Name.fieldKey(), sampleNames, CompareType.IN); - filter.addCondition(FieldKey.fromParts("Container"), container, CompareType.NEQ); - - return new TableSelector(ExperimentService.get().getTinfoMaterial(), filter, null).exists(); - } - private record ExistingRowSelect(Set columns, boolean includeParent) {} private @NotNull ExistingRowSelect getExistingRowSelect(@Nullable Set dataColumns) @@ -1051,7 +1029,8 @@ public Map> getExistingRows( { // Issue 52922: cross-folder merge without Product Folders enabled silently ignores the cross-folder // row update. Use a relaxed container filter to find existing data from cross-containers. - ContainerFilter cf = new ContainerFilter.AllInProjectPlusShared(container, user); + // Use elevated user to avoid data being filtered out due to permission + ContainerFilter cf = new ContainerFilter.AllInProjectPlusShared(container, ElevatedUser.getElevatedUser(user, ReaderRole.class)); Set containerIds = new HashSet<>(Objects.requireNonNull(cf.getIds())); containerIds.remove(container.getEntityId()); @@ -1063,7 +1042,7 @@ public Map> getExistingRows( filter.addCondition(FieldKey.fromParts("Container"), containerIds, CompareType.IN); var row = new TableSelector(ExperimentService.get().getTinfoMaterial(), CaseInsensitiveHashSet.of(RowId.name(), Name.name()), filter, null).setMaxRows(1).getMap(); if (row != null) - throw new InvalidKeyException("Sample does not belong to " + container.getName() + " container: " + row.get(Name.name()) + " (" + row.get(RowId.name()) + ")."); + throw new InvalidKeyException("Sample does not exist in " + container.getName() + ": (RowId) " + row.get(RowId.name()) + "."); } if (!missingNames.isEmpty()) @@ -1074,7 +1053,7 @@ public Map> getExistingRows( var row = new TableSelector(ExperimentService.get().getTinfoMaterial(), CaseInsensitiveHashSet.of(Name.name()), filter, null).setMaxRows(1).getMap(); if (row != null) - throw new InvalidKeyException("Sample does not belong to " + container.getName() + " container: " + row.get(Name.name()) + "."); + throw new InvalidKeyException("Sample does not exist in " + container.getName() + ": " + row.get(Name.name()) + "."); } } } @@ -1082,9 +1061,9 @@ public Map> getExistingRows( if (verifyExisting) { if (!missingRowIds.isEmpty()) - throw new InvalidKeyException("Sample does not exist: (RowId) " + missingRowIds.iterator().next() + "."); + throw new InvalidKeyException("Sample does not exist in " + container.getName() + ": (RowId) " + missingRowIds.iterator().next() + "."); if (!missingNames.isEmpty()) - throw new InvalidKeyException("Sample does not exist: " + missingNames.iterator().next() + "."); + throw new InvalidKeyException("Sample does not exist in " + container.getName() + ": " + missingNames.iterator().next() + "."); } // if contains domain fields, check for aliquot specific fields @@ -1281,7 +1260,12 @@ public DataIterator getDataIterator(DataIteratorContext context) boolean isContainerField = name.equalsIgnoreCase(containerFieldLabel); if (!isContainerField) isContainerField = name.equalsIgnoreCase("Container") || name.equalsIgnoreCase("Folder"); - if (isReservedHeader(name) || isContainerField) + if (isContainerField) + { + drop.add(name); + continue; + } + if (isReservedHeader(name)) { // Allow some fields on exp.materials to be loaded by the TabLoader. // Skip over other reserved names 'RowId', 'Run', etc. @@ -1306,8 +1290,6 @@ public DataIterator getDataIterator(DataIteratorContext context) continue; if (isExpMaterialColumn(Units, name)) continue; - if (isContainerField && context.isCrossFolderImport() && !context.getInsertOption().updateOnly) - continue; if (isExpMaterialColumn(RowId, name)) { keysCheck.add(RowId.name()); diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 9a188ba7177..ee5d06d21f5 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -4527,9 +4527,6 @@ protected Set getLineageImportAliases() throws IOException protected void initContext(DataLoader dl, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment) { _context = createDataIteratorContext(_insertOption, getOptionParamsMap(), getLookupResolutionType(), auditBehaviorType, auditUserComment, errors, null, getContainer()); - - if (_context.isCrossFolderImport() && !getContainer().hasProductFolders()) - _context.setCrossFolderImport(false); } @Override