diff --git a/comments/admin.py b/comments/admin.py index 259d64fe20..c5f2017bee 100644 --- a/comments/admin.py +++ b/comments/admin.py @@ -1,8 +1,12 @@ from admin_auto_filters.filters import AutocompleteFilterFactory +from django.conf import settings from django.contrib import admin from django.contrib.postgres.search import SearchQuery +from django.utils.html import format_html -from utils.models import CustomTranslationAdmin +from comments.services.text_archive import get_full_text +from utils.models import CustomTranslationAdmin, uniques_ordered_list +from utils.translation import build_supported_localized_fieldname from .models import Comment, KeyFactor, KeyFactorDriver @@ -30,12 +34,14 @@ class CommentAdmin(CustomTranslationAdmin): "created_at", "is_soft_deleted", "is_private", + "is_text_archived", ] list_filter = [ AutocompleteFilterFactory("Author", "author"), AutocompleteFilterFactory("Post", "on_post"), "is_soft_deleted", "is_private", + "is_text_archived", AutocompleteFilterFactory("Project", "on_project"), ] autocomplete_fields = [ @@ -43,7 +49,7 @@ class CommentAdmin(CustomTranslationAdmin): "on_post", "on_project", ] - readonly_fields = ["included_forecast"] + readonly_fields = ["included_forecast", "is_text_archived"] fields = [ "author", "text", @@ -52,6 +58,7 @@ class CommentAdmin(CustomTranslationAdmin): "is_soft_deleted", "included_forecast", "is_private", + "is_text_archived", ] # `search_fields` must be non-empty for Django admin to render the search box # and dispatch to `get_search_results`, but its contents are unused because we @@ -62,6 +69,55 @@ class CommentAdmin(CustomTranslationAdmin): def should_update_translations(self, obj): return not obj.on_post.is_private() + @admin.display(description="Archived text (read-only, fetched from S3)") + def archived_text(self, obj): + """ + The full text of an archived comment, read back from the archive. + + The admin is where staff investigate a comment, and the row itself + now holds nothing but a 200-character stub. Reading this costs an S3 + round trip per change-page load, which is why `get_fields` only adds + it for rows that are actually archived. + """ + + text = get_full_text(obj) + + if text is None: + return format_html( + "{}", + "The archived text could not be retrieved from S3. " + "Only the stub above remains in the database.", + ) + + return format_html( + '
{}
', + text, + ) + + def get_fields(self, request, obj=None): + fields = list(super().get_fields(request, obj)) + + if obj and obj.is_text_archived: + fields.append("archived_text") + + return uniques_ordered_list(fields) + + def get_readonly_fields(self, request, obj=None): + readonly_fields = list(super().get_readonly_fields(request, obj)) + + if obj and obj.is_text_archived: + # Only a stub of the text is left in the db, so editing it here + # would bypass the `update_comment` guard and leave the row out of + # sync with the archived original. `archived_text` is not a model + # field at all, so it has to be declared read-only to appear. + readonly_fields += ["text", "archived_text"] + [ + build_supported_localized_fieldname("text", lang) + for lang, _label in settings.LANGUAGES + ] + + return uniques_ordered_list(readonly_fields) + def get_search_results(self, request, queryset, search_term): search_term = search_term.strip() if not search_term: diff --git a/comments/management/commands/archive_bot_comment_texts.py b/comments/management/commands/archive_bot_comment_texts.py new file mode 100644 index 0000000000..709bf77f74 --- /dev/null +++ b/comments/management/commands/archive_bot_comment_texts.py @@ -0,0 +1,180 @@ +import time +from collections.abc import Callable + +from django.conf import settings +from django.core.management.base import BaseCommand, CommandError + +from comments.services.text_archive import ( + ARCHIVE_AGE_DAYS, + ARCHIVE_MIN_TEXT_LENGTH, + ARCHIVE_STUB_LENGTH, + DEFAULT_BATCH_SIZE, + DEFAULT_CONCURRENCY, + S3_KEY_PREFIX, + ArchiveStats, + archive_bot_comment_texts, + check_is_enabled, +) + + +def format_duration(seconds: float) -> str: + seconds = int(seconds) + hours, remainder = divmod(seconds, 3600) + minutes, seconds = divmod(remainder, 60) + + if hours: + return f"{hours}h{minutes:02d}m" + if minutes: + return f"{minutes}m{seconds:02d}s" + + return f"{seconds}s" + + +class ProgressWriter: + """ + Prints a running one-line summary with a rate and an ETA. + + The daily run has only a day of new comments to clear, but the first + backfill works through hundreds of thousands of rows over hours, so the + point is to make a long run observable rather than to look pretty. Output + is one line per batch, not a redrawn line, so it survives being piped to a + log file. + """ + + def __init__(self, stdout, total: int = 0): + self.stdout = stdout + self.total = total + self.started = time.monotonic() + + @property + def elapsed(self) -> float: + return time.monotonic() - self.started + + def write(self, line: str) -> None: + self.stdout.write(line) + self.stdout.flush() + + def update(self, done: int, summary: str, detail: str = "") -> None: + elapsed = self.elapsed + rate = done / elapsed if elapsed else 0 + percent = (done / self.total * 100) if self.total else 0 + remaining = max(self.total - done, 0) + eta = format_duration(remaining / rate) if rate else "?" + + line = ( + f" {done:,}/{self.total:,} ({percent:.1f}%) {summary} " + f"{rate:.1f}/s elapsed {format_duration(elapsed)} eta {eta}" + ) + + if detail: + line += f" [{detail}]" + + self.write(line) + + +class Command(BaseCommand): + help = ( + "Moves the full text of private bot comments older than " + f"{ARCHIVE_AGE_DAYS} days and longer than {ARCHIVE_MIN_TEXT_LENGTH} " + f"characters to S3, leaving a {ARCHIVE_STUB_LENGTH}-character stub in " + "the database. Runs daily as a cron job; the full text stays " + "readable through the comment-detail endpoint." + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Report what would be archived without writing to S3 or the database", + ) + parser.add_argument( + "--limit", + type=int, + default=None, + help="Maximum number of comments to archive (useful for the first backfill)", + ) + parser.add_argument( + "--batch-size", + type=int, + default=DEFAULT_BATCH_SIZE, + help=f"Comments per database update (default: {DEFAULT_BATCH_SIZE})", + ) + parser.add_argument( + "--concurrency", + type=int, + default=DEFAULT_CONCURRENCY, + help=( + "Uploads to keep in flight at once. S3 has no multi-object PUT, " + "so this is what makes a large backfill finish in minutes " + f"rather than hours (default: {DEFAULT_CONCURRENCY})" + ), + ) + + def handle(self, *args, **options): + dry_run = options["dry_run"] + + if not check_is_enabled(): + raise CommandError( + "AWS_STORAGE_BUCKET_COMMENTS_TEXT is not configured, " + "comment text archiving is disabled." + ) + + progress = ProgressWriter(self.stdout) + on_progress: Callable[[ArchiveStats], None] | None = None + + if not dry_run: + progress.write( + f"Archiving to {settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT}/" + f"{S3_KEY_PREFIX}/ with concurrency {options['concurrency']}, " + f"batches of {options['batch_size']}" + ) + progress.write("Finding eligible comments...") + + def write_progress(stats: ArchiveStats) -> None: + progress.total = stats.total + detail = ", ".join( + f"{count} {label}" + for label, count in ( + ("failed", stats.failed), + ("skipped", stats.skipped), + ) + if count + ) + progress.update( + stats.archived + stats.failed + stats.skipped, + f"{stats.chars_reclaimed:,} chars reclaimed", + detail, + ) + + on_progress = write_progress + + stats = archive_bot_comment_texts( + dry_run=dry_run, + limit=options["limit"], + batch_size=options["batch_size"], + concurrency=options["concurrency"], + on_progress=on_progress, + ) + + verb = "Would archive" if dry_run else "Archived" + elapsed = "" if dry_run else f" in {format_duration(progress.elapsed)}" + progress.write( + f"{verb} {stats.archived:,} comment(s), " + f"reclaiming {stats.chars_reclaimed:,} characters{elapsed}" + ) + + if stats.sample_ids: + sample = ", ".join(str(pk) for pk in stats.sample_ids) + progress.write(f"Sample comment ids: {sample}") + + if stats.skipped: + self.stdout.write( + self.style.WARNING( + f"Skipped {stats.skipped} comment(s) edited during the run" + ) + ) + + if stats.failed: + self.stdout.write( + self.style.ERROR(f"Failed to upload {stats.failed} comment(s)") + ) diff --git a/comments/migrations/0027_comment_is_text_archived.py b/comments/migrations/0027_comment_is_text_archived.py new file mode 100644 index 0000000000..1f84979835 --- /dev/null +++ b/comments/migrations/0027_comment_is_text_archived.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.15 on 2026-08-19 17:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('comments', '0026_comment_key_factor_votes_score'), + ] + + operations = [ + migrations.AddField( + model_name='comment', + name='is_text_archived', + field=models.BooleanField(db_index=True, default=False, editable=False, help_text='True if the full text has been moved to S3 and only a truncated stub remains in the text columns. Archived comments cannot be edited; use the comment-detail endpoint to read them.'), + ), + ] diff --git a/comments/models.py b/comments/models.py index c35836561b..3ca2ca808c 100644 --- a/comments/models.py +++ b/comments/models.py @@ -99,6 +99,18 @@ class Comment(TimeStampedModel, TranslatedModel): is_soft_deleted = models.BooleanField(default=False, db_index=True) # Some comments with KeyFactors can have empty text text = models.TextField(max_length=150_000, blank=True) + # Set by the `archive_bot_comment_texts` command. The full text lives in S3 + # under a key derived from the comment id, so no pointer is stored here. + # example path: + # /s3/buckets/metaculus-web-content-blobs?prefix=comments_text/.json + is_text_archived = models.BooleanField( + default=False, + editable=False, + help_text="True if the full text has been moved to S3 and only a " + "truncated stub remains in the text columns. Archived comments " + "cannot be edited; use the comment-detail endpoint to read them.", + db_index=True, + ) on_post = models.ForeignKey( Post, models.CASCADE, null=True, related_name="comments" ) diff --git a/comments/serializers/common.py b/comments/serializers/common.py index 4d435c4f42..cdde435628 100644 --- a/comments/serializers/common.py +++ b/comments/serializers/common.py @@ -5,6 +5,7 @@ from rest_framework import serializers from rest_framework.exceptions import ValidationError +from authentication.auth import FallbackTokenAuthentication from comments.constants import TimeWindow from comments.models import Comment, KeyFactor, CommentsOfTheWeekEntry from comments.utils import comments_extract_user_mentions_mapping @@ -15,6 +16,7 @@ from users.models import User from users.serializers import BaseUserSerializer from utils.dtypes import flatten, generate_map_from_list +from utils.frontend import build_frontend_url from .key_factors import serialize_key_factors_many, KeyFactorWriteSerializer @@ -76,6 +78,7 @@ class Meta: "text_edited_at", "is_soft_deleted", "text", + "is_text_archived", "on_post", "on_post_data", "included_forecast", @@ -102,7 +105,42 @@ def get_changed_my_mind(self, comment: Comment) -> dict[str, bool | int]: } def get_text(self, value: Comment): - return _("deleted") if value.is_soft_deleted else value.text + if value.is_soft_deleted: + return _("deleted") + + if not value.is_text_archived or not self._reader_is_an_api_client(): + return value.text + + url = build_frontend_url(f"/api/comments/{value.id}/") + + return ( + f"{value.text}... \n\n### WARNING: Content Truncated" + "\nThis comment has been archived to save space." + f"\nTo retrieve full content, please visit [{url}]({url})" + ) + + def _reader_is_an_api_client(self) -> bool: + """ + Whether the truncation notice has to be spelled out in the text itself. + + The web front end renders a "load the rest" button off + `is_text_archived`, so a notice in the body would sit right next to the + button that already does the job. A script has no such affordance, so + it gets the pointer inline. + + An API key is the thing the front end never uses: web users arrive + through `SessionJWTAuthentication` or a session cookie, while keys are + documented as bot-only (see `DEFAULT_AUTHENTICATION_CLASSES`). Absent a + request — internal callers that serialize without one — nothing is + added, since there is no client to inform. + """ + + request = self.context.get("request") + + return isinstance( + getattr(request, "successful_authenticator", None), + FallbackTokenAuthentication, + ) def get_on_post_data(self, value: Comment): """ @@ -181,10 +219,11 @@ def serialize_comment( mentions: list[User] | None = None, author_staff_permission: ObjectPermission = None, key_factors: list[KeyFactor] = None, + request=None, ) -> dict: mentions = mentions or [] serialized_data = CommentSerializer( - comment, context={"current_user": current_user} + comment, context={"current_user": current_user, "request": request} ).data # Permissions @@ -211,6 +250,7 @@ def serialize_comment_many( comments: QuerySet[Comment] | list[Comment], current_user: User | None = None, with_key_factors: bool = False, + request=None, ) -> list[dict]: current_user = ( current_user if current_user and current_user.is_authenticated else None @@ -258,6 +298,7 @@ def serialize_comment_many( post_staff_users_map.get(comment.on_post, {}).get(comment.author_id) ), key_factors=comment_key_factors_map.get(comment.id), + request=request, ) for comment in objects ] diff --git a/comments/services/common.py b/comments/services/common.py index 418f0bf6e6..547cd6cc22 100644 --- a/comments/services/common.py +++ b/comments/services/common.py @@ -1,6 +1,7 @@ import datetime import difflib from collections import defaultdict +import logging from django.db import IntegrityError, transaction from django.db.models import ( @@ -37,6 +38,8 @@ from users.models import User from ..tasks import run_on_post_comment_create +logger = logging.getLogger(__name__) + spam_error = ValidationError( detail="This comment seems to be spam. Please contact " "support@metaculus.com if you believe this was a mistake.", @@ -187,6 +190,17 @@ def perform_create_comment( def update_comment( comment: Comment, text: str = None, included_forecast: Forecast = None ): + if comment.is_text_archived: + # Only a stub of the text remains in the db, so we can neither diff + # against it nor let it be overwritten + logger.info( + f"Attempt to update archived comment {comment.id} by " + f"user {comment.author_id}" + ) + raise ValidationError( + "This comment's text has been archived and can no longer be edited." + ) + differ = difflib.Differ() diff = list(differ.compare(comment.text.splitlines(), text.splitlines())) diff --git a/comments/services/text_archive.py b/comments/services/text_archive.py new file mode 100644 index 0000000000..6be79f40fe --- /dev/null +++ b/comments/services/text_archive.py @@ -0,0 +1,375 @@ +import json +import logging +from collections.abc import Callable +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass, field +from datetime import timedelta + +from botocore.config import Config +from django.conf import settings +from django.core.serializers.json import DjangoJSONEncoder +from django.db.models import Count, Q, QuerySet, Sum, TextField, Value +from django.db.models.functions import Coalesce, Length, NullIf, Substr +from django.utils import timezone + +from comments.models import Comment +from utils.aws import get_boto_client +from utils.translation import build_supported_localized_fieldname + +logger = logging.getLogger(__name__) + +# Comments older than this are eligible for archiving +ARCHIVE_AGE_DAYS = 30 +# Only archive comments whose text is longer than this. Below this, it's not important +# to move. +ARCHIVE_MIN_TEXT_LENGTH = 500 +# Length of the stub left behind in the text columns +ARCHIVE_STUB_LENGTH = 200 + +S3_KEY_PREFIX = "comments_text" + +DEFAULT_BATCH_SIZE = 500 +# S3 has no multi-object PUT, so the only way to cut the wall-clock cost of the +# uploads is to keep several of them in flight at once. They are latency bound, +# not bandwidth bound, so this scales close to linearly. +DEFAULT_CONCURRENCY = 8 + +# `text` is the base column shadowed by modeltranslation: it holds a duplicate +# of the original content that is written on save but never read back (reads of +# `comment.text` resolve to `text_original` through the translation +# descriptor). `text_original` may be NULL or empty on rows that were never +# saved through the descriptor, so fall back to the base column. +# `output_field` is required, not decorative: `text_original` is a +# modeltranslation `TranslationTextField` and `Value("")` a `CharField`, which +# Django refuses to reconcile on its own as soon as the expression is selected +# rather than wrapped in `Length`/`Substr`. +ORIGINAL_TEXT = Coalesce( + NullIf("text_original", Value("")), "text", output_field=TextField() +) + +# Eligibility asks whether a text is longer than `ARCHIVE_MIN_TEXT_LENGTH`, not +# how long it is. Asking it as `length(text) > N` makes Postgres fetch and +# decompress the whole out-of-line value — up to 150k characters — for every +# row it considers. Slicing the first N+1 characters answers the same question +# from the first few TOAST chunks, because a text is longer than N exactly when +# its leading slice of N+1 characters is N+1 characters long. +LONG_TEXT_PREFIX_LENGTH = Length(Substr(ORIGINAL_TEXT, 1, ARCHIVE_MIN_TEXT_LENGTH + 1)) + + +def check_is_enabled() -> bool: + return bool(settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT) + + +def build_key(comment_id: int) -> str: + """ + The archive key is derived from the comment id, so it never needs to be + stored on the comment itself. This function is the only place that knows + the key layout. + """ + + return f"{S3_KEY_PREFIX}/{comment_id}.json" + + +def get_archive_s3_client(concurrency: int = 1): + """ + S3 client for the archive. Building a client is expensive, so callers that + upload many objects should build one and pass it around. The connection + pool has to be at least as large as the number of concurrent uploads, or + botocore serialises them behind the default pool of 10. + """ + + return get_boto_client( + "s3", + config=Config( + max_pool_connections=max(concurrency, 10), + # S3 answers a request rate it cannot sustain with 503 SlowDown. + # We run far below the limit, but `standard` mode covers the + # throttling error codes explicitly and backs off with jitter, + # rather than relying on the looser `legacy` default. + retries={"mode": "standard", "max_attempts": 5}, + ), + ) + + +def upload_text( + comment_id: int, + text: str, + post_id: int | None = None, + author_id: int | None = None, + s3=None, +) -> str: + """ + Uploads the full original text of a comment to S3 and returns the key. + + Only the original text is stored: bot/private comments are never + translated (see `trigger_update_comment_translations`), and storing + machine translations of an archived text would be pointless anyway. + + `post_id` and `author_id` are duplicated into the object so that a survey + of the bucket can group the texts without joining back to the database. + `post_id` is genuinely absent on comments that hang off a project. + """ + + s3 = s3 or get_archive_s3_client() + key = build_key(comment_id) + + s3.put_object( + Bucket=settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT, + Key=key, + Body=json.dumps( + { + "comment_id": comment_id, + "post_id": post_id, + "author_id": author_id, + "archived_at": timezone.now(), + "text": text, + }, + cls=DjangoJSONEncoder, + ), + ContentType="application/json", + ) + + return key + + +def fetch_text(comment_id: int, s3=None) -> str | None: + """ + Reads the archived full text of a comment back from S3. + Returns None if the object is missing. + """ + + s3 = s3 or get_archive_s3_client() + + try: + obj = s3.get_object( + Bucket=settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT, + Key=build_key(comment_id), + ) + except s3.exceptions.NoSuchKey: + logger.error("Archived text is missing for comment %s", comment_id) + + return None + + return json.loads(obj["Body"].read().decode("utf-8"))["text"] + + +def get_full_text(comment: Comment) -> str | None: + """ + Full text of a comment, transparently reading from the archive when the + stored text has been truncated. + """ + + if not comment.is_text_archived: + return comment.text + + return fetch_text(comment.pk) + + +def get_archivable_comments() -> QuerySet[Comment]: + """ + Long private bot comments old enough to be archived. + + Soft-deleted comments are included: their text is not rendered anywhere, + but it still occupies the row, and archiving keeps it recoverable. + """ + + cutoff = timezone.now() - timedelta(days=ARCHIVE_AGE_DAYS) + + return ( + # `rewrite(False)` is essential, not an optimisation. Comment is + # registered with modeltranslation, whose queryset rewrites every + # mention of `text` into the current language's column. Without it, + # `Length(ORIGINAL_TEXT)` degrades to measuring `text_original` twice + # and rows whose text only lives in the base column are never seen. + Comment.objects.rewrite(False) + .filter( + author__is_bot=True, + is_private=True, + is_text_archived=False, + created_at__lt=cutoff, + ) + .annotate(text_prefix_length=LONG_TEXT_PREFIX_LENGTH) + .filter(text_prefix_length__gt=ARCHIVE_MIN_TEXT_LENGTH) + ) + + +@dataclass +class ArchiveStats: + # Number of comments the run expects to process + total: int = 0 + archived: int = 0 + failed: int = 0 + skipped: int = 0 + chars_reclaimed: int = 0 + sample_ids: list[int] = field(default_factory=list) + + +def _build_truncate_kwargs() -> dict: + """ + Update kwargs that keep a stub in `text_original`, empty the base column + and drop every machine translation. + + The stub is kept in `text_original` because that is the only copy anything + reads: `comment.text` resolves through the modeltranslation descriptor to + the current language, falling back to `text_original`, and never to the + base column — a row whose stub lives only in `text` reads back as an empty + string in every language. The base column is emptied rather than nulled; + it is NOT NULL. + """ + + stub = Substr(ORIGINAL_TEXT, 1, ARCHIVE_STUB_LENGTH) + kwargs = {"text": Value(""), "text_original": stub, "is_text_archived": True} + + for lang, _label in settings.LANGUAGES: + if lang == settings.ORIGINAL_LANGUAGE_CODE: + continue + + kwargs[build_supported_localized_fieldname("text", lang)] = None + + return kwargs + + +def archive_bot_comment_texts( + dry_run: bool = False, + limit: int | None = None, + batch_size: int = DEFAULT_BATCH_SIZE, + concurrency: int = DEFAULT_CONCURRENCY, + on_progress: Callable[[ArchiveStats], None] | None = None, +) -> ArchiveStats: + """ + Moves the full text of long, private, old bot comments to S3, leaving a + truncated stub in the database. + + `on_progress` is called with the running stats after every batch. + """ + + stats = ArchiveStats() + queryset = get_archivable_comments() + + if dry_run: + # Aggregate without transferring any text. The limit has to be applied + # before aggregating, so that the reported totals describe the rows the + # real run would actually touch. This is the one place that measures + # the full length of every candidate text rather than its leading + # slice, because the point of the report is the exact saving. + scoped = queryset.order_by("id") + + if limit is not None: + scoped = scoped[:limit] + + totals = scoped.annotate(text_length=Length(ORIGINAL_TEXT)).aggregate( + count=Count("id"), chars=Sum("text_length") + ) + count = totals["count"] or 0 + + stats.total = count + stats.archived = count + stats.chars_reclaimed = max( + (totals["chars"] or 0) - count * ARCHIVE_STUB_LENGTH, 0 + ) + sample = queryset.order_by("id").values_list("id", flat=True)[:5] + stats.sample_ids = list(sample) + + return stats + + # The eligibility query is the expensive half of this command: it joins + # users, cannot use an index for the text-length test, and walks every + # private bot comment. Running it once and keeping the ids is what stops + # the run from re-issuing that scan on every batch — hundreds of heavy + # queries spread over the hours the uploads take, which is enough sustained + # load to matter to everything else using the database. Ids are all that is + # held: even a million of them is a few megabytes, and the text itself is + # still fetched a page at a time below. + candidate_ids = list(queryset.order_by("id").values_list("id", flat=True)) + + if limit is not None: + candidate_ids = candidate_ids[:limit] + + stats.total = len(candidate_ids) + + started_at = timezone.now() + truncate_kwargs = _build_truncate_kwargs() + concurrency = max(concurrency, 1) + # One client, shared by every worker: botocore clients are safe to call + # from multiple threads once built, and building one per upload is pure + # overhead + s3 = get_archive_s3_client(concurrency) + + for offset in range(0, len(candidate_ids), batch_size): + page_ids = candidate_ids[offset : offset + batch_size] + + # A plain primary-key lookup, with none of the eligibility work: the + # ids were already vetted. `is_text_archived` is re-checked because the + # id list is a snapshot and a concurrent run may have taken these rows + # in the meantime. + # `original_text` is annotated rather than selecting both columns: + # they hold the same content, and a page of 500 comments that may run + # to 150k characters each is worth not loading twice. Its length is + # measured in Python for the same reason — the text is in hand already, + # so asking the database for `length()` would detoast it a second time. + rows = list( + Comment.objects.rewrite(False) + .filter(id__in=page_ids, is_text_archived=False) + .annotate(original_text=ORIGINAL_TEXT) + .values("id", "original_text", "on_post_id", "author_id") + ) + + uploaded_ids = [] + + # Each comment is still its own independently retrievable object; the + # requests are simply issued in parallel, since they are round-trip + # bound. The database update below waits for the whole page, so an + # upload can never be outrun by its own truncation. + with ThreadPoolExecutor(max_workers=concurrency) as pool: + futures = { + pool.submit( + upload_text, + row["id"], + row["original_text"], + row["on_post_id"], + row["author_id"], + s3, + ): row["id"] + for row in rows + } + + for future, comment_id in futures.items(): + try: + future.result() + uploaded_ids.append(comment_id) + except Exception: + logger.exception("Failed to archive text of comment %s", comment_id) + stats.failed += 1 + + continue + + if uploaded_ids: + # Only truncate rows that have not been touched since the run + # began, so an edit racing the upload can never lose text. + # `edited_at` is nullable on rows that predate + # TimeStampedModel.save. + # `rewrite(False)` again: modeltranslation's `update()` rewrites + # the `text` kwarg to `text_original`, which collides with the + # `text_original` kwarg and leaves the base column holding the + # full text — silently forfeiting half the space this reclaims. + untouched = ( + Comment.objects.rewrite(False) + .filter(pk__in=uploaded_ids) + .filter(Q(edited_at__lt=started_at) | Q(edited_at__isnull=True)) + ) + archived_ids = set(untouched.values_list("id", flat=True)) + updated = untouched.update(**truncate_kwargs) + + stats.archived += updated + stats.skipped += len(uploaded_ids) - updated + stats.chars_reclaimed += sum( + max(len(row["original_text"]) - ARCHIVE_STUB_LENGTH, 0) + for row in rows + if row["id"] in archived_ids + ) + stats.sample_ids = (stats.sample_ids + sorted(archived_ids))[:5] + + if on_progress is not None: + on_progress(stats) + + return stats diff --git a/comments/tasks.py b/comments/tasks.py index 3da964b152..dd6d556877 100644 --- a/comments/tasks.py +++ b/comments/tasks.py @@ -101,3 +101,23 @@ def update_current_top_comments_of_week(): # Update the week before week_start_date = week_start_date - timedelta(days=7) update_top_comments_of_week(week_start_date) + + +@dramatiq.actor(time_limit=1_800_000, max_retries=1) +def job_archive_bot_comment_texts(): + # Import here to avoid circular imports + from comments.services.text_archive import ( + archive_bot_comment_texts, + check_is_enabled, + ) + + if not check_is_enabled(): + return + + stats = archive_bot_comment_texts() + + logger.info( + f"Archived the text of {stats.archived} bot comment(s), " + f"reclaiming {stats.chars_reclaimed} characters " + f"({stats.failed} failed, {stats.skipped} skipped)" + ) diff --git a/comments/urls.py b/comments/urls.py index 6c3a590a19..1105c1db52 100644 --- a/comments/urls.py +++ b/comments/urls.py @@ -10,6 +10,7 @@ name="comment-delete", ), path("comments//edit/", common.comment_edit_api_view, name="comment-edit"), + path("comments//", common.comment_detail_api_view, name="comment-detail"), path("comments//vote/", common.comment_vote_api_view, name="comment-vote"), path( "comments//toggle_cmm/", diff --git a/comments/views/common.py b/comments/views/common.py index 2c5a3c1371..ce657dc6fb 100644 --- a/comments/views/common.py +++ b/comments/views/common.py @@ -4,7 +4,7 @@ from django.utils import timezone from rest_framework import serializers, status from rest_framework.decorators import api_view, permission_classes -from rest_framework.exceptions import PermissionDenied, ValidationError +from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError from rest_framework.permissions import AllowAny, IsAuthenticated, IsAdminUser from rest_framework.request import Request from rest_framework.response import Response @@ -24,6 +24,7 @@ serialize_comments_of_the_week_many, ) from comments.services.common import ( + get_comment_permission_for_user, set_comment_excluded_from_week_top, create_comment, perform_create_comment, @@ -35,6 +36,7 @@ toggle_cmm, ) from comments.services.feed import get_comments_feed +from comments.services.text_archive import get_full_text from notifications.services import send_comment_report_notification_to_staff from posts.services.common import get_post_permission_for_user from projects.permissions import ObjectPermission @@ -105,7 +107,7 @@ def comments_list_api_view(request: Request): paginated_comments = paginator.paginate_queryset(comments, request) data = serialize_comment_many( - paginated_comments, request.user, with_key_factors=True + paginated_comments, request.user, with_key_factors=True, request=request ) return paginator.get_paginated_response(data) @@ -232,6 +234,44 @@ def comment_report_api_view(request, pk=int): return Response(status=status.HTTP_204_NO_CONTENT) +@api_view(["GET"]) +@permission_classes([AllowAny]) +def comment_detail_api_view(request: Request, pk: int): + """ + Returns a single comment with its untruncated text, reading the text back + from the archive if it has been moved out of the database. + """ + + comment = get_object_or_404(Comment, pk=pk) + + # Staff read any comment, deleted or private. Archiving would otherwise + # take away the only view they had of a bot's full text: + # `get_comment_permission_for_user` resolves every private comment to no + # permission but the author's, and the row itself now holds only a stub. + is_staff = request.user.is_staff or request.user.is_superuser + + if not is_staff: + permission = get_comment_permission_for_user(comment, user=request.user) + ObjectPermission.can_view(permission, raise_exception=True) + + if comment.is_soft_deleted: + # Mirrors the comment serializer, which never exposes deleted text + raise PermissionDenied("This comment has been deleted.") + + text = get_full_text(comment) + + if text is None: + raise NotFound("The archived text of this comment could not be retrieved.") + + data = serialize_comment_many([comment], request.user, with_key_factors=True)[0] + # The serializer reads the row, which holds only a stub once the comment is + # archived, and blanks the text of deleted comments that only staff reach + # here. + data["text"] = text + + return Response(data) + + @api_view(["POST"]) def comment_create_oldapi_view(request: Request): """ diff --git a/front_end/messages/cs.json b/front_end/messages/cs.json index aafe858dd5..7011268f1e 100644 --- a/front_end/messages/cs.json +++ b/front_end/messages/cs.json @@ -2427,5 +2427,9 @@ "newsHotnessColCluster": "Klastr", "newsHotnessColWeight": "Váha", "newsHotnessColContribution": "Příspěvek", + "loadFullComment": "Komentář zkrácen — načíst zbytek", + "onlyBots": "Pouze boti", + "humansOnly": "Pouze lidé", + "humansAndBots": "Lidé a boti", "feedTileQuestionsRecentlyResolved": "{count, plural, one {# otázka nedávno vyřešena} other {# otázek nedávno vyřešeno}}" } diff --git a/front_end/messages/en.json b/front_end/messages/en.json index f0e89f1a6a..7c490d3684 100644 --- a/front_end/messages/en.json +++ b/front_end/messages/en.json @@ -563,6 +563,7 @@ "year": "year", "years": "years", "error": "Error", + "loadFullComment": "Comment truncated — load the rest", "loading": "Loading", "leaderboards": "Leaderboards", "binary": "Binary", diff --git a/front_end/messages/es.json b/front_end/messages/es.json index 28d501b4c0..7017d65d59 100644 --- a/front_end/messages/es.json +++ b/front_end/messages/es.json @@ -2427,5 +2427,9 @@ "newsHotnessColCluster": "Clúster", "newsHotnessColWeight": "Peso", "newsHotnessColContribution": "Contribución", + "loadFullComment": "Comentario truncado — carga el resto", + "onlyBots": "Solo bots", + "humansOnly": "Solo humanos", + "humansAndBots": "Humanos y Bots", "feedTileQuestionsRecentlyResolved": "{count, plural, one {# pregunta resuelta recientemente} other {# preguntas resueltas recientemente}}" } diff --git a/front_end/messages/pt.json b/front_end/messages/pt.json index 43c9c6c3e3..4441012583 100644 --- a/front_end/messages/pt.json +++ b/front_end/messages/pt.json @@ -2425,5 +2425,9 @@ "newsHotnessColCluster": "Cluster", "newsHotnessColWeight": "Peso", "newsHotnessColContribution": "Contribuição", + "loadFullComment": "Comentário truncado — carregar o resto", + "onlyBots": "Apenas bots", + "humansOnly": "Apenas humanos", + "humansAndBots": "Humanos e bots", "feedTileQuestionsRecentlyResolved": "{count, plural, one {# pergunta recentemente resolvida} other {# perguntas recentemente resolvidas}}" } diff --git a/front_end/messages/zh-TW.json b/front_end/messages/zh-TW.json index 9f5dced51d..16be0f9d05 100644 --- a/front_end/messages/zh-TW.json +++ b/front_end/messages/zh-TW.json @@ -2424,5 +2424,9 @@ "newsHotnessColCluster": "叢集", "newsHotnessColWeight": "權重", "newsHotnessColContribution": "貢獻值", + "loadFullComment": "評論被截斷 — 加載其餘部分", + "onlyBots": "僅限機器人", + "humansOnly": "僅限人類", + "humansAndBots": "人類和機器人", "feedTileQuestionsRecentlyResolved": "{count, plural, one {最近解決了 # 個問題} other {最近解決了 # 個問題}}" } diff --git a/front_end/messages/zh.json b/front_end/messages/zh.json index 0517006b97..c4aad7c41f 100644 --- a/front_end/messages/zh.json +++ b/front_end/messages/zh.json @@ -2429,5 +2429,9 @@ "newsHotnessColWeight": "权重", "newsHotnessColContribution": "贡献值", "viewComment": "查看评论", + "loadFullComment": "评论被截断——加载剩余部分", + "onlyBots": "仅限机器人", + "humansOnly": "仅限人类", + "humansAndBots": "人类和机器人", "feedTileQuestionsRecentlyResolved": "{count, plural, one {# 个问题最近已解决} other {# 个问题最近已解决}}" } diff --git a/front_end/src/app/(main)/components/comments_feed_provider.tsx b/front_end/src/app/(main)/components/comments_feed_provider.tsx index 8d050cd32b..da79227736 100644 --- a/front_end/src/app/(main)/components/comments_feed_provider.tsx +++ b/front_end/src/app/(main)/components/comments_feed_provider.tsx @@ -260,6 +260,7 @@ const CommentsFeedProvider: FC< parent_id: parentId, created_at: nowIso, text_edited_at: nowIso, + is_text_archived: false, is_soft_deleted: false, text, is_private: false, diff --git a/front_end/src/components/comment_feed/comment.tsx b/front_end/src/components/comment_feed/comment.tsx index 8af9f3f45a..c316292934 100644 --- a/front_end/src/components/comment_feed/comment.tsx +++ b/front_end/src/components/comment_feed/comment.tsx @@ -45,6 +45,7 @@ import { usePublicSettings } from "@/contexts/public_settings_context"; import { useCommentDraft } from "@/hooks/use_comment_draft"; import useContainerSize from "@/hooks/use_container_size"; import useScrollTo from "@/hooks/use_scroll_to"; +import ClientCommentsApi from "@/services/api/comments/comments.client"; import { CommentType } from "@/types/comment"; import { ErrorResponse } from "@/types/fetch"; import { @@ -294,6 +295,24 @@ const Comment: FC = ({ }, [questionLayout?.scrollToCommentId, comment.id, questionLayout]); const [errorMessage, setErrorMessage] = useState(); const [commentMarkdown, setCommentMarkdown] = useState(comment.text); + // Long bot comments are archived to S3 with only a stub left in the row, so + // the full text has to be fetched on demand + const [isTruncated, setIsTruncated] = useState(comment.is_text_archived); + const [isLoadingFullText, setIsLoadingFullText] = useState(false); + const loadFullText = useCallback(async () => { + setIsLoadingFullText(true); + try { + const { text } = await ClientCommentsApi.getComment(comment.id); + setCommentMarkdown(text); + originalTextRef.current = text; + setIsTruncated(false); + } catch (err) { + logError(err); + toast.error(t("unexpectedError")); + } finally { + setIsLoadingFullText(false); + } + }, [comment.id, t]); const [tempCommentMarkdown, setTempCommentMarkdown] = useState(""); const [includeEditForecast, setIncludeEditForecast] = useState(false); const [includedForecast, setIncludedForecast] = useState( @@ -1008,6 +1027,16 @@ const Comment: FC = ({ contentEditableClassName="text-base font-normal leading-6 [&_p]:!text-gray-700 dark:[&_p]:!text-gray-700-dark [&_ul]:!text-gray-700 dark:[&_ul]:!text-gray-700-dark [&_ol]:!text-gray-700 dark:[&_ol]:!text-gray-700-dark" /> )} + {isTruncated && ( + + )} {commentKeyFactors.length > 0 && canListKeyFactors && postData && ( diff --git a/front_end/src/services/api/comments/comments.shared.ts b/front_end/src/services/api/comments/comments.shared.ts index a96f14b44b..0c529ffc51 100644 --- a/front_end/src/services/api/comments/comments.shared.ts +++ b/front_end/src/services/api/comments/comments.shared.ts @@ -91,6 +91,10 @@ class CommentsApi extends ApiService { return response; } + async getComment(id: number): Promise { + return await this.get(`/comments/${id}/`); + } + async getCommentsOfWeek(start_date: string): Promise { return await this.get( `/comments/comments-of-week/?start_date=${start_date}` diff --git a/front_end/src/types/comment.ts b/front_end/src/types/comment.ts index a29330c119..68ccfec5e1 100644 --- a/front_end/src/types/comment.ts +++ b/front_end/src/types/comment.ts @@ -20,6 +20,9 @@ export type BECommentType = { text_edited_at: string; is_soft_deleted: boolean; text: string; + // Set when only a stub of `text` is stored; the rest is fetched on demand + // from `getComment`. + is_text_archived: boolean; included_forecast?: ForecastType; is_private: boolean; vote_score?: number; diff --git a/front_end/src/utils/comments.ts b/front_end/src/utils/comments.ts index 26740ae75e..38fb149ae0 100644 --- a/front_end/src/utils/comments.ts +++ b/front_end/src/utils/comments.ts @@ -17,6 +17,7 @@ export function parseComment( created_at: comment.created_at, text_edited_at: comment.text_edited_at, is_soft_deleted: comment.is_soft_deleted, + is_text_archived: comment.is_text_archived, included_forecast: comment.included_forecast, is_private: comment.is_private, vote_score: comment.vote_score, diff --git a/metaculus_web/settings.py b/metaculus_web/settings.py index 27eed75342..54d8c71e59 100644 --- a/metaculus_web/settings.py +++ b/metaculus_web/settings.py @@ -429,6 +429,12 @@ def get_jwt_encryption_config(): AWS_STORAGE_BUCKET_POST_VERSION_HISTORY = os.environ.get( "AWS_STORAGE_BUCKET_POST_VERSION_HISTORY" ) +# S3 bucket holding the `comments_text/` prefix of archived comment texts. +# Comment text archiving will be disabled if this isn’t set. There is +# deliberately no fallback to another bucket: the archive is the only copy of +# the text, so a missing setting must disable the feature rather than silently +# write somewhere unintended. +AWS_STORAGE_BUCKET_COMMENTS_TEXT = os.environ.get("AWS_STORAGE_BUCKET_COMMENTS_TEXT") # Cloudflare captcha # https://developers.cloudflare.com/turnstile/get-started/server-side-validation/ diff --git a/misc/management/commands/cron.py b/misc/management/commands/cron.py index 95aef64855..e9aa44dae8 100644 --- a/misc/management/commands/cron.py +++ b/misc/management/commands/cron.py @@ -10,6 +10,7 @@ from comments.tasks import ( update_current_top_comments_of_week, + job_archive_bot_comment_texts, job_finalize_and_send_weekly_top_comments, ) from misc.jobs import sync_itn_articles @@ -242,6 +243,13 @@ def handle(self, *args, **options): max_instances=1, replace_existing=True, ) + scheduler.add_job( + close_old_connections(job_archive_bot_comment_texts.send), + trigger=CronTrigger.from_crontab("0 4 * * *"), # Daily at 04:00 UTC + id="comments_archive_bot_comment_texts", + max_instances=1, + replace_existing=True, + ) # # Cache warm-up jobs diff --git a/tests/unit/test_comments/test_text_archive.py b/tests/unit/test_comments/test_text_archive.py new file mode 100644 index 0000000000..727b2601f2 --- /dev/null +++ b/tests/unit/test_comments/test_text_archive.py @@ -0,0 +1,559 @@ +import json +from datetime import timedelta +from io import StringIO + +import pytest # noqa +from django.core.management import call_command +from django.core.management.base import CommandError +from django.urls import reverse +from django.utils import timezone +from rest_framework.exceptions import ValidationError +from rest_framework.test import APIClient + +from comments.models import Comment +from comments.services.common import update_comment +from comments.services.text_archive import ( + ARCHIVE_MIN_TEXT_LENGTH, + ARCHIVE_STUB_LENGTH, + archive_bot_comment_texts, + build_key, + get_archivable_comments, + upload_text, +) +from posts.models import Post +from projects.permissions import ObjectPermission +from tests.unit.test_comments.factories import factory_comment +from tests.unit.test_posts.factories import factory_post +from tests.unit.test_projects.factories import factory_project +from tests.unit.test_questions.conftest import * # noqa +from tests.unit.test_users.factories import factory_user + +LONG_TEXT = "b" * (ARCHIVE_MIN_TEXT_LENGTH + 500) + + +@pytest.fixture() +def bot(user1): + return factory_user(username="bot1", email="bot1@metaculus.com", is_bot=True) + + +@pytest.fixture() +def staff_user(): + return factory_user(username="staff1", email="staff1@metaculus.com", is_staff=True) + + +@pytest.fixture() +def post(user1): + return factory_post( + author=user1, + default_project=factory_project( + default_permission=ObjectPermission.FORECASTER, + ), + curation_status=Post.CurationStatus.APPROVED, + ) + + +def factory_archivable_comment(author, post, text=LONG_TEXT, **kwargs): + kwargs.setdefault("created_at", timezone.now() - timedelta(days=60)) + kwargs.setdefault("is_private", True) + + return factory_comment( + author=author, + on_post=post, + text=text, + text_original=text, + **kwargs, + ) + + +@pytest.fixture() +def s3_stub(mocker, settings): + """ + Minimal in-memory stand-in for the S3 client used by the archive service. + """ + + objects = {} + + class Client: + class exceptions: + class NoSuchKey(Exception): + pass + + def put_object(self, Bucket, Key, Body, **kwargs): + objects[Key] = Body + + def get_object(self, Bucket, Key): + # A key mapped to None is one the listing still reports but whose + # object has gone: exactly what S3 answers with NoSuchKey + if objects.get(Key) is None: + raise Client.exceptions.NoSuchKey() + + return {"Body": mocker.Mock(read=lambda: objects[Key].encode("utf-8"))} + + def get_paginator(self, operation_name): + assert operation_name == "list_objects_v2" + + class Paginator: + def paginate(self, Bucket, Prefix): + # One page is enough here; the real paginator's chunking + # is botocore's concern, not ours + yield { + "Contents": [ + {"Key": key} + for key in sorted(objects) + if key.startswith(Prefix) + ] + } + + return Paginator() + + mocker.patch( + "comments.services.text_archive.get_boto_client", return_value=Client() + ) + settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT = "test-bucket" + + return objects + + +class TestArchivableCommentsQueryset: + def test_includes_long_old_private_bot_comments(self, bot, post): + comment = factory_archivable_comment(bot, post) + + assert list(get_archivable_comments()) == [comment] + + def test_excludes_recent_comments(self, bot, post): + factory_archivable_comment(bot, post, created_at=timezone.now()) + + assert not get_archivable_comments().exists() + + def test_excludes_short_comments(self, bot, post): + factory_archivable_comment(bot, post, text="a" * ARCHIVE_MIN_TEXT_LENGTH) + + assert not get_archivable_comments().exists() + + def test_excludes_public_comments(self, bot, post): + factory_archivable_comment(bot, post, is_private=False) + + assert not get_archivable_comments().exists() + + def test_excludes_human_comments(self, user1, post): + factory_archivable_comment(user1, post) + + assert not get_archivable_comments().exists() + + def test_excludes_already_archived_comments(self, bot, post): + factory_archivable_comment(bot, post, is_text_archived=True) + + assert not get_archivable_comments().exists() + + def test_includes_soft_deleted_comments(self, bot, post): + comment = factory_archivable_comment(bot, post, is_soft_deleted=True) + + assert list(get_archivable_comments()) == [comment] + + +class TestArchiveBotCommentTexts: + def test_archives_text_to_s3_and_truncates_row(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + + stats = archive_bot_comment_texts() + + assert stats.archived == 1 + assert stats.failed == 0 + assert stats.skipped == 0 + assert stats.chars_reclaimed == len(LONG_TEXT) - ARCHIVE_STUB_LENGTH + + # Full text is in S3, alongside the ids a bucket survey needs + payload = json.loads(s3_stub[build_key(comment.pk)]) + assert payload["comment_id"] == comment.pk + assert payload["post_id"] == post.pk + assert payload["author_id"] == bot.pk + assert payload["text"] == LONG_TEXT + + # Only a stub is left, in the one column that is read back + comment.refresh_from_db() + assert comment.is_text_archived is True + assert comment.text_original == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + assert comment.text == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + # `rewrite(False)` is what makes this assertion meaningful: a plain + # `values_list("text")` is rewritten by modeltranslation to read + # `text_original`, so it would pass even if the base column still + # held the full text. + assert ( + Comment.objects.rewrite(False) + .filter(pk=comment.pk) + .values_list("text", flat=True)[0] + == "" + ) + + def test_drops_translations(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post, text_en="translated") + + archive_bot_comment_texts() + + comment.refresh_from_db() + assert comment.text_en is None + + def test_does_not_bump_edited_at(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + edited_at = comment.edited_at + + archive_bot_comment_texts() + + comment.refresh_from_db() + assert comment.edited_at == edited_at + + def test_is_idempotent(self, bot, post, s3_stub): + factory_archivable_comment(bot, post) + + assert archive_bot_comment_texts().archived == 1 + assert archive_bot_comment_texts().archived == 0 + + def test_dry_run_writes_nothing(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + + stats = archive_bot_comment_texts(dry_run=True) + + assert stats.archived == 1 + assert stats.chars_reclaimed == len(LONG_TEXT) - ARCHIVE_STUB_LENGTH + assert stats.sample_ids == [comment.pk] + + assert s3_stub == {} + comment.refresh_from_db() + assert comment.is_text_archived is False + assert comment.text_original == LONG_TEXT + + def test_dry_run_totals_are_scoped_to_the_limit(self, bot, post, s3_stub): + for _ in range(3): + factory_archivable_comment(bot, post) + + per_comment = len(LONG_TEXT) - ARCHIVE_STUB_LENGTH + unlimited = archive_bot_comment_texts(dry_run=True) + limited = archive_bot_comment_texts(dry_run=True, limit=1) + + assert unlimited.archived == 3 + assert unlimited.chars_reclaimed == 3 * per_comment + + # The limited estimate must describe only the rows a real run would + # touch, not the whole queryset + assert limited.archived == 1 + assert limited.chars_reclaimed == per_comment + + def test_upload_failure_leaves_comment_intact(self, bot, post, s3_stub, mocker): + comment = factory_archivable_comment(bot, post) + mocker.patch( + "comments.services.text_archive.upload_text", + side_effect=RuntimeError("s3 is down"), + ) + + stats = archive_bot_comment_texts() + + assert stats.archived == 0 + assert stats.failed == 1 + + comment.refresh_from_db() + assert comment.is_text_archived is False + assert comment.text_original == LONG_TEXT + + def test_respects_limit(self, bot, post, s3_stub): + factory_archivable_comment(bot, post) + factory_archivable_comment(bot, post) + + assert archive_bot_comment_texts(limit=1).archived == 1 + assert get_archivable_comments().count() == 1 + + def test_archives_comments_whose_text_is_only_in_the_base_column( + self, bot, post, s3_stub + ): + """ + Rows written before modeltranslation was introduced have an empty + `text_original`. They are the largest rows in the table, so they must + not fall through the eligibility filter. + """ + + comment = factory_archivable_comment(bot, post) + Comment.objects.rewrite(False).filter(pk=comment.pk).update( + text=LONG_TEXT, text_original="" + ) + + assert archive_bot_comment_texts().archived == 1 + + payload = json.loads(s3_stub[build_key(comment.pk)]) + assert payload["text"] == LONG_TEXT + # The stub ends up in `text_original` even though the text came from + # the base column, which is where every read looks for it + comment.refresh_from_db() + assert comment.text_original == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + assert ( + Comment.objects.rewrite(False) + .filter(pk=comment.pk) + .values_list("text", flat=True)[0] + == "" + ) + + def test_processes_multiple_batches(self, bot, post, s3_stub): + for _ in range(5): + factory_archivable_comment(bot, post) + + assert archive_bot_comment_texts(batch_size=2).archived == 5 + assert not get_archivable_comments().exists() + + +class TestArchiveCommand: + def test_dry_run_reports_without_writing(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + out = StringIO() + + call_command("archive_bot_comment_texts", "--dry-run", stdout=out) + + assert "Would archive 1 comment(s)" in out.getvalue() + assert s3_stub == {} + comment.refresh_from_db() + assert comment.is_text_archived is False + + def test_archives(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + out = StringIO() + + call_command("archive_bot_comment_texts", stdout=out) + + assert "Archived 1 comment(s)" in out.getvalue() + comment.refresh_from_db() + assert comment.is_text_archived is True + + def test_errors_when_bucket_is_not_configured(self, bot, post, settings): + settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT = None + + with pytest.raises(CommandError): + call_command("archive_bot_comment_texts", "--dry-run") + + +class TestArchivedCommentEditing: + def test_archived_comment_cannot_be_edited(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + comment.refresh_from_db() + + with pytest.raises(ValidationError): + update_comment(comment, text="new text") + + def test_unarchived_comment_can_still_be_edited(self, bot, post): + comment = factory_archivable_comment(bot, post, created_at=timezone.now()) + + update_comment(comment, text="new text") + + comment.refresh_from_db() + assert comment.text == "new text" + + +class TestCommentDetailApiView: + def test_anonymous_reads_a_public_comment(self, user1, post, anon_client): + """ + The endpoint is open to logged-out callers; what they may read is + decided per comment, not by whether they have an account. + """ + + comment = factory_comment( + author=user1, on_post=post, text=LONG_TEXT, text_original=LONG_TEXT + ) + + response = anon_client.get(reverse("comment-detail", kwargs={"pk": comment.pk})) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + def test_anonymous_reads_archived_public_text( + self, user1, post, s3_stub, anon_client + ): + """ + Only private bot comments are archived today. This covers the case the + open endpoint exists for: a public comment archived on age alone, whose + row holds a stub a logged-out reader has to be able to open. + """ + + comment = factory_comment( + author=user1, on_post=post, text=LONG_TEXT, text_original=LONG_TEXT + ) + upload_text(comment.pk, LONG_TEXT, comment.on_post_id, comment.author_id) + Comment.objects.rewrite(False).filter(pk=comment.pk).update( + text="", + text_original=LONG_TEXT[:ARCHIVE_STUB_LENGTH], + is_text_archived=True, + ) + + response = anon_client.get(reverse("comment-detail", kwargs={"pk": comment.pk})) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + assert response.data["is_text_archived"] is True + + def test_anonymous_cannot_read_a_private_comment( + self, bot, post, s3_stub, anon_client + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = anon_client.get(reverse("comment-detail", kwargs={"pk": comment.pk})) + + assert response.status_code == 403 + + def test_anonymous_cannot_read_a_deleted_comment(self, user1, post, anon_client): + comment = factory_comment( + author=user1, on_post=post, text=LONG_TEXT, is_soft_deleted=True + ) + + response = anon_client.get(reverse("comment-detail", kwargs={"pk": comment.pk})) + + assert response.status_code == 403 + + def test_author_reads_archived_text( + self, bot, post, s3_stub, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = create_client_for_user(bot).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + # The endpoint returns the whole comment, not just its text + assert response.data["id"] == comment.pk + assert response.data["author"]["id"] == bot.pk + assert response.data["on_post"] == post.pk + assert response.data["is_text_archived"] is True + + def test_returns_db_text_when_not_archived( + self, bot, post, s3_stub, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + + response = create_client_for_user(bot).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + def test_other_user_cannot_read_private_comment( + self, bot, post, s3_stub, user2, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = create_client_for_user(user2).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 403 + + def test_soft_deleted_comment_is_not_readable( + self, bot, post, s3_stub, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + Comment.objects.filter(pk=comment.pk).update(is_soft_deleted=True) + + response = create_client_for_user(bot).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 403 + + def test_staff_reads_someone_elses_private_archived_text( + self, bot, post, s3_stub, staff_user, create_client_for_user + ): + """ + Archiving must not take away the view staff already had in the admin. + """ + + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = create_client_for_user(staff_user).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + def test_staff_reads_soft_deleted_archived_text( + self, bot, post, s3_stub, staff_user, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + Comment.objects.filter(pk=comment.pk).update(is_soft_deleted=True) + + response = create_client_for_user(staff_user).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + def test_superuser_reads_someone_elses_private_archived_text( + self, bot, post, s3_stub, user_admin, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = create_client_for_user(user_admin).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + def test_missing_archive_object_returns_404( + self, bot, post, s3_stub, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + s3_stub.clear() + + response = create_client_for_user(bot).get( + reverse("comment-detail", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 404 + + +class TestArchivedTextNoticeForApiClients: + """ + The web front end offers a button that loads the rest of the comment, so + the notice is only spelled out for callers that have no such affordance. + """ + + def _list_own_private_comments(self, client, post): + return client.get( + reverse("comment-list"), {"post": post.pk, "is_private": True} + ) + + def test_api_key_caller_gets_a_pointer_to_the_full_text( + self, bot, post, s3_stub, create_client_for_user + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + response = self._list_own_private_comments(create_client_for_user(bot), post) + + assert response.status_code == 200 + (data,) = [c for c in response.data["results"] if c["id"] == comment.pk] + assert data["is_text_archived"] is True + assert "Content Truncated" in data["text"] + assert f"/api/comments/{comment.pk}/" in data["text"] + + def test_session_caller_gets_the_bare_stub(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + + client = APIClient() + client.force_login(bot) + response = self._list_own_private_comments(client, post) + + assert response.status_code == 200 + (data,) = [c for c in response.data["results"] if c["id"] == comment.pk] + # The flag is still there; it is what the front end renders the button from + assert data["is_text_archived"] is True + assert "Content Truncated" not in data["text"] + assert data["text"] == LONG_TEXT[:ARCHIVE_STUB_LENGTH]