feat(aincrad): bulk-sync OPAL puzzle PDFs from a local directory

aincrad/tests.py

@@ -1,6 +1,7 @@
 from hashlib import sha256
 
 import pytest
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test.utils import override_settings
 
 from arch.factories import HintFactory, ProblemFactory
@@ -10,7 +11,8 @@
 from dashboard.models import Announcement
 from hanabi.factories import HanabiContestFactory, HanabiPlayerFactory
 from hanabi.models import HanabiParticipation, HanabiReplay
-from opal.factories import OpalPuzzleFactory
+from opal.factories import OpalHuntFactory, OpalPuzzleFactory
+from opal.models import OpalPuzzle
 from payments.factories import PaymentLogFactory
 from roster.factories import (
     InvoiceFactory,
@@ -25,6 +27,10 @@
 TARGET_HASH = sha256(EXAMPLE_PASSWORD.encode("ascii")).hexdigest()
 
 
+def opal_pdf(body: bytes) -> SimpleUploadedFile:
+    return SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 " + body)
+
+
 @pytest.fixture
 def aincrad_setup(db):
     active_semester = SemesterFactory.create(name="New")
@@ -748,9 +754,8 @@ def test_announcement(otis):
 @pytest.mark.django_db
 @override_settings(API_TARGET_HASH=TARGET_HASH)
 def test_opal_handler(otis):
-    OpalPuzzleFactory.create(
-        hunt__slug="teammate", slug="tetrogram", is_metapuzzle=True
-    )
+    hunt = OpalHuntFactory.create(slug="teammate")
+    OpalPuzzleFactory.create(hunt=hunt, slug="tetrogram", is_metapuzzle=True)
     resp = otis.post_20x(
         "api",
         json={
@@ -763,6 +768,27 @@ def test_opal_handler(otis):
     assert puzzle_json["hunt__slug"] == "teammate"
     assert puzzle_json["slug"] == "tetrogram"
     assert puzzle_json["is_metapuzzle"] is True
+    # A puzzle with no PDF yet reports no fingerprint, which is what tells a sync
+    # to upload rather than skip.
+    assert puzzle_json["content"] == ""
+    assert puzzle_json["content_hash"] == ""
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+def test_opal_handler_reports_content_hash(otis):
+    puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram")
+    puzzle.content.save("tetrogram.pdf", opal_pdf(b"first"), save=False)
+    puzzle.content_hash = "a" * 64
+    puzzle.save()
+
+    resp = otis.post_20x(
+        "api",
+        json={"action": "opal_list", "token": EXAMPLE_PASSWORD},
+    )
+    puzzle_json = resp.json()["puzzles"][0]
+    assert puzzle_json["content_hash"] == "a" * 64
+    assert puzzle_json["content"].endswith("tetrogram.pdf")
 
 
 @pytest.mark.django_db
@@ -783,3 +809,151 @@ def test_apply_uuid_handler(otis):
         ).count()
         == 1
     )
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+def test_opal_pdf_upload(otis):
+    puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram")
+    assert puzzle.content_hash == ""
+
+    resp = otis.post_20x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": puzzle.pk,
+            "content": opal_pdf(b"first draft"),
+        },
+    )
+    assert resp.json()["status"] == "created"
+    puzzle.refresh_from_db()
+    assert puzzle.content.read() == b"%PDF-1.4 first draft"
+    assert puzzle.content.name.startswith("opals/teammate/")
+    assert puzzle.content.name.endswith("/tetrogram.pdf")
+    assert puzzle.content_hash == sha256(b"%PDF-1.4 first draft").hexdigest()
+    assert resp.json()["content_hash"] == puzzle.content_hash
+
+    first_name = puzzle.content.name
+
+    # Same bytes again: the server should recognize them and leave storage alone.
+    resp = otis.post_20x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": puzzle.pk,
+            "content": opal_pdf(b"first draft"),
+        },
+    )
+    assert resp.json()["status"] == "unchanged"
+    puzzle.refresh_from_db()
+    assert puzzle.content.name == first_name
+
+    # Different bytes: rewritten in place, with no suffixed duplicate left behind.
+    resp = otis.post_20x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": puzzle.pk,
+            "content": opal_pdf(b"second draft"),
+        },
+    )
+    assert resp.json()["status"] == "updated"
+    puzzle.refresh_from_db()
+    assert puzzle.content.name == first_name
+    assert puzzle.content.read() == b"%PDF-1.4 second draft"
+    assert puzzle.content_hash == sha256(b"%PDF-1.4 second draft").hexdigest()
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+def test_opal_pdf_upload_renamed_file_leaves_no_orphan(otis):
+    puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram")
+    otis.post_20x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": puzzle.pk,
+            "content": SimpleUploadedFile("old_name.pdf", b"%PDF-1.4 draft"),
+        },
+    )
+    puzzle.refresh_from_db()
+    old_name = puzzle.content.name
+    assert puzzle.content.storage.exists(old_name)
+
+    otis.post_20x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": puzzle.pk,
+            "content": SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 draft"),
+        },
+    )
+    puzzle.refresh_from_db()
+    assert puzzle.content.name.endswith("tetrogram.pdf")
+    assert not puzzle.content.storage.exists(old_name)
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+@pytest.mark.parametrize(
+    "upload",
+    (
+        SimpleUploadedFile("tetrogram.tex", b"%PDF-1.4 not really"),
+        SimpleUploadedFile("tetrogram.pdf", b"\\documentclass{article}"),
+    ),
+    ids=("wrong extension", "wrong bytes"),
+)
+def test_opal_pdf_upload_rejects_non_pdf(otis, upload: SimpleUploadedFile):
+    puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram")
+    otis.post_40x(
+        "opal-pdf-upload",
+        data={"token": EXAMPLE_PASSWORD, "pk": puzzle.pk, "content": upload},
+    )
+    puzzle.refresh_from_db()
+    assert not puzzle.content
+    assert puzzle.content_hash == ""
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+def test_opal_pdf_upload_failed_auth(otis):
+    puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram")
+    resp = otis.post_40x(
+        "opal-pdf-upload",
+        data={
+            "token": "this wrong password is not a puzzle",
+            "pk": puzzle.pk,
+            "content": opal_pdf(b"first draft"),
+        },
+    )
+    assert resp.status_code == 418
+    puzzle.refresh_from_db()
+    assert not puzzle.content
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+def test_opal_pdf_upload_unknown_puzzle(otis):
+    otis.post_not_found(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": 1729,
+            "content": opal_pdf(b"first draft"),
+        },
+    )
+    assert not OpalPuzzle.objects.exists()
+
+
+@pytest.mark.django_db
+@override_settings(API_TARGET_HASH=TARGET_HASH)
+@pytest.mark.parametrize("pk", ("", "tetrogram"), ids=("empty", "not a number"))
+def test_opal_pdf_upload_malformed_pk(otis, pk: str):
+    otis.post_40x(
+        "opal-pdf-upload",
+        data={
+            "token": EXAMPLE_PASSWORD,
+            "pk": pk,
+            "content": opal_pdf(b"first draft"),
+        },
+    )

aincrad/urls.py

@@ -2,4 +2,7 @@
 
 from . import views
 
-urlpatterns = [path(r"api/", views.api, name="api")]
+urlpatterns = [
+    path(r"api/", views.api, name="api"),
+    path(r"api/opal-pdf/", views.opal_pdf_upload, name="opal-pdf-upload"),
+]

aincrad/views.py

@@ -1,5 +1,6 @@
 import json
 import logging
+import os
 import string
 from datetime import timedelta
 from decimal import Decimal
@@ -10,6 +11,7 @@
 from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
 from django.core.exceptions import PermissionDenied, SuspiciousOperation
+from django.core.files.uploadhandler import TemporaryFileUploadHandler
 from django.db.models.aggregates import Sum
 from django.db.models.query import QuerySet, prefetch_related_objects
 from django.db.models.query_utils import Q
@@ -24,7 +26,7 @@
 from arch.models import Hint, Problem
 from dashboard.models import Announcement, PSet
 from hanabi.models import HanabiContest, HanabiParticipation, HanabiPlayer, HanabiReplay
-from opal.models import OpalPuzzle
+from opal.models import OpalPuzzle, sha256_of
 from payments.models import Job
 from roster.models import (
     ApplyUUID,
@@ -666,6 +668,7 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse:
                     "order",
                     "num_to_unlock",
                     "content",
+                    "content_hash",
                     "is_metapuzzle",
                     "answer",
                     "partial_answers",
@@ -677,6 +680,90 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse:
     )
 
 
+@csrf_exempt
+def opal_pdf_upload(request: HttpRequest) -> JsonResponse:
+    """Replace one OPAL puzzle's PDF, skipping the write if the bytes are unchanged.
+
+    Separate from `api` because that view is JSON-only (`json.loads(request.body)`),
+    and one puzzle per request because that keeps a partial sync legible: the client
+    gets a per-file verdict instead of one opaque result for the whole batch.
+    """
+    if request.method != "POST":
+        raise PermissionDenied("Must use POST")
+
+    # Swap the upload handler before anything reads request.POST, which is what
+    # populates request.FILES. The project configures memory-only uploads capped at
+    # FILE_UPLOAD_MAX_MEMORY_SIZE, and that handler measures the whole request body:
+    # a PDF over the cap would be discarded with no error at all, leaving
+    # request.FILES empty while the sync script saw a 200. Streaming to disk instead
+    # takes the size limit out of the picture. Safe under @csrf_exempt, which stops
+    # CsrfViewMiddleware from touching request.POST first.
+    request.upload_handlers = [TemporaryFileUploadHandler(request)]
+
+    if (bad_token := reject_bad_token(request.POST.get("token"))) is not None:
+        return bad_token
+
+    # isdigit() rather than leaving it to the ORM, which raises ValueError (a 500)
+    # on a pk that is not a number at all.
+    pk = request.POST.get("pk", "")
+    if not pk.isdigit():
+        raise SuspiciousOperation("No valid puzzle pk provided")
+    puzzle = get_object_or_404(OpalPuzzle, pk=int(pk))
+
+    upload = request.FILES.get("content")
+    if upload is None:
+        raise SuspiciousOperation("No file provided")
+
+    filename = os.path.basename(upload.name or "")
+    if not filename.lower().endswith(".pdf"):
+        return JsonResponse(
+            {"error": f"{filename} is not named like a PDF"}, status=400
+        )
+    # Cheap guard against a sync pointed at the wrong directory, or at a PDF that
+    # never finished being written. The model only validates the extension.
+    header = upload.read(5)
+    upload.seek(0)
+    if header != b"%PDF-":
+        return JsonResponse({"error": f"{filename} is not a PDF"}, status=400)
+
+    digest = sha256_of(upload)
+    # Same bytes under a new name still has to be written, since the storage key
+    # ends in the basename and the old one would otherwise stay the live file.
+    stored_name = os.path.basename(puzzle.content.name or "") if puzzle.content else ""
+    if digest == puzzle.content_hash and stored_name == filename:
+        return JsonResponse(
+            {
+                "status": "unchanged",
+                "pk": puzzle.pk,
+                "hunt": puzzle.hunt.slug,
+                "slug": puzzle.slug,
+                "content": puzzle.content.name,
+                "content_hash": digest,
+            }
+        )
+
+    status = "updated" if puzzle.content else "created"
+    if puzzle.content:
+        # Delete the old blob before writing. The storage key ends in the file's
+        # basename, so a renamed PDF would strand the old one; and locally
+        # FileSystemStorage would suffix the new name rather than overwrite it.
+        puzzle.content.delete(save=False)
+    puzzle.content.save(filename, upload, save=False)
+    puzzle.content_hash = digest
+    puzzle.save()
+
+    return JsonResponse(
+        {
+            "status": status,
+            "pk": puzzle.pk,
+            "hunt": puzzle.hunt.slug,
+            "slug": puzzle.slug,
+            "content": puzzle.content.name,
+            "content_hash": digest,
+        }
+    )
+
+
 def announcement_handler(action: str, data: JSONData) -> JsonResponse:
     del action
     announcement, is_new = Announcement.objects.update_or_create(
@@ -698,6 +785,17 @@ def apply_handler(action: str, data: JSONData) -> JsonResponse:
     return JsonResponse({"pk": au.pk})
 
 
+def reject_bad_token(token: str | None) -> JsonResponse | None:
+    """Return the response to send if `token` is no good, or None if it checks out."""
+    if token is None:
+        raise SuspiciousOperation("No token provided")
+    elif settings.API_TARGET_HASH is None:
+        return JsonResponse({"error": "Not accepting tokens right now"}, status=503)
+    elif sha256(token.encode("ascii")).hexdigest() != settings.API_TARGET_HASH:
+        return JsonResponse({"error": "🧋"}, status=418)
+    return None
+
+
 @csrf_exempt
 def api(request: HttpRequest) -> JsonResponse:
     if not request.method == "POST":
@@ -711,12 +809,8 @@ def api(request: HttpRequest) -> JsonResponse:
         raise SuspiciousOperation("You need to provide an action, silly")
     action = data["action"]
 
-    if "token" not in data:
-        raise SuspiciousOperation("No token provided")
-    elif settings.API_TARGET_HASH is None:
-        return JsonResponse({"error": "Not accepting tokens right now"}, status=503)
-    elif sha256(data["token"].encode("ascii")).hexdigest() != settings.API_TARGET_HASH:
-        return JsonResponse({"error": "🧋"}, status=418)
+    if (bad_token := reject_bad_token(data.get("token"))) is not None:
+        return bad_token
 
     if action in (
         "grade_problem_set",