do not allow arguments with default values to be required (DAT-17436) (…

…#5848)
liquibase · May 1, 2024 · 69d3680 · 69d3680
1 parent fb4d03e
commit 69d3680
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 7 deletions.
diff --git a/...egration-tests/src/test/resources/liquibase/extension/testing/command/dropAll.test.groovy b/...egration-tests/src/test/resources/liquibase/extension/testing/command/dropAll.test.groovy
@@ -14,8 +14,6 @@ CommandTests.define {
 Short Description: Drop all database objects owned by the user
 Long Description: NOT SET
 Required Args:
-  force (Boolean) Argument to allow use of dropAll with values of 'true' or 'false'. The default is 'false'.
-  requireForce (Boolean) Argument to require user of dropAll to supply a 'force' argument, with values of 'true' or 'false'. The default is 'false'.
   url (String) The JDBC database connection URL
     OBFUSCATED
 Optional Args:
@@ -27,9 +25,13 @@ Optional Args:
     Default: null
   driverPropertiesFile (String) The JDBC driver properties file
     Default: null
+  force (Boolean) Argument to allow use of dropAll with values of 'true' or 'false'. The default is 'false'.
+    Default: false
   password (String) Password to use to connect to the database
     Default: null
     OBFUSCATED
+  requireForce (Boolean) Argument to require user of dropAll to supply a 'force' argument, with values of 'true' or 'false'. The default is 'false'.
+    Default: false
   schemas (String) Schemas to include in drop
     Default: null
   username (String) Username to use to connect to the database

diff --git a/liquibase-standard/src/main/java/liquibase/command/CommandFactory.java b/liquibase-standard/src/main/java/liquibase/command/CommandFactory.java
@@ -198,6 +198,9 @@ protected void register(String[] commandName, CommandArgumentDefinition<?> defin
         if (commandArgumentDefinitions.get(commandNameKey).contains(definition)) {
            throw new IllegalArgumentException("Duplicate argument '" + definition.getName() + "' found for command '" + commandNameKey + "'");
         }
+        if (definition.isRequired() && definition.getDefaultValue() != null) {
+            throw new IllegalArgumentException("Argument '" + definition.getName() + "' for command '" + commandNameKey + "' has both a default value and the isRequired flag set to true. Arguments with default values cannot be marked as required.");
+        }
         this.commandArgumentDefinitions.get(commandNameKey).add(definition);
     }
 

diff --git a/liquibase-standard/src/main/java/liquibase/command/core/DropAllCommandStep.java b/liquibase-standard/src/main/java/liquibase/command/core/DropAllCommandStep.java
@@ -40,12 +40,10 @@ public class DropAllCommandStep extends AbstractCommandStep {
         SCHEMAS_ARG.setSupersededBy(CATALOG_AND_SCHEMAS_ARG);
         REQUIRE_FORCE_ARG = builder.argument("requireForce", Boolean.class)
                 .description("Argument to require user of dropAll to supply a 'force' argument, with values of 'true' or 'false'. The default is 'false'.")
-                .required()
                 .defaultValue(false)
                 .build();
         FORCE_ARG = builder.argument("force", Boolean.class)
                 .description("Argument to allow use of dropAll with values of 'true' or 'false'. The default is 'false'.")
-                .required()
                 .defaultValue(false)
                 .build();
     }

diff --git a/liquibase-standard/src/test/groovy/liquibase/command/CommandArgumentDefinitionTest.groovy b/liquibase-standard/src/test/groovy/liquibase/command/CommandArgumentDefinitionTest.groovy
@@ -78,10 +78,10 @@ class CommandArgumentDefinitionTest extends Specification {
         arg1.defaultValue == null
 
         when:
-        def arg2 = builder.argument("arg2", String).required().defaultValue("default value").description("This is arg2").build()
+        def arg2 = builder.argument("arg2", String).defaultValue("default value").description("This is arg2").build()
         then:
         arg2.name == "arg2"
-        arg2.required
+        !arg2.required
         arg2.defaultValue == "default value"
 
         when:
@@ -90,7 +90,13 @@ class CommandArgumentDefinitionTest extends Specification {
         def e = thrown(IllegalArgumentException)
         e.message == "Invalid argument format: kabob-case"
 
+        when:
+        builder.argument("arg3", String).defaultValue("def").required().build()
+        then:
+        def e2 = thrown(IllegalArgumentException)
+        e2.message == "Argument 'arg3' for command 'mock' has both a default value and the isRequired flag set to true. Arguments with default values cannot be marked as required."
+
         then:
-        StringUtil.join(Scope.currentScope.getSingleton(CommandFactory).getCommandDefinition("mock").getArguments(), ", ") == "arg1=arg1, arg2=arg2 (required)"
+        StringUtil.join(Scope.currentScope.getSingleton(CommandFactory).getCommandDefinition("mock").getArguments(), ", ") == "arg1=arg1, arg2=arg2"
     }
 }