Issue 52098, 49422: when looking up by alternate keys do that first so number names will resolve well (#6642)

Author: labkey-susanh

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))

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;
+    }
+
+}

api/src/org/labkey/api/data/RemapCache.java

@@ -33,7 +33,7 @@
  */
 public class RemapCache
 {
-    Map<Key, SimpleTranslator.RemapPostConvert> remaps = new HashMap<>();
+    Map<Key, SimpleTranslator.RemapConverter> 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<Key, SimpleTranslator.RemapPostConvert> remapCache, boolean includePkLookup)
+    private SimpleTranslator.RemapConverter remapper(Key key, Map<Key, SimpleTranslator.RemapConverter> 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> 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

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;
-    }
 }

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<String> _dontUpdateColumnNames = new CaseInsensitiveHashSet();
     private final Set<String> _alternateKeys = new CaseInsensitiveHashSet();
     private String _dataSource;
+    private boolean _withLookupRemapping = true;
 
     private final Map<String, Object> _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()

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.
  */