Fix #1317: fix crash when attachment is deleted

kinto-remote-settings/src/kinto_remote_settings/signer/utils.py

@@ -286,19 +286,25 @@ def records_diff(left, right):
 
 def attachments_size_diff(left, right):
     """
-    Return the size of the new attachments in `left` list of records.
+    Return the net size of the attachments in `left` versus `right`.
     """
-    right_by_location = {
-        r["attachment"]["location"]: r["attachment"]["size"]
-        for r in right
-        if "attachment" in r
+    right_by_id = {r["id"]: r for r in right}
+    right_locations = {
+        r["attachment"]["location"] for r in right if r.get("attachment")
     }
     changed = 0
     for r in left:
-        if "attachment" not in r or r["attachment"]["location"] in right_by_location:
-            # Locations are unique. If present in both, no change.
+        if "attachment" not in r:
             continue
-        changed += r["attachment"]["size"]
+        attachment = r["attachment"]
+        if attachment is None:
+            # Attachment removed: subtract previous size if it existed.
+            previous = right_by_id.get(r["id"])
+            if previous and previous.get("attachment"):
+                changed -= previous["attachment"]["size"]
+        elif attachment["location"] not in right_locations:
+            # New file on storage (locations are unique).
+            changed += attachment["size"]
     return changed
 
 

kinto-remote-settings/tests/signer/test_utils.py

@@ -308,6 +308,24 @@ def test_fetch_cert():
         ([], [], 0),
         # No attachments
         ([{"id": "a"}, {"id": "b"}], [{"id": "c"}], 0),
+        # Deleted attachment
+        (
+            [{"id": "1", "attachment": None}],
+            [{"id": "1", "attachment": {"location": "file1", "size": 100}}],
+            -100,
+        ),
+        # Previously deleted attachment
+        (
+            [{"id": "1", "attachment": {"location": "file1", "size": 100}}],
+            [{"id": "1", "attachment": None}],
+            100,
+        ),
+        # Created and deleted attachment
+        (
+            [{"id": "1", "attachment": None}],
+            [],
+            0,
+        ),
         # No change
         (
             [
@@ -321,37 +339,37 @@ def test_fetch_cert():
         # New file
         (
             [
-                {"attachment": {"location": "file1", "size": 100}},
-                {"attachment": {"location": "file2", "size": 200}},
+                {"id": "1", "attachment": {"location": "file1", "size": 100}},
+                {"id": "2", "attachment": {"location": "file2", "size": 200}},
             ],
             [
-                {"attachment": {"location": "file1", "size": 100}},
+                {"id": "1", "attachment": {"location": "file1", "size": 100}},
             ],
             200,
         ),
         # 2 new attachments
         (
             [
-                {"attachment": {"location": "a", "size": 50}},
-                {"attachment": {"location": "b", "size": 75}},
-                {"attachment": {"location": "c", "size": 25}},
+                {"id": "1", "attachment": {"location": "a", "size": 50}},
+                {"id": "2", "attachment": {"location": "b", "size": 75}},
+                {"id": "3", "attachment": {"location": "c", "size": 25}},
             ],
             [
-                {"attachment": {"location": "a", "size": 50}},
+                {"id": "1", "attachment": {"location": "a", "size": 50}},
             ],
             100,  # b + c
         ),
         (
             [
                 {"id": "1"},
-                {"attachment": {"location": "x", "size": 30}},
+                {"id": "2", "attachment": {"location": "x", "size": 30}},
             ],
             [],
             30,
         ),
         (
             [],
-            [{"attachment": {"location": "a", "size": 1}}],
+            [{"id": "1", "attachment": {"location": "a", "size": 1}}],
             0,
         ),
     ],