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

Performant pagination #112

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion sandman/model/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from flask import current_app, g
from flask.ext.admin import Admin
from flask.ext.admin.contrib.sqla import ModelView
from flask.ext.sqlalchemy import SQLAlchemy
from sqlalchemy.engine import reflection
from sqlalchemy.ext.declarative import declarative_base, DeferredReflection
from sqlalchemy.orm import relationship
Expand Down Expand Up @@ -199,4 +200,7 @@ def activate(admin=True, browser=True, name='admin', reflect_all=False):
# actually the same thing.

sandman_model = Model
Model = declarative_base(cls=(Model, DeferredReflection))
# Model = declarative_base(cls=(Model, DeferredReflection))

class Model(Model, DeferredReflection, db.Model):
__abstract__ = True
61 changes: 25 additions & 36 deletions sandman/sandman.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,34 +135,28 @@ def _single_resource_html_response(resource):
tablename=tablename))


def _collection_json_response(cls, resources, start, stop, depth=0):
def _collection_json_response(cls, resources, depth=0):
"""Return the JSON representation of the collection *resources*.

:param list resources: list of :class:`sandman.model.Model`s to render
:rtype: :class:`flask.Response`

"""

top_level_json_name = None
if cls.__top_level_json_name__ is not None:
top_level_json_name = cls.__top_level_json_name__
else:
top_level_json_name = 'resources'

result_list = []
for resource in resources:
result_list.append(resource.as_dict(depth))

payload = {}
if start is not None:
payload[top_level_json_name] = result_list[start:stop]
else:
payload[top_level_json_name] = result_list
resource_key = cls.__top_level_json_name__ or 'resources'

payload = {
resource_key: [each.as_dict(depth) for each in resources.items],
'pagination': {
'page': resources.page,
'per_page': resources.per_page,
'count': resources.total,
}
}
return jsonify(payload)


def _collection_html_response(resources, start=0, stop=20):
def _collection_html_response(resources):
"""Return the HTML representation of the collection *resources*.

:param list resources: list of :class:`sandman.model.Model`s to render
Expand All @@ -171,7 +165,7 @@ def _collection_html_response(resources, start=0, stop=20):
"""
return make_response(render_template(
'collection.html',
resources=resources[start:stop]))
resources=resources.items))


def _validate(cls, method, resource=None):
Expand Down Expand Up @@ -244,27 +238,22 @@ def retrieve_collection(collection, query_arguments=None):
:rtype: class:`sandman.model.Model`

"""
session = _get_session()
cls = endpoint_class(collection)
if query_arguments:
filters = []
order = []
limit = None
for key, value in query_arguments.items():
if key == 'page':
if key in ['page', 'limit']:
continue
if value.startswith('%'):
filters.append(getattr(cls, key).like(str(value), escape='/'))
elif key == 'sort':
order.append(getattr(cls, value))
elif key == 'limit':
limit = value
elif key:
filters.append(getattr(cls, key) == value)
resources = session.query(cls).filter(*filters).order_by(
*order).limit(limit)
resources = cls.query.filter(*filters).order_by(*order)
else:
resources = session.query(cls).all()
resources = cls.query
return resources


Expand Down Expand Up @@ -303,7 +292,7 @@ def resource_created_response(resource):
return response


def collection_response(cls, resources, start=None, stop=None):
def collection_response(cls, resources):
"""Return a response for the *resources* of the appropriate content type.

:param resources: resources to be returned in request
Expand All @@ -312,9 +301,9 @@ def collection_response(cls, resources, start=None, stop=None):

"""
if _get_acceptable_response_type() == JSON:
return _collection_json_response(cls, resources, start, stop)
return _collection_json_response(cls, resources)
else:
return _collection_html_response(resources, start, stop)
return _collection_html_response(resources)


def resource_response(resource, depth=0):
Expand Down Expand Up @@ -530,13 +519,13 @@ def get_collection(collection):

_validate(cls, request.method, resources)

start = stop = None

if request.args and 'page' in request.args:
page = int(request.args['page'])
results_per_page = app.config.get('RESULTS_PER_PAGE', 20)
start, stop = page * results_per_page, (page + 1) * results_per_page
return collection_response(cls, resources, start, stop)
try:
page = int(request.args.get('page', 1))
except (TypeError, ValueError):
raise InvalidAPIUsage(422)
per_page = app.config.get('RESULTS_PER_PAGE', 20)
resources = resources.paginate(page, per_page, error_out=False)
return collection_response(cls, resources)


@app.route('/', methods=['GET'])
Expand Down
5 changes: 3 additions & 2 deletions tests/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Models for unit testing sandman"""

from flask.ext.sqlalchemy import BaseQuery
from flask.ext.admin.contrib.sqla import ModelView
from sandman.model import register, Model, activate
from sandman.model.models import db
Expand Down Expand Up @@ -90,11 +91,11 @@ def validate_GET(resource=None):

"""

if isinstance(resource, list):
if isinstance(resource, BaseQuery):
return True
elif resource and resource.GenreId == 1:
return False
return True

register((Artist, Album, Playlist, Track, MediaType, Style, SomeModel))
activate(browser=True)
activate(browser=False)
32 changes: 20 additions & 12 deletions tests/test_sandman.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,27 +70,29 @@ class TestSandmanBasicVerbs(TestSandmanBase):
def test_get(self):
"""Test simple HTTP GET"""
response = self.get_response('/artists', 200)
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 275

def test_get_with_limit(self):
"""Test simple HTTP GET"""
response = self.get_response('/artists', 200, params={'limit': 10})
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 10
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 275
assert len(parsed['resources']) == 20

def test_get_with_filter(self):
"""Test simple HTTP GET"""
response = self.get_response('/artists', 200, params={'Name': 'AC/DC'})
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 1
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 1
assert len(parsed['resources']) == 1

def test_get_with_like_filter(self):
"""Test simple HTTP GET"""
response = self.get_response('/artists', 200, params={'Name': '%AC%DC%'})
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 1
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 1
assert len(parsed['resources']) == 1

def test_get_with_sort(self):
"""Test simple HTTP GET"""
response = self.get_response('/artists', 200, params={'sort': 'Name'})
assert json.loads(response.get_data(as_text=True))[u'resources'][0]['Name'] == 'A Cor Do Som'
parsed = json.loads(response.get_data(as_text=True))
assert parsed['resources'][0]['Name'] == 'A Cor Do Som'

def test_get_attribute(self):
"""Test simple HTTP GET"""
Expand Down Expand Up @@ -263,7 +265,9 @@ class TestSandmanUserDefinitions(TestSandmanBase):
def test_user_defined_endpoint(self):
"""Make sure user-defined endpoint exists."""
response = self.get_response('/styles', 200)
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 25
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 25
assert len(parsed['resources']) == 20

def test_user_validation_reject(self):
"""Test user-defined validation which on request which should be
Expand Down Expand Up @@ -293,7 +297,9 @@ def test_responds_with_top_level_json_name_if_present(self):
"""Test top level json element is the one defined on the Model
rather than the string 'resources'"""
response = self.get_response('/albums', 200)
assert len(json.loads(response.get_data(as_text=True))[u'Albums']) == 347
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 347
assert len(parsed['Albums']) == 20

class TestSandmanValidation(TestSandmanBase):
"""Sandman tests related to request validation"""
Expand Down Expand Up @@ -369,7 +375,9 @@ def test_get_json(self):
response = self.get_response('/artists',
200,
headers={'Accept': 'application/json'})
assert len(json.loads(response.get_data(as_text=True))[u'resources']) == 275
parsed = json.loads(response.get_data(as_text=True))
assert parsed['pagination']['count'] == 275
assert len(parsed['resources']) == 20

def test_get_unknown_url(self):
"""Test sending a GET request to a URL that would match the
Expand Down