change: Implement 4: Filter candidates by real format and minimum size from downloaded bytes
Repository Validate / repository-checks (pull_request) Successful in 3s
Repository Validate / repository-checks (pull_request) Successful in 3s
Fix for review-fix on issue #6: non-PNG/ICO hits in the bounded candidate window are now classified as format mismatches instead of access problems, so working searches no longer degrade to `partial_download` (exit 6) or `access_error` (exit 5). Changes: - `tools/images.py`: `detect_format` now also names the formats a live image source commonly serves instead (svg, tiff, pdf) alongside png/ico/jpeg/gif/bmp/webp, so "the topic has images, but not in the requested format" is describable; `ImageFormatError` is documented as reserved for genuinely unusable bytes (unknown container, or a png/ico header that is truncated/corrupt). - `tools/example.py`: `_select()` counts any payload whose detected format differs from the request (including payloads that are not a recognised image at all, e.g. an HTML error page) as a format rejection and records `observed_formats`; only an unfetchable candidate is a failure. `Selection` gained `other_formats`. `_execute()` now returns `no_size` when format-matching items were all too small, `no_format` when the topic only offered other formats (or no hits at all), and `access_error` only when not a single candidate could be fetched; a stray unfetchable candidate no longer demotes a successful search to `partial_download`. Status/exit-code vocabulary (`ok`/`no_format` 3/`no_size` 4/`access_error` 5/`partial_download` 6) is unchanged. - `tests/test_images.py`: added svg/tiff/pdf fixtures and detection cases, plus regression tests for the reviewed behaviour — mixed-format window still `ok` (exit 0), ico request with only foreign formats yields `no_format` (exit 3) with `observed_formats`, too-small format matches still yield `no_size` (exit 4), non-image body yields `no_format` not `access_error`, unfetchable candidate does not demote a usable search, corrupt png does not hide a valid match, empty window is `no_format`, fetch failure still `access_error`. - `tests/test_cli.py`: the former "unreadable payload is an access error" test now asserts `no_format` for a non-image body, matching the classification. Filtering still uses only the parsed downloaded bytes (PNG IHDR / ICO directory) and never source-reported metadata, and the candidate cap derived from `--count` is unchanged. Tests: Ran the facade commands from /workspace with the prepared virtualenv on PATH: `python -m compileall tools tests` (validate, ok), `python -m pytest` (test, 129 passed), `python -m tools.example examples/input.json` (smoke, exit 0, dry_run envelope) and `python -m tools.example --help` (acceptance, exit 0). Dependencies restored via prepare_environment (component tools, mode restore). Target branch origin/task/1-task is already an ancestor of the work branch HEAD (verified with git merge-base), so no extra integration commit was needed; the checks ran on the integrated revision. Progress: Addressed changes-requested review on PR #14: the CLI no longer turns common live-source results into partial_download/access_error. Any payload whose real format is not the requested png/ico (jpeg, gif, bmp, webp, svg, tiff, pdf, or an unrecognised body such as an HTML page) is now a format mismatch feeding no_format, with observed_formats/other_format_candidates detail; only unfetchable candidates are failures, and access_error is emitted only when nothing at all could be fetched. images.detect_format gained svg/tiff/pdf recognition and ImageFormatError is scoped to genuinely unusable png/ico payloads. Statuses/exits unchanged (no_format 3, no_size 4, access_error 5). Tests extended in tests/test_images.py (svg/tiff/pdf detection, mixed-format window stays ok, ico-only-foreign yields no_format, no_size preserved alongside other formats, non-image body is no_format, unfetchable candidate does not demote a search, corrupt png does not hide a match, empty window) and tests/test_cli.py aligned. Facade validate/test/smoke/help all green (129 passed) on the integrated branch; no protected facade paths touched. Published by developer-agent through Tool Backend. Closes #6
This commit was merged in pull request #14.
This commit is contained in:
@@ -18,7 +18,8 @@
|
|||||||
},
|
},
|
||||||
"projects/product-54354138-1#6": {
|
"projects/product-54354138-1#6": {
|
||||||
"events": [
|
"events": [
|
||||||
"Implemented byte-based format and minimum-size filtering for the image search path. New tools/images.py: detect_format (png/ico plus jpeg/gif/bmp/webp so \"other format\" is distinguishable from \"unreadable\"), read_png_dimensions (IHDR), read_ico_dimensions (ICONDIR entries, 0 => 256, largest entry), inspect_image/ImageInfo, ImageFormatError (code image_unreadable), and png_bytes/ico_bytes builders. tools/example.py: candidate_cap(count) bounded window (count*4, max 50) requested from the source, _select() fetches and parses every candidate, KeptImage.to_item() reports source/url/format/actual width+height/size_bytes, _download() writes the already verified bytes, and _execute() distinguishes ok/no_format/no_size/access_error/partial_download with the existing exit codes; metadata-based filtering removed. tests/test_images.py added (31 tests) and tests/test_cli.py fixtures aligned to real image bytes. All facade commands pass: compileall, pytest (114 passed), smoke, help, package import."
|
"Implemented byte-based format and minimum-size filtering for the image search path. New tools/images.py: detect_format (png/ico plus jpeg/gif/bmp/webp so \"other format\" is distinguishable from \"unreadable\"), read_png_dimensions (IHDR), read_ico_dimensions (ICONDIR entries, 0 => 256, largest entry), inspect_image/ImageInfo, ImageFormatError (code image_unreadable), and png_bytes/ico_bytes builders. tools/example.py: candidate_cap(count) bounded window (count*4, max 50) requested from the source, _select() fetches and parses every candidate, KeptImage.to_item() reports source/url/format/actual width+height/size_bytes, _download() writes the already verified bytes, and _execute() distinguishes ok/no_format/no_size/access_error/partial_download with the existing exit codes; metadata-based filtering removed. tests/test_images.py added (31 tests) and tests/test_cli.py fixtures aligned to real image bytes. All facade commands pass: compileall, pytest (114 passed), smoke, help, package import.",
|
||||||
|
"Addressed changes-requested review on PR #14: the CLI no longer turns common live-source results into partial_download/access_error. Any payload whose real format is not the requested png/ico (jpeg, gif, bmp, webp, svg, tiff, pdf, or an unrecognised body such as an HTML page) is now a format mismatch feeding no_format, with observed_formats/other_format_candidates detail; only unfetchable candidates are failures, and access_error is emitted only when nothing at all could be fetched. images.detect_format gained svg/tiff/pdf recognition and ImageFormatError is scoped to genuinely unusable png/ico payloads. Statuses/exits unchanged (no_format 3, no_size 4, access_error 5). Tests extended in tests/test_images.py (svg/tiff/pdf detection, mixed-format window stays ok, ico-only-foreign yields no_format, no_size preserved alongside other formats, non-image body is no_format, unfetchable candidate does not demote a search, corrupt png does not hide a match, empty window) and tests/test_cli.py aligned. Facade validate/test/smoke/help all green (129 passed) on the integrated branch; no protected facade paths touched."
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+11
-5
@@ -557,10 +557,15 @@ def test_resize_downscales_png_stored_inside_ico(
|
|||||||
assert png_size(inner) == (50, 50)
|
assert png_size(inner) == (50, 50)
|
||||||
|
|
||||||
|
|
||||||
def test_unreadable_payload_is_an_access_error(
|
def test_non_image_payload_is_no_format_not_access_error(
|
||||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Bytes that are not an image must never become a "no match" verdict."""
|
"""A body that is not a png really is "not the requested format".
|
||||||
|
|
||||||
|
Live sources answer with HTML error pages, licence documents and vector
|
||||||
|
files; those must produce the topic verdict ``no_format`` (exit 3) instead
|
||||||
|
of an access error or a partial download.
|
||||||
|
"""
|
||||||
|
|
||||||
registry = FakeRegistry(
|
registry = FakeRegistry(
|
||||||
[
|
[
|
||||||
@@ -574,8 +579,9 @@ def test_unreadable_payload_is_an_access_error(
|
|||||||
monkeypatch, capsys, registry, *BASE_ARGS, "--resize", "--download", "--output-dir", str(tmp_path)
|
monkeypatch, capsys, registry, *BASE_ARGS, "--resize", "--download", "--output-dir", str(tmp_path)
|
||||||
)
|
)
|
||||||
|
|
||||||
assert code == envelope.EXIT_ACCESS_ERROR
|
assert code == envelope.EXIT_NO_FORMAT
|
||||||
assert document["status"] == envelope.STATUS_ACCESS_ERROR
|
assert document["status"] == envelope.STATUS_NO_FORMAT
|
||||||
assert document["error"]["code"] == "candidate_unreadable"
|
assert document["error"]["code"] == "no_format_match"
|
||||||
|
assert document["error"]["details"]["other_format_candidates"] == 1
|
||||||
assert document["items"] == []
|
assert document["items"] == []
|
||||||
assert list(tmp_path.iterdir()) == []
|
assert list(tmp_path.iterdir()) == []
|
||||||
|
|||||||
+205
-8
@@ -47,6 +47,33 @@ def gif_payload(width: int = 100, height: int = 100) -> bytes:
|
|||||||
return b"GIF89a" + struct.pack("<HH", width, height) + b"\x00" * 8
|
return b"GIF89a" + struct.pack("<HH", width, height) + b"\x00" * 8
|
||||||
|
|
||||||
|
|
||||||
|
def jpeg_payload() -> bytes:
|
||||||
|
"""A payload whose real format is JPEG, i.e. another format we must not accept."""
|
||||||
|
|
||||||
|
return b"\xff\xd8\xff\xe0" + b"\x00" * 12
|
||||||
|
|
||||||
|
|
||||||
|
def svg_payload(width: int = 100, height: int = 100) -> bytes:
|
||||||
|
"""A real SVG document, the most common non-bitmap hit on a live source."""
|
||||||
|
|
||||||
|
return (
|
||||||
|
'<?xml version="1.0" encoding="UTF-8"?>\n'
|
||||||
|
f'<svg xmlns="http://www.w3.org/2000/svg" width="{width}" height="{height}"></svg>\n'
|
||||||
|
).encode("utf-8")
|
||||||
|
|
||||||
|
|
||||||
|
def tiff_payload() -> bytes:
|
||||||
|
"""A payload whose real format is TIFF."""
|
||||||
|
|
||||||
|
return b"II*\x00" + b"\x00" * 16
|
||||||
|
|
||||||
|
|
||||||
|
def pdf_payload() -> bytes:
|
||||||
|
"""A payload whose real format is PDF, served by document-friendly sources."""
|
||||||
|
|
||||||
|
return b"%PDF-1.7\n%\xe2\xe3\xcf\xd3\n"
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# tools.images: detection
|
# tools.images: detection
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -98,9 +125,14 @@ def test_ico_dimensions_use_the_largest_directory_entry() -> None:
|
|||||||
(png_payload(2, 2), images.PNG),
|
(png_payload(2, 2), images.PNG),
|
||||||
(ico_payload(2, 2), images.ICO),
|
(ico_payload(2, 2), images.ICO),
|
||||||
(gif_payload(), images.GIF),
|
(gif_payload(), images.GIF),
|
||||||
(b"\xff\xd8\xff\xe0" + b"\x00" * 12, images.JPEG),
|
(jpeg_payload(), images.JPEG),
|
||||||
(b"BM" + b"\x00" * 20, images.BMP),
|
(b"BM" + b"\x00" * 20, images.BMP),
|
||||||
(b"RIFF" + b"\x00\x00\x00\x00" + b"WEBP" + b"\x00" * 8, images.WEBP),
|
(b"RIFF" + b"\x00\x00\x00\x00" + b"WEBP" + b"\x00" * 8, images.WEBP),
|
||||||
|
(svg_payload(), images.SVG),
|
||||||
|
(svg_payload().replace(b"<?xml", b"\xef\xbb\xbf<?xml", 1), images.SVG),
|
||||||
|
(tiff_payload(), images.TIFF),
|
||||||
|
(b"MM\x00*" + b"\x00" * 16, images.TIFF),
|
||||||
|
(pdf_payload(), images.PDF),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
def test_detect_format_names_the_real_format(payload: bytes, expected: str) -> None:
|
def test_detect_format_names_the_real_format(payload: bytes, expected: str) -> None:
|
||||||
@@ -112,12 +144,15 @@ def test_detect_format_returns_none_for_non_images(payload: bytes) -> None:
|
|||||||
assert images.detect_format(payload) is None
|
assert images.detect_format(payload) is None
|
||||||
|
|
||||||
|
|
||||||
def test_inspect_image_rejects_formats_whose_size_is_not_readable() -> None:
|
@pytest.mark.parametrize("payload", [gif_payload(), jpeg_payload(), svg_payload(), tiff_payload(), pdf_payload()])
|
||||||
|
def test_inspect_image_rejects_formats_whose_size_is_not_readable(payload: bytes) -> None:
|
||||||
with pytest.raises(ImageFormatError) as excinfo:
|
with pytest.raises(ImageFormatError) as excinfo:
|
||||||
images.inspect_image(gif_payload())
|
images.inspect_image(payload)
|
||||||
|
|
||||||
assert excinfo.value.code == "image_unreadable"
|
assert excinfo.value.code == "image_unreadable"
|
||||||
assert excinfo.value.details["format"] == images.GIF
|
# The format is named, so the CLI can report it as another format rather
|
||||||
|
# than as an access problem.
|
||||||
|
assert excinfo.value.details["format"] == images.detect_format(payload)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(
|
@pytest.mark.parametrize(
|
||||||
@@ -399,18 +434,180 @@ def test_fetch_failure_yields_access_error(
|
|||||||
assert document["error"]["details"]["failures"][0]["url"] == "https://example.test/a.png"
|
assert document["error"]["details"]["failures"][0]["url"] == "https://example.test/a.png"
|
||||||
|
|
||||||
|
|
||||||
def test_unreadable_payload_yields_access_error_not_no_format(
|
def test_non_image_payload_is_no_format_not_access_error(
|
||||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
) -> None:
|
) -> None:
|
||||||
|
"""Bytes that are no image at all are still "not the requested format".
|
||||||
|
|
||||||
|
A live source answers with HTML error pages, licence documents and vector
|
||||||
|
files; none of them is a png, so the topic verdict is ``no_format`` and the
|
||||||
|
caller must not see an access error (or a partial download) instead.
|
||||||
|
"""
|
||||||
|
|
||||||
candidate = StubCandidate(
|
candidate = StubCandidate(
|
||||||
"junk", "https://example.test/a.png", payload=b"not-an-image", mime_type="image/png"
|
"junk", "https://example.test/a.png", payload=b"not-an-image", mime_type="image/png"
|
||||||
)
|
)
|
||||||
|
|
||||||
code, document, _ = run_search(monkeypatch, capsys, [candidate], *BASE_ARGS)
|
code, document, _ = run_search(monkeypatch, capsys, [candidate], *BASE_ARGS)
|
||||||
|
|
||||||
assert code == envelope.EXIT_ACCESS_ERROR
|
assert code == envelope.EXIT_NO_FORMAT
|
||||||
assert document["status"] == envelope.STATUS_ACCESS_ERROR
|
assert document["status"] == envelope.STATUS_NO_FORMAT
|
||||||
assert document["error"]["details"]["failures"][0]["error"]["code"] == "image_unreadable"
|
assert document["items"] == []
|
||||||
|
assert document["error"]["code"] == "no_format_match"
|
||||||
|
details = document["error"]["details"]
|
||||||
|
assert details["other_format_candidates"] == 1
|
||||||
|
assert details["observed_formats"] == {example.UNKNOWN_FORMAT: 1}
|
||||||
|
assert details["unreadable_candidates"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_search_keeps_png_when_the_window_also_serves_other_formats(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""The real source mixes formats; a working search stays ``ok``.
|
||||||
|
|
||||||
|
Every non-png payload in the bounded window used to be classified as an
|
||||||
|
unreadable candidate, which demoted a successful search to
|
||||||
|
``partial_download`` (exit 6). They are format mismatches now.
|
||||||
|
"""
|
||||||
|
|
||||||
|
candidates = [
|
||||||
|
StubCandidate("gif", "https://example.test/a.gif", payload=gif_payload(), mime_type="image/png"),
|
||||||
|
StubCandidate("jpeg", "https://example.test/a.jpg", payload=jpeg_payload(), mime_type="image/png"),
|
||||||
|
StubCandidate("svg", "https://example.test/a.svg", payload=svg_payload(), mime_type="image/png"),
|
||||||
|
StubCandidate("tiff", "https://example.test/a.tiff", payload=tiff_payload(), mime_type="image/png"),
|
||||||
|
StubCandidate("pdf", "https://example.test/a.pdf", payload=pdf_payload(), mime_type="image/png"),
|
||||||
|
StubCandidate("page", "https://example.test/a.html", payload=b"<html>rate limited</html>"),
|
||||||
|
StubCandidate("fox", "https://example.test/fox.png", payload=png_payload(800, 600)),
|
||||||
|
]
|
||||||
|
|
||||||
|
code, document, _ = run_search(
|
||||||
|
monkeypatch, capsys, candidates, *BASE_ARGS, "--width", "640", "--height", "480"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_OK
|
||||||
|
assert document["status"] == envelope.STATUS_OK
|
||||||
|
assert document["error"] is None
|
||||||
|
assert [item["url"] for item in document["items"]] == ["https://example.test/fox.png"]
|
||||||
|
assert (document["items"][0]["width"], document["items"][0]["height"]) == (800, 600)
|
||||||
|
|
||||||
|
|
||||||
|
def test_unfetchable_candidate_does_not_demote_a_usable_search(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""A working search stays ``ok`` even when one candidate cannot be fetched."""
|
||||||
|
|
||||||
|
candidates = [
|
||||||
|
StubCandidate(
|
||||||
|
"missing",
|
||||||
|
"https://example.test/missing.png",
|
||||||
|
error=SourceAccessError("image source returned HTTP status 503"),
|
||||||
|
),
|
||||||
|
StubCandidate("fox", "https://example.test/fox.png", payload=png_payload(300, 200)),
|
||||||
|
]
|
||||||
|
|
||||||
|
code, document, _ = run_search(
|
||||||
|
monkeypatch, capsys, candidates, *BASE_ARGS, "--width", "100", "--height", "100"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_OK
|
||||||
|
assert document["status"] == envelope.STATUS_OK
|
||||||
|
assert [item["url"] for item in document["items"]] == ["https://example.test/fox.png"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_ico_request_with_only_foreign_formats_yields_no_format_not_access_error(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""REQ-007: ico may be unavailable, and that is ``no_format`` (exit 3)."""
|
||||||
|
|
||||||
|
candidates = [
|
||||||
|
StubCandidate("gif", "https://example.test/a.gif", payload=gif_payload()),
|
||||||
|
StubCandidate("jpeg", "https://example.test/a.jpg", payload=jpeg_payload()),
|
||||||
|
StubCandidate("svg", "https://example.test/a.svg", payload=svg_payload()),
|
||||||
|
StubCandidate("tiff", "https://example.test/a.tiff", payload=tiff_payload()),
|
||||||
|
StubCandidate("page", "https://example.test/a.html", payload=b"<html></html>"),
|
||||||
|
]
|
||||||
|
|
||||||
|
code, document, _ = run_search(
|
||||||
|
monkeypatch,
|
||||||
|
capsys,
|
||||||
|
candidates,
|
||||||
|
"--topic",
|
||||||
|
"red fox",
|
||||||
|
"--format",
|
||||||
|
"ico",
|
||||||
|
"--source",
|
||||||
|
"fake",
|
||||||
|
"--width",
|
||||||
|
"64",
|
||||||
|
"--height",
|
||||||
|
"64",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_NO_FORMAT
|
||||||
|
assert document["status"] == envelope.STATUS_NO_FORMAT
|
||||||
|
details = document["error"]["details"]
|
||||||
|
assert details["requested_format"] == "ico"
|
||||||
|
assert details["other_format_candidates"] == 5
|
||||||
|
assert details["observed_formats"] == {
|
||||||
|
"gif": 1,
|
||||||
|
"jpeg": 1,
|
||||||
|
"svg": 1,
|
||||||
|
"tiff": 1,
|
||||||
|
example.UNKNOWN_FORMAT: 1,
|
||||||
|
}
|
||||||
|
assert details["unreadable_candidates"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_size_survives_other_formats_in_the_same_window(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""REQ-008: format-matching but too-small images keep their own status."""
|
||||||
|
|
||||||
|
candidates = [
|
||||||
|
StubCandidate("gif", "https://example.test/a.gif", payload=gif_payload()),
|
||||||
|
StubCandidate("small", "https://example.test/a.png", payload=png_payload(100, 100)),
|
||||||
|
StubCandidate("svg", "https://example.test/a.svg", payload=svg_payload()),
|
||||||
|
]
|
||||||
|
|
||||||
|
code, document, _ = run_search(
|
||||||
|
monkeypatch, capsys, candidates, *BASE_ARGS, "--width", "200", "--height", "200"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_NO_SIZE
|
||||||
|
assert document["status"] == envelope.STATUS_NO_SIZE
|
||||||
|
details = document["error"]["details"]
|
||||||
|
assert details["format_matches"] == 1
|
||||||
|
assert details["other_format_candidates"] == 2
|
||||||
|
assert details["unreadable_candidates"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_corrupt_payload_of_the_requested_format_does_not_hide_a_valid_match(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
corrupt = images.PNG_SIGNATURE + struct.pack(">I", 13) + b"IDAT" + b"\x00" * 13
|
||||||
|
candidates = [
|
||||||
|
StubCandidate("broken", "https://example.test/broken.png", payload=corrupt),
|
||||||
|
StubCandidate("fox", "https://example.test/fox.png", payload=png_payload(200, 200)),
|
||||||
|
]
|
||||||
|
|
||||||
|
code, document, _ = run_search(
|
||||||
|
monkeypatch, capsys, candidates, *BASE_ARGS, "--width", "100", "--height", "100"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_OK
|
||||||
|
assert document["status"] == envelope.STATUS_OK
|
||||||
|
assert [item["url"] for item in document["items"]] == ["https://example.test/fox.png"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_empty_search_window_is_no_format_not_access_error(
|
||||||
|
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
code, document, _ = run_search(monkeypatch, capsys, [], *BASE_ARGS)
|
||||||
|
|
||||||
|
assert code == envelope.EXIT_NO_FORMAT
|
||||||
|
assert document["status"] == envelope.STATUS_NO_FORMAT
|
||||||
|
assert document["error"]["details"]["candidate_count"] == 0
|
||||||
|
assert document["error"]["details"]["unreadable_candidates"] == 0
|
||||||
|
|
||||||
|
|
||||||
def test_candidate_window_is_bounded_by_the_requested_count(
|
def test_candidate_window_is_bounded_by_the_requested_count(
|
||||||
|
|||||||
+64
-49
@@ -17,7 +17,7 @@ import re
|
|||||||
import struct
|
import struct
|
||||||
import sys
|
import sys
|
||||||
import zlib
|
import zlib
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass, field
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any, Mapping, Sequence
|
from typing import Any, Mapping, Sequence
|
||||||
|
|
||||||
@@ -386,11 +386,19 @@ class KeptImage:
|
|||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class Selection:
|
class Selection:
|
||||||
"""Outcome of inspecting a bounded window of downloaded candidates."""
|
"""Outcome of inspecting a bounded window of downloaded candidates.
|
||||||
|
|
||||||
|
``format_rejects`` counts candidates whose real payload is *not* the
|
||||||
|
requested format -- including payloads that are not a recognised image at
|
||||||
|
all. ``size_rejects`` counts candidates of the requested format that are
|
||||||
|
too small. ``failures`` only holds candidates whose bytes could not be
|
||||||
|
fetched at all, because that is the one situation worth an access error.
|
||||||
|
"""
|
||||||
|
|
||||||
kept: tuple[KeptImage, ...] = ()
|
kept: tuple[KeptImage, ...] = ()
|
||||||
format_rejects: int = 0
|
format_rejects: int = 0
|
||||||
size_rejects: int = 0
|
size_rejects: int = 0
|
||||||
|
other_formats: Mapping[str, int] = field(default_factory=dict)
|
||||||
failures: tuple[dict[str, Any], ...] = ()
|
failures: tuple[dict[str, Any], ...] = ()
|
||||||
|
|
||||||
@property
|
@property
|
||||||
@@ -400,16 +408,28 @@ class Selection:
|
|||||||
return len(self.kept) + self.format_rejects + self.size_rejects
|
return len(self.kept) + self.format_rejects + self.size_rejects
|
||||||
|
|
||||||
|
|
||||||
|
#: Bucket name for a downloaded payload that is not a recognised image at all.
|
||||||
|
UNKNOWN_FORMAT = "unknown"
|
||||||
|
|
||||||
|
|
||||||
|
def _count_other_format(other_formats: dict[str, int], detected: str | None) -> None:
|
||||||
|
"""Record which other format a rejected candidate really was."""
|
||||||
|
|
||||||
|
key = detected or UNKNOWN_FORMAT
|
||||||
|
other_formats[key] = other_formats.get(key, 0) + 1
|
||||||
|
|
||||||
|
|
||||||
def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> Selection:
|
def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> Selection:
|
||||||
"""Fetch and inspect candidates, keeping only what the real bytes justify.
|
"""Fetch and inspect candidates, keeping only what the real bytes justify.
|
||||||
|
|
||||||
Every kept item has been downloaded and parsed: its format comes from the
|
Every kept item has been downloaded and parsed: its format comes from the
|
||||||
container signature and its width/height come from the PNG ``IHDR`` or the
|
container signature and its width/height come from the PNG ``IHDR`` or the
|
||||||
ICO directory, never from what the source claimed. A candidate that serves
|
ICO directory, never from what the source claimed. A candidate that serves
|
||||||
other-format bytes counts as a format rejection, a candidate of the wanted
|
other-format bytes -- a JPEG, an SVG, a TIFF, an HTML error page, anything
|
||||||
format that is too small counts as a size rejection, and a candidate whose
|
that is not a real ``png``/``ico`` payload -- counts as a *format rejection*
|
||||||
bytes cannot be fetched or parsed is recorded as a failure instead of being
|
so the caller gets ``no_format`` instead of an access error, and a candidate
|
||||||
silently dropped.
|
of the wanted format that is too small counts as a size rejection. Only a
|
||||||
|
candidate whose bytes cannot be fetched at all is recorded as a failure.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
wanted_format = str(request["format"])
|
wanted_format = str(request["format"])
|
||||||
@@ -417,6 +437,7 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
|||||||
kept: list[KeptImage] = []
|
kept: list[KeptImage] = []
|
||||||
format_rejects = 0
|
format_rejects = 0
|
||||||
size_rejects = 0
|
size_rejects = 0
|
||||||
|
other_formats: dict[str, int] = {}
|
||||||
failures: list[dict[str, Any]] = []
|
failures: list[dict[str, Any]] = []
|
||||||
|
|
||||||
for candidate in candidates:
|
for candidate in candidates:
|
||||||
@@ -433,24 +454,19 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
|||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
detected = images.detect_format(payload)
|
detected = images.detect_format(payload)
|
||||||
if detected is None:
|
|
||||||
failures.append(
|
|
||||||
_failure(
|
|
||||||
candidate,
|
|
||||||
error_fragment(
|
|
||||||
"image_unreadable",
|
|
||||||
"downloaded payload is not a recognised image format",
|
|
||||||
details={"bytes": len(payload)},
|
|
||||||
),
|
|
||||||
)
|
|
||||||
)
|
|
||||||
continue
|
|
||||||
if detected != wanted_format:
|
if detected != wanted_format:
|
||||||
|
# Whatever these bytes are (another image format, or no recognisable
|
||||||
|
# image at all), they are not the requested format: that is a format
|
||||||
|
# verdict about the topic, never an access problem.
|
||||||
format_rejects += 1
|
format_rejects += 1
|
||||||
|
_count_other_format(other_formats, detected)
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
info = images.inspect_image(payload)
|
info = images.inspect_image(payload)
|
||||||
except ImageFormatError as error:
|
except ImageFormatError as error:
|
||||||
|
# The signature claims the requested format but the header is
|
||||||
|
# corrupt, so the payload itself is unusable rather than merely the
|
||||||
|
# wrong format.
|
||||||
failures.append(_failure(candidate, error.to_dict()))
|
failures.append(_failure(candidate, error.to_dict()))
|
||||||
continue
|
continue
|
||||||
if not _meets_minimum_size(info, request):
|
if not _meets_minimum_size(info, request):
|
||||||
@@ -462,6 +478,7 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
|||||||
kept=tuple(kept),
|
kept=tuple(kept),
|
||||||
format_rejects=format_rejects,
|
format_rejects=format_rejects,
|
||||||
size_rejects=size_rejects,
|
size_rejects=size_rejects,
|
||||||
|
other_formats=dict(other_formats),
|
||||||
failures=tuple(failures),
|
failures=tuple(failures),
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -881,8 +898,14 @@ def _execute(argv: Sequence[str]) -> Response:
|
|||||||
requested format;
|
requested format;
|
||||||
* ``no_size`` -- images of the requested format existed, but all of them
|
* ``no_size`` -- images of the requested format existed, but all of them
|
||||||
are smaller than requested;
|
are smaller than requested;
|
||||||
* ``access_error`` -- the source could not be searched or served bytes that
|
* ``access_error`` -- the source could not be searched, or not a single
|
||||||
are not readable images, which is never reported as a "no match".
|
candidate could be fetched, which is never reported as a "no match".
|
||||||
|
|
||||||
|
A candidate whose downloaded bytes are a different format (a JPEG, an SVG,
|
||||||
|
an HTML error page, ...) is a format mismatch about the topic, not an access
|
||||||
|
problem: it feeds ``no_format`` instead of turning a working search into an
|
||||||
|
error, and a stray unfetchable candidate never demotes a usable answer to
|
||||||
|
``partial_download``.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
request, source, registry = _request_from(argv)
|
request, source, registry = _request_from(argv)
|
||||||
@@ -904,17 +927,11 @@ def _execute(argv: Sequence[str]) -> Response:
|
|||||||
"candidate_count": len(candidates),
|
"candidate_count": len(candidates),
|
||||||
"candidates_inspected": selection.inspected,
|
"candidates_inspected": selection.inspected,
|
||||||
"format_matches": selection.size_rejects,
|
"format_matches": selection.size_rejects,
|
||||||
|
"other_format_candidates": selection.format_rejects,
|
||||||
|
"observed_formats": dict(selection.other_formats),
|
||||||
"size_matches": 0,
|
"size_matches": 0,
|
||||||
"unreadable_candidates": len(unreadable),
|
"unreadable_candidates": len(unreadable),
|
||||||
}
|
}
|
||||||
if unreadable:
|
|
||||||
return builder.access_error(
|
|
||||||
error=error_fragment(
|
|
||||||
"candidate_unreadable",
|
|
||||||
"the source served images that could not be read",
|
|
||||||
details={**details, "failures": unreadable},
|
|
||||||
)
|
|
||||||
)
|
|
||||||
if selection.size_rejects:
|
if selection.size_rejects:
|
||||||
return builder.build(
|
return builder.build(
|
||||||
STATUS_NO_SIZE,
|
STATUS_NO_SIZE,
|
||||||
@@ -924,31 +941,29 @@ def _execute(argv: Sequence[str]) -> Response:
|
|||||||
details=details,
|
details=details,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
return builder.build(
|
if selection.format_rejects or not unreadable:
|
||||||
STATUS_NO_FORMAT,
|
# Either the source really had no images in the requested format, or
|
||||||
|
# it had no hits at all; both answer the topic, not an outage.
|
||||||
|
return builder.build(
|
||||||
|
STATUS_NO_FORMAT,
|
||||||
|
error=error_fragment(
|
||||||
|
"no_format_match",
|
||||||
|
f"the source returned no {request['format']} images for {request['topic']!r}",
|
||||||
|
details=details,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
return builder.access_error(
|
||||||
error=error_fragment(
|
error=error_fragment(
|
||||||
"no_format_match",
|
"candidate_unreadable",
|
||||||
f"the source returned no {request['format']} images for {request['topic']!r}",
|
"none of the candidates could be downloaded",
|
||||||
details=details,
|
details={**details, "failures": unreadable},
|
||||||
),
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
if not request["download"]:
|
if not request["download"]:
|
||||||
if unreadable:
|
# Every reported item was fetched and verified against its real bytes, so
|
||||||
return builder.build(
|
# an unfetchable candidate is simply skipped: it must not demote an
|
||||||
STATUS_PARTIAL_DOWNLOAD,
|
# otherwise usable answer to a partial-download exit code.
|
||||||
items=[entry.to_item() for entry in selection.kept],
|
|
||||||
error=error_fragment(
|
|
||||||
"candidate_incomplete",
|
|
||||||
f"{len(unreadable)} of {selection.inspected + len(unreadable)} "
|
|
||||||
"candidates could not be read",
|
|
||||||
details={
|
|
||||||
"failed": len(unreadable),
|
|
||||||
"total": selection.inspected + len(unreadable),
|
|
||||||
"failures": unreadable,
|
|
||||||
},
|
|
||||||
),
|
|
||||||
)
|
|
||||||
return builder.build(STATUS_OK, items=[entry.to_item() for entry in selection.kept])
|
return builder.build(STATUS_OK, items=[entry.to_item() for entry in selection.kept])
|
||||||
|
|
||||||
items, download_failures = _download(selection.kept, request)
|
items, download_failures = _download(selection.kept, request)
|
||||||
|
|||||||
+48
-12
@@ -9,11 +9,12 @@ read the real format and the real pixel dimensions out of those bytes.
|
|||||||
It deliberately depends on nothing but the standard library:
|
It deliberately depends on nothing but the standard library:
|
||||||
|
|
||||||
* :func:`detect_format` recognises the container signatures of the two accepted
|
* :func:`detect_format` recognises the container signatures of the two accepted
|
||||||
formats (``png``, ``ico``) plus a handful of common formats (``jpeg``,
|
formats (``png``, ``ico``) plus the formats a real image source commonly
|
||||||
``gif``, ``bmp``, ``webp``). Recognising the *other* formats is what makes
|
serves instead (``jpeg``, ``gif``, ``bmp``, ``webp``, ``svg``, ``tiff``,
|
||||||
the ``no_format`` outcome possible: "the source has images, but none of them
|
``pdf``). Every recognised-but-different format, and every payload that is
|
||||||
are the format you asked for" is a different answer from "the source has no
|
not a recognised image at all, counts as *another format* rather than an
|
||||||
usable images at all".
|
access problem: "the source has images, but none of them are the format you
|
||||||
|
asked for" is a different answer from "the source could not be used".
|
||||||
* :func:`read_png_dimensions` reads the ``IHDR`` header of a PNG.
|
* :func:`read_png_dimensions` reads the ``IHDR`` header of a PNG.
|
||||||
* :func:`read_ico_dimensions` reads the ICONDIR/ICONDIRENTRY directory of an
|
* :func:`read_ico_dimensions` reads the ICONDIR/ICONDIRENTRY directory of an
|
||||||
ICO file (the directory stores the real sizes, including the ``0``-means-256
|
ICO file (the directory stores the real sizes, including the ``0``-means-256
|
||||||
@@ -22,9 +23,11 @@ It deliberately depends on nothing but the standard library:
|
|||||||
* :func:`png_bytes` and :func:`ico_bytes` *build* the same formats so tests (and
|
* :func:`png_bytes` and :func:`ico_bytes` *build* the same formats so tests (and
|
||||||
callers) can produce real payloads without Pillow or any other dependency.
|
callers) can produce real payloads without Pillow or any other dependency.
|
||||||
|
|
||||||
Every failure raises :class:`ImageFormatError`, a :class:`~tools.errors.ToolkitError`
|
:class:`ImageFormatError` -- a :class:`~tools.errors.ToolkitError` subclass with
|
||||||
subclass whose ``code`` is stable so the CLI can report an unreadable payload as
|
a stable ``code`` -- is reserved for bytes that are genuinely unusable: an
|
||||||
an access problem instead of silently treating it as "no match".
|
unknown container, or a ``png``/``ico`` payload whose header is truncated or
|
||||||
|
corrupt. A readable payload of another format is never an error, so the CLI
|
||||||
|
keeps a working search out of the ``access_error`` status.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@@ -47,8 +50,11 @@ __all__ = [
|
|||||||
"ImageInfo",
|
"ImageInfo",
|
||||||
"JPEG",
|
"JPEG",
|
||||||
"KNOWN_FORMATS",
|
"KNOWN_FORMATS",
|
||||||
|
"PDF",
|
||||||
"PNG",
|
"PNG",
|
||||||
"PNG_SIGNATURE",
|
"PNG_SIGNATURE",
|
||||||
|
"SVG",
|
||||||
|
"TIFF",
|
||||||
"WEBP",
|
"WEBP",
|
||||||
"detect_format",
|
"detect_format",
|
||||||
"ico_bytes",
|
"ico_bytes",
|
||||||
@@ -68,9 +74,15 @@ JPEG = "jpeg"
|
|||||||
GIF = "gif"
|
GIF = "gif"
|
||||||
BMP = "bmp"
|
BMP = "bmp"
|
||||||
WEBP = "webp"
|
WEBP = "webp"
|
||||||
|
#: Vector and document formats a live image source serves instead of a bitmap.
|
||||||
|
#: Naming them lets a search say *which* other formats the topic offered instead
|
||||||
|
#: of degrading into an access problem.
|
||||||
|
SVG = "svg"
|
||||||
|
TIFF = "tiff"
|
||||||
|
PDF = "pdf"
|
||||||
|
|
||||||
#: Every format :func:`detect_format` can name.
|
#: Every format :func:`detect_format` can name.
|
||||||
KNOWN_FORMATS: tuple[str, ...] = (PNG, ICO, JPEG, GIF, BMP, WEBP)
|
KNOWN_FORMATS: tuple[str, ...] = (PNG, ICO, JPEG, GIF, BMP, WEBP, SVG, TIFF, PDF)
|
||||||
|
|
||||||
#: The 8-byte signature every PNG file starts with.
|
#: The 8-byte signature every PNG file starts with.
|
||||||
PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n"
|
PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n"
|
||||||
@@ -88,6 +100,10 @@ _GIF_SIGNATURES = (b"GIF87a", b"GIF89a")
|
|||||||
_BMP_SIGNATURE = b"BM"
|
_BMP_SIGNATURE = b"BM"
|
||||||
_RIFF_SIGNATURE = b"RIFF"
|
_RIFF_SIGNATURE = b"RIFF"
|
||||||
_WEBP_SIGNATURE = b"WEBP"
|
_WEBP_SIGNATURE = b"WEBP"
|
||||||
|
_TIFF_SIGNATURES = (b"II*\x00", b"MM\x00*")
|
||||||
|
_PDF_SIGNATURE = b"%PDF-"
|
||||||
|
#: How many leading bytes are searched for an ``<svg`` root element.
|
||||||
|
_SVG_PROBE = 1024
|
||||||
|
|
||||||
#: Number of payload bytes worth quoting in an error detail.
|
#: Number of payload bytes worth quoting in an error detail.
|
||||||
_SIGNATURE_PROBE = 16
|
_SIGNATURE_PROBE = 16
|
||||||
@@ -131,12 +147,26 @@ def _probe(payload: bytes) -> bytes:
|
|||||||
return bytes(payload[:_SIGNATURE_PROBE])
|
return bytes(payload[:_SIGNATURE_PROBE])
|
||||||
|
|
||||||
|
|
||||||
|
def _looks_like_svg(data: bytes) -> bool:
|
||||||
|
"""True when *data* starts with an SVG root element (or its XML prologue)."""
|
||||||
|
|
||||||
|
head = data[:_SVG_PROBE]
|
||||||
|
if head.startswith(b"\xef\xbb\xbf"):
|
||||||
|
head = head[3:]
|
||||||
|
head = head.lstrip()
|
||||||
|
if head.lower().startswith(b"<svg"):
|
||||||
|
return True
|
||||||
|
return head.startswith(b"<?xml") and b"<svg" in data[:_SVG_PROBE].lower()
|
||||||
|
|
||||||
|
|
||||||
def detect_format(payload: bytes) -> str | None:
|
def detect_format(payload: bytes) -> str | None:
|
||||||
"""Return the real container format of *payload*, or ``None``.
|
"""Return the real container format of *payload*, or ``None``.
|
||||||
|
|
||||||
``None`` means "these bytes are not an image format the toolkit knows", which
|
A recognised format other than ``png``/``ico`` is a perfectly good answer:
|
||||||
is an access/payload problem. A recognised format other than ``png``/``ico``
|
the source does have images, just not in the requested format. ``None``
|
||||||
is a perfectly good answer that the CLI turns into ``no_format``.
|
means the bytes are not a recognised image at all -- still "not the requested
|
||||||
|
format" from the caller's point of view, and never an access problem on its
|
||||||
|
own.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
if not isinstance(payload, (bytes, bytearray, memoryview)):
|
if not isinstance(payload, (bytes, bytearray, memoryview)):
|
||||||
@@ -154,6 +184,12 @@ def detect_format(payload: bytes) -> str | None:
|
|||||||
return BMP
|
return BMP
|
||||||
if data.startswith(_RIFF_SIGNATURE) and data[8:12] == _WEBP_SIGNATURE:
|
if data.startswith(_RIFF_SIGNATURE) and data[8:12] == _WEBP_SIGNATURE:
|
||||||
return WEBP
|
return WEBP
|
||||||
|
if data.startswith(_TIFF_SIGNATURES):
|
||||||
|
return TIFF
|
||||||
|
if data.startswith(_PDF_SIGNATURE):
|
||||||
|
return PDF
|
||||||
|
if _looks_like_svg(data):
|
||||||
|
return SVG
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user