airship
/
deckhand
Code Issues Proposed changes

Implement sort filter 

This PS implements the sort filter, allowing (for now)
the GET /revisions and GET /revision/{revision_id}/documents
endpoints to be sorted as per the API documentation in
Deckhand [0].

An additional filter has also been added to the 2 aforementioned
endpoints as well -- order -- which determines the order in
which sorted results are returned: "asc" for ascending
order and "desc" for descending order.

[0] http://deckhand.readthedocs.io/en/latest/api_ref.html#get-revisions-revision-id-documents

Change-Id: Ifb9e15b8379b0a28889a14c331d81d9a4147f1d4
This commit is contained in:
Felipe Monteiro 2017-11-29 04:07:15 +00:00
parent 639ac18dac
commit 16c7ec196f
13 changed files with 302 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
deckhand/control/common.py
View File

@ -41,7 +41,9 @@ def sanitize_params(allowed_params):
'status.bucket': 'bucket_name',
'metadata.label': 'metadata.labels',
# Mappings for revisions.
'tag': 'tags.[*].tag'
'tag': 'tags.[*].tag',
# Mappings for sorting.
'createdAt': 'created_at'
}
def decorator(func):

13
deckhand/control/revision_documents.py
View File

@ -24,6 +24,7 @@ from deckhand.engine import document_validation
from deckhand.engine import secrets_manager
from deckhand import errors
from deckhand import policy
from deckhand import utils
LOG = logging.getLogger(__name__)
@ -37,7 +38,7 @@ class RevisionDocumentsResource(api_base.BaseResource):
@common.sanitize_params([
'schema', 'metadata.name', 'metadata.layeringDefinition.abstract',
'metadata.layeringDefinition.layer', 'metadata.label',
'status.bucket'])
'status.bucket', 'order', 'sort'])
def on_get(self, req, resp, sanitized_params, revision_id):
"""Returns all documents for a `revision_id`.
@ -49,6 +50,12 @@ class RevisionDocumentsResource(api_base.BaseResource):
include_encrypted = policy.conditional_authorize(
'deckhand:list_encrypted_documents', req.context, do_raise=False)
order_by = sort_by = None
if 'order' in sanitized_params:
order_by = sanitized_params.pop('order')
if 'sort' in sanitized_params:
sort_by = sanitized_params.pop('sort')
filters = sanitized_params.copy()
filters['metadata.storagePolicy'] = ['cleartext']
if include_encrypted:
@ -62,8 +69,10 @@ class RevisionDocumentsResource(api_base.BaseResource):
LOG.exception(six.text_type(e))
raise falcon.HTTPNotFound(description=e.format_message())
sorted_documents = utils.multisort(documents, sort_by, order_by)
resp.status = falcon.HTTP_200
resp.body = self.view_builder.list(documents)
resp.body = self.view_builder.list(sorted_documents)
class RenderedDocumentsResource(api_base.BaseResource):

13
deckhand/control/revisions.py
View File

@ -20,6 +20,7 @@ from deckhand.control.views import revision as revision_view
from deckhand.db.sqlalchemy import api as db_api
from deckhand import errors
from deckhand import policy
from deckhand import utils
class RevisionsResource(api_base.BaseResource):
@ -56,13 +57,19 @@ class RevisionsResource(api_base.BaseResource):
resp.body = revision_resp
@policy.authorize('deckhand:list_revisions')
@common.sanitize_params(['tag'])
@common.sanitize_params(['tag', 'order', 'sort'])
def _list_revisions(self, req, resp, sanitized_params):
order_by = sort_by = None
if 'order' in sanitized_params:
order_by = sanitized_params.pop('order')
if 'sort' in sanitized_params:
sort_by = sanitized_params.pop('sort')
revisions = db_api.revision_get_all(**sanitized_params)
revisions_resp = self.view_builder.list(revisions)
sorted_revisions = utils.multisort(revisions, sort_by, order_by)
resp.status = falcon.HTTP_200
resp.body = revisions_resp
resp.body = self.view_builder.list(sorted_revisions)
@policy.authorize('deckhand:delete_revisions')
def on_delete(self, req, resp):

11
deckhand/db/sqlalchemy/api.py
View File

@ -651,6 +651,7 @@ def _exclude_deleted_documents(documents):
def _filter_revision_documents(documents, unique_only, **filters):
"""Return the list of documents that match filters.
:param documents: List of documents to apply ``filters`` to.
:param unique_only: Return only unique documents if ``True``.
:param filters: Dictionary attributes (including nested) used to filter
out revision documents.
@ -665,11 +666,6 @@ def _filter_revision_documents(documents, unique_only, **filters):
documents = _exclude_deleted_documents(documents)
for document in documents:
# NOTE(fmontei): Only want to include non-validation policy documents
# for this endpoint.
if document['schema'].startswith(types.VALIDATION_POLICY_SCHEMA):
continue
if _apply_filters(document, **filters):
# Filter out redundant documents from previous revisions, i.e.
# documents schema and metadata.name are repeated.
@ -681,8 +677,7 @@ def _filter_revision_documents(documents, unique_only, **filters):
if unique_key not in filtered_documents:
filtered_documents[unique_key] = document
# TODO(fmontei): Sort by user-specified parameter.
return sorted(filtered_documents.values(), key=lambda d: d['created_at'])
return list(filtered_documents.values())
@require_revision_exists
@ -696,8 +691,6 @@ def revision_get_documents(revision_id=None, include_history=True,
and up to current revision, if ``True``. Default is ``True``.
:param unique_only: Return only unique documents if ``True. Default is
``True``.
:param filters: Dictionary attributes (including nested) used to filter
out revision documents.
:param session: Database session object.
:param filters: Key-value pairs used for filtering out revision documents.
:returns: All revision documents for ``revision_id`` that match the

71
deckhand/tests/functional/gabbits/revision-documents-filters.yaml
View File

@ -5,6 +5,7 @@
# * metadata.layeringDefinition.abstract
# * metadata.layeringDefinition.layer
# * status.bucket
# * sort
defaults:
request_headers:
@ -105,3 +106,73 @@ tests:
- mop
- mop
- mop
- name: sort_by_metadata_name
desc: Verify revision documents sorted by metadata.name
GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents
query_parameters:
sort: metadata.name
status: 200
response_multidoc_jsonpaths:
$.`len`: 4
$.[*].metadata.name:
- global-1234
- layering-policy
- region-1234
- site-1234
- name: sort_by_schema
desc: Verify revision documents sorted by schema
GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents
query_parameters:
sort: schema
status: 200
response_multidoc_jsonpaths:
$.`len`: 4
$.[*].schema:
- deckhand/LayeringPolicy/v1
- example/Kind/v1
- example/Kind/v1
- example/Kind/v1
- name: sort_by_schema_then_metadata
desc: Verify revision documents sorted by (schema, metadata)
GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents
query_parameters:
sort:
- schema
- metadata.name
status: 200
response_multidoc_jsonpaths:
$.`len`: 4
$.[*].schema:
- deckhand/LayeringPolicy/v1
- example/Kind/v1
- example/Kind/v1
- example/Kind/v1
$.[*].metadata.name:
- layering-policy
- global-1234
- region-1234
- site-1234
- name: sort_by_metadata_then_schema
desc: Verify revision documents sorted by (metadata, schema)
GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents
query_parameters:
sort:
- metadata.name
- schema
status: 200
response_multidoc_jsonpaths:
$.`len`: 4
$.[*].metadata.name:
- global-1234
- layering-policy
- region-1234
- site-1234
$.[*].schema:
- example/Kind/v1
- deckhand/LayeringPolicy/v1
- example/Kind/v1
- example/Kind/v1

50
deckhand/tests/functional/gabbits/revision-filters.yaml
View File

@ -2,6 +2,7 @@
# * tag
# 2. Test failure paths for filtering revisions for the following filters:
# * tag
# 3. Test success paths for sorting and ordering.
defaults:
request_headers:
@ -17,14 +18,26 @@ tests:
response_headers: null
- name: initialize
desc: Create initial documents
PUT: /api/v1.0/buckets/mop/documents
desc: Create first revision for testing
PUT: /api/v1.0/buckets/bucket_a/documents
status: 200
data: <@resources/design-doc-layering-sample.yaml
- name: initialize_again
desc: Create second revision for testing
PUT: /api/v1.0/buckets/bucket_b/documents
status: 200
data: <@resources/sample-doc.yaml
- name: initialize_once_again
desc: Create third revision for testing
PUT: /api/v1.0/buckets/bucket_c/documents
status: 200
data: <@resources/sample-schema.yaml
- name: create_tag
desc: Create a tag for testing filtering a revision by tag
POST: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/tags/foo
POST: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/tags/foo
status: 201
- name: create_another_tag
@ -42,7 +55,7 @@ tests:
$.`len`: 1
$.[0].count: 1
$.[0].results[0].id: $HISTORY['initialize'].$RESPONSE['$.[0].status.revision']
$.[0].results[0].buckets: [mop]
$.[0].results[0].buckets: [bucket_a]
$.[0].results[0].tags:
# Tags are sorted alphabetically.
- bar
@ -60,7 +73,7 @@ tests:
$.`len`: 1
$.[0].count: 1
$.[0].results[0].id: $HISTORY['initialize'].$RESPONSE['$.[0].status.revision']
$.[0].results[0].buckets: [mop]
$.[0].results[0].buckets: [bucket_a]
$.[0].results[0].tags:
- bar
- foo
@ -75,3 +88,30 @@ tests:
$.`len`: 1
$.[0].count: 0
$.[0].results: []
- name: verify_sort_by_id
desc: Verify revision documents sorted by ID, in ascending order
GET: /api/v1.0/revisions
query_parameters:
sort: id
status: 200
response_multidoc_jsonpaths:
$.[0].results.`len`: 3
$.[0].results[*].id:
- 1
- 2
- 3
- name: verify_sort_by_id_desc
desc: Verify revision documents sorted by ID, in descending order
GET: /api/v1.0/revisions
query_parameters:
sort: id
order: desc
status: 200
response_multidoc_jsonpaths:
$.[0].results.`len`: 3
$.[0].results[*].id:
- 3
- 2
- 1

104
deckhand/tests/unit/control/test_revision_documents_controller.py
View File

@ -80,3 +80,107 @@ class TestRevisionDocumentsControllerNegativeRBAC(
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
self.assertEmpty(list(yaml.safe_load_all(resp.text)))
class TestRevisionDocumentsControllerSorting(test_base.BaseControllerTest):
def test_list_revision_documents_sorting_metadata_name(self):
rules = {'deckhand:list_cleartext_documents': '@',
'deckhand:list_encrypted_documents': '@',
'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
documents_factory = factories.DocumentFactory(2, [1, 1])
documents = documents_factory.gen_test({})
expected_names = ['bar', 'baz', 'foo']
for idx in range(len(documents)):
documents[idx]['metadata']['name'] = expected_names[idx]
resp = self.app.simulate_put(
'/api/v1.0/buckets/mop/documents',
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all(documents))
self.assertEqual(200, resp.status_code)
revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][
'revision']
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/documents' % revision_id,
params={'sort': 'metadata.name'}, params_csv=False,
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
retrieved_documents = list(yaml.safe_load_all(resp.text))
self.assertEqual(3, len(retrieved_documents))
self.assertEqual(expected_names,
[d['metadata']['name'] for d in retrieved_documents])
def test_list_revision_documents_sorting_by_metadata_name_and_schema(self):
rules = {'deckhand:list_cleartext_documents': '@',
'deckhand:list_encrypted_documents': '@',
'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
documents_factory = factories.DocumentFactory(2, [1, 1])
documents = documents_factory.gen_test({})
expected_names = ['foo', 'baz', 'bar']
expected_schemas = ['deckhand/Certificate/v1',
'deckhand/Certificate/v1',
'deckhand/LayeringPolicy/v1']
for idx in range(len(documents)):
documents[idx]['metadata']['name'] = expected_names[idx]
documents[idx]['schema'] = expected_schemas[idx]
resp = self.app.simulate_put(
'/api/v1.0/buckets/mop/documents',
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all(documents))
self.assertEqual(200, resp.status_code)
revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][
'revision']
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/documents' % revision_id,
params={'sort': ['schema', 'metadata.name']}, params_csv=False,
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
retrieved_documents = list(yaml.safe_load_all(resp.text))
self.assertEqual(3, len(retrieved_documents))
self.assertEqual(['baz', 'foo', 'bar'],
[d['metadata']['name'] for d in retrieved_documents])
self.assertEqual(expected_schemas,
[d['schema'] for d in retrieved_documents])
def test_list_revision_documents_sorting_by_schema(self):
rules = {'deckhand:list_cleartext_documents': '@',
'deckhand:list_encrypted_documents': '@',
'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
documents_factory = factories.DocumentFactory(2, [1, 1])
documents = documents_factory.gen_test({})
expected_schemas = ['deckhand/Certificate/v1',
'deckhand/CertificateKey/v1',
'deckhand/LayeringPolicy/v1']
for idx in range(len(documents)):
documents[idx]['schema'] = expected_schemas[idx]
resp = self.app.simulate_put(
'/api/v1.0/buckets/mop/documents',
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all(documents))
self.assertEqual(200, resp.status_code)
revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][
'revision']
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/documents' % revision_id,
params={'sort': 'schema'}, params_csv=False,
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
retrieved_documents = list(yaml.safe_load_all(resp.text))
self.assertEqual(3, len(retrieved_documents))
self.assertEqual(expected_schemas,
[d['schema'] for d in retrieved_documents])

8
deckhand/tests/unit/db/test_documents.py
View File

@ -220,6 +220,8 @@ class TestDocuments(base.TestDbBase):
self.assertEqual(3, len(documents))
documents = self.create_documents(bucket_name, [])
documents = sorted(
documents, key=lambda d: d['name'])
for idx in range(3):
self.assertTrue(documents[idx]['deleted'])
@ -234,7 +236,7 @@ class TestDocuments(base.TestDbBase):
payload = self.documents_factory.gen_test(self.document_mapping)
bucket_name = test_utils.rand_name('bucket')
# Create just 1 document.
documents = self.create_documents(bucket_name, payload[0])
self.create_documents(bucket_name, payload[0])
# Create the document in payload[0] but create a new document for
# payload[1].
@ -260,13 +262,13 @@ class TestDocuments(base.TestDbBase):
payload = self.documents_factory.gen_test(self.document_mapping)
bucket_name = test_utils.rand_name('bucket')
# Create just 1 document.
documents = self.create_documents(bucket_name, payload[1:])
self.create_documents(bucket_name, payload[1:])
# Create the document in payload[0] but create a new document for
# payload[1].
documents = self.create_documents(bucket_name, payload[0])
# The first document will be first, followed by the two deleted docs.
documents = sorted(documents, key=lambda d: d['deleted'])
documents = sorted(documents, key=lambda d: (d['deleted'], d['name']))
# Information about the deleted and created document should've been
# returned. The 1st document is the deleted one and the 2nd document
# is the created one.

10
deckhand/tests/unit/db/test_revision_documents.py
View File

@ -21,9 +21,8 @@ class TestRevisionDocumentsFiltering(base.TestDbBase):
def test_document_filtering_by_bucket_name(self):
document = base.DocumentFixture.get_minimal_fixture()
bucket_name = test_utils.rand_name('bucket')
self.create_documents(bucket_name, document)
revision_id = self.create_documents(bucket_name, [])[0]['revision_id']
revision_id = self.create_documents(bucket_name, [document])[0][
'revision_id']
filters = {'bucket_name': bucket_name}
retrieved_documents = self.list_revision_documents(
@ -37,7 +36,8 @@ class TestRevisionDocumentsFiltering(base.TestDbBase):
bucket_name = test_utils.rand_name('bucket')
self.create_documents(bucket_name, documents)
revision_id = self.create_documents(bucket_name, [])[0]['revision_id']
revision_id = self.create_documents(bucket_name, [])[0][
'revision_id']
retrieved_documents = self.list_revision_documents(
revision_id, include_history=False, deleted=False)
@ -69,6 +69,8 @@ class TestRevisionDocumentsFiltering(base.TestDbBase):
retrieved_documents = self.list_revision_documents(
created_documents[0]['revision_id'],
**{'metadata.storagePolicy': ['cleartext', 'encrypted']})
retrieved_documents = sorted(retrieved_documents,
key=lambda d: d['created_at'])
self.assertEqual([d['id'] for d in all_created_documents],
[d['id'] for d in retrieved_documents])

4
deckhand/tests/unit/db/test_revision_rollback.py
View File

@ -42,6 +42,8 @@ class TestRevisionRollback(base.TestDbBase):
rollback_documents = self.list_revision_documents(
rollback_revision['id'])
rollback_documents = sorted(rollback_documents,
key=lambda d: d['created_at'])
self.assertEqual([1, 1, 1, 3],
[d['revision_id'] for d in rollback_documents])
self.assertEqual([1, 1, 1, 3],
@ -75,6 +77,8 @@ class TestRevisionRollback(base.TestDbBase):
rollback_documents = self.list_revision_documents(
rollback_revision['id'])
rollback_documents = sorted(rollback_documents,
key=lambda d: d['created_at'])
self.assertEqual([1, 1, 4, 4],
[d['revision_id'] for d in rollback_documents])
self.assertEqual([1, 1, 4, 4],

4
deckhand/tests/unit/db/test_revisions.py
View File

@ -55,6 +55,8 @@ class TestRevisions(base.TestDbBase):
revision_documents = self.list_revision_documents(
updated_documents[0]['revision_id'])
revision_documents = sorted(revision_documents,
key=lambda d: d['created_at'])
self.assertEqual(4, len(revision_documents))
self.assertEqual([orig_revision_id] * 3 + [new_revision_id],
@ -174,6 +176,8 @@ class TestRevisions(base.TestDbBase):
alt_revision_docs = self.list_revision_documents(
alt_created_documents[0]['revision_id'])
alt_revision_docs = sorted(alt_revision_docs,
key=lambda d: d['created_at'])
self.assertEqual(2, len(alt_revision_docs))
expected_doc_ids = [created_documents[0]['id'],

23
deckhand/utils.py
View File

@ -144,3 +144,26 @@ def jsonpath_replace(data, value, jsonpath, pattern=None):
d = d.get(path)
return _do_replace()
def multisort(data, sort_by=None, order_by=None):
"""Sort a dictionary by multiple keys.
The order of the keys is important. The first key takes precedence over
the second key, and so forth.
:param data: Dictionary to be sorted.
:param sort_by: list or string of keys to sort ``data`` by.
:type sort_by: list or string
:returns: Sorted dictionary by each key.
"""
if sort_by is None:
sort_by = 'created_at'
if order_by not in ['asc', 'desc']:
order_by = 'asc'
if not isinstance(sort_by, list):
sort_by = [sort_by]
return sorted(data, key=lambda d: [
jsonpath_parse(d, sort_key) for sort_key in sort_by],
reverse=True if order_by == 'desc' else False)

17
doc/source/api_ref.rst
View File

@ -76,12 +76,16 @@ Supported query string parameters:
* ``metadata.label`` - string, optional, repeatable - Uses the format
``metadata.label=key=value``. Repeating this parameter indicates all
requested labels must apply (AND not OR).
* ``sort`` - string, optional, repeatable - Defines the sort order for returning
results. Default is by creation date. Repeating this parameter indicates use
of multi-column sort with the most significant sorting column applied first.
* ``status.bucket`` - string, optional, repeatable - Used to select documents
only from a particular bucket. Repeating this parameter indicates documents
from any of the specified buckets should be returned.
* ``sort`` - string, optional, repeatable - Defines the sort order for returning
results. Default is by creation date. Repeating this parameter indicates use
of multi-column sort with the most significant sorting column applied first.
* ``order`` - string, optional - Valid values are "asc" and "desc". Default is
"asc". Controls the order in which the ``sort`` result is returned: "asc"
returns sorted results in ascending order, while "desc" returns results in
descending order.
GET ``/revisions/{revision_id}/rendered-documents``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -108,6 +112,13 @@ Supported query string parameters:
* ``tag`` - string, optional, repeatable - Used to select revisions that have
been tagged with particular tags.
* ``sort`` - string, optional, repeatable - Defines the sort order for returning
results. Default is by creation date. Repeating this parameter indicates use
of multi-column sort with the most significant sorting column applied first.
* ``order`` - string, optional - Valid values are "asc" and "desc". Default is
"asc". Controls the order in which the ``sort`` result is returned: "asc"
returns sorted results in ascending order, while "desc" returns results in
descending order.
Sample response: