From 8ab8b1e8e51c8f1af8a3908029ec2ca8c10418f0 Mon Sep 17 00:00:00 2001 From: Sergei Maertens <sergei@maykinmedia.nl> Date: Mon, 9 Dec 2024 16:03:30 +0100 Subject: [PATCH 1/2] :boom: [#3283] Make TemporaryFileUpload.submission non-nullable We've provided a transition period from Open Forms 2.7, which ensures that a submission is stored. Support for legacy uploads is now removed. The celery background jobs should by now have deleted all the old unclaimed uploads, unless absurdly large retention values have been set. --- docs/installation/upgrade-300.rst | 12 +++++++ src/openforms/formio/components/vanilla.py | 5 +-- ...non_legacy_submission_not_null_and_more.py | 32 +++++++++++++++++++ .../submissions/models/submission_files.py | 16 ---------- .../submissions/tests/test_models.py | 18 ----------- 5 files changed, 45 insertions(+), 38 deletions(-) create mode 100644 src/openforms/submissions/migrations/0013_remove_temporaryfileupload_non_legacy_submission_not_null_and_more.py diff --git a/docs/installation/upgrade-300.rst b/docs/installation/upgrade-300.rst index 82c096d543..02f3b24ad7 100644 --- a/docs/installation/upgrade-300.rst +++ b/docs/installation/upgrade-300.rst @@ -103,3 +103,15 @@ Removal of /api/v2/location/get-street-name-and-city endpoint The /api/v2/location/get-street-name-and-city was deprecated for some time, and is now removed in favor of the /api/v2/geo/address-autocomplete endpoint. + +Legacy temporary file uploads no longer supported +================================================= + +Before Open Forms 2.7.0, temporary file uploads (as created by the file upload form +component) would not be related to the submission they were uploaded in. We call these +legacy temporary file uploads, and support for them has been removed in Open Forms 3.0. + +The setting ``TEMPORARY_UPLOADS_REMOVED_AFTER_DAYS`` controls how long file uploads are +kept. Ensure that this many days have passed since the last legacy upload before +upgrading to Open Forms 3.0, otherwise you will run into database errors during the +upgrade. diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index ff866894b8..109cb0560d 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -383,10 +383,7 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: {"originalName": _("Name does not match the uploaded file.")} ) - if ( - not temporary_upload.legacy - and temporary_upload.submission != self.context["submission"] - ): + if temporary_upload.submission != self.context["submission"]: raise serializers.ValidationError({"url": _("Invalid URL.")}) with temporary_upload.content.open("rb") as infile: diff --git a/src/openforms/submissions/migrations/0013_remove_temporaryfileupload_non_legacy_submission_not_null_and_more.py b/src/openforms/submissions/migrations/0013_remove_temporaryfileupload_non_legacy_submission_not_null_and_more.py new file mode 100644 index 0000000000..8807109dee --- /dev/null +++ b/src/openforms/submissions/migrations/0013_remove_temporaryfileupload_non_legacy_submission_not_null_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.17 on 2024-12-09 15:04 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("submissions", "0012_alter_submission_price"), + ] + + operations = [ + migrations.RemoveConstraint( + model_name="temporaryfileupload", + name="non_legacy_submission_not_null", + ), + migrations.RemoveField( + model_name="temporaryfileupload", + name="legacy", + ), + migrations.AlterField( + model_name="temporaryfileupload", + name="submission", + field=models.ForeignKey( + help_text="Submission the temporary file upload belongs to.", + on_delete=django.db.models.deletion.CASCADE, + to="submissions.submission", + verbose_name="submission", + ), + ), + ] diff --git a/src/openforms/submissions/models/submission_files.py b/src/openforms/submissions/models/submission_files.py index 0d6998160c..c0f039c7c3 100644 --- a/src/openforms/submissions/models/submission_files.py +++ b/src/openforms/submissions/models/submission_files.py @@ -8,7 +8,6 @@ from django.core.files.base import File from django.db import models -from django.db.models import Q from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -49,17 +48,9 @@ def select_prune(self, age: timedelta): class TemporaryFileUpload(DeleteFileFieldFilesMixin, models.Model): uuid = models.UUIDField(_("UUID"), unique=True, default=uuid.uuid4) - legacy = models.BooleanField( - _("legacy"), - default=False, - help_text=_("Whether the instance is linked to a submission instance."), - ) - # TODO DeprecationWarning null=True is a transitional state, and should be removed - # at some point: submission = models.ForeignKey( "submissions.Submission", on_delete=models.CASCADE, - null=True, verbose_name=_("submission"), help_text=_("Submission the temporary file upload belongs to."), ) @@ -83,13 +74,6 @@ class TemporaryFileUpload(DeleteFileFieldFilesMixin, models.Model): class Meta: verbose_name = _("temporary file upload") verbose_name_plural = _("temporary file uploads") - constraints = [ - models.CheckConstraint( - check=(Q(legacy=False) & Q(submission__isnull=False)) - | (Q(legacy=True) & Q(submission__isnull=True)), - name="non_legacy_submission_not_null", - ), - ] class SubmissionFileAttachmentQuerySet( diff --git a/src/openforms/submissions/tests/test_models.py b/src/openforms/submissions/tests/test_models.py index 4f6b5ca2ce..ac8e757e76 100644 --- a/src/openforms/submissions/tests/test_models.py +++ b/src/openforms/submissions/tests/test_models.py @@ -3,7 +3,6 @@ from unittest.mock import patch from django.core.exceptions import ValidationError -from django.db import IntegrityError from django.test import TestCase, override_settings, tag from freezegun import freeze_time @@ -23,7 +22,6 @@ SubmissionFileAttachmentFactory, SubmissionReportFactory, SubmissionStepFactory, - TemporaryFileUploadFactory, ) @@ -589,19 +587,3 @@ def test_names_do_not_break_pdf_saving_to_disk(self): report.generate_submission_report_pdf() self.assertTrue(report.content.storage.exists(report.content.name)) - - -class TemporaryFileUploadTests(TestCase): - def test_legacy_check_constraint(self): - with self.assertRaises(IntegrityError): - TemporaryFileUploadFactory.create( - submission=None, - legacy=False, - ) - - def test_non_legacy_check_constraint(self): - with self.assertRaises(IntegrityError): - TemporaryFileUploadFactory.create( - submission=SubmissionFactory.create(), - legacy=True, - ) From 6adb90d82b4fea1a76593903aa388df5140bf9d1 Mon Sep 17 00:00:00 2001 From: Sergei Maertens <sergei@maykinmedia.nl> Date: Tue, 10 Dec 2024 14:00:24 +0100 Subject: [PATCH 2/2] :technologist: [#3283] Add upgrade check script Prevent upgrades going through if known constraint violations exist in the database. --- Dockerfile | 1 + bin/check_temporary_uploads.py | 33 +++++++++++++++++++++++++ src/openforms/upgrades/upgrade_paths.py | 6 +++++ 3 files changed, 40 insertions(+) create mode 100755 bin/check_temporary_uploads.py diff --git a/Dockerfile b/Dockerfile index 7e00153532..3d48433480 100644 --- a/Dockerfile +++ b/Dockerfile @@ -92,6 +92,7 @@ RUN mkdir /app/bin /app/log /app/media /app/private_media /app/certifi_ca_bundle COPY \ ./bin/check_celery_worker_liveness.py \ ./bin/report_component_problems.py \ + ./bin/check_temporary_uploads.py \ ./bin/ # prevent writing to the container layer, which would degrade performance. diff --git a/bin/check_temporary_uploads.py b/bin/check_temporary_uploads.py new file mode 100755 index 0000000000..1b4bf3d4e3 --- /dev/null +++ b/bin/check_temporary_uploads.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python +import sys +from pathlib import Path + +import django + +SRC_DIR = Path(__file__).parent.parent / "src" +sys.path.insert(0, str(SRC_DIR.resolve())) + + +def check_for_null_submissions_in_temporary_uploads(): + from openforms.submissions.models import TemporaryFileUpload + + problematic_uploads = TemporaryFileUpload.objects.filter(submission__isnull=True) + if problematic_uploads.exists(): + print("There are still legacy temporary uploads. You must clear those first.") + return False + + return True + + +def main(skip_setup=False) -> bool: + from openforms.setup import setup_env + + if not skip_setup: + setup_env() + django.setup() + + return check_for_null_submissions_in_temporary_uploads() + + +if __name__ == "__main__": + main() diff --git a/src/openforms/upgrades/upgrade_paths.py b/src/openforms/upgrades/upgrade_paths.py index 540e8a3df5..4d7d79c532 100644 --- a/src/openforms/upgrades/upgrade_paths.py +++ b/src/openforms/upgrades/upgrade_paths.py @@ -76,6 +76,12 @@ def run_checks(self) -> bool: # If your current version falls outside of a supported range, you need to do another # upgrade path (first) or there is no upgrade path at all. UPGRADE_PATHS = { + "3.0": UpgradeConstraint( + valid_ranges={ + VersionRange(minimum="2.8.0"), + }, + scripts=["check_temporary_uploads"], + ), "2.8": UpgradeConstraint( valid_ranges={ VersionRange(minimum="2.7.4"),