Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject Duplicate Submissions #5047

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6a177da
862 Avoid duplicate submissions to be saved
rajpatel24 Aug 5, 2024
c9aee65
Add root_uuid field to instance model with unique constraint
rajpatel24 Aug 15, 2024
98a7236
Merge branch 'beta' of github.com:kobotoolbox/kpi into 862-reject_dup…
rajpatel24 Aug 15, 2024
71de437
Merge branch 'beta' of github.com:kobotoolbox/kpi into 862-reject_dup…
rajpatel24 Aug 19, 2024
c9ed0f1
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Aug 22, 2024
7630294
Merge branch 'beta-refactored' of github.com:kobotoolbox/kpi into 862…
rajpatel24 Aug 28, 2024
f3c89f6
Reject duplicate submissions and improve UUID extraction
rajpatel24 Aug 28, 2024
74dab52
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Sep 3, 2024
37ed595
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Sep 3, 2024
061e431
Enhance test cases and submission flow based on PR feedback
rajpatel24 Sep 4, 2024
1001b2a
Resolve merge conflicts
rajpatel24 Sep 6, 2024
f7b0e1b
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Sep 12, 2024
a01bf8f
Improve submission flow based on the PR feedback
rajpatel24 Sep 12, 2024
c2fc93e
Merge branch 'beta-refactored' of github.com:kobotoolbox/kpi into 862…
rajpatel24 Sep 13, 2024
53c422b
Fix migration conflict: rename and update migration for root_uuid field
rajpatel24 Sep 13, 2024
aa01a6c
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Sep 17, 2024
498ae91
Merge branch '862-reject_duplicate_submissions' of github.com:kobotoo…
noliveleger Sep 17, 2024
40b0d03
Fix code linter errors
rajpatel24 Sep 17, 2024
24a51bf
Address PR feedback and improve submission flow
rajpatel24 Sep 18, 2024
33e2c46
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
noliveleger Sep 18, 2024
81d0b9d
Add test for duplicate submission with identical attachment name but …
rajpatel24 Sep 18, 2024
728d0d1
Merge branch '862-reject_duplicate_submissions' of github.com:kobotoo…
rajpatel24 Sep 18, 2024
cc37b6c
Improve code linting compliance
rajpatel24 Sep 19, 2024
cdc71db
Update logic to handle edit submissions with identical attachment nam…
rajpatel24 Sep 23, 2024
306888b
Refactor the command to clean duplicate submissions and add a unique …
rajpatel24 Sep 27, 2024
b3b9b35
Merge branch 'beta-refactored' into 862-reject_duplicate_submissions
rajpatel24 Oct 2, 2024
219cc37
Merge branch 'main' into 862-reject_duplicate_submissions
noliveleger Oct 7, 2024
c416e42
Fix bad merge
noliveleger Oct 7, 2024
b227f5b
Merge branch 'main' into 862-reject_duplicate_submissions
noliveleger Oct 10, 2024
90b3f85
Merge branch 'main' into 862-reject_duplicate_submissions
noliveleger Oct 15, 2024
b18f4a5
Update comments in tests to make them more obvious
noliveleger Oct 15, 2024
362bfa8
Fix failing test case for duplicate submissions with altered attachments
rajpatel24 Oct 17, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jobs:
CACHE_URL: redis://localhost:6379/3
ENKETO_REDIS_MAIN_URL: redis://localhost:6379/0
KOBOCAT_MEDIA_ROOT: /tmp/test_media
SKIP_TESTS_WITH_CONCURRENCY: 'True'
strategy:
matrix:
python-version: ['3.8', '3.10']
Expand Down
18 changes: 18 additions & 0 deletions kobo/apps/openrosa/apps/api/tests/fixtures/users.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"model": "auth.user",
"pk": 2,
"fields": {
"username": "bob",
"password": "pbkdf2_sha256$260000$T1eA0O4Ub6c6FAaCsb0fqU$6vX4qMw1VV9tMXFf1d9pL/5z5/2T1MQYYn7vB3p+I2Y=",
"email": "[email protected]",
"first_name": "bob",
"last_name": "bob",
"is_active": true,
"is_staff": false,
"is_superuser": false,
"last_login": null,
"date_joined": "2015-02-12T19:52:14.406Z"
}
}
]
Original file line number Diff line number Diff line change
@@ -1,22 +1,38 @@
# coding: utf-8
import multiprocessing
import os
import uuid
from collections import defaultdict
from functools import partial

import pytest
import requests
import simplejson as json
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from django.core.files.base import ContentFile
from django.core.files.uploadedfile import InMemoryUploadedFile
from django.test.testcases import LiveServerTestCase
from django_digest.test import DigestAuth
from rest_framework.authtoken.models import Token

from kobo.apps.kobo_auth.shortcuts import User
from kobo.apps.openrosa.apps.main.models import UserProfile
from kobo.apps.openrosa.libs.tests.mixins.request_mixin import RequestMixin
from kobo.apps.openrosa.libs.utils.guardian import assign_perm
from rest_framework import status

from kobo.apps.openrosa.apps.api.tests.viewsets.test_abstract_viewset import \
TestAbstractViewSet
from kobo.apps.openrosa.apps.api.viewsets.xform_submission_api import XFormSubmissionApi
from kobo.apps.openrosa.apps.api.tests.viewsets.test_abstract_viewset import (
TestAbstractViewSet,
)
from kobo.apps.openrosa.apps.api.viewsets.xform_submission_api import (
XFormSubmissionApi,
)
from kobo.apps.openrosa.apps.logger.models import Attachment
from kobo.apps.openrosa.apps.main import tests as main_tests
from kobo.apps.openrosa.libs.constants import (
CAN_ADD_SUBMISSIONS
)
from kobo.apps.openrosa.libs.utils import logger_tools
from kobo.apps.openrosa.libs.utils.logger_tools import OpenRosaTemporarilyUnavailable


Expand Down Expand Up @@ -216,7 +232,7 @@ def test_post_submission_authenticated_bad_json(self):
self.assertTrue('error' in rendered_response.data)
self.assertTrue(
rendered_response.data['error'].startswith(
'Received empty submission'
'Instance ID is required'
)
)
self.assertTrue(rendered_response.status_code == 400)
Expand Down Expand Up @@ -438,3 +454,107 @@ def test_submission_blocking_flag(self):
)
response = self.view(request, username=username)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)


class ConcurrentSubmissionTestCase(RequestMixin, LiveServerTestCase):
rajpatel24 marked this conversation as resolved.
Show resolved Hide resolved
"""
Inherit from LiveServerTestCase to be able to test concurrent requests
to submission endpoint in different transactions (and different processes).
Otherwise, DB is populated only on the first request but still empty on
subsequent ones.
"""
fixtures = ['kobo/apps/openrosa/apps/api/tests/fixtures/users']

def setUp(self):
self.user = User.objects.get(username='bob')
self.token, _ = Token.objects.get_or_create(user=self.user)
UserProfile.objects.get_or_create(user=self.user)

def publish_xls_form(self):
path = os.path.join(
settings.OPENROSA_APP_DIR,
'apps',
'main',
'tests',
'fixtures',
'transportation',
'transportation.xls',
)

with open(path, 'rb') as f:
xls_file = ContentFile(f.read(), name='transportation.xls')

self.xform = logger_tools.publish_xls_form(xls_file, self.user)

@pytest.mark.skipif(
settings.SKIP_TESTS_WITH_CONCURRENCY,
reason='GitLab does not seem to support multi-processes'
)
def test_post_concurrent_same_submissions(self):
DUPLICATE_SUBMISSIONS_COUNT = 2 # noqa

self.publish_xls_form()
username = 'bob'
survey = 'transport_2011-07-25_19-05-49'
results = defaultdict(int)

with multiprocessing.Pool() as pool:
for result in pool.map(
partial(
submit_data,
live_server_url=self.live_server_url,
survey_=survey,
username_=username,
token_=self.token.key
),
range(DUPLICATE_SUBMISSIONS_COUNT),
):
results[result] += 1

assert results[status.HTTP_201_CREATED] == 1
assert results[status.HTTP_202_ACCEPTED] == DUPLICATE_SUBMISSIONS_COUNT - 1


def submit_data(identifier, survey_, username_, live_server_url, token_):
"""
Submit data to live server.

It has to be outside `ConcurrentSubmissionTestCase` class to be pickled by
`multiprocessing.Pool().map()`.
"""
media_file = '1335783522563.jpg'
main_directory = os.path.dirname(main_tests.__file__)
path = os.path.join(
main_directory,
'fixtures',
'transportation',
'instances',
survey_,
media_file,
)
with open(path, 'rb') as f:
f = InMemoryUploadedFile(
f,
'media_file',
media_file,
'image/jpg',
os.path.getsize(path),
None,
)
submission_path = os.path.join(
main_directory,
'fixtures',
'transportation',
'instances',
survey_,
f'{survey_}.xml',
)
with open(submission_path) as sf:
files = {'xml_submission_file': sf, 'media_file': f}
headers = {'Authorization': f'Token {token_}'}
response = requests.post(
f'{live_server_url}/{username_}/submission',
files=files,
headers=headers
)
return response.status_code
38 changes: 38 additions & 0 deletions kobo/apps/openrosa/apps/logger/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ class BuildDbQueriesNoConfirmationProvidedError(Exception):
pass


class ConflictingSubmissionUUIDError(Exception):
def __init__(self, message='Submission with this instance ID already exists'):
super().__init__(message)


class DuplicateInstanceError(Exception):
def __init__(self, message='Duplicate Instance'):
super().__init__(message)


class DuplicateUUIDError(Exception):
pass

Expand All @@ -18,9 +28,37 @@ class FormInactiveError(Exception):
pass


class InstanceEmptyError(Exception):
def __init__(self, message='Empty instance'):
super().__init__(message)


class InstanceInvalidUserError(Exception):
def __init__(self, message='Could not determine the user'):
super().__init__(message)


class InstanceIdMissingError(Exception):
def __init__(self, message='Could not determine the instance ID'):
super().__init__(message)


class InstanceMultipleNodeError(Exception):
pass


class InstanceParseError(Exception):
def __init__(self, message='The instance could not be parsed'):
super().__init__(message)


class MissingValidationStatusPayloadError(Exception):
pass


class TemporarilyUnavailableError(Exception):
pass


class XLSFormError(Exception):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@
</kids>
<gps>-1.2627557 36.7926442 0.0 30.0</gps>
<web_browsers>chrome ie</web_browsers>
<meta>
<instanceID>uuid:364f173c688e482486a48661700466gg</instanceID>
</meta>
</new_repeats>
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@
</kids>
<gps>-1.2627557 36.7926442 0.0 30.0</gps>
<web_browsers>chrome ie</web_browsers>
<meta>
<instanceID>uuid:364f173c688e482486a48661700466gg</instanceID>
</meta>
</new_repeats>
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<?xml version='1.0' ?><survey_names id="survey_names"><start>2012-08-17T11:24:53.254+03</start><name>HD</name><end>2012-08-17T11:24:57.897+03</end></survey_names>
<?xml version='1.0' ?><survey_names id="survey_names"><start>2012-08-17T11:24:53.254+03</start><name>HD</name><end>2012-08-17T11:24:57.897+03</end><meta><instanceID>uuid:729f173c688e482486a48661700455ff</instanceID></meta></survey_names>
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,7 @@
<has_children>0</has_children>
<gps>-1.2836198 36.8795437 0.0 1044.0</gps>
<web_browsers>firefox chrome safari</web_browsers>
<meta>
<instanceID>uuid:364f173c688e482486a48661700466gg</instanceID>
</meta>
</tutorial>
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@
<has_children>0</has_children>
<gps>-1.2836198 36.8795437 0.0 1044.0</gps>
<web_browsers>firefox chrome safari</web_browsers>
<meta>
<instanceID>uuid:639f173c688e482486a48661700456ty</instanceID>
</meta>
</tutorial>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version='1.0' ?>
<tutorial id="tutorial">
<name>Larry
Again
</name><!-- newline to make sure we can handle it -->
<age>23</age>
<picture>1335783522563.jpg</picture>
<has_children>0</has_children>
<gps>-1.2836198 36.8795437 0.0 1044.0</gps>
<web_browsers>firefox chrome safari</web_browsers>
<meta>
<instanceID>uuid:729f173c688e482486a48661700455ff</instanceID>
</meta>
</tutorial>
Loading
Loading