Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ echo "3.1 Running 'python manage.py fix_sortedm2m_columns' to fix any missing so
echo "******************************************"
python manage.py fix_sortedm2m_columns

echo "****************** STEP 4.6/5: docker-entrypoint.sh ************************"
echo "4.6 Running 'python manage.py backfill_num_pages' to fill missing publication page counts"
echo "******************************************"
python manage.py backfill_num_pages

# echo "****************** STEP 4.3/5: docker-entrypoint.sh ************************"
# echo "4.3 Running 'python manage.py rename_person_images' to rename person images"
# echo "******************************************"
Expand Down
5 changes: 5 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ djangorestframework==3.15.2
# See: https://pypi.org/project/Wand/
wand==0.6.13

# pypdf - pure-Python PDF library, used to auto-count a publication's pages
# (issue #1298) by reading the PDF's page tree directly (no rendering).
# See: https://pypi.org/project/pypdf/
pypdf==6.13.2

# Pillow - Python Imaging Library fork, for image manipulation
# Note: Pillow 10.0+ dropped Python 3.7 support; 11.0+ dropped Python 3.8
# See: https://pypi.org/project/Pillow/
Expand Down
72 changes: 72 additions & 0 deletions website/management/commands/backfill_num_pages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import logging
from django.core.management.base import BaseCommand
from website.models import Publication
import website.utils.fileutils as ml_fileutils

# This retrieves a Python logging instance (or creates it)
_logger = logging.getLogger(__name__)


class Command(BaseCommand):
help = (
"Backfills Publication.num_pages for existing publications that have a "
"PDF but no page count, by reading the page count directly from the PDF "
"(issue #1298). Only fills empty values, so manually entered counts are "
"never overwritten. Idempotent: once a publication has a count it is "
"skipped, so this is safe to run on every container start."
)

def add_arguments(self, parser):
parser.add_argument(
"--dry-run",
action="store_true",
help="Report what would change without writing to the database.",
)

def handle(self, *args, **options):
dry_run = options["dry_run"]
_logger.debug(
f"Running backfill_num_pages.py (dry_run={dry_run}) to populate "
f"num_pages for publications missing it."
)

# Only publications that have a PDF but no page count yet.
candidates = Publication.objects.filter(
num_pages__isnull=True
).exclude(pdf_file="").exclude(pdf_file__isnull=True)

num_updated = 0
num_skipped = 0
for pub in candidates:
page_count = ml_fileutils.get_pdf_page_count(pub.pdf_file)
if not page_count:
# Couldn't read the PDF (missing on disk, corrupt, etc.).
# Leave num_pages empty and move on.
_logger.debug(
f"Skipping pub id={pub.pk} '{pub.title}': could not determine "
f"page count from {pub.pdf_file.name}"
)
num_skipped += 1
continue

if dry_run:
_logger.debug(
f"[dry-run] Would set num_pages={page_count} for pub "
f"id={pub.pk} '{pub.title}'"
)
else:
# Write directly via the queryset so this stays a pure data
# backfill — no thumbnail regeneration or file-rename side
# effects from the model's save().
Publication.objects.filter(pk=pub.pk).update(num_pages=page_count)
_logger.debug(
f"Set num_pages={page_count} for pub id={pub.pk} '{pub.title}'"
)
num_updated += 1

verb = "Would update" if dry_run else "Updated"
_logger.info(
f"backfill_num_pages: {verb} {num_updated} publication(s); "
f"skipped {num_skipped} (PDF unreadable/missing)."
)
_logger.debug("Completed backfill_num_pages.py")
27 changes: 25 additions & 2 deletions website/models/publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from datetime import date # for date comparisons
import re # for regular expressions
import website.utils.timeutils as timeutils
import website.utils.fileutils as ml_fileutils # for auto-counting PDF pages

class PubAwardType(models.TextChoices):
BEST_ARTIFACT_AWARD = "Best Artifact Award"
Expand Down Expand Up @@ -77,8 +78,9 @@ class Publication(Artifact):
acmid = models.CharField(max_length=255, blank=True, null=True)

# Page numbers
num_pages = models.IntegerField(null=True)
num_pages.help_text = "The total number of pages in this publication (including references)"
num_pages = models.IntegerField(blank=True, null=True)
num_pages.help_text = "The total number of pages in this publication (including references). " \
"Leave blank to auto-calculate from the uploaded PDF."

# TODO, see if there is an IntegerRangeField or something like that for page_num_start and end
# There is an IntegerRangeField but it's only for Postgres... hmm
Expand All @@ -97,6 +99,27 @@ class Publication(Artifact):

award = models.CharField(max_length=50, choices=PubAwardType.choices, blank=True, null=True)

def save(self, *args, **kwargs):
"""
Extends Artifact.save() to auto-populate num_pages from the uploaded PDF
when it hasn't been set manually (issue #1298). This removes a required
field from the publication form so students can add papers with less
friction; the page count is still recorded, just derived automatically.

We only fill an empty value, so a manually entered num_pages is never
overwritten. The count is computed after super().save() because that's
when the PDF is guaranteed to be on disk (Artifact.save() also defers
thumbnail generation for the same reason). If we fill it, we persist with
a second, narrowly-scoped save(update_fields=['num_pages']).
"""
super().save(*args, **kwargs)

if self.pdf_file and not self.num_pages:
page_count = ml_fileutils.get_pdf_page_count(self.pdf_file)
if page_count:
self.num_pages = page_count
super().save(update_fields=['num_pages'])

def get_upload_dir(self, filename):
return os.path.join(self.UPLOAD_DIR, filename)

Expand Down
166 changes: 165 additions & 1 deletion website/tests/test_publication.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
"""Tests for Publication model methods (BibTeX, forum name, author lookup)."""

import io
import os
import tempfile
from unittest.mock import MagicMock

from django.test import SimpleTestCase
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import SimpleTestCase, override_settings

from website.tests.base import DatabaseTestCase


def _make_pdf_bytes(num_pages):
"""
Build a minimal, valid in-memory PDF with `num_pages` blank pages, using
pypdf's own writer. Guarantees a real PDF (so pypdf can re-read its page
tree) with a known page count for assertions.
"""
from pypdf import PdfWriter
writer = PdfWriter()
for _ in range(num_pages):
writer.add_blank_page(width=612, height=792) # US Letter
buf = io.BytesIO()
writer.write(buf)
return buf.getvalue()


# --- BibTeX citation regression -------------------------------------------


Expand Down Expand Up @@ -209,3 +228,148 @@ def test_returns_first_author_in_sorted_order(self):
second = self.make_person(first_name="Second", last_name="Author")
pub.authors.add(first, second) # SortedManyToManyField preserves order
self.assertEqual(pub.get_person(), first)


# --- get_pdf_page_count helper (#1298) ------------------------------------


class GetPdfPageCountTests(SimpleTestCase):
"""
Unit tests for fileutils.get_pdf_page_count, which backs the auto-population
of Publication.num_pages (#1298). The helper must return the page count for a
valid PDF and None (never raise) for anything it can't read, so a bad upload
never blocks saving a publication.
"""

def _mock_field(self, name, path, exists=True):
"""Build a minimal FileField stand-in with the attrs the helper reads."""
field = MagicMock()
field.name = name
field.path = path
field.storage.exists.return_value = exists
return field

def test_returns_page_count_for_valid_pdf(self):
from website.utils import fileutils
with tempfile.NamedTemporaryFile(suffix=".pdf", delete=False) as tmp:
tmp.write(_make_pdf_bytes(7))
tmp_path = tmp.name
try:
field = self._mock_field("paper.pdf", tmp_path)
self.assertEqual(fileutils.get_pdf_page_count(field), 7)
finally:
os.unlink(tmp_path)

def test_returns_none_for_non_pdf_extension(self):
from website.utils import fileutils
field = self._mock_field("notes.txt", "/tmp/notes.txt")
self.assertIsNone(fileutils.get_pdf_page_count(field))

def test_returns_none_when_file_missing_from_storage(self):
from website.utils import fileutils
field = self._mock_field("paper.pdf", "/tmp/does-not-exist.pdf", exists=False)
self.assertIsNone(fileutils.get_pdf_page_count(field))

def test_returns_none_for_corrupt_pdf(self):
from website.utils import fileutils
with tempfile.NamedTemporaryFile(suffix=".pdf", delete=False) as tmp:
tmp.write(b"%PDF-1.4 not actually a real pdf")
tmp_path = tmp.name
try:
field = self._mock_field("paper.pdf", tmp_path)
self.assertIsNone(fileutils.get_pdf_page_count(field))
finally:
os.unlink(tmp_path)

def test_returns_none_for_empty_field(self):
from website.utils import fileutils
empty = MagicMock()
empty.name = ""
self.assertIsNone(fileutils.get_pdf_page_count(empty))


# --- num_pages auto-population on save (#1298) ----------------------------


@override_settings(MEDIA_ROOT=tempfile.mkdtemp())
class NumPagesAutoFillTests(DatabaseTestCase):
"""
Integration tests for Publication.save() auto-filling num_pages from the
uploaded PDF (#1298). Uses a temp MEDIA_ROOT so the real PDFs/thumbnails the
save path writes don't pollute the project's media/ directory.
"""

def _pdf_upload(self, name, num_pages):
return SimpleUploadedFile(
name, _make_pdf_bytes(num_pages), content_type="application/pdf"
)

def test_num_pages_autofilled_from_pdf(self):
pub = self.make_publication(
title="Auto Pages", pdf_file=self._pdf_upload("auto.pdf", 5)
)
pub.refresh_from_db()
self.assertEqual(pub.num_pages, 5)

def test_manual_num_pages_is_preserved(self):
"""A page count the editor typed in must never be overwritten."""
pub = self.make_publication(
title="Manual Pages",
num_pages=99,
pdf_file=self._pdf_upload("manual.pdf", 5),
)
pub.refresh_from_db()
self.assertEqual(pub.num_pages, 99)


# --- backfill_num_pages management command (#1298) ------------------------


@override_settings(MEDIA_ROOT=tempfile.mkdtemp())
class BackfillNumPagesCommandTests(DatabaseTestCase):
"""
Tests for the backfill_num_pages management command, which populates
num_pages for legacy publications that have a PDF but no page count (#1298).
"""

def _pdf_upload(self, name, num_pages):
return SimpleUploadedFile(
name, _make_pdf_bytes(num_pages), content_type="application/pdf"
)

def _make_legacy_pub(self, title, num_pages_in_pdf):
"""
Create a publication and then null out num_pages directly in the DB to
simulate a legacy row (save() auto-fills it, so we can't create one with
an empty count through the normal path).
"""
from website.models import Publication
pub = self.make_publication(
title=title, pdf_file=self._pdf_upload(f"{title}.pdf", num_pages_in_pdf)
)
Publication.objects.filter(pk=pub.pk).update(num_pages=None)
return pub

def test_backfills_missing_page_count(self):
from django.core.management import call_command
pub = self._make_legacy_pub("Legacy", 8)
call_command("backfill_num_pages")
pub.refresh_from_db()
self.assertEqual(pub.num_pages, 8)

def test_dry_run_makes_no_changes(self):
from django.core.management import call_command
pub = self._make_legacy_pub("DryRun", 8)
call_command("backfill_num_pages", "--dry-run")
pub.refresh_from_db()
self.assertIsNone(pub.num_pages)

def test_existing_count_is_not_overwritten(self):
"""Publications that already have a count are left untouched."""
from django.core.management import call_command
pub = self.make_publication(
title="HasCount", num_pages=42, pdf_file=self._pdf_upload("hc.pdf", 8)
)
call_command("backfill_num_pages")
pub.refresh_from_db()
self.assertEqual(pub.num_pages, 42)
46 changes: 44 additions & 2 deletions website/utils/fileutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import time # for generating unique filenames
from wand.image import Image, Color # for creating thumbnails
from wand.exceptions import WandException# for creating thumbnails
from pypdf import PdfReader # for counting PDF pages
from pypdf.errors import PdfReadError # for counting PDF pages

# This retrieves a Python logging instance (or creates it)
_logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -262,5 +264,45 @@ def generate_thumbnail_for_pdf(pdf_file_field, thumbnail_image_field, thumbnail_

_logger.error(f"Thumbnail generation failed at all resolutions for: {pdf_file_field.name}")
return None




def get_pdf_page_count(pdf_file_field):
"""
Returns the number of pages in the given PDF, or None if the count can't be
determined (no file, not a PDF, file missing on disk, or unreadable/corrupt).

Uses pypdf, which reads the PDF's internal page tree directly rather than
rendering pages, so it stays fast and light even for large documents. This
backs the auto-population of Publication.num_pages (issue #1298) so students
no longer have to count pages by hand.

Returning None rather than raising is intentional: a malformed PDF should
never block saving a publication. The caller simply leaves num_pages unset.

Args:
pdf_file_field (models.FileField): The PDF file field to inspect.

Returns:
int | None: The page count, or None if it can't be determined.

Example:
>>> get_pdf_page_count(publication.pdf_file)
12
"""
if not pdf_file_field or not pdf_file_field.name:
return None

if not pdf_file_field.name.lower().endswith('.pdf'):
_logger.debug(f"Not counting pages for non-PDF file: {pdf_file_field.name}")
return None

if not pdf_file_field.storage.exists(pdf_file_field.name):
_logger.debug(f"Cannot count pages; file not found in storage: {pdf_file_field.name}")
return None

try:
reader = PdfReader(pdf_file_field.path)
return len(reader.pages)
except (PdfReadError, OSError, ValueError) as e:
_logger.warning(f"Could not determine page count for {pdf_file_field.name}: {e}")
return None
Loading