Merge pull request #19 from openSUSE/refactor-callback

Refactor: Introduce new callback.py file

Author: tomschr

changelog.d/19.refactor.rst

@@ -0,0 +1 @@
+Introduce new :file:`callback.py` file to separate :func:`validate_doctypes` function from the build command.

src/docbuild/cli/callback.py

@@ -0,0 +1,76 @@
+# --- Callback Function ---
+import click
+from pydantic import Field, ValidationError
+
+from ..models.doctype import Doctype
+from ..utils.merge import merge_doctypes
+
+
+def validate_doctypes(
+    ctx: click.Context,
+    param: click.Parameter | None,
+    doctypes: tuple[str, ...],
+) -> list[Doctype]:
+    """Click callback function to validate a list of doctype strings.
+
+    Each string must conform to the format: PRODUCT/DOCSET@LIFECYCLE/LANGS
+    LANGS can be a single language code, a comma-separated list (no spaces),
+    or '*' for all.
+    Defaults and wildcards (*) are handled.
+
+    :param param: The click parameter that triggered this callback.
+    :param doctypes: A tuple of doctype strings to validate.
+    :return: A list of validated Doctype objects.
+    :raises click.Abort: If any doctype string is invalid, the command is aborted.
+    """
+    processed_data = []  # Store successfully parsed/validated data
+
+    if not doctypes:
+        return []
+
+    # click.echo(f"Our doctypes: {doctypes=}")
+    for doctype_str in doctypes:
+        try:
+            doctype = Doctype.from_str(doctype_str)
+            click.echo(f'Got {doctype}')
+            processed_data.append(doctype)
+
+        except ValidationError as err:
+            for error in err.errors():
+                field = error['loc'][0]
+                # Convert to string to ensure it works as dictionary key
+                field_name = str(field)
+                msg = error['msg']
+                # Make accessing of .description and .examples safe(r) if
+                # the definition in Doctype is not present
+                safe_field = Field(description=None, examples=None)
+                hint = getattr(
+                    Doctype.model_fields.get(field_name, safe_field),
+                    'description',
+                    None,
+                )
+                examples = getattr(
+                    Doctype.model_fields.get(field_name, safe_field),
+                    'examples',
+                    None,
+                )
+                click.secho(
+                    f"ERROR in '{field}': {msg}",
+                    fg='red',
+                    err=True,
+                )
+                if hint:
+                    click.echo(f'  → Hint: {hint}')
+                if examples:
+                    click.echo(f'  → Examples: {", ".join(examples)}')
+                click.echo()
+            raise click.Abort(err) from err
+
+    # --- Optional: Post-validation checks across all inputs ---
+    # This part becomes more complex if you need to merge/check '*' against lists.
+    # For now, we'll just validate each argument independently.
+    # You might handle the combination logic in the main command.
+
+    processed_data = merge_doctypes(*processed_data)
+    ctx.obj.doctypes = processed_data
+    return processed_data

src/docbuild/cli/cmd_build.py

@@ -35,84 +35,12 @@
 """  # noqa: D301
 
 import click
-from pydantic import Field, ValidationError
 
 from ..models.doctype import Doctype
-from ..utils.merge import merge_doctypes
+from .callback import validate_doctypes
 from .context import DocBuildContext
 
 
-# --- Callback Function ---
-def validate_doctypes(
-    ctx: click.Context,
-    param: click.Parameter | None,
-    doctypes: tuple[str, ...],
-) -> list[Doctype]:
-    """Click callback function to validate a list of doctype strings.
-
-    Each string must conform to the format: PRODUCT/DOCSET@LIFECYCLE/LANGS
-    LANGS can be a single language code, a comma-separated list (no spaces),
-    or '*' for all.
-    Defaults and wildcards (*) are handled.
-
-    :param param: The click parameter that triggered this callback.
-    :param doctypes: A tuple of doctype strings to validate.
-    :return: A list of validated Doctype objects.
-    :raises click.Abort: If any doctype string is invalid, the command is aborted.
-    """
-    processed_data = []  # Store successfully parsed/validated data
-
-    if not doctypes:
-        return []
-
-    # click.echo(f"Our doctypes: {doctypes=}")
-    for doctype_str in doctypes:
-        try:
-            doctype = Doctype.from_str(doctype_str)
-            click.echo(f'Got {doctype}')
-            processed_data.append(doctype)
-
-        except ValidationError as err:
-            for error in err.errors():
-                field = error['loc'][0]
-                # Convert to string to ensure it works as dictionary key
-                field_name = str(field)
-                msg = error['msg']
-                # Make accessing of .description and .examples safe(r) if
-                # the definition in Doctype is not present
-                safe_field = Field(description=None, examples=None)
-                hint = getattr(
-                    Doctype.model_fields.get(field_name, safe_field),
-                    'description',
-                    None,
-                )
-                examples = getattr(
-                    Doctype.model_fields.get(field_name, safe_field),
-                    'examples',
-                    None,
-                )
-                click.secho(
-                    f"ERROR in '{field}': {msg}",
-                    fg='red',
-                    err=True,
-                )
-                if hint:
-                    click.echo(f'  → Hint: {hint}')
-                if examples:
-                    click.echo(f'  → Examples: {", ".join(examples)}')
-                click.echo()
-            raise click.Abort(err) from err
-
-    # --- Optional: Post-validation checks across all inputs ---
-    # This part becomes more complex if you need to merge/check '*' against lists.
-    # For now, we'll just validate each argument independently.
-    # You might handle the combination logic in the main command.
-
-    processed_data = merge_doctypes(*processed_data)
-    ctx.obj.doctypes = processed_data
-    return processed_data
-
-
 @click.command(
     help=__doc__.replace('\b\n\n', '\b\n').replace('``', ''),  # type: ignore
 )

tests/cli/test_cmd_build.py

@@ -5,7 +5,8 @@
 from pydantic import Field, ValidationError
 import pytest
 
-import docbuild.cli.cmd_build as cmd_build
+from docbuild.cli import callback as callback_module
+from docbuild.cli.callback import validate_doctypes
 from docbuild.cli.cmd_cli import cli
 
 
@@ -14,7 +15,7 @@ def test_validate_doctypes_with_empty_doctypes():
     ctx = Context(cmd)
     ctx.obj = SimpleNamespace()
 
-    result = cmd_build.validate_doctypes(ctx, None, tuple())
+    result = validate_doctypes(ctx, None, tuple())
     assert not result
     # No ctx.obj.doctypes as this doesn't exist
 
@@ -29,28 +30,13 @@ def raise_for_invalid(s: str) -> DummyDoctype:
         return DummyDoctype(s)
 
     monkeypatch.setattr(
-        cmd_build.Doctype,
+        callback_module.Doctype,
         'from_str',
         staticmethod(raise_for_invalid),
     )
 
     with pytest.raises(Abort, match='is not a valid Product'):
-        cmd_build.validate_doctypes(ctx, None, ('wrong/1/en-us',))
-
-
-@pytest.mark.skip('Replace --role with --env-config')
-def test_validate_doctypes_called_from_build(context, fake_envfile, runner):
-    result = runner.invoke(
-        cli,
-        ['--role=production', 'build', 'sles/17/en-us'],
-        obj=context,
-    )
-
-    assert fake_envfile.mock.call_count == 1
-    assert result.exit_code == 0
-    # assert "Got sles/17@supported/en-us" in result.output
-    assert context.doctypes == [DummyDoctype('sles/17/en-us')]
-    assert context.role == 'production'
+        validate_doctypes(ctx, None, ('wrong/1/en-us',))
 
 
 class DummyDoctype:
@@ -68,7 +54,7 @@ def __str__(self):
 def patch_doctype(monkeypatch):
     # Patch Doctype.from_str to return a DummyDoctype for testing
     monkeypatch.setattr(
-        cmd_build,
+        callback_module,
         'Doctype',
         type(
             'Doctype',
@@ -86,20 +72,20 @@ def patch_doctype(monkeypatch):
         ),
     )
     # Patch merge_doctypes to just return the list for simplicity
-    monkeypatch.setattr(cmd_build, 'merge_doctypes', lambda *args: list(args))
+    monkeypatch.setattr(callback_module, 'merge_doctypes', lambda *args: list(args))
 
 
 def test_validate_doctypes_empty(ctx):
     context = ctx(SimpleNamespace())
-    result = cmd_build.validate_doctypes(context, None, ())
+    result = validate_doctypes(context, None, ())
     assert result == []
     assert not hasattr(context.obj, 'doctypes') or context.obj.doctypes == []
 
 
 def test_validate_doctypes_valid(ctx):
     context = ctx(SimpleNamespace())
     doctypes = ('foo/1/en-us', 'bar/2/de-de')
-    result = cmd_build.validate_doctypes(context, None, doctypes)
+    result = validate_doctypes(context, None, doctypes)
     assert result == [DummyDoctype('foo/1/en-us'), DummyDoctype('bar/2/de-de')]
     assert context.obj.doctypes == result
 
@@ -113,7 +99,7 @@ def errors(self):
             return [{'loc': ['field'], 'msg': 'bad', 'type': 'value_error'}]
 
     monkeypatch.setattr(
-        cmd_build,
+        callback_module,
         'Doctype',
         type(
             'Doctype',
@@ -133,7 +119,7 @@ def errors(self):
         ),
     )
     with pytest.raises(click.Abort):
-        cmd_build.validate_doctypes(context, None, ('bad/doctype',))
+        validate_doctypes(context, None, ('bad/doctype',))
 
 
 @pytest.mark.parametrize(
@@ -151,7 +137,7 @@ def test_validate_doctypes_with_doctypes(ctx, doctypes, expected):
     """Test validate_doctypes with different doctype inputs."""
     context = ctx(SimpleNamespace(doctypes=[]))
 
-    result = cmd_build.validate_doctypes(context, None, doctypes)
+    result = validate_doctypes(context, None, doctypes)
     assert result == expected
     assert context.obj.doctypes == expected
 
@@ -177,10 +163,10 @@ def mock_from_str(s: str):
         return DummyDoctype(s)
 
     # Patch necessary methods
-    monkeypatch.setattr(cmd_build, 'ValidationError', MockValidationError)
-    monkeypatch.setattr(cmd_build.Doctype, 'from_str', staticmethod(mock_from_str))
+    monkeypatch.setattr(callback_module, 'ValidationError', MockValidationError)
+    monkeypatch.setattr(callback_module.Doctype, 'from_str', staticmethod(mock_from_str))
     monkeypatch.setattr(
-        cmd_build.Doctype,
+        callback_module.Doctype,
         'model_fields',
         {
             'product': type(
@@ -196,7 +182,7 @@ def mock_from_str(s: str):
 
     # Test that the function properly aborts and formats error messages
     with pytest.raises(click.Abort):
-        cmd_build.validate_doctypes(context, None, ('invalid/product/en-us',))
+        validate_doctypes(context, None, ('invalid/product/en-us',))
 
     captured = capsys.readouterr()
     assert "ERROR in 'product': Invalid product name" in captured.err
@@ -217,9 +203,9 @@ def mock_merge(*args):
         mock_merge_called_with = args
         return list(args)
 
-    monkeypatch.setattr(cmd_build, 'merge_doctypes', mock_merge)
+    monkeypatch.setattr(callback_module, 'merge_doctypes', mock_merge)
 
-    cmd_build.validate_doctypes(context, None, doctypes)
+    validate_doctypes(context, None, doctypes)
 
     # Verify merge_doctypes was called with both doctypes
     assert len(mock_merge_called_with) == 2
@@ -233,7 +219,7 @@ def mock_merge(*args):
 def test_validate_doctypes_echo_outputs(ctx, capsys):
     """Test the echo statements in validate_doctypes."""
     context = ctx(SimpleNamespace())
-    cmd_build.validate_doctypes(context, None, ('sles/15/en-us',))
+    validate_doctypes(context, None, ('sles/15/en-us',))
 
     captured = capsys.readouterr()
     assert 'Got sles/15' in captured.out
@@ -254,7 +240,7 @@ def mock_validation_error(*args, **kwargs):
         )
 
     monkeypatch.setattr(
-        cmd_build.Doctype,
+        callback_module.Doctype,
         'model_fields',
         {
             'product': Field(
@@ -264,10 +250,10 @@ def mock_validation_error(*args, **kwargs):
             ),
         },
     )
-    monkeypatch.setattr(cmd_build.Doctype, 'from_str', mock_validation_error)
+    monkeypatch.setattr(callback_module.Doctype, 'from_str', mock_validation_error)
 
     with pytest.raises(click.Abort, match=r'Mock validation error'):
-        cmd_build.validate_doctypes(
+        validate_doctypes(
             click.Context(click.Command('dummy')),
             None,
             ('foo/bar',),
@@ -297,7 +283,7 @@ def mock_validation_error(*args, **kwargs):
         )
 
     monkeypatch.setattr(
-        cmd_build.Doctype,
+        callback_module.Doctype,
         'model_fields',
         {
             'product': type(
@@ -310,10 +296,10 @@ def mock_validation_error(*args, **kwargs):
             )(),
         },
     )
-    monkeypatch.setattr(cmd_build.Doctype, 'from_str', mock_validation_error)
+    monkeypatch.setattr(callback_module.Doctype, 'from_str', mock_validation_error)
 
     with pytest.raises(click.Abort, match=r'Mock validation error'):
-        cmd_build.validate_doctypes(
+        validate_doctypes(
             click.Context(click.Command('dummy')),
             None,
             ('foo/bar',),
@@ -342,7 +328,7 @@ def mock_validation_error(*args, **kwargs):
         )
 
     monkeypatch.setattr(
-        cmd_build.Doctype,
+        callback_module.Doctype,
         'model_fields',
         {
             'product': type(
@@ -355,10 +341,10 @@ def mock_validation_error(*args, **kwargs):
             )(),
         },
     )
-    monkeypatch.setattr(cmd_build.Doctype, 'from_str', mock_validation_error)
+    monkeypatch.setattr(callback_module.Doctype, 'from_str', mock_validation_error)
 
     with pytest.raises(click.Abort, match=r'Mock validation error'):
-        cmd_build.validate_doctypes(
+        validate_doctypes(
             click.Context(click.Command('dummy')),
             None,
             ('foo/bar',),