From 3e5aa61ee59396132d3f4d1ab6ae8eb790ab1370 Mon Sep 17 00:00:00 2001
From: Magnus Hagander <magnus@hagander.net>
Date: Mon, 5 Aug 2024 14:00:57 +0200
Subject: [PATCH] Calculate, store and use etags on speaker photos

This allows us to return 304 not modified for speaker photos, as they
change very infrequently. Actual etag is maintained as an md5() of the
contents, by a database trigger the same way we do for util/storage
fields.
---
 .../migrations/0115_speaker_photo_hashvals.py | 47 +++++++++++++++++++
 postgresqleu/confreg/models.py                |  2 +
 postgresqleu/confreg/views.py                 | 29 +++++++++---
 3 files changed, 71 insertions(+), 7 deletions(-)
 create mode 100644 postgresqleu/confreg/migrations/0115_speaker_photo_hashvals.py

diff --git a/postgresqleu/confreg/migrations/0115_speaker_photo_hashvals.py b/postgresqleu/confreg/migrations/0115_speaker_photo_hashvals.py
new file mode 100644
index 00000000..8a4172f1
--- /dev/null
+++ b/postgresqleu/confreg/migrations/0115_speaker_photo_hashvals.py
@@ -0,0 +1,47 @@
+# Generated by Django 4.2.11 on 2024-08-05 11:38
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('confreg', '0114_conference_callforpapersmaxsubmissions'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='speaker',
+            name='photo512_hashval',
+            field=models.BinaryField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='speaker',
+            name='photo_hashval',
+            field=models.BinaryField(blank=True, null=True),
+        ),
+        migrations.RunSQL(
+            """
+CREATE FUNCTION confreg_speaker_update_hash() RETURNS trigger AS $$
+BEGIN
+        NEW.photo_hashval = decode(md5(NEW.photo), 'hex');
+        NEW.photo512_hashval = decode(md5(NEW.photo512), 'hex');
+        RETURN NEW;
+END;
+$$ LANGUAGE plpgsql
+        """,
+            "DROP FUNCTION confreg_speaker_update_hash();",
+        ),
+        migrations.RunSQL(
+            """
+CREATE TRIGGER confreg_speaker_update_hash_trigger
+BEFORE INSERT OR UPDATE OF photo, photo512 ON confreg_speaker
+FOR EACH ROW EXECUTE FUNCTION confreg_speaker_update_hash();
+            """,
+            "DROP TRIGGER confreg_speaker_update_hash_trigger",
+        ),
+        migrations.RunSQL(
+            "UPDATE confreg_speaker SET photo_hashval=decode(md5(photo), 'hex'), photo512_hashval=decode(md5(photo512), 'hex') WHERE photo IS NOT NULL OR photo512 IS NOT NULL",
+            "",
+        ),
+    ]
diff --git a/postgresqleu/confreg/models.py b/postgresqleu/confreg/models.py
index 15d57257..7216cb70 100644
--- a/postgresqleu/confreg/models.py
+++ b/postgresqleu/confreg/models.py
@@ -980,7 +980,9 @@ class Speaker(models.Model):
     company = models.CharField(max_length=100, null=False, blank=True)
     abstract = models.TextField(null=False, blank=True, verbose_name="Bio")
     photo = ImageBinaryField(blank=True, null=True, verbose_name="Photo (low res)", max_length=1000000, resolution=(128, 128))
+    photo_hashval = models.BinaryField(blank=True, null=True)
     photo512 = ImageBinaryField(blank=True, null=True, verbose_name="Photo", max_length=1000000, resolution=PRIMARY_SPEAKER_PHOTO_RESOLUTION, auto_scale=True)
+    photo512_hashval = models.BinaryField(blank=True, null=True)
     lastmodified = models.DateTimeField(auto_now=True, null=False, blank=False)
     speakertoken = models.TextField(null=False, blank=False, unique=True)
     attributes = models.JSONField(blank=True, null=False, default=dict)
diff --git a/postgresqleu/confreg/views.py b/postgresqleu/confreg/views.py
index f91f11dd..3ba7eb80 100644
--- a/postgresqleu/confreg/views.py
+++ b/postgresqleu/confreg/views.py
@@ -4,7 +4,7 @@ from django.shortcuts import render, get_object_or_404
 from django.core.exceptions import PermissionDenied
 from django.core import paginator
 from django.http import HttpResponseRedirect, HttpResponsePermanentRedirect, HttpResponse, Http404
-from django.http import HttpResponseForbidden
+from django.http import HttpResponseForbidden, HttpResponseNotModified
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.contrib import messages
@@ -1704,19 +1704,34 @@ def speaker_card(request, confname, speakerid, cardformat):
 
 
 def speakerphoto(request, speakerid, phototype='1/'):
-    speaker = get_object_or_404(Speaker, pk=speakerid)
+    # Fetch just the hashes so we can return 304 cheaply on not modified
+    speaker = get_object_or_404(Speaker.objects.only('id', 'photo_hashval', 'photo512_hashval'), pk=speakerid)
     if phototype is None or phototype == '1/':
-        if not speaker.photo:
+        if not speaker.photo_hashval:
             raise Http404()
-        photo = bytes(speaker.photo)
+        etag = speaker.photo_hashval
     elif phototype == '5/':
-        if not speaker.photo512:
+        if not speaker.photo512_hashval:
             raise Http404()
-        photo = bytes(speaker.photo512)
+        etag = speaker.photo512_hashval
     else:
         raise Http404()
+    etag = '"' + etag.hex() + '"'
+
+    if 'If-None-Match' in request.headers:
+        if request.headers['If-None-Match'] == etag:
+            return HttpResponseNotModified()
+
+    # Only fetch the actual photo when needed
+    if phototype is None or phototype == '1/':
+        photo = bytes(speaker.photo)
+    else:
+        photo = bytes(speaker.photo512)
+
     content_type = 'image/png' if photo[:8] == b'\x89\x50\x4E\x47\x0D\x0A\x1A\x0A' else 'image/jpg'
-    return HttpResponse(photo, content_type=content_type)
+    r = HttpResponse(photo, content_type=content_type)
+    r['ETag'] = etag
+    return r
 
 
 @login_required
-- 
2.39.5