Skip to content

Issue 53168: An aliquot can be updated to be its own parent’s parent #7091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
XingY merged 3 commits into from
Oct 6, 2025
Merged

Issue 53168: An aliquot can be updated to be its own parent’s parent #7091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a5b88c
Issue 53168: An aliquot can be updated to be its own parent’s parent
XingY Oct 2, 2025
4f55e83
Merge branch 'develop' into fb_issue53168
XingY Oct 3, 2025
d228c79
fix aliquot msg
XingY Oct 3, 2025
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
47 changes: 47 additions & 0 deletions experiment/src/org/labkey/experiment/ExpDataIterators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,25 @@ public List<ExpMaterial> getAliquotOutputs()
}
}

static Set<ExpData> 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<ExpMaterial> 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/<data class name> or MaterialInputs/<sample type name>. Either / or . works as a delimiter
Expand Down Expand Up @@ -1723,6 +1742,8 @@ else if (!aliquotParent.isOperationPermitted(SampleTypeService.SampleOperations.
}
}

Set<ExpData> existingChildData = null;
Set<ExpMaterial> existingChildMaterials = null;
for (Pair<String, String> pair : entityNamePairs)
{
String entityColName = pair.first;
Expand Down Expand Up @@ -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() != child.getRowId() ? "aliquoted" : "derived", runItem.getName()));
}
}

if (sample != null)
parentMaterials.put(sample, sampleRole(sample));
else
Expand Down Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" %>

<%!
Expand Down Expand Up @@ -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<Map<String, Object>> 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"));
Expand Down Expand Up @@ -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<String> columnNames = new HashSet<>();
columnNames.add("rowId");
columnNames.add("lsid");
Expand Down
49 changes: 39 additions & 10 deletions experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,33 +647,33 @@ 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"));
rows.add(CaseInsensitiveHashMap.of("name", "P1-2"));
rows.add(CaseInsensitiveHashMap.of("name", "P1-3"));
rows.add(CaseInsensitiveHashMap.of("name", "P1-4,test"));

List<Map<String, Object>> inserted = svc.insertRows(user, c, rows, errors, null, null);
List<Map<String, Object>> 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"));
Expand All @@ -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 derived 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");
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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<Map<String, Object>> 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"));
Expand Down