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

Question: Get CommandSpec for actual command #1839

Open
rsenden opened this issue Oct 6, 2022 · 4 comments
Open

Question: Get CommandSpec for actual command #1839

rsenden opened this issue Oct 6, 2022 · 4 comments

Comments

@rsenden
Copy link
Contributor

rsenden commented Oct 6, 2022

Assume code like the following:

public class MyCommand {
    @Mixin private MyTopLevelMixin myTopLevelMixin;
    ...
}

public class MyTopLevelMixin {
    @Mixin private MySubMixin mySubMixin;
    ...
}

public class MySubMixin {
    @Spec(Spec.Target.MIXEE) private CommandSpec mixee;
    ...
}

It seems like the CommandSpec injected into MySubMixin represents MyTopLevelMixin, rather than MyCommand. This seems logical, as MySubMixin is mixed into MyTopLevelMixin and not directly into MyCommand. However, what if MySubMixin requires the CommandSpec for MyCommand instead?

For now, I'm using the following code to retrieve the CommandSpec of the actual command being invoked; this seems to work fine but I'm wondering whether this is the best approach?

mixee.commandLine()==null ? mixee : mixee.commandLine().getCommandSpec();

The null-check is necessary as picocli seems to inject the Mixins for all commands (even if they are not currently being executed), whereas mixee.commandLine() only returns a CommandLine instance for the currently executing command.

Are there any better approaches for injecting/retrieving the CommandSpec for the actual command, bypassing any intermediate Mixins? How about introducing a Spec.Target.COMMAND that injects the CommandSpec for the actual command?

@remkop
Copy link
Owner

remkop commented Dec 23, 2022

@rsenden Apologies for the late reply, this dropped below my inbox horizon...

About alternative approaches, one idea is to have a MIXEE spec in every Mixin:

public class MyCommand {
    @Mixin private MyTopLevelMixin myTopLevelMixin;
    ...
}

public class MyTopLevelMixin {
    @Spec(Spec.Target.MIXEE) CommandSpec mixee;

    @Mixin private MySubMixin mySubMixin;
    ...
}

public class MySubMixin {
    @Spec(Spec.Target.MIXEE) private CommandSpec mixee;
    ...
}

Then, from MySubMixin, one can have code like this:

((MyTopLevelMixin) mixee.userObject()).mixee; // gives the MyCommand commandSpec

This in addition to your solution, which I am glad to hear also works.
I hesitate to add more API, because I feel it is already quite complex...

@rsenden
Copy link
Contributor Author

rsenden commented Dec 23, 2022

@remkop Thanks for the reply. Even though the mixee.commandLine().getCommandSpec() based approach works, it seems somewhat hacky. I'm not sure whether it will work in all circumstances, or whether this could potentially break with future picocli versions.

Your approach is more explicit and less hacky, however it would be annoying having to add CommandSpec to every intermediate mixin, even if that mixin itself doesn't use the CommandSpec. We'd likely also need to recursively walk up the tree, as MySubMixin may not know how many levels up it should go; maybe in some commands it's nested 2 levels deep whereas in other commands it's nested 3 levels deep.

I think that accessing the CommandSpec of the actual command may actually be more common than accessing the CommandSpec of the immediate parent Mixin (it sure is in our code), so I still think it makes sense to add explicit support for this.

@remkop
Copy link
Owner

remkop commented Jan 25, 2023

I had not considered mixins nested in mixins, but I can see that it makes sense in your use case.

Not sure how easy it will be to implement something like this...
Looking at the implementation for populating @Spec(Spec.Target.MIXEE)-annotated CommandSpec fields (in CommandReflection.initFromAnnotatedTypedMembers), there is a CommandSpec object that is passed in and that is set as the MIXEE. In your use case, this CommandSpec object may itself be a mixin, and we would want to inject the MIXEE of that mixin. However, this MIXEE is not tracked anywhere... So this would not be trivial to implement, as it would require all CommandSpecs to track if they are mixed in to another CommandSpec (and not sure about whether it is possible for a mixin instance to be mixed in to multiple command specs, that would increase the complexity even more...)

If the userObject of the MIXEE implements some interface, then couldn't the implementation of that interface could potentially take care of the single/multiple levels? The user manual MIXEE example has only a single level but perhaps gives some idea of using interfaces to solve the problem of 2-level vs 3-level nesting...

@rsenden
Copy link
Contributor Author

rsenden commented Mar 30, 2023

For reference, I have now implemented a work-around as described below, for the following reasons:

  • We're planning to add 'composite' commands that run multiple other commands; I wasn't sure whether the mixee.commandLine().getCommandSpec() approach that we've been using so far might cause any issues with such use cases.
  • I don't like modifying intermediate mixins to add mixee fields to allow for sub-mixins to access the CommandSpec of the command that (indirectly) references a mixin (as described in Question: Get CommandSpec for actual command #1839 (comment))

The new work-around allows for injecting the CommandSpec of the actual command into arbitrary mixins that are (potentially indirectly) referenced from the command. As all of our command implementations extend from a small number of abstract base classes, the run() method in these classes recursively iterates over all mixins to see whether they implement the ICommandAware interface, and if so, call the setter declared in that interface to inject the CommandSpec.

public interface ICommandAware {
    void setCommandSpec(CommandSpec commandSpec);
}
public class AbstractCommand implements Runnable {
    @Spec private CommandSpec commandSpec;
    private boolean mixinsInitialized = false;

    public void run() {
        // We need to do this in the run method; at the time the CommandSpec is being injected
        // (through field above or setter method), the mixins haven't been initialized yet
        initMixins();
        // Do other work
    }

    protected final void initMixins() {
        if ( !mixinsInitialized ) {
            initMixins(commandSpec, commandSpec.mixins());
            mixinsInitialized = true;
        }
    }
    
    private void initMixins(CommandSpec commandSpec, Map<String, CommandSpec> mixins) {
        if ( mixins != null ) {
            for ( CommandSpec mixin : mixins.values() ) {
                Object userObject = mixin.userObject();
                if ( userObject!=null && userObject instanceof ICommandAware) {
                    ((ICommandAware)userObject).setCommandSpec(commandSpec);
                }
                initMixins(commandSpec, mixin.mixins());
            }
        }
    }
}

Although mixins can implement the ICommandAware interface directly, we use another mixin that also provides some useful utility methods:

@Command
public final class CommandHelperMixin implements ICommandAware {
    @Getter private CommandSpec commandSpec;
    @Getter private IMessageResolver messageResolver;
    
    public final void setCommandSpec(CommandSpec commandSpec) {
        this.commandSpec = commandSpec;
        this.messageResolver = new CommandSpecMessageResolver(commandSpec);
    }

    /**
     * Utility method for retrieving the command being invoked as the given 
     * type, returning null if the command is not an instance of the given 
     * type.
     */
    public final <T> T getCommandAs(Class<T> asType) {
        return getAs(getCommand(), asType);
    }

    /**
     * Utility method for retrieving the command instance.
     * @return
     */
    public final Object getCommand() {
        return commandSpec.userObject();
    }

    /**
     * Utility method for getting the given object as the given type,
     * returning null if the given object is not an instance of the
     * given type.
     * 
     * TODO This is potentially a reusable method; consider moving elsewhere.
     * @param <T>
     * @param obj
     * @param asType
     * @return
     */
    @SuppressWarnings("unchecked")
    public final static <T> T getAs(Object obj, Class<T> asType) {
        if ( obj!=null && asType.isAssignableFrom(obj.getClass()) ) {
            return (T)obj;
        }
        return null;
    }
}

This allows mixin implementations to declare the CommandHelperMixin as a mixin, and use this mixin to access the CommandSpec of the actual command from which the mixin is (potentially indirectly) referenced:

    public final class RequireConfirmation {
        @Mixin private CommandHelperMixin commandHelper; // This is where we declare the mixin mentioned above
        @Option(names = {"-y", "--confirm"}, defaultValue = "false")
        private boolean confirmed;
        
        public void checkConfirmed() {
            if (!confirmed) {
                // Get the CommandSpec of the actual command from CommandHelperMixin
                CommandSpec spec = commandHelper.getCommandSpec();
                if ( System.console()==null ) {
                    throw new ParameterException(spec.commandLine(), "Missing option: Confirm operation with -y / --confirm (interactive prompt not available)");
                } else {
                    String expectedResponse = PicocliSpecHelper.getRequiredMessageString(spec, "expectedConfirmPromptResponse");
                    String response = System.console().readLine(getPrompt());
                    if ( response.equalsIgnoreCase(expectedResponse) ) {
                        return;
                    } else {
                        throw new IllegalStateException("Aborting: operation aborted by user");
                    }
                }
            }
        }
        
        private String getPrompt() {
            // Get the CommandSpec of the actual command from CommandHelperMixin
            CommandSpec spec = commandHelper.getCommandSpec();
            String prompt = PicocliSpecHelper.getMessageString(spec, "confirmPrompt");
            if ( StringUtils.isBlank(prompt) ) {
                String[] descriptionLines = spec.optionsMap().get("-y").description();
                if ( descriptionLines==null || descriptionLines.length<1 ) {
                    throw new RuntimeException("No proper description found for generating prompt for --confirm option");
                }
                prompt = spec.optionsMap().get("-y").description()[0]+"?";
            }
            String promptOptions = PicocliSpecHelper.getRequiredMessageString(spec, "confirmPromptOptions");
            return String.format("%s (%s) ", prompt, promptOptions);
        }
    }

Of course, it would be great if at some point, picocli can provide native support for injecting the CommandSpec of the actual command into mixins, for example with an annotation like @Spec(Target.COMMAND) or by including the ICommandAware interface and handling the injection within picocli, such that we don't need to do the injection ourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants