Merge pull request #7703 from freedomofpress/7639-apiv2-minor-versioning

 feat(`api2`): `Prefer: securedrop=x` limits response shape to minor version `2.x`

Author: legoktm

securedrop/journalist_app/api2/__init__.py

@@ -24,6 +24,29 @@
 PREFIX_MAX_LEN = inspect(Source).columns["uuid"].type.length
 
 
+# Magic numbers to avoid having to define an `IntEnum` somewhere that can be
+# imported from `securedrop.models`:
+#
+# 0. Initial implementation
+# 1. `Index` and `BatchResponse` include `journalists`
+# 2. `Reply` and `Submission` objects include `interaction_count`
+# 3. `BatchRequest` accepts `events` to process, with results returned in
+#    `BatchResponse.events`
+API_MINOR_VERSION = 3  # 2.x
+
+
+def get_request_minor_version() -> int:
+    try:
+        prefer = request.headers.get("Prefer", f"securedrop={API_MINOR_VERSION}")
+        minor_version = int(prefer.split("=")[1])
+        if 0 <= minor_version <= API_MINOR_VERSION:
+            return minor_version
+        else:
+            return API_MINOR_VERSION
+    except (IndexError, ValueError):
+        return API_MINOR_VERSION
+
+
 @blp.get("/index")
 @blp.get("/index/<string:source_prefix>")
 def index(source_prefix: Optional[str] = None) -> Response:
@@ -39,6 +62,7 @@ def index(source_prefix: Optional[str] = None) -> Response:
     the source index into 16 shards.  (Non-source metadata is not filtered by
     the prefix and is always returned.)
     """
+    minor = get_request_minor_version()
     index = Index()
 
     source_query: EagerQuery = eager_query("Source")
@@ -53,16 +77,23 @@ def index(source_prefix: Optional[str] = None) -> Response:
         source_query = source_query.filter(Source.uuid.startswith(source_prefix))
 
     for source in source_query.all():
-        index.sources[source.uuid] = json_version(source.to_api_v2())
+        index.sources[source.uuid] = json_version(source.to_api_v2(minor))
         for item in source.collection:
-            index.items[item.uuid] = json_version(item.to_api_v2())
+            index.items[item.uuid] = json_version(item.to_api_v2(minor))
 
     journalist_query: EagerQuery = eager_query("Journalist")
     for journalist in journalist_query.all():
-        index.journalists[journalist.uuid] = json_version(journalist.to_api_v2())
+        index.journalists[journalist.uuid] = json_version(journalist.to_api_v2(minor))
 
-    version = json_version(asdict(index))
-    response = jsonify(asdict(index))
+    # We want to enforce the *current* shape of `Index`, so we should wait until
+    # we have the dictionary representation to delete top-level keys unsupported
+    # by the current minor version.
+    index_dict = asdict(index)
+    if minor < 1:
+        del index_dict["journalists"]
+
+    version = json_version(index_dict)
+    response = jsonify(index_dict)
 
     # If the request's `If-None-Match` header matches the version,
     # return HTTP 304 with an empty response.
@@ -94,9 +125,12 @@ def data() -> Response:
     except (TypeError, ValueError) as exc:
         abort(422, f"malformed request; {exc}")
 
+    minor = get_request_minor_version()
     response = BatchResponse()
 
-    if requested.events:
+    if minor < 3 and requested.events:
+        abort(400, "Events are not supported for API minor version < 3")
+    if minor >= 3 and requested.events:
         if len(requested.events) > EVENTS_MAX:
             abort(429, f"a BatchRequest MUST NOT include more than {EVENTS_MAX} events")
 
@@ -114,11 +148,11 @@ def data() -> Response:
 
         # Process events in snowflake order.
         for event in sorted(events, key=lambda e: int(e.id)):
-            result = handler.process(event)
+            result = handler.process(event, minor)
             for uuid, source in result.sources.items():
-                response.sources[uuid] = source.to_api_v2() if source is not None else None
+                response.sources[uuid] = source.to_api_v2(minor) if source is not None else None
             for uuid, item in result.items.items():
-                response.items[uuid] = item.to_api_v2() if item is not None else None
+                response.items[uuid] = item.to_api_v2(minor) 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.
@@ -127,7 +161,7 @@ def data() -> Response:
     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()
+            response.sources[source.uuid] = source.to_api_v2(minor)
 
     if requested.items:
         # If an item was explicitly requested but was already emitted by a
@@ -138,7 +172,7 @@ def data() -> Response:
         for item in submission_query.filter(
             Submission.uuid.in_(str(uuid) for uuid in left_to_read)
         ):
-            response.items[item.uuid] = item.to_api_v2()
+            response.items[item.uuid] = item.to_api_v2(minor)
 
         reply_query: EagerQuery = eager_query("Reply")
         for item in reply_query.filter(Reply.uuid.in_(str(uuid) for uuid in left_to_read)):
@@ -147,13 +181,23 @@ def data() -> Response:
                 # `Submission` and `Reply` tables.  This is vanishingly unlikely,
                 # but SQLite can't enforce uniqueness between them.
                 raise MultipleResultsFound(f"found {item.uuid} in both submissions and replies")
-            response.items[item.uuid] = item.to_api_v2()
+            response.items[item.uuid] = item.to_api_v2(minor)
 
     if requested.journalists:
         journalist_query: EagerQuery = eager_query("Journalist")
         for journalist in journalist_query.filter(
             Journalist.uuid.in_(str(uuid) for uuid in requested.journalists)
         ):
-            response.journalists[journalist.uuid] = journalist.to_api_v2()
+            response.journalists[journalist.uuid] = journalist.to_api_v2(minor)
+
+    response_dict = asdict(response)
+
+    # We want to enforce the *current* shape of `BatchResponse`, so we should
+    # wait until we have the dictionary representation to delete top-level keys
+    # unsupported by the current minor version.
+    if minor < 1:
+        del response_dict["journalists"]
+    if minor < 3:
+        del response_dict["events"]
 
-    return jsonify(asdict(response))
+    return jsonify(response_dict)

securedrop/journalist_app/api2/events.py

@@ -41,10 +41,17 @@ class EventHandler:
     """
 
     def __init__(self, session: Session, redis: Redis) -> None:
+        """
+        Configure the `EventHandler`.  Attributes set here are for internal use
+        by the `EventHandler`; handler methods are static and do not have access
+        to them, which means they cannot influence the processing of a given
+        event.
+        """
+
         self._session = session
         self._redis = redis
 
-    def process(self, event: Event) -> EventResult:
+    def process(self, event: Event, minor: int) -> EventResult:
         """The per-event entry-point for handling a single event."""
 
         try:
@@ -73,7 +80,7 @@ def process(self, event: Event) -> EventResult:
             )
 
         self.mark_progress(event)  # prevent races
-        result = handler(event)
+        result = handler(event, minor)
         self.mark_progress(event, result.status[0])  # enforce idempotence
         return result
 
@@ -103,7 +110,7 @@ def mark_progress(
             )
 
     @staticmethod
-    def handle_item_deleted(event: Event) -> EventResult:
+    def handle_item_deleted(event: Event, minor: int) -> EventResult:
         item = find_item(event.target.item_uuid)
         if item is None:
             return EventResult(
@@ -119,7 +126,7 @@ def handle_item_deleted(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_reply_sent(event: Event) -> EventResult:
+    def handle_reply_sent(event: Event, minor: int) -> EventResult:
         try:
             source = Source.query.filter(Source.uuid == event.target.source_uuid).one()
         except NoResultFound:
@@ -142,7 +149,7 @@ def handle_reply_sent(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_source_deleted(event: Event) -> EventResult:
+    def handle_source_deleted(event: Event, minor: int) -> EventResult:
         try:
             source = Source.query.filter(Source.uuid == event.target.source_uuid).one()
         except NoResultFound:
@@ -154,7 +161,7 @@ def handle_source_deleted(event: Event) -> EventResult:
                 ),
             )
 
-        current_version = json_version(source.to_api_v2())
+        current_version = json_version(source.to_api_v2(minor))
         if event.target.version != current_version:
             return EventResult(
                 event_id=event.id,
@@ -176,7 +183,7 @@ def handle_source_deleted(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_source_conversation_deleted(event: Event) -> EventResult:
+    def handle_source_conversation_deleted(event: Event, minor: int) -> EventResult:
         try:
             source = Source.query.filter(Source.uuid == event.target.source_uuid).one()
         except NoResultFound:
@@ -188,7 +195,7 @@ def handle_source_conversation_deleted(event: Event) -> EventResult:
                 ),
             )
 
-        current_version = json_version(source.to_api_v2())
+        current_version = json_version(source.to_api_v2(minor))
         if event.target.version != current_version:
             return EventResult(
                 event_id=event.id,
@@ -212,7 +219,7 @@ def handle_source_conversation_deleted(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_source_starred(event: Event) -> EventResult:
+    def handle_source_starred(event: Event, minor: int) -> EventResult:
         try:
             source = Source.query.filter(Source.uuid == event.target.source_uuid).one()
         except NoResultFound:
@@ -235,7 +242,7 @@ def handle_source_starred(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_source_unstarred(event: Event) -> EventResult:
+    def handle_source_unstarred(event: Event, minor: int) -> EventResult:
         try:
             source = Source.query.filter(Source.uuid == event.target.source_uuid).one()
         except NoResultFound:
@@ -258,7 +265,7 @@ def handle_source_unstarred(event: Event) -> EventResult:
         )
 
     @staticmethod
-    def handle_item_seen(event: Event) -> EventResult:
+    def handle_item_seen(event: Event, minor: int) -> EventResult:
         item = find_item(event.target.item_uuid)
         if item is None:
             return EventResult(

securedrop/models.py

@@ -182,7 +182,7 @@ def public_key(self) -> Optional[str]:
         except GpgKeyNotFoundError:
             return None
 
-    def to_api_v2(self) -> Dict[str, Any]:
+    def to_api_v2(self, minor: int) -> Dict[str, Any]:
         if self.last_updated:
             last_updated = self.last_updated
         else:
@@ -191,7 +191,7 @@ def to_api_v2(self) -> Dict[str, Any]:
         starred = bool(self.star and self.star.starred)
         collection = {}
         for item in self.collection:
-            collection[item.uuid] = item.to_api_v2()
+            collection[item.uuid] = item.to_api_v2(minor)
 
         return {
             "uuid": self.uuid,
@@ -287,7 +287,7 @@ def is_file(self) -> bool:
     def is_message(self) -> bool:
         return self.filename.endswith("msg.gpg")
 
-    def to_api_v2(self) -> Dict[str, Any]:
+    def to_api_v2(self, minor: int) -> Dict[str, Any]:
         if self.is_file:
             seen_by = [f.journalist.uuid for f in self.seen_files if f.journalist]
         else:  # is_message
@@ -297,17 +297,21 @@ def to_api_v2(self) -> Dict[str, Any]:
         # (format: {interaction_count}-{journalist_filename}-*)
         interaction_count = int(self.filename.split("-")[0])
 
-        return {
+        data = {
             "kind": "file" if self.is_file else "message",
             "uuid": self.uuid,
             "source": self.source.uuid,
             "size": self.size,
             # TODO: how is this different from seen_by?
             "is_read": self.seen,
             "seen_by": seen_by,
-            "interaction_count": interaction_count,
         }
 
+        if minor >= 2:
+            data["interaction_count"] = interaction_count
+
+        return data
+
     def to_api_v1(self) -> "Dict[str, Any]":
         seen_by = {
             f.journalist.uuid
@@ -405,22 +409,26 @@ def query_options(cls, base: Optional[Load] = None) -> Tuple[Load, ...]:
             base.joinedload(cls.seen_replies).joinedload(SeenReply.journalist),  # type: ignore[attr-defined]
         )
 
-    def to_api_v2(self) -> Dict[str, Any]:
+    def to_api_v2(self, minor: int) -> Dict[str, Any]:
         # Extract interaction_count from filename
         # (format: {interaction_count}-{journalist_filename}-reply.gpg)
         interaction_count = int(self.filename.split("-")[0])
 
-        return {
+        data = {
             "kind": "reply",
             "uuid": self.uuid,
             "source": self.source.uuid,
             "size": self.size,
             "journalist_uuid": self.journalist.uuid,
             "is_deleted_by_source": self.deleted_by_source,
             "seen_by": [r.journalist.uuid for r in self.seen_replies],
-            "interaction_count": interaction_count,
         }
 
+        if minor >= 2:
+            data["interaction_count"] = interaction_count
+
+        return data
+
     def to_api_v1(self) -> "Dict[str, Any]":
         seen_by = [r.journalist.uuid for r in SeenReply.query.filter(SeenReply.reply_id == self.id)]
         return {
@@ -849,7 +857,7 @@ def to_api_v1(self, all_info: bool = True) -> Dict[str, Any]:
 
         return json_user
 
-    def to_api_v2(self) -> Dict[str, Any]:
+    def to_api_v2(self, minor: int) -> Dict[str, Any]:
         return {
             "username": self.username,
             "uuid": self.uuid,