From 5a5b88c71c51f849444d5e98bc84c27ed478ef1d Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 1 Oct 2025 19:17:49 -0700 Subject: [PATCH 1/2] =?UTF-8?q?Issue=2053168:=20An=20aliquot=20can=20be=20?= =?UTF-8?q?updated=20to=20be=20its=20own=20parent=E2=80=99s=20parent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../labkey/experiment/ExpDataIterators.java | 47 ++++++++++++++++++ .../api/ExpDataClassDataTestCase.jsp | 16 +++++- .../experiment/api/ExpSampleTypeTestCase.jsp | 49 +++++++++++++++---- 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index f2a7afb154b..c00cc037956 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -1667,6 +1667,25 @@ public List getAliquotOutputs() } } + static Set getNearestChildDatas(Container c, User user, ExpRunItem start) + { + ExpLineageOptions options = new ExpLineageOptions(); + options.setParents(false); + + ExpLineage lineage = ExperimentService.get().getLineage(c, user, start, options); + return lineage.findNearestChildDatas(start); + } + + static Set getNearestChildMaterials(Container c, User user, ExpRunItem start) + { + ExpLineageOptions options = new ExpLineageOptions(); + options.setParents(false); + + ExpLineage lineage = ExperimentService.get().getLineage(c, user, start, options); + return lineage.findNearestChildMaterials(start); + } + + /** * support for mapping DataClass or SampleSet objects as a parent input using the column name format: * DataInputs/ or MaterialInputs/. Either / or . works as a delimiter @@ -1723,6 +1742,8 @@ else if (!aliquotParent.isOperationPermitted(SampleTypeService.SampleOperations. } } + Set existingChildData = null; + Set existingChildMaterials = null; for (Pair pair : entityNamePairs) { String entityColName = pair.first; @@ -1784,6 +1805,19 @@ else if (aliasPrefix != null && aliasSuffix != null) throw new ValidationException(String.format("Invalid import alias: parent SampleType [%1$s] does not exist or may have been deleted", namePart)); ExpMaterial sample = ExperimentService.get().findExpMaterial(c, user, entityName, sampleType, cache, materialMap); + + if (isUpdatingExisting && sample != null) + { + if (existingChildMaterials == null) + existingChildMaterials = getNearestChildMaterials(c, user, runItem); // lazy initialization + + for (ExpMaterial child : existingChildMaterials) + { + if (child.getRowId() == sample.getRowId()) + throw new ValidationException(String.format("'%s' is %s from sample '%s'. Circular relationships are not allowed.", entityName, child.getRootMaterialRowId() != null ? "aliquoted" : "derived", runItem.getName())); + } + } + if (sample != null) parentMaterials.put(sample, sampleRole(sample)); else @@ -1837,6 +1871,19 @@ else if (DATA_INPUT_PARENT.equalsIgnoreCase(aliasPrefix)) throw new ValidationException(String.format("Invalid import alias: parent DataClass [%1$s] does not exist or may have been deleted", namePart)); ExpData data = ExperimentService.get().findExpData(c, user, dataClass, namePart, entityName, cache, dataMap); + + if (isUpdatingExisting && data != null) + { + if (existingChildData == null) + existingChildData = getNearestChildDatas(c, user, runItem); // lazy initialization + + for (ExpData child : existingChildData) + { + if (child.getRowId() == data.getRowId()) + throw new ValidationException(String.format("'%s' is child of the current source '%s'. Circular relationships are not allowed.", entityName, runItem.getName())); + } + } + if (data != null) parentData.put(data, dataRole(data, user)); else diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp index f85560f2113..716214b0022 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp @@ -109,6 +109,7 @@ <%@ page import="static org.labkey.api.util.PageFlowUtil.encodeURIComponent" %> <%@ page import="static org.labkey.api.exp.api.ExperimentService.asInteger" %> <%@ page import="static org.labkey.api.exp.api.ExperimentService.asLong" %> +<%@ page import="static org.hamcrest.Matchers.containsString" %> <%@ page extends="org.labkey.api.jsp.JspTest.BVT" %> <%! @@ -1044,7 +1045,7 @@ public void testInsertOptionUpdate() throws Exception String longFieldName = "Field100 ABCDEFGHIJKLMNOPQRSTUVWXYZ%()=+-[]_|*`'\":;<>?!@#^AABBCCDDEEFFGGHHIIJJKKLLMMNNOOPPQQRRSSTTU)"; props.add(new GWTPropertyDescriptor(longFieldName, "string")); - ExperimentServiceImpl.get().createDataClass(c, user, dataClassName, null, props, emptyList(), null, null); + ExpDataClass dataClass = ExperimentServiceImpl.get().createDataClass(c, user, dataClassName, null, props, emptyList(), null, null); List> rowsToAdd = new ArrayList<>(); rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1", "prop", "a", longFieldName, "Very", "alias", "Much", "flag", "c100")); rowsToAdd.add(CaseInsensitiveHashMap.of("name", "D-1-d", "prop", "c", longFieldName, "Long", "alias", "Extended", "flag", "c200")); @@ -1082,6 +1083,19 @@ public void testInsertOptionUpdate() throws Exception assertFalse(context.getErrors().hasErrors()); assertEquals(3, count); + // Issue 53168: circular lineage not allowed + rowsToUpdate.clear(); + rowsToUpdate.add(CaseInsensitiveHashMap.of("rowId", dataClass.getData(c, "D-1").getRowId(), "DataInputs/DataClassWithImportOption", "D-1-d")); + try + { + qus.updateRows(user, c, rowsToUpdate, null, new BatchValidationException(), null, null); + fail("Expected to throw exception"); + } + catch (Exception e) + { + assertThat(e.getMessage(), containsString("'D-1-d' is child of the current source 'D-1'. Circular relationships are not allowed.")); + } + Set columnNames = new HashSet<>(); columnNames.add("rowId"); columnNames.add("lsid"); diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index 035d1407080..d485ae1777e 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -647,7 +647,7 @@ public void testUpdateSomeParents() throws Exception // add first parents TableInfo table = schema.getTable("Parent1Samples", null, true, false); - QueryUpdateService svc = table.getUpdateService(); + QueryUpdateService svcParent = table.getUpdateService(); BatchValidationException errors = new BatchValidationException(); rows.add(CaseInsensitiveHashMap.of("name", "P1-1")); @@ -655,25 +655,25 @@ public void testUpdateSomeParents() throws Exception rows.add(CaseInsensitiveHashMap.of("name", "P1-3")); rows.add(CaseInsensitiveHashMap.of("name", "P1-4,test")); - List> inserted = svc.insertRows(user, c, rows, errors, null, null); + List> inserted = svcParent.insertRows(user, c, rows, errors, null, null); assertFalse(errors.hasErrors()); assertEquals("Number of parent1 samples inserted not as expected", 4, inserted.size()); // add second parents table = schema.getTable("Parent2Samples", null, true, false); - svc = table.getUpdateService(); + QueryUpdateService svcParent2 = table.getUpdateService(); rows.clear(); rows.add(CaseInsensitiveHashMap.of("name", "P2-1")); rows.add(CaseInsensitiveHashMap.of("name", "P2-2")); rows.add(CaseInsensitiveHashMap.of("name", "P2-3")); - inserted = svc.insertRows(user, c, rows, errors, null, null); + inserted = svcParent2.insertRows(user, c, rows, errors, null, null); assertFalse(errors.hasErrors()); assertEquals("Number of parent2 samples inserted not as expected", 3, inserted.size()); // add child samples table = schema.getTable("ChildSamples", null, true, false); - svc = table.getUpdateService(); + QueryUpdateService svcChild = table.getUpdateService(); rows.clear(); rows.add(CaseInsensitiveHashMap.of("name", "C1", "MaterialInputs/Parent1Samples", "P1-1,P1-2", "MaterialInputs/Parent2Samples", "P2-1")); @@ -682,10 +682,25 @@ public void testUpdateSomeParents() throws Exception rows.add(CaseInsensitiveHashMap.of("name", "C4", "MaterialInputs/Parent1Samples", "P1-2", "MaterialInputs/Parent2Samples", "P2-2")); rows.add(CaseInsensitiveHashMap.of("name", "C5", "MaterialInputs/Parent1Samples", "P1-1, \"P1-4,test\", P1-2")); - inserted = svc.insertRows(user, c, rows, errors, null, null); + inserted = svcChild.insertRows(user, c, rows, errors, null, null); assertFalse(errors.hasErrors()); assertEquals("Number of child samples inserted not as expected", 5, inserted.size()); + // Issue 53168: circular lineage not allowed + rows.clear(); + rows.add(CaseInsensitiveHashMap.of("rowId", parent2Type.getSample(c, "P2-1").getRowId(), "MaterialInputs/ChildSamples", "C1")); + try + { + svcParent2.updateRows(user, c, rows, null, errors, null, null); + fail("Expected to throw exception"); + } + catch (Exception e) + { + assertThat(e.getMessage(), containsString("'C1' is aliquoted from sample 'P2-1'. Circular relationships are not allowed.")); + } + + errors = new BatchValidationException(); + ExpMaterial P11 = parent1Type.getSample(c, "P1-1"); ExpMaterial P12 = parent1Type.getSample(c, "P1-2"); ExpMaterial P14 = parent1Type.getSample(c, "P1-4,test"); @@ -708,7 +723,7 @@ public void testUpdateSomeParents() throws Exception rows.add(CaseInsensitiveHashMap.of("name", "C1", "MaterialInputs/Parent1Samples", "P1-1")); // change one parent but not the other rows.add(CaseInsensitiveHashMap.of("name", "C4", "MaterialInputs/Parent1Samples", null)); // remove one parent but not the other - svc.mergeRows(user, c, MapDataIterator.of(rows), errors, null, null); + svcChild.mergeRows(user, c, MapDataIterator.of(rows), errors, null, null); assertFalse(errors.hasErrors()); ExpLineage lineage = ExpLineageService.get().getLineage(c, user, C1, opts); @@ -725,7 +740,7 @@ public void testUpdateSomeParents() throws Exception rows.add(CaseInsensitiveHashMap.of("name", "C4", "MaterialInputs/Parent1Samples", "P1-1", "MaterialInputs/Parent2Samples", "P2-1")); // change both parents rows.add(CaseInsensitiveHashMap.of("name", "C2", "MaterialInputs/Parent1Samples", "", "MaterialInputs/Parent2Samples", null)); // remove both parents - svc.mergeRows(user, c, MapDataIterator.of(rows), errors, null, null); + svcChild.mergeRows(user, c, MapDataIterator.of(rows), errors, null, null); assertFalse(errors.hasErrors()); lineage = ExpLineageService.get().getLineage(c, user, C2, opts); @@ -1142,7 +1157,7 @@ public void testInsertOptionUpdate() throws Exception props.add(new GWTPropertyDescriptor(longFieldName, "string")); final String sampleTypeName = "TestSamplesWithRequired"; - SampleTypeService.get().createSampleType(c, user, sampleTypeName, null, props, Collections.emptyList(), -1, -1, -1, -1, null); + ExpSampleType sampleType = SampleTypeService.get().createSampleType(c, user, sampleTypeName, null, props, Collections.emptyList(), -1, -1, -1, -1, null); TableInfo table = getSampleTypeTable(sampleTypeName); QueryUpdateService qus = table.getUpdateService(); @@ -1164,7 +1179,21 @@ public void testInsertOptionUpdate() throws Exception assertFalse(context.getErrors().hasErrors()); assertEquals("Unexpected count from IMPORT on loadRows()", 3, count); - var rows = getSampleRows(sampleTypeName); + // Issue 53168: aliquot cannot be parent to its aliquot parent + BatchValidationException errors = new BatchValidationException(); + List> rows = new ArrayList<>(); + rows.add(CaseInsensitiveHashMap.of("rowId", sampleType.getSample(c, "S-1").getRowId(), "MaterialInputs/" + sampleTypeName, "S-1-1")); + try + { + qus.updateRows(user, c, rows, null, errors, null, null); + fail("Expected to throw exception"); + } + catch (Exception e) + { + assertThat(e.getMessage(), containsString("'S-1-1' is aliquoted from sample 'S-1'. Circular relationships are not allowed.")); + } + + rows = getSampleRows(sampleTypeName); assertEquals("S-1", rows.get(0).get("name")); assertEquals(1, rows.get(0).get("aliquotcount")); assertEquals(0.0, rows.get(0).get("aliquotvolume")); From d228c79f4b699ca60d19bfdcaa21cbb06abead26 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 3 Oct 2025 12:33:17 -0700 Subject: [PATCH 2/2] fix aliquot msg --- experiment/src/org/labkey/experiment/ExpDataIterators.java | 2 +- .../src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index c00cc037956..264733d0a42 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -1814,7 +1814,7 @@ else if (aliasPrefix != null && aliasSuffix != null) for (ExpMaterial child : existingChildMaterials) { if (child.getRowId() == sample.getRowId()) - throw new ValidationException(String.format("'%s' is %s from sample '%s'. Circular relationships are not allowed.", entityName, child.getRootMaterialRowId() != null ? "aliquoted" : "derived", runItem.getName())); + throw new ValidationException(String.format("'%s' is %s from sample '%s'. Circular relationships are not allowed.", entityName, child.getRootMaterialRowId() != child.getRowId() ? "aliquoted" : "derived", runItem.getName())); } } diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index d485ae1777e..ff3606d9a6b 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -696,7 +696,7 @@ public void testUpdateSomeParents() throws Exception } catch (Exception e) { - assertThat(e.getMessage(), containsString("'C1' is aliquoted from sample 'P2-1'. Circular relationships are not allowed.")); + assertThat(e.getMessage(), containsString("'C1' is derived from sample 'P2-1'. Circular relationships are not allowed.")); } errors = new BatchValidationException();