-
-
Notifications
You must be signed in to change notification settings - Fork 226
Enterprise Form Submissions Iterators #35295
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
b9d3b6d
Apply sorting to enterprise domain list
mjriley 463d97b
Add resumable iterator wrapper
mjriley 3eccde7
Add KeysetPaginator
mjriley 5154360
Added enterprise form iterators
mjriley 4112456
Rewire FormSubmissionResource to use iterators
mjriley 399b013
Moved generic API classes into the API application
mjriley 185a143
Removed ResumableIteratorWrapper
mjriley 05eaa9a
Switched received filter to inserted
mjriley 2504668
Rename domain forms generator
mjriley dd334de
Make enterprise form api timezone aware
mjriley 080d837
Rename mobile_user field to username
mjriley 09c104b
Made enterprise form submission report iteration generic
mjriley 409f725
Added happy path test for form resource api
mjriley bff5fac
Remove superuser permissions from Enterprise Forms API test
mjriley c65f0b6
rename api test
mjriley 593882c
isort
mjriley bee7055
Refactor domain iteration logic
mjriley 7082597
rename domain looping functions
mjriley b85a5f5
Added authentication tests
mjriley 2d9d74b
isort
mjriley dedb429
Additional clarifying comments/structures
mjriley d489a36
Allow the iterable query to use a generic converter
mjriley c5d96fb
Changed "test-domain" to "testing-domain" to try to isolate a testing…
mjriley e65970d
Fixed typo
mjriley 7d0e897
Added variable highlighting for iterator documentation
mjriley 18faad3
Removed unused class
mjriley e55664e
Add abstract base classes
mjriley 4cce055
Merge branch 'master' into mjr/enterprise_iterators_draft
mjriley eb67336
Added page_size support and turned limit into a real limit
mjriley File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| from django.http.request import QueryDict | ||
| from urllib.parse import urlencode | ||
| from tastypie.paginator import Paginator | ||
|
|
||
|
|
||
| class KeysetPaginator(Paginator): | ||
| ''' | ||
| An alternate paginator meant to support paginating by keyset rather than by index/offset. | ||
| `objects` is expected to represent a query object that exposes an `.execute(limit)` | ||
| method that returns an iterable. | ||
| The above returned iterable must expose a `.get_next_query_params()` method that will return | ||
| parameters to allow the user to fetch the next page of data. | ||
| Because keyset pagination does not efficiently handle slicing or offset operations, | ||
| these methods have been intentionally disabled | ||
| ''' | ||
| def __init__(self, request_data, objects, | ||
| resource_uri=None, limit=None, max_limit=1000, collection_name='objects'): | ||
| super().__init__( | ||
| request_data, | ||
| objects, | ||
| resource_uri=resource_uri, | ||
| limit=limit, | ||
| max_limit=max_limit, | ||
| collection_name=collection_name | ||
| ) | ||
|
|
||
| def get_offset(self): | ||
| raise NotImplementedError() | ||
|
|
||
| def get_slice(self, limit, offset): | ||
| raise NotImplementedError() | ||
|
|
||
| def get_count(self): | ||
| raise NotImplementedError() | ||
|
|
||
| def get_previous(self, limit, offset): | ||
| raise NotImplementedError() | ||
|
|
||
| def get_next(self, limit, **next_params): | ||
| return self._generate_uri(limit, **next_params) | ||
|
|
||
| def _generate_uri(self, limit, **next_params): | ||
| if self.resource_uri is None: | ||
| return None | ||
|
|
||
| if isinstance(self.request_data, QueryDict): | ||
| # Because QueryDict allows multiple values for the same key, we need to remove existing values | ||
| # prior to updating | ||
| request_params = self.request_data.copy() | ||
| if 'limit' in request_params: | ||
| del request_params['limit'] | ||
| for key in next_params: | ||
| if key in request_params: | ||
| del request_params[key] | ||
|
|
||
| request_params.update({'limit': str(limit), **next_params}) | ||
| encoded_params = request_params.urlencode() | ||
| else: | ||
| request_params = {} | ||
| for k, v in self.request_data.items(): | ||
| if isinstance(v, str): | ||
| request_params[k] = v.encode('utf-8') | ||
| else: | ||
| request_params[k] = v | ||
|
|
||
| request_params.update({'limit': limit, **next_params}) | ||
| encoded_params = urlencode(request_params) | ||
|
|
||
| return '%s?%s' % ( | ||
| self.resource_uri, | ||
| encoded_params | ||
| ) | ||
|
|
||
| def page(self): | ||
| """ | ||
| Generates all pertinent data about the requested page. | ||
| """ | ||
| limit = self.get_limit() | ||
| it = self.objects.execute(limit=limit) | ||
| objects = list(it) | ||
|
|
||
| meta = { | ||
| 'limit': limit, | ||
| } | ||
|
|
||
| if limit: | ||
| next_params = it.get_next_query_params() | ||
| if next_params: | ||
| meta['next'] = self.get_next(limit, **next_params) | ||
|
|
||
| return { | ||
| self.collection_name: objects, | ||
| 'meta': meta, | ||
| } | ||
|
|
||
|
|
||
| class PageableQueryInterface: | ||
|
||
| def execute(limit=None): | ||
| ''' | ||
| Should return an iterable that exposes a `.get_next_query_params()` method | ||
| ''' | ||
| raise NotImplementedError() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| from itertools import islice | ||
|
|
||
|
|
||
| class ResumableIteratorWrapper: | ||
| def __init__(self, sequence_factory_fn, get_element_properties_fn=None, limit=None): | ||
| self.limit = limit | ||
|
|
||
| # if a limit exists, increase it by 1 to allow us to check whether additional items remain at the end | ||
| padded_limit = limit + 1 if limit else None | ||
| self.original_it = iter(sequence_factory_fn(padded_limit)) | ||
| self.it = islice(self.original_it, self.limit) | ||
| self.prev_element = None | ||
| self.iteration_started = False | ||
| self.is_complete = False | ||
|
|
||
| self.get_element_properties_fn = get_element_properties_fn | ||
| if not self.get_element_properties_fn: | ||
| self.get_element_properties_fn = lambda ele: {'value': ele} | ||
|
|
||
| def __iter__(self): | ||
| return self | ||
|
|
||
| def __next__(self): | ||
| self.iteration_started = True | ||
|
|
||
| try: | ||
| self.prev_element = next(self.it) | ||
| except StopIteration: | ||
| if self.limit and not self.is_complete: | ||
| # the end of the limited sequence was reached, check if items beyond the limit remain | ||
| try: | ||
| next(self.original_it) | ||
| except StopIteration: | ||
| # the iteration is fully complete -- no additional items can be fetched | ||
| self.is_complete = True | ||
| else: | ||
| self.is_complete = True | ||
| raise | ||
|
|
||
| return self.prev_element | ||
|
|
||
| def get_next_query_params(self): | ||
| if self.is_complete: | ||
| return None | ||
| if not self.iteration_started: | ||
| return {} | ||
|
|
||
| return self.get_element_properties_fn(self.prev_element) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| from django.test import SimpleTestCase | ||
| from django.http import QueryDict | ||
| from corehq.apps.api.resumable_iterator_wrapper import ResumableIteratorWrapper | ||
| from corehq.apps.api.keyset_paginator import KeysetPaginator | ||
|
|
||
|
|
||
| class SequenceWrapper: | ||
| def __init__(self, seq, get_next_fn=None): | ||
| self.seq = seq | ||
| self.get_next_fn = get_next_fn | ||
|
|
||
| def execute(self, limit=None): | ||
| return ResumableIteratorWrapper(lambda _: self.seq, self.get_next_fn, limit=limit) | ||
|
|
||
|
|
||
| class KeysetPaginatorTests(SimpleTestCase): | ||
| def test_page_fetches_all_results_below_limit(self): | ||
| objects = SequenceWrapper(range(5)) | ||
| paginator = KeysetPaginator(QueryDict(), objects, limit=10) | ||
| page = paginator.page() | ||
| self.assertEqual(page['objects'], [0, 1, 2, 3, 4]) | ||
| self.assertEqual(page['meta'], {'limit': 10}) | ||
|
|
||
| def test_page_includes_next_information_when_more_results_are_available(self): | ||
| objects = SequenceWrapper(range(5), lambda ele: {'next': ele}) | ||
| paginator = KeysetPaginator(QueryDict(), objects, resource_uri='http://test.com/', limit=3) | ||
| page = paginator.page() | ||
| self.assertEqual(page['objects'], [0, 1, 2]) | ||
| self.assertEqual(page['meta'], {'limit': 3, 'next': 'http://test.com/?limit=3&next=2'}) | ||
|
|
||
| def test_does_not_include_duplicate_limits(self): | ||
| request_data = QueryDict(mutable=True) | ||
| request_data['limit'] = 3 | ||
| objects = SequenceWrapper(range(5), lambda ele: {'next': ele}) | ||
| paginator = KeysetPaginator(request_data, objects, resource_uri='http://test.com/') | ||
| page = paginator.page() | ||
| self.assertEqual(page['meta']['next'], 'http://test.com/?limit=3&next=2') | ||
|
|
||
| def test_supports_dict_request_data(self): | ||
| request_data = { | ||
| 'limit': 3, | ||
| 'some_param': 'yes' | ||
| } | ||
| objects = SequenceWrapper(range(5), lambda ele: {'next': ele}) | ||
| paginator = KeysetPaginator(request_data, objects, resource_uri='http://test.com/') | ||
| page = paginator.page() | ||
| self.assertEqual(page['meta']['next'], 'http://test.com/?limit=3&some_param=yes&next=2') | ||
|
|
||
| def test_get_offset_not_implemented(self): | ||
| objects = SequenceWrapper(range(5)) | ||
| paginator = KeysetPaginator(QueryDict(), objects) | ||
|
|
||
| with self.assertRaises(NotImplementedError): | ||
| paginator.get_offset() | ||
|
|
||
| def test_get_slice_not_implemented(self): | ||
| objects = SequenceWrapper(range(5)) | ||
| paginator = KeysetPaginator(QueryDict(), objects) | ||
|
|
||
| with self.assertRaises(NotImplementedError): | ||
| paginator.get_slice(limit=10, offset=20) | ||
|
|
||
| def test_get_count_not_implemented(self): | ||
| objects = SequenceWrapper(range(5)) | ||
| paginator = KeysetPaginator(QueryDict(), objects) | ||
|
|
||
| with self.assertRaises(NotImplementedError): | ||
| paginator.get_count() | ||
|
|
||
| def test_get_previous_not_implemented(self): | ||
| objects = SequenceWrapper(range(5)) | ||
| paginator = KeysetPaginator(QueryDict(), objects) | ||
|
|
||
| with self.assertRaises(NotImplementedError): | ||
| paginator.get_previous(limit=10, offset=20) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| from django.test import SimpleTestCase | ||
| from corehq.apps.api.resumable_iterator_wrapper import ResumableIteratorWrapper | ||
|
|
||
|
|
||
| class ResumableIteratorWrapperTests(SimpleTestCase): | ||
| def test_can_iterate_through_a_wrapped_iterator(self): | ||
| initial_it = iter(range(5)) | ||
| it = ResumableIteratorWrapper(lambda _: initial_it) | ||
| self.assertEqual(list(it), [0, 1, 2, 3, 4]) | ||
|
|
||
| def test_can_iterate_through_a_sequence(self): | ||
| sequence = [0, 1, 2, 3, 4] | ||
| it = ResumableIteratorWrapper(lambda _: sequence) | ||
| self.assertEqual(list(it), [0, 1, 2, 3, 4]) | ||
|
|
||
| def test_can_limit_a_sequence(self): | ||
| sequence = [0, 1, 2, 3, 4] | ||
| it = ResumableIteratorWrapper(lambda _: sequence, limit=4) | ||
| self.assertEqual(list(it), [0, 1, 2, 3]) | ||
|
|
||
| def test_when_limit_is_less_than_sequence_length_is_incomplete(self): | ||
| sequence = [0, 1, 2, 3, 4] | ||
| it = ResumableIteratorWrapper(lambda _: sequence, limit=4) | ||
| list(it) | ||
| self.assertFalse(it.is_complete) | ||
|
|
||
| def test_when_limit_matches_sequence_size_iterator_is_complete(self): | ||
| sequence = [0, 1, 2, 3, 4] | ||
| it = ResumableIteratorWrapper(lambda _: sequence, limit=5) | ||
| list(it) | ||
| self.assertTrue(it.is_complete) | ||
|
|
||
| def test_get_next_query_params_returns_empty_object_prior_to_iteration(self): | ||
| seq = [ | ||
| {'key': 'one', 'val': 'val1'}, | ||
| {'key': 'two', 'val': 'val2'}, | ||
| ] | ||
| it = ResumableIteratorWrapper(lambda _: seq) | ||
| self.assertEqual(it.get_next_query_params(), {}) | ||
|
|
||
| def test_default_get_next_query_params_returns_identity_object(self): | ||
| seq = [ | ||
| {'key': 'one', 'val': 'val1'}, | ||
| {'key': 'two', 'val': 'val2'}, | ||
| ] | ||
| it = ResumableIteratorWrapper(lambda _: seq, ) | ||
| next(it) | ||
| self.assertEqual(it.get_next_query_params(), {'value': {'key': 'one', 'val': 'val1'}}) | ||
|
|
||
| def test_custom_get_next_query_params_fn(self): | ||
| seq = [ | ||
| {'key': 'one', 'val': 'val1'}, | ||
| {'key': 'two', 'val': 'val2'}, | ||
| ] | ||
|
|
||
| def custom_element_properties_fn(ele): | ||
| return (ele['key'], ele['val']) | ||
|
|
||
| it = ResumableIteratorWrapper(lambda _: seq, custom_element_properties_fn) | ||
| next(it) | ||
| self.assertEqual(it.get_next_query_params(), ('one', 'val1')) | ||
|
|
||
| def test_get_next_query_params_returns_none_when_fully_iterated(self): | ||
| it = ResumableIteratorWrapper(lambda _: range(5)) | ||
| list(it) | ||
| self.assertIsNone(it.get_next_query_params()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially rename limit to indicate page size as it is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in eb67336