From b5e03e22252b2987d5e275502d4fb904132bf6bc Mon Sep 17 00:00:00 2001 From: Adam Coldrick Date: Wed, 1 Jul 2015 11:09:00 +0000 Subject: [PATCH] Support offset/limit pagination in the API Offset/limit pagination is easier to implement than marker/limit pagination complete with the improvements discussed in the spec[0] for pagination in StoryBoard. It is also an adequate solution for StoryBoard for now, at least until the aforementioned spec is finalised. Change-Id: I576ba7f2d92fb7be5914a90083116e73ffbcb36e --- storyboard/api/v1/project_groups.py | 12 +++++++--- storyboard/api/v1/projects.py | 17 +++++++++----- storyboard/api/v1/stories.py | 16 ++++++++----- storyboard/api/v1/timeline.py | 14 ++++++++---- storyboard/api/v1/users.py | 17 +++++++++----- storyboard/db/api/base.py | 35 ++++++++++++++++++++--------- storyboard/db/api/projects.py | 5 +++-- storyboard/db/api/stories.py | 6 ++--- storyboard/db/api/users.py | 6 +++-- 9 files changed, 88 insertions(+), 40 deletions(-) diff --git a/storyboard/api/v1/project_groups.py b/storyboard/api/v1/project_groups.py index 541ffca5..24d7aee7 100644 --- a/storyboard/api/v1/project_groups.py +++ b/storyboard/api/v1/project_groups.py @@ -115,13 +115,14 @@ class ProjectGroupsController(rest.RestController): @decorators.db_exceptions @secure(checks.guest) - @wsme_pecan.wsexpose([wmodels.ProjectGroup], int, int, wtypes.text, + @wsme_pecan.wsexpose([wmodels.ProjectGroup], int, int, int, wtypes.text, wtypes.text, wtypes.text, wtypes.text) - def get(self, marker=None, limit=None, name=None, title=None, + def get(self, marker=None, offset=None, limit=None, name=None, title=None, sort_field='id', sort_dir='asc'): """Retrieve a list of projects groups. :param marker: The resource id where the page should begin. + :param offset: The offset to start the page at. :param limit: The number of project groups to retrieve. :param name: A string to filter the name by. :param title: A string to filter the title by. @@ -133,9 +134,12 @@ class ProjectGroupsController(rest.RestController): limit = max(0, limit) # Resolve the marker record. - marker_group = project_groups.project_group_get(marker) + marker_group = None + if marker is not None: + marker_group = project_groups.project_group_get(marker) groups = project_groups.project_group_get_all(marker=marker_group, + offset=offset, limit=limit, name=name, title=title, @@ -151,6 +155,8 @@ class ProjectGroupsController(rest.RestController): response.headers['X-Total'] = str(group_count) if marker_group: response.headers['X-Marker'] = str(marker_group.id) + if offset is not None: + response.headers['X-Offset'] = str(offset) return [wmodels.ProjectGroup.from_db_model(group) for group in groups] diff --git a/storyboard/api/v1/projects.py b/storyboard/api/v1/projects.py index 1bae292c..7fd563f6 100644 --- a/storyboard/api/v1/projects.py +++ b/storyboard/api/v1/projects.py @@ -81,13 +81,15 @@ class ProjectsController(rest.RestController): @decorators.db_exceptions @secure(checks.guest) - @wsme_pecan.wsexpose([wmodels.Project], int, int, wtypes.text, wtypes.text, - int, wtypes.text, wtypes.text) - def get(self, marker=None, limit=None, name=None, description=None, - project_group_id=None, sort_field='id', sort_dir='asc'): + @wsme_pecan.wsexpose([wmodels.Project], int, int, int, wtypes.text, + wtypes.text, int, wtypes.text, wtypes.text) + def get(self, marker=None, offset=None, limit=None, name=None, + description=None, project_group_id=None, sort_field='id', + sort_dir='asc'): """Retrieve a list of projects. :param marker: The resource id where the page should begin. + :param offset: The offset to start the page at. :param limit: The number of projects to retrieve. :param name: A string to filter the name by. :param description: A string to filter the description by. @@ -102,10 +104,13 @@ class ProjectsController(rest.RestController): limit = max(0, limit) # Resolve the marker record. - marker_project = projects_api.project_get(marker) + marker_project = None + if marker is not None: + marker_project = projects_api.project_get(marker) projects = \ projects_api.project_get_all(marker=marker_project, + offset=offset, limit=limit, name=name, description=description, @@ -123,6 +128,8 @@ class ProjectsController(rest.RestController): response.headers['X-Total'] = str(project_count) if marker_project: response.headers['X-Marker'] = str(marker_project.id) + if offset is not None: + response.headers['X-Offset'] = str(offset) return [wmodels.Project.from_db_model(p) for p in projects] diff --git a/storyboard/api/v1/stories.py b/storyboard/api/v1/stories.py index 667a9461..2188e456 100644 --- a/storyboard/api/v1/stories.py +++ b/storyboard/api/v1/stories.py @@ -69,12 +69,12 @@ class StoriesController(rest.RestController): @decorators.db_exceptions @secure(checks.guest) @wsme_pecan.wsexpose([wmodels.Story], wtypes.text, wtypes.text, - [wtypes.text], int, int, int, [wtypes.text], int, int, - wtypes.text, wtypes.text, wtypes.text) + [wtypes.text], int, int, int, [wtypes.text], int, + int, int, wtypes.text, wtypes.text, wtypes.text) def get_all(self, title=None, description=None, status=None, assignee_id=None, project_group_id=None, project_id=None, - tags=None, marker=None, limit=None, tags_filter_type='all', - sort_field='id', sort_dir='asc'): + tags=None, marker=None, offset=None, limit=None, + tags_filter_type='all', sort_field='id', sort_dir='asc'): """Retrieve definitions of all of the stories. :param title: A string to filter the title by. @@ -85,6 +85,7 @@ class StoriesController(rest.RestController): :param project_id: Filter stories by project ID. :param tags: A list of tags to filter by. :param marker: The resource id where the page should begin. + :param offset: The offset to start the page at. :param limit: The number of stories to retrieve. :param tags_filter_type: Type of tags filter. :param sort_field: The name of the field to sort on. @@ -96,7 +97,9 @@ class StoriesController(rest.RestController): limit = max(0, limit) # Resolve the marker record. - marker_story = stories_api.story_get(marker) + marker_story = None + if marker: + marker_story = stories_api.story_get(marker) stories = stories_api \ .story_get_all(title=title, @@ -107,6 +110,7 @@ class StoriesController(rest.RestController): project_id=project_id, tags=tags, marker=marker_story, + offset=offset, tags_filter_type=tags_filter_type, limit=limit, sort_field=sort_field, sort_dir=sort_dir) @@ -126,6 +130,8 @@ class StoriesController(rest.RestController): response.headers['X-Total'] = str(story_count) if marker_story: response.headers['X-Marker'] = str(marker_story.id) + if offset is not None: + response.headers['X-Offset'] = str(offset) return [wmodels.Story.from_db_model(s) for s in stories] diff --git a/storyboard/api/v1/timeline.py b/storyboard/api/v1/timeline.py index 1b6eeab6..48bcecb8 100644 --- a/storyboard/api/v1/timeline.py +++ b/storyboard/api/v1/timeline.py @@ -64,15 +64,16 @@ class TimeLineEventsController(rest.RestController): @decorators.db_exceptions @secure(checks.guest) - @wsme_pecan.wsexpose([wmodels.TimeLineEvent], int, [wtypes.text], int, int, - wtypes.text, wtypes.text) + @wsme_pecan.wsexpose([wmodels.TimeLineEvent], int, [wtypes.text], int, + int, int, wtypes.text, wtypes.text) def get_all(self, story_id=None, event_type=None, marker=None, - limit=None, sort_field=None, sort_dir=None): + offset=None, limit=None, sort_field=None, sort_dir=None): """Retrieve all events that have happened under specified story. :param story_id: Filter events by story ID. :param event_type: A selection of event types to get. :param marker: The resource id where the page should begin. + :param offset: The offset to start the page at. :param limit: The number of events to retrieve. :param sort_field: The name of the field to sort on. :param sort_dir: Sort direction for results (asc, desc). @@ -92,13 +93,16 @@ class TimeLineEventsController(rest.RestController): abort(400, msg) # Resolve the marker record. - marker_event = events_api.event_get(marker) + marker_event = None + if marker is not None: + marker_event = events_api.event_get(marker) event_count = events_api.events_get_count(story_id=story_id, event_type=event_type) events = events_api.events_get_all(story_id=story_id, event_type=event_type, marker=marker_event, + offset=offset, limit=limit, sort_field=sort_field, sort_dir=sort_dir) @@ -109,6 +113,8 @@ class TimeLineEventsController(rest.RestController): response.headers['X-Total'] = str(event_count) if marker_event: response.headers['X-Marker'] = str(marker_event.id) + if offset is not None: + response.headers['X-Offset'] = str(offset) return [wmodels.TimeLineEvent.resolve_event_values( wmodels.TimeLineEvent.from_db_model(event)) for event in events] diff --git a/storyboard/api/v1/users.py b/storyboard/api/v1/users.py index f882d4fe..c9578a1e 100644 --- a/storyboard/api/v1/users.py +++ b/storyboard/api/v1/users.py @@ -57,13 +57,14 @@ class UsersController(rest.RestController): @decorators.db_exceptions @secure(checks.guest) - @wsme_pecan.wsexpose([wmodels.User], int, int, wtypes.text, wtypes.text, - wtypes.text, wtypes.text) - def get(self, marker=None, limit=None, full_name=None, + @wsme_pecan.wsexpose([wmodels.User], int, int, int, wtypes.text, + wtypes.text, wtypes.text, wtypes.text) + def get(self, marker=None, offset=None, limit=None, full_name=None, sort_field='id', sort_dir='asc'): """Page and filter the users in storyboard. :param marker: The resource id where the page should begin. + :param offset: The offset to start the page at. :param limit: The number of users to retrieve. :param username: A string of characters to filter the username with. :param full_name: A string of characters to filter the full_name with. @@ -76,9 +77,13 @@ class UsersController(rest.RestController): limit = max(0, limit) # Resolve the marker record. - marker_user = users_api.user_get(marker) + marker_user = None + if marker is not None: + marker_user = users_api.user_get(marker) - users = users_api.user_get_all(marker=marker_user, limit=limit, + users = users_api.user_get_all(marker=marker_user, + offset=offset, + limit=limit, full_name=full_name, filter_non_public=True, sort_field=sort_field, @@ -91,6 +96,8 @@ class UsersController(rest.RestController): response.headers['X-Total'] = str(user_count) if marker_user: response.headers['X-Marker'] = str(marker_user.id) + if offset is not None: + response.headers['X-Offset'] = str(offset) return [wmodels.User.from_db_model(u) for u in users] diff --git a/storyboard/db/api/base.py b/storyboard/db/api/base.py index 1a3fc84a..cc8ad9a7 100644 --- a/storyboard/db/api/base.py +++ b/storyboard/db/api/base.py @@ -95,15 +95,26 @@ def get_engine(): def paginate_query(query, model, limit, sort_key, marker=None, - sort_dir=None, sort_dirs=None): + offset=None, sort_dir=None, sort_dirs=None): + if offset is not None: + # If we are doing offset-based pagination, don't set a + # limit or a marker. + # FIXME: Eventually the webclient should be smart enough + # to do marker-based pagination, at which point this will + # be unnecessary. + start, end = (offset, offset + limit) + limit, marker = (None, None) try: - return utils_paginate_query(query=query, - model=model, - limit=limit, - sort_keys=[sort_key], - marker=marker, - sort_dir=sort_dir, - sort_dirs=sort_dirs) + sorted_query = utils_paginate_query(query=query, + model=model, + limit=limit, + sort_keys=[sort_key], + marker=marker, + sort_dir=sort_dir, + sort_dirs=sort_dirs) + if offset is not None: + return sorted_query.slice(start, end) + return sorted_query except ValueError as ve: raise exc.DBValueError(message=str(ve)) except InvalidSortKey: @@ -187,8 +198,9 @@ def entity_get(kls, entity_id, filter_non_public=False, session=None): return entity -def entity_get_all(kls, filter_non_public=False, marker=None, limit=None, - sort_field='id', sort_dir='asc', session=None, **kwargs): +def entity_get_all(kls, filter_non_public=False, marker=None, offset=None, + limit=None, sort_field='id', sort_dir='asc', session=None, + **kwargs): # Sanity checks, in case someone accidentally explicitly passes in 'None' if not sort_field: sort_field = 'id' @@ -208,6 +220,7 @@ def entity_get_all(kls, filter_non_public=False, marker=None, limit=None, limit=limit, sort_key=sort_field, marker=marker, + offset=offset, sort_dir=sort_dir) # Execute the query @@ -220,7 +233,7 @@ def entity_get_all(kls, filter_non_public=False, marker=None, limit=None, raise exc.DBInvalidUnicodeParameter() if len(entities) > 0 and filter_non_public: - sample_entity = entities[0] if len(entities) > 0 else None + sample_entity = entities[0] public_fields = getattr(sample_entity, "_public_fields", []) entities = [_filter_non_public_fields(entity, public_fields) diff --git a/storyboard/db/api/projects.py b/storyboard/db/api/projects.py index 75168f34..5125b3f8 100644 --- a/storyboard/db/api/projects.py +++ b/storyboard/db/api/projects.py @@ -28,8 +28,8 @@ def project_get_by_name(name): return query.filter_by(name=name).first() -def project_get_all(marker=None, limit=None, sort_field=None, sort_dir=None, - project_group_id=None, **kwargs): +def project_get_all(marker=None, offset=None, limit=None, sort_field=None, + sort_dir=None, project_group_id=None, **kwargs): # Sanity checks, in case someone accidentally explicitly passes in 'None' if not sort_field: sort_field = 'id' @@ -45,6 +45,7 @@ def project_get_all(marker=None, limit=None, sort_field=None, sort_dir=None, limit=limit, sort_key=sort_field, marker=marker, + offset=offset, sort_dir=sort_dir) # Execute the query diff --git a/storyboard/db/api/stories.py b/storyboard/db/api/stories.py index 6c8b5f1a..b47afed0 100644 --- a/storyboard/db/api/stories.py +++ b/storyboard/db/api/stories.py @@ -56,8 +56,8 @@ def story_get(story_id, session=None): def story_get_all(title=None, description=None, status=None, assignee_id=None, project_group_id=None, project_id=None, tags=None, - marker=None, limit=None, tags_filter_type="all", - sort_field='id', sort_dir='asc'): + marker=None, offset=None, limit=None, + tags_filter_type="all", sort_field='id', sort_dir='asc'): # Sanity checks, in case someone accidentally explicitly passes in 'None' if not sort_field: sort_field = 'id' @@ -86,12 +86,12 @@ def story_get_all(title=None, description=None, status=None, assignee_id=None, query = query.filter(models.StorySummary.status.in_(status)) # paginate the query - query = api_base.paginate_query(query=query, model=models.StorySummary, limit=limit, sort_key=sort_field, marker=marker, + offset=offset, sort_dir=sort_dir) raw_stories = query.all() diff --git a/storyboard/db/api/users.py b/storyboard/db/api/users.py index 1651efc1..f057a6db 100644 --- a/storyboard/db/api/users.py +++ b/storyboard/db/api/users.py @@ -25,10 +25,12 @@ def user_get(user_id, filter_non_public=False): return entity -def user_get_all(marker=None, limit=None, filter_non_public=False, - sort_field=None, sort_dir=None, **kwargs): +def user_get_all(marker=None, offset=None, limit=None, + filter_non_public=False, sort_field=None, sort_dir=None, + **kwargs): return api_base.entity_get_all(models.User, marker=marker, + offset=offset, limit=limit, filter_non_public=filter_non_public, sort_field=sort_field,