Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e661698
Consolidate Dataclass data update methods - use DIB for update only
XingY Mar 10, 2026
09dd6b0
Enable upgrade script
XingY Mar 10, 2026
b5e2ac2
CRLF
XingY Mar 10, 2026
dc19ef1
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Mar 10, 2026
4d9ff01
fix build
XingY Mar 11, 2026
a678378
merge from develop
XingY Mar 11, 2026
a68e8a8
fix merge
XingY Mar 11, 2026
9d5aa75
fix merge
XingY Mar 11, 2026
4893660
fix update
XingY Mar 12, 2026
4b5e0ef
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Mar 15, 2026
d3aea2b
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Mar 23, 2026
44f7811
merge from develop
XingY Apr 6, 2026
a35f233
fix attachment
XingY Apr 7, 2026
28142df
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Apr 20, 2026
04f83ff
Fix alias in audit
XingY Apr 21, 2026
3446ba4
don't check for fail fast for DataClassUpdateAddColumnsDataIterator
XingY Apr 21, 2026
2a91cc6
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Apr 21, 2026
312a13c
Make sample consistent with data for failfast
XingY Apr 22, 2026
60898fa
Fix reclassify
XingY Apr 23, 2026
988d5fd
fix folder import
XingY Apr 23, 2026
22745ed
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Apr 23, 2026
e07508c
Remove `altUpdateKeys` from `QueryInfo`
XingY Apr 24, 2026
896c5cb
CC review
XingY Apr 24, 2026
53b876f
revert null values
XingY Apr 24, 2026
edca844
crlf
XingY Apr 24, 2026
3080753
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Apr 24, 2026
401896e
Merge remote-tracking branch 'origin/develop' into fb_sourceDIB
XingY Apr 27, 2026
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
16 changes: 14 additions & 2 deletions api/src/org/labkey/api/audit/AuditHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.labkey.api.data.ColumnInfo;
import org.labkey.api.data.Container;
import org.labkey.api.data.MultiValuedForeignKey;
import org.labkey.api.data.MultiValuedRenderContext;
import org.labkey.api.data.TableInfo;
import org.labkey.api.dataiterator.DataIterator;
import org.labkey.api.dataiterator.ExistingRecordDataIterator;
Expand Down Expand Up @@ -122,11 +123,22 @@ static Pair<Map<String, Object>, Map<String, Object>> getOldAndNewRecordForMerge
}
}

boolean isAliasInput = row.containsKey(ExperimentService.ALIASCOLUMNALIAS) && "Alias".equalsIgnoreCase(lcName);

boolean isExtraAuditField = extraFieldsToInclude != null && extraFieldsToInclude.contains(nameFromAlias);
if (!excludedFromDetailDiff.contains(nameFromAlias) && (row.containsKey(nameFromAlias) || isExpInput))
if (!excludedFromDetailDiff.contains(nameFromAlias) && (row.containsKey(nameFromAlias) || isExpInput || isAliasInput))
{
Object oldValue = entry.getValue();
Object newValue = row.get(nameFromAlias);

// See ExpDataIterator: step1.addColumn(ExperimentService.ALIASCOLUMNALIAS, colNameMap.get(Alias.name()))
if (isAliasInput && newValue == null)
{
newValue = row.get(ExperimentService.ALIASCOLUMNALIAS);
if (oldValue instanceof String aliasStr)
oldValue = Arrays.asList(aliasStr.split(MultiValuedRenderContext.VALUE_DELIMITER_REGEX));
}

// compare dates using string values to allow for both Date and Timestamp types
if (newValue instanceof Date && oldValue != null)
{
Expand Down Expand Up @@ -167,7 +179,7 @@ else if (!Objects.equals(oldValue, newValue) || isExtraAuditField)
// If multivalued columns change, the value in this table will remain the key to the junction table
// but at this point newValue will look like the newly chosen values not that key. So we skip
// this in the diff unless the value changes from non-null to null or vice versa.
if (isMultiValued)
if (isMultiValued && !isAliasInput)
{
if ((oldValue == null && newValue != null) || (newValue == null && oldValue != null))
{
Expand Down
5 changes: 0 additions & 5 deletions api/src/org/labkey/api/data/TableInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,6 @@ void addColumn(ColumnInfo column)
*/
@NotNull List<ColumnInfo> getAlternateKeyColumns();

@NotNull default Set<String> getAltKeysForUpdate()
{
return Collections.emptySet();
}

@Nullable default Set<String> getDisabledSystemFields()
{
return Collections.emptySet();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
package org.labkey.api.dataiterator;

import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.labkey.api.collections.IntHashMap;
import org.labkey.api.collections.Sets;
import org.labkey.api.data.ColumnInfo;
import org.labkey.api.data.CompareType;
import org.labkey.api.data.Container;
import org.labkey.api.data.JdbcType;
import org.labkey.api.data.SimpleFilter;
import org.labkey.api.data.TableInfo;
import org.labkey.api.data.TableSelector;
import org.labkey.api.exp.api.ExperimentService;
import org.labkey.api.exp.query.ExpDataTable;
import org.labkey.api.query.BatchValidationException;
import org.labkey.api.query.FieldKey;
import org.labkey.api.query.ValidationException;

import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

import static org.labkey.api.exp.query.ExpDataTable.Column.LSID;
import static org.labkey.api.exp.query.ExpDataTable.Column.ClassId;
import static org.labkey.api.util.IntegerUtils.asInteger;

/**
* DataIterator that adds the LSID column for DataClass update operations.
* Queries the LSID from exp.data based on the provided key (rowId or name) and dataClassId.
* The LSID is needed downstream for attachment handling.
*/
public class DataClassUpdateAddColumnsDataIterator extends WrapperDataIterator
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Feels like this could somehow extend a base class of ExistingRecordDataIterator so we could share a lot of the logic (same for SampleUpdateAddColumnsDataIterator). Not necessarily in scope for this PR but now that we have three implementations...

{
private final Container _targetContainer;
final CachingDataIterator _unwrapped;

private final long _dataClassId;
final int _lsidColIndex;
final ColumnInfo pkColumn;
final Supplier<Object> pkSupplier;

int lastPrefetchRowNumber = -1;
final IntHashMap<String> lsids = new IntHashMap<>();
final DataIteratorContext _context;

public DataClassUpdateAddColumnsDataIterator(DataIterator in, @NotNull DataIteratorContext context, TableInfo target, Container container, long dataClassId, String keyColumnName)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Switch the first argument to be of type CachingDataIterator and remove the casting a couple lines below. I recommend the same for SampleUpdateAddColumnsDataIterator.

{
super(in);
this._unwrapped = (CachingDataIterator)in;
_context = context;
_targetContainer = container;
_dataClassId = dataClassId;

var map = DataIteratorUtil.createColumnNameMap(in);

Integer lsidIdx = map.get(ExpDataTable.Column.LSID.name());
if (lsidIdx == null)
throw new IllegalStateException("LSID column not found in input.");
this._lsidColIndex = lsidIdx;

Integer index = map.get(keyColumnName);
ColumnInfo col = target.getColumn(keyColumnName);
if (null == index || null == col)
throw new IllegalArgumentException("Key column not found: " + keyColumnName);
pkSupplier = in.getSupplier(index);
pkColumn = col;
}

@Override
public Supplier<Object> getSupplier(int i)
{
if (i != _lsidColIndex)
return _delegate.getSupplier(i);
return () -> get(i);
}

@Override
public Object get(int i)
{
Integer rowNumber = asInteger(_delegate.get(0));

if (i == _lsidColIndex)
return lsids.get(rowNumber);

return _delegate.get(i);
}

@Override
public boolean isConstant(int i)
{
if (i != _lsidColIndex)
return _delegate.isConstant(i);
return false;
}

@Override
public Object getConstantValue(int i)
{
if (i != _lsidColIndex)
return _delegate.getConstantValue(i);
return null;
}

protected void prefetchExisting() throws BatchValidationException
{
Integer rowNumber = asInteger(_delegate.get(0));
if (rowNumber <= lastPrefetchRowNumber)
return;

lsids.clear();

int rowsToFetch = 50;
String keyFieldName = pkColumn.getName();
boolean numericKey = pkColumn.isNumericType();
JdbcType jdbcType = pkColumn.getJdbcType();
Map<Integer, Object> rowKeyMap = new LinkedHashMap<>();
Map<Object, Set<Integer>> keyRowMap = new LinkedHashMap<>();
Set<Object> notFoundKeys = new HashSet<>();

do
{
lastPrefetchRowNumber = asInteger(_delegate.get(0));
Object keyObj = pkSupplier.get();
Object key = jdbcType.convert(keyObj);

if (numericKey)
{
if (null == key)
throw new IllegalArgumentException(keyFieldName + " value not provided on row " + lastPrefetchRowNumber);
}
else if (StringUtils.isEmpty((String) key))
throw new IllegalArgumentException(keyFieldName + " value not provided on row " + lastPrefetchRowNumber);

rowKeyMap.put(lastPrefetchRowNumber, key);
notFoundKeys.add(key);
// if keyRowMap doesn't contain key, add new set, then add row number to set for this key
if (!keyRowMap.containsKey(key))
keyRowMap.put(key, new HashSet<>());
keyRowMap.get(key).add(lastPrefetchRowNumber);
lsids.put(lastPrefetchRowNumber, null);
}
while (--rowsToFetch > 0 && _delegate.next());

SimpleFilter filter = new SimpleFilter(ClassId.fieldKey(), _dataClassId);
filter.addCondition(pkColumn.getFieldKey(), rowKeyMap.values(), CompareType.IN);
filter.addCondition(FieldKey.fromParts("Container"), _targetContainer);

Set<String> columns = Sets.newCaseInsensitiveHashSet(keyFieldName, LSID.name());
Map<String, Object>[] results = new TableSelector(ExperimentService.get().getTinfoData(), columns, filter, null).getMapArray();

for (Map<String, Object> result : results)
{
Object key = result.get(keyFieldName);
Object lsidObj = result.get(LSID.name());

Set<Integer> rowInds = keyRowMap.get(key);
if (lsidObj != null)
{
for (Integer rowInd : rowInds)
lsids.put(rowInd, (String) lsidObj);
notFoundKeys.remove(key);
}
}

if (!notFoundKeys.isEmpty())
_context.getErrors().addRowError(new ValidationException("Data not found for " + notFoundKeys));

// backup to where we started so caller can iterate through them one at a time
_unwrapped.reset(); // unwrapped _delegate
_delegate.next();
}

@Override
public boolean next() throws BatchValidationException
{
if (_context.getErrors().hasErrors())
return false;

// NOTE: we have to call mark() before we call next() if we want the 'next' row to be cached
_unwrapped.mark(); // unwrapped _delegate
boolean ret = super.next();
if (!_context.getErrors().hasErrors() && ret)
{
prefetchExisting();
if (_context.getErrors().hasErrors())
return false;
}

return ret;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ public Supplier<Object> getSupplier(int i)
@Override
public Object get(int i)
{
assert(i <= existingColIndex);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Provide a message when this fails so we have a bit more context.


if (i<existingColIndex)
return _delegate.get(i);
Integer rowNumber = asInteger(_delegate.get(0));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.labkey.api.dataiterator;

import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.labkey.api.collections.IntHashMap;
import org.labkey.api.collections.Sets;
import org.labkey.api.data.ColumnInfo;
Expand Down Expand Up @@ -33,17 +34,19 @@ public class SampleUpdateAddColumnsDataIterator extends WrapperDataIterator
final int _aliquotedFromColIndex;
final int _rootMaterialRowIdColIndex;
final int _currentSampleStateColIndex;
final DataIteratorContext _context;

// prefetch of existing records
int lastPrefetchRowNumber = -1;
final IntHashMap<String> aliquotParents = new IntHashMap<>();
final IntHashMap<Integer> aliquotRoots = new IntHashMap<>();
final IntHashMap<Integer> sampleState = new IntHashMap<>();

public SampleUpdateAddColumnsDataIterator(DataIterator in, TableInfo target, long sampleTypeId, String keyColumnName)
public SampleUpdateAddColumnsDataIterator(DataIterator in, @NotNull DataIteratorContext context, TableInfo target, long sampleTypeId, String keyColumnName)
{
super(in);
this._unwrapped = (CachingDataIterator)in;
this._context = context;
this.target = target;
this._sampleTypeId = sampleTypeId;

Expand Down Expand Up @@ -167,11 +170,19 @@ else if (StringUtils.isEmpty((String) key))
@Override
public boolean next() throws BatchValidationException
{
if (_context.getErrors().hasErrors())
return false;

// NOTE: we have to call mark() before we call next() if we want the 'next' row to be cached
_unwrapped.mark(); // unwrapped _delegate
boolean ret = super.next();
if (ret)
if (!_context.getErrors().hasErrors() && ret)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Switch the checks so the "cheaper" check is first.

if (ret && !_context.getErrors().hasErrors())

{
prefetchExisting();
if (_context.getErrors().hasErrors())
return false;
}

return ret;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ public Object get(int i)
{
if (null != _keyColumnInfo.get(i))
return _keyValues.get(i);

return _data.get(i);
}

Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/exp/api/ExperimentService.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ public interface ExperimentService extends ExperimentRunTypeSource

String EXPERIMENTAL_FEATURE_FROM_EXPANCESTORS = "org.labkey.api.exp.api.ExperimentService#FROM_EXPANCESTORS";

String EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE = "org.labkey.experiment.api.SampleTypeUpdateServiceDI#ALLOW_ROW_ID_SAMPLE_MERGE";

int SIMPLE_PROTOCOL_FIRST_STEP_SEQUENCE = 1;
int SIMPLE_PROTOCOL_CORE_STEP_SEQUENCE = 10;
int SIMPLE_PROTOCOL_EXTRA_STEP_SEQUENCE = 15;
Expand All @@ -150,7 +152,7 @@ static void setInstance(ExperimentService impl)

enum QueryOptions
{
UseLsidForUpdate,
UseProvidedLsidForXarImport,
GetSampleRecomputeCol,
SkipBulkRemapCache,
DeferRequiredLineageValidation,
Expand Down
Loading
Loading