Merge pull request #7706 from freedomofpress/7704-written-and-read

fix(`/data`): don't reread items already emitted by processed events

Author: legoktm

securedrop/journalist_app/api2/__init__.py

@@ -121,21 +121,28 @@ def data() -> Response:
                 response.items[uuid] = item.to_api_v2() if item is not None else None
             response.events[result.event_id] = result.status
 
+    # The set of items (UUIDs) that were emitted by processed events.
+    items_emitted = frozenset(response.items.keys())
+
     if requested.sources:
         source_query: EagerQuery = eager_query("Source")
         for source in source_query.filter(Source.uuid.in_(str(uuid) for uuid in requested.sources)):
             response.sources[source.uuid] = source.to_api_v2()
 
     if requested.items:
+        # If an item was explicitly requested but was already emitted by a
+        # processed event, we don't need to (and shouldn't) reread it.
+        left_to_read = set(requested.items) - items_emitted
+
         submission_query: EagerQuery = eager_query("Submission")
         for item in submission_query.filter(
-            Submission.uuid.in_(str(uuid) for uuid in requested.items)
+            Submission.uuid.in_(str(uuid) for uuid in left_to_read)
         ):
             response.items[item.uuid] = item.to_api_v2()
 
         reply_query: EagerQuery = eager_query("Reply")
-        for item in reply_query.filter(Reply.uuid.in_(str(uuid) for uuid in requested.items)):
-            if item.uuid in response.items:
+        for item in reply_query.filter(Reply.uuid.in_(str(uuid) for uuid in left_to_read)):
+            if item.uuid in response.items.keys() - items_emitted:
                 # Fail if we get unlucky and hit a UUID collision between the
                 # `Submission` and `Reply` tables.  This is vanishingly unlikely,
                 # but SQLite can't enforce uniqueness between them.

securedrop/tests/test_journalist_api2.py

@@ -949,3 +949,52 @@ def test_api2_source_conversation_deleted_resubmission(
         )
         assert res3.status_code == 200
         assert res3.json["events"][event.id][0] == 208
+
+
+def test_api2_reply_sent_then_requested_item_is_deduped(
+    journalist_app,
+    journalist_api_token,
+    test_files,
+):
+    """
+    When a reply is created by a `REPLY_SENT` event and the same UUID is also
+    requested in the same `BatchRequest`, the request should succeed (200) and
+    return the reply once.
+    """
+    with journalist_app.test_client() as app:
+        # Fetch an existing reply so that we can resubmit it.
+        source = test_files["source"]
+        reply = test_files["replies"][0]
+        reply_res = app.get(
+            url_for("api.download_reply", source_uuid=source.uuid, reply_uuid=reply.uuid),
+            headers=get_api_headers(journalist_api_token),
+        )
+        reply_ct = reply_res.data
+        armored_ct = ascii_armor(reply_ct)
+
+        # Get current source version to build a valid "reply_sent" event.
+        index = app.get(
+            url_for("api2.index"),
+            headers=get_api_headers(journalist_api_token),
+        )
+        assert index.status_code == 200
+        source_version = index.json["sources"][source.uuid]
+
+        new_reply_uuid = str(uuid.uuid4())
+        event = Event(
+            id="987654321",
+            target=SourceTarget(source_uuid=source.uuid, version=source_version),
+            type=EventType.REPLY_SENT,
+            data={"uuid": new_reply_uuid, "reply": armored_ct},
+        )
+
+        # The same batch both creates the reply (`events`) and requests it (`items`).
+        response = app.post(
+            url_for("api2.data"),
+            json={"events": [asdict(event)], "items": [new_reply_uuid]},
+            headers=get_api_headers(journalist_api_token),
+        )
+        assert response.status_code == 200
+        assert response.json["events"][event.id] == [200, None]
+        assert new_reply_uuid in response.json["items"]
+        assert response.json["items"][new_reply_uuid] is not None