diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 0cffb184810..d111fb55845 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -303,7 +303,7 @@ else if (mvIndicatorColumns.contains(column.name)) { // Allow String values through if the column is a lookup and the settings allow lookups by alternate key. // The lookup table unique indices or display column value will be used to convert the column to the lookup value. - if (!(settings.isAllowLookupByAlternateKey() && column.clazz == String.class && prop.getLookup() != null)) + if (!(settings.getLookupResolutionType().useAlternateKey() && column.clazz == String.class && prop.getLookup() != null)) { // Otherwise, just use the expected PropertyDescriptor's column type column.clazz = prop.getPropertyDescriptor().getPropertyType().getJavaType(); @@ -771,7 +771,7 @@ else if (sampleLookup.isLookup()) } } - if (dataTable != null && settings.isAllowLookupByAlternateKey()) + if (dataTable != null && settings.getLookupResolutionType().useAlternateKey()) { ColumnInfo column = dataTable.getColumn(pd.getName()); ForeignKey fk = column != null ? column.getFk() : null; @@ -993,7 +993,7 @@ else if (o != remapped) // If allowLookupByAlternateKey is true or the sample lookup is by name, we call findExpMaterial which will attempt to resolve by name first and then rowId. // If allowLookupByAlternateKey is false, we will only try resolving by the rowId. ExpMaterial material = null; - if (settings.isAllowLookupByAlternateKey() || isSampleLookupByName) + if (settings.getLookupResolutionType().useAlternateKey() || isSampleLookupByName) { String materialName = o.toString(); if (inputMaterials.containsKey(materialName)) diff --git a/api/src/org/labkey/api/data/LookupResolutionType.java b/api/src/org/labkey/api/data/LookupResolutionType.java new file mode 100644 index 00000000000..7801cbe77b5 --- /dev/null +++ b/api/src/org/labkey/api/data/LookupResolutionType.java @@ -0,0 +1,20 @@ +package org.labkey.api.data; + +public enum LookupResolutionType +{ + primaryKey(false), // known that the user will always supply the pk value + alternateThenPrimaryKey(true); // If there is a situation where it's sometimes a primary and sometimes an alternate key, check for the alternate key first + + final boolean _useAlternateKey; + + LookupResolutionType(boolean useAlternateKey) + { + _useAlternateKey = useAlternateKey; + } + + public boolean useAlternateKey() + { + return _useAlternateKey; + } + +} diff --git a/api/src/org/labkey/api/data/RemapCache.java b/api/src/org/labkey/api/data/RemapCache.java index 2ec84e8ae45..4a5716f3898 100644 --- a/api/src/org/labkey/api/data/RemapCache.java +++ b/api/src/org/labkey/api/data/RemapCache.java @@ -33,7 +33,7 @@ */ public class RemapCache { - Map remaps = new HashMap<>(); + Map remaps = new HashMap<>(); private final boolean _allowBulkLoads; public RemapCache() @@ -140,17 +140,17 @@ private Key key(@NotNull TableInfo table) return new Key(table); } - private SimpleTranslator.RemapPostConvert remapper(Key key, Map remapCache, boolean includePkLookup) + private SimpleTranslator.RemapConverter remapper(Key key, Map remapCache, boolean includePkLookup) { return remapCache.computeIfAbsent(key, (k) -> { TableInfo table = key.getTable(); - return new SimpleTranslator.RemapPostConvert(table, true, SimpleTranslator.RemapMissingBehavior.Null, _allowBulkLoads, includePkLookup, null); + return new SimpleTranslator.RemapConverter(table, true, _allowBulkLoads, includePkLookup); }); } private V remap(Key key, String value, boolean includePkLookup) { - SimpleTranslator.RemapPostConvert remap = remapper(key, remaps, includePkLookup); + SimpleTranslator.RemapConverter remap = remapper(key, remaps, includePkLookup); if (remap == null) throw new NotFoundException("Failed to create remap: " + key._schemaKey.toString() + "." + key._queryName); //noinspection unchecked diff --git a/api/src/org/labkey/api/dataiterator/CoerceDataIterator.java b/api/src/org/labkey/api/dataiterator/CoerceDataIterator.java index c3dc5414389..f900733f5eb 100644 --- a/api/src/org/labkey/api/dataiterator/CoerceDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/CoerceDataIterator.java @@ -17,7 +17,6 @@ import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.data.ColumnInfo; -import org.labkey.api.data.JdbcType; import org.labkey.api.data.MultiValuedForeignKey; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.PropertyType; @@ -68,7 +67,7 @@ void init(TableInfo target, boolean useImportAliases, boolean outputAllColumns) else if (to.getFk() instanceof MultiValuedForeignKey) addColumn(to.getName(), i); // pass-through multi-value columns -- converting will stringify a collection else - addConvertColumn(to.getName(), i, to.getJdbcType(), to.getFk(), RemapMissingBehavior.OriginalValue); + addConvertColumn(to.getName(), i, to.getJdbcType(), to.getFk(), RemapMissingBehavior.Error, true); } else { @@ -88,10 +87,4 @@ else if (to.getFk() instanceof MultiValuedForeignKey) } } } - - @Override - protected Object addConversionException(String fieldName, Object value, JdbcType target, Exception x) - { - return value; - } } diff --git a/api/src/org/labkey/api/dataiterator/DataIteratorContext.java b/api/src/org/labkey/api/dataiterator/DataIteratorContext.java index 90d2f28dbfc..12713d9605b 100644 --- a/api/src/org/labkey/api/dataiterator/DataIteratorContext.java +++ b/api/src/org/labkey/api/dataiterator/DataIteratorContext.java @@ -19,6 +19,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.QueryImportPipelineJob; import org.labkey.api.query.QueryUpdateService; @@ -50,7 +51,7 @@ public class DataIteratorContext boolean _failFast = true; boolean _verbose = false; boolean _supportAutoIncrementKey = false; - boolean _allowImportLookupByAlternateKey = false; + LookupResolutionType _lookupResolutionType = LookupResolutionType.primaryKey; QueryImportPipelineJob _backgroundJob = null; boolean _crossTypeImport = false; boolean _crossFolderImport = false; @@ -60,6 +61,7 @@ public class DataIteratorContext private final Set _dontUpdateColumnNames = new CaseInsensitiveHashSet(); private final Set _alternateKeys = new CaseInsensitiveHashSet(); private String _dataSource; + private boolean _withLookupRemapping = true; private final Map _responseInfo = new HashMap<>(); // information from the import/loadRows context to be passed back to the API response object private Logger _logger; @@ -162,15 +164,31 @@ public void setSupportAutoIncrementKey(boolean supportAutoIncrementKey) _supportAutoIncrementKey = supportAutoIncrementKey; } - public boolean isAllowImportLookupByAlternateKey() + @NotNull + public LookupResolutionType getLookupResolutionType() + { + return _lookupResolutionType == null ? LookupResolutionType.primaryKey : _lookupResolutionType; + } + + public void setLookupResolutionType(LookupResolutionType lookupResolutionType) + { + _lookupResolutionType = lookupResolutionType; + } + + public boolean isWithLookupRemapping() + { + return _withLookupRemapping; + } + + public void setWithLookupRemapping(boolean withLookupRemapping) { - return _allowImportLookupByAlternateKey; + _withLookupRemapping = withLookupRemapping; } /** When true, allow importing lookup columns by the lookup table's alternate key instead of by primary key. */ public void setAllowImportLookupByAlternateKey(boolean allowImportLookupByAlternateKey) { - _allowImportLookupByAlternateKey = allowImportLookupByAlternateKey; + _lookupResolutionType = allowImportLookupByAlternateKey ? LookupResolutionType.alternateThenPrimaryKey : LookupResolutionType.primaryKey; } public boolean isCrossTypeImport() diff --git a/api/src/org/labkey/api/dataiterator/EmbargoDataIterator.java b/api/src/org/labkey/api/dataiterator/EmbargoDataIterator.java index 9e67db9f97f..4d546f8cbcd 100644 --- a/api/src/org/labkey/api/dataiterator/EmbargoDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/EmbargoDataIterator.java @@ -13,7 +13,7 @@ /** * StatementDataIterator is already complicated enough, so the cache-ahead functionality is implemented by a separate class called * EmbargoDataIterator. This class is similar to CachingDataIterator, however, where CachingDataIterator - * caches rows that have have already been seen, EmbargoDataIterator caches rows _ahead_ and holds them back until + * caches rows that have already been seen, EmbargoDataIterator caches rows _ahead_ and holds them back until * the StatementDataIterator indicates that they have been 'saved' and may be released. */ diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index 0ba41669dd0..5207cc42439 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java @@ -44,6 +44,7 @@ import org.labkey.api.data.EnumTableInfo; import org.labkey.api.data.ForeignKey; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.MultiValuedForeignKey; import org.labkey.api.data.MvUtil; import org.labkey.api.data.SimpleFilter; @@ -67,6 +68,7 @@ import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.Pair; @@ -114,6 +116,7 @@ */ public class SimpleTranslator extends AbstractDataIterator implements DataIterator, ScrollableDataIterator { + public static final String DEPRECATED_NULL_MISSING_VALUE_RESOLUTION = "deprecatedNullMissingValueResolution"; private static final Logger LOG = LogManager.getLogger(SimpleTranslator.class); /** @@ -183,19 +186,17 @@ protected Object addConversionException(String fieldName, @Nullable Object value else if (null != value && null != target) msg = ConvertHelper.getStandardConversionErrorMessage(value, fieldName, target.getJavaClass()); else if (null != x) - msg = StringUtils.defaultString(x.getMessage(), x.toString()); + msg = Objects.toString(x.getMessage(), x.toString()); else msg = "Could not convert value"; addFieldError(fieldName, msg); return null; } - public static class RemapPostConvert + public static class RemapConverter { - private final String _fieldName; private final TableInfo _targetTable; private final boolean _includeTitleColumn; - private final RemapMissingBehavior _missing; private boolean _includePkLookup; // if true, will perform an initial PK lookup before attempting the AK lookup private final boolean _allowBulkLoads; @@ -205,19 +206,18 @@ public static class RemapPostConvert private Triple> _titleColumnLookupMap = null; private Pair> _pkColumnLookupMap = null; - public RemapPostConvert(@NotNull TableInfo targetTable, boolean includeTitleColumn, RemapMissingBehavior missing, boolean allowBulkLoads, boolean includePkLookup, @Nullable String fieldName) + public RemapConverter(@NotNull TableInfo targetTable, boolean includeTitleColumn, boolean allowBulkLoads, boolean includePkLookup) { _targetTable = targetTable; _includeTitleColumn = includeTitleColumn; - _missing = missing; _allowBulkLoads = allowBulkLoads; _includePkLookup = includePkLookup; - _fieldName = fieldName; } public void setIncludePkLookup(boolean includePkLookup) { _includePkLookup = includePkLookup; + _maps = null; } public ColumnInfo getPkColumn() @@ -304,23 +304,10 @@ public Object mappedValue(Object k) if (_titleColumnLookupMap != null) { - Object v = fetch(_titleColumnLookupMap, String.valueOf(k)); - if (v != null) - return v; + return fetch(_titleColumnLookupMap, String.valueOf(k)); } - switch (_missing) - { - case Null: return null; - case OriginalValue: return k; - case Error: - default: - if (_fieldName != null) - throw new ConversionExceptionWithMessage("Value '" + k + "' not found for field " + _fieldName + " in the current context."); - else - throw new ConversionException("Could not translate value: " + k); - - } + return null; } private final Object MISS = new Object(); @@ -352,11 +339,23 @@ private Object fetch(Triple> triple, } // ArrayListValuedHashMap returns an empty collection if 'k' is not in the map. - if (bulkLoaded == null || bulkLoaded.isEmpty()) + if (bulkLoaded == null || bulkLoaded.isEmpty() ) { - TableSelector ts = createSelector(pkCol, altKeyCol, k); - ts.fillMultiValuedMap(map); - vs = map.get(k); + // when the given key (e.g., a rowId value) cannot be assigned to the alternate key value, + // don't attempt to select the value from the database, lest a syntax error result. + // This can happen for system fields like createdBy or modifiedBy that are added to a + // row during the insert or update process already mapped to their primary keys. + // Alternate keys must be of type String. + if (k instanceof String) + { + TableSelector ts = createSelector(pkCol, altKeyCol, k); + ts.fillMultiValuedMap(map); + vs = map.get(k); + } + else + { + vs = Collections.emptyList(); + } } else { @@ -938,37 +937,39 @@ public enum RemapMissingBehavior /** Every incoming value must have an entry in the map. */ Error, - /** Incoming values without a map entry will be replaced with null. */ + /** @Deprecated Prefer Error instead. Incoming values without a map entry will be replaced with null. */ Null, /** Incoming values without a map entry will pass through. */ OriginalValue } - protected class RemapPostConvertColumn extends SimpleConvertColumn + protected class RemappingConvertColumn extends SimpleConvertColumn { final SimpleConvertColumn _convertCol; final ColumnInfo _toCol; final RemapMissingBehavior _missing; final boolean _includeTitleColumn; + LookupResolutionType _lookupResolutionType; - final private RemapPostConvert _remapper; + final private RemapConverter _remapper; - public RemapPostConvertColumn(final @NotNull SimpleConvertColumn convertCol, final int fromIndex, final @NotNull ColumnInfo toCol, RemapMissingBehavior missing, boolean includeTitleColumn) + public RemappingConvertColumn(final @NotNull SimpleConvertColumn convertCol, final int fromIndex, final @NotNull ColumnInfo toCol, RemapMissingBehavior missing, boolean includeTitleColumn, @NotNull LookupResolutionType lookupResolutionType) { super(convertCol.fieldName, convertCol.index, convertCol.type); _convertCol = convertCol; _toCol = toCol; _missing = missing; _includeTitleColumn = includeTitleColumn; - _remapper = new RemapPostConvert(_toCol.getFkTableInfo(), _includeTitleColumn, _missing, false, true, _convertCol.fieldName); + _remapper = new RemapConverter(_toCol.getFkTableInfo(), _includeTitleColumn, false, true); + _lookupResolutionType = lookupResolutionType; } - @Override - protected Object convert(Object o) + private Object convertWithPrimaryColumn(Object o) { try { + // _convertCol here will be the column for the primary key type Object value = _convertCol.convert(o); ForeignKey fk = _toCol.getFk(); // issue 40909 : allow String columns to resolve lookups by alternate key if the raw lookup fails to resolve @@ -976,6 +977,7 @@ protected Object convert(Object o) { if (_remapper.getPkColumn().getJdbcType().isText()) { + _remapper.setIncludePkLookup(true); Object remappedValue = _remapper.mappedValue(o); value = remappedValue != null ? remappedValue : value; } @@ -984,52 +986,54 @@ protected Object convert(Object o) } catch (ConversionException ex) { - // don't want to attempt to resolve by target table PK because we already know there is a type mismatch - _remapper.setIncludePkLookup(false); - return _remapper.mappedValue(o); + return null; } } - } - - protected class RemapColumn implements Supplier - { - final Supplier _inputColumn; - final Map _map; - final RemapMissingBehavior _missing; - - public RemapColumn(final int index, Map map, RemapMissingBehavior missing) - { - _inputColumn = _data.getSupplier(index); - _map = map; - _missing = missing; - } - public RemapColumn(Supplier call, Map map, RemapMissingBehavior missing) + private Object convertWithRemapper(Object o) { - _inputColumn = call; - _map = map; - _missing = missing; + if (_lookupResolutionType.useAlternateKey()) + { + try + { + _remapper.setIncludePkLookup(false); + return _remapper.mappedValue(o); + } + catch (ConversionException ex) + { + return null; + } + } + return null; } @Override - public Object get() + protected Object convert(Object o) { - Object k = _inputColumn.get(); - if (null == k) + if (o == null) return null; - Object v = _map.get(k); - if (null != v || _map.containsKey(k)) - return v; - switch (_missing) + + Object value; + + value = convertWithRemapper(o); + if (value != null) { - case Null: return null; - case OriginalValue: return k; - case Error: - default: throw new ConversionException("Could not translate value: " + String.valueOf(k)); + return value; + } + + value = convertWithPrimaryColumn(o); + if (value == null) + { + if (_missing == RemapMissingBehavior.OriginalValue) + return o; + else if (_missing == RemapMissingBehavior.Null) + return null; + else + throw new ConversionExceptionWithMessage("Value '" + o + "' not found for field " + _toCol.getName() + " in the current context."); } + return value; } } - protected class NullColumn implements Supplier { @@ -1262,10 +1266,11 @@ public int addConvertColumn(ColumnInfo col, int fromIndex) * @param fromIndex Source column to create the output column from. * @param mvIndex Missing value column index. * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or null if not required. + * @param withLookupRemapping Indicates if remapping of lookup columns should be attempted or not */ - public int addConvertColumn(ColumnInfo col, int fromIndex, int mvIndex, @Nullable RemapMissingBehavior remapMissingBehavior) + private int addConvertColumn(ColumnInfo col, int fromIndex, int mvIndex, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { - SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, null, null, col.getJdbcType(), remapMissingBehavior); + SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, null, null, col.getJdbcType(), remapMissingBehavior, withLookupRemapping); return addColumn(col, c); } @@ -1283,9 +1288,10 @@ public int addConvertColumn(ColumnInfo col, int fromIndex, int mvIndex, @Nullabl * @param fromIndex Source column to create the output column from and pull data from. * @param toType Convert the source data values to this type. * @param toFk When isAllowImportLookupByAlternateKey is turned on, remap lookup values using the foreign key if there is a conversion failure. - * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or null if not required. + * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or original value if not required. + * @param withLookupRemapping Indicates if we should attempt to resolve lookups or not */ - public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullable ForeignKey toFk, @Nullable RemapMissingBehavior remapMissingBehavior) + public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullable ForeignKey toFk, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { var col = new BaseColumnInfo(_data.getColumnInfo(fromIndex)); col.setName(name); @@ -1293,7 +1299,7 @@ public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullab if (toFk != null) col.setFk(toFk); - return addConvertColumn(col, fromIndex, fromIndex, remapMissingBehavior); + return addConvertColumn(col, fromIndex, fromIndex, remapMissingBehavior, withLookupRemapping); } /** @@ -1311,19 +1317,20 @@ public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullab * @param pd PropertyDescriptor used for missing value enabled-ness. * @param pt Convert the source data values to this type. * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or null if not required. + * @param withLookupRemapping Indicates if we should try to remap lookups during the conversion or not. */ - public int addConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable RemapMissingBehavior remapMissingBehavior) + public int addConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { - SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, pd, pt, col.getJdbcType(), remapMissingBehavior); + SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, pd, pt, col.getJdbcType(), remapMissingBehavior, withLookupRemapping); return addColumn(col, c); } public SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, @Nullable RemapMissingBehavior remapMissingBehavior) { - return createConvertColumn(col, fromIndex, NO_MV_INDEX, null, col.getPropertyType(), col.getJdbcType(), remapMissingBehavior); + return createConvertColumn(col, fromIndex, NO_MV_INDEX, null, col.getPropertyType(), col.getJdbcType(), remapMissingBehavior, true); } - private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable JdbcType type, @Nullable RemapMissingBehavior remapMissingBehavior) + private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable JdbcType type, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { final String name = col.getName(); @@ -1338,15 +1345,23 @@ private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fro c = new PropertyConvertColumn(name, fromIndex, mvIndex, mv, pt, type); ForeignKey fk = col.getFk(); - if (fk != null && _context.isAllowImportLookupByAlternateKey() && fk.allowImportByAlternateKey()) + LookupResolutionType lookupResolutionType = _context.getLookupResolutionType(); + if (withLookupRemapping && fk != null && lookupResolutionType.useAlternateKey() && fk.allowImportByAlternateKey()) { - // Issue 48347: if the lookup field has a "Lookup Validator", then treat the missing values as an error - boolean hasValidator = pd != null && pd.getValidators().stream().anyMatch(v -> PropertyValidatorType.Lookup.getLabel().equalsIgnoreCase(v.getName())); - RemapMissingBehavior missing = remapMissingBehavior; if (missing == null) - missing = col.isRequired() || hasValidator ? RemapMissingBehavior.Error : RemapMissingBehavior.Null; - c = new RemapPostConvertColumn(c, fromIndex, col, missing, true); + { + if (OptionalFeatureService.get().isFeatureEnabled(DEPRECATED_NULL_MISSING_VALUE_RESOLUTION)) + { + // Issue 48347: if the lookup field has a "Lookup Validator", then treat the missing values as an error + boolean hasValidator = pd != null && pd.getValidators().stream().anyMatch(v -> PropertyValidatorType.Lookup.getLabel().equalsIgnoreCase(v.getName())); + + missing = col.isRequired() || hasValidator ? RemapMissingBehavior.Error : RemapMissingBehavior.Null; + } + else + missing = RemapMissingBehavior.Error; + } + c = new RemappingConvertColumn(c, fromIndex, col, missing, true, lookupResolutionType); } boolean multiValue = fk instanceof MultiValuedForeignKey; @@ -1394,19 +1409,6 @@ public int addTimestampColumn(String name) return addColumn(col, new TimestampColumn()); } - /** - * Translate values from the source data iterator to those contained in the in-memory map. - * @param fromIndex Source column to wrap. - * @param map Mapping from source to value. - * @param missing Tell me how to handle incoming values not present in the map. - */ - public int addRemapColumn(int fromIndex, @NotNull Map map, RemapMissingBehavior missing) - { - ColumnInfo col = new BaseColumnInfo(_data.getColumnInfo(fromIndex)); - RemapColumn remap = new RemapColumn(fromIndex, map, missing); - return addColumn(col, remap); - } - public int addSharedTableLookupColumn(int fromIndex, @Nullable FieldKey extraColumnFieldKey, @Nullable ForeignKey fk, @NotNull Map dataspaceTableIdMap) { @@ -2116,7 +2118,7 @@ public void convertTest() throws Exception DataIteratorContext context = new DataIteratorContext(); simpleData.beforeFirst(); SimpleTranslator t = new SimpleTranslator(simpleData, context); - t.addConvertColumn("IntNotNull", 1, JdbcType.INTEGER, null, null); + t.addConvertColumn("IntNotNull", 1, JdbcType.INTEGER, null, null, true); assertEquals(1, t.getColumnCount()); assertEquals(JdbcType.INTEGER, t.getColumnInfo(0).getJdbcType()); assertEquals(JdbcType.INTEGER, t.getColumnInfo(1).getJdbcType()); @@ -2136,10 +2138,10 @@ public void convertTest() throws Exception context.setVerbose(true); simpleData.beforeFirst(); SimpleTranslator t = new SimpleTranslator(simpleData, context); - t.addConvertColumn("Text", 2, JdbcType.INTEGER, null, null); - assertEquals(t.getColumnCount(), 1); - assertEquals(t.getColumnInfo(0).getJdbcType(), JdbcType.INTEGER); - assertEquals(t.getColumnInfo(1).getJdbcType(), JdbcType.INTEGER); + t.addConvertColumn("Text", 2, JdbcType.INTEGER, null, null, true); + assertEquals(1, t.getColumnCount()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(0).getJdbcType()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(1).getJdbcType()); try { assertFalse(t.next()); @@ -2157,10 +2159,10 @@ public void convertTest() throws Exception context.setVerbose(true); simpleData.beforeFirst(); SimpleTranslator t = new SimpleTranslator(simpleData, context); - t.addConvertColumn("Text", 2, JdbcType.INTEGER, null, null); - assertEquals(t.getColumnCount(), 1); - assertEquals(t.getColumnInfo(0).getJdbcType(), JdbcType.INTEGER); - assertEquals(t.getColumnInfo(1).getJdbcType(), JdbcType.INTEGER); + t.addConvertColumn("Text", 2, JdbcType.INTEGER, null, null, true); + assertEquals(1, t.getColumnCount()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(0).getJdbcType()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(1).getJdbcType()); for (int i=1 ; i<=4 ; i++) { assertTrue(t.next()); @@ -2204,14 +2206,14 @@ public void convertRemapTest() throws Exception public StringExpression getURL(ColumnInfo parent) { return null; } }; - // with remap with allowImportLookupByAlternateKey + // with remap with alternate key then primary key // don't throw error if remap can't be resolved { DataIteratorContext context = new DataIteratorContext(); - context.setAllowImportLookupByAlternateKey(true); + context.setLookupResolutionType(LookupResolutionType.alternateThenPrimaryKey); simpleData.beforeFirst(); SimpleTranslator t = new SimpleTranslator(simpleData, context); - t.addConvertColumn("Lookup", 5, JdbcType.INTEGER, fk, RemapMissingBehavior.OriginalValue); + t.addConvertColumn("Lookup", 5, JdbcType.INTEGER, fk, RemapMissingBehavior.OriginalValue, true); assertEquals(1, t.getColumnCount()); assertEquals(JdbcType.INTEGER, t.getColumnInfo(0).getJdbcType()); assertEquals(JdbcType.INTEGER, t.getColumnInfo(1).getJdbcType()); @@ -2234,12 +2236,45 @@ public void convertRemapTest() throws Exception // fourth row assertTrue(t.next()); assertEquals(4, t.get(0)); - assertNull(t.get(1)); // empty string converts to null + assertEquals("", t.get(1)); // since remapping to the original, returns the empty string // no more rows assertFalse(t.next()); } + // with remap with alternate then primary key, missing behavior is OriginalValue + // don't throw error if remap can't be resolved + { + DataIteratorContext context = new DataIteratorContext(); + context.setLookupResolutionType(LookupResolutionType.alternateThenPrimaryKey); + simpleData.beforeFirst(); + SimpleTranslator t = new SimpleTranslator(simpleData, context); + t.addConvertColumn("Lookup", 5, JdbcType.INTEGER, fk, RemapMissingBehavior.Error, true); + assertEquals(1, t.getColumnCount()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(0).getJdbcType()); + assertEquals(JdbcType.INTEGER, t.getColumnInfo(1).getJdbcType()); + + // first row + assertTrue(t.next()); + assertEquals(1, t.get(0)); + assertEquals(0, t.get(1)); // convert string "0" -> rowId ordinal 0 + + // second row + assertTrue(t.next()); + assertEquals(2, t.get(0)); + assertEquals(1, t.get(1)); // convert string "Two" -> rowId ordinal 1 + + // third row -- fails to resolve + try + { + t.next(); + fail("Should have thrown a conversion exception."); + } catch (BatchValidationException x) + { + assertTrue((x.getMessage().contains("Lookup: Value 'FAIL' not found for field Lookup in the current context."))); + } + } + } @Test diff --git a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java index 6b262774b99..d4f7d0e697a 100644 --- a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java +++ b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java @@ -280,7 +280,7 @@ public DataIterator getDataIterator(DataIteratorContext context) if (null == pair.target || isAttachment) convert.addColumn(pair.indexFrom); else - convert.addConvertColumn(pair.target, pair.indexFrom, pair.indexMv, pd, pt, pair.target.getRemapMissingBehavior()); + convert.addConvertColumn(pair.target, pair.indexFrom, pair.indexMv, pd, pt, pair.target.getRemapMissingBehavior(), context.isWithLookupRemapping()); } // diff --git a/api/src/org/labkey/api/dataiterator/TriggerDataBuilderHelper.java b/api/src/org/labkey/api/dataiterator/TriggerDataBuilderHelper.java index 9ee30d5c45b..6a70fc30932 100644 --- a/api/src/org/labkey/api/dataiterator/TriggerDataBuilderHelper.java +++ b/api/src/org/labkey/api/dataiterator/TriggerDataBuilderHelper.java @@ -145,6 +145,7 @@ public DataIterator getDataIterator(DataIteratorContext context) boolean skipExistingRecord = !context.getInsertOption().allowUpdate || mergeKeys == null || isNewFolderImport; DataIterator coerce = new CoerceDataIterator(pre, context, _target, !context.getInsertOption().updateOnly); + context.setWithLookupRemapping(false); coerce = LoggingDataIterator.wrap(coerce); if (skipExistingRecord) diff --git a/api/src/org/labkey/api/dataiterator/ValidatorIterator.java b/api/src/org/labkey/api/dataiterator/ValidatorIterator.java index 53343da40dc..e42921963aa 100644 --- a/api/src/org/labkey/api/dataiterator/ValidatorIterator.java +++ b/api/src/org/labkey/api/dataiterator/ValidatorIterator.java @@ -168,7 +168,8 @@ public boolean next() throws BatchValidationException if (null != msg) { - addFieldError(_data.getColumnInfo(i).getName(), msg); + if (!getRowError().hasFieldErrors(_data.getColumnInfo(i).getName())) + addFieldError(_data.getColumnInfo(i).getName(), msg); validRow = false; break; } @@ -192,6 +193,7 @@ public boolean next() throws BatchValidationException if (!validRow) { + assert hasErrors(); // we'll never return true once we hit a validation error hasValidationErrors = true; checkShouldCancel(); diff --git a/api/src/org/labkey/api/exp/list/ListDefinition.java b/api/src/org/labkey/api/exp/list/ListDefinition.java index 40e5a70b432..9dd121c74ff 100644 --- a/api/src/org/labkey/api/exp/list/ListDefinition.java +++ b/api/src/org/labkey/api/exp/list/ListDefinition.java @@ -21,6 +21,7 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.DomainNotFoundException; import org.labkey.api.exp.PropertyType; @@ -299,8 +300,8 @@ public static BodySetting getForValue(int value) ListItem getListItemForEntityId(String entityId, User user); int insertListItems(User user, Container container, List listItems) throws IOException; - int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, boolean importByAlternateKey) throws IOException; - int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, boolean importByAlternateKey, QueryUpdateService.InsertOption insertOption) throws IOException; + int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) throws IOException; + int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) throws IOException; @Nullable TableInfo getTable(User user); @Nullable TableInfo getTable(User user, Container c); diff --git a/api/src/org/labkey/api/qc/DataLoaderSettings.java b/api/src/org/labkey/api/qc/DataLoaderSettings.java index 8d27f664360..edba3eb83ab 100644 --- a/api/src/org/labkey/api/qc/DataLoaderSettings.java +++ b/api/src/org/labkey/api/qc/DataLoaderSettings.java @@ -15,6 +15,8 @@ */ package org.labkey.api.qc; +import org.labkey.api.data.LookupResolutionType; + /** * User: klum * Date: Oct 9, 2011 @@ -26,7 +28,7 @@ public class DataLoaderSettings private boolean _allowEmptyData; private boolean _throwOnErrors; private boolean _allowUnexpectedColumns; // don't load columns not in target domain - private boolean _allowLookupByAlternateKey = true; // import lookup column by unique index on target column or by title display column (if unique) + private LookupResolutionType _lookupResolutionType = LookupResolutionType.alternateThenPrimaryKey; public boolean isBestEffortConversion() { @@ -68,13 +70,18 @@ public void setAllowUnexpectedColumns(boolean allowUnexpectedColumns) _allowUnexpectedColumns = allowUnexpectedColumns; } - public boolean isAllowLookupByAlternateKey() + public void setAllowLookupByAlternateKey(boolean allowLookupByAlternateKey) { - return _allowLookupByAlternateKey; + _lookupResolutionType = allowLookupByAlternateKey ? LookupResolutionType.alternateThenPrimaryKey : LookupResolutionType.primaryKey; } - public void setAllowLookupByAlternateKey(boolean allowLookupByAlternateKey) + public LookupResolutionType getLookupResolutionType() + { + return _lookupResolutionType; + } + + public void setLookupResolutionType(LookupResolutionType lookupResolutionType) { - _allowLookupByAlternateKey = allowLookupByAlternateKey; + _lookupResolutionType = lookupResolutionType; } } diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index 1b04cc3d5ec..bbd97e36b83 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -34,6 +34,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.TableInfo; import org.labkey.api.dataiterator.DataIterator; @@ -304,13 +305,14 @@ public enum Params saveToPipeline, useAsync, importIdentity, - importLookupByAlternateKey, format, insertOption, crossTypeImport, allowCreateStorage, + importLookupByAlternateKey, // deprecated. Prefer lookupResolutionType crossFolderImport, - useTransactionAuditCache + useTransactionAuditCache, + lookupResolutionType, } @Nullable @@ -325,7 +327,6 @@ protected Map getOptionParamsMap() { _optionParamsMap = new HashMap<>(); _optionParamsMap.put(Params.importIdentity, Boolean.valueOf(getParam(Params.importIdentity))); - _optionParamsMap.put(Params.importLookupByAlternateKey, Boolean.valueOf(getParam(Params.importLookupByAlternateKey))); _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))); @@ -339,6 +340,26 @@ protected boolean getOptionParamValue(Params p) return getOptionParamsMap().getOrDefault(p, false); } + @NotNull + protected LookupResolutionType getLookupResolutionType() + { + String paramValue = getParam(Params.lookupResolutionType); + if (paramValue == null) + { + paramValue = getParam(Params.importLookupByAlternateKey); + if (paramValue == null) + return LookupResolutionType.primaryKey; + else + { + if (Boolean.valueOf(paramValue)) + return LookupResolutionType.alternateThenPrimaryKey; + else + return LookupResolutionType.primaryKey; + } + } + return LookupResolutionType.valueOf(paramValue); + } + protected boolean skipInsertOptionValidation() { return false; @@ -575,6 +596,7 @@ else if (!dataFileDir.exists()) .setAuditBehaviorType(behaviorType) .setAuditUserComment(_auditUserComment) .setOptionParamsMap(getOptionParamsMap()) + .setLookupResolutionType(getLookupResolutionType()) .setAllowLineageColumns(allowLineageColumns()) .setJobDescription(getQueryImportDescription()) .setJobNotificationProvider(getQueryImportJobNotificationProviderName()); @@ -789,17 +811,16 @@ protected ActionURL getSuccessURL(FORM form) /* TODO change prototype to take DataIteratorBuilder, and DataIteratorContext */ protected int importData(DataLoader dl, FileStream file, String originalName, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, TransactionAuditProvider.@Nullable TransactionAuditEvent auditEvent, @Nullable String auditUserComment) throws IOException { - return importData(dl, _target, _updateService, _insertOption, getOptionParamsMap(), errors, auditBehaviorType, auditEvent, auditUserComment, getUser(), getContainer(), null); + return importData(dl, _target, _updateService, _insertOption, getOptionParamsMap(), getLookupResolutionType(), errors, auditBehaviorType, auditEvent, auditUserComment, getUser(), getContainer(), null); } - public static DataIteratorContext createDataIteratorContext(QueryUpdateService.InsertOption insertOption, Map optionParamsMap, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment, BatchValidationException errors, @Nullable Logger logger, @Nullable Container container) + public static DataIteratorContext createDataIteratorContext(QueryUpdateService.InsertOption insertOption, Map optionParamsMap, LookupResolutionType lookupResolutionType, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment, BatchValidationException errors, @Nullable Logger logger, @Nullable Container container) { - return createDataIteratorContext(insertOption, optionParamsMap, auditBehaviorType, auditUserComment, errors, logger, container, null); + return createDataIteratorContext(insertOption, optionParamsMap, lookupResolutionType, auditBehaviorType, auditUserComment, errors, logger, container, null); } - public static DataIteratorContext createDataIteratorContext(QueryUpdateService.InsertOption insertOption, Map optionParamsMap, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment, BatchValidationException errors, @Nullable Logger logger, @Nullable Container container, QueryImportPipelineJob importJob) + public static DataIteratorContext createDataIteratorContext(QueryUpdateService.InsertOption insertOption, Map optionParamsMap, LookupResolutionType lookupResolutionType, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment, BatchValidationException errors, @Nullable Logger logger, @Nullable Container container, QueryImportPipelineJob importJob) { - boolean importLookupByAlternateKey = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.importLookupByAlternateKey, false); boolean importIdentity = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.importIdentity, false); boolean crossTypeImport = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.crossTypeImport, false); boolean allowCreateStorage = optionParamsMap.getOrDefault(AbstractQueryImportAction.Params.allowCreateStorage, false); @@ -809,7 +830,8 @@ public static DataIteratorContext createDataIteratorContext(QueryUpdateService.I DataIteratorContext context = new DataIteratorContext(errors); context.setBackgroundJob(importJob); context.setInsertOption(insertOption); - context.setAllowImportLookupByAlternateKey(importLookupByAlternateKey); + if (lookupResolutionType != null) + context.setLookupResolutionType(lookupResolutionType); if (auditBehaviorType != null) { context.putConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, auditBehaviorType); @@ -831,9 +853,9 @@ public static DataIteratorContext createDataIteratorContext(QueryUpdateService.I return context; } - public static int importData(DataLoader dl, TableInfo target, QueryUpdateService updateService, QueryUpdateService.InsertOption insertOption, Map optionParamsMap, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, TransactionAuditProvider.@Nullable TransactionAuditEvent auditEvent, @Nullable String auditUserComment, User user, Container container, @Nullable Logger logger) throws IOException + public static int importData(DataLoader dl, TableInfo target, QueryUpdateService updateService, QueryUpdateService.InsertOption insertOption, Map optionParamsMap, LookupResolutionType lookupResolutionType, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, TransactionAuditProvider.@Nullable TransactionAuditEvent auditEvent, @Nullable String auditUserComment, User user, Container container, @Nullable Logger logger) throws IOException { - return importData(dl, target, updateService, createDataIteratorContext(insertOption, optionParamsMap, auditBehaviorType, auditUserComment, errors, logger, container), auditEvent, user, container); + return importData(dl, target, updateService, createDataIteratorContext(insertOption, optionParamsMap, lookupResolutionType, auditBehaviorType, auditUserComment, errors, logger, container), auditEvent, user, container); } public static int importData(DataLoader dl, TableInfo target, QueryUpdateService updateService, @NotNull DataIteratorContext context, TransactionAuditProvider.@Nullable TransactionAuditEvent auditEvent, User user, Container container) throws IOException diff --git a/api/src/org/labkey/api/query/QueryImportPipelineJob.java b/api/src/org/labkey/api/query/QueryImportPipelineJob.java index ae26ecb1ef3..a6b40ae3373 100644 --- a/api/src/org/labkey/api/query/QueryImportPipelineJob.java +++ b/api/src/org/labkey/api/query/QueryImportPipelineJob.java @@ -4,6 +4,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.data.Container; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.TableInfo; import org.labkey.api.dataiterator.DataIteratorContext; import org.labkey.api.gwt.client.AuditBehaviorType; @@ -60,6 +61,7 @@ public static class QueryImportAsyncContextBuilder String _auditUserComment = null; boolean _allowLineageColumns = false; Map _optionParamsMap = new HashMap<>(); + LookupResolutionType _lookupResolutionType = null; String _jobDescription; @@ -208,6 +210,17 @@ public QueryImportAsyncContextBuilder setOptionParamsMap(Map 0) - return true; - - return false; + var fieldErrors = _fieldErrors.get(name); + return null != fieldErrors && !fieldErrors.isEmpty(); } public boolean hasErrors() diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index a6f8be9a8cb..3827bffe7c9 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -80,6 +80,7 @@ import org.labkey.api.data.dialect.SqlDialectManager; import org.labkey.api.data.dialect.SqlDialectRegistry; import org.labkey.api.data.statistics.StatsService; +import org.labkey.api.dataiterator.SimpleTranslator; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.property.TestDomainKind; import org.labkey.api.external.tools.ExternalToolsViewService; @@ -534,6 +535,10 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) "Restore Object-Level Discussions", "This option and all support for Object-Level Discussions will be removed in LabKey Server v25.7.", false, false, FeatureType.Deprecated)); + AdminConsole.addOptionalFeatureFlag(new OptionalFeatureFlag(SimpleTranslator.DEPRECATED_NULL_MISSING_VALUE_RESOLUTION, + "Resolve Missing Lookup Values to Null", + "When Lookup Validation for a field is not selected and lookup by alternate key is enabled, resolves missing lookup values to null instead of throwing an error. This option will be removed in LabKey Server v25.11.", + false, false, OptionalFeatureService.FeatureType.Deprecated)); SiteValidationService svc = SiteValidationService.get(); if (null != svc) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 7908efde0bb..3d6e6f5102f 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -19,7 +19,6 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.json.JSONArray; import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.attachments.AttachmentFile; import org.labkey.api.collections.CaseInsensitiveHashMap; @@ -132,7 +131,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -177,9 +175,9 @@ import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; import static org.labkey.api.query.AbstractQueryImportAction.configureLoader; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.insert; +import static org.labkey.experiment.api.SampleTypeUpdateServiceDI.PARENT_RECOMPUTE_NAME_COL; import static org.labkey.experiment.api.SampleTypeUpdateServiceDI.PARENT_RECOMPUTE_NAME_SET; import static org.labkey.experiment.api.SampleTypeUpdateServiceDI.ROOT_RECOMPUTE_ROWID_COL; -import static org.labkey.experiment.api.SampleTypeUpdateServiceDI.PARENT_RECOMPUTE_NAME_COL; import static org.labkey.experiment.api.SampleTypeUpdateServiceDI.ROOT_RECOMPUTE_ROWID_SET; @@ -2634,7 +2632,6 @@ public boolean next() throws BatchValidationException for (TypeData typeData : typeFolderData.values()) { writeRowsToFile(typeData); // write the last rows that have been collected since the last write, if any - if (!_context.getErrors().hasErrors()) // Issue 48402: Stop early since the transaction may have been aborted _importPartition(typeData); } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 2e6baf738de..c53cee90f69 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1077,7 +1077,7 @@ else if (Column.ClassId.name().equalsIgnoreCase(name)) // CoerceDataIterator to handle the lookup/alternatekeys functionality of loadRows(), // TODO check if this covers all the functionality, in particular how is alternateKeyCandidates used? di = LoggingDataIterator.wrap(new CoerceDataIterator(di, context, ExpDataClassDataTableImpl.this, false)); - + context.setWithLookupRemapping(false); TableInfo dataClassTInfo = ExpDataClassDataTableImpl.this; if (c.hasProductFolders() && !c.isProject()) { diff --git a/experiment/src/org/labkey/experiment/api/LineageTest.java b/experiment/src/org/labkey/experiment/api/LineageTest.java index c27ba852917..3bb98ed58e2 100644 --- a/experiment/src/org/labkey/experiment/api/LineageTest.java +++ b/experiment/src/org/labkey/experiment/api/LineageTest.java @@ -13,6 +13,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.RenderContext; import org.labkey.api.data.Results; @@ -476,6 +477,7 @@ public void testListAndSampleLineage() throws Exception rows.add(CaseInsensitiveHashMap.of("SampleId", "sally")); DataIteratorContext context = new DataIteratorContext(); + context.setLookupResolutionType(LookupResolutionType.alternateThenPrimaryKey); context.setAllowImportLookupByAlternateKey(true); errors = new BatchValidationException(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index c10ceef0cb0..d771d7c3b08 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1499,13 +1499,14 @@ public DataIterator getDataIterator(DataIteratorContext context) var addRequiredColsDI = new SampleUpdateAddColumnsDataIterator(new CachingDataIterator(addAliquotedFrom), materialTable, sampleType.getRowId(), columnNameMap.containsKey("lsid")); SimpleTranslator c = new _SamplesCoerceDataIterator(addRequiredColsDI, context, sampleType, materialTable); + context.setWithLookupRemapping(false); return LoggingDataIterator.wrap(c); } // CoerceDataIterator to handle the lookup/alternatekeys functionality of loadRows(), // TODO: check if this covers all the functionality, in particular how is alternateKeyCandidates used? 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"); Set idColNames = Sets.newCaseInsensitiveHashSet("genId"); @@ -1885,7 +1886,7 @@ else if (name.equalsIgnoreCase("StoredAmount")) if (isScopedField) _addConvertColumn(name, i, to.getJdbcType(), to.getFk(), aliquotedFromDataColInd, scopedFields.get(name)); else - addConvertColumn(to.getName(), i, to.getJdbcType(), to.getFk(), RemapMissingBehavior.OriginalValue); + addConvertColumn(to.getName(), i, to.getJdbcType(), to.getFk(), to.getRemapMissingBehavior(), true); } } else @@ -1931,7 +1932,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.OriginalValue); + 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())); addColumn(col, c); diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index eb2217ed5d8..5ad2ee36e8f 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -4573,7 +4573,7 @@ protected Set getLineageImportAliases() throws IOException protected void initContext(DataLoader dl, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String auditUserComment) throws IOException { - _context = createDataIteratorContext(_insertOption, getOptionParamsMap(), auditBehaviorType, auditUserComment, errors, null, getContainer()); + _context = createDataIteratorContext(_insertOption, getOptionParamsMap(), getLookupResolutionType(), auditBehaviorType, auditUserComment, errors, null, getContainer()); if (_context.isCrossFolderImport() && !getContainer().hasProductFolders()) _context.setCrossFolderImport(false); diff --git a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java index ba810338ff7..774b3e123f4 100644 --- a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java +++ b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java @@ -8,6 +8,7 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.DbScope; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.TableInfo; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.dataiterator.DataIteratorContext; @@ -293,7 +294,7 @@ protected void importTsvData(FolderImportContext ctx, XarContext xarContext, Str DataIteratorContext context = new DataIteratorContext(errors); context.setInsertOption(QueryUpdateService.InsertOption.MERGE); context.putConfigParameter(QueryUpdateService.ConfigParameters.SkipInsertOptionValidation, Boolean.TRUE); // allow merge during folder import, needed for eval data loading - context.setAllowImportLookupByAlternateKey(true); + context.setLookupResolutionType(LookupResolutionType.alternateThenPrimaryKey); ((AbstractQueryUpdateService)qus).setAttachmentDirectory(dir.getDir(tableName)); Map options = new HashMap<>(); try diff --git a/list/src/org/labkey/list/controllers/ListController.java b/list/src/org/labkey/list/controllers/ListController.java index a1b98244969..28b6beb8892 100644 --- a/list/src/org/labkey/list/controllers/ListController.java +++ b/list/src/org/labkey/list/controllers/ListController.java @@ -739,7 +739,7 @@ public ModelAndView getView(ListDefinitionForm form, BindException errors) throw @Override protected int importData(DataLoader dl, FileStream file, String originalName, BatchValidationException errors, @Nullable AuditBehaviorType auditBehaviorType, @Nullable TransactionAuditProvider.TransactionAuditEvent auditEvent, @Nullable String auditUserComment) throws IOException { - return _list.importListItems(getUser(), getContainer(), dl, errors, null, null, false, getOptionParamValue(Params.importLookupByAlternateKey), _insertOption); + return _list.importListItems(getUser(), getContainer(), dl, errors, null, null, false, getLookupResolutionType(), _insertOption); } @Override diff --git a/list/src/org/labkey/list/model/ListDefinitionImpl.java b/list/src/org/labkey/list/model/ListDefinitionImpl.java index 29398ba717a..bf4018c9260 100644 --- a/list/src/org/labkey/list/model/ListDefinitionImpl.java +++ b/list/src/org/labkey/list/model/ListDefinitionImpl.java @@ -25,6 +25,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbScope; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -586,18 +587,18 @@ public int insertListItems(User user, Container container, List listIt MapLoader loader = new MapLoader(rows); // TODO: Find out the attachment directory? - return insertListItems(user, container, loader, ve, null, null, false, false); + return insertListItems(user, container, loader, ve, null, null, false, LookupResolutionType.primaryKey); } @Override - public int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, boolean importByAlternateKey) + public int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) { - return importListItems(user, container, loader, errors, attachmentDir, progress, supportAutoIncrementKey, importByAlternateKey, QueryUpdateService.InsertOption.INSERT); + return importListItems(user, container, loader, errors, attachmentDir, progress, supportAutoIncrementKey, lookupResolutionType, QueryUpdateService.InsertOption.INSERT); } @Override - public int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, boolean importByAlternateKey, QueryUpdateService.InsertOption insertOption) + public int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) { ListQuerySchema schema = new ListQuerySchema(user, container); TableInfo table = schema.getTable(_def.getName()); @@ -605,7 +606,7 @@ public int importListItems(User user, Container container, DataLoader loader, @N { ListQueryUpdateService lqus = (ListQueryUpdateService) table.getUpdateService(); if (null != lqus) - return lqus.insertUsingDataIterator(loader, user, container, errors, attachmentDir, progress, supportAutoIncrementKey, importByAlternateKey, insertOption); + return lqus.insertUsingDataIterator(loader, user, container, errors, attachmentDir, progress, supportAutoIncrementKey, insertOption, lookupResolutionType); } return 0; } diff --git a/list/src/org/labkey/list/model/ListImporter.java b/list/src/org/labkey/list/model/ListImporter.java index e68bd4d2d67..a1da940aaa5 100644 --- a/list/src/org/labkey/list/model/ListImporter.java +++ b/list/src/org/labkey/list/model/ListImporter.java @@ -27,6 +27,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.DbScope; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.PHI; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlExecutor; @@ -258,7 +259,7 @@ private boolean processSingle(VirtualFile sourceDir, ListDefinition def, String } } - def.importListItems(user, c, loader, batchErrors, sourceDir.getDir(FileUtil.makeLegalName(def.getName())), null, supportAI, false, _importContext.useMerge() ? QueryUpdateService.InsertOption.MERGE : QueryUpdateService.InsertOption.IMPORT); + def.importListItems(user, c, loader, batchErrors, sourceDir.getDir(FileUtil.makeLegalName(def.getName())), null, supportAI, LookupResolutionType.primaryKey, _importContext.useMerge() ? QueryUpdateService.InsertOption.MERGE : QueryUpdateService.InsertOption.IMPORT); } for (ValidationException v : batchErrors.getRowErrors()) diff --git a/list/src/org/labkey/list/model/ListQueryUpdateService.java b/list/src/org/labkey/list/model/ListQueryUpdateService.java index e7f7e392ed2..fb82f664d9d 100644 --- a/list/src/org/labkey/list/model/ListQueryUpdateService.java +++ b/list/src/org/labkey/list/model/ListQueryUpdateService.java @@ -26,6 +26,7 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.DbScope; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.Selector.ForEachBatchBlock; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -204,7 +205,7 @@ private User getListUser(User user, Container container) } public int insertUsingDataIterator(DataLoader loader, User user, Container container, BatchValidationException errors, @Nullable VirtualFile attachmentDir, - @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, boolean importLookupsByAlternateKey, InsertOption insertOption) + @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, InsertOption insertOption, LookupResolutionType lookupResolutionType) { if (!_list.isVisible(user)) throw new UnauthorizedException("You do not have permission to insert data into this table."); @@ -214,7 +215,7 @@ public int insertUsingDataIterator(DataLoader loader, User user, Container conta context.setFailFast(false); context.setInsertOption(insertOption); // this method is used by ListImporter and BackgroundListImporter context.setSupportAutoIncrementKey(supportAutoIncrementKey); - context.setAllowImportLookupByAlternateKey(importLookupsByAlternateKey); + context.setLookupResolutionType(lookupResolutionType); setAttachmentDirectory(attachmentDir); TableInfo ti = _list.getTable(updatedUser); diff --git a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp index 54a728e7834..8653053f38f 100644 --- a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp +++ b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp @@ -29,6 +29,7 @@ <%@ page import="java.sql.SQLException" %> <%@ page import="java.util.Arrays" %> <%@ page import="java.util.List" %> +<%@ page import="org.labkey.api.data.LookupResolutionType" %> <%@ page extends="org.labkey.api.jsp.JspTest.DRT" %> <%! final static String tableName = "testGetSelectSqlSort"; @@ -43,7 +44,7 @@ ListDefinition list = s.getList(c, tableName); var data = new ReaderInputStream(new StringReader("A,B,C\n6,4,3\n1,8,6\n7,1,9\n2,5,1\n8,9,4\n3,2,7\n9,6,10\n4,10,2\n10,3,5\n5,7,8\n")); - list.insertListItems(user, c, new TabLoader.CsvFactory().createLoader(data,true), new BatchValidationException(), null, null, false, false); + list.insertListItems(user, c, new TabLoader.CsvFactory().createLoader(data,true), new BatchValidationException(), null, null, false, LookupResolutionType.primaryKey); // wrap this list so we can mangle the sort properties UserSchema schema = (UserSchema)DefaultSchema.get(user,c,"lists"); diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 694d2e80881..bdab01e471b 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -50,6 +50,7 @@ import org.labkey.api.data.Filter; import org.labkey.api.data.ILineageDisplayColumn; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.RenderContext; import org.labkey.api.data.Results; import org.labkey.api.data.SQLFragment; @@ -984,7 +985,7 @@ public UploadLog saveUploadData(User user, Dataset dsd, FileStream tsv, String f * Return an array of LSIDs from the newly created dataset entries, * along with the upload log. */ - public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study, DatasetDefinition dsd, DataLoader dl, boolean importLookupByAlternateKey, FileStream fileIn, String originalFileName, Map columnMap, BatchValidationException errors, QueryUpdateService.InsertOption insertOption, @Nullable AuditBehaviorType auditBehaviorType) + public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study, DatasetDefinition dsd, DataLoader dl, LookupResolutionType lookupResolutionType, FileStream fileIn, String originalFileName, Map columnMap, BatchValidationException errors, QueryUpdateService.InsertOption insertOption, @Nullable AuditBehaviorType auditBehaviorType) { DbScope scope = StudySchema.getInstance().getScope(); @@ -1003,7 +1004,7 @@ public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study if (defaultQCStateId != null) defaultQCState = DataStateManager.getInstance().getStateForRowId(study.getContainer(), defaultQCStateId.intValue()); lsids = StudyManager.getInstance().importDatasetData(user, dsd, dl, columnMap, errors, DatasetDefinition.CheckForDuplicates.sourceOnly, - defaultQCState, insertOption, null, importLookupByAlternateKey, auditBehaviorType); + defaultQCState, insertOption, null, lookupResolutionType, auditBehaviorType); if (!errors.hasErrors()) { diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 8e28fa95ff5..fd015743067 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -2694,7 +2694,7 @@ protected int importData(DataLoader dl, FileStream file, String originalName, Ba columnMap.put(_form.getSequenceNum(), column); } - Pair, UploadLog> result = StudyPublishManager.getInstance().importDatasetTSV(getUser(), _study, _def, dl, getOptionParamValue(Params.importLookupByAlternateKey), file, originalName, columnMap, errors, _form.getInsertOption(), auditBehaviorType); + Pair, UploadLog> result = StudyPublishManager.getInstance().importDatasetTSV(getUser(), _study, _def, dl, getLookupResolutionType(), file, originalName, columnMap, errors, _form.getInsertOption(), auditBehaviorType); if (!result.getKey().isEmpty()) { diff --git a/study/src/org/labkey/study/model/DatasetDataIteratorBuilder.java b/study/src/org/labkey/study/model/DatasetDataIteratorBuilder.java index 7bc3649a7d7..4274755ed5d 100644 --- a/study/src/org/labkey/study/model/DatasetDataIteratorBuilder.java +++ b/study/src/org/labkey/study/model/DatasetDataIteratorBuilder.java @@ -235,7 +235,7 @@ public DataIterator getDataIterator(DataIteratorContext context) else if (match == keyColumn && _datasetDefinition.getKeyManagementType() == Dataset.KeyManagementType.None) { // usually we let DataIterator handle convert, but we need to convert for consistent _key/lsid generation - out = it.addConvertColumn(match.getName(), in, match.getJdbcType(), null, null != match.getMvColumnName() ? SimpleTranslator.RemapMissingBehavior.OriginalValue : null); + out = it.addConvertColumn(match.getName(), in, match.getJdbcType(), null, null != match.getMvColumnName() ? SimpleTranslator.RemapMissingBehavior.OriginalValue : null, true); } else if (match.getPropertyType() == PropertyType.FILE_LINK) { diff --git a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp index 68d1b78438e..6801849ec80 100644 --- a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp +++ b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp @@ -683,7 +683,7 @@ private void importRows(Dataset def, List> rows, @Nullable L StudyManager.getInstance().importDatasetData( _context.getUser(), (DatasetDefinition) def, dl, columnMap, - errors, DatasetDefinition.CheckForDuplicates.sourceAndDestination, null, QueryUpdateService.InsertOption.IMPORT, logger, false, null); + errors, DatasetDefinition.CheckForDuplicates.sourceAndDestination, null, QueryUpdateService.InsertOption.IMPORT, logger, null, null); if (expectedErrors == null) { diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 54aea179175..77b0e7ef14f 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -58,6 +58,7 @@ import org.labkey.api.data.DbScope.Transaction; import org.labkey.api.data.Filter; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.PHI; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.PropertyManager.WritablePropertyMap; @@ -3443,14 +3444,14 @@ public List importDatasetData(User user, DatasetDefinition def, @Nullable DataState defaultQCState, QueryUpdateService.InsertOption insertOption, Logger logger, - boolean importLookupByAlternateKey, + LookupResolutionType lookupResolutionType, @Nullable AuditBehaviorType auditBehaviorType) throws IOException { DataIteratorContext context = new DataIteratorContext(errors); context.setInsertOption(insertOption); - context.setAllowImportLookupByAlternateKey(importLookupByAlternateKey); + context.setLookupResolutionType(lookupResolutionType); Map options = new HashMap<>(); options.put(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, auditBehaviorType);