From 3b3c87e608316aef0baefba7c4478619aac07b52 Mon Sep 17 00:00:00 2001 From: bob walker Date: Mon, 14 Sep 2026 10:20:42 +0100 Subject: [PATCH] Record an album or tag write immich refused immich answers a bulk album or tag write with HTTP 200 and one result per asset, so a key without album permission produced "filing 0 asset(s)", no recorded failure and exit 0. Refusals other than duplicate now come back from the client and are recorded against the photo and its wiki pages. The album and tag stages also get separate error handling, so a failed album write no longer skips tagging for the run. Co-Authored-By: Claude Fable 5.1 --- rgl-photo-backup | 45 ++++++++++++++++++++++++------ test_rgl_photo_backup.py | 59 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/rgl-photo-backup b/rgl-photo-backup index 06e368b..caa3d30 100755 --- a/rgl-photo-backup +++ b/rgl-photo-backup @@ -1166,13 +1166,23 @@ class Immich: album = self.call("GET", "/albums/%s" % albumid) or {} return {a["id"] for a in album.get("assets", [])} + # + # immich answers a bulk write with one result per id. A duplicate is what + # a second run should see; any other refusal (no_permission, not_found) + # is handed back so the caller can record it against the photo, since + # the request as a whole came back 200. + # def addtoalbum(self, albumid, ids): - added = 0 + added, refused = 0, [] for start in range(0, len(ids), BATCH): results = self.call("PUT", "/albums/%s/assets" % albumid, {"ids": ids[start:start + BATCH]}) or [] - added += sum(1 for r in results if r.get("success")) - return added + for r in results: + if r.get("success"): + added += 1 + elif r.get("error") != "duplicate": + refused.append((r.get("id"), r.get("error") or "refused")) + return (added, refused) # Albums shared with this user are listed too, and one of those could # carry the RGL name, so only the ones this user owns count. @@ -1198,12 +1208,18 @@ class Immich: # is what a second run should see and not a failure def tagassets(self, tagid, ids): tagged = already = 0 + refused = [] for start in range(0, len(ids), BATCH): results = self.call("PUT", "/tags/%s/assets" % tagid, {"ids": ids[start:start + BATCH]}) or [] - tagged += sum(1 for r in results if r.get("success")) - already += sum(1 for r in results if r.get("error") == "duplicate") - return (tagged, already) + for r in results: + if r.get("success"): + tagged += 1 + elif r.get("error") == "duplicate": + already += 1 + else: + refused.append((r.get("id"), r.get("error") or "refused")) + return (tagged, already, refused) def isoutc(epoch): @@ -1326,8 +1342,16 @@ def syncphase(conf, immich): inalbum = immich.albumassets(albumid) missing = [a for a in matched if a not in inalbum] if missing: - say("album %s: filing %d asset(s)" % (ALBUM, immich.addtoalbum(albumid, missing))) + added, refused = immich.addtoalbum(albumid, missing) + say("album %s: filing %d asset(s)" % (ALBUM, added)) + for assetid, why in refused: + side = matched.get(assetid, {}) + failures.add(side.get("key", assetid), sidenodes(side), "album %s: %s" % (ALBUM, why)) + except BackupError as e: + failures.add("album", [], e) + # Tagging gets its own try so a refused album does not also cost the tags + try: wanted = {} for assetid, side in matched.items(): for tag in tagsfor(side): @@ -1338,11 +1362,14 @@ def syncphase(conf, immich): if tag not in ids: failures.add(tag, [], "immich returned no id for this tag") continue - tagged, already = immich.tagassets(ids[tag], wanted[tag]) + tagged, already, refused = immich.tagassets(ids[tag], wanted[tag]) if tagged: say("tag %s: %d tagged, %d already" % (tag, tagged, already)) + for assetid, why in refused: + side = matched.get(assetid, {}) + failures.add(side.get("key", assetid), sidenodes(side), "tag %s: %s" % (tag, why)) except BackupError as e: - failures.add("album/tags", [], e) + failures.add("tags", [], e) for assetid, side in fresh.items(): try: diff --git a/test_rgl_photo_backup.py b/test_rgl_photo_backup.py index 3fc7edc..e2119c3 100644 --- a/test_rgl_photo_backup.py +++ b/test_rgl_photo_backup.py @@ -1119,13 +1119,13 @@ class ImmichClient(TestCase): self.assertEqual(im.albums("u1"), {"RGL": "al1"}) self.assertEqual(im.createalbum("X"), "al2") self.assertEqual(im.albumassets("al1"), {"a1", "a2"}) - self.assertEqual(im.addtoalbum("al1", ["a3", "a1"]), 1) + self.assertEqual(im.addtoalbum("al1", ["a3", "a1"]), (1, [])) def test_tags(self): im = self.client(FakeResponse([{"value": "rgl", "id": "t1"}, {"value": "rgl/licence/x", "id": "t2"}]), FakeResponse([{"success": True}, {"success": False, "error": "duplicate"}])) self.assertEqual(im.tagids(["rgl", "rgl/licence/x"]), {"rgl": "t1", "rgl/licence/x": "t2"}) - self.assertEqual(im.tagassets("t1", ["a1", "a2"]), (1, 1)) + self.assertEqual(im.tagassets("t1", ["a1", "a2"]), (1, 1, [])) def test_tagids_skips_malformed_entries(self): im = self.client(FakeResponse([{"value": "rgl", "id": "t1"}, {"value": "rgl/broken"}])) @@ -1136,10 +1136,30 @@ class ImmichClient(TestCase): im = self.client(FakeResponse([{"success": True}] * 500), FakeResponse([{"success": True}] * 500), FakeResponse([{"success": True}] * 200)) - self.assertEqual(im.addtoalbum("al", ids), 1200) + self.assertEqual(im.addtoalbum("al", ids), (1200, [])) self.assertEqual(len(rb.OPEN.requests), 3) +class ImmichRefusals(TestCase): + def client(self, *responses): + rb.OPEN = FakeOpen(*responses) + return rb.Immich("https://immich.example/api", "KEY") + + def test_album_refusals_are_returned_not_dropped(self): + im = self.client(FakeResponse([{"id": "a1", "success": False, "error": "no_permission"}, + {"id": "a2", "success": False, "error": "duplicate"}, + {"id": "a3", "success": True}])) + self.assertEqual(im.addtoalbum("al1", ["a1", "a2", "a3"]), + (1, [("a1", "no_permission")])) + + def test_tag_refusals_are_returned_not_dropped(self): + im = self.client(FakeResponse([{"id": "a1", "success": False, "error": "not_found"}, + {"id": "a2", "success": False, "error": "duplicate"}, + {"id": "a3", "success": True}])) + self.assertEqual(im.tagassets("t1", ["a1", "a2", "a3"]), + (1, 1, [("a1", "not_found")])) + + class Hashing(TestCase): def test_checksum_is_base64_sha1(self): path = self.write("f", b"hello") @@ -1401,12 +1421,43 @@ class SyncPhase(TestCase): FakeResponse({"assets": {"items": [], "nextPage": None}}), FakeResponse({"id": "a1", "status": "created"}, status=201), # upload 111 http_error(500), # albums + FakeResponse([{"value": "rgl", "id": "t1"}, + {"value": "rgl/licence/cc-by-sa-2.0", "id": "t2"}, + {"value": "rgl/photographer/P", "id": "t3"}]), # tags still run + FakeResponse([{"success": True}]), FakeResponse([{"success": True}]), + FakeResponse([{"success": True}]), FakeResponse({"id": "a1", "exifInfo": {}}), # re-read a1 FakeResponse(b"")) # update a1 failures = rb.syncphase(self.conf(limit=1), rb.Immich("https://i/api", "k")) self.assertEqual(len(failures), 1) - reason = failures.items[0][2] + key, names, reason = failures.items[0] + self.assertEqual(key, "album") self.assertTrue("albums" in reason or "HTTP 500" in reason) + self.assertEqual(rb.OPEN.requests[4].full_url, "https://i/api/tags") + + def test_album_refusal_is_recorded_against_the_photo(self): + rb.OPEN = FakeOpen( + FakeResponse({"id": "u1"}), + FakeResponse({"assets": {"items": [], "nextPage": None}}), + FakeResponse({"id": "a1", "status": "created"}, status=201), # upload 111 + FakeResponse([{"albumName": "RGL", "id": "al1", "ownerId": "u1"}]), + FakeResponse({"assets": []}), + FakeResponse([{"id": "a1", "success": False, "error": "no_permission"}]), + FakeResponse([{"value": "rgl", "id": "t1"}, + {"value": "rgl/licence/cc-by-sa-2.0", "id": "t2"}, + {"value": "rgl/photographer/P", "id": "t3"}]), + FakeResponse([{"id": "a1", "success": False, "error": "no_permission"}]), + FakeResponse([{"success": True}]), FakeResponse([{"success": True}]), + FakeResponse({"id": "a1", "exifInfo": {}}), + FakeResponse(b"")) + failures = rb.syncphase(self.conf(limit=1), rb.Immich("https://i/api", "k")) + self.assertEqual(len(failures), 2) + (k1, names1, r1), (k2, names2, r2) = failures.items + self.assertEqual((k1, names1), ("111", ["A, E1"])) + self.assertIn("album RGL: no_permission", r1) + self.assertEqual((k2, names2), ("111", ["A, E1"])) + self.assertIn("tag rgl: no_permission", r2) + self.assertIn("no_permission", self.stderr.getvalue()) class Run(TestCase):