diff --git a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java index 1835704bd0..e98c7e09dc 100644 --- a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java +++ b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java @@ -34,6 +34,7 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.params.FieldDefinition.ColumnType; import org.labkey.test.params.FieldInfo; +import org.labkey.test.params.experiment.DataClassDefinition; import org.labkey.test.params.experiment.SampleTypeDefinition; import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; @@ -58,12 +59,13 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; +import java.util.regex.Pattern; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.labkey.test.params.FieldDefinition.DOMAIN_TRICKY_CHARACTERS; import static org.labkey.test.util.EscapeUtil.escapeForNameExpression; @@ -514,53 +516,134 @@ public void testWithTrickyFieldNames() throws IOException, CommandException } + // Field used in the testInputsExpression test. + private static final String SHARED_FIELD_NAME = "FieldB"; + @Test - public void testInputsExpression() + public void testInputsExpression() throws IOException, CommandException { + verifyNames( "InputsExpressionTest", - "Name\tFieldB\tMaterialInputs/InputsExpressionTest", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/InputsExpressionTest", "${Inputs:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Pat"); - final String urlLikeDefaultVal = "a+b%c#d"; verifyNames( "InputsExpressionTest2", - "Name\tFieldB\tMaterialInputs/InputsExpressionTest2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/InputsExpressionTest2", "${Inputs:first:defaultValue('" + urlLikeDefaultVal + "')}_${batchRandomId}", null, "Fed", true, urlLikeDefaultVal); verifyNames( "Inputs/ExpressionTest2", - "Name\tFieldB\tMaterialInputs/Inputs$SExpressionTest2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs$SExpressionTest2", "${Inputs:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Bat", false); verifyNames( "Inputs/WithDataTypeExpression", - "Name\tFieldB\tMaterialInputs/Inputs/WithDataTypeExpression", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs/WithDataTypeExpression", "${Inputs/Inputs$SWithDataTypeExpression:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Red"); verifyNames( "Inputs/WithDataTypeExpression2", - "Name\tFieldB\tMaterialInputs/Inputs$SWithDataTypeExpression2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs$SWithDataTypeExpression2", "${Inputs/Inputs$SWithDataTypeExpression2:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Ted", false); verifyNames( "Material/WithDataTypeExpression", - "Name\tFieldB\tMaterialInputs/Material/WithDataTypeExpression", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Material/WithDataTypeExpression", "${MaterialInputs/Material$SWithDataTypeExpression:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Ned"); verifyNames( "Material/WithDataTypeExpression2", - "Name\tFieldB\tMaterialInputs/Material$SWithDataTypeExpression2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Material$SWithDataTypeExpression2", "${MaterialInputs/Material$SWithDataTypeExpression2:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Med", false); + // Test that a name expression using a generic "Inputs" and a field name can resolve the field name in Material and Data classes. + log("Test a name expression that uses \"Inputs/SomeFieldName\"."); + + log("Create a DataClass to use as a parent."); + final String dataParentClass = "InputsDataParentFieldExpression"; + DataClassDefinition dpDef = new DataClassDefinition(dataParentClass); + dpDef.setFields(List.of(new FieldDefinition(SHARED_FIELD_NAME, ColumnType.String))); + TestDataGenerator dpGen = dpDef.create(createDefaultConnection(), getProjectName()); + dpGen.addCustomRow(Map.of("Name", "DS-1", + SHARED_FIELD_NAME, "ABC")); + dpGen.addCustomRow(Map.of("Name", "DS-2", + SHARED_FIELD_NAME, "DEF")); + dpGen.insertRows(createDefaultConnection(), dpGen.getRows()); + + final String nameExpression = "Gen-${Inputs/" + SHARED_FIELD_NAME + ":defaultValue('none')}-${genId}"; + + StringBuilder data = new StringBuilder(); + data.append(SHARED_FIELD_NAME); + data.append("\tDataInputs/InputsDataParentFieldExpression\n"); + data.append("a\tDS-1\n"); + data.append("b\tDS-2\n"); + data.append("c\tDS-1, DS-2\n"); + data.append("d\n"); + + goToProjectHome(); + final String sampleTypeName = "InputsExpressionTestData"; + createSampleTypeForNameValidation(sampleTypeName, + nameExpression, null, data.toString()); + + DataRegionTable materialTable = new DataRegionTable("Material", this); + List actualNames = materialTable.getColumnDataAsText("Name"); + + List dataSourceParentNames = List.of( + Pattern.quote("Gen-none-") + ".*", + Pattern.quote("Gen-[ABC, DEF]-") + ".*", + Pattern.quote("Gen-DEF-") + ".*", + Pattern.quote("Gen-ABC-") + ".*" + ); + validateNamesGenerated(actualNames, dataSourceParentNames); + + data = new StringBuilder(); + data.append("Name\t"); + data.append(SHARED_FIELD_NAME); + data.append("\n"); + data.append("S-1\tUVW\n"); + data.append("S-2\tXYZ\n"); + + goToProjectHome(); + final String parentSampleType = "InputsMaterialParentFieldExpression"; + createSampleTypeForNameValidation(parentSampleType, + "S-${genId}", null, data.toString()); + + data = new StringBuilder(); + data.append(SHARED_FIELD_NAME); + data.append("\tMaterialInputs/"); + data.append(parentSampleType); + data.append("\n"); + data.append("i\tS-1\n"); + data.append("j\tS-2\n"); + data.append("k\tS-1, S-2\n"); + data.append("l\n"); + + goToProjectHome(); + SampleTypeHelper sampleHelper = new SampleTypeHelper(this); + sampleHelper.goToSampleType(sampleTypeName).bulkImport(data.toString()); + + List sampleTypeParentNames = new ArrayList<>(); + sampleTypeParentNames.add(Pattern.quote("Gen-none-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-[UVW, XYZ]-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-XYZ-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-UVW-") + ".*"); + sampleTypeParentNames.addAll(dataSourceParentNames); + + materialTable = new DataRegionTable("Material", this); + actualNames = materialTable.getColumnDataAsText("Name"); + + validateNamesGenerated(actualNames, sampleTypeParentNames); + } @Test @@ -663,30 +746,87 @@ private void verifyNames(String sampleTypeName, String header, String nameExpres // Name generated and uses defaultValue('SS') "\tb\t\n"; + createSampleTypeForNameValidation(sampleTypeName, nameExpression, currentTypeAlias, data); + + DataRegionTable materialTable = new DataRegionTable("Material", this); + List actualNames = materialTable.getColumnDataAsText("Name"); + + List expectedNames = new ArrayList<>(); + + expectedNames.add(Pattern.quote(defaultValue + "_") + ".*"); + + String batchRandomId = actualNames.getFirst().substring(actualNames.getFirst().lastIndexOf("_") + 1); + + expectedNames.add(Pattern.quote(name2 + "_" + batchRandomId)); + + String pattern = useFirst ? name1 : "[" + name1 + ", " + name2 + "]"; + pattern = pattern + "_" + batchRandomId; + expectedNames.add(Pattern.quote(pattern)); + + expectedNames.add(Pattern.quote(name2)); + + expectedNames.add(Pattern.quote(name1)); + + validateNamesGenerated(actualNames, expectedNames); + } + + private void createSampleTypeForNameValidation(String sampleTypeName, String nameExpression, @Nullable String currentTypeAlias, String data) + { + log(String.format("Create a sample type %s with a name expression of %s and populate it using the name expression.", + sampleTypeName, nameExpression)); + SampleTypeHelper sampleHelper = new SampleTypeHelper(this); SampleTypeDefinition definition = new SampleTypeDefinition(sampleTypeName) .setNameExpression(nameExpression); if (currentTypeAlias != null) definition = definition.setParentAliases(Map.of(currentTypeAlias, "(Current Sample Type)")); - definition = definition.setFields(List.of(new FieldDefinition("FieldB", ColumnType.String))); + definition = definition.setFields(List.of(new FieldDefinition(SHARED_FIELD_NAME, ColumnType.String))); sampleHelper.createSampleType(definition, data); assertTextPresent(nameExpression); - DataRegionTable materialTable = new DataRegionTable("Material", this); - List names = materialTable.getColumnDataAsText("Name"); + } - // The next two lines assume the name expression has specific values in specific locations, and as far as I - // can tell, that is how the tests are written. - assertTrue("First name (" + names.get(0) + ") expected to start with " + defaultValue + "_ but it did not", names.get(0).startsWith(defaultValue + "_")); - String batchRandomId = names.get(0).substring(names.get(0).lastIndexOf("_") + 1); + /** + *

+ * Validate that every name in {@code actualNames} matches one of the regex patterns in + * {@code expectedNames}, using a multiset diff: every expected pattern must match at least + * one actual name, every actual name must be matched by some expected pattern, and the + * counts must be equal. Order is intentionally NOT verified. + *

+ *

+ * On a mismatch the failure reports both the patterns that found no match and the actual + * names that found no pattern, so a "missing row" or "extra row" failure is immediately + * diagnosable instead of just a "wrong count" error. + *

+ *

+ * This method was generated by Claude. + *

+ */ + private void validateNamesGenerated(List actualNames, List expectedNames) + { + checker().verifyEquals("Number of samples not as expected.", + expectedNames.size(), actualNames.size()); - assertEquals("Second name not as expected", name2 + "_" + batchRandomId, names.get(1)); + List unmatchedActuals = new ArrayList<>(actualNames); + List unmatchedPatterns = new ArrayList<>(); + for (String pattern : expectedNames) + { + Optional match = unmatchedActuals.stream() + .filter(a -> Pattern.matches(pattern, a)) + .findFirst(); + if (match.isPresent()) + unmatchedActuals.remove(match.get()); + else + unmatchedPatterns.add(pattern); + } - assertEquals("Third name not as expected", (useFirst ? name1 : ("[" + name1 + ", " + name2 + "]")) + "_" + batchRandomId, names.get(2)); + checker().verifyTrue("Expected patterns with no matching actual name: " + unmatchedPatterns, + unmatchedPatterns.isEmpty()); + checker().verifyTrue("Actual names not matching any expected pattern (extras): " + unmatchedActuals, + unmatchedActuals.isEmpty()); - assertEquals("Fourth name not as expected", name2, names.get(3)); - assertEquals("Fifth name not as expected", name1, names.get(4)); + checker().screenShotIfNewError("Generated_Names_Error"); } /**