Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ideas] MultiOption Validation #1410

Closed
sbernard31 opened this issue Aug 13, 2021 · 6 comments
Closed

[ideas] MultiOption Validation #1410

sbernard31 opened this issue Aug 13, 2021 · 6 comments

Comments

@sbernard31
Copy link
Contributor

Thx to ITypeConverter we have a very good way to do validation for one option input. 👍
Coupled with a #1407 it is even possible to display only the help usage corresponding to the error.

But in some cases we need more complex multi option validation mechanism. Especially because using ArgGroup to build complex rules could be quickly verbose and reveal some corner case issue. ( e.g. #1409 )


When I create my commands I would like be able to make it inherit some kind of Validable interface with a validate() method which will be called before the run() or call() method (and obviously after the args parsing)

The validate() raise exception if there was any invalid input and will be handled by IParameterExceptionHandler.
The exception could concern several options and maybe IParameterExceptionHandler wanted to know each options are concerns to display target help usage (e.g. display only help for option in error or maybe only the ArgGroup in error)
So we probably need a kind of MultiParameterException.

We can imagine that we could also make validation as ArgGroup level by making it implement Validable too.
If an exception is raised, it could be catch in a ArgGroupParameterException. (something like this)


As this mechanism does not exist for now, I workaround this by using run() or call() like if it was a validate method. Consequently, I need to execute my command out of the Command definition class but in my use case this is not an issue.

Then I create an IExecutionExceptionHandler which acts like a IParameterExceptionHandler.

I share some code :

/**
 * An exception raised to warn about invalid CLI options. <br>
 * Generally this exception concern validation about more than 1 options.
 */
public class MultiParameterException extends RuntimeException {

    private static final long serialVersionUID = 1L;

    private String[] options;

    /**
     * @param msg the error message
     * @param options list of option concerned by the exception
     */
    public MultiParameterException(String msg, String... options) {
        super(msg);
        this.options = options;
    }

    /**
     * @param msg the error message
     * @param options list of option concerned by the exception
     */
    public MultiParameterException(String msg, Exception cause, String... options) {
        super(msg, cause);
        this.options = options;
    }

    public String[] getOptions() {
        return options;
    }
}
ublic class ExecutionExceptionHandler implements IExecutionExceptionHandler {

    @Override
    public int handleExecutionException(Exception ex, CommandLine cmd, ParseResult parseResult) throws Exception {
        PrintWriter writer = cmd.getErr();
        if (ex.getMessage() != null)
            writer.print(cmd.getColorScheme().errorText(ex.getMessage()));
        else
            writer.print(cmd.getColorScheme().errorText(ex.getClass().getSimpleName()));
        writer.printf("%n");

        if (ex instanceof InvalidOptionsException) {
            writer.printf("%n");
            Help help = cmd.getHelpFactory().create(cmd.getCommandSpec(), cmd.getColorScheme());
            Layout layout = help.createDefaultLayout();
            for (String optionName : ((InvalidOptionsException) ex).getOptions()) {
                OptionSpec option = cmd.getCommandSpec().findOption(optionName);
                if (option != null) {
                    layout.addOption(option, help.createDefaultParamLabelRenderer());
                }
            }
            writer.print(layout.toString());
        } else {
            writer.print(cmd.getColorScheme().stackTraceText(ex));
        }

        writer.printf("%n");
        CommandSpec spec = cmd.getCommandSpec();
        writer.printf("Try '%s --help' for more information.%n", spec.qualifiedName());

        return AutoComplete.EXIT_CODE_INVALID_INPUT;
    }
}

Example of validations :

    @Override
    public void run() {
        // Some post-validation which imply several options.
        // For validation about only one option just ITypeConverter instead
        if (identity.isx509()) {
            if (identity.getX509().certUsage == CertificateUsage.SERVICE_CERTIFICATE_CONSTRAINT
                    && identity.getX509().trustStore.isEmpty()) {
                throw new InvalidOptionsException(
                        "You need to set a truststore when you are using \"service certificate constraint\" usage",
                        "-cu", "-ts");
            }
        }
    }
@remkop
Copy link
Owner

remkop commented Aug 14, 2021

You can achieve the desired outcome by throwing a ParameterException instead of a InvalidOptionsException from the run/call method.

See the user manual section on custom validation: https://picocli.info/#_custom_validation

@sbernard31
Copy link
Contributor Author

Thx to pointing this 🙏 (I will be able to remove my custom ExecutionExceptionHandler)

I feel there is maybe still good point in ideas above.

  1. Separating execution from validation sounds cleaner too me, so I feel that the Validatable interface could be pretty good way to achieve this. (but I guess you could consider this as just cosmetics)
  2. I think it could make sense to add a MultiParameterException or a ArgGroupParameterException which could be used in ShortErrorMessageHandler example. ([idea] Providing a ShortErrorMessageHandler #1407)
  3. I also feel that being able "to write a ArgGroup Validation in the class which defined it" could be very cool. This allow to implement an ArgGroup in a separated class with its validation to make it resuable

e.g :

public class mySection implements ValidatableArgGroup{

    @Option(names = { "-a", "--option-a" }, defaultValue = "Default A", description = "Should be 4th")
    private String a;

    @Option(names = { "-b", "--option-b" }, defaultValue = "Default B", description = "Should be 3rd")
    private String b;

   public void validate(CommandSpec cmd, ArgGroup ArgGroup) throws ParameterException {
       // Do some validation
       if (!valid)
          throw new ArgGroupParameterException(cmd, Argroup, "this is not valid because ...."); 
   }
}

and then can be reusable like this :

Command(name = "myCommand",
         mixinStandardHelpOptions = true,
         description = "A command with not well ordered option.",
         sortOptions = false)
public class Example {

    @ArgGroup(validate = false,
              heading = "%n@|italic " //
                      + "A reused Section with custom validation." //
                      + "|@%n")
    public mySection reusedSection = new mySection();
    
    public static void main(String... args) {
        Example example = new Example();
        int exitCode = new CommandLine(example).execute(args);

        System.exit(exitCode);
    }
}

There is nothing blocking to me here but I feel this kind of modification could make the API better and fit very well with #1407.

@sbernard31
Copy link
Contributor Author

You can achieve the desired outcome by throwing a ParameterException instead of a InvalidOptionsException from the run/call method.

Just to precise that I need a custom ParameterException to be able to "attached" all the option which is concerned to be able to display it in ShortErrorMessageHandler with

if (ex instanceof /*InvalidOptionsException*/ MultiParameterException) {
            writer.printf("%n");
            Help help = cmd.getHelpFactory().create(cmd.getCommandSpec(), cmd.getColorScheme());
            Layout layout = help.createDefaultLayout();
            for (String optionName : ((/*InvalidOptionsException*/ MultiParameterException) ex).getOptions()) {
                OptionSpec option = cmd.getCommandSpec().findOption(optionName);
                if (option != null) {
                    layout.addOption(option, help.createDefaultParamLabelRenderer());
                }
            }
            writer.print(layout.toString());
        } 

@remkop
Copy link
Owner

remkop commented Aug 16, 2021

You can obtain the group via ParameterException.getArgSpec().group(), from which you can get all details of the group.

@remkop
Copy link
Owner

remkop commented Aug 19, 2021

Overall, I feel that picocli as a library should be very minimalistic about validation.
The user manual covers validation in some common scenarios, and I have always felt that for picocli to provide a validation API would not add much value...

The ArgGroup validation idea is interesting, I can see that. But then for picocli to implement just that, without also providing something similar for general Commands, would be imbalanced/incomplete.

So we are back where we started: I think picocli should not provide validation API, but instead have documentation that shows how applications can do validation in different scenarios.

If you found a new scenario or a way to do things with the existing API that belongs in the docs or in the picocli-examples module, PRs are welcome!

@remkop
Copy link
Owner

remkop commented Feb 15, 2022

Good discussions, many thanks!

@remkop remkop closed this as completed Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants