diff --git a/api/src/org/labkey/api/data/NameGenerator.java b/api/src/org/labkey/api/data/NameGenerator.java index bf35e3250c6..c4c2b1aeb84 100644 --- a/api/src/org/labkey/api/data/NameGenerator.java +++ b/api/src/org/labkey/api/data/NameGenerator.java @@ -2099,13 +2099,13 @@ public static class NameGenerationException extends Exception { private final int _rowNumber; - NameGenerationException(String message, int rowNumber) + public NameGenerationException(String message, int rowNumber) { super(message); _rowNumber = rowNumber; } - NameGenerationException(int rowNumber, Throwable t) + public NameGenerationException(int rowNumber, Throwable t) { super(t); _rowNumber = rowNumber; diff --git a/api/src/org/labkey/api/data/NameGeneratorState.java b/api/src/org/labkey/api/data/NameGeneratorState.java index 2b10c77b068..5ee110e36d2 100644 --- a/api/src/org/labkey/api/data/NameGeneratorState.java +++ b/api/src/org/labkey/api/data/NameGeneratorState.java @@ -57,12 +57,12 @@ public class NameGeneratorState implements AutoCloseable { private final @NotNull NameGenerator _nameGenerator; private final boolean _incrementSampleCounts; - private final User _user; + protected final User _user; private final Map _batchExpressionContext; private Function,Map> getSampleCountsFunction; private final Map _newNames = new CaseInsensitiveHashMap<>(); - private int _rowNumber = 0; + protected int _rowNumber = 0; private final Map, Object> _lookupCache; private final Map> _ancestorCache; private final Map> _ancestorSearchCache; @@ -70,11 +70,11 @@ public class NameGeneratorState implements AutoCloseable private final NameGenerator.ProjectSampleCounters _sampleCounters; private boolean _counterSequencesCleaned = false; - private final Container _container; + protected final Container _container; - private final Map materialCache = new HashMap<>(); - private final Map dataCache = new HashMap<>(); - private final RemapCache renameCache; + protected final Map materialCache = new HashMap<>(); + protected final Map dataCache = new HashMap<>(); + protected final RemapCache renameCache; private final Map> objectPropertiesCache = new HashMap<>(); public NameGeneratorState(@NotNull NameGenerator nameGenerator, boolean incrementSampleCounts, NameGenerator.SampleNameExpressionSummary expressionSummary) @@ -292,8 +292,8 @@ private String genName(@NotNull Map rowMap, StringExpressionFactory.FieldKeyStringExpression expression = activeNameGenerator.getParsedNameExpression(); String name; - if (expression instanceof NameGenerator.NameGenerationExpression) - name = ((NameGenerator.NameGenerationExpression) expression).eval(ctx, _prefixCounterSequences); + if (expression instanceof NameGenerator.NameGenerationExpression nge) + name = nge.eval(ctx, _prefixCounterSequences); else name = expression.eval(ctx); if (name == null || name.isEmpty()) @@ -417,14 +417,14 @@ private void addAncestorLookupValues( String parentType = ancestorOptions.parentType(); if (!StringUtils.isEmpty(parentType)) { - if (parentObject instanceof ExpMaterial) + if (parentObject instanceof ExpMaterial expMaterial) { - if (!((ExpMaterial) parentObject).getSampleType().getName().equalsIgnoreCase(parentType)) + if (!expMaterial.getSampleType().getName().equalsIgnoreCase(parentType)) continue; } - else if (parentObject instanceof ExpData) + else if (parentObject instanceof ExpData expData) { - if (!((ExpData) parentObject).getDataClass(_user).getName().equalsIgnoreCase(parentType)) + if (!expData.getDataClass(_user).getName().equalsIgnoreCase(parentType)) continue; } } @@ -490,7 +490,6 @@ else if (parentObject instanceof ExpData) } } } - } private void addParentLookupContext(String parentTypeName/* already decoded */, diff --git a/api/src/org/labkey/api/data/StatementUtils.java b/api/src/org/labkey/api/data/StatementUtils.java index 35da912196f..55ed6e6d7a8 100644 --- a/api/src/org/labkey/api/data/StatementUtils.java +++ b/api/src/org/labkey/api/data/StatementUtils.java @@ -304,7 +304,9 @@ int getPrecision() private final static String pgRowVarPrefix = "$1."; private String makeVariableName(String name) { - return (_dialect.isSqlServer() ? "@p" + (parameters.size()+1) : pgRowVarPrefix) + AliasManager.makeLegalName(name, _dialect); + String shortName = StringUtils.substring(name,0,32); // name is just for readability, make it short + String uniquePrefix = (_dialect.isSqlServer() ? "@" : pgRowVarPrefix) + ("p" + (parameters.size()+1) + "_"); + return uniquePrefix + AliasManager.makeLegalName(shortName, _dialect, true, uniquePrefix.length()); } private String makePgRowTypeName(String variableName) diff --git a/api/src/org/labkey/api/dataiterator/NameExpressionDataIterator.java b/api/src/org/labkey/api/dataiterator/NameExpressionDataIterator.java index 0ead39c9aac..9eddbcbe92a 100644 --- a/api/src/org/labkey/api/dataiterator/NameExpressionDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/NameExpressionDataIterator.java @@ -63,7 +63,6 @@ public NameExpressionDataIterator(DataIterator di, DataIteratorContext context, _container = container; _getNonConflictCountFn = getNonConflictCountFn; _counterSeqPrefix = counterSeqPrefix; - } public NameExpressionDataIterator setAllowUserSpecifiedNames(boolean allowUserSpecifiedNames) @@ -80,7 +79,7 @@ public NameExpressionDataIterator addExtraPropsFn(Supplier> MapDataIterator getInput() { - return (MapDataIterator)_delegate; + return (MapDataIterator) _delegate; } private BatchValidationException getErrors() diff --git a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java index f9aa266de51..5b38a5767e1 100644 --- a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java +++ b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java @@ -37,6 +37,7 @@ enum Column Folder, Inputs, IsAliquot, + IsPlated, LSID, MaterialExpDate, MaterialSourceId, @@ -60,8 +61,7 @@ enum Column SourceProtocolApplication, SourceProtocolLSID, StoredAmount, - Units, - IsPlated; + Units; public FieldKey fieldKey() { diff --git a/api/src/org/labkey/api/query/AliasManager.java b/api/src/org/labkey/api/query/AliasManager.java index 3ea56113e71..857913f8552 100644 --- a/api/src/org/labkey/api/query/AliasManager.java +++ b/api/src/org/labkey/api/query/AliasManager.java @@ -67,7 +67,7 @@ public static String makeLegalName(String str, @NotNull SqlDialect dialect, bool return makeLegalName(str, dialect, truncate, 0); } - private static String makeLegalName(String str, @NotNull SqlDialect dialect, boolean truncate, int reserveCount) + public static String makeLegalName(String str, @NotNull SqlDialect dialect, boolean truncate, int reserveCount) { return dialect.makeLegalName(str, truncate, reserveCount); } diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 1bdb4385312..628feb5231a 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -280,8 +280,8 @@ public ExpMaterialValidatorIterator(DataIterator data, DataIteratorContext conte boolean isUpdateOnly = context.getInsertOption().updateOnly; Map columnNameMap = DataIteratorUtil.createColumnNameMap(data); - if (!isUpdateOnly && columnNameMap.containsKey(ExpMaterial.ALIQUOTED_FROM_INPUT)) - _aliquotedFromColIdx = columnNameMap.get(ExpMaterial.ALIQUOTED_FROM_INPUT); + if (!isUpdateOnly && columnNameMap.containsKey(ALIQUOTED_FROM_INPUT)) + _aliquotedFromColIdx = columnNameMap.get(ALIQUOTED_FROM_INPUT); else if (isUpdateOnly && columnNameMap.containsKey(AliquotedFromLSID.name())) _aliquotedFromColIdx = columnNameMap.get(AliquotedFromLSID.name()); else @@ -378,7 +378,7 @@ protected AliquotRollupDataIterator(DataIterator di, DataIteratorContext context _storedAmountCol = map.get(StoredAmount.name()); _unitsCol = map.get(Units.name()); _sampleStateCol = map.get(SampleState.name()); - _aliquotedFromCol = map.get(ExpMaterial.ALIQUOTED_FROM_INPUT); + _aliquotedFromCol = map.get(ALIQUOTED_FROM_INPUT); _rootMaterialRowIdCol = map.get(RootMaterialRowId.name()); _rootIdToRecomputeCol = map.get(ROOT_RECOMPUTE_ROWID_COL); _parentNameToRecomputeCol = map.get(PARENT_RECOMPUTE_NAME_COL); @@ -492,51 +492,45 @@ public static class AliasDataIteratorBuilder implements DataIteratorBuilder private final Container _container; private final User _user; private final TableInfo _expAliasTable; + private final boolean _isSample; + private final ExpObject _dataType; - public AliasDataIteratorBuilder(@NotNull DataIteratorBuilder in, Container container, User user, TableInfo expTable) + public AliasDataIteratorBuilder(@NotNull DataIteratorBuilder in, Container container, User user, TableInfo expAliasTable, ExpObject dataType, boolean isSample) { _in = in; _container = container; _user = user; - _expAliasTable = expTable; + _expAliasTable = expAliasTable; + _isSample = isSample; + _dataType = dataType; } @Override public DataIterator getDataIterator(DataIteratorContext context) { DataIterator pre = _in.getDataIterator(context); - return LoggingDataIterator.wrap(new AliasDataIterator(pre, context, _container, _user, _expAliasTable)); + return LoggingDataIterator.wrap(new AliasDataIterator(pre, context, _container, _user, _expAliasTable, _dataType, _isSample)); } } - private static class AliasDataIterator extends WrapperDataIterator + private static class AliasDataIterator extends ExpDataTypeDataIterator { // For some reason I don't quite understand we don't want to pass through a column called "alias" so we rename it to ALIASCOLUMNALIAS - final DataIteratorContext _context; - final Supplier _lsidCol; final Supplier _aliasCol; - final Container _container; - final User _user; + final Supplier _lsidCol; + final Supplier _nameCol; Map _lsidAliasMap = new HashMap<>(); private final TableInfo _expAliasTable; - protected AliasDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, TableInfo expTable) + protected AliasDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, TableInfo expAliasTable, ExpObject dataType, boolean isSample) { - super(di); - _context = context; + super(di, context, container, user, dataType, isSample); Map map = DataIteratorUtil.createColumnNameMap(di); - _lsidCol = map.get("lsid")==null ? null : di.getSupplier(map.get("lsid")); - _aliasCol = map.get(ALIASCOLUMNALIAS)==null ? null : di.getSupplier(map.get(ALIASCOLUMNALIAS)); - - _container = container; - _user = user; - _expAliasTable = expTable; - } - - private BatchValidationException getErrors() - { - return _context.getErrors(); + _aliasCol = map.get(ALIASCOLUMNALIAS) == null ? null : di.getSupplier(map.get(ALIASCOLUMNALIAS)); + _lsidCol = map.get("lsid") == null ? null : di.getSupplier(map.get("lsid")); + _nameCol = map.get("name") == null ? null : di.getSupplier(map.get("name")); + _expAliasTable = expAliasTable; } @Override @@ -544,41 +538,59 @@ public boolean next() throws BatchValidationException { boolean hasNext = super.next(); + // skip processing if aliases are not being modified + if (_aliasCol == null) + return hasNext; + // skip processing if there are errors upstream if (getErrors().hasErrors()) return hasNext; - // after the last row, insert all aliases - if (!hasNext) + if (hasNext) { - final ExperimentService svc = ExperimentService.get(); + // Collect alias values and map them by LSID + String lsid = null; - try (DbScope.Transaction transaction = svc.getTinfoDataClass().getSchema().getScope().ensureTransaction()) + if (_nameCol != null && (_context.getInsertOption().mergeRows || _context.getInsertOption().updateOnly)) { - for (Map.Entry entry : _lsidAliasMap.entrySet()) + Object nameValue = _nameCol.get(); + if (nameValue instanceof String name) { - String lsid = entry.getKey(); - Object aliases = entry.getValue(); - AliasInsertHelper.handleInsertUpdate(_container, _user, lsid, _expAliasTable, aliases); + ExpObject obj = getExpObjectByName(name); + if (obj != null) + lsid = obj.getLSID(); } - transaction.commit(); } - return false; + if (lsid == null && _lsidCol != null) + { + Object lsidValue = _lsidCol.get(); + if (lsidValue instanceof String lsidString) + lsid = lsidString; + } + + if (!StringUtils.isEmpty(lsid)) + _lsidAliasMap.put(lsid, _aliasCol.get()); + + return true; } - // For each iteration, collect the lsid and alias col values. - if (_lsidCol != null && _aliasCol != null) - { - Object lsidValue = _lsidCol.get(); - Object aliasValue = _aliasCol.get(); + if (_lsidAliasMap.isEmpty()) + return false; - if (aliasValue != null && lsidValue instanceof String) + // after the last row, insert all aliases + try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) + { + for (Map.Entry entry : _lsidAliasMap.entrySet()) { - _lsidAliasMap.put((String) lsidValue, aliasValue); + String lsid = entry.getKey(); + Object aliases = entry.getValue(); + AliasInsertHelper.handleInsertUpdate(_container, _user, lsid, _expAliasTable, aliases); } + transaction.commit(); } - return true; + + return false; } } @@ -635,7 +647,7 @@ protected AutoLinkToStudyDataIterator(DataIterator di, UserSchema schema, Contai for (String name : nameMap.keySet()) { - if (ExperimentService.isInputOutputColumn(name) || equalsIgnoreCase("parent", name) || equalsIgnoreCase("AliquotedFrom", name)) + if (ExperimentService.isInputOutputColumn(name) || equalsIgnoreCase("parent", name) || equalsIgnoreCase(ALIQUOTED_FROM_INPUT, name)) { _parentCols.add(nameMap.get(name)); } @@ -727,45 +739,22 @@ public DataIterator getDataIterator(DataIteratorContext context) } } - private static class FlagDataIterator extends WrapperDataIterator + private static class FlagDataIterator extends ExpDataTypeDataIterator { final DataIteratorContext _context; - final User _user; final Integer _lsidCol; final Integer _nameCol; final Integer _flagCol; - final boolean _isSample; // as opposed to DataClass - private final ExpSampleType _sampleType; - private final ExpDataClass _dataClass; - final Container _container; - protected FlagDataIterator(DataIterator di, DataIteratorContext context, User user, boolean isSample, ExpObject expObject, Container container) + protected FlagDataIterator(DataIterator di, DataIteratorContext context, User user, boolean isSample, ExpObject dataType, Container container) { - super(di); + super(di, context, container, user, dataType, isSample); _context = context; - _user = user; - _isSample = isSample; Map map = DataIteratorUtil.createColumnNameMap(di); _lsidCol = map.get("lsid"); _nameCol = map.get("name"); _flagCol = map.containsKey("flag") ? map.get("flag") : map.get("comment"); - if (isSample) - { - _sampleType = (ExpSampleType) expObject; - _dataClass = null; - } - else - { - _sampleType = null; - _dataClass = (ExpDataClass) expObject; - } - _container = container; - } - - private BatchValidationException getErrors() - { - return _context.getErrors(); } @Override @@ -779,55 +768,39 @@ public boolean next() throws BatchValidationException if (getErrors().hasErrors()) return true; - if (_lsidCol != null && _flagCol != null) + if (_flagCol == null) + return true; + + ExpObject expObject = null; + if (_nameCol != null && (_context.getInsertOption().mergeRows || _context.getInsertOption().updateOnly)) + { + Object nameValue = get(_nameCol); + if (nameValue instanceof String name) + expObject = getExpObjectByName(name); + } + + if (expObject == null && _lsidCol != null) { Object lsidValue = get(_lsidCol); + if (lsidValue instanceof String lsid) + expObject = getExpObjectByLsid(lsid); + } + if (expObject != null) + { Object flagValue = get(_flagCol); + String flag = Objects.toString(flagValue, null); - if (lsidValue instanceof String lsid) + try { - String flag = Objects.toString(flagValue, null); - - try - { - if (_isSample) - { - ExpMaterial sample = null; - if (_context.getInsertOption() == QueryUpdateService.InsertOption.MERGE && _nameCol != null) - { - Object nameValue = get(_nameCol); - if (nameValue instanceof String) - sample = _sampleType.getSample(_container, (String) nameValue); - } - - if (sample == null) - sample = ExperimentService.get().getExpMaterial(lsid); - if (sample != null) - sample.setComment(_user, flag, false); - } - else - { - ExpData data = null; - if (_context.getInsertOption() == QueryUpdateService.InsertOption.MERGE && _nameCol != null) - { - Object nameValue = get(_nameCol); - if (nameValue instanceof String) - data = _dataClass.getData(_container, (String) nameValue); - } - if (data == null) - data = ExperimentService.get().getExpData(lsid); - if (data != null) - data.setComment(_user, flag, false); - } - } - catch (ValidationException e) - { - throw new BatchValidationException(e); - } + expObject.setComment(_user, flag, false); + } + catch (ValidationException e) + { + throw new BatchValidationException(e); } - } + return true; } } @@ -885,15 +858,16 @@ public DataIterator getDataIterator(DataIteratorContext context) static boolean hasAliquots(int sampleTypeRowId, List names) { - SimpleFilter f = new SimpleFilter(FieldKey.fromParts("Name"), names, IN); - f.addCondition(FieldKey.fromParts("MaterialSourceId"), sampleTypeRowId); - f.addCondition(FieldKey.fromParts(AliquotedFromLSID.name()), null, CompareType.NONBLANK); - return new TableSelector(ExperimentService.get().getTinfoMaterial(), f, null).exists(); + SimpleFilter f = new SimpleFilter(Name.fieldKey(), names, IN); + f.addCondition(MaterialSourceId.fieldKey(), sampleTypeRowId); + f.addCondition(AliquotedFromLSID.fieldKey(), null, CompareType.NONBLANK); + + return new TableSelector(ExperimentService.get().getTinfoMaterial(), Set.of(RowId.name()), f, null).exists(); } public static String getAliquotParent(Object parentObj, DataIteratorContext context, TSVWriter tsvWriter) { - Collection parentNames = getParentNames(parentObj, tsvWriter, "AliquotedFrom", null); + Collection parentNames = getParentNames(parentObj, tsvWriter, ALIQUOTED_FROM_INPUT, null); if (parentNames != null) { List parents = parentNames.stream() @@ -903,7 +877,7 @@ public static String getAliquotParent(Object parentObj, DataIteratorContext cont if (!parents.isEmpty()) { if (parents.size() > 1) - context.getErrors().addRowError(new ValidationException("Multiple AliquotedFrom values are provided.")); + context.getErrors().addRowError(new ValidationException(String.format("Multiple %s values are provided.", ALIQUOTED_FROM_INPUT))); return parents.get(0); } } @@ -920,9 +894,8 @@ static Collection getParentNames(Object parentObj, TSVWriter tsvWriter, return values == null ? null : values.collect(Collectors.toList()); } - static class DerivationDataIteratorBase extends WrapperDataIterator + static class DerivationDataIteratorBase extends ExpDataTypeDataIterator { - final DataIteratorContext _context; final Integer _lsidCol; final Integer _nameCol; final Map _parentCols; @@ -931,46 +904,25 @@ static class DerivationDataIteratorBase extends WrapperDataIterator /** Cache sample type lookups because even though we do caching in SampleTypeService, it's still a lot of overhead to check permissions for the user */ final Map _sampleTypes = new HashMap<>(); final Map _dataClasses = new HashMap<>(); - - final ExpSampleType _currentSampleType; - final ExpDataClass _currentDataClass; - - final Container _container; - final User _user; - final boolean _isSample; final TSVWriter _tsvWriter; protected DerivationDataIteratorBase(DataIterator di, DataIteratorContext context, Container container, User user, ExpObject currentDataType, boolean isSample, boolean checkRequiredParent) { - super(di); - _context = context; - _isSample = isSample; + super(di, context, container, user, currentDataType, isSample); Set requiredParents = new CaseInsensitiveHashSet(); - if (isSample) + + try { - _currentSampleType = (ExpSampleType) currentDataType; - try - { - if (checkRequiredParent) - requiredParents.addAll(_currentSampleType.getRequiredImportAliases().values()); - } - catch (IOException ignore) + if (checkRequiredParent) { + if (isSample()) + requiredParents.addAll(getSampleType().getRequiredImportAliases().values()); + else + requiredParents.addAll(getDataClass().getRequiredImportAliases().values()); } - _currentDataClass = null; } - else + catch (IOException ignore) { - _currentSampleType = null; - _currentDataClass = (ExpDataClass) currentDataType; - try - { - if (checkRequiredParent) - requiredParents.addAll(_currentDataClass.getRequiredImportAliases().values()); - } - catch (IOException ignore) - { - } } Map map = DataIteratorUtil.createColumnNameMap(di); @@ -979,13 +931,11 @@ protected DerivationDataIteratorBase(DataIterator di, DataIteratorContext contex _parentCols = new HashMap<>(); _requiredParentCols = new HashMap<>(); _aliquotParents = new LinkedHashMap<>(); - _container = container; - _user = user; for (Map.Entry entry : map.entrySet()) { String name = entry.getKey(); - if (ExperimentService.isInputOutputColumn(name) || _isSample && equalsIgnoreCase("parent",name)) + if (ExperimentService.isInputOutputColumn(name) || isSample() && equalsIgnoreCase("parent", name)) { _parentCols.put(entry.getValue(), entry.getKey()); if (requiredParents.contains(name)) @@ -1003,11 +953,6 @@ protected int write() }; } - private BatchValidationException getErrors() - { - return _context.getErrors(); - } - protected Set> _getParentParts() { Set> allParts = new HashSet<>(); @@ -1034,13 +979,8 @@ protected Set> _getParentParts() allParts.add(new Pair<>(_parentCols.get(parentCol), null)); } } - return allParts; - } - @Override - public boolean next() throws BatchValidationException - { - return super.next(); + return allParts; } protected void _processRun(ExpRunItem runItem, @@ -1068,7 +1008,7 @@ protected void _processRun(ExpRunItem runItem, if (pair.first == null && pair.second == null) // no parents or children columns provided in input data and no existing parents to be updated return; - if (_isSample && aliquotedFrom == null && !((ExpMaterial) runItem).isOperationPermitted(SampleTypeService.SampleOperations.EditLineage)) + if (isSample() && aliquotedFrom == null && !((ExpMaterial) runItem).isOperationPermitted(SampleTypeService.SampleOperations.EditLineage)) throw new ValidationException(String.format("Sample %s with status %s cannot have its lineage updated.", runItem.getName(), ((ExpMaterial) runItem).getStateLabel())); // the parent columns provided in the input are all empty and there are no existing parents not mentioned in the input that need to be retained. @@ -1094,7 +1034,7 @@ protected void _processRun(ExpRunItem runItem, previousSampleRelatives = clearRunItemSourceRun(_user, runItem, false); } - if (_isSample) + if (isSample()) { ExpMaterial sample = (ExpMaterial) runItem; currentMaterialMap = new HashMap<>(); @@ -1158,29 +1098,13 @@ protected DerivationDataIterator(DataIterator di, DataIteratorContext context, C { super(di, context, container, user, currentDataType, isSample, false /* for insert/merge, required parents are always checked in StandardDataIteratorBuilder */); _skipAliquot = skipAliquot || context.getConfigParameterBoolean(SampleTypeService.ConfigParameters.DeferAliquotRuns); - - Map map = DataIteratorUtil.createColumnNameMap(di); _lsidNames = new HashMap<>(); _parentNames = new LinkedHashMap<>(); _aliquotParents = new LinkedHashMap<>(); _candidateAliquotNames = new ArrayList<>(); - Integer aliquotParentCol = -1; - for (Map.Entry entry : map.entrySet()) - { - String name = entry.getKey(); - if (_isSample && ExpMaterial.ALIQUOTED_FROM_INPUT.equalsIgnoreCase(name)) - { - aliquotParentCol = entry.getValue(); - } - } - - _aliquotParentCol = aliquotParentCol; - } - - private BatchValidationException getErrors() - { - return _context.getErrors(); + Map map = DataIteratorUtil.createColumnNameMap(di); + _aliquotParentCol = isSample() ? map.getOrDefault(ALIQUOTED_FROM_INPUT, -1) : -1; } @Override @@ -1204,7 +1128,7 @@ public boolean next() throws BatchValidationException { Object o = get(_aliquotParentCol); - String aliquotParentName = getAliquotParent(o, _context, _tsvWriter); + String aliquotParentName = getAliquotParent(o, _context, _tsvWriter); if (aliquotParentName != null) _aliquotParents.put(lsid, aliquotParentName.trim()); @@ -1228,19 +1152,15 @@ else if (!_skipAliquot && _context.getInsertOption().mergeRows) { try { - boolean allowBulkLoadRemapCache = true; - if (_context.getConfigParameterBoolean(SkipBulkRemapCache)) - allowBulkLoadRemapCache = false; - - RemapCache cache = new RemapCache(allowBulkLoadRemapCache); + RemapCache cache = new RemapCache(!_context.getConfigParameterBoolean(SkipBulkRemapCache)); Map materialCache = new HashMap<>(); Map dataCache = new HashMap<>(); - if (_isSample && _context.getInsertOption().mergeRows) + if (isSample() && _context.getInsertOption().mergeRows) { if (!_candidateAliquotNames.isEmpty()) { - if (hasAliquots(_currentSampleType.getRowId(), _candidateAliquotNames)) + if (hasAliquots(getSampleType().getRowId(), _candidateAliquotNames)) { // AliquotedFrom is used to determine if aliquot/meta field value should be retained or discarded // In the case of merge, one can argue AliquotedFrom can be queried for existing data, instead of making it a required field. @@ -1259,21 +1179,17 @@ else if (!_skipAliquot && _context.getInsertOption().mergeRows) lsids.addAll(_aliquotParents.keySet()); for (String lsid : lsids) { - Set> parentNames = _parentNames.getOrDefault(lsid, Collections.emptySet()); - ExpRunItem runItem; String aliquotedFrom = _aliquotParents.get(lsid); String dataType = null; - if (_isSample) + if (isSample()) { ExpMaterial m = null; if (_context.getInsertOption().mergeRows) // column lsid generated from dbseq might not be valid for existing materials, lookup by name instead { String sampleName = _lsidNames.get(lsid); if (!StringUtils.isEmpty(sampleName)) - { - m = _currentSampleType.getSample(_container, sampleName); - } + m = getSampleType().getSample(_container, sampleName); } if (m == null) @@ -1293,23 +1209,22 @@ else if (!_skipAliquot && _context.getInsertOption().mergeRows) { String dataName = _lsidNames.get(lsid); if (!StringUtils.isEmpty(dataName)) - { - d = _currentDataClass.getData(_container, dataName); - } + d = getDataClass().getData(_container, dataName); } if (d == null) d = ExperimentService.get().getExpData(lsid); if (d != null) - { dataCache.put(d.getRowId(), d); - } + runItem = d; } + if (runItem == null) // nothing to do if the item does not exist continue; + Set> parentNames = _parentNames.getOrDefault(lsid, Collections.emptySet()); _processRun(runItem, runRecords, parentNames, cache, materialCache, dataCache, aliquotedFrom, dataType, false); } @@ -1329,8 +1244,6 @@ else if (!_skipAliquot && _context.getInsertOption().mergeRows) } return hasNext; } - - } static class ImportWithUpdateDerivationDataIterator extends DerivationDataIteratorBase @@ -1340,7 +1253,6 @@ static class ImportWithUpdateDerivationDataIterator extends DerivationDataIterat final Integer _aliquotParentCol; // Map of Data name and its aliquotedFromLSID final Map _aliquotParents; - final boolean _useLsid; protected ImportWithUpdateDerivationDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, ExpObject currentDataType, boolean isSample, boolean checkRequiredParent) @@ -1350,26 +1262,10 @@ protected ImportWithUpdateDerivationDataIterator(DataIterator di, DataIteratorCo Map map = DataIteratorUtil.createColumnNameMap(di); _parentNames = new LinkedHashMap<>(); _aliquotParents = new LinkedHashMap<>(); - - Integer aliquotParentCol = -1; - for (Map.Entry entry : map.entrySet()) - { - if (_isSample && "AliquotedFromLSID".equalsIgnoreCase(entry.getKey())) - { - aliquotParentCol = entry.getValue(); - } - } - - _aliquotParentCol = aliquotParentCol; - + _aliquotParentCol = isSample() ? map.getOrDefault(AliquotedFromLSID.name(), -1) : -1; _useLsid = map.containsKey("lsid") && context.getConfigParameterBoolean(ExperimentService.QueryOptions.UseLsidForUpdate); } - private BatchValidationException getErrors() - { - return _context.getErrors(); - } - @Override public boolean next() throws BatchValidationException { @@ -1406,7 +1302,7 @@ else if (o instanceof Number) } else { - getErrors().addRowError(new ValidationException("Expected string value for aliquot parent name: " + o, "AliquotedFromLSID")); + getErrors().addRowError(new ValidationException("Expected string value for aliquot parent name: " + o, AliquotedFromLSID.name())); } if (aliquotParentName != null) @@ -1441,7 +1337,6 @@ else if (o instanceof Number) Map materialCache = new HashMap<>(); Map dataCache = new HashMap<>(); - List runRecords = new ArrayList<>(); Set keys = new LinkedHashSet<>(); keys.addAll(_parentNames.keySet()); @@ -1455,25 +1350,25 @@ else if (o instanceof Number) ExpRunItem runItem; String aliquotedFromLSID = _aliquotParents.get(key); String dataType = null; - if (_isSample) + if (isSample()) { - ExpMaterial m = _useLsid ? experimentService.getExpMaterial(key) : _currentSampleType.getSample(_container, key); + ExpMaterial m = _useLsid ? experimentService.getExpMaterial(key) : getSampleType().getSample(_container, key); if (m != null) { materialCache.put(m.getRowId(), m); - dataType = _currentSampleType.getName(); + dataType = getSampleType().getName(); } runItem = m; } else { - ExpData d = _useLsid ? experimentService.getExpData(key) : _currentDataClass.getData(_container, key); + ExpData d = _useLsid ? experimentService.getExpData(key) : getDataClass().getData(_container, key); if (d != null) { dataCache.put(d.getRowId(), d); - dataType = _currentDataClass.getName(); + dataType = getDataClass().getName(); } runItem = d; } @@ -1500,7 +1395,6 @@ else if (o instanceof Number) } return hasNext; } - } private static String checkForLockedSampleRelativeChange(Set previousSampleRelatives, Set currentSampleRelatives, String sampleName, String relationPlural) @@ -3136,7 +3030,6 @@ public MultiDataTypeCrossProjectDataIteratorBuilder(@NotNull User user, @NotNull _isCrossFolder = isCrossFolder; _dataType = dataType; _isSamples = isSamples; - } @Override @@ -3282,5 +3175,4 @@ public boolean next() throws BatchValidationException return true; } } - } diff --git a/experiment/src/org/labkey/experiment/ExpDataTypeDataIterator.java b/experiment/src/org/labkey/experiment/ExpDataTypeDataIterator.java new file mode 100644 index 00000000000..6d55c52a188 --- /dev/null +++ b/experiment/src/org/labkey/experiment/ExpDataTypeDataIterator.java @@ -0,0 +1,91 @@ +package org.labkey.experiment; + +import org.jetbrains.annotations.NotNull; +import org.labkey.api.data.Container; +import org.labkey.api.dataiterator.DataIterator; +import org.labkey.api.dataiterator.DataIteratorContext; +import org.labkey.api.dataiterator.WrapperDataIterator; +import org.labkey.api.exp.api.ExpDataClass; +import org.labkey.api.exp.api.ExpObject; +import org.labkey.api.exp.api.ExpSampleType; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.query.BatchValidationException; +import org.labkey.api.security.User; + +/** + * WrapperDataIterator that operate on ExpObjects and need to track sample/data class information. + */ +public class ExpDataTypeDataIterator extends WrapperDataIterator +{ + protected final DataIteratorContext _context; + protected final Container _container; + protected final User _user; + + private final boolean _isSample; + private final ExpDataClass _dataClass; + private final ExpSampleType _sampleType; + + protected ExpDataTypeDataIterator(DataIterator di, DataIteratorContext context, Container container, User user, ExpObject dataType, boolean isSample) + { + super(di); + _context = context; + _container = container; + _user = user; + _isSample = isSample; + + if (isSample) + { + _sampleType = (ExpSampleType) dataType; + _dataClass = null; + } + else + { + _sampleType = null; + _dataClass = (ExpDataClass) dataType; + } + } + + protected BatchValidationException getErrors() + { + return _context.getErrors(); + } + + protected boolean isSample() + { + return _isSample; + } + + protected ExpObject getExpObjectByLsid(String lsid) + { + ExpObject obj; + if (isSample()) + obj = ExperimentService.get().getExpMaterial(lsid); + else + obj = ExperimentService.get().getExpData(lsid); + return obj; + } + + protected ExpObject getExpObjectByName(String name) + { + ExpObject obj; + if (isSample()) + obj = getSampleType().getSample(_container, name); + else + obj = getDataClass().getData(_container, name); + return obj; + } + + protected @NotNull ExpSampleType getSampleType() + { + if (_sampleType == null) + throw new IllegalStateException("Requested a sample type when the iterator is not for samples. Check isSample() first."); + return _sampleType; + } + + protected @NotNull ExpDataClass getDataClass() + { + if (_dataClass == null) + throw new IllegalStateException("Requested a data class when the iterator is not for data. Check isSample() first."); + return _dataClass; + } +} diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 90408498d8b..e357d02181c 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -955,7 +955,7 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon }, DbScope.CommitTaskOption.POSTCOMMIT)); } DataIteratorBuilder builder = LoggingDataIterator.wrap(step0); - return LoggingDataIterator.wrap(new AliasDataIteratorBuilder(builder, getUserSchema().getContainer(), getUserSchema().getUser(), ExperimentService.get().getTinfoDataAliasMap())); + return LoggingDataIterator.wrap(new AliasDataIteratorBuilder(builder, getUserSchema().getContainer(), getUserSchema().getUser(), ExperimentService.get().getTinfoDataAliasMap(), _dataClass, false)); } catch (IOException e) { @@ -1375,7 +1375,8 @@ protected Map _update(User user, Container c, Map row = new CaseInsensitiveHashMap<>(); row.put("aa", 30); row.put("bb", "bye"); + String expectedComment = "waving in the wind"; + row.put("flag", expectedComment); rows.add(row); List> ret; @@ -317,6 +319,7 @@ private void testInsertIntoSubfolder(ExpDataClassImpl dataClass, TableInfo table assertNotNull(data); assertEquals(sub, data.getContainer()); assertEquals(2, dataClass.getDatas().size()); + assertEquals(expectedComment, data.getComment()); } private void testInsertDuplicate(ExpDataClassImpl dataClass, TableInfo table) throws Exception @@ -1041,9 +1044,9 @@ public void testInsertOptionUpdate() throws Exception ExperimentServiceImpl.get().createDataClass(c, user, dataClassName, null, props, emptyList(), null, null); List> rowsToAdd = new ArrayList<>(); - rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1", "prop", "a", longFieldName, "Very")); - rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1-d", "prop", "c", longFieldName, "Long")); - rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-2", "prop", "b", longFieldName, "Field")); + rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1", "prop", "a", longFieldName, "Very", "alias", "Much", "flag", "c100")); + rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1-d", "prop", "c", longFieldName, "Long", "alias", "Extended", "flag", "c200")); + rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-2", "prop", "b", longFieldName, "Field", "alias", "Paddock")); TableInfo table = getDataClassTable(dataClassName); QueryUpdateService qus = table.getUpdateService(); @@ -1066,10 +1069,11 @@ public void testInsertOptionUpdate() throws Exception // update regular properties as well as datainputs List> rowsToUpdate = new ArrayList<>(); - rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "D-1", "prop", "a1", "DataInputs/DataClassWithImportOption", null)); + rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "D-1", "prop", "a1", "DataInputs/DataClassWithImportOption", null, "alias", "A lot")); rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "D-1-d", "prop", "c1", "DataInputs/DataClassWithImportOption", "D-1")); - rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "D-2", "prop", "b1", "DataInputs/DataClassWithImportOption", null)); + rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "D-2", "prop", "b1", "DataInputs/DataClassWithImportOption", null, "alias", "Grassland")); + context = new DataIteratorContext(); context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); count = qus.loadRows(user, c, MapDataIterator.of(rowsToUpdate), context, null); @@ -1077,21 +1081,98 @@ public void testInsertOptionUpdate() throws Exception assertEquals(3, count); Set columnNames = new HashSet<>(); + columnNames.add("rowId"); + columnNames.add("lsid"); columnNames.add("Name"); columnNames.add("prop"); columnNames.add(longFieldName); + columnNames.add("alias"); + columnNames.add("flag"); - List> rows = Arrays.asList(new TableSelector(table, columnNames, null, new Sort("Name")).getMapArray()); + ts = new TableSelector(table, columnNames, null, new Sort("Name")); + ts.setForDisplay(true); + String aliasAlias = "alias$alias$name"; + String flagAlias = "flag$"; + + List> rows = Arrays.asList(ts.getMapArray()); + + assertEquals("D-1", rows.get(0).get("Name")); + assertEquals("D-1-d", rows.get(1).get("Name")); + assertEquals("D-2", rows.get(2).get("Name")); + int d2RowId = (Integer) rows.get(2).get("RowId"); + + // prop assertEquals("a1", rows.get(0).get("prop")); assertEquals("c1", rows.get(1).get("prop")); assertEquals("b1", rows.get(2).get("prop")); + + // long field assertEquals("Very", rows.get(0).get(longFieldAlias)); assertEquals("Long", rows.get(1).get(longFieldAlias)); assertEquals("Field", rows.get(2).get(longFieldAlias)); + // alias + assertEquals("A lot", rows.get(0).get(aliasAlias)); + assertNull(rows.get(1).get(aliasAlias)); + assertEquals("Grassland", rows.get(2).get(aliasAlias)); + + // flag + assertEquals("c100", rows.get(0).get(flagAlias)); + assertEquals("c200", rows.get(1).get(flagAlias)); + assertNull(rows.get(2).get(flagAlias)); + ts = new TableSelector(dataInputTInfo, filter, null); - int newInputCount = (int) ts.getRowCount(); + int newInputCount = (int) ts.getRowCount(); assertEquals(inputCount + 1, newInputCount); + + List> rowsToMerge = new ArrayList<>(); + rowsToMerge.add(CaseInsensitiveHashMap.of("name", "D-2", "prop", "gene", longFieldName, "Pasture", "alias", "Exceedingly", "flag", "cOne")); + rowsToMerge.add(CaseInsensitiveHashMap.of("name", "D-22", "prop", null, longFieldName, "Goal", "alias", "Immensely", "flag", "cEight")); + + context = new DataIteratorContext(); + context.setInsertOption(QueryUpdateService.InsertOption.MERGE); + count = qus.loadRows(user, c, MapDataIterator.of(rowsToMerge), context, null); + + assertFalse(context.getErrors().hasErrors()); + assertEquals(2, count); + + ts = new TableSelector(table, columnNames, null, new Sort("Name")); + ts.setForDisplay(true); + + rows = Arrays.asList(ts.getMapArray()); + assertEquals(4, rows.size()); + + assertEquals("D-1", rows.get(0).get("Name")); + assertEquals("D-1-d", rows.get(1).get("Name")); + assertEquals("D-2", rows.get(2).get("Name")); + assertEquals("D-22", rows.get(3).get("Name")); + + // verify merge retained existing row + assertEquals(d2RowId, rows.get(2).get("RowId")); + + // prop + assertEquals("a1", rows.get(0).get("prop")); + assertEquals("c1", rows.get(1).get("prop")); + assertEquals("gene", rows.get(2).get("prop")); + assertNull(rows.get(3).get("prop")); + + // long field + assertEquals("Very", rows.get(0).get(longFieldAlias)); + assertEquals("Long", rows.get(1).get(longFieldAlias)); + assertEquals("Pasture", rows.get(2).get(longFieldAlias)); + assertEquals("Goal", rows.get(3).get(longFieldAlias)); + + // alias + assertEquals("A lot", rows.get(0).get(aliasAlias)); + assertNull(rows.get(1).get(aliasAlias)); + assertEquals("Exceedingly", rows.get(2).get(aliasAlias)); + assertEquals("Immensely", rows.get(3).get(aliasAlias)); + + // flag + assertEquals("c100", rows.get(0).get(flagAlias)); + assertEquals("c200", rows.get(1).get(flagAlias)); + assertEquals("cOne", rows.get(2).get(flagAlias)); + assertEquals("cEight", rows.get(3).get(flagAlias)); } private @NotNull TableInfo getDataClassTable(String dataClassName) @@ -1145,7 +1226,9 @@ public void testUpdateAuditForLongField() throws Exception List> rowsToUpdate = new ArrayList<>(); rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "A-1", fieldName, "Updated")); + context = new DataIteratorContext(); context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); + context.getConfigParameters().put(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, AuditBehaviorType.DETAILED); count = qus.loadRows(user, c, MapDataIterator.of(rowsToUpdate), context, null); assertFalse("Unexpected errors from update", context.getErrors().hasErrors()); assertEquals("Number of rows updated not as expected", 1, count); @@ -1160,7 +1243,5 @@ public void testUpdateAuditForLongField() throws Exception String oldRecordMap = ((DetailedAuditTypeEvent) events.get(0)).getOldRecordMap(); assertTrue("Old record map (" + oldRecordMap + ") does not contain expected field", oldRecordMap.contains(encodeURIComponent(fieldName.toLowerCase()) + "=Initial")); } - } - %> diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index a91f7f8ef55..4bfa864166b 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -1627,7 +1627,7 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon } DataIteratorBuilder builder = LoggingDataIterator.wrap(persist); - return LoggingDataIterator.wrap(new AliasDataIteratorBuilder(builder, getUserSchema().getContainer(), getUserSchema().getUser(), ExperimentService.get().getTinfoMaterialAliasMap())); + return LoggingDataIterator.wrap(new AliasDataIteratorBuilder(builder, getUserSchema().getContainer(), getUserSchema().getUser(), ExperimentService.get().getTinfoMaterialAliasMap(), _ss, true)); } catch (IOException e) { diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index 37c86a68589..fcd8981bddf 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -17,8 +17,6 @@ <%@ page import="org.apache.commons.lang3.StringUtils" %> <%@ page import="org.junit.After" %> <%@ page import="org.junit.Before" %> - - <%@ page import="org.junit.Test" %> <%@ page import="org.labkey.api.audit.AuditLogService" %> <%@ page import="org.labkey.api.audit.SampleTimelineAuditEvent" %> @@ -51,7 +49,6 @@ <%@ page import="org.labkey.api.exp.query.ExpSchema" %> <%@ page import="org.labkey.api.exp.query.SamplesSchema" %> <%@ page import="org.labkey.api.gwt.client.AuditBehaviorType" %> - <%@ page import="org.labkey.api.gwt.client.model.GWTPropertyDescriptor" %> <%@ page import="org.labkey.api.query.BatchValidationException" %> <%@ page import="org.labkey.api.query.DefaultSchema" %> @@ -59,7 +56,6 @@ <%@ page import="org.labkey.api.query.QuerySchema" %> <%@ page import="org.labkey.api.query.QueryService" %> <%@ page import="org.labkey.api.query.QueryUpdateService" %> - <%@ page import="static org.hamcrest.CoreMatchers.hasItems" %> <%@ page import="static org.junit.Assert.*" %> <%@ page import="org.labkey.api.query.SchemaKey" %> @@ -93,14 +89,9 @@ <%@ page import="java.util.concurrent.TimeUnit" %> <%@ page import="org.jetbrains.annotations.NotNull" %> <%@ page import="org.labkey.api.dataiterator.MapDataIterator" %> - <%@ page extends="org.labkey.api.jsp.JspTest.BVT" %> <%! -/** - * User: kevink - * Date: 11/24/16 - */ private static final String PROJECT_NAME = "_testSampleType"; private final ExpProvisionedTableTestHelper helper = new ExpProvisionedTableTestHelper(); @@ -1205,6 +1196,7 @@ public void testInsertOptionUpdate() throws Exception rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "S-1-1", "intVal", null)); rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "S-2", "intVal", 200)); + context = new DataIteratorContext(); context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); count = qus.loadRows(user, c, MapDataIterator.of(rowsToUpdate), context, null); @@ -1238,6 +1230,8 @@ public void testInsertOptionUpdate() throws Exception // update a sample that doesn't exist should throw error rowsToUpdate = new ArrayList<>(); rowsToUpdate.add(CaseInsensitiveHashMap.of("name", "S-1-absent", "intVal", 100)); + context = new DataIteratorContext(); + context.setInsertOption(QueryUpdateService.InsertOption.UPDATE); 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"; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index b8ff3fd8425..36edcf87a17 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -44,6 +44,7 @@ import org.labkey.api.data.MultiValuedForeignKey; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.NameGeneratorState; +import org.labkey.api.data.RemapCache; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Table; @@ -133,6 +134,7 @@ import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs; import static org.labkey.api.dataiterator.SampleUpdateAddColumnsDataIterator.CURRENT_SAMPLE_STATUS_COLUMN_NAME; import static org.labkey.api.exp.api.ExpRunItem.PARENT_IMPORT_ALIAS_MAP_PROP; +import static org.labkey.api.exp.api.ExperimentService.QueryOptions.SkipBulkRemapCache; import static org.labkey.api.exp.api.SampleTypeDomainKind.ALIQUOT_ROLLUP_FIELD_LABELS; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipAliquotRollup; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipMaxSampleCounterFunction; @@ -338,9 +340,9 @@ public boolean next() throws BatchValidationException Object nameObj = (_delegate).get(parentNameToRecomputeCol); if (nameObj != null) { - if (nameObj instanceof String) + if (nameObj instanceof String name) { - nameToRecompute.add((String) nameObj); + nameToRecompute.add(name); } else if (nameObj instanceof Number) { @@ -807,7 +809,7 @@ else if (!isAliquot && isAliquotField) validRowCopy.put(updateField, updateValue); } - if (ExpMaterialTable.Column.SampleState.name().toLowerCase().equals(updateField.toLowerCase())) + if (ExpMaterialTable.Column.SampleState.name().equalsIgnoreCase(updateField)) hasStatusCol = true; } // had a locked status before and either not updating the status or updating to a new locked status @@ -989,9 +991,9 @@ private Map getMaterialMap(Integer rowId, String lsid, User user { Filter filter; if (rowId != null) - filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId), rowId); + filter = new SimpleFilter(ExpMaterialTable.Column.RowId.fieldKey(), rowId); else if (lsid != null) - filter = new SimpleFilter(FieldKey.fromParts(LSID), lsid); + filter = new SimpleFilter(LSID.fieldKey(), lsid); else throw new QueryUpdateServiceException("Either RowId or LSID is required to get Sample Type Material."); @@ -1072,8 +1074,8 @@ public boolean hasExistingRowsInOtherContainers(Container container, Map columns, boole remap = CaseInsensitiveHashMap.of(); // AliquotRollupDataIterator needs "samplestate", "storedamount", "rootmaterialrowId", "units" for MERGE option - Set includedColumns = new CaseInsensitiveHashSet("name", "lsid", "rowid", "samplestate", "storedamount", "rootmaterialrowId", "units"); + Set includedColumns = new CaseInsensitiveHashSet(Name.name(), LSID.name(), RowId.name(), SampleState.name(), StoredAmount.name(), RootMaterialRowId.name(), Units.name()); for (ColumnInfo column : getQueryTable().getColumns()) { if (dataColumns.contains(column.getColumnName())) @@ -1132,7 +1134,6 @@ else if (dataColumns.contains(remap.get(column.getColumnName()))) hasParentInput = true; break; } - } } catch (IOException ignored) @@ -1184,7 +1185,7 @@ else if (name != null && materialSourceId != null) if (!rowIdRowNumMap.isEmpty()) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId), rowIdRowNumMap.keySet(), CompareType.IN); + SimpleFilter filter = new SimpleFilter(ExpMaterialTable.Column.RowId.fieldKey(), rowIdRowNumMap.keySet(), CompareType.IN); filter.addCondition(FieldKey.fromParts("Container"), container); Map[] rows = new TableSelector(queryTableInfo, selectColumns, filter, null).getMapArray(); for (Map row : rows) @@ -1222,8 +1223,8 @@ else if (name != null && materialSourceId != null) if (!nameRowNumMap.isEmpty()) { allKeys.addAll(nameRowNumMap.keySet()); - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("MaterialSourceId"), sampleTypeId); - filter.addCondition(FieldKey.fromParts("Name"), nameRowNumMap.keySet(), CompareType.IN); + SimpleFilter filter = new SimpleFilter(ExpMaterialTable.Column.MaterialSourceId.fieldKey(), sampleTypeId); + filter.addCondition(ExpMaterialTable.Column.Name.fieldKey(), nameRowNumMap.keySet(), CompareType.IN); filter.addCondition(FieldKey.fromParts("Container"), container); Map[] rows = new TableSelector(queryTableInfo, selectColumns, filter, null).getMapArray(); for (Map row : rows) @@ -1243,7 +1244,7 @@ else if (name != null && materialSourceId != null) // Issue 52922: cross folder merge without Product Folders enabled silently ignores the cross folder row update ContainerFilter allCf = new ContainerFilter.AllInProjectPlusShared(container, user); // use a relaxed CF to find existing data from cross containers - SimpleFilter existingDataFilter = new SimpleFilter(FieldKey.fromParts("MaterialSourceId"), sampleTypeId); + SimpleFilter existingDataFilter = new SimpleFilter(ExpMaterialTable.Column.MaterialSourceId.fieldKey(), sampleTypeId); existingDataFilter.addCondition(FieldKey.fromParts("Container"), allCf.getIds(), CompareType.IN); existingDataFilter.addCondition(FieldKey.fromParts(useLsid ? "LSID" : "Name"), allKeys, CompareType.IN); @@ -1254,7 +1255,6 @@ else if (name != null && materialSourceId != null) if (!dataContainer.equals(container.getId())) throw new InvalidKeyException("Sample does not belong to " + container.getName() + " container: " + row.get("name") + "."); } - } if (verifyExisting && !allKeys.isEmpty()) @@ -1508,7 +1508,7 @@ public DataIterator getDataIterator(DataIteratorContext context) DataIterator c = LoggingDataIterator.wrap(new _SamplesCoerceDataIterator(source, context, sampleType, materialTable)); context.setWithLookupRemapping(false); SimpleTranslator addColumns = new SimpleTranslator(c, context); - addColumns.setDebugName("add genId and other requried columns"); + addColumns.setDebugName("add genId and other required columns"); Set idColNames = Sets.newCaseInsensitiveHashSet("genId"); materialTable.getColumns().stream().filter(ColumnInfo::isUniqueIdField).forEach(columnInfo -> idColNames.add(columnInfo.getName())); addColumns.selectAll(idColNames); @@ -1533,14 +1533,7 @@ public DataIterator getDataIterator(DataIteratorContext context) // sampleset.createSampleNames() + generate lsid // TODO: does not handle insertIgnore - DataIterator names = new _GenerateNamesDataIterator(sampleType, container, user, DataIteratorUtil.wrapMap(dataIterator, false), context, batchSize) - .setAllowUserSpecifiedNames(NameExpressionOptionService.get().getAllowUserSpecificNamesValue(container)) - .addExtraPropsFn(() -> { - if (container != null) - return Map.of(NameExpressionOptionService.FOLDER_PREFIX_TOKEN, StringUtils.trimToEmpty(NameExpressionOptionService.get().getExpressionPrefix(container))); - else - return Collections.emptyMap(); - }); + DataIterator names = new _GenerateNamesDataIterator(sampleType, container, user, DataIteratorUtil.wrapMap(dataIterator, false), context, batchSize); return LoggingDataIterator.wrap(names); } @@ -1615,76 +1608,76 @@ private static boolean isAliquotRollupHeader(String name) static class _GenerateNamesDataIterator extends SimpleTranslator { - final ExpSampleTypeImpl sampleType; - final SampleNameGeneratorState nameState; + final boolean _allowUserSpecifiedNames; // whether manual names specification is allowed or only name expression generation + final int _batchSize; + final RemapCache _cache; + final Container _container; + final List>> _extraPropsFns; + final Map _materialCache; + final SampleNameGeneratorState _nameState; final Lsid.LsidBuilder lsidBuilder; final DbSequence _lsidDbSeq; - final Container _container; - final int _batchSize; - boolean first = true; - Map importAliasMap = null; - boolean _allowUserSpecifiedNames = true; // whether manual names specification is allowed or only name expression generation - Set _existingNames = null; - List>> _extraPropsFns = new ArrayList<>(); + final ExpSampleTypeImpl _sampleType; + final User _user; + Set _existingNames = null; String generatedName = null; - _GenerateNamesDataIterator(ExpSampleTypeImpl sampleType, Container dataContainer, User user, MapDataIterator source, DataIteratorContext context, int batchSize) + _GenerateNamesDataIterator( + @NotNull ExpSampleTypeImpl sampleType, + Container container, + User user, + MapDataIterator source, + DataIteratorContext context, + int batchSize + ) { super(source, context); - this.sampleType = sampleType; + _allowUserSpecifiedNames = NameExpressionOptionService.get().getAllowUserSpecificNamesValue(container); + _cache = new RemapCache(!context.getConfigParameterBoolean(SkipBulkRemapCache)); + _container = container; + _extraPropsFns = new ArrayList<>(); + _materialCache = new HashMap<>(); + _sampleType = sampleType; + _user = user; + try { - this.importAliasMap = sampleType.getImportAliases(); - _extraPropsFns.add(() -> { - if (this.importAliasMap != null) - return Map.of(PARENT_IMPORT_ALIAS_MAP_PROP, this.importAliasMap); - else - return Collections.emptyMap(); - }); + Map importAliasMap = sampleType.getImportAliases(); + _extraPropsFns.add(() -> Map.of(PARENT_IMPORT_ALIAS_MAP_PROP, importAliasMap)); } catch (IOException e) { // do nothing } + + _extraPropsFns.add(() -> { + if (_container == null) + return Collections.emptyMap(); + return Map.of(NameExpressionOptionService.FOLDER_PREFIX_TOKEN, StringUtils.trimToEmpty(NameExpressionOptionService.get().getExpressionPrefix(_container))); + }); + boolean skipDuplicateCheck = context.getConfigParameterBoolean(SkipMaxSampleCounterFunction); - nameState = sampleType.getNameGenState(skipDuplicateCheck, true, dataContainer, user); + _nameState = sampleType.getNameGenState(skipDuplicateCheck, true, _container, user); lsidBuilder = sampleType.generateSampleLSID(); - _container = sampleType.getContainer(); _batchSize = batchSize; - if (context.getConfigParameterBoolean(ExperimentService.QueryOptions.UseLsidForUpdate)) + boolean useLsidForUpdate = context.getConfigParameterBoolean(ExperimentService.QueryOptions.UseLsidForUpdate); + if (useLsidForUpdate) selectAll(CaseInsensitiveHashSet.of(Name.name(), RootMaterialRowId.name())); else selectAll(CaseInsensitiveHashSet.of(Name.name(), LSID.name(), RootMaterialRowId.name())); - _lsidDbSeq = sampleType.getSampleLsidDbSeq(_batchSize, _container); + _lsidDbSeq = sampleType.getSampleLsidDbSeq(_batchSize, sampleType.getContainer()); - addColumn(new BaseColumnInfo("name", JdbcType.VARCHAR), (Supplier)() -> generatedName); - if (!context.getConfigParameterBoolean(ExperimentService.QueryOptions.UseLsidForUpdate)) - addColumn(new BaseColumnInfo("lsid", JdbcType.VARCHAR), (Supplier)() -> lsidBuilder.setObjectId(String.valueOf(_lsidDbSeq.next())).toString()); + addColumn(new BaseColumnInfo("name", JdbcType.VARCHAR), (Supplier)() -> generatedName); + if (!useLsidForUpdate) + addColumn(new BaseColumnInfo("lsid", JdbcType.VARCHAR), (Supplier)() -> lsidBuilder.setObjectId(String.valueOf(_lsidDbSeq.next())).toString()); // Ensure we have a cpasType column and it is of the right value addColumn(new BaseColumnInfo("cpasType", JdbcType.VARCHAR), new SimpleTranslator.ConstantColumn(sampleType.getLSID())); addColumn(new BaseColumnInfo("materialSourceId", JdbcType.INTEGER), new SimpleTranslator.ConstantColumn(sampleType.getRowId())); } - _GenerateNamesDataIterator setAllowUserSpecifiedNames(boolean allowUserSpecifiedNames) - { - _allowUserSpecifiedNames = allowUserSpecifiedNames; - return this; - } - - _GenerateNamesDataIterator addExtraPropsFn(Supplier> extraProps) - { - _extraPropsFns.add(extraProps); - return this; - } - - void onFirst() - { - first = false; - } - @Override protected void processNextInput() { @@ -1728,7 +1721,7 @@ else if (aliquotedFromObj instanceof Number) } } - generatedName = nameState.nextName(map, _extraPropsFns); + generatedName = _nameState.nextName(map, _extraPropsFns); } catch (NameGenerator.DuplicateNameException dup) @@ -1739,34 +1732,25 @@ else if (aliquotedFromObj instanceof Number) { // Failed to generate a name due to some part of the expression not in the row if (isAliquot) - { - addRowError("Failed to generate name for aliquot on row " + e.getRowNumber() + " using aliquot naming pattern " + sampleType.getAliquotNameExpression() + ". Check the syntax of the aliquot naming pattern and the data values for the aliquot."); - } + addRowError("Failed to generate name for aliquot on row " + e.getRowNumber() + " using aliquot naming pattern " + _sampleType.getAliquotNameExpression() + ". Check the syntax of the aliquot naming pattern and the data values for the aliquot."); + else if (_sampleType.hasNameExpression()) + addRowError("Failed to generate name for sample on row " + e.getRowNumber() + " using naming pattern " + _sampleType.getNameExpression() + ". Check the syntax of the naming pattern and the data values for the sample."); + else if (_sampleType.hasNameAsIdCol()) + addRowError("SampleID or Name is required for sample on row " + e.getRowNumber()); else - { - if (sampleType.hasNameExpression()) - addRowError("Failed to generate name for sample on row " + e.getRowNumber() + " using naming pattern " + sampleType.getNameExpression() + ". Check the syntax of the naming pattern and the data values for the sample."); - else if (sampleType.hasNameAsIdCol()) - addRowError("SampleID or Name is required for sample on row " + e.getRowNumber()); - else - addRowError("All id columns are required for sample on row " + e.getRowNumber()); - } + addRowError("All id columns are required for sample on row " + e.getRowNumber()); } } @Override public boolean next() throws BatchValidationException { - // consider add onFirst() as callback from SimpleTranslator - if (first) - onFirst(); - // calls processNextInput() boolean next = super.next(); if (!next) { - if (null != nameState) - nameState.cleanUp(); + if (null != _nameState) + _nameState.cleanUp(); } return next; } @@ -1775,8 +1759,8 @@ public boolean next() throws BatchValidationException public void close() throws IOException { super.close(); - if (null != nameState) - nameState.close(); + if (null != _nameState) + _nameState.close(); } private boolean rowExists(String name) @@ -1784,8 +1768,8 @@ private boolean rowExists(String name) if (_existingNames == null) { _existingNames = new HashSet<>(); - SamplesSchema schema = new SamplesSchema(User.getSearchUser(), _container); - TableSelector ts = new TableSelector(schema.getTable(sampleType, null), Collections.singleton("Name")).setMaxRows(1_000_000); + SamplesSchema schema = new SamplesSchema(User.getSearchUser(), _sampleType.getContainer()); + TableSelector ts = new TableSelector(schema.getTable(_sampleType, null), Collections.singleton("Name")).setMaxRows(1_000_000); ts.fillSet(_existingNames); } return _existingNames.contains(name); @@ -1795,7 +1779,7 @@ private boolean rowExists(String name) static class _SamplesCoerceDataIterator extends SimpleTranslator { private static final String INVALID_ALIQUOT_PROPERTY = "An aliquot-specific property [%1$s] value has been ignored for a non-aliquot sample."; - private static final String INVALID_NONALIQUOT_PROPERTY = "A sample property [%1$s] value has been ignored for an aliquot."; + private static final String INVALID_NON_ALIQUOT_PROPERTY = "A sample property [%1$s] value has been ignored for an aliquot."; private final ExpSampleTypeImpl _sampleType; private final Measurement.Unit _metricUnit; @@ -1832,9 +1816,9 @@ void init(TableInfo target, boolean useImportAliases, boolean initRollupCounts) { if (getAliquotedFromColName().equalsIgnoreCase(from.getName())) aliquotedFromDataColInd = i; - else if ("Units".equalsIgnoreCase(from.getName())) + else if (Units.name().equalsIgnoreCase(from.getName())) unitDataColInd = i; - else if ("StoredAmount".equalsIgnoreCase(from.getName()) || "Amount".equalsIgnoreCase(from.getName())) + else if (StoredAmount.name().equalsIgnoreCase(from.getName()) || "Amount".equalsIgnoreCase(from.getName())) amountDataColInd = i; } } @@ -1850,7 +1834,7 @@ else if ("StoredAmount".equalsIgnoreCase(from.getName()) || "Amount".equalsIgnor boolean isScopedField = scopedFields.containsKey(name); String ignoredAliquotPropValue = String.format(INVALID_ALIQUOT_PROPERTY, name); - String ignoredMetaPropValue = String.format(INVALID_NONALIQUOT_PROPERTY, name); + String ignoredMetaPropValue = String.format(INVALID_NON_ALIQUOT_PROPERTY, name); if (to.getPropertyType() == PropertyType.ATTACHMENT || to.getPropertyType() == PropertyType.FILE_LINK) { if (isScopedField) @@ -1873,11 +1857,11 @@ else if (to.getFk() instanceof MultiValuedForeignKey) else addColumn(to.getName(), i); } - else if (name.equalsIgnoreCase("Units")) + else if (Units.name().equalsIgnoreCase(name)) { addColumn(to, new SampleUnitsConvertColumn(name, i, to.getJdbcType())); } - else if (name.equalsIgnoreCase("StoredAmount")) + else if (StoredAmount.name().equalsIgnoreCase(name)) { addColumn(to, new SampleAmountConvertColumn(name, i, to.getJdbcType())); } @@ -1933,7 +1917,7 @@ private void _addConvertColumn(String name, int fromIndex, JdbcType toType, Fore private void _addConvertColumn(ColumnInfo col, int fromIndex, int derivationDataColInd, boolean isAliquotField) { SimpleConvertColumn c = createConvertColumn(col, fromIndex, RemapMissingBehavior.Error); - c = new DerivationScopedConvertColumn(fromIndex, c, derivationDataColInd, isAliquotField, String.format(INVALID_ALIQUOT_PROPERTY, col.getName()), String.format(INVALID_NONALIQUOT_PROPERTY, col.getName())); + c = new DerivationScopedConvertColumn(fromIndex, c, derivationDataColInd, isAliquotField, String.format(INVALID_ALIQUOT_PROPERTY, col.getName()), String.format(INVALID_NON_ALIQUOT_PROPERTY, col.getName())); addColumn(col, c); } @@ -1993,7 +1977,7 @@ else if (aliquotedFrom instanceof Number) { aliquotParentName = aliquotedFrom.toString(); } - if (aliquotParentName == null || StringUtils.isEmpty(aliquotParentName)) // if AliquotedFrom is empty, is root + if (StringUtils.isEmpty(aliquotParentName)) // if AliquotedFrom is empty, is root return 0; return null; // for aliquot, initialize rollup count/amount to null