Skip to content

Commit 36310b1

Browse files
labkey-tchadlabkey-tchad
committed
Improve FieldInfo and use random names for nameexpressiontest
1 parent ee0a4c4 commit 36310b1

6 files changed

Lines changed: 87 additions & 52 deletions

File tree

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.jetbrains.annotations.Contract;
44
import org.jetbrains.annotations.NotNull;
55
import org.labkey.test.params.FieldDefinition.ColumnType;
6+
import org.labkey.test.util.EscapeUtil;
67
import org.labkey.test.util.TestDataGenerator;
78

89
import java.util.Objects;
@@ -106,6 +107,15 @@ public String getName()
106107
return _fieldKey.getName();
107108
}
108109

110+
/**
111+
* Get name escaped for use in sample or source name expressions
112+
*/
113+
@Contract(pure = true)
114+
public String getExpName()
115+
{
116+
return EscapeUtil.escapeForNameExpression(getName());
117+
}
118+
109119
@Contract(pure = true)
110120
public FieldKey child(String name)
111121
{

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

Lines changed: 63 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import org.labkey.test.pages.experiment.CreateSampleTypePage;
3333
import org.labkey.test.pages.experiment.UpdateSampleTypePage;
3434
import org.labkey.test.params.FieldDefinition;
35+
import org.labkey.test.params.FieldDefinition.ColumnType;
36+
import org.labkey.test.params.FieldInfo;
3537
import org.labkey.test.params.experiment.SampleTypeDefinition;
3638
import org.labkey.test.util.AuditLogHelper;
3739
import org.labkey.test.util.DataRegionTable;
@@ -40,6 +42,7 @@
4042
import org.labkey.test.util.SampleTypeHelper;
4143
import org.labkey.test.util.TestDataGenerator;
4244
import org.labkey.test.util.TextUtils;
45+
import org.labkey.test.util.data.TestDataUtils;
4346
import org.labkey.test.util.exp.SampleTypeAPIHelper;
4447
import org.openqa.selenium.TimeoutException;
4548
import org.openqa.selenium.WebElement;
@@ -61,7 +64,7 @@
6164
import static org.junit.Assert.assertEquals;
6265
import static org.junit.Assert.assertTrue;
6366
import static org.labkey.test.params.FieldDefinition.DOMAIN_TRICKY_CHARACTERS;
64-
import static org.labkey.test.util.data.TestDataUtils.getEscapedNameExpression;
67+
import static org.labkey.test.util.EscapeUtil.escapeForNameExpression;
6568

6669
@Category({Daily.class})
6770
@BaseWebDriverTest.ClassTimeout(minutes = 5)
@@ -71,7 +74,16 @@ public class SampleTypeNameExpressionTest extends BaseWebDriverTest
7174
private static final String DEFAULT_SAMPLE_PARENT_VALUE = "SS" + TestDataGenerator.randomString(3).replaceAll("[_)]", "."); // '_' is used as delimiter to get batchRandomId and ) is used to close the defaultValue()
7275

7376
private static final String PARENT_SAMPLE_TYPE = "PS" + DOMAIN_TRICKY_CHARACTERS;
74-
private static final String PARENT_SAMPLE_TYPE_INPUT = "PS" + getEscapedNameExpression(DOMAIN_TRICKY_CHARACTERS);
77+
private static final String PARENT_SAMPLE_TYPE_INPUT = escapeForNameExpression(PARENT_SAMPLE_TYPE);
78+
79+
private static final FieldInfo COL_STR = FieldInfo.random("{Str", ColumnType.String);
80+
private static final FieldInfo COL_INT = FieldInfo.random("}Int", ColumnType.Integer);
81+
private static final FieldInfo COL_DATE = FieldInfo.random("$Date", ColumnType.DateAndTime);
82+
83+
// No random names for deriving sample type
84+
// Issue 53306: LabKey Server: Derive samples form does not distinguish between fields that differ by 'special characters'
85+
private static final FieldInfo COL_DSTR = new FieldInfo("DStr", ColumnType.String);
86+
private static final FieldInfo COL_DINT = new FieldInfo("DInt", ColumnType.Integer);
7587

7688
private static final String PARENT_SAMPLE_01 = "parent01";
7789
private static final String PARENT_SAMPLE_02 = "parent02";
@@ -108,10 +120,10 @@ public static void setupProject() throws IOException, CommandException
108120
private void addDataRow(TestDataGenerator dataGenerator, String name, int intVal)
109121
{
110122
Map<String, Object> sampleData = Map.of(
111-
"name", name,
112-
"Int", intVal,
113-
"Str", "Parent Sample " + ((char) (intVal + 95)),
114-
"Date", intVal + "/14/2020");
123+
"name", name,
124+
COL_INT.getName(), intVal,
125+
COL_STR.getName(), "Parent Sample " + ((char) (intVal + 95)),
126+
COL_DATE.getName(), intVal + "/14/2020");
115127
dataGenerator.addCustomRow(sampleData);
116128
}
117129

@@ -126,9 +138,9 @@ private void doSetup() throws IOException, CommandException
126138

127139
SampleTypeDefinition definition = new SampleTypeDefinition(PARENT_SAMPLE_TYPE);
128140
definition = definition.setFields(List.of(
129-
new FieldDefinition("Str", FieldDefinition.ColumnType.String),
130-
new FieldDefinition("Int", FieldDefinition.ColumnType.Integer),
131-
new FieldDefinition("Date", FieldDefinition.ColumnType.DateAndTime)));
141+
COL_STR.getFieldDefinition(),
142+
COL_INT.getFieldDefinition(),
143+
COL_DATE.getFieldDefinition()));
132144

133145
TestDataGenerator dataGenerator = SampleTypeAPIHelper.createEmptySampleType(getCurrentContainerPath(), definition);
134146

@@ -159,20 +171,23 @@ public void preTest()
159171
@Test
160172
public void testSimpleNameExpression()
161173
{
162-
String nameExpression = "${A}-${B}.${genId}.${batchRandomId}.${container}.${randomId}";
163-
String data = """
164-
A\tB\tC
165-
a\tb\tc
166-
a\tb\tc
167-
a\tb\tc
168-
""";
174+
FieldInfo colA = FieldInfo.random("A");
175+
FieldInfo colB = FieldInfo.random("B");
176+
FieldInfo colC = FieldInfo.random("C");
177+
String nameExpression = "${%s}-${%s}.${genId}.${batchRandomId}.${container}.${randomId}"
178+
.formatted(escapeForNameExpression(colA.getName()), escapeForNameExpression(colB.getName()));
179+
String data = TestDataUtils.stringFromRows(List.of(
180+
List.of(colA.getName(), colB.getName(), colC.getName()),
181+
List.of("a", "b", "c"),
182+
List.of("a", "b", "c"),
183+
List.of("a", "b", "c")));
169184

170185
SampleTypeHelper sampleHelper = new SampleTypeHelper(this);
171186
sampleHelper.createSampleType(new SampleTypeDefinition("SimpleNameExprTest")
172187
.setNameExpression(nameExpression)
173-
.setFields(List.of(new FieldDefinition("A", FieldDefinition.ColumnType.String),
174-
new FieldDefinition("B", FieldDefinition.ColumnType.String),
175-
new FieldDefinition("C", FieldDefinition.ColumnType.String))),
188+
.setFields(List.of(colA.getFieldDefinition(),
189+
colB.getFieldDefinition(),
190+
colC.getFieldDefinition())),
176191
data
177192
);
178193

@@ -230,10 +245,10 @@ public void testDeriveFromCommentLikeParents()
230245
expectedNames.add("[" + PARENT_SAMPLE_01 + ", " + PARENT_SAMPLE_02 + ", " + PARENT_SAMPLE_03 + "]-child");
231246
assertEquals("Sample names are not as expected", expectedNames, names);
232247

233-
log("Verify import tsv should successfully create derivatives from parent starting with #, as long as this is not the 1st field in the row");
234-
data = "Description\tMaterialInputs/" + EscapeUtil.fieldKeyEncodePart(PARENT_SAMPLE_TYPE) + "\n"; // fully encoded
235-
data += "Parent with leading # should work\t" + PARENT_SAMPLE_03 + "\n";
236-
data += "Parents with leading # should work\t" + PARENT_SAMPLE_03 + "," + PARENT_SAMPLE_02 + "\n";
248+
log("Verify import tsv should successfully create derivatives from parent starting with #, as long as values are quoted");
249+
data = "MaterialInputs/" + EscapeUtil.fieldKeyEncodePart(PARENT_SAMPLE_TYPE) + "\n"; // fully encoded
250+
data += "\"" + PARENT_SAMPLE_03 + "\"\n";
251+
data += "\"" + PARENT_SAMPLE_03 + "," + PARENT_SAMPLE_02 + "\"\n";
237252

238253
sampleHelper.bulkImport(data);
239254

@@ -476,7 +491,7 @@ public void testLookupNameExpression() throws Exception
476491
// now create a sampleType with a Color column that looks up to Colors
477492
var sampleTypeDef = new SampleTypeDefinition(nameExpSamples)
478493
.setFields(List.of(new FieldDefinition("ColorLookup", colorsLookup),
479-
new FieldDefinition("Noun", FieldDefinition.ColumnType.String)))
494+
new FieldDefinition("Noun", ColumnType.String)))
480495
.setNameExpression("TEST-${ColorLookup/ColorCode}"); // hopefully this will resolve the 'ColorCode' column from the list
481496
SampleTypeAPIHelper.createEmptySampleType(getProjectName(), sampleTypeDef);
482497

@@ -542,7 +557,7 @@ private void verifyNames(String sampleTypeName, String header, String nameExpres
542557
.setNameExpression(nameExpression);
543558
if (currentTypeAlias != null)
544559
definition = definition.setParentAliases(Map.of(currentTypeAlias, "(Current Sample Type)"));
545-
definition = definition.setFields(List.of(new FieldDefinition("FieldB", FieldDefinition.ColumnType.String)));
560+
definition = definition.setFields(List.of(new FieldDefinition("FieldB", ColumnType.String)));
546561
sampleHelper.createSampleType(definition, data);
547562

548563
assertTextPresent(nameExpression);
@@ -587,7 +602,7 @@ public void testDeriveSampleFromSampleDetailsPage() throws Exception
587602
SampleTypeHelper sampleHelper = new SampleTypeHelper(this);
588603

589604
final String sampleType = "DerivedUI_SampleType";
590-
final String nameExpression = String.format("DUI_${genId}_${materialInputs/%s/Str}", PARENT_SAMPLE_TYPE_INPUT);
605+
final String nameExpression = String.format("DUI_${genId}_${materialInputs/%s/%s}", PARENT_SAMPLE_TYPE_INPUT, COL_STR.getExpName());
591606

592607
// TODO: When Issue 44760 this test can be updated to use a parent alias in the name expression.
593608

@@ -600,8 +615,8 @@ public void testDeriveSampleFromSampleDetailsPage() throws Exception
600615
createPage.setNameExpression(nameExpression);
601616

602617
createPage.addFields(Arrays.asList(
603-
new FieldDefinition("Int", FieldDefinition.ColumnType.Integer),
604-
new FieldDefinition("Str", FieldDefinition.ColumnType.String)));
618+
COL_DINT.getFieldDefinition(),
619+
COL_DSTR.getFieldDefinition()));
605620

606621
createPage.clickSave();
607622

@@ -614,7 +629,7 @@ public void testDeriveSampleFromSampleDetailsPage() throws Exception
614629
checker().verifyTrue(String.format("Doesn't look like there is a link to the parent sample '%s'.", PARENT_SAMPLE_01),
615630
isElementPresent(Locator.linkWithText(PARENT_SAMPLE_01)));
616631

617-
final String ancestorNameExpression = String.format("GrandChild_${MaterialInputs/%s/..[MaterialInputs/%s]/Str}_${genId}", sampleType, PARENT_SAMPLE_TYPE_INPUT);
632+
final String ancestorNameExpression = String.format("GrandChild_${MaterialInputs/%s/..[MaterialInputs/%s]/%s}_${genId}", sampleType, PARENT_SAMPLE_TYPE_INPUT, COL_STR.getExpName());
618633
log("Change the sample type name expression to support grandparent property lookup: " + ancestorNameExpression);
619634
goToProjectHome();
620635
SampleTypeHelper sampleTypeHelper = new SampleTypeHelper(this);
@@ -634,8 +649,9 @@ public void testDeriveSampleFromSampleDetailsPage() throws Exception
634649

635650
String flagStringBulkImport = "bulk imported grand child.";
636651
log("Derive a sample using bulk import but give it no name. The name expression should be used to name the derived sample.");
637-
String importData = "MaterialInputs/DerivedUI_SampleType\tStr\n" +
638-
derivedSampleName + "\t" + flagStringBulkImport + "\n";
652+
String importData = TestDataUtils.stringFromRows(List.of(
653+
List.of("MaterialInputs/%s".formatted(sampleType), COL_DSTR.getName()),
654+
List.of(derivedSampleName, flagStringBulkImport)));
639655
sampleHelper.goToSampleType(sampleType);
640656
sampleHelper.getSamplesDataRegionTable()
641657
.clickImportBulkData()
@@ -645,7 +661,7 @@ public void testDeriveSampleFromSampleDetailsPage() throws Exception
645661
waitForElement(Locator.tagWithText("td", flagStringBulkImport));
646662

647663
DataRegionTable table = sampleHelper.getSamplesDataRegionTable();
648-
int newSampleRowInd = table.getRowIndex("Str", flagStringBulkImport);
664+
int newSampleRowInd = table.getRowIndex(COL_DSTR.getName(), flagStringBulkImport);
649665
String grandImportChildSampleName = table.getDataAsText(newSampleRowInd, "Name");
650666
click(Locator.tagWithText("td", grandImportChildSampleName));
651667
waitForElement(Locator.tagWithText("td", flagStringBulkImport));
@@ -673,8 +689,8 @@ private String deriveSample(String parentSampleName, String parentSampleType, St
673689
selectOptionByText(Locator.name("targetSampleTypeId"), String.format("%s in /%s", targetSampleType, getProjectName()));
674690
clickButton("Next");
675691

676-
setFormElement(Locator.name("outputSample1_Int"), intVal);
677-
setFormElement(Locator.name("outputSample1_Str"), strVal);
692+
setFormElement(Locator.name("outputSample1_%s".formatted(COL_DINT.getName())), intVal);
693+
setFormElement(Locator.name("outputSample1_%s".formatted(COL_DSTR.getName())), strVal);
678694
clickButton("Submit");
679695

680696
waitForElement(Locator.tagWithText("td", strVal));
@@ -755,18 +771,27 @@ public void testNameExpressionPreview() throws IOException, CommandException
755771
createPage.addParentAlias(parentAlias, String.format("Sample Type: %1$s (%2$s)", PARENT_SAMPLE_TYPE, PROJECT_NAME));
756772

757773
log("Use a name expression using a field from the named parent, with parent type not encoded.");
758-
String nameExpressionBad = String.format("SNP_${genId}_${%1$s/Int}_${materialInputs/%2$s/Str}", parentAlias, PARENT_SAMPLE_TYPE);
774+
String nameExpressionBad = String.format("SNP_${genId}_${%s/%s}_${materialInputs/%s/%s}", parentAlias, COL_INT.getExpName(), PARENT_SAMPLE_TYPE, COL_STR.getExpName());
759775
createPage.setNameExpression(nameExpressionBad);
760776
actualMsg = createPage.getNameExpressionPreview();
761777
checker().withScreenshot("Parent_Fields_Preview_Error")
762778
.verifyTrue("Tool-tip message does not contain expected example.", actualMsg.contains("Unable to generate example name from the current pattern. Check for syntax errors."));
763779
// Make the tooltip go away.
764780
mouseOver(createPage.getComponentElement());
765781

782+
log("Use a name expression using a field from the named parent, with parent type not encoded.");
783+
nameExpressionBad = String.format("SNP_${genId}_${%s/$s}_${materialInputs/%s/%s}", parentAlias, COL_INT.getExpName(), PARENT_SAMPLE_TYPE, COL_STR.getName());
784+
createPage.setNameExpression(nameExpressionBad);
785+
actualMsg = createPage.getNameExpressionPreview();
786+
checker().withScreenshot("Parent_Fields_Preview_Error")
787+
.verifyTrue("Tool-tip message does not contain expected example.", actualMsg.contains("Unable to generate example name from the current pattern. Check for syntax errors."));
788+
// Make the tooltip go away.
789+
mouseOver(createPage.getComponentElement());
790+
766791
log("Use a name expression using a field from the named parent, with parent type encoded correctly.");
767-
String nameExpression = String.format("SNP_${genId}_${%1$s/Int}_${materialInputs/%2$s/Str}", parentAlias, PARENT_SAMPLE_TYPE_INPUT);
792+
String nameExpression = String.format("SNP_${genId}_${%s/%s}_${materialInputs/%s/%s}", parentAlias, COL_INT.getExpName(), PARENT_SAMPLE_TYPE_INPUT, COL_STR.getExpName());
768793
createPage.setNameExpression(nameExpression);
769-
expectedMsg = generateExpectedToolTip("SNP_1001_3_parentStrValue");
794+
expectedMsg = generateExpectedToolTip("SNP_1001_3_parent%sValue".formatted(COL_STR.getName()));
770795
actualMsg = createPage.getNameExpressionPreview();
771796
log("Verify that the preview shows the fields as expected.");
772797
checker().withScreenshot("Parent_Fields_Preview_Error")
@@ -777,7 +802,7 @@ public void testNameExpressionPreview() throws IOException, CommandException
777802

778803
log("Use a name expression with a formatted date.");
779804

780-
nameExpression = String.format("SNP_${genId}_${%s/Date:date('yyyy-MM-dd')}", parentAlias);
805+
nameExpression = String.format("SNP_${genId}_${%s/%s:date('yyyy-MM-dd')}", parentAlias, COL_DATE.getExpName());
781806

782807
createPage.setNameExpression(nameExpression);
783808

‎src/org/labkey/test/tests/list/ListLookupTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.labkey.test.tests.list;
22

3-
import org.apache.commons.csv.CSVFormat;
43
import org.jetbrains.annotations.Nullable;
54
import org.junit.BeforeClass;
65
import org.junit.Test;
@@ -286,7 +285,7 @@ private void validateListValues(List<Map<String, String>> expectedValue)
286285
private String tsvFromColumn(List<String> column)
287286
{
288287
List<List<String>> rows = column.stream().map(Collections::singletonList).toList();
289-
return TestDataUtils.stringFromRows(rows, CSVFormat.TDF);
288+
return TestDataUtils.stringFromRows(rows);
290289
}
291290

292291
@Override

‎src/org/labkey/test/util/EscapeUtil.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,9 @@
2323
import java.net.URLEncoder;
2424
import java.nio.charset.StandardCharsets;
2525
import java.util.List;
26+
import java.util.regex.Pattern;
2627
import java.util.stream.Collectors;
2728

28-
/**
29-
* UNDONE: Refactor useful methods from PageFlowUtil into util.jar that can be used by the test harness then delete this class.
30-
*/
3129
public class EscapeUtil
3230
{
3331
static public String jsString(String s)
@@ -146,4 +144,10 @@ public static String getMarkupEscapedValue(String value)
146144
{
147145
return StringEscapeUtils.escapeXml11(value);
148146
}
147+
148+
private static final Pattern nameExpressionNeedsEscaping = Pattern.compile("([\\\\$/&}~,.])");
149+
public static String escapeForNameExpression(String name)
150+
{
151+
return nameExpressionNeedsEscaping.matcher(name).replaceAll("\\\\$1");
152+
}
149153
}

‎src/org/labkey/test/util/TestDataGenerator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class TestDataGenerator
7474
private static final char WIDE_PLACEHOLDER = '\u03A0'; // 'Π' - Wide character can't be picked from the string with 'charAt'
7575
private static final String NON_LATIN_STRING = "\u0438\uC548\u306F"; // "и안は"
7676
// chose a Character random from this String
77-
public static final String CHARSET_STRING = "ABCDEFG01234abcdefvxyz~!@#$%^&*()-+=_{}[]|:;\"',.<>" + NON_LATIN_STRING + WIDE_PLACEHOLDER;
77+
public static final String CHARSET_STRING = "ABCDEFG01234abcdefvxyz~!@#$%^&*()-+=_{}[]|:;\"\\',.<>" + NON_LATIN_STRING + WIDE_PLACEHOLDER;
7878
public static final String ALPHANUMERIC_STRING = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvxyz";
7979
public static final String DOMAIN_SPECIAL_STRING = "+- _.:&()/";
8080
public static final String ILLEGAL_DOMAIN_NAME_CHARSET = "<>[]{};,`\"~!@#$%^*=|?\\";

‎src/org/labkey/test/util/data/TestDataUtils.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,11 @@ public static <T> String stringFromRows(List<List<T>> rows, CSVFormat format)
443443
return stringWriter.toString();
444444
}
445445

446+
public static <T> String stringFromRows(List<List<T>> rows)
447+
{
448+
return stringFromRows(rows, CSVFormat.TDF);
449+
}
450+
446451
/**
447452
* Used to quote values to be written to a TSV file
448453
* @see org.labkey.api.data.TSVWriter
@@ -507,12 +512,4 @@ protected boolean shouldQuote(String value)
507512
return StringUtils.containsAny(value, _escapedChars);
508513
}
509514
}
510-
511-
private static final String[] DECODED = {"\\", "$", "/", "&", "}", "~", ",", "."};
512-
private static final String[] ENCODED = {"\\\\", "\\$", "\\/", "\\&", "\\}", "\\~", "\\,", "\\."};
513-
public static String getEscapedNameExpression(String encoded)
514-
{
515-
return StringUtils.replaceEach(encoded, DECODED, ENCODED);
516-
}
517-
518515
}

0 commit comments

Comments
 (0)