From c9ad2b7b679b6bfa1f4761bc3236dd0301e7e4cd Mon Sep 17 00:00:00 2001 From: RyanHealey Date: Wed, 28 Dec 2022 15:30:24 +0000 Subject: [PATCH 1/3] Fix inconsistencies between simple and repeating param implementations as well as inconsistencies with the api docs --- .../simpledsl/internal/DslParamsImpl.java | 4 +- .../internal/RepeatingParamValues.java | 15 ++- .../simpledsl/internal/DslParamsImplTest.java | 43 ++++++- .../internal/RepeatingParamValuesTest.java | 120 +++++++++++++++++- 4 files changed, 175 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/lmax/simpledsl/internal/DslParamsImpl.java b/src/main/java/com/lmax/simpledsl/internal/DslParamsImpl.java index e3f6f48..cb72725 100644 --- a/src/main/java/com/lmax/simpledsl/internal/DslParamsImpl.java +++ b/src/main/java/com/lmax/simpledsl/internal/DslParamsImpl.java @@ -61,9 +61,7 @@ public RepeatingGroup[] valuesAsGroup(final String groupName) @Override public boolean hasValue(final String name) { - return findDslParam(name) - .map(DslParam::hasValue) - .orElse(false); + return getDslParam(name).hasValue(); } @Override diff --git a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java index 2330f39..f8c0646 100644 --- a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java +++ b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java @@ -22,7 +22,8 @@ class RepeatingParamValues implements RepeatingGroup @Override public boolean hasValue(final String name) { - return valuesByName.containsKey(name.toLowerCase()); + return !getValues(name).isEmpty(); + } @Override @@ -35,6 +36,10 @@ public boolean hasParam(final String name) public String value(final String name) { final String[] strings = values(name); + if (strings.length > 1) + { + throw new IllegalArgumentException("values() should be used when multiple values are allowed"); + } return strings.length > 0 ? strings[0] : null; } @@ -53,6 +58,12 @@ public DslArg[] getParams() private List getValues(final String name) { - return name != null ? valuesByName.get(name.toLowerCase()) : null; + + if (name == null || stream(dslArgs).noneMatch(arg -> arg.getName().equals(name.toLowerCase()))) + { + throw new IllegalArgumentException(String.format("Parameter %s does not exist in this repeating group", name)); + } + + return valuesByName.get(name.toLowerCase()); } } diff --git a/src/test/java/com/lmax/simpledsl/internal/DslParamsImplTest.java b/src/test/java/com/lmax/simpledsl/internal/DslParamsImplTest.java index 434aad8..9583852 100644 --- a/src/test/java/com/lmax/simpledsl/internal/DslParamsImplTest.java +++ b/src/test/java/com/lmax/simpledsl/internal/DslParamsImplTest.java @@ -580,10 +580,51 @@ public void shouldReturnIfAParamHasBeenDefinedHasAValueCaseInsensitively() assertTrue(params.hasParamAndValue("A")); } + @Test + void shouldThrowIllegalArgumentExceptionIfParameterDoesNotExist() + { + final SimpleDslParam aParam = new SimpleDslParam("a", asList("value1", "value2")); + + final DslParams params = new DslParamsImpl(new DslArg[0], Collections.singletonMap("a", aParam)); + + final IllegalArgumentException exception1 = assertThrows(IllegalArgumentException.class, + () -> params.value("b") + ); + final IllegalArgumentException exception2 = assertThrows(IllegalArgumentException.class, + () -> params.values("b") + ); + final IllegalArgumentException exception3 = assertThrows(IllegalArgumentException.class, + () -> params.hasValue("b") + ); + final IllegalArgumentException exception4 = assertThrows(IllegalArgumentException.class, + () -> params.valuesAsGroup("b") + ); + + assertEquals("b is not a parameter", exception1.getMessage()); + assertEquals("b is not a parameter", exception2.getMessage()); + assertEquals("b is not a parameter", exception3.getMessage()); + assertEquals("b is not a parameter", exception4.getMessage()); + } + + @Test + void shouldThrowIllegalArgumentExceptionIfParameterIsNotARepeatingGroup() + { + final SimpleDslParam aParam = new SimpleDslParam("a", asList("value1", "value2")); + + final DslParams params = new DslParamsImpl(new DslArg[0], Collections.singletonMap("a", aParam)); + + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> params.valuesAsGroup("a") + ); + + assertEquals("a is not a repeating group", exception.getMessage()); + + } + private enum TestValues { VALUE_1, VALUE_2, - VALUE_3; + VALUE_3 } } diff --git a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java index f13d93a..78ae76f 100644 --- a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java +++ b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java @@ -1,4 +1,3 @@ - /* * Copyright 2011 LMAX Ltd. * @@ -19,6 +18,7 @@ import com.lmax.simpledsl.api.DslArg; import com.lmax.simpledsl.api.OptionalArg; import com.lmax.simpledsl.api.RequiredArg; +import com.lmax.simpledsl.api.SimpleDslArg; import org.junit.jupiter.api.Test; import java.util.Collections; @@ -27,7 +27,11 @@ import java.util.Map; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class RepeatingParamValuesTest @@ -61,4 +65,118 @@ public void shouldReturnIfAParamHasBeenDefinedCaseInsensitively() assertTrue(params.hasParam("FOO")); assertTrue(params.hasParam("BaR")); } + + @Test + public void shouldReturnValue() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", Collections.singletonList("123")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertEquals("abc", params.value("foo")); + assertEquals("123", params.value("bar")); + } + + @Test + public void shouldReturnMultipleValues() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", asList("123", "456")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertArrayEquals(new String[]{"123", "456"}, params.values("bar")); + } + + @Test + public void shouldReportOptionalValueAsPresentWhenValueProvided() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", Collections.singletonList("123")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertTrue(params.hasValue("bar")); + } + + @Test + public void shouldNotReportOptionalValueAsPresentWhenNoValueProvided() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", emptyList()); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertFalse(params.hasValue("bar")); + } + + @Test + public void shouldReportRequiredValueAsPresentWhenEmptyValueProvided() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("")); + values.put("bar", Collections.singletonList("123")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertTrue(params.hasValue("foo")); + } + + @Test + public void shouldReportOptionalValueAsPresentWhenEmptyValueProvided() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", Collections.singletonList("")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertTrue(params.hasValue("bar")); + } + + @Test + void shouldThrowWhenParameterDoesNotExist() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("")); + values.put("bar", Collections.singletonList("123")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertThrows(IllegalArgumentException.class, + () -> params.value("fake")); + assertThrows(IllegalArgumentException.class, + () -> params.values("fake")); + assertThrows(IllegalArgumentException.class, + () -> params.hasValue("fake")); + } + + @Test + void shouldThrowExceptionWhenAttemptToGetValueButContainsMultiple() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final SimpleDslArg otherArg = new OptionalArg("bar").setAllowMultipleValues(); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("abc")); + values.put("bar", asList("123", "456")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> params.value("bar") + ); + + assertEquals("values() should be used when multiple values are allowed", exception.getMessage()); + } } \ No newline at end of file From 6373cad6a93d434975caee5faadbac334a0055af Mon Sep 17 00:00:00 2001 From: RyanHealey Date: Thu, 12 Jan 2023 17:52:00 +0000 Subject: [PATCH 2/3] Fix failing test --- .../simpledsl/internal/RepeatingParamValues.java | 2 +- .../simpledsl/internal/DslParamsParserTest.java | 6 +++--- .../internal/RepeatingParamValuesTest.java | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java index f8c0646..6f813ce 100644 --- a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java +++ b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java @@ -59,7 +59,7 @@ public DslArg[] getParams() private List getValues(final String name) { - if (name == null || stream(dslArgs).noneMatch(arg -> arg.getName().equals(name.toLowerCase()))) + if (name == null || stream(dslArgs).noneMatch(arg -> arg.getName().equalsIgnoreCase(name))) { throw new IllegalArgumentException(String.format("Parameter %s does not exist in this repeating group", name)); } diff --git a/src/test/java/com/lmax/simpledsl/internal/DslParamsParserTest.java b/src/test/java/com/lmax/simpledsl/internal/DslParamsParserTest.java index ca51a9f..1beb369 100644 --- a/src/test/java/com/lmax/simpledsl/internal/DslParamsParserTest.java +++ b/src/test/java/com/lmax/simpledsl/internal/DslParamsParserTest.java @@ -646,7 +646,7 @@ public void shouldMatchAllowedValuesCaseInsensitivelyAndReturnValuesUsingTheCase { final String[] args = {"a: value", "myGroup: joe", "myValue: a"}; final DslArg[] parameters = { - new RequiredArg("a"), + new RequiredArg("A"), new RepeatingArgGroup( new RequiredArg("myGroup").setAllowedValues("Joe", "Jenny"), new RequiredArg("myValue").setAllowedValues("A", "B")) @@ -659,8 +659,8 @@ public void shouldMatchAllowedValuesCaseInsensitivelyAndReturnValuesUsingTheCase assertEquals("value", params.value("a")); final RepeatingGroup[] groups = params.valuesAsGroup("myGroup"); assertEquals(1, groups.length); - assertEquals("Joe", groups[0].value("myGroup")); - assertEquals("A", groups[0].value("myValue")); + assertEquals("Joe", groups[0].values("myGroup")[0]); + assertEquals("A", groups[0].values("myValue")[0]); } @Test diff --git a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java index 78ae76f..c5fabdf 100644 --- a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java +++ b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java @@ -179,4 +179,18 @@ void shouldThrowExceptionWhenAttemptToGetValueButContainsMultiple() assertEquals("values() should be used when multiple values are allowed", exception.getMessage()); } + + @Test + public void shouldRetrieveValueCaseWithACaseInsensitiveKey() + { + final RequiredArg requiredArg = new RequiredArg("foo"); + final OptionalArg otherArg = new OptionalArg("bar"); + final Map> values = new HashMap<>(); + values.put("foo", Collections.singletonList("")); + values.put("bar", Collections.singletonList("123")); + final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); + + assertTrue(params.hasValue("Bar")); + assertEquals("123", params.value("BAR")); + } } \ No newline at end of file From 5937a8f883512b6ef74b3ef4bb6872a88078f6fd Mon Sep 17 00:00:00 2001 From: RyanHealey Date: Fri, 13 Jan 2023 16:52:15 +0000 Subject: [PATCH 3/3] fix null pointer exception for hasValue in a repeating group when there is no value --- .../java/com/lmax/simpledsl/internal/RepeatingParamValues.java | 3 ++- .../com/lmax/simpledsl/internal/RepeatingParamValuesTest.java | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java index 6f813ce..303e142 100644 --- a/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java +++ b/src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java @@ -22,7 +22,8 @@ class RepeatingParamValues implements RepeatingGroup @Override public boolean hasValue(final String name) { - return !getValues(name).isEmpty(); + final List values = getValues(name); + return !(values == null || values.isEmpty()); } diff --git a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java index c5fabdf..abd38d8 100644 --- a/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java +++ b/src/test/java/com/lmax/simpledsl/internal/RepeatingParamValuesTest.java @@ -27,7 +27,6 @@ import java.util.Map; import static java.util.Arrays.asList; -import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -113,7 +112,6 @@ public void shouldNotReportOptionalValueAsPresentWhenNoValueProvided() final OptionalArg otherArg = new OptionalArg("bar"); final Map> values = new HashMap<>(); values.put("foo", Collections.singletonList("abc")); - values.put("bar", emptyList()); final RepeatingParamValues params = new RepeatingParamValues(asList(requiredArg, otherArg).toArray(new DslArg[0]), values); assertFalse(params.hasValue("bar"));