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..72527ddc49 --- /dev/null +++ b/comments/management/commands/archive_bot_comment_texts.py @@ -0,0 +1,179 @@ +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. + + This command 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 monthly as a cron job; the full text stays " + "readable through the comment-full-text 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("Counting 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/management/commands/sync_archived_comment_texts.py b/comments/management/commands/sync_archived_comment_texts.py new file mode 100644 index 0000000000..2e642f7d22 --- /dev/null +++ b/comments/management/commands/sync_archived_comment_texts.py @@ -0,0 +1,197 @@ +import time + +from django.conf import settings +from django.core.management.base import BaseCommand, CommandError + +from comments.services.text_archive import ( + DEFAULT_BATCH_SIZE, + DEFAULT_CONCURRENCY, + S3_KEY_PREFIX, + SyncStats, + check_is_enabled, + sync_archived_comment_texts, +) + + +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. + + This command works through hundreds of thousands of objects 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 = ( + "Truncates comments whose full text is already in the S3 archive, " + "without uploading anything. This is the second half of a one-off " + "migration: `archive_bot_comment_texts` is run once against a copy of " + "the database to populate the bucket, then this brings the real " + "database in line with it. Afterwards the monthly cron job takes over." + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Report what would be truncated without writing to the database", + ) + parser.add_argument( + "--verify", + action="store_true", + help=( + "Re-read every archived object and require it to match the row " + "before truncating. Much slower, and downloads the whole " + "archive, but it is the only check that the archived copy is " + "still current" + ), + ) + 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=( + "Downloads to keep in flight at once, for --verify " + f"(default: {DEFAULT_CONCURRENCY})" + ), + ) + + def handle(self, *args, **options): + if not check_is_enabled(): + raise CommandError( + "AWS_STORAGE_BUCKET_COMMENTS_TEXT is not configured, " + "comment text archiving is disabled." + ) + + dry_run = options["dry_run"] + progress = ProgressWriter(self.stdout) + + progress.write( + f"Syncing against {settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT}/" + f"{S3_KEY_PREFIX}/" + + (" (verifying every object)" if options["verify"] else "") + ) + progress.write("Listing the archive...") + + def on_progress(stats: SyncStats) -> None: + progress.total = stats.total + done = ( + stats.synced + + stats.already_archived + + stats.orphaned + + stats.ineligible + + stats.mismatched + + stats.verify_failed + ) + detail = ", ".join( + f"{count} {label}" + for label, count in ( + ("already archived", stats.already_archived), + ("orphaned", stats.orphaned), + ("ineligible", stats.ineligible), + ("mismatched", stats.mismatched), + ("unreadable", stats.verify_failed), + ) + if count + ) + progress.update(done, f"{stats.chars_reclaimed:,} chars reclaimed", detail) + + stats = sync_archived_comment_texts( + dry_run=dry_run, + verify=options["verify"], + batch_size=options["batch_size"], + concurrency=options["concurrency"], + on_progress=on_progress, + ) + + verb = "Would sync" if dry_run else "Synced" + progress.write( + f"\n{verb} {stats.synced:,} of {stats.total:,} archived object(s), " + f"reclaiming {stats.chars_reclaimed:,} characters " + f"in {format_duration(progress.elapsed)}" + ) + + for label, count in ( + ("already truncated", stats.already_archived), + ("orphaned (no such comment)", stats.orphaned), + ("ineligible (not a long private bot comment)", stats.ineligible), + ): + if count: + progress.write(f" {count:,} {label}") + + if stats.sample_ids: + sample = ", ".join(str(pk) for pk in stats.sample_ids) + progress.write(f"Sample comment ids: {sample}") + + if stats.mismatched: + # Not fatal: these keep their text and the monthly job re-archives + # them, but a large number means the archive is further out of + # date than expected + self.stdout.write( + self.style.WARNING( + f"{stats.mismatched:,} archived object(s) did not match the " + "current text and were left alone" + ) + ) + + if stats.verify_failed: + # Distinct from a mismatch: nothing is known about these objects, + # so a non-zero count here means the bucket is what needs looking at + self.stdout.write( + self.style.ERROR( + f"{stats.verify_failed:,} archived object(s) could not be read " + "back and were left alone" + ) + ) 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..8963d98a6e --- /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(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-full-text endpoint to read them.'), + ), + ] diff --git a/comments/models.py b/comments/models.py index c35836561b..b23585ba5a 100644 --- a/comments/models.py +++ b/comments/models.py @@ -99,6 +99,15 @@ 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. + 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-full-text endpoint to read them.", + ) 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..b7cf427e8c 100644 --- a/comments/serializers/common.py +++ b/comments/serializers/common.py @@ -76,6 +76,10 @@ class Meta: "text_edited_at", "is_soft_deleted", "text", + # TODO: consumed by the front end in a later commit, which will + # replace the stub in `text` with a "load full text" affordance + # backed by the `comment-full-text` endpoint + "is_text_archived", "on_post", "on_post_data", "included_forecast", diff --git a/comments/services/common.py b/comments/services/common.py index 418f0bf6e6..62e0baba3f 100644 --- a/comments/services/common.py +++ b/comments/services/common.py @@ -187,6 +187,13 @@ 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 + 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..eb7459675a --- /dev/null +++ b/comments/services/text_archive.py @@ -0,0 +1,532 @@ +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() +) + + +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, 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. + """ + + 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, + "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_length=Length(ORIGINAL_TEXT)) + .filter(text_length__gt=ARCHIVE_MIN_TEXT_LENGTH) + ) + + +@dataclass +class ArchiveStats: + # Number of comments the run expects to process. Only populated when a + # progress callback asks for it, since counting means measuring the length + # of every candidate text. + 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 leave a stub in both copies of the original text and + drop every machine translation. + """ + + stub = Substr(ORIGINAL_TEXT, 1, ARCHIVE_STUB_LENGTH) + kwargs = {"text": stub, "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. + scoped = queryset.order_by("id") + + if limit is not None: + scoped = scoped[:limit] + + totals = scoped.aggregate(count=Count("id"), chars=Sum("text_length")) + count = totals["count"] or 0 + + stats.archived = count + stats.chars_reclaimed = max( + (totals["chars"] or 0) - count * ARCHIVE_STUB_LENGTH, 0 + ) + stats.sample_ids = list( + queryset.order_by("id").values_list("id", flat=True)[:5] + ) + + return stats + + if on_progress is not None: + # Counting is not free: the eligibility filter measures the length of + # every candidate text, so this reads the whole candidate set + total = queryset.count() + stats.total = min(total, limit) if limit is not None else total + + started_at = timezone.now() + truncate_kwargs = _build_truncate_kwargs() + cursor = 0 + 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) + + while limit is None or stats.archived + stats.failed < limit: + page_size = batch_size + if limit is not None: + page_size = min(batch_size, limit - stats.archived - stats.failed) + + # `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. + rows = list( + queryset.filter(id__gt=cursor) + .order_by("id") + .annotate(original_text=ORIGINAL_TEXT) + .values("id", "original_text", "text_length")[:page_size] + ) + + if not rows: + break + + # Advance past the whole page, including rows that failed to upload, so + # a persistent failure can never stall the run. Skipped rows stay + # eligible for the next one. + cursor = rows[-1]["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"], s3): row["id"] + for row in rows + } + + for future, comment_id in futures.items(): + try: + future.result() + except Exception: + logger.exception("Failed to archive text of comment %s", comment_id) + stats.failed += 1 + + continue + + uploaded_ids.append(comment_id) + + 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(row["text_length"] - 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 + + +def list_archived_comment_ids(s3=None) -> set[int]: + """ + Every comment id that already has an object in the archive, read straight + from the bucket. + + The bucket is the authority on what has been uploaded: the point of the + sync below is to reconcile a database that knows nothing about uploads + performed elsewhere. It is not, however, the authority on what may be + truncated — see `get_syncable_comments`. + """ + + s3 = s3 or get_archive_s3_client() + prefix = f"{S3_KEY_PREFIX}/" + comment_ids = set() + + for page in s3.get_paginator("list_objects_v2").paginate( + Bucket=settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT, Prefix=prefix + ): + for obj in page.get("Contents", []): + stem = obj["Key"][len(prefix) :].removesuffix(".json") + + if stem.isdigit(): + comment_ids.add(int(stem)) + + return comment_ids + + +@dataclass +class SyncStats: + # Objects found in the bucket + total: int = 0 + synced: int = 0 + # Present in the bucket, but the row is already truncated + already_archived: int = 0 + # Present in the bucket with no matching row: deleted since the upload + orphaned: int = 0 + # A row the archiver would never have uploaded, or one already at or + # below the stub length: nothing to reclaim, and a hint that the bucket + # holds keys this command did not put there + ineligible: int = 0 + # `--verify` only: the archived text no longer matches the row + mismatched: int = 0 + # `--verify` only: the archived object could not be read back at all, + # which says nothing about whether it matches + verify_failed: int = 0 + chars_reclaimed: int = 0 + sample_ids: list[int] = field(default_factory=list) + + +def get_syncable_comments(comment_ids) -> QuerySet[Comment]: + """ + Rows this command is allowed to truncate against an archive uploaded + elsewhere. + + Anything the archiver uploaded was a long, private bot comment, so those + invariants are re-asserted here rather than trusting the key alone: a + stray or mistyped object in the bucket must not be able to truncate a row + the archiver would never have touched. + + Nothing here can tell whether the archived copy is still current — that + is what `--verify` is for. + """ + + return ( + Comment.objects.rewrite(False) + .filter( + pk__in=comment_ids, + author__is_bot=True, + is_private=True, + is_text_archived=False, + ) + .annotate(text_length=Length(ORIGINAL_TEXT)) + .filter(text_length__gt=ARCHIVE_STUB_LENGTH) + ) + + +def _verify_archived_text(rows, s3, concurrency: int) -> tuple[set[int], set[int]]: + """ + Ids whose archived object still matches the row's text exactly, and ids + whose object could not be read back at all. + + The two are kept apart because they mean different things: a mismatch is + a stale archive, an unreadable object is an S3 problem. Both leave the + row alone. + """ + + verified = set() + unreadable = set() + + with ThreadPoolExecutor(max_workers=concurrency) as pool: + futures = {pool.submit(fetch_text, row["id"], s3): row for row in rows} + + for future, row in futures.items(): + try: + archived = future.result() + except Exception: + logger.exception( + "Failed to read archived text of comment %s", row["id"] + ) + unreadable.add(row["id"]) + + continue + + if archived is None: + # `fetch_text` swallows a missing object and logs it + unreadable.add(row["id"]) + elif archived == row["original_text"]: + verified.add(row["id"]) + + return verified, unreadable + + +def sync_archived_comment_texts( + dry_run: bool = False, + verify: bool = False, + batch_size: int = DEFAULT_BATCH_SIZE, + concurrency: int = DEFAULT_CONCURRENCY, + on_progress: Callable[[SyncStats], None] | None = None, +) -> SyncStats: + """ + Truncates rows whose text is already in the archive, uploading nothing. + + This exists for one migration. The uploads are slow and bandwidth-heavy, + so they are performed once against a copy of the database; this then + brings the real database in line with the bucket without moving the text + a second time. + + `verify` re-reads every object and requires it to match the row before + truncating. That is the safe-but-slow path, and the only thing standing + between an archive that has gone stale and a lost edit: without it a row + is truncated on the strength of its key being in the bucket. + """ + + stats = SyncStats() + concurrency = max(concurrency, 1) + # One client for the whole run, listing and verification alike: building + # one is expensive, and the verification below would otherwise build a + # fresh one for every batch. + s3 = get_archive_s3_client(concurrency) + archived_ids = sorted(list_archived_comment_ids(s3)) + stats.total = len(archived_ids) + + truncate_kwargs = _build_truncate_kwargs() + columns = ["id", "text_length"] + (["original_text"] if verify else []) + + for start in range(0, len(archived_ids), batch_size): + chunk = archived_ids[start : start + batch_size] + + # Two queries per chunk so the accounting is exact: what the database + # knows about these ids, then which of them may be truncated. + states = dict( + Comment.objects.rewrite(False) + .filter(pk__in=chunk) + .values_list("id", "is_text_archived") + ) + syncable = get_syncable_comments(chunk) + + if verify: + syncable = syncable.annotate(original_text=ORIGINAL_TEXT) + + rows = list(syncable.values(*columns)) + + stats.orphaned += len(chunk) - len(states) + already = sum(1 for archived in states.values() if archived) + stats.already_archived += already + stats.ineligible += len(states) - already - len(rows) + + if verify and rows: + verified, unreadable = _verify_archived_text(rows, s3, concurrency) + stats.mismatched += len(rows) - len(verified) - len(unreadable) + stats.verify_failed += len(unreadable) + rows = [row for row in rows if row["id"] in verified] + + if rows and not dry_run: + # Re-select at write time: a row archived or shortened between the + # select above and this update must not be truncated again. + eligible = get_syncable_comments([row["id"] for row in rows]) + synced_ids = set(eligible.values_list("id", flat=True)) + updated = eligible.update(**truncate_kwargs) + + stats.synced += updated + stats.ineligible += len(rows) - updated + rows = [row for row in rows if row["id"] in synced_ids] + elif rows: + stats.synced += len(rows) + + stats.chars_reclaimed += sum( + max(row["text_length"] - ARCHIVE_STUB_LENGTH, 0) for row in rows + ) + stats.sample_ids = (stats.sample_ids + sorted(row["id"] for row in rows))[:5] + + if on_progress is not None: + on_progress(stats) + + return stats diff --git a/comments/tasks.py b/comments/tasks.py index 3da964b152..777a88dc7a 100644 --- a/comments/tasks.py +++ b/comments/tasks.py @@ -101,3 +101,34 @@ 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) + + +# The monthly run walks a month of long private bot comments, so it needs far +# more than dramatiq's default 10-minute time limit. Retries are capped at one: +# every batch commits as it goes, so a failed run resumes rather than repeats, +# and the default of 20 would just replay the same failure for hours. +@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(): + # Logged as an error rather than skipped silently: once this job is + # scheduled, a missing bucket means the monthly cleanup never runs + logger.error( + "AWS_STORAGE_BUCKET_COMMENTS_TEXT is not configured, " + "comment text archiving cannot run" + ) + + 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..f98ab9e933 100644 --- a/comments/urls.py +++ b/comments/urls.py @@ -10,6 +10,11 @@ name="comment-delete", ), path("comments//edit/", common.comment_edit_api_view, name="comment-edit"), + path( + "comments//full-text/", + common.comment_full_text_api_view, + name="comment-full-text", + ), 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..e93b4c1dcc 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 @@ -232,6 +234,35 @@ def comment_report_api_view(request, pk=int): return Response(status=status.HTTP_204_NO_CONTENT) +@api_view(["GET"]) +def comment_full_text_api_view(request: Request, pk: int): + """ + Returns the untruncated text of a single comment, reading it 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. + if not (request.user.is_staff or request.user.is_superuser): + 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.") + + return Response({"id": comment.pk, "text": text}) + + @api_view(["POST"]) def comment_create_oldapi_view(request: Request): """ 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 272c1bd62f..7833783a6b 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 @@ -241,6 +242,14 @@ def handle(self, *args, **options): max_instances=1, replace_existing=True, ) + scheduler.add_job( + close_old_connections(job_archive_bot_comment_texts.send), + # First day of every month at 04:00 + trigger=CronTrigger.from_crontab("0 4 1 * *"), + 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..c8cfea164d --- /dev/null +++ b/tests/unit/test_comments/test_text_archive.py @@ -0,0 +1,690 @@ +import json +from datetime import timedelta +from io import StringIO + +import pytest # noqa +from django.contrib import admin +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 comments.admin import CommentAdmin +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, + list_archived_comment_ids, + sync_archived_comment_texts, + 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 + payload = json.loads(s3_stub[build_key(comment.pk)]) + assert payload["comment_id"] == comment.pk + assert payload["text"] == LONG_TEXT + + # Only a stub is left in both copies of the original text + comment.refresh_from_db() + assert comment.is_text_archived is True + assert comment.text_original == 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] + == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + ) + + 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 + assert ( + Comment.objects.rewrite(False) + .filter(pk=comment.pk) + .values_list("text", flat=True)[0] + == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + ) + + 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 TestCommentFullTextApiView: + 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-full-text", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 200 + assert response.data["text"] == LONG_TEXT + + 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-full-text", 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-full-text", 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-full-text", 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-full-text", 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-full-text", 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-full-text", 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-full-text", kwargs={"pk": comment.pk}) + ) + + assert response.status_code == 404 + + +class TestCommentAdminArchivedText: + """ + The admin is where staff investigate a comment, so it has to show the + archived text rather than the stub the row was left with. + """ + + @pytest.fixture() + def comment_admin(self): + return CommentAdmin(Comment, admin.site) + + def test_renders_the_archived_text(self, bot, post, s3_stub, comment_admin): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + comment.refresh_from_db() + + assert LONG_TEXT in comment_admin.archived_text(comment) + + def test_says_so_when_the_archive_cannot_be_read( + self, bot, post, s3_stub, comment_admin + ): + comment = factory_archivable_comment(bot, post) + archive_bot_comment_texts() + comment.refresh_from_db() + s3_stub.clear() + + assert "could not be retrieved" in comment_admin.archived_text(comment) + + def test_field_is_only_added_for_archived_comments( + self, bot, post, s3_stub, comment_admin + ): + comment = factory_archivable_comment(bot, post) + + assert "archived_text" not in comment_admin.get_fields(None, comment) + + archive_bot_comment_texts() + comment.refresh_from_db() + + assert "archived_text" in comment_admin.get_fields(None, comment) + + def test_text_and_archived_text_are_read_only_once_archived( + self, bot, post, s3_stub, comment_admin + ): + comment = factory_archivable_comment(bot, post) + + assert "text" not in comment_admin.get_readonly_fields(None, comment) + + archive_bot_comment_texts() + comment.refresh_from_db() + + readonly_fields = comment_admin.get_readonly_fields(None, comment) + assert "text" in readonly_fields + assert "text_original" in readonly_fields + assert "archived_text" in readonly_fields + + +@pytest.fixture() +def archived_elsewhere(bot, post, s3_stub): + """ + A comment whose text is in the archive while the row still holds it in + full: the state the production database is in after the uploads have been + performed against a copy of it. + """ + + comment = factory_archivable_comment(bot, post) + upload_text(comment.pk, LONG_TEXT) + + return comment + + +class TestListArchivedCommentIds: + def test_reads_ids_from_the_bucket(self, bot, post, s3_stub): + comments = [factory_archivable_comment(bot, post) for _ in range(3)] + for comment in comments: + upload_text(comment.pk, LONG_TEXT) + + assert list_archived_comment_ids() == {c.pk for c in comments} + + def test_ignores_keys_that_are_not_comment_ids(self, bot, post, s3_stub): + comment = factory_archivable_comment(bot, post) + upload_text(comment.pk, LONG_TEXT) + s3_stub["comments_text/not-an-id.json"] = "{}" + + assert list_archived_comment_ids() == {comment.pk} + + +class TestSyncArchivedCommentTexts: + def test_truncates_without_uploading(self, archived_elsewhere, s3_stub): + before = dict(s3_stub) + + stats = sync_archived_comment_texts() + + assert stats.synced == 1 + assert stats.chars_reclaimed == len(LONG_TEXT) - ARCHIVE_STUB_LENGTH + # Nothing was written back to the archive + assert s3_stub == before + + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.is_text_archived is True + assert archived_elsewhere.text_original == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + assert ( + Comment.objects.rewrite(False) + .filter(pk=archived_elsewhere.pk) + .values_list("text", flat=True)[0] + == LONG_TEXT[:ARCHIVE_STUB_LENGTH] + ) + + def test_does_not_bump_edited_at(self, archived_elsewhere, s3_stub): + edited_at = archived_elsewhere.edited_at + + sync_archived_comment_texts() + + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.edited_at == edited_at + + def test_dry_run_writes_nothing(self, archived_elsewhere, s3_stub): + stats = sync_archived_comment_texts(dry_run=True) + + assert stats.synced == 1 + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.is_text_archived is False + assert archived_elsewhere.text_original == LONG_TEXT + + def test_counts_already_truncated_rows(self, archived_elsewhere, s3_stub): + sync_archived_comment_texts() + + stats = sync_archived_comment_texts() + + assert stats.synced == 0 + assert stats.already_archived == 1 + + def test_counts_objects_with_no_comment(self, bot, post, s3_stub): + upload_text(999_999_999, LONG_TEXT) + + stats = sync_archived_comment_texts() + + assert stats.synced == 0 + assert stats.orphaned == 1 + + def test_ignores_keys_for_comments_the_archiver_would_never_upload( + self, user1, bot, post, s3_stub + ): + """ + The bucket says what was uploaded, not what may be truncated. A key + pointing at a public human comment is a mistake, not an instruction. + """ + + human = factory_archivable_comment(user1, post, is_private=False) + upload_text(human.pk, LONG_TEXT) + + stats = sync_archived_comment_texts() + + assert stats.synced == 0 + assert stats.ineligible == 1 + + human.refresh_from_db() + assert human.is_text_archived is False + assert human.text_original == LONG_TEXT + + def test_counts_rows_too_short_to_truncate_as_ineligible(self, bot, post, s3_stub): + short = factory_archivable_comment(bot, post, text="c" * ARCHIVE_STUB_LENGTH) + upload_text(short.pk, short.text) + + stats = sync_archived_comment_texts() + + assert stats.synced == 0 + assert stats.ineligible == 1 + + def test_verify_skips_rows_whose_archive_no_longer_matches( + self, archived_elsewhere, s3_stub + ): + upload_text(archived_elsewhere.pk, "something else entirely") + + stats = sync_archived_comment_texts(verify=True) + + assert stats.synced == 0 + assert stats.mismatched == 1 + + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.text_original == LONG_TEXT + + def test_verify_accepts_a_matching_archive(self, archived_elsewhere, s3_stub): + stats = sync_archived_comment_texts(verify=True) + + assert stats.synced == 1 + assert stats.mismatched == 0 + assert stats.verify_failed == 0 + + def test_verify_counts_an_unreadable_object_apart_from_a_mismatch( + self, archived_elsewhere, s3_stub, mocker + ): + """ + A read failure says nothing about whether the archive matches, so it + must not be reported as drift. + """ + + mocker.patch( + "comments.services.text_archive.fetch_text", + side_effect=RuntimeError("s3 is down"), + ) + + stats = sync_archived_comment_texts(verify=True) + + assert stats.synced == 0 + assert stats.mismatched == 0 + assert stats.verify_failed == 1 + + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.text_original == LONG_TEXT + + def test_verify_counts_a_missing_object_as_unreadable( + self, archived_elsewhere, s3_stub + ): + s3_stub.clear() + # Put the key back in the listing without a body behind it + s3_stub[build_key(archived_elsewhere.pk)] = None + + stats = sync_archived_comment_texts(verify=True) + + assert stats.synced == 0 + assert stats.mismatched == 0 + assert stats.verify_failed == 1 + + +class TestSyncCommand: + def test_syncs(self, archived_elsewhere, s3_stub): + out = StringIO() + + call_command("sync_archived_comment_texts", stdout=out) + + assert "Synced 1 of 1 archived object(s)" in out.getvalue() + archived_elsewhere.refresh_from_db() + assert archived_elsewhere.is_text_archived is True + + def test_errors_when_bucket_is_not_configured(self, settings): + settings.AWS_STORAGE_BUCKET_COMMENTS_TEXT = None + + with pytest.raises(CommandError): + call_command("sync_archived_comment_texts")