Fix inconsistencies between simple and repeating param implementation…

…s as well as inconsistencies with the api docs

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

@@ -6,6 +6,8 @@
 import java.util.List;
 import java.util.Map;
 
+import static java.util.Arrays.stream;
+
 class RepeatingParamValues implements RepeatingGroup
 {
     private final DslArg[] dslArgs;
@@ -20,13 +22,18 @@ class RepeatingParamValues implements RepeatingGroup
     @Override
     public boolean hasValue(final String name)
     {
-        return valuesByName.containsKey(name.toLowerCase());
+        return !getValues(name).isEmpty();
+
     }
 
     @Override
     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;
     }
 
@@ -45,6 +52,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().equals(name.toLowerCase())))
+        {
+            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

@@ -510,10 +510,51 @@ public void shouldReturnOptionalListWhenMultipleParameterValueIsSupplied()
         assertEquals(Optional.of(asList("value1", "value2")), params.valuesAsOptional("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/RepeatingParamValuesTest.java

@@ -0,0 +1,153 @@
+/*
+ * Copyright 2011 LMAX Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.lmax.simpledsl.internal;
+
+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;
+import java.util.HashMap;
+import java.util.List;
+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
+{
+
+    @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"));
+        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<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());
+    }
+}