Skip to content

Commit 5f37d08

Browse files
cnathecnathe
authored
Issue 53431: Data Class and Sample Type data doesn't round-trip via folder export/import for field names with special char (#2556)
- SampleTypeFolderExportImportTest fixes to use generated field names from SampleTypeAPIHelper.sampleTypeTestFields() - FieldDefinition and FieldInfo to keep track of namePart when randomFieldName is used
1 parent 1805da6 commit 5f37d08

5 files changed

Lines changed: 84 additions & 26 deletions

File tree

‎src/org/labkey/test/params/FieldDefinition.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public class FieldDefinition extends PropertyDescriptor
5858
// Collection of JSON properties not explicitly known by 'PropertyDescriptor'
5959
private final Map<String, Object> _extraFieldProperties = new HashMap<>();
6060

61+
private String _namePart;
62+
6163
/**
6264
* Define a non-lookup field of the specified type
6365
* @param name field name
@@ -467,6 +469,16 @@ public void setAliquotOption(ExpSchema.DerivationDataScopeType aliquotOption)
467469
_aliquotOption = aliquotOption;
468470
}
469471

472+
public void setNamePart(String namePart)
473+
{
474+
_namePart = namePart;
475+
}
476+
477+
public boolean isNamePartMatch(String namePart)
478+
{
479+
return _namePart != null && _namePart.equals(namePart);
480+
}
481+
470482
public enum RangeType
471483
{
472484
Equals("Equals", Filter.Operator.EQUAL),

‎src/org/labkey/test/params/FieldInfo.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public class FieldInfo implements CharSequence, WrapsFieldKey
2020
private final String _label;
2121
private final ColumnType _columnType;
2222
private final Consumer<FieldDefinition> _fieldDefinitionMutator;
23+
private String _namePart; // used for random field generation to track the name part used
2324

2425
private FieldInfo(FieldKey fieldKey, String label, ColumnType columnType, Consumer<FieldDefinition> fieldDefinitionMutator)
2526
{
@@ -54,7 +55,9 @@ public FieldInfo(String name)
5455
*/
5556
public static FieldInfo random(String namePart, ColumnType columnType)
5657
{
57-
return new FieldInfo(TestDataGenerator.randomFieldName(namePart), columnType);
58+
FieldInfo field = new FieldInfo(TestDataGenerator.randomFieldName(namePart), columnType);
59+
field.setNamePart(namePart);
60+
return field;
5861
}
5962

6063
/**
@@ -165,9 +168,18 @@ private FieldDefinition getFieldDefinition(ColumnType columnType)
165168
{
166169
_fieldDefinitionMutator.accept(fieldDefinition);
167170
}
171+
if (_namePart != null)
172+
{
173+
fieldDefinition.setNamePart(_namePart);
174+
}
168175
return fieldDefinition;
169176
}
170177

178+
private void setNamePart(String namePart)
179+
{
180+
_namePart = namePart;
181+
}
182+
171183
@Override
172184
public int length()
173185
{

‎src/org/labkey/test/tests/FileAttachmentColumnTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ private void importSampleDataUI(String sampleTypeName, String containerPath, Lis
451451
for (File file : files)
452452
{
453453
sampleFileData.add(Map.of("Name", file.getName(), "Color", "green",
454-
LIST_ATTACHMENT_FIELD.getName(), file.getName()));
454+
"file", file.getName()));
455455
}
456456
helper.bulkImport(sampleFileData);
457457
}
@@ -542,7 +542,7 @@ private void validateSampleData(String sampleType, String folderPath, List<File>
542542
}
543543
else
544544
{
545-
WebElement fileLinkCell = samplesRegion.findCell(rowIndex, LIST_ATTACHMENT_FIELD.getName());
545+
WebElement fileLinkCell = samplesRegion.findCell(rowIndex, "file");
546546
Optional<WebElement> optionalFileLink = Locator.tag("a").findOptionalElement(fileLinkCell);
547547
checker().withScreenshot("unexpected_file_state")
548548
.awaiting(Duration.ofSeconds(2),

‎src/org/labkey/test/tests/SampleTypeFolderExportImportTest.java‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.labkey.test.params.experiment.DataClassDefinition;
4545
import org.labkey.test.params.experiment.SampleTypeDefinition;
4646
import org.labkey.test.util.DataRegionTable;
47+
import org.labkey.test.util.EscapeUtil;
4748
import org.labkey.test.util.LogMethod;
4849
import org.labkey.test.util.PortalHelper;
4950
import org.labkey.test.util.SampleTypeHelper;
@@ -435,6 +436,9 @@ public void testExportImportDerivedSamples() throws Exception
435436

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

451455
TestDataGenerator parentDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, parentType);
452-
parentDgen.addCustomRow(Map.of("Name", "Parent1", "intColumn", 1, "floatColumn", 1.1, "stringColumn", "one"));
453-
parentDgen.addCustomRow(Map.of("Name", "Parent2", "intColumn", 2, "floatColumn", 2.2, "stringColumn", "two"));
454-
parentDgen.addCustomRow(Map.of("Name", "Parent3", "intColumn", 3, "floatColumn", 3.3, "stringColumn", "three"));
456+
parentDgen.addCustomRow(Map.of("Name", "Parent1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one"));
457+
parentDgen.addCustomRow(Map.of("Name", "Parent2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two"));
458+
parentDgen.addCustomRow(Map.of("Name", "Parent3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three"));
455459
parentDgen.insertRows();
456460

457461
TestDataGenerator testDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, testSampleType);
458-
testDgen.addCustomRow(Map.of("Name", "Child1", "intColumn", 1, "decimalColumn", 1.1, "stringColumn", "one",
462+
testDgen.addCustomRow(Map.of("Name", "Child1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one",
459463
"Parent", "Parent1"));
460-
testDgen.addCustomRow(Map.of("Name", "Child2", "intColumn", 2, "decimalColumn", 2.2, "stringColumn", "two",
464+
testDgen.addCustomRow(Map.of("Name", "Child2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two",
461465
"Parent", "Parent2"));
462-
testDgen.addCustomRow(Map.of("Name", "Child3", "intColumn", 3, "decimalColumn", 3.3, "stringColumn", "three",
466+
testDgen.addCustomRow(Map.of("Name", "Child3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three",
463467
"Parent", "Parent3", "DataClassParent", "data1"));
464-
testDgen.addCustomRow(Map.of("Name", "Child4", "intColumn", 4, "decimalColumn", 4.4, "stringColumn", "four",
468+
testDgen.addCustomRow(Map.of("Name", "Child4", intColumn.getName(), 4, decimalColumn.getName(), 4.4, stringColumn.getName(), "four",
465469
"Parent", "Parent3, Parent2"));
466-
testDgen.addCustomRow(Map.of("Name", "Child5", "intColumn", 5, "decimalColumn", 5.5, "stringColumn", "five",
470+
testDgen.addCustomRow(Map.of("Name", "Child5", intColumn.getName(), 5, decimalColumn.getName(), 5.5, stringColumn.getName(), "five",
467471
"Parent", "Parent1, Parent2"));
468-
testDgen.addCustomRow(Map.of("Name", "Child6", "intColumn", 6, "decimalColumn", 6.6, "stringColumn", "six",
472+
testDgen.addCustomRow(Map.of("Name", "Child6", intColumn.getName(), 6, decimalColumn.getName(), 6.6, stringColumn.getName(), "six",
469473
"Parent", "Parent3, Parent2", "SelfParent", "Child5"));
470-
testDgen.addCustomRow(Map.of("Name", "Child7", "intColumn", 7, "decimalColumn", 7.7, "stringColumn", "seven",
474+
testDgen.addCustomRow(Map.of("Name", "Child7", intColumn.getName(), 7, decimalColumn.getName(), 7.7, stringColumn.getName(), "seven",
471475
"Parent", "Parent3, Parent2", "SelfParent", "Child5", "DataClassParent", "data2, data3"));
472476
testDgen.insertRows();
473477

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

530+
String intColFieldKey = EscapeUtil.fieldKeyEncodePart(intColumn.getName());
531+
assertNotNull("expect intColumn to be present in exported row", exportedRow.get(intColFieldKey));
526532
assertThat("expect export and import values to be equivalent",
527-
exportedRow.get("intColumn"), equalTo(matchingMap.get("intColumn")));
533+
exportedRow.get(intColFieldKey), equalTo(matchingMap.get(intColFieldKey)));
534+
String stringColFieldKey = EscapeUtil.fieldKeyEncodePart(stringColumn.getName());
535+
assertNotNull("expect stringColumn to be present in exported row", exportedRow.get(stringColFieldKey));
528536
assertThat("expect export and import values to be equivalent",
529-
exportedRow.get("stringColumn"), equalTo(matchingMap.get("stringColumn")));
537+
exportedRow.get(stringColFieldKey), equalTo(matchingMap.get(stringColFieldKey)));
538+
String decimalColFieldKey = EscapeUtil.fieldKeyEncodePart(decimalColumn.getName());
539+
assertNotNull("expect decimalColumn to be present in exported row", exportedRow.get(decimalColFieldKey));
530540
assertThat("expect export and import values to be equivalent",
531-
exportedRow.get("decimalColumn"), equalTo(matchingMap.get("decimalColumn")));
541+
exportedRow.get(decimalColFieldKey), equalTo(matchingMap.get(decimalColFieldKey)));
532542

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

557567
// create a test sampleType
558568
List<FieldDefinition> testFields = SampleTypeAPIHelper.sampleTypeTestFields(true);
569+
FieldDefinition intColumn = getFieldByNamePart(testFields, "int,./Column");
570+
FieldDefinition stringColumn = getFieldByNamePart(testFields, "stringColumn");
571+
FieldDefinition decimalColumn = getFieldByNamePart(testFields, "decimalColumn");
559572
SampleTypeDefinition testSampleType = new SampleTypeDefinition(testSamples).setFields(testFields)
560-
.addParentAlias("SelfParent"); // to derive from samles in the current type
573+
.addParentAlias("SelfParent"); // to derive from samples in the current type
561574

562575
TestDataGenerator parentDgen = SampleTypeAPIHelper.createEmptySampleType(subfolderPath, testSampleType);
563-
parentDgen.addCustomRow(Map.of("Name", "sample1", "intColumn", 1, "decimalColumn", 1.1, "stringColumn", "one"));
564-
parentDgen.addCustomRow(Map.of("Name", "sample2", "intColumn", 2, "decimalColumn", 2.2, "stringColumn", "two"));
565-
parentDgen.addCustomRow(Map.of("Name", "sample3", "intColumn", 3, "decimalColumn", 3.3, "stringColumn", "three"));
576+
parentDgen.addCustomRow(Map.of("Name", "sample1", intColumn.getName(), 1, decimalColumn.getName(), 1.1, stringColumn.getName(), "one"));
577+
parentDgen.addCustomRow(Map.of("Name", "sample2", intColumn.getName(), 2, decimalColumn.getName(), 2.2, stringColumn.getName(), "two"));
578+
parentDgen.addCustomRow(Map.of("Name", "sample3", intColumn.getName(), 3, decimalColumn.getName(), 3.3, stringColumn.getName(), "three"));
566579
parentDgen.insertRows();
567580

568581
goToProjectFolder(getProjectName(), subfolder);
@@ -685,8 +698,28 @@ public void testExportImportSampleTypesWithAssayRuns() throws Exception
685698
File downloadedFile = doAndWaitForDownload(() -> waitAndClick(WAIT_FOR_JAVASCRIPT, Locator.tagWithAttribute("a", "title", "Download attached file"), 0));
686699
assertElementPresent("Did not find the expected number of icons for " + SAMPLE_TXT_FILE.getName() + " from the imported samples.", Locator.tagContainingText("a", "sample.txt"), 1);
687700
checker().verifyTrue("Incorrect file content for sample.txt after folder import", FileUtils.contentEquals(downloadedFile, SAMPLE_TXT_FILE));
701+
702+
// verify the other sample type data is round-tripped as expected
703+
importedDataTable = DataRegionTable.DataRegion(getDriver()).withName("Material").waitFor();
704+
checker().verifyEquals("Name column data not as expected", List.of("sample3", "sample2", "sample1"),
705+
importedDataTable.getColumnDataAsText("Name"));
706+
checker().verifyEquals("intColumn column data not as expected", List.of("3", "2", "1"),
707+
importedDataTable.getColumnDataAsText(intColumn.getName()));
708+
checker().verifyEquals("decimalColumn column data not as expected", List.of("3.3", "2.2", "1.1"),
709+
importedDataTable.getColumnDataAsText(decimalColumn.getName()));
710+
checker().verifyEquals("stringColumn column data not as expected", List.of("three", "two", "one"),
711+
importedDataTable.getColumnDataAsText(stringColumn.getName()));
688712
}
689713

714+
private FieldDefinition getFieldByNamePart(List<FieldDefinition> fields, String namePart)
715+
{
716+
for (FieldDefinition field : fields)
717+
{
718+
if (field.isNamePartMatch(namePart))
719+
return field;
720+
}
721+
return null;
722+
}
690723

691724
private StringBuilder checkDisplayFields(String displayField, List<String> columnLabels)
692725
{

‎src/org/labkey/test/util/exp/SampleTypeAPIHelper.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.labkey.remoteapi.query.Sort;
1010
import org.labkey.test.WebTestHelper;
1111
import org.labkey.test.params.FieldDefinition;
12+
import org.labkey.test.params.FieldInfo;
1213
import org.labkey.test.params.experiment.SampleTypeDefinition;
1314
import org.labkey.test.util.DomainUtils;
1415
import org.labkey.test.util.TestDataGenerator;
@@ -58,13 +59,13 @@ public static TestDataGenerator createEmptySampleType(String containerPath, Samp
5859
public static List<FieldDefinition> sampleTypeTestFields(boolean withFileField)
5960
{
6061
List<FieldDefinition> fields = new ArrayList<>(Arrays.asList(
61-
new FieldDefinition("intColumn", FieldDefinition.ColumnType.Integer),
62-
new FieldDefinition("decimalColumn", FieldDefinition.ColumnType.Decimal),
63-
new FieldDefinition("stringColumn", FieldDefinition.ColumnType.String),
64-
new FieldDefinition("sampleDate", FieldDefinition.ColumnType.DateAndTime),
65-
new FieldDefinition("boolColumn", FieldDefinition.ColumnType.Boolean)));
62+
FieldInfo.random("int,./Column", FieldDefinition.ColumnType.Integer).getFieldDefinition(), // Issue 53431: include special chars that are fieldKey encoded
63+
FieldInfo.random("decimalColumn", FieldDefinition.ColumnType.Decimal).getFieldDefinition(),
64+
FieldInfo.random("stringColumn", FieldDefinition.ColumnType.String).getFieldDefinition(),
65+
FieldInfo.random("sampleDate", FieldDefinition.ColumnType.DateAndTime).getFieldDefinition(),
66+
FieldInfo.random("boolColumn", FieldDefinition.ColumnType.Boolean).getFieldDefinition()));
6667
if (withFileField)
67-
fields.add(new FieldDefinition("fileColumn", FieldDefinition.ColumnType.File));
68+
fields.add(FieldInfo.random("file,./Column", FieldDefinition.ColumnType.File).getFieldDefinition());
6869
return fields;
6970
}
7071

0 commit comments

Comments
 (0)