Bind sliders from a parsed spec

Amolith created

Keeping field classification and binding in the same contract instead of
re-parsing the form field in two places

Change summary

src/main/java/eu/siacs/conversations/entities/Conversation.java     | 39 
src/test/java/eu/siacs/conversations/entities/ConversationTest.java | 80 
2 files changed, 60 insertions(+), 59 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Conversation.java 🔗

@@ -1869,7 +1869,9 @@ public class Conversation extends AbstractEntity
         }
     }
 
-    static Float steppedSliderStep(final Element field) {
+    record SliderSpec(String datatype, float min, float max, float value, float step) {}
+
+    static SliderSpec sliderSpec(final Element field) {
         final Element validate = field.findChild("validate", "http://jabber.org/protocol/xdata-validate");
         final String datatype = validate == null ? null : validate.getAttribute("datatype");
         if (!isNumericDatatype(datatype)) return null;
@@ -1904,7 +1906,9 @@ public class Conversation extends AbstractEntity
             }
         }
         if (step == null || (step > 0f && !landsOnStep(max, min, step))) return null;
-        return step == 0f || landsOnStep(parsedValue, min, step) ? step : null;
+        return step == 0f || landsOnStep(parsedValue, min, step)
+                ? new SliderSpec(datatype, min, max, parsedValue, step)
+                : null;
     }
 
     static String formatSliderValue(final float value, final String datatype) {
@@ -1941,10 +1945,6 @@ public class Conversation extends AbstractEntity
         return values;
     }
 
-    private static Float rangeFloat(final Element range, final String name) {
-        return range == null ? null : parseFloat(range.getAttribute(name));
-    }
-
     private static Float parseFloat(final String value) {
         if (value == null || value.equals("")) return null;
         try {
@@ -3059,34 +3059,23 @@ public class Conversation extends AbstractEntity
                 @Override
                 public void bind(Item item) {
                     final Field boundField = (Field) item;
-                    final Element validate = boundField.el.findChild("validate", "http://jabber.org/protocol/xdata-validate");
-                    final String datatype = validate == null ? null : validate.getAttribute("datatype");
-                    final Element range = validate == null ? null : validate.findChild("range", "http://jabber.org/protocol/xdata-validate");
-                    final Float step = steppedSliderStep(boundField.el);
-                    final Float min = rangeFloat(range, "min");
-                    final Float max = rangeFloat(range, "max");
-                    if (step == null || min == null || max == null) {
+                    final SliderSpec spec = sliderSpec(boundField.el);
+                    if (spec == null) {
                         throw new IllegalStateException("Invalid slider field bound to slider view holder");
                     }
 
-                    float value = min;
-                    final Float parsedValue = parseFloat(firstValue(boundField.el));
-                    if (parsedValue != null && parsedValue >= min && parsedValue <= max) {
-                        value = parsedValue;
-                    }
-
                     field = boundField;
                     binding.slider.clearOnChangeListeners();
                     setTextOrHide(binding.label, field.getLabel());
                     setTextOrHide(binding.desc, field.getDesc());
-                    binding.slider.setValueFrom(min);
-                    binding.slider.setValueTo(max);
-                    binding.slider.setValue(value);
-                    binding.slider.setStepSize(step);
+                    binding.slider.setValueFrom(spec.min());
+                    binding.slider.setValueTo(spec.max());
+                    binding.slider.setValue(spec.value());
+                    binding.slider.setStepSize(spec.step());
 
                     binding.slider.addOnChangeListener((slider, sliderValue, fromUser) -> {
                         if (!fromUser) return;
-                        field.setValues(List.of(formatSliderValue(sliderValue, datatype)));
+                        field.setValues(List.of(formatSliderValue(sliderValue, spec.datatype())));
                     });
                 }
             }
@@ -3278,7 +3267,7 @@ public class Conversation extends AbstractEntity
                             range != null && range.getAttribute("min") != null && range.getAttribute("max") != null && isNumericDatatype(datatype)
                         ) {
                             // has a range and is numeric, use a slider if it can represent every valid value
-                            viewType = steppedSliderStep(el) == null ? TYPE_TEXT_FIELD : TYPE_SLIDER_FIELD;
+                            viewType = sliderSpec(el) == null ? TYPE_TEXT_FIELD : TYPE_SLIDER_FIELD;
                         } else if (fieldType.equals("list-single")) {
                             if (fillableFieldCount == 1 && actionsAdapter.countProceed() < 1 && Option.forField(el).size() < 50) {
                                 viewType = TYPE_BUTTON_GRID_FIELD;

src/test/java/eu/siacs/conversations/entities/ConversationTest.java 🔗

@@ -182,84 +182,82 @@ public class ConversationTest {
     }
 
     @Test
-    public void steppedSliderStepRejectsInRangeValueMissingFromOptionLattice() {
+    public void sliderSpecRejectsInRangeValueMissingFromOptionLattice() {
         final var field = sliderField("xs:integer", "0", "68", "7", "0", "34", "68");
 
-        Assert.assertNull(
-                "A value the option-derived slider cannot represent should fall back to text input",
-                Conversation.steppedSliderStep(field));
+        assertNotSlider("A value the option-derived slider cannot represent should fall back to text input", field);
     }
 
     @Test
-    public void steppedSliderStepAcceptsCompatibleOptionLattice() {
+    public void sliderSpecAcceptsCompatibleOptionLattice() {
         final var field = sliderField("xs:integer", "0", "70", "35", "0", "35", "70");
 
-        Assert.assertEquals(Float.valueOf(35f), Conversation.steppedSliderStep(field));
+        assertSliderStep(field, 35f);
     }
 
     @Test
-    public void steppedSliderStepUsesIntegerStepWithoutOptions() {
+    public void sliderSpecUsesIntegerStepWithoutOptions() {
         final var field = sliderField("xs:integer", "0", "10", "7");
 
-        Assert.assertEquals(Float.valueOf(1f), Conversation.steppedSliderStep(field));
+        assertSliderStep(field, 1f);
     }
 
     @Test
-    public void steppedSliderStepUsesContinuousStepWithoutOptionsForDecimal() {
+    public void sliderSpecUsesContinuousStepWithoutOptionsForDecimal() {
         final var field = sliderField("xs:decimal", "0", "1", "0.000001");
 
-        Assert.assertEquals(Float.valueOf(0f), Conversation.steppedSliderStep(field));
+        assertSliderStep(field, 0f);
     }
 
     @Test
-    public void steppedSliderStepRejectsMissingOrMalformedRangeBounds() {
+    public void sliderSpecRejectsMissingOrMalformedRangeBounds() {
         final var missingMin = sliderField("xs:integer", null, "10", "5");
         final var malformedMax = sliderField("xs:integer", "0", "not-a-number", "5");
 
-        Assert.assertNull(Conversation.steppedSliderStep(missingMin));
-        Assert.assertNull(Conversation.steppedSliderStep(malformedMax));
+        assertNotSlider(missingMin);
+        assertNotSlider(malformedMax);
     }
 
     @Test
-    public void steppedSliderStepRejectsIntegerValueThatCannotLandOnStep() {
+    public void sliderSpecRejectsIntegerValueThatCannotLandOnStep() {
         final var field = sliderField("xs:integer", "0", "10", "0.5");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsFractionalIntegerBounds() {
+    public void sliderSpecRejectsFractionalIntegerBounds() {
         final var field = sliderField("xs:integer", "0.5", "10.5", "1.5");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsFractionalIntegerOptions() {
+    public void sliderSpecRejectsFractionalIntegerOptions() {
         final var field = sliderField("xs:integer", "0", "1", "0", "0", "0.5", "1");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsFractionalIntegerValueAtFloatPrecisionBoundary() {
+    public void sliderSpecRejectsFractionalIntegerValueAtFloatPrecisionBoundary() {
         final var field = sliderField("xs:integer", "16777216", "16777218", "16777216.5");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsIntegerValueRoundedByFloatParsing() {
+    public void sliderSpecRejectsIntegerValueRoundedByFloatParsing() {
         final var field = sliderField("xs:integer", "16777216", "16777218", "16777217");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsIntegerRangeWithUnrepresentableIntermediateValue() {
+    public void sliderSpecRejectsIntegerRangeWithUnrepresentableIntermediateValue() {
         final var field = sliderField("xs:integer", "16777216", "16777218", "16777216");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
@@ -273,33 +271,33 @@ public class ConversationTest {
     }
 
     @Test
-    public void steppedSliderStepRejectsIncompatibleBounds() {
+    public void sliderSpecRejectsIncompatibleBounds() {
         final var field = sliderField("xs:integer", "0", "70", "34", "0", "34", "68");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsMalformedOptions() {
+    public void sliderSpecRejectsMalformedOptions() {
         final var field = sliderField("xs:integer", "0", "70", "35", "0", "not-a-number", "70");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
-    public void steppedSliderStepRejectsValuesOutsideRange() {
+    public void sliderSpecRejectsValuesOutsideRange() {
         final var below = sliderField("xs:integer", "0", "70", "-5", "0", "35", "70");
         final var above = sliderField("xs:integer", "0", "70", "999", "0", "35", "70");
 
-        Assert.assertNull(Conversation.steppedSliderStep(below));
-        Assert.assertNull(Conversation.steppedSliderStep(above));
+        assertNotSlider(below);
+        assertNotSlider(above);
     }
 
     @Test
-    public void steppedSliderStepRejectsDuplicateOptions() {
+    public void sliderSpecRejectsDuplicateOptions() {
         final var field = sliderField("xs:integer", "0", "70", "35", "0", "35", "35", "70");
 
-        Assert.assertNull(Conversation.steppedSliderStep(field));
+        assertNotSlider(field);
     }
 
     @Test
@@ -433,6 +431,20 @@ public class ConversationTest {
         }
     }
 
+    private static void assertSliderStep(final Element field, final float step) {
+        final var spec = Conversation.sliderSpec(field);
+        Assert.assertNotNull("Field should be usable as a slider", spec);
+        Assert.assertEquals(step, spec.step(), 0.0001f);
+    }
+
+    private static void assertNotSlider(final Element field) {
+        Assert.assertNull(Conversation.sliderSpec(field));
+    }
+
+    private static void assertNotSlider(final String message, final Element field) {
+        Assert.assertNull(message, Conversation.sliderSpec(field));
+    }
+
     private static Element sliderField(
             final String datatype,
             final String min,