mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-10 19:17:02 +01:00
tool/tagger.py did not run at all: 48957a1 renamed pack.default_dir to
default_dirs and missed the caller, so the tagger raised ImportError at
module scope through two releases. Nothing could have caught it -- tool/ is
outside --cov and mypy ran with [signal_bot, fake_signal]. Both are widened
here, and tests/test_tool_imports.py imports every script under tool/.
--import was a filename check and shutil.copy2. Pointed at a phone export
that overlaps the album it re-imported the same photo under whatever name
the exporter used that time. It now identifies files by magic bytes, strips
them through the same media_strip path !add_photo uses, and dedups on exact
bytes twice: the source hash, then -- only for what survives -- the hash
after stripping. sources.json records both, so a second run on the same
folder is a no-op. What the album refuses is moved to _skipped/<reason>/
beside the source, never deleted.
Two things that made "strip on import" impossible until now:
media_tags claims -all= zeroes the QuickTime date tags and allows them by
value. It does not; measured, it leaves CreateDate at its real value, and
every mp4 was being refused. -time:all= is what zeroes them.
And a tag allowlist cannot see the worst class at all. IMG_6924.mp4 carries
mebx timed-metadata holding per-frame face bounding boxes and a face id,
and those survive a complete tag strip because they live in mdat as opaque
samples. media_tracks.py drops the track with ffmpeg and verifies with
ffprobe that only video and audio survived. This is not the ffmpeg mistake
media_strip warns about: that was ffmpeg replacing exiftool for tags, this
is track removal with exiftool still doing every tag, and the gate is a
positive read-back rather than an exit code.
The roster was write-once. The setup form only appeared on an empty album,
Album.setup overwrote the file whole, and keys were positional (p{len+1}),
so inserting somebody renumbered everyone after them and repointed every
tags.json entry at the wrong person. Keys now come from the highest number
in use, the form merges, and a person can be added mid-session.
--untagged filters the page and never the server's list. /media/{index} and
/api/tag both index the full album, so a shorter list server-side would
show item i of the album while tagging item i of the filtered set.
Measured on the live album: 174 source files -> 1 not media, 82 already in
pack, 24 duplicate in batch, 67 imported, and re-running imports nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBrE1b9HvFw3xvG9bJiDFt
239 lines
10 KiB
Python
239 lines
10 KiB
Python
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
|
"""Importing a folder: the two-stage dedup, and the arithmetic that proves it."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import TYPE_CHECKING
|
|
|
|
from signal_bot import pack_ledger
|
|
from signal_bot.media_sniff import MediaKind
|
|
from signal_bot.pack import MEDIA_DIR
|
|
from signal_bot.pack_custom import FILE_MODE
|
|
from signal_bot.pack_import import plan_import, run_import
|
|
from signal_bot.pack_report import SKIPPED_DIR, Outcome
|
|
from tests.pack_fixture import CLIP, PIXEL
|
|
|
|
if TYPE_CHECKING:
|
|
from pathlib import Path
|
|
|
|
# A second distinct PNG, so a test can have two files that are genuinely
|
|
# different rather than two names for one.
|
|
OTHER = PIXEL.replace(b"IDAT", b"IDAT", 1) + b"\x00"
|
|
|
|
|
|
class FakeStripper:
|
|
"""Strips by mapping bytes, so no test ever shells out to exiftool."""
|
|
|
|
def __init__(self, mapping: dict[bytes, bytes | None] | None = None) -> None:
|
|
self.mapping = mapping or {}
|
|
|
|
def strip(self, payload: bytes, kind: MediaKind) -> bytes | None:
|
|
"""The mapped output, or the payload unchanged."""
|
|
assert isinstance(kind, MediaKind)
|
|
return self.mapping.get(payload, payload)
|
|
|
|
|
|
def _album(tmp_path: Path, files: dict[str, bytes] | None = None) -> Path:
|
|
directory = tmp_path / "pack"
|
|
media = directory / MEDIA_DIR
|
|
media.mkdir(parents=True)
|
|
for name, payload in (files or {}).items():
|
|
(media / name).write_bytes(payload)
|
|
return directory
|
|
|
|
|
|
def _source(tmp_path: Path, files: dict[str, bytes]) -> Path:
|
|
source = tmp_path / "batch" / "Folder"
|
|
source.mkdir(parents=True)
|
|
for name, payload in files.items():
|
|
(source / name).write_bytes(payload)
|
|
return source
|
|
|
|
|
|
def test_every_file_lands_in_exactly_one_bucket(tmp_path: Path) -> None:
|
|
# The done condition, mechanised: the buckets must sum to what was seen.
|
|
directory = _album(tmp_path, {"have.png": PIXEL})
|
|
source = _source(
|
|
tmp_path,
|
|
{"new.png": OTHER, "same.png": PIXEL, "copy.png": OTHER, "notes.txt": b"hello"},
|
|
)
|
|
report = plan_import(source, directory, FakeStripper())
|
|
assert sum(report.counts.values()) == len(report.decisions) == 4
|
|
assert report.counts == {
|
|
Outcome.NOT_MEDIA: 1,
|
|
Outcome.UNSTRIPPABLE: 0,
|
|
Outcome.ALREADY_IN_PACK: 1,
|
|
Outcome.EXACT_DUP: 1,
|
|
Outcome.IMPORTED: 1,
|
|
}
|
|
assert report.before == 1
|
|
assert report.after == 2
|
|
|
|
|
|
def test_the_report_prints_every_bucket_including_the_empty_ones(tmp_path: Path) -> None:
|
|
# A suppressed zero is a silently missing category; "0 could not be
|
|
# stripped" is how the operator sees the video fix still holding.
|
|
report = plan_import(_source(tmp_path, {}), _album(tmp_path), FakeStripper())
|
|
body = "\n".join(report.lines())
|
|
for outcome in Outcome:
|
|
assert str(outcome) in body or outcome is Outcome.IMPORTED
|
|
assert "could not be stripped" in body
|
|
assert "accounted for" in body
|
|
|
|
|
|
def test_a_file_already_in_the_album_is_not_imported_twice(tmp_path: Path) -> None:
|
|
directory = _album(tmp_path, {"have.png": PIXEL})
|
|
source = _source(tmp_path, {"renamed.png": PIXEL})
|
|
report = plan_import(source, directory, FakeStripper())
|
|
assert report.decisions[0].outcome is Outcome.ALREADY_IN_PACK
|
|
|
|
|
|
def test_two_sources_that_strip_to_one_photo_collapse(tmp_path: Path) -> None:
|
|
# The measured case that a source-hash-only ledger would import twice:
|
|
# the same photo re-encoded, identical once the metadata is gone.
|
|
directory = _album(tmp_path)
|
|
source = _source(tmp_path, {"a.png": PIXEL, "b.png": OTHER})
|
|
stripper = FakeStripper({PIXEL: PIXEL, OTHER: PIXEL})
|
|
report = plan_import(source, directory, stripper)
|
|
outcomes = [d.outcome for d in report.decisions]
|
|
assert outcomes == [Outcome.IMPORTED, Outcome.EXACT_DUP]
|
|
|
|
|
|
def test_a_chat_upload_and_an_exported_copy_collapse(tmp_path: Path) -> None:
|
|
# Same as above but across runs: the album already holds the stripped
|
|
# bytes, and the export folder holds the unstripped original.
|
|
directory = _album(tmp_path, {"hash.png": PIXEL})
|
|
source = _source(tmp_path, {"original.png": OTHER})
|
|
report = plan_import(source, directory, FakeStripper({OTHER: PIXEL}))
|
|
assert report.decisions[0].outcome is Outcome.ALREADY_IN_PACK
|
|
|
|
|
|
def test_a_file_the_stripper_will_not_vouch_for_is_refused(tmp_path: Path) -> None:
|
|
report = plan_import(
|
|
_source(tmp_path, {"a.png": PIXEL}), _album(tmp_path), FakeStripper({PIXEL: None})
|
|
)
|
|
assert report.decisions[0].outcome is Outcome.UNSTRIPPABLE
|
|
|
|
|
|
def test_format_is_decided_by_bytes_not_by_suffix(tmp_path: Path) -> None:
|
|
directory = _album(tmp_path)
|
|
source = _source(tmp_path, {"photo.txt": PIXEL, "fake.jpg": b"not an image at all"})
|
|
report = plan_import(source, directory, FakeStripper())
|
|
by_name = {d.source.name: d for d in report.decisions}
|
|
assert by_name["fake.jpg"].outcome is Outcome.NOT_MEDIA
|
|
imported = by_name["photo.txt"]
|
|
assert imported.outcome is Outcome.IMPORTED
|
|
# The suffix is corrected to match the bytes: pack.media_type looks the
|
|
# stored suffix up in a closed table, and a miss fails the *next* load.
|
|
assert imported.stored is not None
|
|
assert imported.stored.name == "photo.png"
|
|
|
|
|
|
def test_a_suffix_that_agrees_with_the_bytes_is_kept(tmp_path: Path) -> None:
|
|
# .jpeg and .jpg are both image/jpeg, so a .jpeg keeps its name.
|
|
jpeg = b"\xff\xd8\xff" + b"\x00" * 40
|
|
source = _source(tmp_path, {"photo.jpeg": jpeg})
|
|
report = plan_import(source, _album(tmp_path), FakeStripper())
|
|
assert report.decisions[0].stored is not None
|
|
assert report.decisions[0].stored.name == "photo.jpeg"
|
|
|
|
|
|
def test_a_name_collision_disambiguates_rather_than_dropping_a_photo(tmp_path: Path) -> None:
|
|
# The old bug: two exports both saying IMG_0001.jpg silently lost one.
|
|
directory = _album(tmp_path, {"IMG_1.png": PIXEL})
|
|
source = _source(tmp_path, {"IMG_1.png": OTHER})
|
|
report = plan_import(source, directory, FakeStripper())
|
|
assert report.decisions[0].stored is not None
|
|
assert report.decisions[0].stored.name == "IMG_1-2.png"
|
|
|
|
|
|
def test_video_is_imported_like_anything_else(tmp_path: Path) -> None:
|
|
report = plan_import(_source(tmp_path, {"c.mp4": CLIP}), _album(tmp_path), FakeStripper())
|
|
assert report.decisions[0].outcome is Outcome.IMPORTED
|
|
|
|
|
|
def test_a_subdirectory_in_the_source_is_ignored(tmp_path: Path) -> None:
|
|
source = _source(tmp_path, {"a.png": PIXEL})
|
|
(source / "nested").mkdir()
|
|
report = plan_import(source, _album(tmp_path), FakeStripper())
|
|
assert len(report.decisions) == 1
|
|
|
|
|
|
def test_running_the_import_stores_the_new_file_and_the_ledger(tmp_path: Path) -> None:
|
|
directory = _album(tmp_path)
|
|
source = _source(tmp_path, {"a.png": PIXEL})
|
|
run_import(source, directory, FakeStripper())
|
|
stored = directory / MEDIA_DIR / "a.png"
|
|
assert stored.read_bytes() == PIXEL
|
|
assert stored.stat().st_mode & 0o777 == FILE_MODE
|
|
assert pack_ledger.read(directory)["a.png"].source == pack_ledger.digest(PIXEL)
|
|
|
|
|
|
def test_a_rejected_file_is_moved_and_never_deleted(tmp_path: Path) -> None:
|
|
directory = _album(tmp_path, {"have.png": PIXEL})
|
|
source = _source(tmp_path, {"same.png": PIXEL, "notes.txt": b"hello"})
|
|
run_import(source, directory, FakeStripper())
|
|
parent = source.parent
|
|
assert (parent / SKIPPED_DIR / "already-in-pack" / "same.png").read_bytes() == PIXEL
|
|
assert (parent / SKIPPED_DIR / "not-media" / "notes.txt").read_bytes() == b"hello"
|
|
assert not (source / "same.png").exists()
|
|
|
|
|
|
def test_the_skipped_folder_sits_beside_the_source_not_under_home(tmp_path: Path) -> None:
|
|
# Derived from the source argument so a tmp_path source keeps every write
|
|
# inside tmp_path -- conftest redirects XDG_DATA_HOME, not $HOME.
|
|
source = _source(tmp_path, {"notes.txt": b"hello"})
|
|
run_import(source, _album(tmp_path), FakeStripper())
|
|
assert (source.parent / SKIPPED_DIR).is_dir()
|
|
|
|
|
|
def test_importing_the_same_folder_twice_is_a_no_op(tmp_path: Path) -> None:
|
|
# The property the ledger exists for.
|
|
directory = _album(tmp_path)
|
|
source = _source(tmp_path, {"a.png": PIXEL})
|
|
run_import(source, directory, FakeStripper())
|
|
again = _source(tmp_path / "second", {"a.png": PIXEL})
|
|
report = run_import(again, directory, FakeStripper())
|
|
assert report.counts[Outcome.IMPORTED] == 0
|
|
assert report.counts[Outcome.ALREADY_IN_PACK] == 1
|
|
|
|
|
|
def test_an_import_creates_the_album_directory_when_there_is_none(tmp_path: Path) -> None:
|
|
directory = tmp_path / "fresh"
|
|
directory.mkdir()
|
|
report = run_import(_source(tmp_path, {"a.png": PIXEL}), directory, FakeStripper())
|
|
assert (directory / MEDIA_DIR / "a.png").exists()
|
|
assert report.before == 0
|
|
|
|
|
|
def test_the_import_does_not_write_tags(tmp_path: Path) -> None:
|
|
# An imported file is simply absent from tags.json, which is what
|
|
# --untagged keys off and what pack_load already tolerates.
|
|
directory = _album(tmp_path)
|
|
run_import(_source(tmp_path, {"a.png": PIXEL}), directory, FakeStripper())
|
|
assert not (directory / "tags.json").exists()
|
|
|
|
|
|
def test_a_photo_that_arrives_mid_run_is_not_stored_twice(tmp_path: Path) -> None:
|
|
# Planning strips every candidate, which takes minutes on a real batch.
|
|
# A photo uploaded from the chat during that window is in the album by
|
|
# the time the apply pass runs, and storing it anyway puts it there
|
|
# twice: once hash-named by !add_photo, once under its original name.
|
|
# Measured: that happened to 6 photos during the first live import.
|
|
directory = _album(tmp_path)
|
|
source = _source(tmp_path, {"a.png": PIXEL})
|
|
|
|
class Interloper(FakeStripper):
|
|
"""Drops the photo into the album the moment planning reads it."""
|
|
|
|
def strip(self, payload: bytes, kind: MediaKind) -> bytes | None:
|
|
(directory / MEDIA_DIR / "from-the-chat.png").write_bytes(payload)
|
|
return super().strip(payload, kind)
|
|
|
|
report = run_import(source, directory, Interloper())
|
|
assert report.counts[Outcome.IMPORTED] == 0
|
|
assert report.counts[Outcome.ALREADY_IN_PACK] == 1
|
|
assert sorted(p.name for p in (directory / MEDIA_DIR).iterdir()) == ["from-the-chat.png"]
|
|
# And it is moved aside like any other repeat, never left behind.
|
|
assert (source.parent / SKIPPED_DIR / "already-in-pack" / "a.png").exists()
|