Refactor API endpoints and extract util functions

tildes/openapi_beta.yaml

@@ -57,6 +57,7 @@ paths:
                     $ref: '#/components/schemas/Pagination'
         "400":
           $ref: "#/components/responses/ValidationError"
+
   /topic/{topic_id36}:
     get:
       summary: Get a single topic and its comments
@@ -122,7 +123,6 @@ components:
           type: integer
           minimum: 1
           maximum: 100
-          default: 50
         required: false
         description: The maximum number of items to return. The `limit` is itself limited to prevent abuse.
 
@@ -135,6 +135,14 @@ components:
             type: array
             items:
               $ref: "#/components/schemas/Error"
+    AuthorizationError:
+      description: You are not authorized to perform this action
+      content:
+        application/json:
+          schema:
+            type: array
+            items:
+              $ref: "#/components/schemas/Error"
 
   schemas:
     Topic:
@@ -226,15 +234,8 @@ components:
         - is_removed
         - is_deleted
         - exemplary
-        - collapsed
-        - collapsed_individual
-        - is_op
-        - is_me
-        - is_new
         - voted
         - bookmarked
-        - depth
-        - children
       properties:
         id:
           type: string
@@ -262,8 +263,10 @@ components:
           nullable: true
         collapsed:
           type: boolean
+          nullable: true
         collapsed_individual:
           type: boolean
+          nullable: true
         is_op:
           type: boolean
           nullable: true

tildes/tildes/lib/id.py

@@ -5,6 +5,7 @@
 
 import re
 import string
+from typing import Literal, Tuple
 
 
 ID36_REGEX = re.compile("^[a-z0-9]+$", re.IGNORECASE)
@@ -41,3 +42,20 @@ def id36_to_id(id36_val: str) -> int:
 
     # Python's stdlib can handle this, much simpler in this direction
     return int(id36_val, 36)
+
+
+def split_anchored_id(anchored_id: str) -> Tuple[Literal["comment", "topic"], str]:
+    """Extract the anchor part from an anchored ID."""
+    if not anchored_id or not isinstance(anchored_id, str):
+        raise ValueError("Invalid anchored ID provided")
+
+    type_char, _, id36 = anchored_id.partition("-")
+    if not id36:
+        raise ValueError("Invalid anchored ID provided")
+
+    if type_char == "c":
+        return ("comment", id36)
+    elif type_char == "t":
+        return ("topic", id36)
+    else:
+        raise ValueError(f"Invalid anchored ID type: {type_char}")

tildes/tildes/views/api/beta/api_utils.py

@@ -0,0 +1,86 @@
+# Copyright (c) 2018 Tildes contributors <code@tildes.net>
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""JSON API utils."""
+
+from typing import Tuple
+from pyramid.request import Request
+from pyramid.response import Response
+from tildes.lib.id import split_anchored_id
+from tildes.models.pagination import PaginatedQuery, PaginatedResults
+
+
+def query_apply_pagination(  # noqa
+    query, before, after, error_if_no_anchor: bool = False
+) -> PaginatedQuery:
+    """Apply pagination parameters to a query."""
+    # Start by parsing the before/after parameters and extracting the anchor type
+    # We don't know if the ID has an anchor, so we just try to split it
+    # If it doesn't have an anchor, we just use the ID as is.
+    anchor_type = None
+    if before and after:
+        raise ValueError("Cannot specify both before and after parameters")
+    if before:
+        try:
+            anchor_type, before = split_anchored_id(before)
+        except ValueError as exc:
+            if error_if_no_anchor:
+                raise ValueError(
+                    "Expected an anchored ID for 'before' parameter"
+                ) from exc
+    if after:
+        try:
+            anchor_type, after = split_anchored_id(after)
+        except ValueError as exc:
+            if error_if_no_anchor:
+                raise ValueError(
+                    "Expected an anchored ID for 'after' parameter"
+                ) from exc
+
+    if anchor_type:
+        query = query.anchor_type(anchor_type)
+    if before:
+        query = query.before_id36(before)
+    if after:
+        query = query.after_id36(after)
+    return query
+
+
+def get_next_and_prev_link(request: Request, page: PaginatedResults) -> Tuple[str, str]:
+    """Get the next and previous links for pagination."""
+    next_link = None
+    prev_link = None
+
+    if page.has_next_page:
+        query_vars = request.GET.copy()
+        query_vars.pop("before", None)
+        query_vars.update({"after": page.next_page_after_id36})
+        next_link = request.current_route_url(_query=query_vars)
+
+    if page.has_prev_page:
+        query_vars = request.GET.copy()
+        query_vars.pop("after", None)
+        query_vars.update({"before": page.prev_page_before_id36})
+        prev_link = request.current_route_url(_query=query_vars)
+
+    return (next_link, prev_link)
+
+
+def build_error_response(
+    message: str,
+    status: int = 400,
+    field: str = "N/A",
+    error_type: str = "ValidationError",
+) -> Response:
+    """Build a standardized error response."""
+    return Response(
+        status=status,
+        content_type="application/json",
+        json=[
+            {
+                "message": message,
+                "field": field,
+                "exception": error_type,
+            }
+        ],
+    )

tildes/tildes/views/api/beta/comment.py

@@ -0,0 +1,79 @@
+# Copyright (c) 2018 Tildes contributors <code@tildes.net>
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""JSON API helper functions related to comments."""
+
+from pyramid.request import Request
+from tildes.models.comment import Comment
+from tildes.models.comment.comment_tree import CommentInTree
+
+
+def comment_to_dict(request: Request, comment: Comment) -> dict:
+    """Convert a Comment object to a dictionary for JSON serialization."""
+
+    # Check permissions for viewing comment details (and set safe defaults)
+    author = None
+    rendered_html = None
+    exemplary = None
+    is_op = None
+    is_me = None
+    is_new = None
+    if request.has_permission("view", comment):
+        author = comment.user.username
+        rendered_html = comment.rendered_html
+        exemplary = comment.is_label_active("exemplary")
+        is_me = request.user == comment.user if request.user else False
+        if request.has_permission("view_author", comment.topic):
+            is_op = comment.user == comment.topic.user
+        is_new = (
+            (comment.created_time > comment.topic.last_visit_time)
+            if (
+                hasattr(comment.topic, "last_visit_time")
+                and comment.topic.last_visit_time
+                and not is_me
+            )
+            else False
+        )
+
+    return {
+        "id": comment.comment_id36,
+        "topic_id": comment.topic.topic_id36,
+        "author": author,
+        "rendered_html": rendered_html,
+        "created_at": comment.created_time.isoformat(),
+        "edited_at": (
+            comment.last_edited_time.isoformat() if comment.last_edited_time else None
+        ),
+        "votes": comment.num_votes,
+        "is_removed": comment.is_removed,
+        "is_deleted": comment.is_deleted,
+        "exemplary": exemplary,
+        "collapsed": (
+            (comment.collapsed_state == "full")
+            if hasattr(comment, "collapsed_state")
+            else None
+        ),
+        "collapsed_individual": (
+            (comment.collapsed_state == "individual")
+            if hasattr(comment, "collapsed_state")
+            else None
+        ),
+        "is_op": is_op,
+        "is_me": is_me,
+        "is_new": is_new,
+        "voted": comment.user_voted,
+        "bookmarked": comment.user_bookmarked,
+    }
+
+
+def comment_subtree_to_dict(request: Request, comments: list[CommentInTree]) -> list:
+    """Convert a comment subtree to a list of dictionaries for JSON serialization."""
+    comments_list = []
+    for comment in comments:
+        comment_dict = comment_to_dict(request, comment)
+        comment_dict["depth"] = comment.depth
+        comment_dict["children"] = (
+            comment_subtree_to_dict(request, comment.replies) if comment.replies else []
+        )
+        comments_list.append(comment_dict)
+    return comments_list

tildes/tildes/views/api/beta/topic.py

@@ -5,17 +5,21 @@
 
 from marshmallow.exceptions import ValidationError
 from pyramid.request import Request
-from pyramid.response import Response
 from pyramid.view import view_config
 from tildes.enums import CommentTreeSortOption, TopicSortOption
 from tildes.models.comment import CommentTree, Comment
-from tildes.models.comment.comment_tree import CommentInTree
 from tildes.models.topic import Topic
 from tildes.schemas.fields import ShortTimePeriod
 from tildes.lib.id import id36_to_id
+from tildes.views.api.beta.api_utils import (
+    build_error_response,
+    get_next_and_prev_link,
+    query_apply_pagination,
+)
+from tildes.views.api.beta.comment import comment_subtree_to_dict
 
 
-def _topic_to_dict(topic: Topic) -> dict:
+def topic_to_dict(topic: Topic) -> dict:
     """Convert a Topic object to a dictionary for JSON serialization."""
     return {
         "id": topic.topic_id36,
@@ -41,71 +45,10 @@ def _topic_to_dict(topic: Topic) -> dict:
     }
 
 
-def _comment_to_dict(request: Request, comment: Comment) -> dict:
-    """Convert a Comment object to a dictionary for JSON serialization."""
-
-    # Check permissions for viewing comment details (and set safe defaults)
-    author = None
-    rendered_html = None
-    exemplary = None
-    is_op = None
-    is_me = None
-    is_new = None
-    if request.has_permission("view", comment):
-        author = comment.user.username
-        rendered_html = comment.rendered_html
-        exemplary = comment.is_label_active("exemplary")
-        is_me = request.user == comment.user if request.user else False
-        if request.has_permission("view_author", comment.topic):
-            is_op = comment.user == comment.topic.user
-        is_new = (
-            (comment.created_time > comment.topic.last_visit_time)
-            if (comment.topic.last_visit_time and not is_me)
-            else False
-        )
-
-    return {
-        "id": comment.comment_id36,
-        "topic_id": comment.topic.topic_id36,
-        "author": author,
-        "rendered_html": rendered_html,
-        "created_at": comment.created_time.isoformat(),
-        "edited_at": (
-            comment.last_edited_time.isoformat() if comment.last_edited_time else None
-        ),
-        "votes": comment.num_votes,
-        "is_removed": comment.is_removed,
-        "is_deleted": comment.is_deleted,
-        "exemplary": exemplary,
-        "collapsed": (comment.collapsed_state == "full"),
-        "collapsed_individual": (comment.collapsed_state == "individual"),
-        "is_op": is_op,
-        "is_me": is_me,
-        "is_new": is_new,
-        "voted": comment.user_voted,
-        "bookmarked": comment.user_bookmarked,
-    }
-
-
-def _comment_subtree_to_dict(request: Request, comments: list[CommentInTree]) -> list:
-    """Convert a comment subtree to a list of dictionaries for JSON serialization."""
-    comments_list = []
-    for comment in comments:
-        comment_dict = _comment_to_dict(request, comment)
-        comment_dict["depth"] = comment.depth
-        comment_dict["children"] = (
-            _comment_subtree_to_dict(request, comment.replies)
-            if comment.replies
-            else []
-        )
-        comments_list.append(comment_dict)
-    return comments_list
-
-
 @view_config(route_name="apibeta.topics", openapi=True, renderer="json")
 def get_topics(request: Request) -> dict:  # noqa
     """Get a list of topics (without comments)."""
-    limit = request.openapi_validated.parameters.query.get("limit")
+    limit = request.openapi_validated.parameters.query.get("limit", 50)
     period_raw = request.openapi_validated.parameters.query.get("period")
     tag = request.openapi_validated.parameters.query.get("tag", None)
     order_raw = request.openapi_validated.parameters.query.get("order", None)
@@ -117,13 +60,7 @@ def get_topics(request: Request) -> dict:  # noqa
         period = ShortTimePeriod(allow_none=True)
         period = period.deserialize(period_raw)
     except ValidationError as exc:
-        return Response(
-            status=400,
-            content_type="application/json",
-            json=[
-                {"message": str(exc), "field": "period", "exception": "ValidationError"}
-            ],
-        )
+        return build_error_response(str(exc), field="period")
 
     try:
         if order_raw:
@@ -131,17 +68,7 @@ def get_topics(request: Request) -> dict:  # noqa
         else:
             order = TopicSortOption.ACTIVITY
     except KeyError:
-        return Response(
-            status=400,
-            content_type="application/json",
-            json=[
-                {
-                    "message": f"Invalid order value: {order_raw}",
-                    "field": "order",
-                    "exception": "ValidationError",
-                }
-            ],
-        )
+        return build_error_response(f"Invalid order value: {order_raw}", field="order")
 
     query = request.query(Topic).join_all_relationships().apply_sort_option(order)
 
@@ -155,24 +82,9 @@ def get_topics(request: Request) -> dict:  # noqa
 
     # apply before/after pagination restrictions if relevant
     try:
-        if before and after:
-            raise ValueError("Cannot specify both before and after parameters")
-        if before:
-            query = query.before_id36(before)
-        if after:
-            query = query.after_id36(after)
+        query = query_apply_pagination(query, before, after, error_if_no_anchor=False)
     except ValueError as exc:
-        return Response(
-            status=400,
-            content_type="application/json",
-            json=[
-                {
-                    "message": str(exc),
-                    "field": "before/after",
-                    "exception": "ValidationError",
-                }
-            ],
-        )
+        return build_error_response(str(exc), field="pagination")
 
     # Execute the query
     topics = query.get_page(limit)
@@ -180,25 +92,11 @@ def get_topics(request: Request) -> dict:  # noqa
 
     # Build the JSON topic data
     for topic in topics:
-        processed_topic = _topic_to_dict(topic)
+        processed_topic = topic_to_dict(topic)
         processed_topics.append(processed_topic)
 
     # Construct the paging next and previous link if there are more topics
-    if topics.has_next_page:
-        query_vars = request.GET.copy()
-        query_vars.pop("before", None)
-        query_vars.update({"after": topics.next_page_after_id36})
-        next_link = request.current_route_url(_query=query_vars)
-    else:
-        next_link = None
-
-    if topics.has_prev_page:
-        query_vars = request.GET.copy()
-        query_vars.pop("after", None)
-        query_vars.update({"before": topics.prev_page_before_id36})
-        prev_link = request.current_route_url(_query=query_vars)
-    else:
-        prev_link = None
+    (next_link, prev_link) = get_next_and_prev_link(request, topics)
 
     # Construct the final response JSON object
     response = {
@@ -223,16 +121,9 @@ def get_topic(request: Request) -> dict:
         try:
             comment_order = CommentTreeSortOption[comment_order_raw.upper()]
         except KeyError:
-            return Response(
-                status=400,
-                content_type="application/json",
-                json=[
-                    {
-                        "message": f"Invalid order value: {comment_order_raw}",
-                        "field": "order",
-                        "exception": "ValidationError",
-                    }
-                ],
+            return build_error_response(
+                f"Invalid order value: {comment_order_raw}",
+                field="order",
             )
     else:
         if request.user and request.user.comment_sort_order_default:
@@ -247,17 +138,7 @@ def get_topic(request: Request) -> dict:
         if not topic:
             raise ValueError(f"Topic with ID {topic_id36} not found")
     except ValueError as exc:
-        return Response(
-            status=400,
-            content_type="application/json",
-            json=[
-                {
-                    "message": str(exc),
-                    "field": "topic_id36",
-                    "exception": "ValidationError",
-                }
-            ],
-        )
+        return build_error_response(str(exc), field="topic_id36")
 
     # deleted and removed comments need to be included since they're necessary for
     # building the tree if they have replies
@@ -280,8 +161,8 @@ def get_topic(request: Request) -> dict:
             tree.uncollapse_new_comments(topic.last_visit_time)
             tree.finalize_collapsing_maximized()
 
-    commentsjson = _comment_subtree_to_dict(request, tree.tree)
+    commentsjson = comment_subtree_to_dict(request, tree.tree)
 
     # Construct the final response JSON object
-    response = {"topic": _topic_to_dict(topic), "comments": commentsjson}
+    response = {"topic": topic_to_dict(topic), "comments": commentsjson}
     return response