verdi code setup: validate the uniqueness of the full label (aiidat…

…eam#5205)

The full label of a `Code` instance is defined as `label@computer.label`.
Recently, the uniqueness of the full label was not even checked and it
was possible to create codes with duplicate full labels. Since this was
fixed though, `verdi code setup` would only fail when all parameters are
parsed and it would try to store the code. In interactive mode, this is
pretty annoying as the command will not fail until all parameters have
been specified.

Here, a validator is added to the `--label` option to check the
uniqueness of the full label. In order to check this, it needs the
selected computer. To ensure the computer is parsed before the label, the
option is defined before the label when the command is defined. At least
for the interactive mode, this guarantees that the computer is prompted
for before the label, so the callback of the label can check for
uniqueness. If it fails, the user is shown the reason why and prompted
again.

Note that the parsing order of parameters in the non-interactive mode is
not guaranteed by the definition order but by the order in which they
are passed by the caller. Therefore, the callback of the label cannot be
relied upon, because if the label is specified first, the callback won't
know the computer and will have to exit. Therefore, the callback has to
be called manually once again in the body of the command when it is
guaranteed that both the label and computer will have been defined.
There is a slight overhead in the sense that the callback is called
twice, but the first call will exit almost instantly and so the overhead
will be minimal.

aiida/cmdline/commands/cmd_code.py

@@ -58,24 +58,31 @@ def set_code_builder(ctx, param, value):
     return value
 
 
+# Defining the ``COMPUTER`` option first guarantees that the user is prompted for the computer first. This is necessary
+# because the ``LABEL`` option has a callback that relies on the computer being already set. Execution order is
+# guaranteed only for the interactive case, however. For the non-interactive case, the callback is called explicitly
+# once more in the command body to cover the case when the label is specified before the computer.
 @verdi_code.command('setup')
+@options_code.ON_COMPUTER()
+@options_code.COMPUTER()
 @options_code.LABEL()
 @options_code.DESCRIPTION()
 @options_code.INPUT_PLUGIN()
-@options_code.ON_COMPUTER()
-@options_code.COMPUTER()
 @options_code.REMOTE_ABS_PATH()
 @options_code.FOLDER()
 @options_code.REL_PATH()
 @options_code.PREPEND_TEXT()
 @options_code.APPEND_TEXT()
 @options.NON_INTERACTIVE()
 @options.CONFIG_FILE()
+@click.pass_context
 @with_dbenv()
-def setup_code(non_interactive, **kwargs):
+def setup_code(ctx, non_interactive, **kwargs):
     """Setup a new code."""
     from aiida.orm.utils.builders.code import CodeBuilder
 
+    options_code.validate_label_uniqueness(ctx, None, kwargs['label'])
+
     if kwargs.pop('on_computer'):
         kwargs['code_type'] = CodeBuilder.CodeType.ON_COMPUTER
     else:
@@ -97,13 +104,17 @@ def setup_code(non_interactive, **kwargs):
     echo.echo_success(f'Code<{code.pk}> {code.full_label} created')
 
 
+# Defining the ``COMPUTER`` option first guarantees that the user is prompted for the computer first. This is necessary
+# because the ``LABEL`` option has a callback that relies on the computer being already set. Execution order is
+# guaranteed only for the interactive case, however. For the non-interactive case, the callback is called explicitly
+# once more in the command body to cover the case when the label is specified before the computer.
 @verdi_code.command('duplicate')
 @arguments.CODE(callback=set_code_builder)
+@options_code.ON_COMPUTER(contextual_default=get_on_computer)
+@options_code.COMPUTER(contextual_default=get_computer_name)
 @options_code.LABEL(contextual_default=partial(get_default, 'label'))
 @options_code.DESCRIPTION(contextual_default=partial(get_default, 'description'))
 @options_code.INPUT_PLUGIN(contextual_default=partial(get_default, 'input_plugin'))
-@options_code.ON_COMPUTER(contextual_default=get_on_computer)
-@options_code.COMPUTER(contextual_default=get_computer_name)
 @options_code.REMOTE_ABS_PATH(contextual_default=partial(get_default, 'remote_abs_path'))
 @options_code.FOLDER(contextual_default=partial(get_default, 'code_folder'))
 @options_code.REL_PATH(contextual_default=partial(get_default, 'code_rel_path'))
@@ -118,6 +129,8 @@ def code_duplicate(ctx, code, non_interactive, **kwargs):
     from aiida.common.exceptions import ValidationError
     from aiida.orm.utils.builders.code import CodeBuilder
 
+    options_code.validate_label_uniqueness(ctx, None, kwargs['label'])
+
     if kwargs.pop('on_computer'):
         kwargs['code_type'] = CodeBuilder.CodeType.ON_COMPUTER
     else:

aiida/cmdline/params/options/commands/code.py

@@ -23,6 +23,33 @@ def is_not_on_computer(ctx):
     return bool(not is_on_computer(ctx))
 
 
+def validate_label_uniqueness(ctx, _, value):
+    """Validate the uniqueness of the full label of the code, i.e., `label@computer.label`.
+
+    .. note:: For this to work, the computer parameter already needs to have been parsed. In interactive mode, this
+        means that the computer parameter needs to be defined after the label parameter in the command definition. For
+        non-interactive mode, the parsing order will always be determined by the order the parameters are specified by
+        the caller and so this validator may get called before the computer is parsed. For that reason, this validator
+        should also be called in the command itself, to ensure it has both the label and computer parameter available.
+    """
+    from aiida.common import exceptions
+    from aiida.orm import load_code
+
+    computer = ctx.params.get('computer', None)
+
+    if computer is not None:
+        full_label = f'{value}@{computer.label}'
+
+        try:
+            load_code(full_label)
+        except exceptions.NotExistent:
+            pass
+        else:
+            raise click.BadParameter(f'the code `{full_label}` already exists.')
+
+    return value
+
+
 ON_COMPUTER = OverridableOption(
     '--on-computer/--store-in-db',
     is_eager=False,
@@ -66,6 +93,7 @@ def is_not_on_computer(ctx):
 
 LABEL = options.LABEL.clone(
     prompt='Label',
+    callback=validate_label_uniqueness,
     cls=InteractiveOption,
     help="This label can be used to identify the code (using 'label@computerlabel'), as long as labels are unique per "
     'computer.'

tests/cmdline/commands/test_code.py

@@ -173,9 +173,7 @@ def test_noninteractive_upload(run_cli_command, non_interactive_editor):
 def test_interactive_remote(run_cli_command, aiida_localhost, non_interactive_editor):
     """Test interactive remote code setup."""
     label = 'interactive_remote'
-    user_input = '\n'.join([
-        label, 'description', 'core.arithmetic.add', 'yes', aiida_localhost.label, '/remote/abs/path'
-    ])
+    user_input = '\n'.join(['yes', aiida_localhost.label, label, 'desc', 'core.arithmetic.add', '/remote/abs/path'])
     run_cli_command(cmd_code.setup_code, user_input=user_input)
     assert isinstance(load_code(label), Code)
 
@@ -187,7 +185,7 @@ def test_interactive_upload(run_cli_command, non_interactive_editor):
     label = 'interactive_upload'
     dirname = os.path.dirname(__file__)
     basename = os.path.basename(__file__)
-    user_input = '\n'.join([label, 'description', 'core.arithmetic.add', 'no', dirname, basename])
+    user_input = '\n'.join(['no', label, 'description', 'core.arithmetic.add', dirname, basename])
     run_cli_command(cmd_code.setup_code, user_input=user_input)
     assert isinstance(load_code(label), Code)
 
@@ -198,7 +196,7 @@ def test_mixed(run_cli_command, aiida_localhost, non_interactive_editor):
     """Test mixed (interactive/from config) code setup."""
     label = 'mixed_remote'
     options = ['--description=description', '--on-computer', '--remote-abs-path=/remote/abs/path']
-    user_input = '\n'.join([label, 'core.arithmetic.add', aiida_localhost.label])
+    user_input = '\n'.join([aiida_localhost.label, label, 'core.arithmetic.add'])
     run_cli_command(cmd_code.setup_code, options, user_input=user_input)
     assert isinstance(load_code(label), Code)
 
@@ -208,7 +206,7 @@ def test_mixed(run_cli_command, aiida_localhost, non_interactive_editor):
 def test_code_duplicate_interactive(run_cli_command, aiida_local_code_factory, non_interactive_editor):
     """Test code duplication interactive."""
     label = 'code_duplicate_interactive'
-    user_input = f'{label}\n\n\n\n\n\n'
+    user_input = f'\n\n{label}\n\n\n\n'
     code = aiida_local_code_factory('core.arithmetic.add', '/bin/cat', label='code')
     run_cli_command(cmd_code.code_duplicate, [str(code.pk)], user_input=user_input)
 
@@ -226,7 +224,7 @@ def test_code_duplicate_ignore(run_cli_command, aiida_local_code_factory, non_in
     Regression test for: https://github.com/aiidateam/aiida-core/issues/3770
     """
     label = 'code_duplicate_interactive'
-    user_input = f'{label}\n!\n\n\n\n\n'
+    user_input = f'\n\n{label}\n!\n\n\n'
     code = aiida_local_code_factory('core.arithmetic.add', '/bin/cat', label='code')
     run_cli_command(cmd_code.code_duplicate, [str(code.pk)], user_input=user_input)
 
@@ -279,3 +277,40 @@ def test_from_config_url(non_interactive_editor, run_cli_command, aiida_localhos
     fake_url = 'https://my.url.com'
     run_cli_command(cmd_code.setup_code, ['--non-interactive', '--config', fake_url])
     assert isinstance(load_code(label), Code)
+
+
+@pytest.mark.usefixtures('clear_database_before_test')
+@pytest.mark.parametrize('non_interactive_editor', ('sleep 1; vim -cwq',), indirect=True)
+def test_code_setup_duplicate_full_label_interactive(
+    run_cli_command, aiida_local_code_factory, aiida_localhost, non_interactive_editor
+):
+    """Test ``verdi code setup`` in interactive mode when specifying a full label that already exists."""
+    label = 'some-label'
+    aiida_local_code_factory('core.arithmetic.add', '/bin/cat', computer=aiida_localhost, label=label)
+    assert isinstance(load_code(label), Code)
+
+    label_unique = 'label-unique'
+    user_input = '\n'.join(['yes', aiida_localhost.label, label, label_unique, 'd', 'core.arithmetic.add', '/bin/bash'])
+    run_cli_command(cmd_code.setup_code, user_input=user_input)
+    assert isinstance(load_code(label_unique), Code)
+
+
+@pytest.mark.usefixtures('clear_database_before_test')
+@pytest.mark.parametrize('label_first', (True, False))
+def test_code_setup_duplicate_full_label_non_interactive(
+    run_cli_command, aiida_local_code_factory, aiida_localhost, label_first
+):
+    """Test ``verdi code setup`` in non-interactive mode when specifying a full label that already exists."""
+    label = 'some-label'
+    aiida_local_code_factory('core.arithmetic.add', '/bin/cat', computer=aiida_localhost, label=label)
+    assert isinstance(load_code(label), Code)
+
+    options = ['-n', '-D', 'd', '-P', 'core.arithmetic.add', '--on-computer', '--remote-abs-path=/remote/abs/path']
+
+    if label_first:
+        options.extend(['--label', label, '--computer', aiida_localhost.label])
+    else:
+        options.extend(['--computer', aiida_localhost.label, '--label', label])
+
+    result = run_cli_command(cmd_code.setup_code, options, raises=True)
+    assert f'the code `{label}@{aiida_localhost.label}` already exists.' in result.output