Skip to content

Commit

Permalink
InteractiveOption: Fix validation being skipped if ! provided
Browse files Browse the repository at this point in the history
The `InteractiveOption` reserves the exclamation point `!` as a special
character in order to "skip" the option and not define it. This is
necessary for options that are _not_ required but that do specify a
defualt. Without this character, it is impossible to _not_ set the
default as, if the user doesn't specify anything specific and simply
presses enter, the default is taken, even if the user did not want to
specify any specific value.

The problem with the implementation, however, is that when `!` was
provided, the option would return `None` as the value, bypassing any
validation that might be defined. This would make it possible to bypass
the validation of a required option.

The solution is to, when `!` is provided for an interactive option, it
is translated to `None` and is then processed as normal, validating it
as any other value.
  • Loading branch information
sphuber committed Sep 13, 2023
1 parent 13df42c commit c4b183b
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 7 deletions.
4 changes: 2 additions & 2 deletions aiida/cmdline/params/options/commands/computer.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def should_call_default_memory_per_machine(ctx): # pylint: disable=invalid-name
prompt='Default number of CPUs per machine',
cls=InteractiveOption,
prompt_fn=should_call_default_mpiprocs_per_machine,
required_fn=False,
required=False,
type=click.INT,
help='The default number of MPI processes that should be executed per machine (node), if not otherwise specified.'
'Use 0 to specify no default value.',
Expand All @@ -137,7 +137,7 @@ def should_call_default_memory_per_machine(ctx): # pylint: disable=invalid-name
prompt='Default amount of memory per machine (kB).',
cls=InteractiveOption,
prompt_fn=should_call_default_memory_per_machine,
required_fn=False,
required=False,
type=click.INT,
help='The default amount of RAM (kB) that should be allocated per machine (node), if not otherwise specified.'
)
Expand Down
4 changes: 0 additions & 4 deletions aiida/cmdline/params/options/commands/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,31 +177,27 @@ def get_quicksetup_password(ctx, param, value): # pylint: disable=unused-argume
SETUP_USER_EMAIL = options.USER_EMAIL.clone(
prompt='Email Address (for sharing data)',
default=get_config_option('autofill.user.email'),
required_fn=lambda x: get_config_option('autofill.user.email') is None,
required=True,
cls=options.interactive.InteractiveOption
)

SETUP_USER_FIRST_NAME = options.USER_FIRST_NAME.clone(
prompt='First name',
default=get_config_option('autofill.user.first_name'),
required_fn=lambda x: get_config_option('autofill.user.first_name') is None,
required=True,
cls=options.interactive.InteractiveOption
)

SETUP_USER_LAST_NAME = options.USER_LAST_NAME.clone(
prompt='Last name',
default=get_config_option('autofill.user.last_name'),
required_fn=lambda x: get_config_option('autofill.user.last_name') is None,
required=True,
cls=options.interactive.InteractiveOption
)

SETUP_USER_INSTITUTION = options.USER_INSTITUTION.clone(
prompt='Institution',
default=get_config_option('autofill.user.institution'),
required_fn=lambda x: get_config_option('autofill.user.institution') is None,
required=True,
cls=options.interactive.InteractiveOption
)
Expand Down
5 changes: 4 additions & 1 deletion aiida/cmdline/params/options/interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ def process_value(self, ctx: click.Context, value: t.Any) -> t.Any:
return self.prompt_for_value(ctx)

if value == self.CHARACTER_IGNORE_DEFAULT and source is click.core.ParameterSource.PROMPT:
return None
# This means the user does not want to set a specific value for this option, so the ``!`` character is
# translated to ``None`` and processed as normal. If the option is required, a validation error will be
# raised further down below, forcing the user to specify a valid value.
value = None

try:
return super().process_value(ctx, value)
Expand Down
16 changes: 16 additions & 0 deletions tests/cmdline/params/options/test_interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,22 @@ def test_not_required_interactive_default(run_cli_command):
assert expected in result.output


def test_interactive_ignore_default_not_required_option(run_cli_command):
"""Test that if an option is not required ``!`` is accepted and is translated to ``None``."""
cmd = create_command(required=False)
result = run_cli_command(cmd, user_input='!\n')
expected = 'Opt: !\nNone\n'
assert expected in result.output


def test_interactive_ignore_default_required_option(run_cli_command):
"""Test that if an option is required, ``!`` is translated to ``None`` and so is not accepted."""
cmd = create_command(required=True)
result = run_cli_command(cmd, user_input='!\nvalue\n')
expected = 'Opt: !\nError: Missing parameter: opt\nOpt: value\nvalue\n'
assert expected in result.output


def test_get_help_message():
"""Test the :meth:`aiida.cmdline.params.options.interactive.InteractiveOption.get_help_message`."""
option = InteractiveOption('-s', type=click.STRING)
Expand Down

0 comments on commit c4b183b

Please sign in to comment.