Refactor DataRegionTable to handle tricky characters better (#2513)

- Update `DataRegionTable` to use `FieldReferenceManager.java` to organize and identify columns
- Clean up numerous IntelliJ warnings
- Remove usages of some deprecated `DataRegionTable` methods

Author: labkey-tchad

src/org/labkey/test/components/ui/grids/FieldReferenceManager.java

@@ -1,36 +1,45 @@
 package org.labkey.test.components.ui.grids;
 
-import org.apache.commons.lang3.mutable.Mutable;
-import org.apache.commons.lang3.mutable.MutableObject;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.labkey.test.params.FieldKey;
 import org.labkey.test.params.WrapsFieldKey;
+import org.labkey.test.util.CachingSupplier;
 import org.labkey.test.util.selenium.WebElementUtils;
 import org.openqa.selenium.NoSuchElementException;
 import org.openqa.selenium.WebElement;
 
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 public class FieldReferenceManager
 {
     private final List<FieldReference> _fieldReferences;
-    private final Map<FieldKey, FieldReference> fieldKeys = new LinkedHashMap<>();
-    private final Map<String, FieldReference> fieldLabels = new LinkedHashMap<>();
+    private final Map<Integer, FieldReference> _fieldsByIndex;
+    private final Map<FieldKey, FieldReference> _fieldKeys = new LinkedHashMap<>();
+    private final Map<String, FieldReference> _fieldLabels = new LinkedHashMap<>();
 
     public <T extends FieldReference> FieldReferenceManager(List<T> columnHeaders)
     {
-        this._fieldReferences = Collections.unmodifiableList(columnHeaders);
+        _fieldReferences = List.copyOf(columnHeaders);
+        _fieldsByIndex = columnHeaders.stream().collect(Collectors.toMap(FieldReference::getDomIndex, Function.identity()));
     }
 
     public List<FieldReference> getColumnHeaders()
     {
         return _fieldReferences;
     }
 
+    public FieldReference getColumnHeader(int index)
+    {
+        return _fieldsByIndex.get(index);
+    }
+
     /**
      * Find field by uncertain field identifier in order of precedence:
      * <ol>
@@ -40,7 +49,7 @@ public List<FieldReference> getColumnHeaders()
      *     <li>Field Label</li>
      * </ol>
      */
-    public final FieldReference findFieldReference(CharSequence fieldIdentifier)
+    public final @NotNull FieldReference findFieldReference(CharSequence fieldIdentifier)
     {
         List<Supplier<FieldReference>> options;
 
@@ -64,20 +73,32 @@ public final FieldReference findFieldReference(CharSequence fieldIdentifier)
             .orElseThrow(() -> new NoSuchElementException("Unable to locate field: " + fieldIdentifier));
     }
 
+    public final @Nullable FieldReference findFieldReferenceOrNull(CharSequence fieldIdentifier)
+    {
+        try
+        {
+            return findFieldReference(fieldIdentifier);
+        }
+        catch (NoSuchElementException e)
+        {
+            return null;
+        }
+    }
+
     private FieldReference findColumnHeaderByFieldKey(FieldKey fieldIdentifier)
     {
-        if (fieldKeys.containsKey(fieldIdentifier))
+        if (_fieldKeys.containsKey(fieldIdentifier))
         {
-            return fieldKeys.get(fieldIdentifier);
+            return _fieldKeys.get(fieldIdentifier);
         }
-        else if (fieldKeys.size() < _fieldReferences.size())
+        else if (_fieldKeys.size() < _fieldReferences.size())
         {
             for (FieldReference header : _fieldReferences)
             {
-                if (!fieldKeys.containsValue(header))
+                if (!_fieldKeys.containsValue(header))
                 {
                     FieldKey fieldKey = header.getFieldKey();
-                    fieldKeys.put(fieldKey, header);
+                    _fieldKeys.put(fieldKey, header);
                     if (fieldKey.equals(fieldIdentifier))
                     {
                         return header;
@@ -91,18 +112,18 @@ else if (fieldKeys.size() < _fieldReferences.size())
 
     private FieldReference findColumnHeaderByLabel(String label)
     {
-        if (fieldLabels.containsKey(label))
+        if (_fieldLabels.containsKey(label))
         {
-            return fieldLabels.get(label);
+            return _fieldLabels.get(label);
         }
-        else if (fieldLabels.size() < _fieldReferences.size())
+        else if (_fieldLabels.size() < _fieldReferences.size())
         {
             for (FieldReference header : _fieldReferences)
             {
-                if (!fieldLabels.containsValue(header))
+                if (!_fieldLabels.containsValue(header))
                 {
                     String columnLabel = header.getLabel();
-                    fieldLabels.put(columnLabel, header);
+                    _fieldLabels.put(columnLabel, header);
                     if (columnLabel.equals(label))
                     {
                         return header;
@@ -118,8 +139,8 @@ public static class FieldReference
     {
         private final WebElement _element;
         private final int _domIndex;
-        private final Mutable<String> _fieldLabel = new MutableObject<>();
-        private final Mutable<FieldKey> _fieldKey = new MutableObject<>();
+        private final CachingSupplier<String> _fieldLabel = new CachingSupplier<>(this::labelSupplier);
+        private final CachingSupplier<FieldKey> _fieldKey = new CachingSupplier<>(this::fieldKeySupplier);
 
         public FieldReference(WebElement element, int domIndex)
         {
@@ -134,34 +155,12 @@ public WebElement getElement()
 
         public FieldKey getFieldKey()
         {
-            if (_fieldKey.getValue() == null)
-            {
-                String path = _element.getDomAttribute("data-fieldkey");
-                if (path == null)
-                {
-                    // Some grids don't have a field key, but have a similar value in the ID attribute
-                    path = _element.getDomAttribute("id");
-                }
-
-                if (path != null)
-                {
-                    _fieldKey.setValue(FieldKey.fromFieldKey(path));
-                }
-                else
-                {
-                    _fieldKey.setValue(FieldKey.EMPTY);
-                }
-            }
-            return _fieldKey.getValue();
+            return _fieldKey.get();
         }
 
         public String getLabel()
         {
-            if (_fieldLabel.getValue() == null)
-            {
-                _fieldLabel.setValue(WebElementUtils.getTextContent(getElement()).trim());
-            }
-            return _fieldLabel.getValue();
+            return _fieldLabel.get();
         }
 
         public String getName()
@@ -173,5 +172,29 @@ public int getDomIndex()
         {
             return _domIndex;
         }
+
+        protected String labelSupplier()
+        {
+            return WebElementUtils.getTextContent(getElement()).trim();
+        }
+
+        protected FieldKey fieldKeySupplier()
+        {
+            String path = getElement().getDomAttribute("data-fieldkey");
+            if (path == null)
+            {
+                // Some grids don't have a field key, but have a similar value in the ID attribute
+                path = getElement().getDomAttribute("id");
+            }
+
+            if (path != null)
+            {
+                return FieldKey.fromFieldKey(path);
+            }
+            else
+            {
+                return FieldKey.EMPTY;
+            }
+        }
     }
 }

src/org/labkey/test/pages/query/UpdateQueryRowPage.java

@@ -59,21 +59,21 @@ public void setFields(Map<String, ?> fields)
         for (Map.Entry<String, ?> entry : fields.entrySet())
         {
             Object value = entry.getValue();
-            if (value instanceof String)
+            if (value instanceof String s)
             {
-                setField(entry.getKey(), (String) value);
+                setField(entry.getKey(), s);
             }
-            else if (value instanceof Boolean)
+            else if (value instanceof Boolean b)
             {
-                setField(entry.getKey(), (Boolean) value);
+                setField(entry.getKey(), b);
             }
-            else if (value instanceof Integer)
+            else if (value instanceof Integer i)
             {
-                setField(entry.getKey(), (Integer) value);
+                setField(entry.getKey(), i);
             }
-            else if (value instanceof File)
+            else if (value instanceof File f)
             {
-                setField(entry.getKey(), (File) value);
+                setField(entry.getKey(), f);
             }
             else
             {

src/org/labkey/test/pages/study/OverviewPage.java

@@ -16,12 +16,11 @@
 package org.labkey.test.pages.study;
 
 import org.jetbrains.annotations.Nullable;
-import org.labkey.test.CachingLocator;
 import org.labkey.test.Locator;
 import org.labkey.test.WebDriverWrapper;
 import org.labkey.test.WebTestHelper;
 import org.labkey.test.pages.LabKeyPage;
-import org.labkey.test.selenium.LazyWebElement;
+import org.labkey.test.util.CachingSupplier;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 
@@ -133,7 +132,7 @@ public Map<String, List<CountPair>> getDatasetRowData()
     public Map<String, List<CountPair>> getDatasetRowData(@Nullable Integer leftVisitIndex, @Nullable Integer endVisitIndex)
     {
         Map<String, List<CountPair>> datasetRowData = new HashMap<>();
-        List<WebElement> overviewRows = elementCache().getStudyOverviewRows();
+        List<WebElement> overviewRows = elementCache().studyOverviewRows.get();
         overviewRows = overviewRows.subList(1, overviewRows.size());
 
         for (WebElement row : overviewRows)
@@ -178,7 +177,7 @@ else if (!countStr.isEmpty())
 
     public List<String> getVisits()
     {
-        WebElement headerRow = elementCache().getStudyOverviewRows().get(0);
+        WebElement headerRow = elementCache().studyOverviewRows.get().get(0);
         List<WebElement> visitCells = Locator.css("td").findElements(headerRow);
         visitCells = visitCells.subList(1, visitCells.size() - 1);
 
@@ -193,16 +192,13 @@ protected Elements newElementCache()
 
     protected class Elements extends LabKeyPage<?>.ElementCache
     {
-        public final WebElement participantCountCheckbox = new LazyWebElement(Locator.checkboxByNameAndValue("visitStatistic", "ParticipantCount"), this);
-        public final WebElement rowCountCheckbox = new LazyWebElement(Locator.checkboxByNameAndValue("visitStatistic", "RowCount"), this);
-        public final WebElement studyOverview = new LazyWebElement(Locator.tagWithId("table", "studyOverview"), this);
+        public final WebElement participantCountCheckbox = Locator.checkboxByNameAndValue("visitStatistic", "ParticipantCount").findWhenNeeded(this);
+        public final WebElement rowCountCheckbox = Locator.checkboxByNameAndValue("visitStatistic", "RowCount").findWhenNeeded(this);
+        public final WebElement studyOverview = Locator.tagWithId("table", "studyOverview").findWhenNeeded(this);
 
-        private final Locator studyOverviewRow = new CachingLocator(Locator.CssLocator.union(Locator.css(".labkey-row"), Locator.css(".labkey-alternate-row")));
+        private final Locator studyOverviewRow = Locator.CssLocator.union(Locator.css(".labkey-row"), Locator.css(".labkey-alternate-row"));
 
-        public List<WebElement> getStudyOverviewRows()
-        {
-            return studyOverviewRow.findElements(studyOverview);
-        }
+        public CachingSupplier<List<WebElement>> studyOverviewRows = new CachingSupplier<>(() -> studyOverviewRow.findElements(studyOverview));
     }
 
     public static class CountPair

src/org/labkey/test/params/FieldKey.java

@@ -2,7 +2,6 @@
 
 import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -66,22 +65,15 @@ public static FieldKey fromParts(String... parts)
      * @param fieldKey String or FieldKey
      * @return FieldKey representation of the String, or the identity if a FieldKey was provided
      */
-    public static @Nullable FieldKey fromFieldKey(CharSequence fieldKey)
+    public static FieldKey fromFieldKey(CharSequence fieldKey)
     {
         if (fieldKey instanceof WrapsFieldKey fk)
         {
             return fk.getFieldKey();
         }
         else
         {
-            try
-            {
-                return fromParts(Arrays.stream(fieldKey.toString().split(SEPARATOR)).map(FieldKey::decodePart).toList());
-            }
-            catch (IllegalArgumentException iae)
-            {
-                return null;
-            }
+            return fromParts(Arrays.stream(fieldKey.toString().split(SEPARATOR)).map(FieldKey::decodePart).toList());
         }
     }
 
@@ -149,6 +141,10 @@ public String getName()
         return _name;
     }
 
+    /**
+     * Inverse of {@link #fromParts(String...)}
+     * @return decoded parts of the field key
+     */
     public String[] getNameArray()
     {
         return Arrays.stream(_fieldKey.split(SEPARATOR)).map(FieldKey::decodePart).toArray(String[]::new);

src/org/labkey/test/tests/AbstractExportTest.java

@@ -35,8 +35,8 @@
 import java.util.HashSet;
 import java.util.List;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -103,7 +103,7 @@ public void preTest()
         exportHelper = new DataRegionExportHelper(dataRegion);
 
         if (hasSelectors())
-            dataRegion.uncheckAll();
+            dataRegion.uncheckAllOnPage();
     }
 
     @Test

src/org/labkey/test/tests/ButtonCustomizationTest.java

@@ -25,11 +25,13 @@
 import org.labkey.test.params.FieldDefinition;
 import org.labkey.test.params.FieldDefinition.ColumnType;
 import org.labkey.test.util.DataRegionTable;
+import org.labkey.test.util.DataRegionTable.DataRegionFinder;
 import org.labkey.test.util.PortalHelper;
 import org.labkey.test.util.WikiHelper;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.Assert.assertTrue;
 
@@ -101,12 +103,10 @@ public void testSteps()
         goToManageLists();
         clickAndWait(Locator.linkWithText(LIST_NAME));
         assertButtonNotPresent(METADATA_OVERRIDE_BUTTON);
-        DataRegionTable.findDataRegion(this).clickInsertNewRow();
-        setFormElement(Locator.name("quf_name"), "Seattle");
-        clickButton("Submit");
-        DataRegionTable.findDataRegion(this).clickInsertNewRow();
-        setFormElement(Locator.name("quf_name"), "Portland");
-        clickButton("Submit");
+        new DataRegionFinder(getDriver()).find().clickInsertNewRow()
+            .update(Map.of("name", "Seattle"));
+        new DataRegionFinder(getDriver()).find().clickInsertNewRow()
+            .update(Map.of("name", "Portland"));
 
         // assert custom buttons can be added to the standard set:
         beginAt("/query/" + PROJECT_NAME + "/schema.view?schemaName=lists");
@@ -188,7 +188,7 @@ public void testSteps()
                 Locator.lkButton(METADATA_LINK_BUTTON).waitForElement(getDriver(), WAIT_FOR_JAVASCRIPT)
                         .getAttribute("class").contains("labkey-disabled-button"));
 
-        buttonRegion.checkCheckbox(buttonRegion.getIndexWhereDataAppears("Portland", "Name"));
+        buttonRegion.checkCheckbox(buttonRegion.getRowIndex("Portland", "Name"));
         // wait for the button to enable:
         waitForElement(Locator.lkButton(METADATA_LINK_BUTTON), 10000);
 

src/org/labkey/test/tests/DataRegionTest.java

@@ -277,7 +277,7 @@ private void dataRegionTest(URL url, String dataRegionName) throws MalformedURLE
         clickAndWait(selectionPart.append(Locator.tagWithClass("span", "show-selected")));
         assertEquals(5, table.getDataRowCount());
 
-        table.showAll();
+        table.rowSelector().showAll();
         assertEquals(15, table.getDataRowCount());
     }