From af2e42f01546f4442d152ee1792c0b6f484e2920 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 17 Jun 2024 01:25:11 -0400 Subject: [PATCH 1/3] Fix requirements.in to match #4890 `pip-compile` was attempting to downgrade `django-allauth` because the last upgrade was done directly the `.txt` requirements files instead of `requirements.in` --- dependencies/pip/requirements.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies/pip/requirements.in b/dependencies/pip/requirements.in index 4179d9a0e5..86899e795f 100644 --- a/dependencies/pip/requirements.in +++ b/dependencies/pip/requirements.in @@ -30,7 +30,7 @@ celery[redis] dict2xml dj-static dj-stripe -django-allauth<0.58 # Backwards incompatible changes +django-allauth django-braces django-celery-beat django-constance From 2d03413365c69ad9d52c081e7a96e21ffba6f89d Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 17 Jun 2024 01:30:02 -0400 Subject: [PATCH 2/3] =?UTF-8?q?Update=20formpack=20requirement=20to=20incl?= =?UTF-8?q?ude=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kobotoolbox/formpack#323 --- dependencies/pip/dev_requirements.txt | 11 +---------- dependencies/pip/requirements.in | 2 +- dependencies/pip/requirements.txt | 7 +------ 3 files changed, 3 insertions(+), 17 deletions(-) diff --git a/dependencies/pip/dev_requirements.txt b/dependencies/pip/dev_requirements.txt index bb7e8d031e..420a1d637f 100644 --- a/dependencies/pip/dev_requirements.txt +++ b/dependencies/pip/dev_requirements.txt @@ -8,7 +8,7 @@ # via -r dependencies/pip/requirements.in -e git+https://github.com/trevoriancox/django-dont-vary-on.git@01a804122b7ddcdc22f50b40993f91c27b03bef6#egg=django-dont-vary-on # via -r dependencies/pip/requirements.in --e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack +-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack # via -r dependencies/pip/requirements.in -e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account # via -r dependencies/pip/requirements.in @@ -530,15 +530,6 @@ six==1.16.0 # via # asttokens # azure-core - # bcrypt - # click-repl - # django-organizations - # google-auth - # google-auth-httplib2 - # grpcio - # mongomock - # paramiko - # pathlib2 # isodate # python-dateutil smsapi-client==2.9.5 diff --git a/dependencies/pip/requirements.in b/dependencies/pip/requirements.in index 86899e795f..cf74602ea5 100644 --- a/dependencies/pip/requirements.in +++ b/dependencies/pip/requirements.in @@ -2,7 +2,7 @@ # https://github.com/bndr/pipreqs is a handy utility, too. # formpack --e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack +-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack # service-account -e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account diff --git a/dependencies/pip/requirements.txt b/dependencies/pip/requirements.txt index e1aedfaad1..ed16aadb32 100644 --- a/dependencies/pip/requirements.txt +++ b/dependencies/pip/requirements.txt @@ -8,7 +8,7 @@ # via -r dependencies/pip/requirements.in -e git+https://github.com/trevoriancox/django-dont-vary-on.git@01a804122b7ddcdc22f50b40993f91c27b03bef6#egg=django-dont-vary-on # via -r dependencies/pip/requirements.in --e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack +-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack # via -r dependencies/pip/requirements.in -e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account # via -r dependencies/pip/requirements.in @@ -443,11 +443,6 @@ shortuuid==1.0.13 six==1.16.0 # via # azure-core - # click-repl - # django-organizations - # google-auth - # google-auth-httplib2 - # grpcio # isodate # python-dateutil smsapi-client==2.9.5 From a4dfe912a6a1d9d273ec31d7ae06456e071dd258 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 17 Jun 2024 01:31:03 -0400 Subject: [PATCH 3/3] Repair damage from kobotoolbox/formpack#322 --- kobo/apps/reports/report_data.py | 5 +++ kpi/models/asset_version.py | 2 + kpi/serializers/v2/asset.py | 1 + kpi/utils/bugfix.py | 67 ++++++++++++++++++++++++++++++++ kpi/views/v2/asset.py | 13 ++++++- 5 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 kpi/utils/bugfix.py diff --git a/kobo/apps/reports/report_data.py b/kobo/apps/reports/report_data.py index 0661ec3324..cb90a15411 100644 --- a/kobo/apps/reports/report_data.py +++ b/kobo/apps/reports/report_data.py @@ -6,6 +6,7 @@ from rest_framework import serializers from formpack import FormPack +from kpi.utils.bugfix import repair_file_column_content_and_save from kpi.utils.log import logging from .constants import ( FUZZY_VERSION_ID_KEY, @@ -21,6 +22,10 @@ def build_formpack(asset, submission_stream=None, use_all_form_versions=True): are assumed to have been collected with that version of the form. """ + # Cope with kobotoolbox/formpack#322, which wrote invalid content into the + # database + repair_file_column_content_and_save(asset) + if asset.has_deployment: if use_all_form_versions: _versions = asset.deployed_versions diff --git a/kpi/models/asset_version.py b/kpi/models/asset_version.py index eeea8423ad..0bddf9e528 100644 --- a/kpi/models/asset_version.py +++ b/kpi/models/asset_version.py @@ -29,6 +29,8 @@ class AssetVersion(models.Model): ) version_content = models.JSONField() uid_aliases = models.JSONField(null=True) + # Tee hee, `deployed_content` is never written to the database! + # TODO: It should be changed to a property instead, no? deployed_content = models.JSONField(null=True) _deployment_data = models.JSONField(default=dict) deployed = models.BooleanField(default=False) diff --git a/kpi/serializers/v2/asset.py b/kpi/serializers/v2/asset.py index ebe0b9576d..96e696a780 100644 --- a/kpi/serializers/v2/asset.py +++ b/kpi/serializers/v2/asset.py @@ -893,6 +893,7 @@ def validate_settings(self, settings: dict) -> dict: return {**self.instance.settings, **settings} def _content(self, obj): + # FIXME: Is this dead code? return json.dumps(obj.content) def _get_status(self, perm_assignments): diff --git a/kpi/utils/bugfix.py b/kpi/utils/bugfix.py new file mode 100644 index 0000000000..96f0e5be93 --- /dev/null +++ b/kpi/utils/bugfix.py @@ -0,0 +1,67 @@ +# coding: utf-8 + +import datetime + +from django.db import transaction +from formpack.utils.bugfix import repair_file_column_content_in_place + +from kpi.models import Asset, AssetVersion + + +def repair_file_column_content_and_save(asset, include_versions=True) -> bool: + """ + Given an `Asset`, repair any damage caused to its `content` and the + `version_content` of all related `AssetVersion`s by + kobotoolbox/formpack#322, which wrongly transformed the `file` column into + `media::file` and included it in the list of translated columns. + + Writes only to the `content` (or `version_content`) field for each modified + model instance. + + Returns `True` if any change was made + """ + + # When was this bug introduced? See formpack commit + # 443a8e940756976a9f88fb577dbbc53510726536 + BAD_TIME = datetime.datetime( + 2024, 4, 30, 15, 27, 2, tzinfo=datetime.timezone.utc + ) + + any_change = False + with transaction.atomic(): + # Lock and refetch the latest content to guard against clobbering + # concurrent updates + content = ( + Asset.objects.filter(pk=asset.pk, date_modified__gte=BAD_TIME) + .select_for_update() + .values('content') + .first() + ) + if not content: + # This asset was not modified recently enough to be affected by + # this bug + return False + + if repair_file_column_content_in_place(content): + Asset.objects.filter(pk=asset.pk).update(content=content) + any_change = True + # Also update the in-memory asset instance passed to this function + asset.content = content + + if not include_versions: + return any_change + + # Previous versions of the content may need repair, regardless of + # whether or not the current content did + for pk, version_content in ( + asset.asset_versions.filter(date_modified__gte=BAD_TIME) + .select_for_update() + .values_list('pk', 'version_content') + ): + if repair_file_column_content_in_place(version_content): + AssetVersion.objects.filter(pk=pk).update( + version_content=version_content + ) + any_change = True + + return any_change diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index 36845e5401..1e70df08ca 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -55,8 +55,9 @@ AssetListSerializer, AssetSerializer, ) -from kpi.utils.hash import calculate_hash from kpi.serializers.v2.reports import ReportsDetailSerializer +from kpi.utils.bugfix import repair_file_column_content_and_save +from kpi.utils.hash import calculate_hash from kpi.utils.kobo_to_xlsform import to_xlsform_structure from kpi.utils.ss_structure_to_mdtable import ss_structure_to_mdtable from kpi.utils.object_permission import ( @@ -403,6 +404,16 @@ def get_object(self): raise Http404 self.check_object_permissions(self.request, asset) + + # Cope with kobotoolbox/formpack#322, which wrote invalid content + # into the database. For performance, consider only the current + # content, not previous versions. Previous versions are handled in + # `kobo.apps.reports.report_data.build_formpack()` + if self.request.method == 'GET': + repair_file_column_content_and_save( + asset, include_versions=False + ) + return asset return super().get_object()