Skip to content

Issue 53431: Data Class and Sample Type data doesn't round-trip via folder export/import for field names with special char #2556

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.

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

Open
wants to merge 5 commits into
base: release25.7-SNAPSHOT
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions src/org/labkey/test/params/FieldDefinition.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public class FieldDefinition extends PropertyDescriptor
// Collection of JSON properties not explicitly known by 'PropertyDescriptor'
private final Map<String, Object> _extraFieldProperties = new HashMap<>();

private String _namePart;

/**
* Define a non-lookup field of the specified type
* @param name field name
Expand Down Expand Up @@ -467,6 +469,16 @@ public void setAliquotOption(ExpSchema.DerivationDataScopeType aliquotOption)
_aliquotOption = aliquotOption;
}

public void setNamePart(String namePart)
{
_namePart = namePart;
}

public boolean isNamePartMatch(String namePart)
{
return _namePart != null && _namePart.equals(namePart);
}

public enum RangeType
{
Equals("Equals", Filter.Operator.EQUAL),
Expand Down
14 changes: 13 additions & 1 deletion src/org/labkey/test/params/FieldInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class FieldInfo implements CharSequence, WrapsFieldKey
private final String _label;
private final ColumnType _columnType;
private final Consumer<FieldDefinition> _fieldDefinitionMutator;
private String _namePart; // used for random field generation to track the name part used

private FieldInfo(FieldKey fieldKey, String label, ColumnType columnType, Consumer<FieldDefinition> fieldDefinitionMutator)
{
Expand Down Expand Up @@ -54,7 +55,9 @@ public FieldInfo(String name)
*/
public static FieldInfo random(String namePart, ColumnType columnType)
{
return new FieldInfo(TestDataGenerator.randomFieldName(namePart), columnType);
FieldInfo field = new FieldInfo(TestDataGenerator.randomFieldName(namePart), columnType);
field.setNamePart(namePart);
return field;
}

/**
Expand Down Expand Up @@ -165,9 +168,18 @@ private FieldDefinition getFieldDefinition(ColumnType columnType)
{
_fieldDefinitionMutator.accept(fieldDefinition);
}
if (_namePart != null)
{
fieldDefinition.setNamePart(_namePart);
}
return fieldDefinition;
}

private void setNamePart(String namePart)
{
_namePart = namePart;
}

@Override
public int length()
{
Expand Down
4 changes: 2 additions & 2 deletions src/org/labkey/test/tests/FileAttachmentColumnTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private void importSampleDataUI(String sampleTypeName, String containerPath, Lis
for (File file : files)
{
sampleFileData.add(Map.of("Name", file.getName(), "Color", "green",
LIST_ATTACHMENT_FIELD.getName(), file.getName()));
"file", file.getName()));
}
helper.bulkImport(sampleFileData);
}
Expand Down Expand Up @@ -542,7 +542,7 @@ private void validateSampleData(String sampleType, String folderPath, List<File>
}
else
{
WebElement fileLinkCell = samplesRegion.findCell(rowIndex, LIST_ATTACHMENT_FIELD.getName());
WebElement fileLinkCell = samplesRegion.findCell(rowIndex, "file");
Optional<WebElement> optionalFileLink = Locator.tag("a").findOptionalElement(fileLinkCell);
checker().withScreenshot("unexpected_file_state")
.awaiting(Duration.ofSeconds(2),
Expand Down
67 changes: 50 additions & 17 deletions src/org/labkey/test/tests/SampleTypeFolderExportImportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.labkey.test.params.experiment.DataClassDefinition;
import org.labkey.test.params.experiment.SampleTypeDefinition;
import org.labkey.test.util.DataRegionTable;
import org.labkey.test.util.EscapeUtil;
import org.labkey.test.util.LogMethod;
import org.labkey.test.util.PortalHelper;
import org.labkey.test.util.SampleTypeHelper;
Expand Down Expand Up @@ -435,6 +436,9 @@ public void testExportImportDerivedSamples() throws Exception

// arrange - 2 sample types, one with samples derived from parents in the other (and also parents in the same one)
List<FieldDefinition> testFields = SampleTypeAPIHelper.sampleTypeTestFields(false);
FieldDefinition intColumn = getFieldByNamePart(testFields, "int,./Column");
FieldDefinition stringColumn = getFieldByNamePart(testFields, "stringColumn");
FieldDefinition decimalColumn = getFieldByNamePart(testFields, "decimalColumn");
DataClassDefinition dataClassType = new DataClassDefinition(dataClass).setFields(DataClassAPIHelper.dataClassTestFields());
SampleTypeDefinition parentType = new SampleTypeDefinition(parentSampleType).setFields(testFields);
SampleTypeDefinition testSampleType = new SampleTypeDefinition(testSamples).setFields(testFields)
Expand All @@ -449,25 +453,25 @@ public void testExportImportDerivedSamples() throws Exception
dataClassDgen.insertRows();

TestDataGenerator parentDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, parentType);
parentDgen.addCustomRow(Map.of("Name", "Parent1", "intColumn", 1, "floatColumn", 1.1, "stringColumn", "one"));
parentDgen.addCustomRow(Map.of("Name", "Parent2", "intColumn", 2, "floatColumn", 2.2, "stringColumn", "two"));
parentDgen.addCustomRow(Map.of("Name", "Parent3", "intColumn", 3, "floatColumn", 3.3, "stringColumn", "three"));
parentDgen.addCustomRow(Map.of("Name", "Parent1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one"));
parentDgen.addCustomRow(Map.of("Name", "Parent2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two"));
parentDgen.addCustomRow(Map.of("Name", "Parent3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three"));
parentDgen.insertRows();

TestDataGenerator testDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, testSampleType);
testDgen.addCustomRow(Map.of("Name", "Child1", "intColumn", 1, "decimalColumn", 1.1, "stringColumn", "one",
testDgen.addCustomRow(Map.of("Name", "Child1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one",
"Parent", "Parent1"));
testDgen.addCustomRow(Map.of("Name", "Child2", "intColumn", 2, "decimalColumn", 2.2, "stringColumn", "two",
testDgen.addCustomRow(Map.of("Name", "Child2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two",
"Parent", "Parent2"));
testDgen.addCustomRow(Map.of("Name", "Child3", "intColumn", 3, "decimalColumn", 3.3, "stringColumn", "three",
testDgen.addCustomRow(Map.of("Name", "Child3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three",
"Parent", "Parent3", "DataClassParent", "data1"));
testDgen.addCustomRow(Map.of("Name", "Child4", "intColumn", 4, "decimalColumn", 4.4, "stringColumn", "four",
testDgen.addCustomRow(Map.of("Name", "Child4", intColumn.getName(), 4, decimalColumn.getName(), 4.4, stringColumn.getName(), "four",
"Parent", "Parent3, Parent2"));
testDgen.addCustomRow(Map.of("Name", "Child5", "intColumn", 5, "decimalColumn", 5.5, "stringColumn", "five",
testDgen.addCustomRow(Map.of("Name", "Child5", intColumn.getName(), 5, decimalColumn.getName(), 5.5, stringColumn.getName(), "five",
"Parent", "Parent1, Parent2"));
testDgen.addCustomRow(Map.of("Name", "Child6", "intColumn", 6, "decimalColumn", 6.6, "stringColumn", "six",
testDgen.addCustomRow(Map.of("Name", "Child6", intColumn.getName(), 6, decimalColumn.getName(), 6.6, stringColumn.getName(), "six",
"Parent", "Parent3, Parent2", "SelfParent", "Child5"));
testDgen.addCustomRow(Map.of("Name", "Child7", "intColumn", 7, "decimalColumn", 7.7, "stringColumn", "seven",
testDgen.addCustomRow(Map.of("Name", "Child7", intColumn.getName(), 7, decimalColumn.getName(), 7.7, stringColumn.getName(), "seven",
"Parent", "Parent3, Parent2", "SelfParent", "Child5", "DataClassParent", "data2, data3"));
testDgen.insertRows();

Expand Down Expand Up @@ -523,12 +527,18 @@ public void testExportImportDerivedSamples() throws Exception
.findFirst().orElse(null);
assertNotNull("expect all matching rows to come through", matchingMap);

String intColFieldKey = EscapeUtil.fieldKeyEncodePart(intColumn.getName());
assertNotNull("expect intColumn to be present in exported row", exportedRow.get(intColFieldKey));
assertThat("expect export and import values to be equivalent",
exportedRow.get("intColumn"), equalTo(matchingMap.get("intColumn")));
exportedRow.get(intColFieldKey), equalTo(matchingMap.get(intColFieldKey)));
String stringColFieldKey = EscapeUtil.fieldKeyEncodePart(stringColumn.getName());
assertNotNull("expect stringColumn to be present in exported row", exportedRow.get(stringColFieldKey));
assertThat("expect export and import values to be equivalent",
exportedRow.get("stringColumn"), equalTo(matchingMap.get("stringColumn")));
exportedRow.get(stringColFieldKey), equalTo(matchingMap.get(stringColFieldKey)));
String decimalColFieldKey = EscapeUtil.fieldKeyEncodePart(decimalColumn.getName());
assertNotNull("expect decimalColumn to be present in exported row", exportedRow.get(decimalColFieldKey));
assertThat("expect export and import values to be equivalent",
exportedRow.get("decimalColumn"), equalTo(matchingMap.get("decimalColumn")));
exportedRow.get(decimalColFieldKey), equalTo(matchingMap.get(decimalColFieldKey)));

List<String> sourceParents = Arrays.asList(exportedRow.get("Inputs/Materials/parentSamples").toString()
.replace(" ", "").split(","));
Expand Down Expand Up @@ -556,13 +566,16 @@ public void testExportImportSampleTypesWithAssayRuns() throws Exception

// create a test sampleType
List<FieldDefinition> testFields = SampleTypeAPIHelper.sampleTypeTestFields(true);
FieldDefinition intColumn = getFieldByNamePart(testFields, "int,./Column");
FieldDefinition stringColumn = getFieldByNamePart(testFields, "stringColumn");
FieldDefinition decimalColumn = getFieldByNamePart(testFields, "decimalColumn");
SampleTypeDefinition testSampleType = new SampleTypeDefinition(testSamples).setFields(testFields)
.addParentAlias("SelfParent"); // to derive from samles in the current type
.addParentAlias("SelfParent"); // to derive from samples in the current type

TestDataGenerator parentDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, testSampleType);
parentDgen.addCustomRow(Map.of("Name", "sample1", "intColumn", 1, "decimalColumn", 1.1, "stringColumn", "one"));
parentDgen.addCustomRow(Map.of("Name", "sample2", "intColumn", 2, "decimalColumn", 2.2, "stringColumn", "two"));
parentDgen.addCustomRow(Map.of("Name", "sample3", "intColumn", 3, "decimalColumn", 3.3, "stringColumn", "three"));
parentDgen.addCustomRow(Map.of("Name", "sample1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one"));
parentDgen.addCustomRow(Map.of("Name", "sample2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two"));
parentDgen.addCustomRow(Map.of("Name", "sample3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three"));
parentDgen.insertRows();

goToProjectFolder(getProjectName(), subfolder);
Expand Down Expand Up @@ -685,8 +698,28 @@ public void testExportImportSampleTypesWithAssayRuns() throws Exception
File downloadedFile = doAndWaitForDownload(() -> waitAndClick(WAIT_FOR_JAVASCRIPT, Locator.tagWithAttribute("a", "title", "Download attached file"), 0));
assertElementPresent("Did not find the expected number of icons for " + SAMPLE_TXT_FILE.getName() + " from the imported samples.", Locator.tagContainingText("a", "sample.txt"), 1);
checker().verifyTrue("Incorrect file content for sample.txt after folder import", FileUtils.contentEquals(downloadedFile, SAMPLE_TXT_FILE));

// verify the other sample type data is round-tripped as expected
importedDataTable = DataRegionTable.DataRegion(getDriver()).withName("Material").waitFor();
checker().verifyEquals("Name column data not as expected", List.of("sample3", "sample2", "sample1"),
importedDataTable.getColumnDataAsText("Name"));
checker().verifyEquals("intColumn column data not as expected", List.of("3", "2", "1"),
importedDataTable.getColumnDataAsText(intColumn.getName()));
checker().verifyEquals("decimalColumn column data not as expected", List.of("3.3", "2.2", "1.1"),
importedDataTable.getColumnDataAsText(decimalColumn.getName()));
checker().verifyEquals("stringColumn column data not as expected", List.of("three", "two", "one"),
importedDataTable.getColumnDataAsText(stringColumn.getName()));
}

private FieldDefinition getFieldByNamePart(List<FieldDefinition> fields, String namePart)
{
for (FieldDefinition field : fields)
{
if (field.isNamePartMatch(namePart))
return field;
}
return null;
}

private StringBuilder checkDisplayFields(String displayField, List<String> columnLabels)
{
Expand Down
13 changes: 7 additions & 6 deletions src/org/labkey/test/util/exp/SampleTypeAPIHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.labkey.remoteapi.query.Sort;
import org.labkey.test.WebTestHelper;
import org.labkey.test.params.FieldDefinition;
import org.labkey.test.params.FieldInfo;
import org.labkey.test.params.experiment.SampleTypeDefinition;
import org.labkey.test.util.DomainUtils;
import org.labkey.test.util.TestDataGenerator;
Expand Down Expand Up @@ -58,13 +59,13 @@ public static TestDataGenerator createEmptySampleType(String containerPath, Samp
public static List<FieldDefinition> sampleTypeTestFields(boolean withFileField)
{
List<FieldDefinition> fields = new ArrayList<>(Arrays.asList(
new FieldDefinition("intColumn", FieldDefinition.ColumnType.Integer),
new FieldDefinition("decimalColumn", FieldDefinition.ColumnType.Decimal),
new FieldDefinition("stringColumn", FieldDefinition.ColumnType.String),
new FieldDefinition("sampleDate", FieldDefinition.ColumnType.DateAndTime),
new FieldDefinition("boolColumn", FieldDefinition.ColumnType.Boolean)));
FieldInfo.random("int,./Column", FieldDefinition.ColumnType.Integer).getFieldDefinition(), // Issue 53431: include special chars that are fieldKey encoded
FieldInfo.random("decimalColumn", FieldDefinition.ColumnType.Decimal).getFieldDefinition(),
FieldInfo.random("stringColumn", FieldDefinition.ColumnType.String).getFieldDefinition(),
FieldInfo.random("sampleDate", FieldDefinition.ColumnType.DateAndTime).getFieldDefinition(),
FieldInfo.random("boolColumn", FieldDefinition.ColumnType.Boolean).getFieldDefinition()));
if (withFileField)
fields.add(new FieldDefinition("fileColumn", FieldDefinition.ColumnType.File));
fields.add(FieldInfo.random("file,./Column", FieldDefinition.ColumnType.File).getFieldDefinition());
return fields;
}

Expand Down