Api pagination by will-moore · Pull Request #5067 · ome/openmicroscopy

will-moore · 2017-01-31T16:25:10Z

What this PR does

Updates pagination to use ?offset=200 to show page 2 of results instead of ?page=2.
We also count the total number of objects that would be returned by all "list objects" queries and return this in a new "meta" json object, along with other pagination info (current limit, offset & max limit):
E.g. /images/

"meta": {
    "totalCount": 16739,
    "limit": 200,
    "maxLimit": 500,
    "offset": 0
},

This PR also adds new config settings for the default api "limit" and the "max limit" in a new api_settings.py. Other api settings have also been moved to this file.

Testing this PR

Go to any top-level object listing, e.g. /projects/ or /images/ and check for "meta" in json.
- Check that 'totalCount' is correct (same as in clients)
Filter E.g. filter images by Dataset / orphaned and check that totalCount is correct.
Try using ?offset=100 and / or ?limit=50. Check that these values are shown in "meta" (and that they do pagination as expected)
Test the max limit by trying a limit above 500. E.g. use ?limit=1000. Should see that the actual limit used in the query is restricted to the max_limit (500).
Configure the default limit bin/omero config set omero.web.api.limit 300
Configure the max limit bin/omero config set omero.web.api.max_limit 1000
Check that these are used correctly in the scenarios above.

Related reading

Discussed at web meeting 2017-01-26 and 2017-02-08

…':{}

snoopycrimecop · 2017-02-08T03:33:11Z

Conflicting PR. Removed from build OMERO-DEV-merge-push#584. See the console output for more details.
Possible conflicts:

PR Api wells #5047 will-moore 'Api wells'
- components/tools/OmeroWeb/test/integration/test_api_images.py
- components/tools/OmeroWeb/test/integration/test_api_containers.py
PR Api child count #5058 will-moore 'Api child count'
- components/tools/OmeroWeb/test/integration/test_api_images.py
- components/tools/OmeroWeb/test/integration/test_api_containers.py

snoopycrimecop · 2017-02-09T03:33:22Z

Conflicting PR. Removed from build OMERO-DEV-merge-push#585. See the console output for more details.
Possible conflicts:

PR remove django 1.6 related workarounds #4991 aleksandra-tarkowska 'remove django 1.6 related workarounds'
- components/tools/OmeroWeb/omeroweb/settings.py
PR Api wells #5047 will-moore 'Api wells'
- components/tools/OmeroWeb/test/integration/test_api_images.py
- components/tools/OmeroWeb/test/integration/test_api_containers.py
PR Api child count #5058 will-moore 'Api child count'
- components/tools/OmeroWeb/test/integration/test_api_images.py
- components/tools/OmeroWeb/test/integration/test_api_containers.py
- components/tools/OmeroWeb/omeroweb/settings.py

snoopycrimecop · 2017-02-20T03:32:52Z

Conflicting PR. Removed from build OMERO-DEV-merge-push#593. See the console output for more details.
Possible conflicts:

Conflict detection failed
- components/tools/OmeroWeb/test/integration/test_api_images.py

jburel · 2017-02-20T10:49:51Z

components/tools/OmeroPy/src/omero/gateway/__init__.py

+        unnecessary objects to query.
+        We return just the query and omero.sys.ParametersI for the query.
+
+        :param obj_type:    Object type. E.g. "Project" see above


typo e.g.

jburel · 2017-02-20T10:50:03Z

components/tools/OmeroPy/src/omero/gateway/__init__.py

+        :param opts:        Dict of options for filtering by
+                            offset, limit and owner for all objects.
+                            Additional opts handled by _getQueryString()
+                            E.g. filter Dataset by 'project'


jburel · 2017-02-20T11:04:41Z

components/tools/OmeroWeb/omeroweb/api/decorators.py

 from functools import update_wrapper
-from api_exceptions import NotFoundError, BadRequestError, CreatedObject
+from api_exceptions import NotFoundError, BadRequestError, CreatedObject, \
+    MethodNotSupportedError


jburel · 2017-02-21T09:18:36Z

components/tools/OmeroPy/src/omero/gateway/__init__.py

+                            E.g. filter Dataset by 'project'
+        :return:            (query, params)
+        """
+        # We disable pagination since we want to count ALL results


something to test for large number of results returned

jburel · 2017-02-21T09:20:45Z

components/tools/OmeroWeb/omeroweb/api/api_settings.py

+
+# For any given release of api, we may support
+# one or more versions of the api.
+# E.g. /api/v1.0/


jburel · 2017-02-21T09:32:20Z

components/tools/OmeroWeb/test/integration/test_api_projects.py

        # Check that page 2 gives next 2 projects
-        page = 2
-        payload = {'limit': limit, 'page': page}
+        payload = {'limit': limit, 'offset': limit}


just to avoid possible mistake or miss some errors, I will use different values e.g. limit 2 projects 3 instead of 2/2

jburel · 2017-02-21T09:39:19Z

FYI Point 1: this is difficult in the sense that for example all projects are loaded via api and not possible to verify in client e.g. log as "root"
Better to be tested one via "standard call" and one via api and compare the result

will-moore · 2017-02-21T09:49:14Z

@jburel You can reproduce client behaviour of filtering by group & owner by using ?group=3&owner=4

jburel · 2017-02-21T09:58:13Z

I was going for the general call. I think it will be better to rely on a test b/c I have to use filtering in order to validate. This means that I cannot validate w/o filtering

will-moore · 2017-02-21T10:44:44Z

OK, I will add a test that checks the "totalCount" when filtering by group & owner.

for example, duplicate Wells were being counted, 1 per WellSample.

will-moore · 2017-02-21T16:13:11Z

Last commit fixes this issue:
When counting wells the query was counting Wells multiple times, once for each WellSample.

Wells are loaded with this query:

  select obj from Well obj join fetch obj.details.owner as owner join fetch obj.details.creationEvent
  left outer join fetch obj.wellSamples as wellSamples
  left outer join fetch wellSamples.image as image
  left outer join fetch wellSamples.plateAcquisition as plateAcquisition
  where obj.plate.id = :pid order by obj.column, obj.row, obj.id

Wells were being counted with this query:

select count(obj) from Well obj join  obj.details.owner as owner join  obj.details.creationEvent
left outer join  obj.wellSamples as wellSamples
left outer join  wellSamples.image as image
left outer join  wellSamples.plateAcquisition as plateAcquisition
where obj.plate.id = :pid

atarkowska · 2017-02-21T16:17:40Z

why do you need all optional joins if you don't use them in counting query?

will-moore · 2017-02-21T16:24:13Z

For every query that we make with the /api/ we also do a 'count' query, so there is a simple conversion function that creates the 'count' query from the original 'data loading' query.
See conn.buildCountQuery(). This essentially does:

        query = query.replace("select obj ", "select count(distinct obj) ")
        query = query.replace("fetch", "")
        query = query.split("order by")[0]

Any more-complex conversion we do will have to work with ALL the queries, so will need to be more customised to different data types.

will-moore · 2017-02-21T22:32:42Z

To test last few commits:

try negative numbers for ?offset and ?limit
Check ['meta']['totalCount'] json is correct for Wells in /plates/:id/wells/

jburel · 2017-02-22T12:07:34Z

maxLimit and limit should be > 0 e.g. ?limit=0 should reset to the default
I should not be able to set negative value in config for them
localhost:4080/api/v0/m/projects/
result
{"meta": {"totalCount": 1, "maxLimit": -1, "limit": -1, "offset": 0}, "data": [{"url:project": "http://localhost:4080/api/v0/m/projects/1/", "omero:details": {"owner": {"UserName": "root", "FirstName": "root", "omero:details": {"@type": "TBD#Details", "permissions": {"canDelete": true, "perm": "------", "canEdit": true, "canAnnotate": true, "canLink": true, "@type": "TBD#Permissions"}}, "LastName": "root", "@id": 0, "@type": "http://www.openmicroscopy.org/Schemas/OME/2016-06#Experimenter"}, "group": {"omero:details": {"@type": "TBD#Details", "permissions": {"canDelete": true, "perm": "rw----", "canEdit": true, "canAnnotate": true, "canLink": true, "@type": "TBD#Permissions"}}, "@id": 0, "@type": "http://www.openmicroscopy.org/Schemas/OME/2016-06#ExperimenterGroup", "Name": "system"}, "@type": "TBD#Details", "permissions": {"canDelete": true, "perm": "rw----", "canEdit": true, "canAnnotate": true, "canLink": true, "@type": "TBD#Permissions"}}, "url:datasets": "http://localhost:4080/api/v0/m/projects/1/datasets/", "@id": 1, "@type": "http://www.openmicroscopy.org/Schemas/OME/2016-06#Project", "Name": "project1"}]}

will-moore · 2017-02-22T12:37:02Z

If I go to https://cowfish.openmicroscopy.org/webtrial/api/v0/m/projects/?limit=-1
I see

"meta": {
"totalCount": 64,
"maxLimit": 500,
"limit": 200,
"offset": 0
},

jburel · 2017-02-22T12:38:36Z

That's not the problem.
I was allowed to set the configuration to negative value
e.g.
bin/omero config set omero.web.api.max_limit -1

will-moore · 2017-02-22T12:38:46Z

If I use ?limit=0 (which is supported by the OMERO api) I get this, which is potentially useful

"meta": {
"totalCount": 64,
"maxLimit": 500,
"limit": 0,
"offset": 0
},
"data": []

will-moore · 2017-02-22T12:39:55Z

Ah, OK - sorry. Yes, I guess the smallest max_limit should be 1

jburel · 2017-02-22T12:40:32Z

It is useful if do not have any results
but when I do http://web-dev-merge.openmicroscopy.org/api/v0/m/plates/1902/wells/?limit=0
{"meta": {"totalCount": 10, "maxLimit": 500, "limit": 0, "offset": 0}, "data": [{"WellSamples": [{"omero:details": {"owner": {"UserName": "root", "FirstName": "root", "MiddleName": "", "omero:details": {"@type": "TBD#Details", "permissions": {"isUserWrite": false, "isWorldWrite": false, "canDelete": true, "isWorldRead": false, "perm": "------", "canEdit": true, "canAnnotate": true, "isGroupAnnotate": false, "isGroupWrite": false, "canLink": true, "isUserRead": false, "@type": "TBD#Permissions", "isGroupRead": false}}, "Email": "", "LastName": "root", "@id": 0, "@type": "http://www.openmicroscopy.org/Schemas/OME/2016-06#Experimenter"}, "group": {"omero:details": {"@type": "TBD#Details", "permissions": {"isUserWrite": true, "isWorldWrite": false, "canDelete": true, "isWorldRead": false, "perm": "
I still have data

will-moore · 2017-02-22T12:52:10Z

Very strange - I can repeat locally too. But I think this must be something about that particular query, (loading Wells and joining WellSamples etc) that is causing limit=0 to be interpreted as limit=1.
It seems to work fine for other queries, e.g.
http://web-dev-merge.openmicroscopy.org/api/v0/m/projects/1226/datasets/?limit=0

jburel · 2017-02-22T12:54:23Z

it works for projects, datasets etc.

Having this fail silently and return 0 is not helpful. Users need to know that something has gone wrong. Now uses same logic as webcient views.get_long_or_default()

will-moore · 2017-02-22T23:38:11Z

@joshmoore Any idea why using params of limit=0 and offset=0 for this query doesn't return 0 objects (I get 1 result):

select obj from Well obj join fetch obj.details.owner as owner join fetch obj.details.creationEvent
left outer join fetch obj.wellSamples as wellSamples
left outer join fetch wellSamples.image as image
left outer join fetch wellSamples.plateAcquisition as plateAcquisition
where obj.plate.id = :pid order by obj.column, obj.row, obj.id

When I don't load WellSamples etc then limit=0 works OK (get no results) with

select obj from Well obj join fetch obj.details.owner as owner join fetch obj.details.creationEvent
where obj.plate.id = :pid order by obj.column, obj.row, obj.id

@jburel I now handle config of omero.web.api.limit and omero.web.api.max_limit to be a minimum value of 1.

joshmoore · 2017-02-23T07:16:02Z

@joshmoore Any idea why using params of limit=0 and offset=0 for this query doesn't return 0 objects (I get 1 result):

This might be caused by https://hibernate.atlassian.net/browse/HHH-8989 -- can you detect when limit==0 and not run the query?

jburel · 2017-02-23T12:28:33Z

The negative values are now correctly handled
@will-moore did you have a chance to look at @joshmoore's suggestion?

will-moore · 2017-02-23T13:07:33Z

Sorry - missed @joshmoore's comment notification somehow. Yes, that should be trivial to do.

jburel · 2017-02-24T11:17:56Z

Looks good Thanks all

will-moore added 5 commits January 27, 2017 16:06

Use ?offset=200 parameter instead of page=2

e3eba8c

Blitz conn.buildCountQuery() based on conn.buildQuery()

4778aa8

api_query.py uses conn.buildCountQuery() to add 'totalCount' to 'meta…

6bbdfc6

…':{}

Test 'meta' dict for listing Datasets & Plates

0e862ff

Test 'meta' dict for listing Images, orphans

bed8a33

jburel added the develop label Feb 1, 2017

will-moore added 2 commits February 7, 2017 12:14

Added omero.web.max_page_size 500 to settings.py

f99f34a

Limit page size to MAX_PAGE

c558aed

will-moore force-pushed the api_pagination branch from b4884f4 to c558aed Compare February 7, 2017 12:15

flake8 fix

3e9b8ee

snoopycrimecop mentioned this pull request Feb 8, 2017

Api filter by children #5088

Merged

2 tasks

Moving api settings to api_settings.py

cfa2530

will-moore closed this Feb 9, 2017

will-moore added 2 commits February 17, 2017 12:46

Merge remote-tracking branch 'origin/develop' into api_pagination

fed1680

Fix merge conflict

7fbb04d

will-moore reopened this Feb 17, 2017

will-moore added 3 commits February 17, 2017 12:56

Don't need to remove opts['child_count'] in buildCountQuery()

5eb47b9

Fix test_api_images.py to use api_settings and MAX_LIMIT

1057dce

Fix API_VERSIONS to '0' from merge conflict

88ee449

will-moore added 3 commits February 20, 2017 11:07

Merge remote-tracking branch 'origin/develop' into api_pagination

d5f425c

flake8 fixes from merge conflict

418eb3b

Tiny fix to avoid merge conflict

faddc5d

jburel reviewed Feb 21, 2017

View reviewed changes

will-moore closed this Feb 21, 2017

will-moore reopened this Feb 21, 2017

Use 'select count(distinct obj) to avoid duplicates

ca8807d

for example, duplicate Wells were being counted, 1 per WellSample.

Check ['meta']['totalCount'] for api_project and api_wells tests

d094f49

will-moore added 4 commits February 22, 2017 14:17

Move 'offset' and 'limit' validation to validate_opts()

832b433

Remove try/except from gateway.util.getIntOrDefault()

57d9199

Having this fail silently and return 0 is not helpful. Users need to know that something has gone wrong. Now uses same logic as webcient views.get_long_or_default()

Add test_invalid_parameter() to test_api_errors.py

fb4b621

tests include limit=-1 and limit=0

2d9cbfb

will-moore mentioned this pull request Feb 23, 2017

Api wells2 #5069

Merged

If ?limit=0 result = []

f08de8d

jburel merged commit a161fa5 into ome:develop Feb 24, 2017

jburel added this to the 5.3.0 milestone Mar 29, 2017

will-moore deleted the api_pagination branch February 18, 2019 04:15

Conversation

will-moore commented Jan 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Testing this PR

Related reading

Uh oh!

snoopycrimecop commented Feb 8, 2017

Uh oh!

snoopycrimecop commented Feb 9, 2017

Uh oh!

snoopycrimecop commented Feb 20, 2017

Uh oh!

jburel Feb 20, 2017

Choose a reason for hiding this comment

Uh oh!

jburel Feb 20, 2017

Choose a reason for hiding this comment

Uh oh!

jburel Feb 20, 2017

Choose a reason for hiding this comment

Uh oh!

jburel Feb 21, 2017

Choose a reason for hiding this comment

Uh oh!

jburel Feb 21, 2017

Choose a reason for hiding this comment

Uh oh!

jburel Feb 21, 2017

Choose a reason for hiding this comment

Uh oh!

jburel commented Feb 21, 2017

Uh oh!

will-moore commented Feb 21, 2017

Uh oh!

jburel commented Feb 21, 2017

Uh oh!

will-moore commented Feb 21, 2017

Uh oh!

will-moore commented Feb 21, 2017

Uh oh!

atarkowska commented Feb 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-moore commented Feb 21, 2017

Uh oh!

will-moore commented Feb 21, 2017

Uh oh!

jburel commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-moore commented Feb 22, 2017

Uh oh!

jburel commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-moore commented Feb 22, 2017

Uh oh!

will-moore commented Feb 22, 2017

Uh oh!

jburel commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-moore commented Feb 22, 2017

Uh oh!

jburel commented Feb 22, 2017

Uh oh!

will-moore commented Feb 22, 2017

Uh oh!

joshmoore commented Feb 23, 2017

Uh oh!

jburel commented Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-moore commented Feb 23, 2017

Uh oh!

jburel commented Feb 24, 2017

Uh oh!

Reviewers

Assignees

will-moore commented Jan 31, 2017 •

edited

Loading

atarkowska commented Feb 21, 2017 •

edited

Loading

jburel commented Feb 22, 2017 •

edited

Loading

jburel commented Feb 22, 2017 •

edited

Loading

jburel commented Feb 22, 2017 •

edited

Loading

jburel commented Feb 23, 2017 •

edited

Loading