Skip to content

Commit bfeff39

Browse files
committed
fix: resolve catalog tooling review gaps
1 parent 43e0f9a commit bfeff39

9 files changed

Lines changed: 311 additions & 72 deletions

File tree

.github/workflows/link-audit.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
- name: Set up Python
2424
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
2525
with:
26-
python-version: "3.12"
26+
python-version-file: ".python-version"
2727
cache: pip
2828

2929
- name: Install Python dependencies

.github/workflows/validate.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
- name: Set up Python
2525
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
2626
with:
27-
python-version: "3.12"
27+
python-version-file: ".python-version"
2828
cache: pip
2929

3030
- name: Install Python dependencies

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ temp/
4545
.tmp/
4646

4747
# Python
48+
.venv/
49+
venv/
4850
__pycache__/
4951
*.pyc
5052

CONTRIBUTING.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@ it for review and provide repeatable evidence.
4949

5050
## Local setup
5151

52-
Install Python 3.12, Ruby, Bundler, and the repository dependencies:
52+
Install the exact Python version from `.python-version`, Ruby, Bundler, and the
53+
repository dependencies:
5354

5455
```bash
5556
python -m venv .venv
5657
. .venv/bin/activate
57-
python -m pip install --upgrade pip
5858
python -m pip install -r requirements-dev.lock.txt
5959
bundle install
6060
```

pyproject.toml

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,3 @@
1-
[build-system]
2-
requires = ["setuptools==80.9.0"]
3-
build-backend = "setuptools.build_meta"
4-
5-
[project]
6-
name = "flypython-catalog-tools"
7-
version = "0.1.0"
8-
description = "Validation and safe link checking for the FlyPython resource catalog"
9-
requires-python = ">=3.12,<3.13"
10-
dependencies = [
11-
"PyYAML==6.0.3",
12-
"requests==2.32.5",
13-
"urllib3==2.6.3",
14-
]
15-
16-
[project.optional-dependencies]
17-
dev = ["pytest==9.1.1"]
18-
19-
[project.scripts]
20-
flypython-check-links = "tools.check_links:main"
21-
flypython-validate-catalog = "tools.validate_catalog:main"
22-
23-
[tool.setuptools]
24-
packages = ["tools"]
25-
261
[tool.pytest.ini_options]
272
addopts = "-q"
283
testpaths = ["tests"]

tests/test_catalog.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from tools.catalog import (
99
CatalogLoadError,
10+
canonical_hostname,
1011
load_catalog,
1112
normalize_url,
1213
validate_catalog,
@@ -32,6 +33,13 @@ def test_loader_rejects_duplicate_yaml_keys(tmp_path) -> None:
3233
load_catalog(catalog)
3334

3435

36+
def test_loader_rejects_non_string_mapping_keys(tmp_path) -> None:
37+
catalog = tmp_path / "resources.yml"
38+
catalog.write_text("? [catalog]\n: {}\nresources: []\n", encoding="utf-8")
39+
with pytest.raises(CatalogLoadError, match="mapping keys must be strings"):
40+
load_catalog(catalog)
41+
42+
3543
def test_schema_duplicate_https_date_and_parity_errors(valid_catalog: dict) -> None:
3644
data = deepcopy(valid_catalog)
3745
data["resources"][0]["url"] = "http://example.invalid/docs"
@@ -71,6 +79,31 @@ def test_normalize_url_preserves_ipv6_brackets(url: str, expected: str) -> None:
7179
assert normalize_url(url) == expected
7280

7381

82+
def test_hostname_canonicalization_handles_idna_and_trailing_dot() -> None:
83+
assert canonical_hostname("BÜCHER.example.") == "xn--bcher-kva.example"
84+
85+
86+
def test_validator_detects_idna_equivalent_duplicate_urls(valid_catalog: dict) -> None:
87+
data = deepcopy(valid_catalog)
88+
data["resources"][0]["url"] = "https://bücher.example/docs/"
89+
data["resources"][1]["url"] = "https://xn--bcher-kva.example/docs"
90+
91+
codes = {
92+
issue.code for issue in validate_catalog(data, today=date(2026, 8, 31))
93+
}
94+
95+
assert "duplicate-url" in codes
96+
97+
98+
def test_validator_reports_non_string_mapping_keys(valid_catalog: dict) -> None:
99+
data = deepcopy(valid_catalog)
100+
data["catalog"][1] = "unexpected"
101+
102+
issues = validate_catalog(data, today=date(2026, 8, 31))
103+
104+
assert any(issue.code == "invalid-key" for issue in issues)
105+
106+
74107
def test_validator_exit_code_for_invalid_catalog(tmp_path) -> None:
75108
catalog = tmp_path / "resources.yml"
76109
catalog.write_text("catalog: {}\nresources: []\n", encoding="utf-8")

tests/test_check_links.py

Lines changed: 114 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,8 @@ def test_unsupported_redirect_status_is_fatal() -> None:
170170

171171
@pytest.mark.parametrize("status_code", [403, 408, 425, 429, 500, 503])
172172
def test_transient_and_access_denied_statuses_need_review(status_code) -> None:
173-
session = FakeSession(FakeResponse(status_code), FakeResponse(status_code))
173+
head = FakeResponse(status_code)
174+
session = FakeSession(head, FakeResponse(status_code))
174175
checker = LinkChecker(
175176
guard=guard_for(),
176177
session_factory=lambda: session,
@@ -179,6 +180,8 @@ def test_transient_and_access_denied_statuses_need_review(status_code) -> None:
179180
min_interval=0,
180181
)
181182
assert checker.check_one(link()).status == "review"
183+
assert len(session.calls) == 1
184+
assert head.closed is True
182185

183186

184187
@pytest.mark.parametrize(
@@ -345,6 +348,7 @@ def head(self, url, **kwargs):
345348
guard=guard_for(),
346349
session_factory=lambda: BrokenSession(FakeResponse(200)),
347350
workers=1,
351+
retries=0,
348352
min_interval=0,
349353
)
350354
result = checker.check_one(link())
@@ -370,6 +374,7 @@ def head(self, url, **kwargs):
370374
guard=guard_for(),
371375
session_factory=lambda: BrokenSession(FakeResponse(200)),
372376
workers=1,
377+
retries=0,
373378
min_interval=0,
374379
)
375380

@@ -385,18 +390,22 @@ def head(self, url, **kwargs):
385390
guard=guard_for(),
386391
session_factory=lambda: SlowSession(FakeResponse(200)),
387392
workers=1,
393+
retries=0,
388394
min_interval=0,
389395
)
390396

391397
assert checker.check_one(link()).status == "review"
392398

393399

394-
def test_retry_configuration_ignores_unbounded_retry_after() -> None:
395-
session = build_session(guard_for(), retries=2, backoff_factor=0.5)
400+
def test_adapter_transport_and_status_retries_are_disabled() -> None:
401+
session = build_session(guard_for())
396402
try:
397403
retry = session.get_adapter("https://").max_retries
398404
assert retry.respect_retry_after_header is False
399405
assert retry.backoff_max == 5.0
406+
assert retry.total == 0
407+
assert retry.connect == 0
408+
assert retry.read == 0
400409
assert retry.status == 0
401410
assert not retry.status_forcelist
402411
finally:
@@ -434,6 +443,108 @@ def head(self, url, **kwargs):
434443
assert second.closed is True
435444

436445

446+
@pytest.mark.parametrize("retry_after", ["60", "9" * 400])
447+
def test_large_retry_after_stops_without_get_fallback(
448+
monkeypatch, retry_after: str
449+
) -> None:
450+
response = FakeResponse(429, headers={"Retry-After": retry_after})
451+
session = FakeSession(response)
452+
checker = LinkChecker(
453+
guard=guard_for(),
454+
session_factory=lambda: session,
455+
workers=1,
456+
retries=2,
457+
min_interval=0,
458+
)
459+
sleeps = []
460+
monkeypatch.setattr(check_links.time, "sleep", sleeps.append)
461+
462+
result = checker.check_one(link())
463+
464+
assert result.status == "review"
465+
assert len(session.calls) == 1
466+
assert sleeps == []
467+
assert response.closed is True
468+
469+
470+
def test_bounded_retry_after_is_honored(monkeypatch) -> None:
471+
first = FakeResponse(429, headers={"Retry-After": "2"})
472+
second = FakeResponse(200)
473+
474+
class RetrySession(FakeSession):
475+
def __init__(self):
476+
super().__init__(first)
477+
self.responses = iter([first, second])
478+
479+
def head(self, url, **kwargs):
480+
self.calls.append(("HEAD", url, kwargs))
481+
return next(self.responses)
482+
483+
session = RetrySession()
484+
checker = LinkChecker(
485+
guard=guard_for(),
486+
session_factory=lambda: session,
487+
workers=1,
488+
retries=1,
489+
min_interval=0,
490+
)
491+
sleeps = []
492+
monkeypatch.setattr(check_links.time, "sleep", sleeps.append)
493+
494+
result = checker.check_one(link())
495+
496+
assert result.status == "working"
497+
assert len(session.calls) == 2
498+
assert sleeps == [2.0]
499+
500+
501+
def test_transport_timeout_retry_is_manual() -> None:
502+
response = FakeResponse(200)
503+
504+
class FlakySession(FakeSession):
505+
def __init__(self):
506+
super().__init__(response)
507+
self.attempt = 0
508+
509+
def head(self, url, **kwargs):
510+
self.calls.append(("HEAD", url, kwargs))
511+
self.attempt += 1
512+
if self.attempt == 1:
513+
raise requests.Timeout("timed out")
514+
return response
515+
516+
session = FlakySession()
517+
checker = LinkChecker(
518+
guard=guard_for(),
519+
session_factory=lambda: session,
520+
workers=1,
521+
retries=1,
522+
backoff_factor=0,
523+
min_interval=0,
524+
)
525+
526+
result = checker.check_one(link())
527+
528+
assert result.status == "working"
529+
assert len(session.calls) == 2
530+
531+
532+
def test_wrapped_timeout_remains_review_needed() -> None:
533+
class WrappedTimeoutSession(FakeSession):
534+
def head(self, url, **kwargs):
535+
raise requests.ConnectionError(TimeoutError("timed out"))
536+
537+
checker = LinkChecker(
538+
guard=guard_for(),
539+
session_factory=lambda: WrappedTimeoutSession(FakeResponse(200)),
540+
workers=1,
541+
retries=0,
542+
min_interval=0,
543+
)
544+
545+
assert checker.check_one(link()).status == "review"
546+
547+
437548
def test_responses_close_when_result_processing_fails(monkeypatch) -> None:
438549
head = FakeResponse(404)
439550
response = FakeResponse(200)

tools/catalog.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ def _construct_unique_mapping(
5555
mapping: dict[Any, Any] = {}
5656
for key_node, value_node in node.value:
5757
key = loader.construct_object(key_node, deep=deep)
58+
if not isinstance(key, str):
59+
raise ConstructorError(
60+
"while constructing a mapping",
61+
node.start_mark,
62+
"mapping keys must be strings",
63+
key_node.start_mark,
64+
)
5865
if key in mapping:
5966
raise ConstructorError(
6067
"while constructing a mapping",
@@ -97,9 +104,15 @@ def load_catalog(path: str | Path) -> dict[str, Any]:
97104
return value
98105

99106

107+
def canonical_hostname(value: str) -> str:
108+
"""Return the lowercase IDNA form used for URL equality and host buckets."""
109+
110+
return value.rstrip(".").encode("idna").decode("ascii").lower()
111+
112+
100113
def normalize_url(value: str) -> str:
101114
parsed = urlsplit(value.strip())
102-
host = (parsed.hostname or "").lower()
115+
host = canonical_hostname(parsed.hostname or "")
103116
port = parsed.port
104117
default_port = (parsed.scheme.lower() == "https" and port == 443) or (
105118
parsed.scheme.lower() == "http" and port == 80
@@ -131,9 +144,19 @@ def _missing_or_unknown(
131144
value: Mapping[str, Any], expected: set[str], location: str
132145
) -> list[ValidationIssue]:
133146
issues: list[ValidationIssue] = []
134-
for key in sorted(expected - set(value)):
147+
string_keys = {key for key in value if isinstance(key, str)}
148+
for key in value:
149+
if not isinstance(key, str):
150+
issues.append(
151+
ValidationIssue(
152+
"invalid-key",
153+
f"{location}[{key!r}]",
154+
"mapping keys must be strings",
155+
)
156+
)
157+
for key in sorted(expected - string_keys):
135158
issues.append(ValidationIssue("missing-field", location, f"missing {key!r}"))
136-
for key in sorted(set(value) - expected):
159+
for key in sorted(string_keys - expected):
137160
issues.append(
138161
ValidationIssue("unknown-field", f"{location}.{key}", "unknown field")
139162
)

0 commit comments

Comments
 (0)