CLI-159: It is now possible to set a required flag on a group explicitly. If this group is then added as child to another group, it is only validated if options of this group are found on the command line or its required flag is true.

Oliver Heger · Oliver Heger · commit cf06f677daf0 · 2008-07-27T20:32:14.000Z
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/cli/trunk@680178 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/java/org/apache/commons/cli2/builder/GroupBuilder.java b/src/java/org/apache/commons/cli2/builder/GroupBuilder.java
@@ -33,6 +33,7 @@ public class GroupBuilder {
     private List options;
     private int minimum;
     private int maximum;
+    private boolean required;
 
     /**
      * Creates a new GroupBuilder
@@ -47,7 +48,7 @@ public GroupBuilder() {
      */
     public Group create() {
         final GroupImpl group =
-            new GroupImpl(options, name, description, minimum, maximum);
+            new GroupImpl(options, name, description, minimum, maximum, required);
 
         reset();
 
@@ -63,6 +64,7 @@ public GroupBuilder reset() {
         options = new ArrayList();
         minimum = 0;
         maximum = Integer.MAX_VALUE;
+        required = true;
         return this;
     }
 
@@ -115,4 +117,16 @@ public GroupBuilder withOption(final Option option) {
         this.options.add(option);
         return this;
     }
+
+    /**
+     * Sets the required flag. This flag is evaluated for groups that are
+     * added to other groups as child groups. If set to <b>true</b> the
+     * minimum and maximum constraints of the child group are always evaluated.
+     * @param newRequired the required flag
+     * @return this builder
+     */
+    public GroupBuilder withRequired(final boolean newRequired) {
+        this.required = newRequired;
+        return this;
+    }
 }
diff --git a/src/java/org/apache/commons/cli2/option/GroupImpl.java b/src/java/org/apache/commons/cli2/option/GroupImpl.java
@@ -60,13 +60,15 @@ public class GroupImpl
      * @param description a description of this Group
      * @param minimum the minimum number of Options for a valid CommandLine
      * @param maximum the maximum number of Options for a valid CommandLine
+     * @param required a flag whether this group is required
      */
     public GroupImpl(final List options,
                      final String name,
                      final String description,
                      final int minimum,
-                     final int maximum) {
-        super(0, false);
+                     final int maximum,
+                     final boolean required) {
+        super(0, required);
 
         this.name = name;
         this.description = description;
@@ -247,7 +249,7 @@ public void validate(final WriteableCommandLine commandLine)
             final Option option = (Option) i.next();
 
             // needs validation?
-            boolean validate = option.isRequired() || option instanceof Group;
+            boolean validate = option.isRequired();
 
             // if the child option is present then validate it
             if (commandLine.hasOption(option)) {
@@ -470,8 +472,16 @@ public int getMaximum() {
         return maximum;
     }
 
-    public boolean isRequired() {
-        return getMinimum() > 0;
+    /**
+     * Tests whether this option is required. For groups we evaluate the
+     * <code>required</code> flag common to all options, but also take the
+     * minimum constraints into account.
+     *
+     * @return a flag whether this option is required
+     */
+    public boolean isRequired()
+    {
+        return (getParent() == null || super.isRequired()) && getMinimum() > 0;
     }
 
     public void defaults(final WriteableCommandLine commandLine) {
diff --git a/src/test/org/apache/commons/cli2/bug/BugCLI159Test.java b/src/test/org/apache/commons/cli2/bug/BugCLI159Test.java
@@ -0,0 +1,169 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.commons.cli2.bug;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.cli2.CommandLine;
+import org.apache.commons.cli2.Group;
+import org.apache.commons.cli2.Option;
+import org.apache.commons.cli2.OptionException;
+import org.apache.commons.cli2.builder.DefaultOptionBuilder;
+import org.apache.commons.cli2.builder.GroupBuilder;
+import org.apache.commons.cli2.commandline.Parser;
+
+/**
+ * Inconsistent handling of minimum and maximum constraints for groups and their
+ * child groups.
+ *
+ * @author Oliver Heger
+ * @version $Id$
+ */
+public class BugCLI159Test extends TestCase
+{
+    /** The parent group. */
+    private Group parent;
+
+    /** The child group. */
+    private Group child;
+
+    /** The parser. */
+    private Parser parser;
+
+    /**
+     * Creates some test options, including a group with a child group.
+     *
+     * @param childGroupRequired a flag whether the child group is required
+     */
+    private void setUpOptions(boolean childGroupRequired)
+    {
+        final DefaultOptionBuilder obuilder = new DefaultOptionBuilder();
+        final GroupBuilder gbuilder = new GroupBuilder();
+        Option parentOpt = obuilder.withLongName("parent").withShortName("p")
+                .create();
+        Option childOpt1 = obuilder.withLongName("child").withShortName("c")
+                .create();
+        Option childOpt2 = obuilder.withLongName("sub").withShortName("s")
+                .create();
+        Option childOpt3 = obuilder.withLongName("test").withShortName("t")
+                .create();
+        child = gbuilder.withName("childOptions").withOption(childOpt1)
+                .withOption(childOpt2).withOption(childOpt3).withMinimum(2)
+                .withRequired(childGroupRequired).create();
+        parent = gbuilder.withName("options").withOption(parentOpt).withOption(
+                child).withMinimum(0).create();
+        parser = new Parser();
+        parser.setGroup(parent);
+    }
+
+    /**
+     * Tests whether the child group can be omitted.
+     */
+    public void testNoChildGroup() throws OptionException
+    {
+        setUpOptions(false);
+        CommandLine cl = parser.parse(new String[] {
+            "--parent"
+        });
+        assertNotNull("No command line parsed", cl);
+        assertFalse("Child group found", cl.hasOption(child));
+    }
+
+    /**
+     * Tests whether a required child groupd can be omitted.
+     */
+    public void testNoChildGroupRequired()
+    {
+        setUpOptions(true);
+        try
+        {
+            parser.parse(new String[] {
+                "--parent"
+            });
+            fail("Missing child group not detected!");
+        }
+        catch (OptionException oex)
+        {
+            // ok
+        }
+    }
+
+    /**
+     * Tests parsing an empty command line. Because the parent group is optional
+     * this should be possible.
+     */
+    public void testNoOptions() throws OptionException
+    {
+        setUpOptions(false);
+        CommandLine cl = parser.parse(new String[0]);
+        assertFalse("Found parent option", cl.hasOption("--parent"));
+        assertFalse("Found child option", cl.hasOption("--child"));
+    }
+
+    /**
+     * Tests parsing a command line with options of the child group.
+     */
+    public void testWithChildOptions() throws OptionException
+    {
+        setUpOptions(false);
+        CommandLine cl = parser.parse(new String[] {
+            "-ct"
+        });
+        assertTrue("child option not found", cl.hasOption("--child"));
+        assertTrue("test option not found", cl.hasOption("--test"));
+    }
+
+    /**
+     * Tests a command line containing options of the child group, but the
+     * minimum constraint is violated.
+     */
+    public void testWithChildOptionsMissing()
+    {
+        setUpOptions(false);
+        try
+        {
+            parser.parse(new String[] {
+                    "--parent", "--sub"
+            });
+            fail("Missing options of child group not detected!");
+        }
+        catch (OptionException oex)
+        {
+            // ok
+        }
+    }
+
+    /**
+     * Tests whether the root group is always validated.
+     */
+    public void testRequiredRootGroup()
+    {
+        setUpOptions(false);
+        parser.setGroup(child);
+        try
+        {
+            parser.parse(new String[] {
+                "--test"
+            });
+            fail("Missing options not detected!");
+        }
+        catch (OptionException oex)
+        {
+            // ok
+        }
+    }
+}
diff --git a/src/test/org/apache/commons/cli2/option/GroupTest.java b/src/test/org/apache/commons/cli2/option/GroupTest.java
@@ -55,7 +55,7 @@ public static Group buildApacheCommandGroup() {
         options.add(COMMAND_START);
         options.add(COMMAND_STOP);
 
-        return new GroupImpl(options, "httpd-cmds", "The command to pass to the server", 1, 1);
+        return new GroupImpl(options, "httpd-cmds", "The command to pass to the server", 1, 1, true);
     }
 
     public static Group buildApachectlGroup() {
@@ -64,15 +64,26 @@ public static Group buildApachectlGroup() {
         options.add(ParentTest.buildKParent());
 
         return new GroupImpl(options, "apachectl", "Controls the apache http deamon", 0,
-                             Integer.MAX_VALUE);
+                             Integer.MAX_VALUE, true);
     }
 
     public static Group buildAntGroup() {
         final List options = new ArrayList();
         options.add(DefaultOptionTest.buildHelpOption());
         options.add(ArgumentTest.buildTargetsArgument());
 
-        return new GroupImpl(options, "ant", "The options for ant", 0, Integer.MAX_VALUE);
+        return new GroupImpl(options, "ant", "The options for ant", 0, Integer.MAX_VALUE, true);
+    }
+
+    private static Group buildRequiredTestGroup(final boolean required,
+            final int minimum)
+    {
+        final Group group = new GroupImpl(new ArrayList(), "test", null,
+                minimum, Integer.MAX_VALUE, required);
+        final List options = new ArrayList(1);
+        options.add(group);
+        new GroupImpl(options, "parent", null, 0, Integer.MAX_VALUE, false);
+        return group;
     }
 
     /*
@@ -437,4 +448,65 @@ public void testHelpLines_NoName() {
 
         assertFalse(i.hasNext());
     }
+
+    /**
+     * Tests isRequired() for a child group if neither the required flag nor a
+     * minimum constraint is set.
+     */
+    public void testIsRequired_ChildNoFlagNoMinimum()
+    {
+        final Group group = buildRequiredTestGroup(false, 0);
+        assertFalse("Group is required", group.isRequired());
+    }
+
+    /**
+     * Tests isRequired() for a child group that has a minimum constraint, but
+     * the required flag is not set.
+     */
+    public void testIsRequired_ChildNoFlagMinimum()
+    {
+        final Group group = buildRequiredTestGroup(false, 10);
+        assertFalse("Group is required", group.isRequired());
+    }
+
+    /**
+     * Tests isRequired() for a child group that has the required flag set, but
+     * no minimum constraint. In this constellation the group is de facto not
+     * required.
+     */
+    public void testIsRequired_ChildFlagNoMinimum()
+    {
+        final Group group = buildRequiredTestGroup(true, 0);
+        assertFalse("Group is required", group.isRequired());
+    }
+
+    /**
+     * Tests isRequired() for a child group that has both the required flag and
+     * a minimum constraint set. This is indeed a required group.
+     */
+    public void testIsRequired_ChildFlagMinimum()
+    {
+        final Group group = buildRequiredTestGroup(true, 10);
+        assertTrue("Group is not required", group.isRequired());
+    }
+
+    /**
+     * Tests isRequired() for the root group when no minimum constraint is set.
+     */
+    public void testIsRequired_ParentNoMinimum()
+    {
+        final Group parent = (Group) buildRequiredTestGroup(false, 0)
+                .getParent();
+        assertFalse("Group is required", parent.isRequired());
+    }
+
+    /**
+     * Tests isRequired() for the root group with a minimum constraint.
+     */
+    public void testIsRequired_ParentMiminum()
+    {
+        final Group parent = new GroupImpl(new ArrayList(), "test", null, 10,
+                Integer.MAX_VALUE, false);
+        assertTrue("Group not required", parent.isRequired());
+    }
 }
