From 513fb70c852959ccf995fdaa2a5f1d546311342a Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 10 Sep 2025 16:13:23 -0700 Subject: [PATCH 1/9] Add audit detail change for dataclass lineage events --- .../labkey/api/exp/api/ExperimentService.java | 27 ----- .../api/ExpDataClassDataTableImpl.java | 83 +++++++++++++- .../experiment/api/ExperimentServiceImpl.java | 101 +++++++++++++----- .../api/SampleTypeUpdateServiceDI.java | 100 ++++------------- 4 files changed, 172 insertions(+), 139 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 101bb909a1e..a39c96cc2ee 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -593,33 +593,6 @@ static void validateParentAlias(Map aliasMap, Set reserv * ignoring any sample children derived from ExpData children. */ Set getRelatedChildSamples(Container c, User user, ExpData start); - - /** - * Find the ExpData objects, if any, that are parents of the start ExpMaterial. - */ - @NotNull - Set getParentDatas(Container c, User user, ExpMaterial start); - - /** - * Find the ExpMaterial objects, if any, that are parents of the start ExpMaterial. - */ - @NotNull - Set getParentMaterials(Container c, User user, ExpMaterial start); - - /** - * Find all parent ExpData that are parents of the start ExpMaterial, - * stopping at the first parent generation (no grandparents.) - */ - @NotNull - Set getNearestParentDatas(Container c, User user, ExpMaterial start); - - /** - * Find all parent ExpMaterial that are parents of the start ExpMaterial, - * stopping at the first parent generation (no grandparents.) - */ - @NotNull - Set getNearestParentMaterials(Container c, User user, ExpMaterial start); - /** * Get the lineage for the seed Identifiable object. Typically, the seed object is a ExpMaterial, * a ExpData (in a DataClass), or an ExpRun. diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 6ed1a8eeec0..54b5918fa1c 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -20,6 +20,7 @@ import org.apache.commons.collections4.MapUtils; import org.apache.commons.collections4.ListUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -1231,12 +1232,66 @@ public List> insertRows(User user, Container container, List return super._insertRowsUsingDIB(user, container, rows, getDataIteratorContext(errors, InsertOption.INSERT, configParameters), extraScriptContext); } + @Override + public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) + { + Map> dataRows = getExistingRows(user, container, keys, verifyNoCrossFolderData, verifyExisting, columns); + boolean hasParentInput = false; + if (_dataClass != null && columns != null && !columns.isEmpty()) + { + try + { + Map importAliases = _dataClass.getImportAliases(); + for (String col : columns) + { + if (ExperimentService.isInputOutputColumn(col) || Strings.CI.equals("parent",col) || importAliases.containsKey(col)) + { + hasParentInput = true; + break; + } + } + } + catch (IOException ignored) + { + } + } + + if (!hasParentInput) + return dataRows; + + Set lsids = new HashSet<>(); + for (Map dataRow : dataRows.values()) + lsids.add((String) dataRow.get("lsid")); + List seeds = ExperimentServiceImpl.get().getExpDatasByLSID(lsids); + + ExperimentServiceImpl.get().addRowsParentsFields(new HashSet<>(seeds), dataRows, user, container); + + return dataRows; + } + @Override protected Map getRow(User user, Container container, Map keys) throws InvalidKeyException, SQLException { return getRow(user, container, keys, false); } + @Override + public List> getRows(User user, Container container, List> keys) + throws SQLException + { + if (!hasPermission(user, ReadPermission.class)) + throw new UnauthorizedException("You do not have permission to read data from this table."); + + List> result = new ArrayList<>(keys.size()); + for (Map row : keys) + { + Map materialMap = getDataMap(row, user, container, true,false); + if (materialMap != null) + result.add(materialMap); + } + return result; + } + /* This class overrides getRow() in order to support getRow() using "rowid" or "lsid" */ @Override protected Map getRow(User user, Container container, Map keys, boolean allowCrossContainer) throws InvalidKeyException, SQLException @@ -1254,7 +1309,7 @@ protected Map getRow(User user, Container container, Map _select(Container container, Object[] keys) throws throw new IllegalStateException(); } - protected Map _select(Container container, Long rowId, String lsid, String name, Long classId, boolean allowCrossContainer) throws SQLException + protected Map getDataMap(Map keys, User user, Container container, boolean allowCrossContainer, boolean addInputs) throws SQLException { + Long rowId = (Long)JdbcType.BIGINT.convert(keys.get(Column.RowId.name())); + String lsid = (String)JdbcType.VARCHAR.convert(keys.get(Column.LSID.name())); + String name = (String)JdbcType.VARCHAR.convert(keys.get(Name.name())); + Long classId = (Long)JdbcType.BIGINT.convert(keys.get(Column.ClassId.name())); + + if (classId == null) + classId = _dataClass.getRowId(); + if (null == rowId && null == lsid && (null == name || null == classId)) return null; @@ -1284,13 +1347,25 @@ else if (null != lsid) TableInfo queryTable = getQueryTable(); TableSelector selector = new TableSelector(queryTable, filter, null); + Map dataRow = null; try (var results = selector.getResults()) { if (results.next()) { - return FieldKeyRowMap.toNameMap(results.getFieldKeyRowMap()); + dataRow = FieldKeyRowMap.toNameMap(results.getFieldKeyRowMap()); } } - return null; + + if (!addInputs) + return dataRow; + + ExperimentService experimentService = ExperimentService.get(); + ExpData seed = lsid != null ? experimentService.getExpData(lsid) : experimentService.getExpData(_dataClass, rowId); + if (null == seed) + return dataRow; + + ExperimentServiceImpl.get().addParentsFields(seed, dataRow, user, container); + + return dataRow; } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index b0db3143c45..f8a80160311 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -2491,9 +2491,11 @@ public Set getRelatedChildSamples(Container c, User user, ExpData s return lineage.findRelatedChildSamples(start); } - @Override + /** + * Find the Identifiable objects, if any, that are parents of the start Identifiable. + */ @NotNull - public Set getParentDatas(Container c, User user, ExpMaterial start) + public Set getParentDatas(Container c, User user, Identifiable start) { ExpLineageOptions options = new ExpLineageOptions(); options.setChildren(false); @@ -2503,9 +2505,11 @@ public Set getParentDatas(Container c, User user, ExpMaterial start) return lineage.findNearestParentDatas(start); } - @Override + /** + * Find the Identifiable objects, if any, that are parents of the start Identifiable. + */ @NotNull - public Set getParentMaterials(Container c, User user, ExpMaterial start) + public Set getParentMaterials(Container c, User user, Identifiable start) { ExpLineageOptions options = new ExpLineageOptions(); options.setChildren(false); @@ -2515,6 +2519,73 @@ public Set getParentMaterials(Container c, User user, ExpMaterial s return lineage.findNearestParentMaterials(start); } + public void addParentsFields(T seed, Map dataRow, User user, Container container) + { + Set parentSamples = getParentMaterials(container, user, seed); + if (!parentSamples.isEmpty()) + addParentFields(dataRow, parentSamples, ExpMaterial.MATERIAL_INPUT_PARENT + "/", user); + Set parentDatas = getParentDatas(container, user, seed); + if (!parentDatas.isEmpty()) + addParentFields(dataRow, parentDatas, ExpData.DATA_INPUT_PARENT + "/", user); + } + + public void addParentFields(Map sampleRow, Set parents, String parentPrefix, User user) + { + Map> parentByType = new HashMap<>(); + for (ExpRunItem parent : parents) + { + String type = ""; + if (parent instanceof ExpData dataParent) + { + ExpDataClass dataClass = dataParent.getDataClass(user); + if (dataClass == null) + continue; + type = dataClass.getName(); + } + else if (parent instanceof ExpMaterial materialParent) + { + ExpSampleType sampleType = materialParent.getSampleType(); + if (sampleType == null) + continue; + type = sampleType.getName(); + } + + parentByType.computeIfAbsent(type, k -> new ArrayList<>()); + String parentName = parent.getName(); + if (parentName.contains(",")) + parentName = "\"" + parentName + "\""; + parentByType.get(type).add(parentName); + } + + for (String type : parentByType.keySet()) + { + String key = parentPrefix + type; + List parentValues = parentByType.get(type); + String value = String.join(",", parentValues); + sampleRow.put(key, value); + } + } + + public void addRowsParentsFields(Set seeds, Map> dataRows, User user, Container container) + { + Map, Set>> parents = ExperimentServiceImpl.get().getParentMaterialAndDataMap(container, user, new HashSet<>(seeds)); + + for (Map dataRow : dataRows.values()) + { + String lsidKey = (String) dataRow.get("lsid"); + + if (!parents.containsKey(lsidKey)) + continue; + + Pair, Set> sampleParents = parents.get(lsidKey); + + if (!sampleParents.first.isEmpty()) + addParentFields(dataRow, sampleParents.first, ExpMaterial.MATERIAL_INPUT_PARENT + "/", user); + if (!sampleParents.second.isEmpty()) + addParentFields(dataRow, sampleParents.second, ExpData.DATA_INPUT_PARENT + "/", user); + } + } + public Map, Set>> getParentMaterialAndDataMap(Container c, User user, Set seeds) { Map, Set>> results = new HashMap<>(); @@ -2544,28 +2615,6 @@ else if (item instanceof ExpData dItem) return results; } - @Override - @NotNull - public Set getNearestParentDatas(Container c, User user, ExpMaterial start) - { - ExpLineageOptions options = new ExpLineageOptions(); - options.setChildren(false); - - ExpLineage lineage = getLineage(c, user, start, options); - return lineage.findNearestParentDatas(start); - } - - @Override - @NotNull - public Set getNearestParentMaterials(Container c, User user, ExpMaterial start) - { - ExpLineageOptions options = new ExpLineageOptions(); - options.setChildren(false); - - ExpLineage lineage = getLineage(c, user, start, options); - return lineage.findNearestParentMaterials(start); - } - // Get list of ExpRun LSIDs for the start Data or Material @Override public List collectRunsToInvestigate(ExpRunItem start, ExpLineageOptions options) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 126fbe21ca7..b837e7e7f75 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -74,9 +74,7 @@ import org.labkey.api.exp.Lsid; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.api.ExpData; -import org.labkey.api.exp.api.ExpDataClass; import org.labkey.api.exp.api.ExpMaterial; -import org.labkey.api.exp.api.ExpRunItem; import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.NameExpressionOptionService; @@ -104,6 +102,7 @@ import org.labkey.api.search.SearchService; import org.labkey.api.security.User; import org.labkey.api.security.permissions.MoveEntitiesPermission; +import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.usageMetrics.SimpleMetricsService; import org.labkey.api.util.JobRunner; @@ -1008,57 +1007,10 @@ else if (lsid != null) ExpMaterial seed = rowId != null ? experimentService.getExpMaterial(rowId) : experimentService.getExpMaterial(lsid); if (null == seed) return sampleRow; - Set parentSamples = experimentService.getParentMaterials(container, user, seed); - if (!parentSamples.isEmpty()) - addParentFields(sampleRow, parentSamples, ExpMaterial.MATERIAL_INPUT_PARENT + "/", user); - Set parentDatas = experimentService.getParentDatas(container, user, seed); - if (!parentDatas.isEmpty()) - addParentFields(sampleRow, parentDatas, ExpData.DATA_INPUT_PARENT + "/", user); - return sampleRow; - } - - private void addParentFields(Map sampleRow, Set parents, String parentPrefix, User user) - { - Map> parentByType = new HashMap<>(); - for (ExpRunItem parent : parents) - { - String type = ""; - if (parent instanceof ExpData dataParent) - { - ExpDataClass dataClass = dataParent.getDataClass(user); - if (dataClass == null) - continue; - type = dataClass.getName(); - } - else if (parent instanceof ExpMaterial materialParent) - { - ExpSampleType sampleType = materialParent.getSampleType(); - if (sampleType == null) - continue; - type = sampleType.getName(); - } + ExperimentServiceImpl.get().addParentsFields(seed, sampleRow, user, container); - parentByType.computeIfAbsent(type, k -> new ArrayList<>()); - String parentName = parent.getName(); - if (parentName.contains(",")) - parentName = "\"" + parentName + "\""; - parentByType.get(type).add(parentName); - } - - for (String type : parentByType.keySet()) - { - String key = parentPrefix + type; - String value = String.join(",", parentByType.get(type)); - sampleRow.put(key, value); - } - } - - @Override - public List> getRows(User user, Container container, List> keys) - throws QueryUpdateServiceException - { - return getRows(user, container, keys, false /*skip addInputs for insertRows*/); + return sampleRow; } @Override @@ -1084,13 +1036,6 @@ public boolean hasExistingRowsInOtherContainers(Container container, Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) - throws InvalidKeyException, QueryUpdateServiceException - { - return getMaterialMapsWithInput(keys, user, container, verifyNoCrossFolderData, verifyExisting, columns); - } - private record ExistingRowSelect(TableInfo tableInfo, Set columns, boolean includeParent) {} private @NotNull ExistingRowSelect getExistingRowSelect(@Nullable Set dataColumns) @@ -1147,8 +1092,9 @@ else if (dataColumns.contains(remap.get(column.getColumnName()))) return new ExistingRowSelect(selectTable, includedColumns, hasParentInput); } - private Map> getMaterialMapsWithInput(Map> keys, User user, Container container, boolean checkCrossFolderData, boolean verifyExisting, @Nullable Set columns) - throws QueryUpdateServiceException, InvalidKeyException + @Override + public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) + throws InvalidKeyException, QueryUpdateServiceException { ExistingRowSelect existingRowSelect = getExistingRowSelect(columns); TableInfo queryTableInfo = existingRowSelect.tableInfo; @@ -1242,7 +1188,7 @@ else if (name != null && materialSourceId != null) } } - if (checkCrossFolderData && !allKeys.isEmpty()) + if (verifyNoCrossFolderData && !allKeys.isEmpty()) { // Issue 52922: cross folder merge without Product Folders enabled silently ignores the cross folder row update ContainerFilter allCf = new ContainerFilter.AllInProjectPlusShared(container, user); // use a relaxed CF to find existing data from cross containers @@ -1282,37 +1228,27 @@ else if (name != null && materialSourceId != null) if (!existingRowSelect.includeParent) return sampleRows; - List materials = ExperimentServiceImpl.get().getExpMaterialsByLsid(rowNumLsid.values()); + Set lsids = new HashSet<>(); + for (Map sampleRow : sampleRows.values()) + lsids.add((String) sampleRow.get("lsid")); + List seeds = ExperimentServiceImpl.get().getExpMaterialsByLsid(lsids); - Map, Set>> parents = ExperimentServiceImpl.get().getParentMaterialAndDataMap(container, user, new HashSet<>(materials)); - - for (Map.Entry> rowNumSampleRow : sampleRows.entrySet()) - { - Integer rowNum = rowNumSampleRow.getKey(); - String lsidKey = rowNumLsid.get(rowNum); - Map sampleRow = rowNumSampleRow.getValue(); - - if (!parents.containsKey(lsidKey)) - continue; - - Pair, Set> sampleParents = parents.get(lsidKey); - - if (!sampleParents.first.isEmpty()) - addParentFields(sampleRow, sampleParents.first, ExpMaterial.MATERIAL_INPUT_PARENT + "/", user); - if (!sampleParents.second.isEmpty()) - addParentFields(sampleRow, sampleParents.second, ExpData.DATA_INPUT_PARENT + "/", user); - } + ExperimentServiceImpl.get().addRowsParentsFields(new HashSet<>(seeds), sampleRows, user, container); return sampleRows; } - public List> getRows(User user, Container container, List> keys, boolean addInputs) + @Override + public List> getRows(User user, Container container, List> keys) throws QueryUpdateServiceException { + if (!hasPermission(user, ReadPermission.class)) + throw new UnauthorizedException("You do not have permission to read data from this table."); + List> result = new ArrayList<>(keys.size()); for (Map k : keys) { - Map materialMap = getMaterialMap(getMaterialRowId(k), getMaterialLsid(k), user, container, addInputs); + Map materialMap = getMaterialMap(getMaterialRowId(k), getMaterialLsid(k), user, container, false); if (materialMap != null) result.add(materialMap); } From 2222d7fd92226397e9f68d8cfe2530548c548882 Mon Sep 17 00:00:00 2001 From: XingY Date: Sat, 13 Sep 2025 11:50:20 -0700 Subject: [PATCH 2/9] fix stackoverflow --- .../org/labkey/experiment/api/ExpDataClassDataTableImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 54b5918fa1c..068100ff247 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1235,7 +1235,7 @@ public List> insertRows(User user, Container container, List @Override public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) { - Map> dataRows = getExistingRows(user, container, keys, verifyNoCrossFolderData, verifyExisting, columns); + Map> dataRows = super.getExistingRows(user, container, keys, verifyNoCrossFolderData, verifyExisting, columns); boolean hasParentInput = false; if (_dataClass != null && columns != null && !columns.isEmpty()) { From 5b7d1c2f5701b9fe02721a9e980c1537e673ef3b Mon Sep 17 00:00:00 2001 From: XingY Date: Sat, 13 Sep 2025 12:02:01 -0700 Subject: [PATCH 3/9] fix build --- .../org/labkey/experiment/api/ExpDataClassDataTableImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 068100ff247..41d86aecc08 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1233,7 +1233,7 @@ public List> insertRows(User user, Container container, List } @Override - public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) + public Map> getExistingRows(User user, Container container, Map> keys, boolean verifyNoCrossFolderData, boolean verifyExisting, @Nullable Set columns) throws SQLException, QueryUpdateServiceException, InvalidKeyException { Map> dataRows = super.getExistingRows(user, container, keys, verifyNoCrossFolderData, verifyExisting, columns); boolean hasParentInput = false; From d3fc98efca0f08db217e946471db49f5194fd0f4 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 19 Sep 2025 10:00:17 -0700 Subject: [PATCH 4/9] Selenium test for source lineage audit detail --- api/src/org/labkey/api/audit/AuditHandler.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index 47eb678980b..2129ad513fd 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -21,9 +21,11 @@ import java.text.NumberFormat; import java.text.ParseException; import java.text.SimpleDateFormat; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -172,6 +174,15 @@ else if (!Objects.equals(oldValue, newValue) || isExtraAuditField) } else { + if (isExpInput && oldValue != null && newValue != null) + { + // For parent inputs, the order of the values does not matter, so compare as sets + Set oldSet = new HashSet<>(Arrays.asList(oldValue.toString().split(","))); + Set newSet = new HashSet<>(Arrays.asList(newValue.toString().split(","))); + if (oldSet.equals(newSet) && !isExtraAuditField) + continue; + } + originalRow.put(nameFromAlias, oldValue); modifiedRow.put(nameFromAlias, newValue); } From f40bc7103f1e07101ebc18491bce55f7ce9c68dd Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 19 Sep 2025 17:04:59 -0700 Subject: [PATCH 5/9] fix import --- .../labkey/experiment/api/ExpDataClassDataTableImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 18d50fe3593..7c1e3e8d23a 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1359,7 +1359,13 @@ else if (null != lsid) return dataRow; ExperimentService experimentService = ExperimentService.get(); - ExpData seed = lsid != null ? experimentService.getExpData(lsid) : experimentService.getExpData(_dataClass, rowId); + ExpData seed = null; + if (lsid != null && (rowId == null || rowId <= 0)) + seed = experimentService.getExpData(lsid); + else if (rowId != null && rowId > 0) + seed = experimentService.getExpData(_dataClass, rowId); + else if (name != null) + seed = experimentService.getExpData(_dataClass, name); if (null == seed) return dataRow; From 3706d5b6bdcc279691e9fdeb26354542254aaeee Mon Sep 17 00:00:00 2001 From: XingY Date: Sat, 20 Sep 2025 17:41:11 -0700 Subject: [PATCH 6/9] better handle comma --- .../org/labkey/api/audit/AuditHandler.java | 18 +++++++---- .../org/labkey/api/data/NameGenerator.java | 30 +++++++------------ .../labkey/api/exp/api/ExperimentService.java | 16 ++++++++++ .../api/ExpDataClassDataTableImpl.java | 2 +- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index 2129ad513fd..f780413f172 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -13,11 +13,11 @@ import org.labkey.api.exp.api.ExpMaterial; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.gwt.client.AuditBehaviorType; -import org.labkey.api.query.QueryKey; import org.labkey.api.query.QueryService; import org.labkey.api.security.User; import org.labkey.api.util.Pair; +import java.io.IOException; import java.text.NumberFormat; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -25,11 +25,11 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; public interface AuditHandler @@ -177,10 +177,16 @@ else if (!Objects.equals(oldValue, newValue) || isExtraAuditField) if (isExpInput && oldValue != null && newValue != null) { // For parent inputs, the order of the values does not matter, so compare as sets - Set oldSet = new HashSet<>(Arrays.asList(oldValue.toString().split(","))); - Set newSet = new HashSet<>(Arrays.asList(newValue.toString().split(","))); - if (oldSet.equals(newSet) && !isExtraAuditField) - continue; + try + { + Set oldSet = Arrays.stream(ExperimentService.getParentValues(oldValue.toString())).collect(Collectors.toSet()); + Set newSet = Arrays.stream(ExperimentService.getParentValues(newValue.toString())).collect(Collectors.toSet()); + if (oldSet.equals(newSet) && !isExtraAuditField) + continue; + } + catch (IOException ignore) + { + } } originalRow.put(nameFromAlias, oldValue); diff --git a/api/src/org/labkey/api/data/NameGenerator.java b/api/src/org/labkey/api/data/NameGenerator.java index aad531327cb..48030ff9467 100644 --- a/api/src/org/labkey/api/data/NameGenerator.java +++ b/api/src/org/labkey/api/data/NameGenerator.java @@ -745,26 +745,16 @@ public static Stream parentNames(Object value, String parentColName, TSV // using TabLoader instead of just splitting on the comma. boolean likelyAlreadyQuoted = valueStr.contains(",") || valueStr.contains("\n") || valueStr.contains("\r") || (valueStr.startsWith("\"") && valueStr.endsWith("\"")); String quotedStr = likelyAlreadyQuoted ? valueStr : tsvWriter.quoteValue(valueStr); // if value contains comma, no need to quote again - try (TabLoader tabLoader = new TabLoader(quotedStr)) - { - tabLoader.setDelimiterCharacter(','); - tabLoader.setUnescapeBackslashes(false); - // Issue 50924: LKSM: Importing samples using naming expression referencing parent inputs with # result in error - tabLoader.setIncludeComments(true); - // Issue 51056 Samples with single double quotes in the name will not resolve if added as parent samples. - tabLoader.setParseEnclosedQuotes(true); - try - { - String[][] parsedValues = tabLoader.getFirstNLines(1); - values = Arrays.stream(parsedValues[0]); - } - catch (IOException e) - { - if (errors != null) - errors.addRowError(new ValidationException("Unable to parse parent names from " + value, parentColName)); - else - throw new IllegalStateException("Unable to parse parent names from " + valueStr, e); - } + try + { + values = Arrays.stream(ExperimentService.getParentValues(quotedStr)); + } + catch (IOException e) + { + if (errors != null) + errors.addRowError(new ValidationException("Unable to parse parent names from " + value, parentColName)); + else + throw new IllegalStateException("Unable to parse parent names from " + valueStr, e); } } else if (value instanceof Collection coll) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index a39c96cc2ee..3b083214edb 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -75,6 +75,7 @@ import org.labkey.api.query.QueryViewProvider; import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; +import org.labkey.api.reader.TabLoader; import org.labkey.api.security.User; import org.labkey.api.services.ServiceRegistry; import org.labkey.api.util.Pair; @@ -521,6 +522,21 @@ static boolean isInputOutputColumn(String columnName) return null; } + static @NotNull String[] getParentValues(String valueStr) throws IOException + { + try (TabLoader tabLoader = new TabLoader(valueStr)) + { + tabLoader.setDelimiterCharacter(','); + tabLoader.setUnescapeBackslashes(false); + // Issue 50924: LKSM: Importing samples using naming expression referencing parent inputs with # result in error + tabLoader.setIncludeComments(true); + // Issue 51056 Samples with single double quotes in the name will not resolve if added as parent samples. + tabLoader.setParseEnclosedQuotes(true); + String[][] parsedValues = tabLoader.getFirstNLines(1); + return parsedValues[0]; + } + } + static Pair parseInputOutputAlias(String columnName) { if (!isInputOutputColumn(columnName)) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 7c1e3e8d23a..084bebe0307 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1285,7 +1285,7 @@ public List> getRows(User user, Container container, List> result = new ArrayList<>(keys.size()); for (Map row : keys) { - Map materialMap = getDataMap(row, user, container, true,false); + Map materialMap = getDataMap(row, user, container, true, false); if (materialMap != null) result.add(materialMap); } From bca25d58501e78dabd95b169a579c751c0ba13df Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 22 Sep 2025 16:31:34 -0700 Subject: [PATCH 7/9] code review changes --- api/src/org/labkey/api/audit/AuditHandler.java | 2 +- .../labkey/experiment/api/ExpDataClassDataTableImpl.java | 8 ++------ .../org/labkey/experiment/api/ExperimentServiceImpl.java | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index f780413f172..74af12f1990 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -106,7 +106,7 @@ static Pair, Map> getOldAndNewRecordForMerge } String lcName = nameFromAlias.toLowerCase(); // Preserve casing of inputs so we can show the names properly - boolean isExpInput = false; + boolean isExpInput = false; // TODO: extract exp lineage handling out of this generic method String encodedInputColumn = ExperimentService.getEncodedLineageKey(lcName); if (encodedInputColumn != null) { diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 084bebe0307..d16adc98aaa 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1301,10 +1301,6 @@ protected Map getRow(User user, Container container, Map _select(Container container, Object[] keys) throws throw new IllegalStateException(); } - protected Map getDataMap(Map keys, User user, Container container, boolean allowCrossContainer, boolean addInputs) throws SQLException + @Nullable protected Map getDataMap(Map keys, User user, Container container, boolean allowCrossContainer, boolean addInputs) throws SQLException { Long rowId = (Long)JdbcType.BIGINT.convert(keys.get(Column.RowId.name())); String lsid = (String)JdbcType.VARCHAR.convert(keys.get(Column.LSID.name())); @@ -1328,7 +1324,7 @@ protected Map getDataMap(Map keys, User user, Co if (classId == null) classId = _dataClass.getRowId(); - if (null == rowId && null == lsid && (null == name || null == classId)) + if (null == rowId && null == lsid && null == name) return null; // Issue 52886: Use queryTable here, not raw database table, so the rows are from the user schema with names diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 0f08ec3a877..d1441d648b5 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -2568,7 +2568,7 @@ else if (parent instanceof ExpMaterial materialParent) public void addRowsParentsFields(Set seeds, Map> dataRows, User user, Container container) { - Map, Set>> parents = ExperimentServiceImpl.get().getParentMaterialAndDataMap(container, user, new HashSet<>(seeds)); + Map, Set>> parents = ExperimentServiceImpl.get().getParentMaterialAndDataMap(container, user, seeds); for (Map dataRow : dataRows.values()) { From 6ff313a2af51d170fd80abd99f25db13c2c270c6 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 22 Sep 2025 16:47:26 -0700 Subject: [PATCH 8/9] fix test --- api/src/org/labkey/api/audit/AuditHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index 74af12f1990..a09036300f4 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -106,7 +106,7 @@ static Pair, Map> getOldAndNewRecordForMerge } String lcName = nameFromAlias.toLowerCase(); // Preserve casing of inputs so we can show the names properly - boolean isExpInput = false; // TODO: extract exp lineage handling out of this generic method + boolean isExpInput = false; // TODO: extract lineage handling out of this generic method String encodedInputColumn = ExperimentService.getEncodedLineageKey(lcName); if (encodedInputColumn != null) { From 1de15be7e4bb5447c38c3bc2dcba42bd50661083 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 22 Sep 2025 22:22:48 -0700 Subject: [PATCH 9/9] fix blank --- api/src/org/labkey/api/exp/api/ExperimentService.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 3b083214edb..5c6a0141513 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -524,6 +524,8 @@ static boolean isInputOutputColumn(String columnName) static @NotNull String[] getParentValues(String valueStr) throws IOException { + if (StringUtils.isEmpty(valueStr.trim())) + return new String[0]; try (TabLoader tabLoader = new TabLoader(valueStr)) { tabLoader.setDelimiterCharacter(',');