add online/offline indicator to the CLI output of jmp get exporters

packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py

@@ -124,3 +124,111 @@ class OutputMode(str):
 opt_nointeractive = click.option(
     "--nointeractive", is_flag=True, default=False, help="Disable interactive prompts (for use in scripts)."
 )
+
+
+def _normalize_tokens(items: list[str], normalize_case: bool) -> list[str]:
+    """Extract and normalize tokens from comma-separated values."""
+    tokens = (
+        token.strip().lower() if normalize_case else token.strip()
+        for item in items
+        for token in item.split(',')
+    )
+    return [token for token in tokens if token]
+
+
+def _deduplicate_tokens(tokens: list[str]) -> list[str]:
+    """Remove duplicates while preserving order."""
+    return list(dict.fromkeys(tokens))
+
+
+def _validate_tokens(tokens: list[str], allowed_values: set[str], ctx, param) -> None:
+    """Validate tokens against allowed values."""
+    invalid = [t for t in tokens if t not in allowed_values]
+    if invalid:
+        allowed_list = ", ".join(sorted(allowed_values))
+        raise click.BadParameter(
+            f"Invalid value(s) {invalid}. Allowed values are: {allowed_list}",
+            ctx=ctx,
+            param=param
+        )
+
+
+def parse_comma_separated(
+    ctx: click.Context,
+    param: click.Parameter,
+    value: str | tuple[str, ...] | None,
+    allowed_values: set[str] | None = None,
+    normalize_case: bool = True
+) -> list[str]:
+    """Generic comma-separated value parser with validation and normalization.
+
+    Supports both CSV format ("a,b") and repeated flags ("a" "b" from --flag a --flag b).
+    Normalizes by stripping whitespace, optionally lowercasing, deduplicating while preserving order.
+    Optionally validates against allowed values and raises click.BadParameter on invalid tokens.
+
+    Args:
+        ctx: Click context
+        param: Click parameter
+        value: Input value(s) - string for CSV or tuple for repeated flags
+        allowed_values: Set of allowed values for validation (None = no validation)
+        normalize_case: Whether to convert values to lowercase
+
+    Returns:
+        List of normalized, deduplicated values
+
+    Raises:
+        click.BadParameter: If validation fails with invalid tokens
+    """
+    if not value:
+        return []
+
+    # Handle both single string and tuple (from multiple flag usage)
+    items = [value] if isinstance(value, str) else list(value)
+
+    # Process tokens through the pipeline
+    all_tokens = _normalize_tokens(items, normalize_case)
+    unique_tokens = _deduplicate_tokens(all_tokens)
+
+    # Validate if allowed values are specified
+    if allowed_values is not None:
+        _validate_tokens(unique_tokens, allowed_values, ctx, param)
+
+    return unique_tokens
+
+
+def opt_comma_separated(
+    name: str,
+    allowed_values: set[str] | None = None,
+    normalize_case: bool = True,
+    help_text: str | None = None
+):
+    """Create a click option for comma-separated values with optional validation.
+
+    Args:
+        name: Option name (e.g. "with" creates --with option)
+        allowed_values: Set of allowed values for validation (None = no validation)
+        normalize_case: Whether to convert values to lowercase
+        help_text: Custom help text (auto-generated if None)
+
+    Returns:
+        Click option decorator
+    """
+
+    def callback(ctx, param, value):
+        return parse_comma_separated(ctx, param, value, allowed_values, normalize_case)
+
+    # Auto-generate help text if not provided
+    if help_text is None:
+        if allowed_values:
+            allowed_list = ", ".join(sorted(allowed_values))
+            help_text = f"Comma-separated values. Allowed: {allowed_list} (comma-separated or repeated)"
+        else:
+            help_text = "Comma-separated values (comma-separated or repeated)"
+
+    return click.option(
+        f"--{name}",
+        f"{name}_options",
+        callback=callback,
+        multiple=True,
+        help=help_text
+    )

packages/jumpstarter-cli/jumpstarter_cli/get.py

@@ -1,7 +1,7 @@
 import click
 from jumpstarter_cli_common.config import opt_config
 from jumpstarter_cli_common.exceptions import handle_exceptions_with_reauthentication
-from jumpstarter_cli_common.opt import OutputType, opt_output_all
+from jumpstarter_cli_common.opt import OutputType, opt_comma_separated, opt_output_all
 from jumpstarter_cli_common.print import model_print
 
 from .common import opt_selector
@@ -19,15 +19,20 @@ def get():
 @opt_config(exporter=False)
 @opt_selector
 @opt_output_all
-@click.option("--with", "with_options", multiple=True, help="Include additional information (e.g., 'leases')")
+@opt_comma_separated(
+    "with",
+    {"leases", "online"},
+    help_text="Include fields: leases, online (comma-separated or repeated)"
+)
 @handle_exceptions_with_reauthentication(relogin_client)
-def get_exporters(config, selector: str | None, output: OutputType, with_options: tuple[str, ...]):
+def get_exporters(config, selector: str | None, output: OutputType, with_options: list[str]):
     """
     Display one or many exporters
     """
 
     include_leases = "leases" in with_options
-    exporters = config.list_exporters(filter=selector, include_leases=include_leases)
+    include_online = "online" in with_options
+    exporters = config.list_exporters(filter=selector, include_leases=include_leases, include_online=include_online)
 
     model_print(exporters, output)
 

packages/jumpstarter-cli/jumpstarter_cli/get_test.py

@@ -0,0 +1,241 @@
+from unittest.mock import Mock
+
+import click
+import pytest
+from jumpstarter_cli_common.opt import parse_comma_separated
+
+from jumpstarter.client.grpc import Exporter, ExporterList, Lease
+from jumpstarter.config.client import ClientConfigV1Alpha1
+
+
+class TestParseWith:
+    """Test the generic parse_comma_separated function with --with specific validation."""
+
+    @property
+    def allowed_values(self):
+        """Allowed values for --with option"""
+        return {"leases", "online"}
+
+    def test_single_option(self):
+        """Test parsing a single option"""
+        result = parse_comma_separated(None, None, "leases", self.allowed_values)
+        assert result == ["leases"]
+
+    def test_multiple_options(self):
+        """Test parsing multiple comma-separated options"""
+        result = parse_comma_separated(None, None, "leases,online", self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_options_with_spaces(self):
+        """Test parsing options with spaces around commas"""
+        result = parse_comma_separated(None, None, "leases, online", self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_empty_value(self):
+        """Test parsing empty or None value"""
+        assert parse_comma_separated(None, None, None, self.allowed_values) == []
+        assert parse_comma_separated(None, None, "", self.allowed_values) == []
+
+    def test_invalid_options_raise_error(self):
+        """Test that invalid options raise click.BadParameter"""
+        with pytest.raises(
+            click.BadParameter,
+            match="Invalid value\\(s\\) \\['unknown', 'invalid'\\]. Allowed values are: leases, online"
+        ):
+            parse_comma_separated(None, None, "unknown,online,invalid", self.allowed_values)
+
+        with pytest.raises(
+            click.BadParameter,
+            match="Invalid value\\(s\\) \\['invalid'\\]. Allowed values are: leases, online"
+        ):
+            parse_comma_separated(None, None, "online,invalid", self.allowed_values)
+
+    def test_repeated_flags_tuple_input(self):
+        """Test parsing multiple flags as tuple (--with a --with b)"""
+        result = parse_comma_separated(None, None, ("leases", "online"), self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_mixed_csv_and_repeated_flags(self):
+        """Test mixing CSV and repeated flags"""
+        result = parse_comma_separated(None, None, ("leases,online", "leases"), self.allowed_values)
+        assert result == ["leases", "online"]  # deduplicated
+
+    def test_normalization_lowercase(self):
+        """Test that values are normalized to lowercase"""
+        result = parse_comma_separated(None, None, "LEASES,Online", self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_whitespace_stripping(self):
+        """Test that whitespace is stripped from values"""
+        result = parse_comma_separated(None, None, " leases , online ", self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_empty_tokens_dropped(self):
+        """Test that empty tokens are dropped"""
+        result = parse_comma_separated(None, None, "leases,,online,", self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_deduplication_preserves_order(self):
+        """Test that deduplication preserves first occurrence order"""
+        result = parse_comma_separated(None, None, "online,leases,online,leases", self.allowed_values)
+        assert result == ["online", "leases"]
+
+    def test_empty_string_in_tuple(self):
+        """Test handling empty strings in tuple"""
+        result = parse_comma_separated(None, None, ("", "leases", ""), self.allowed_values)
+        assert result == ["leases"]
+
+    def test_complex_mixed_input(self):
+        """Test complex input with CSV, repeated flags, whitespace, and case variation"""
+        result = parse_comma_separated(None, None, (" LEASES, online ", "Online", "leases,"), self.allowed_values)
+        assert result == ["leases", "online"]
+
+    def test_no_validation_mode(self):
+        """Test that arbitrary values are accepted when allowed_values=None"""
+        result = parse_comma_separated(None, None, "arbitrary,values,anything", None)
+        assert result == ["arbitrary", "values", "anything"]
+
+    def test_case_normalization_disabled(self):
+        """Test that case normalization can be disabled"""
+        result = parse_comma_separated(None, None, "LEASES,Online", {"LEASES", "Online"}, normalize_case=False)
+        assert result == ["LEASES", "Online"]
+
+
+class TestGetExportersLogic:
+    def create_test_config(self):
+        """Create a mock config for testing"""
+        config = Mock(spec=ClientConfigV1Alpha1)
+        return config
+
+    def create_test_exporters(self, include_leases=False, include_online_status=False):
+        """Create test exporters with optional lease data"""
+        exporters = [
+            Exporter(
+                namespace="default",
+                name="exporter-1",
+                labels={"type": "device", "env": "test"},
+                online=True
+            ),
+            Exporter(
+                namespace="default",
+                name="exporter-2",
+                labels={"type": "server", "env": "prod"},
+                online=False
+            )
+        ]
+
+        if include_leases:
+            # Add lease to first exporter
+            lease = Mock(spec=Lease)
+            lease.client = "test-client"
+            lease.get_status.return_value = "Active"
+            lease.effective_begin_time = Mock()
+            lease.effective_begin_time.strftime.return_value = "2023-01-01 10:00:00"
+            exporters[0].lease = lease
+
+        return ExporterList(
+            exporters=exporters,
+            next_page_token=None,
+            include_online=include_online_status,
+            include_leases=include_leases
+        )
+
+    def test_with_options_parsing_leases(self):
+        """Test that 'leases' in with_options is parsed correctly"""
+        with_options = ("leases",)
+
+        include_leases = "leases" in with_options
+        include_online = "online" in with_options
+
+        assert include_leases is True
+        assert include_online is False
+
+    def test_with_options_parsing_online(self):
+        """Test that 'online' in with_options is parsed correctly"""
+        with_options = ("online",)
+
+        include_leases = "leases" in with_options
+        include_online = "online" in with_options
+
+        assert include_leases is False
+        assert include_online is True
+
+    def test_with_options_parsing_both(self):
+        """Test that both 'leases' and 'online' in with_options are parsed correctly"""
+        with_options = ("leases", "online")
+
+        include_leases = "leases" in with_options
+        include_online = "online" in with_options
+
+        assert include_leases is True
+        assert include_online is True
+
+    def test_with_options_parsing_empty(self):
+        """Test that empty with_options are parsed correctly"""
+        with_options = ()
+
+        include_leases = "leases" in with_options
+        include_online = "online" in with_options
+
+        assert include_leases is False
+        assert include_online is False
+
+    def test_with_options_parsing_unknown(self):
+        """Test that the parse_with function now validates and rejects unknown options"""
+        # This test verifies that the new parse_with function would reject unknown options
+        # The actual CLI behavior now validates input, so unknown options cause failures
+        # This test documents the expected behavior change
+        pass  # Test is no longer relevant since parse_with now validates input
+
+    def test_exporter_list_creation_basic(self):
+        """Test creating ExporterList with basic exporters"""
+        exporters = self.create_test_exporters()
+
+        assert isinstance(exporters, ExporterList)
+        assert len(exporters.exporters) == 2
+        assert exporters.include_online is False
+        assert exporters.include_leases is False
+
+    def test_exporter_list_creation_with_options(self):
+        """Test creating ExporterList with various options"""
+        exporters = self.create_test_exporters(include_leases=True, include_online_status=True)
+
+        assert isinstance(exporters, ExporterList)
+        assert len(exporters.exporters) == 2
+        assert exporters.include_online is True
+        assert exporters.include_leases is True
+
+
+class TestGetExportersIntegration:
+    """Integration tests for data flow"""
+
+    def test_exporter_to_exporter_list_flow(self):
+        """Test the data flow from individual Exporter objects to ExporterList"""
+        # Create individual exporters
+        exporter1 = Exporter(
+            namespace="lab-1",
+            name="rpi-device-001",
+            labels={"device": "raspberry-pi", "location": "rack-1"},
+            online=True
+        )
+        exporter2 = Exporter(
+            namespace="lab-1",
+            name="server-001",
+            labels={"device": "server", "location": "rack-2"},
+            online=False
+        )
+
+        # Create ExporterList
+        exporter_list = ExporterList(
+            exporters=[exporter1, exporter2],
+            next_page_token=None,
+            include_online=True,
+            include_leases=False
+        )
+
+        # Verify the list contains the exporters and has correct options
+        assert len(exporter_list.exporters) == 2
+        assert exporter_list.exporters[0].name == "rpi-device-001"
+        assert exporter_list.exporters[1].name == "server-001"
+        assert exporter_list.include_online is True
+        assert exporter_list.include_leases is False