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):