Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions api/src/org/labkey/api/data/NameGeneratorState.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@
import java.util.stream.Collectors;

import static org.labkey.api.data.NameGenerator.getParentImportAliasFieldKeys;
import static org.labkey.api.exp.api.ExpMaterial.ALIQUOTED_FROM_INPUT;
import static org.labkey.api.exp.api.ExpRunItem.INPUT_PARENT;
import static org.labkey.api.exp.api.ExpRunItem.PARENT_IMPORT_ALIAS_MAP_PROP;
import static org.labkey.api.exp.api.ExperimentService.isAliquotedFromColumn;

public class NameGeneratorState implements AutoCloseable
{
Expand Down Expand Up @@ -362,7 +362,7 @@ private void addParentLookupValues(String parentTypeName,
boolean isMaterialParent,
@Nullable Map<String, FieldKey> parentImportAliases,
ExpObject parentObject,
Map<FieldKey, ArrayList<Object>> inputLookupValues,
Map<FieldKey, LinkedHashSet<Object>> inputLookupValues,
@Nullable NameGenerator altNameGenerator)
{
String inputType = isMaterialParent ? ExpMaterial.MATERIAL_INPUT_PARENT : ExpData.DATA_INPUT_PARENT;
Expand All @@ -384,7 +384,7 @@ private void addParentLookupValues(String parentTypeName,
continue;

// add to Input/<LookupField>
inputLookupValues.computeIfAbsent(FieldKey.fromParts(INPUT_PARENT, fieldName), (s) -> new ArrayList<>()).add(lookupValue);
inputLookupValues.computeIfAbsent(FieldKey.fromParts(INPUT_PARENT, fieldName), (s) -> new LinkedHashSet<>()).add(lookupValue);

// add to importAlias/<LookupField>
if (parentImportAliases != null)
Expand All @@ -393,19 +393,19 @@ private void addParentLookupValues(String parentTypeName,
.entrySet()
.stream()
.filter(entry -> inputFK.equals(entry.getValue()))
.forEach(entry -> inputLookupValues.computeIfAbsent(FieldKey.fromParts(entry.getKey(), fieldName), (s) -> new ArrayList<>()).add(lookupValue));
.forEach(entry -> inputLookupValues.computeIfAbsent(FieldKey.fromParts(entry.getKey(), fieldName), (s) -> new LinkedHashSet<>()).add(lookupValue));
}

// add to <Type>Inputs/<LookupField>
inputLookupValues.computeIfAbsent(FieldKey.fromParts(inputType, fieldName), (s) -> new ArrayList<>()).add(lookupValue);
inputLookupValues.computeIfAbsent(FieldKey.fromParts(inputType, fieldName), (s) -> new LinkedHashSet<>()).add(lookupValue);
// add to <Type>Inputs/<TypeName>/<LookupField>
inputLookupValues.computeIfAbsent(FieldKey.fromParts(inputType, parentTypeName, fieldName), (s) -> new ArrayList<>()).add(lookupValue);
inputLookupValues.computeIfAbsent(FieldKey.fromParts(inputType, parentTypeName, fieldName), (s) -> new LinkedHashSet<>()).add(lookupValue);
}
}

private void addAncestorLookupValues(
ExpRunItem parentObject,
Map<FieldKey, ArrayList<Object>> inputLookupValues,
Map<FieldKey, LinkedHashSet<Object>> inputLookupValues,
@Nullable NameGenerator altNameGenerator)
{
String parentLsid = parentObject.getLSID();
Expand Down Expand Up @@ -481,8 +481,8 @@ else if (parentObject instanceof ExpData expData)

if (!ancestorLookupValues.isEmpty())
{
inputLookupValues.putIfAbsent(ancestorFieldKey, new ArrayList<>());
List<Object> lookupValues = inputLookupValues.get(ancestorFieldKey);
inputLookupValues.putIfAbsent(ancestorFieldKey, new LinkedHashSet<>());
Set<Object> lookupValues = inputLookupValues.get(ancestorFieldKey);
for (Object lookupVal : ancestorLookupValues)
{
if (!lookupValues.contains(lookupVal))
Expand All @@ -497,7 +497,7 @@ private void addParentLookupContext(String parentTypeName/* already decoded */,
String parentName,
boolean isMaterialParent,
@Nullable Map<String, FieldKey> parentImportAliases,
Map<FieldKey, ArrayList<Object>> inputLookupValues,
Map<FieldKey, LinkedHashSet<Object>> inputLookupValues,
@Nullable NameGenerator altNameGenerator)
{
NameGenerator.ExpressionSummary expressionSummary = getExpressionSummary(altNameGenerator);
Expand Down Expand Up @@ -591,7 +591,7 @@ private Map<FieldKey, Object> additionalContext(@NotNull Map<String, Object> row
if (expressionSummary.hasParentLookup() || expressionSummary.hasParentInputs())
{
Map<FieldKey, Set<String>> inputs = new HashMap<>();
Map<FieldKey, ArrayList<Object>> inputLookupValues = new HashMap<>();
Map<FieldKey, LinkedHashSet<Object>> inputLookupValues = new HashMap<>();

inputs.put(FieldKey.fromParts(INPUT_PARENT), new LinkedHashSet<>());
inputs.put(FieldKey.fromParts(ExpData.DATA_INPUT_PARENT), new LinkedHashSet<>());
Expand Down Expand Up @@ -663,7 +663,7 @@ else if (value.isEmpty())
ctx.putAll(inputValues);

Map<FieldKey, Object> lookupValues = new HashMap<>();
inputLookupValues.forEach((key, value) -> lookupValues.put(key, value.size() > 1 ? value : (value.size() == 1 ? value.get(0) : null)));
inputLookupValues.forEach((key, value) -> lookupValues.put(key, value.size() > 1 ? value : (value.size() == 1 ? value.iterator().next() : null)));
ctx.putAll(lookupValues);
}

Expand Down Expand Up @@ -740,12 +740,12 @@ else if (value.isEmpty())
private void addParentLookupInput(String colName,
Object value,
@Nullable Map<String, FieldKey> parentImportAliases,
Map<FieldKey, ArrayList<Object>> inputLookupValues,
Map<FieldKey, LinkedHashSet<Object>> inputLookupValues,
@Nullable NameGenerator altNameGenerator)
{
String prefix = null;
String dataType = null;
if (ALIQUOTED_FROM_INPUT.equalsIgnoreCase(colName))
if (isAliquotedFromColumn(colName))
{
prefix = ExpMaterial.MATERIAL_INPUT_PARENT;
dataType = getParentTable() != null ? getParentTable().getName() : null;
Expand Down Expand Up @@ -791,7 +791,7 @@ private void addInputs(String colName,
String[] parts = colName.split("/", 2);
String prefix = null;
String decodedDataType = null;
if (ALIQUOTED_FROM_INPUT.equalsIgnoreCase(colName))
if (isAliquotedFromColumn(colName))
{
prefix = ExpMaterial.MATERIAL_INPUT_PARENT;
decodedDataType = getParentTable() != null ? getParentTable().getName() : null;
Expand Down
1 change: 1 addition & 0 deletions api/src/org/labkey/api/exp/api/ExpMaterial.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public interface ExpMaterial extends ExpRunItem
String MATERIAL_INPUTS_PREFIX_LC = MATERIAL_INPUTS_PREFIX.toLowerCase();
String MATERIAL_OUTPUT_CHILD = "MaterialOutputs";
String ALIQUOTED_FROM_INPUT = "AliquotedFrom";
String ALIQUOTED_FROM_INPUT_LABEL = "Aliquoted From";

@Nullable
ExpSampleType getSampleType();
Expand Down
10 changes: 10 additions & 0 deletions api/src/org/labkey/api/exp/api/ExpSampleType.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.collections.CaseInsensitiveHashMap;
import org.labkey.api.data.Container;
import org.labkey.api.data.ContainerFilter;
import org.labkey.api.exp.ExperimentException;
Expand Down Expand Up @@ -242,6 +243,15 @@ String createSampleName(@NotNull Map<String, Object> rowMap,

@NotNull Map<String, String> getImportAliases() throws IOException;

// Issue 53063: support "Aliquoted From"
@NotNull
default Map<String, String> getImportAliasesIncludingAliquot() throws IOException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an issue number reference here? Or with one of these updates (not sure which is the most obvious), just so it's easier to find what's changed for this issue.

{
Map<String, String> aliases = new CaseInsensitiveHashMap<>(getImportAliases());
aliases.put(ExpMaterial.ALIQUOTED_FROM_INPUT_LABEL, ExpMaterial.ALIQUOTED_FROM_INPUT);
return aliases;
}

@NotNull Map<String, String> getRequiredImportAliases() throws IOException;

@NotNull Map<String, Map<String, Object>> getImportAliasMap() throws IOException;
Expand Down
6 changes: 6 additions & 0 deletions api/src/org/labkey/api/exp/api/ExperimentService.java
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,12 @@ static boolean isInputOutputColumn(String columnName)
ExpMaterial.MATERIAL_OUTPUT_CHILD.equalsIgnoreCase(prefix);
}

static boolean isAliquotedFromColumn(String columnName)
{
return ExpMaterial.ALIQUOTED_FROM_INPUT.equalsIgnoreCase(columnName) ||
ExpMaterial.ALIQUOTED_FROM_INPUT_LABEL.equalsIgnoreCase(columnName);
}

// convert MaterialInputs/Blood/Type to MaterialInputs/Blood$SType
static @Nullable String getEncodedLineageKey(String inputColumn /*not encoded*/)
{
Expand Down
1 change: 1 addition & 0 deletions api/src/org/labkey/api/query/QueryUpdateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ enum ConfigParameters
SkipRequiredFieldValidation, // (Bool) skip validation of required fields, used during import when the import of data happens in two hitches (e.g., samples in one file and sample statuses in a second)
BulkLoad, // (Bool) skips detailed auditing
CheckForCrossProjectData, // (Bool) Check if data belong to other projects
ProcessingPartition, // (Bool) Importing a partitioned file from original file
SkipInsertOptionValidation, // (Bool) Skip assert(supportsInsertOption(context.getInsertOption())) for special scenarios (e.g., folder import uses merge action that's otherwise not supported for a table),
PreferPKOverObjectUriAsKey, // (Bool) Prefer getPkColumnNames instead of getObjectURIColumnName to use as keys
SkipReselectRows, // (Bool) If true, skip qus.getRows and use raw returned rows. Applicable for CommandType.insert/insertWithKeys/update/updateChangingKeys
Expand Down
4 changes: 4 additions & 0 deletions experiment/src/org/labkey/experiment/ExpDataIterators.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
import static org.labkey.api.exp.api.ExpData.DATA_INPUTS_PREFIX_LC;
import static org.labkey.api.exp.api.ExpData.DATA_INPUT_PARENT;
import static org.labkey.api.exp.api.ExpMaterial.ALIQUOTED_FROM_INPUT;
import static org.labkey.api.exp.api.ExpMaterial.ALIQUOTED_FROM_INPUT_LABEL;
import static org.labkey.api.exp.api.ExpMaterial.MATERIAL_INPUTS_PREFIX_LC;
import static org.labkey.api.exp.api.ExpMaterial.MATERIAL_INPUT_PARENT;
import static org.labkey.api.exp.api.ExpRunItem.INPUTS_PREFIX_LC;
Expand Down Expand Up @@ -2591,6 +2592,7 @@ public boolean next() throws BatchValidationException
{
_context.setCrossTypeImport(false);
_context.setCrossFolderImport(false);
_context.putConfigParameter(QueryUpdateService.ConfigParameters.ProcessingPartition, true);

boolean hasCrossFolderImport = false;

Expand All @@ -2610,6 +2612,7 @@ public boolean next() throws BatchValidationException
if (_isCrossFolder && !_context.getInsertOption().updateOnly && hasCrossFolderImport) // all updates are cross-folder due to lack of Container column
SimpleMetricsService.get().increment(ExperimentService.MODULE_NAME, _isSamples ? "sampleImport" : "dataClassImport", "multiFolderImport");

_context.putConfigParameter(QueryUpdateService.ConfigParameters.ProcessingPartition, false);
_context.setCrossTypeImport(_isCrossType);
_context.setCrossFolderImport(_isCrossFolder);
}
Expand Down Expand Up @@ -2861,6 +2864,7 @@ private TypeData createSampleHeaderRow(ExpSampleTypeImpl sampleType, Container c
Map<String, String> aliasMap = sampleType.getImportAliases();
validFields.addAll(aliasMap.keySet());
validFields.add(ALIQUOTED_FROM_INPUT);
validFields.add(ALIQUOTED_FROM_INPUT_LABEL);
validFields.add("StorageUnit");
validFields.add("Storage Unit");
validFields.add("StorageUnitLabel");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,32 @@ else if (name != null)
return dataRow;
}


@Override
protected Map<String, Object> updateRow(User user, Container container, Map<String, Object> row, @NotNull Map<String, Object> oldRow, boolean allowOwner, boolean retainCreation)
throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException
{
Map<String, Object> result = super.updateRow(user, container, row, oldRow, allowOwner, retainCreation);

// add MaterialInput/DataInputs field from parent alias
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear why this wasn't necessary before. Can you enlighten me?

Copy link
Contributor Author

@XingY XingY Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a bug that row by row update using alias gets silently ignored. It's reproducible only from API, since our UI will never send the request using alias, and file import will never hit the row by row code. Due to its narrow repro, I didn't create a separate issue for it. In a way, one could argue ignoring import alias for query api update is per design, since this feature was called importAliases.

try
{
Map<String, String> parentAliases = _dataClass.getImportAliases();
for (String alias : parentAliases.keySet())
{
if (row.containsKey(alias))
result.put(parentAliases.get(alias), result.get(alias));
}
}
catch (IOException e)
{
throw new RuntimeException(e);
}

return result;

}

@Override
protected Map<String, Object> _update(User user, Container c, Map<String, Object> row, Map<String, Object> oldRow, Object[] keys) throws SQLException, ValidationException
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1725,7 +1725,7 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon
// TODO: subclass PersistDataIteratorBuilder to index Materials! not DataClass!
try
{
var persist = new ExpDataIterators.PersistDataIteratorBuilder(data, this, propertiesTable, _ss, getUserSchema().getContainer(), getUserSchema().getUser(), _ss.getImportAliases(), sampleTypeObjectId)
var persist = new ExpDataIterators.PersistDataIteratorBuilder(data, this, propertiesTable, _ss, getUserSchema().getContainer(), getUserSchema().getUser(), _ss.getImportAliasesIncludingAliquot(), sampleTypeObjectId)
.setFileLinkDirectory(SAMPLETYPE_FILE_DIRECTORY);
ExperimentServiceImpl experimentServiceImpl = ExperimentServiceImpl.get();
SearchService.TaskIndexingQueue queue = SearchService.get().defaultTask().getQueue(getContainer(), SearchService.PRIORITY.modified);
Expand Down
73 changes: 65 additions & 8 deletions experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -468,23 +468,80 @@ public void testNameExpression() throws Exception

List<Map<String, Object>> aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("aliquotedFrom", expectedName1, "AliquotCount", 10));
aliquotRows.add(CaseInsensitiveHashMap.of("Aliquotedfrom", expectedName2, "aliquotCount", 5));
aliquotRows.add(CaseInsensitiveHashMap.of("ALIQUOTEDFROM", expectedName1, "Aliquotcount", 15));
aliquotRows.add(CaseInsensitiveHashMap.of("AliquotedFrom", expectedName3, "ALIQUOTCOUNT", 2));

List<Map<String, Object>> aliquots = insertSampleRows(sampleTypeName, aliquotRows);

assertExpectedName(st, expectedName1 + "-ALI-0004");
assertEquals(expectedName1, aliquots.get(0).get("AliquotedFrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("Aliquotedfrom", expectedName2, "aliquotCount", 5));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName2 + "-ALI-0005");
assertEquals(expectedName2, aliquots.get(1).get("aliquotedFrom"));
assertEquals(expectedName2, aliquots.get(0).get("aliquotedFrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("ALIQUOTEDFROM", expectedName1, "Aliquotcount", 15));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-ALI-0006");
assertEquals(expectedName1, aliquots.get(2).get("aliquotedfrom"));
assertEquals(expectedName1, aliquots.get(0).get("aliquotedfrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("AliquotedFrom", expectedName3, "ALIQUOTCOUNT", 2));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName3 + "-ALI-0007");
assertEquals(expectedName3, aliquots.get(3).get("ALIQUOTEDFROM"));
assertEquals(expectedName3, aliquots.get(0).get("ALIQUOTEDFROM"));

// Issue 53063: Support "Aliquoted From"
aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("Aliquoted From", expectedName2, "ALIQUOTCOUNT", 2));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName2 + "-ALI-0008");
assertEquals(expectedName2, aliquots.get(0).get("ALIQUOTEDFROM"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("aliquoted from", expectedName3, "ALIQUOTCOUNT", 3));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName3 + "-ALI-0009");
assertEquals(expectedName3, aliquots.get(0).get("aliquotedFrom"));

// test the default aliquot naming pattern (${${AliquotedFrom}-:withCounter}
st.setAliquotNameExpression("");
st.save(user);

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("aliquotedFrom", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-1");
assertEquals(expectedName1, aliquots.get(0).get("AliquotedFrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("Aliquotedfrom", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-2");
assertEquals(expectedName1, aliquots.get(0).get("aliquotedFrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("ALIQUOTEDFROM", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-3");
assertEquals(expectedName1, aliquots.get(0).get("aliquotedfrom"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("AliquotedFrom", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-4");
assertEquals(expectedName1, aliquots.get(0).get("ALIQUOTEDFROM"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("Aliquoted From", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-5");
assertEquals(expectedName1, aliquots.get(0).get("ALIQUOTEDFROM"));

aliquotRows = new ArrayList<>();
aliquotRows.add(CaseInsensitiveHashMap.of("aliquoted from", expectedName1));
aliquots = insertSampleRows(sampleTypeName, aliquotRows);
assertExpectedName(st, expectedName1 + "-6");
assertEquals(expectedName1, aliquots.get(0).get("aliquotedFrom"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10252,6 +10252,7 @@ else if (inputColumns != null)
allColumns.addAll(inputColumns);

Map<String, String> parentAliasColumnMap = new CaseInsensitiveHashMap<>();
parentAliasColumnMap.put(ExpMaterial.ALIQUOTED_FROM_INPUT_LABEL, ExpMaterial.ALIQUOTED_FROM_INPUT);
try
{
if (currentDataType instanceof ExpDataClass dataClass)
Expand All @@ -10270,7 +10271,7 @@ else if (currentDataType instanceof ExpSampleType sampleType)
String columnName = inputColName;
if (parentAliasColumnMap.containsKey(columnName))
columnName = parentAliasColumnMap.get(columnName);
if (ExperimentService.isInputOutputColumn(columnName) || "parent".equalsIgnoreCase(columnName))
if (ExperimentService.isInputOutputColumn(columnName) || ExperimentService.isAliquotedFromColumn(columnName) || "parent".equalsIgnoreCase(columnName))
{
if (seenColumns.contains(columnName))
throw new ApiUsageException(String.format(DUPLICATE_COLUMN_IN_DATA_ERROR, columnName));
Expand Down
Loading