Fix inconsistencies in value implementations

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

src/main/java/com/lmax/simpledsl/internal/RepeatingParamValues.java

@@ -22,7 +22,9 @@ class RepeatingParamValues implements RepeatingGroup
     @Override
     public boolean hasValue(final String name)
     {
-        return valuesByName.containsKey(name.toLowerCase());
+        final List<String> values = getValues(name);
+        return !(values == null || values.isEmpty());
+
     }
 
     @Override
@@ -35,6 +37,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 +59,12 @@ public DslArg[] getParams()
 
     private List<String> getValues(final String name)
     {
-        return name != null ? valuesByName.get(name.toLowerCase()) : null;
+
+        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));
+        }
+
+        return valuesByName.get(name.toLowerCase());
     }
 }

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
     }
 }

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

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,10 @@
 import java.util.Map;
 
 import static java.util.Arrays.asList;
+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 +64,131 @@ 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<String, List<String>> 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<String, List<String>> 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<String, List<String>> 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<String, List<String>> values = new HashMap<>();
+        values.put("foo", Collections.singletonList("abc"));
+        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<String, List<String>> 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<String, List<String>> 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<String, List<String>> 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<String, List<String>> 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());
+    }
+
+    @Test
+    public void shouldRetrieveValueCaseWithACaseInsensitiveKey()
+    {
+        final RequiredArg requiredArg = new RequiredArg("foo");
+        final OptionalArg otherArg = new OptionalArg("bar");
+        final Map<String, List<String>> 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"));
+    }
 }