From 77056dcf0ba6375b32e543580a09e3cfa70f44c7 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Thu, 23 Mar 2023 10:56:22 +0100 Subject: [PATCH 1/3] :bug: [#324] Fixed retrieval of overlapping objects by filtering with exclusive end date. --- src/objects/api/v1/openapi.yaml | 8 +-- src/objects/api/v2/openapi.yaml | 8 +-- .../migrations/0028_auto_20230331_1239.py | 30 ++++++++ src/objects/core/models.py | 6 +- src/objects/core/query.py | 4 +- src/objects/tests/v2/test_filters.py | 47 ++++++++++++ src/objects/tests/v2/test_object_api.py | 72 +++++++++++++++++++ 7 files changed, 163 insertions(+), 12 deletions(-) create mode 100644 src/objects/core/migrations/0028_auto_20230331_1239.py diff --git a/src/objects/api/v1/openapi.yaml b/src/objects/api/v1/openapi.yaml index 4892a747..880fffd2 100644 --- a/src/objects/api/v1/openapi.yaml +++ b/src/objects/api/v1/openapi.yaml @@ -654,12 +654,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date @@ -801,12 +801,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date diff --git a/src/objects/api/v2/openapi.yaml b/src/objects/api/v2/openapi.yaml index 0bd6bf1d..6540a71b 100644 --- a/src/objects/api/v2/openapi.yaml +++ b/src/objects/api/v2/openapi.yaml @@ -742,12 +742,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date @@ -894,12 +894,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date diff --git a/src/objects/core/migrations/0028_auto_20230331_1239.py b/src/objects/core/migrations/0028_auto_20230331_1239.py new file mode 100644 index 00000000..5d47b4b4 --- /dev/null +++ b/src/objects/core/migrations/0028_auto_20230331_1239.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.28 on 2023-03-31 10:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0027_auto_20211203_1209"), + ] + + operations = [ + migrations.AlterField( + model_name="objectrecord", + name="end_at", + field=models.DateField( + help_text="Legal end date of the object record (exclusive)", + null=True, + verbose_name="end at", + ), + ), + migrations.AlterField( + model_name="objectrecord", + name="start_at", + field=models.DateField( + help_text="Legal start date of the object record (inclusive)", + verbose_name="start at", + ), + ), + ] diff --git a/src/objects/core/models.py b/src/objects/core/models.py index 10894698..122bb71c 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -98,10 +98,12 @@ class ObjectRecord(models.Model): encoder=DjangoJSONEncoder, ) start_at = models.DateField( - _("start at"), help_text=_("Legal start date of the object record") + _("start at"), help_text=_("Legal start date of the object record (inclusive)") ) end_at = models.DateField( - _("end at"), null=True, help_text=_("Legal end date of the object record") + _("end at"), + null=True, + help_text=_("Legal end date of the object record (exclusive)"), ) registration_at = models.DateField( _("registration at"), diff --git a/src/objects/core/query.py b/src/objects/core/query.py index 84514c6e..bc9a5346 100644 --- a/src/objects/core/query.py +++ b/src/objects/core/query.py @@ -16,7 +16,7 @@ def filter_for_date(self, date): return ( self.filter(records__start_at__lte=date) .filter( - models.Q(records__end_at__gte=date) + models.Q(records__end_at__gt=date) | models.Q(records__end_at__isnull=True) ) .distinct() @@ -59,7 +59,7 @@ def filter_for_date(self, date): """ return self.filter(start_at__lte=date).filter( - models.Q(end_at__gte=date) | models.Q(end_at__isnull=True) + models.Q(end_at__gt=date) | models.Q(end_at__isnull=True) ) def filter_for_registration_date(self, date): diff --git a/src/objects/tests/v2/test_filters.py b/src/objects/tests/v2/test_filters.py index 52ed5ddc..e8f3d9c2 100644 --- a/src/objects/tests/v2/test_filters.py +++ b/src/objects/tests/v2/test_filters.py @@ -377,6 +377,53 @@ def test_filter_exclude_old_records(self): data = response.json()["results"] self.assertEqual(len(data), 0) + response = self.client.get(self.url, {"data_attrs": "diameter__exact__50"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1) + + def test_filter_exclude_old_records_issue_324(self): + """ + Filter and conflict resolution. + + If record A ends at X and a newer record B starts at X, both match on + date X. Normally, conflict resolution returns only record B. However, + when you add a filter, it first finds the record matching this filter. + If the filter only results in record A, there are no conflicts and + record A is returned. + """ + record_old = ObjectRecordFactory.create( + data={"adres": {"straat": "Bospad"}}, + object__object_type=self.object_type, + start_at=date.today() - timedelta(days=1), + end_at=date.today(), + ) + record_new = ObjectRecordFactory.create( + data={"adres": {"straat": "Dorpsstraat"}}, + object=record_old.object, + start_at=record_old.end_at, + ) + + response = self.client.get( + self.url, {"data_attrs": "adres__straat__exact__Bospad"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 0) + + response = self.client.get( + self.url, {"data_attrs": "adres__straat__exact__Dorpsstraat"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1) + def test_filter_date_field_gte(self): record = ObjectRecordFactory.create( data={"dateField": "2000-10-10"}, object__object_type=self.object_type diff --git a/src/objects/tests/v2/test_object_api.py b/src/objects/tests/v2/test_object_api.py index 12b5a43a..cec92bb9 100644 --- a/src/objects/tests/v2/test_object_api.py +++ b/src/objects/tests/v2/test_object_api.py @@ -380,3 +380,75 @@ def test_updating_object_after_changing_the_startAt_value_returns_200(self, m): response_updating_data_after_startAt_modification.status_code, status.HTTP_200_OK, ) + + +@requests_mock.Mocker() +class ObjectApiTodayTests(TokenAuthMixin, APITestCase): + maxDiff = None + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) + PermissionFactory.create( + object_type=cls.object_type, + mode=PermissionModes.read_and_write, + token_auth=cls.token_auth, + ) + + def test_update_object_issue_324(self, m): + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get( + f"{self.object_type.url}/versions/1", + json=mock_objecttype_version(self.object_type.url), + ) + m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) + today = date.today() + + data = { + "type": self.object_type.url, + "record": { + "typeVersion": 1, + "data": {"adres": {"straat": "Bospad"}, "diameter": 30}, + "startAt": today, + }, + } + url = reverse("object-list") + response = self.client.post(url, data, **GEO_WRITE_KWARGS) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + object = Object.objects.get() + + url = reverse("object-detail", args=[object.uuid]) + data = { + "type": object.object_type.url, + "record": { + "typeVersion": 1, + "data": {"adres": {"straat": "Dorpsstraat"}, "diameter": 30}, + "startAt": today, + }, + } + + response = self.client.put(url, data, **GEO_WRITE_KWARGS) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + object.refresh_from_db() + self.assertEqual(object.object_type, self.object_type) + self.assertEqual(object.records.count(), 2) + url = reverse("object-list") + response = self.client.get(url, {"data_attrs": "adres__straat__exact__Bospad"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 0) + + response = self.client.get( + url, {"data_attrs": "adres__straat__exact__Dorpsstraat"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1) From 767a91f6ffb9598746ee4a5666d14d36e2116a36 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Wed, 29 Mar 2023 00:09:08 +0200 Subject: [PATCH 2/3] :green_heart: Pin CI-build on ubuntu-20.04 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e81a1aa3..59ec0018 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ env: jobs: tests: name: Run the Django test suite - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 services: postgres: @@ -67,7 +67,7 @@ jobs: needs: tests name: Build (and push) Docker image - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 From 889ce02edaa88e070e2ac06a4f019e69953622a5 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Tue, 28 Mar 2023 23:13:34 +0200 Subject: [PATCH 3/3] :rocket: Ensuring docker-compose-quickstart runs --- .github/workflows/quick-start.yml | 8 ++++++-- docker-compose-quickstart.yml | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/quick-start.yml b/.github/workflows/quick-start.yml index 51ee01c7..c4b486d7 100644 --- a/.github/workflows/quick-start.yml +++ b/.github/workflows/quick-start.yml @@ -4,10 +4,13 @@ on: [push] jobs: run: - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: + - uses: actions/checkout@v2 - name: Download docker-compose file run: wget https://raw.githubusercontent.com/maykinmedia/objects-api/master/docker-compose-quickstart.yml -O docker-compose-qs.yml + - name: Overwrite the docker-compose file to get the "current" one + run: cp docker-compose-quickstart.yml docker-compose-qs.yml - name: Start docker containers run: docker-compose -f docker-compose-qs.yml up -d - name: Wait until DB container starts @@ -20,6 +23,7 @@ jobs: run: | curl_status=$(curl -w '%{http_code}' -o /dev/null -s http://localhost:8000/) if [[ $curl_status != 200 ]]; then - printf "index page responds with %curl_status status" >&2 + printf "Index page responds with ${curl_status} status.\r\n\r\n" >&2 + curl -i http://localhost:8000 exit 1 fi diff --git a/docker-compose-quickstart.yml b/docker-compose-quickstart.yml index 16ba19c1..059ef151 100644 --- a/docker-compose-quickstart.yml +++ b/docker-compose-quickstart.yml @@ -12,6 +12,7 @@ services: environment: - DJANGO_SETTINGS_MODULE=objects.conf.docker - SECRET_KEY=${SECRET_KEY:-1(@f(-6s_u(5fd&1sg^uvu2s(c-9sapw)1era8q&)g)h@cwxxg} + - ALLOWED_HOSTS=* ports: - 8000:8000 depends_on: