Skip to content

Commit 95d4c08

Browse files
author
root
committed
fix(workflows): normalize catalog priority overrides
1 parent b141978 commit 95d4c08

2 files changed

Lines changed: 169 additions & 41 deletions

File tree

‎src/specify_cli/workflows/catalog.py‎

Lines changed: 67 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,27 @@ class WorkflowCatalogEntry:
5050
description: str = ""
5151

5252

53+
def _normalize_catalog_priority(
54+
value: Any,
55+
*,
56+
catalog_name: str | int,
57+
error_cls: type[Exception],
58+
) -> int:
59+
"""Normalize a catalog priority to int and reject bool explicitly."""
60+
if isinstance(value, bool):
61+
raise error_cls(
62+
f"Invalid priority for catalog '{catalog_name}': "
63+
f"expected integer, got {value!r}"
64+
)
65+
try:
66+
return int(value)
67+
except (TypeError, ValueError) as exc:
68+
raise error_cls(
69+
f"Invalid priority for catalog '{catalog_name}': "
70+
f"expected integer, got {value!r}"
71+
) from exc
72+
73+
5374
# ---------------------------------------------------------------------------
5475
# WorkflowRegistry
5576
# ---------------------------------------------------------------------------
@@ -210,14 +231,11 @@ def _load_catalog_config(
210231
if not url:
211232
continue
212233
self._validate_catalog_url(url)
213-
try:
214-
priority = int(item.get("priority", idx + 1))
215-
except (TypeError, ValueError):
216-
raise WorkflowValidationError(
217-
f"Invalid priority for catalog "
218-
f"'{item.get('name', idx + 1)}': "
219-
f"expected integer, got {item.get('priority')!r}"
220-
)
234+
priority = _normalize_catalog_priority(
235+
item.get("priority", idx + 1),
236+
catalog_name=item.get("name", idx + 1),
237+
error_cls=WorkflowValidationError,
238+
)
221239
raw_install = item.get("install_allowed", False)
222240
if isinstance(raw_install, str):
223241
install_allowed = raw_install.strip().lower() in (
@@ -516,28 +534,33 @@ def add_catalog(
516534
f"Catalog URL already configured: {url}"
517535
)
518536

519-
# Derive priority from the highest existing priority + 1.
520-
# Coerce existing priorities to int with a safe fallback so a user-edited
521-
# workflow-catalogs.yml with a non-integer priority (e.g. "1") doesn't blow up.
522-
def _coerce_priority(value: Any) -> int:
523-
try:
524-
return int(value)
525-
except (TypeError, ValueError):
526-
return 0
527-
528537
max_priority = max(
529538
(
530-
_coerce_priority(cat.get("priority", 0))
531-
for cat in catalogs
539+
_normalize_catalog_priority(
540+
cat["priority"],
541+
catalog_name=cat.get("name", idx + 1),
542+
error_cls=WorkflowValidationError,
543+
)
544+
for idx, cat in enumerate(catalogs)
532545
if isinstance(cat, dict)
546+
if "priority" in cat
533547
),
534548
default=0,
535549
)
550+
catalog_name = name or f"catalog-{len(catalogs) + 1}"
536551
catalogs.append(
537552
{
538-
"name": name or f"catalog-{len(catalogs) + 1}",
553+
"name": catalog_name,
539554
"url": url,
540-
"priority": max_priority + 1 if priority is None else priority,
555+
"priority": (
556+
max_priority + 1
557+
if priority is None
558+
else _normalize_catalog_priority(
559+
priority,
560+
catalog_name=catalog_name,
561+
error_cls=WorkflowValidationError,
562+
)
563+
),
541564
"install_allowed": install_allowed,
542565
"description": description,
543566
}
@@ -832,14 +855,11 @@ def _load_catalog_config(
832855
if not url:
833856
continue
834857
self._validate_catalog_url(url)
835-
try:
836-
priority = int(item.get("priority", idx + 1))
837-
except (TypeError, ValueError):
838-
raise StepValidationError(
839-
f"Invalid priority for catalog "
840-
f"'{item.get('name', idx + 1)}': "
841-
f"expected integer, got {item.get('priority')!r}"
842-
)
858+
priority = _normalize_catalog_priority(
859+
item.get("priority", idx + 1),
860+
catalog_name=item.get("name", idx + 1),
861+
error_cls=StepValidationError,
862+
)
843863
raw_install = item.get("install_allowed", False)
844864
if isinstance(raw_install, str):
845865
install_allowed = raw_install.strip().lower() in (
@@ -1130,27 +1150,33 @@ def add_catalog(
11301150
f"Catalog URL already configured: {url}"
11311151
)
11321152

1133-
# Coerce existing priorities to int with a safe fallback so a user-edited
1134-
# step-catalogs.yml with a non-integer priority (e.g. "1") doesn't blow up.
1135-
def _coerce_priority(value: Any) -> int:
1136-
try:
1137-
return int(value)
1138-
except (TypeError, ValueError):
1139-
return 0
1140-
11411153
max_priority = max(
11421154
(
1143-
_coerce_priority(cat.get("priority", 0))
1144-
for cat in catalogs
1155+
_normalize_catalog_priority(
1156+
cat["priority"],
1157+
catalog_name=cat.get("name", idx + 1),
1158+
error_cls=StepValidationError,
1159+
)
1160+
for idx, cat in enumerate(catalogs)
11451161
if isinstance(cat, dict)
1162+
if "priority" in cat
11461163
),
11471164
default=0,
11481165
)
1166+
catalog_name = name or f"catalog-{len(catalogs) + 1}"
11491167
catalogs.append(
11501168
{
1151-
"name": name or f"catalog-{len(catalogs) + 1}",
1169+
"name": catalog_name,
11521170
"url": url,
1153-
"priority": max_priority + 1 if priority is None else priority,
1171+
"priority": (
1172+
max_priority + 1
1173+
if priority is None
1174+
else _normalize_catalog_priority(
1175+
priority,
1176+
catalog_name=catalog_name,
1177+
error_cls=StepValidationError,
1178+
)
1179+
),
11541180
"install_allowed": install_allowed,
11551181
"description": description,
11561182
}

‎tests/test_workflows.py‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4539,6 +4539,33 @@ def test_add_catalog_accepts_metadata_overrides(self, project_dir):
45394539
"description": "Workflow source",
45404540
}
45414541

4542+
def test_add_catalog_coerces_string_priority_override(self, project_dir):
4543+
from specify_cli.workflows.catalog import WorkflowCatalog
4544+
4545+
catalog = WorkflowCatalog(project_dir)
4546+
catalog.add_catalog(
4547+
"https://example.com/new-catalog.json",
4548+
"my-catalog",
4549+
priority="7",
4550+
)
4551+
4552+
data = yaml.safe_load(
4553+
(project_dir / ".specify" / "workflow-catalogs.yml").read_text()
4554+
)
4555+
assert data["catalogs"][0]["priority"] == 7
4556+
4557+
def test_add_catalog_rejects_bool_priority_override(self, project_dir):
4558+
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
4559+
4560+
catalog = WorkflowCatalog(project_dir)
4561+
4562+
with pytest.raises(WorkflowValidationError, match=r"expected integer, got True"):
4563+
catalog.add_catalog(
4564+
"https://example.com/new-catalog.json",
4565+
"my-catalog",
4566+
priority=True,
4567+
)
4568+
45424569
def test_catalog_add_cli_accepts_metadata_options(self, project_dir, monkeypatch):
45434570
from typer.testing import CliRunner
45444571
from specify_cli import app
@@ -4620,6 +4647,30 @@ def test_load_catalog_config_non_dict_yaml_raises(self, project_dir):
46204647
with pytest.raises(WorkflowValidationError, match="expected a mapping"):
46214648
catalog.get_active_catalogs()
46224649

4650+
def test_load_catalog_config_rejects_bool_priority(self, project_dir):
4651+
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
4652+
4653+
config_path = project_dir / ".specify" / "workflow-catalogs.yml"
4654+
config_path.write_text(
4655+
yaml.dump(
4656+
{
4657+
"catalogs": [
4658+
{
4659+
"name": "custom",
4660+
"url": "https://example.com/wf-catalog.json",
4661+
"priority": True,
4662+
"install_allowed": True,
4663+
}
4664+
]
4665+
}
4666+
),
4667+
encoding="utf-8",
4668+
)
4669+
4670+
catalog = WorkflowCatalog(project_dir)
4671+
with pytest.raises(WorkflowValidationError, match=r"expected integer, got True"):
4672+
catalog.get_active_catalogs()
4673+
46234674
def test_add_catalog_malformed_yaml_raises(self, project_dir):
46244675
"""A malformed YAML config file must raise WorkflowValidationError when adding a catalog."""
46254676
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
@@ -5039,6 +5090,33 @@ def test_add_catalog_accepts_metadata_overrides(self, project_dir):
50395090
"description": "Step source",
50405091
}
50415092

5093+
def test_add_catalog_coerces_string_priority_override(self, project_dir):
5094+
from specify_cli.workflows.catalog import StepCatalog
5095+
5096+
catalog = StepCatalog(project_dir)
5097+
catalog.add_catalog(
5098+
"https://example.com/new-steps.json",
5099+
"my-steps",
5100+
priority="7",
5101+
)
5102+
5103+
data = yaml.safe_load(
5104+
(project_dir / ".specify" / "step-catalogs.yml").read_text()
5105+
)
5106+
assert data["catalogs"][0]["priority"] == 7
5107+
5108+
def test_add_catalog_rejects_bool_priority_override(self, project_dir):
5109+
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
5110+
5111+
catalog = StepCatalog(project_dir)
5112+
5113+
with pytest.raises(StepValidationError, match=r"expected integer, got True"):
5114+
catalog.add_catalog(
5115+
"https://example.com/new-steps.json",
5116+
"my-steps",
5117+
priority=True,
5118+
)
5119+
50425120
def test_catalog_add_cli_accepts_metadata_options(self, project_dir, monkeypatch):
50435121
from typer.testing import CliRunner
50445122
from specify_cli import app
@@ -5167,6 +5245,30 @@ def test_get_catalog_configs(self, project_dir):
51675245
assert configs[0]["name"] == "default"
51685246
assert isinstance(configs[0]["install_allowed"], bool)
51695247

5248+
def test_load_catalog_config_rejects_bool_priority(self, project_dir):
5249+
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
5250+
5251+
config_path = project_dir / ".specify" / "step-catalogs.yml"
5252+
config_path.write_text(
5253+
yaml.dump(
5254+
{
5255+
"catalogs": [
5256+
{
5257+
"name": "custom",
5258+
"url": "https://example.com/step-catalog.json",
5259+
"priority": True,
5260+
"install_allowed": True,
5261+
}
5262+
]
5263+
}
5264+
),
5265+
encoding="utf-8",
5266+
)
5267+
5268+
catalog = StepCatalog(project_dir)
5269+
with pytest.raises(StepValidationError, match=r"expected integer, got True"):
5270+
catalog.get_active_catalogs()
5271+
51705272
def test_search_with_mock_catalog(self, project_dir, monkeypatch):
51715273
from specify_cli.workflows.catalog import StepCatalog
51725274

0 commit comments

Comments
 (0)