From 8e43f917510dc38635814a89325309b0d00064dd Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 30 Jul 2017 04:11:32 +0100 Subject: [PATCH] Finish retrieving documents by revision_id, including with filters. --- deckhand/control/README.rst | 8 ++- deckhand/control/revisions.py | 15 +++-- deckhand/db/sqlalchemy/api.py | 65 ++++++++++++++++++- deckhand/engine/document_validation.py | 35 ---------- deckhand/errors.py | 4 ++ deckhand/tests/test_utils.py | 11 +++- deckhand/tests/unit/control/test_api.py | 21 ++++-- deckhand/tests/unit/db/test_documents.py | 46 +++++++++++-- .../tests/unit/db/test_documents_negative.py | 44 +++++++++++++ deckhand/utils.py | 47 ++++++++++++++ 10 files changed, 237 insertions(+), 59 deletions(-) create mode 100644 deckhand/tests/unit/db/test_documents_negative.py create mode 100644 deckhand/utils.py diff --git a/deckhand/control/README.rst b/deckhand/control/README.rst index a8b49c57..3f13ade6 100644 --- a/deckhand/control/README.rst +++ b/deckhand/control/README.rst @@ -22,7 +22,9 @@ Document creation can be tested locally using (from root deckhand directory): .. code-block:: console - curl -i -X POST localhost:9000/api/v1.0/documents \ - -H "Content-Type: application/x-yaml" \ - --data-binary "@deckhand/tests/unit/resources/sample.yaml" + $ curl -i -X POST localhost:9000/api/v1.0/documents \ + -H "Content-Type: application/x-yaml" \ + --data-binary "@deckhand/tests/unit/resources/sample.yaml" + # revision_id copy/pasted from previous response. + $ curl -i -X GET localhost:9000/api/v1.0/revisions/0e99c8b9-bab4-4fc7-8405-7dbd22c33a30/documents diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index a182c63d..77b1fdec 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -23,8 +23,7 @@ from oslo_serialization import jsonutils as json from deckhand.control import base as api_base from deckhand.db.sqlalchemy import api as db_api -from deckhand.engine import document_validation -from deckhand import errors as deckhand_errors +from deckhand import errors LOG = logging.getLogger(__name__) @@ -40,6 +39,12 @@ class RevisionsResource(api_base.BaseResource): documents will be as originally posted with no substitutions or layering applied. """ - revision = db_api.revision_get(revision_id) - resp.status = falcon.HTTP_201 - resp.body = revision['documents'] + params = req.params + LOG.debug('PARAMS: %s' % params) + try: + documents = db_api.revision_get_documents(revision_id, **params) + except errors.RevisionNotFound as e: + return self.return_error(resp, falcon.HTTP_403, message=e) + + resp.status = falcon.HTTP_200 + resp.body = json.dumps(documents) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 7d42fda9..172e16d1 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -15,6 +15,7 @@ """Defines interface for DB access.""" +import ast import copy import datetime import threading @@ -36,6 +37,8 @@ from sqlalchemy import sql import sqlalchemy.sql as sa_sql from deckhand.db.sqlalchemy import models +from deckhand import errors +from deckhand import utils sa_logger = None LOG = logging.getLogger(__name__) @@ -190,6 +193,64 @@ def revision_create(session=None): def revision_get(revision_id, session=None): + """Return the specified `revision_id`. + + :raises: RevisionNotFound if the revision was not found. + """ session = session or get_session() - revision = session.query(models.Revision).get(revision_id) - return revision.to_dict() + try: + revision = session.query(models.Revision).filter_by( + id=revision_id).one().to_dict() + except sa_orm.exc.NoResultFound: + raise errors.RevisionNotFound(revision=revision_id) + + return revision + + +def revision_get_documents(revision_id, session=None, **filters): + """Return the documents that match filters for the specified `revision_id`. + + :raises: RevisionNotFound if the revision was not found. + """ + session = session or get_session() + try: + revision = session.query(models.Revision).filter_by( + id=revision_id).one().to_dict() + except sa_orm.exc.NoResultFound: + raise errors.RevisionNotFound(revision=revision_id) + + filtered_documents = _filter_revision_documents( + revision['documents'], **filters) + return filtered_documents + + +def _filter_revision_documents(documents, **filters): + """Return the list of documents that match filters. + + :returns: list of documents that match specified filters. + """ + # TODO: Implement this as an sqlalchemy query. + filtered_documents = [] + + for document in documents: + match = True + + for filter_key, filter_val in filters.items(): + actual_val = utils.multi_getattr(filter_key, document) + + if (isinstance(actual_val, bool) + and isinstance(filter_val, six.text_type)): + try: + filter_val = ast.literal_eval(filter_val.title()) + except ValueError: + # If not True/False, set to None to avoid matching + # `actual_val` which is always boolean. + filter_val = None + + if actual_val != filter_val: + match = False + + if match: + filtered_documents.append(document) + + return filtered_documents diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 2d9eed68..44494fb1 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -98,38 +98,3 @@ class DocumentValidation(object): except jsonschema.exceptions.ValidationError as e: raise errors.InvalidFormat('The provided YAML file is invalid. ' 'Exception: %s.' % e.message) - - def _multi_getattr(self, multi_key, substitutable_data): - """Iteratively check for nested attributes in the YAML data. - - Check for nested attributes included in "dest" attributes in the data - section of the YAML file. For example, a "dest" attribute of - ".foo.bar.baz" should mean that the YAML data adheres to: - - .. code-block:: yaml - - --- - foo: - bar: - baz: - - :param multi_key: A multi-part key that references nested data in the - substitutable part of the YAML data, e.g. ".foo.bar.baz". - :param substitutable_data: The section of data in the YAML data that - is intended to be substituted with secrets. - :returns: Tuple where first value is a boolean indicating that the - nested attribute was found and the second value is the attribute - that was not found, if applicable. - """ - attrs = multi_key.split('.') - # Ignore the first attribute if it is "." as that is a self-reference. - if attrs[0] == '': - attrs = attrs[1:] - - data = substitutable_data - for attr in attrs: - if attr not in data: - return False, attr - data = data.get(attr) - - return True, None diff --git a/deckhand/errors.py b/deckhand/errors.py index f79341d3..0477e9c1 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -57,3 +57,7 @@ class DocumentExists(DeckhandException): msg_fmt = ("Document with kind %(kind)s and schemaVersion " "%(schema_version)s already exists.") + +class RevisionNotFound(DeckhandException): + msg_fmt = ("The requested revision %(revision)s was not found.") + code = 403 diff --git a/deckhand/tests/test_utils.py b/deckhand/tests/test_utils.py index cefb16fd..f3812548 100644 --- a/deckhand/tests/test_utils.py +++ b/deckhand/tests/test_utils.py @@ -34,7 +34,7 @@ def rand_uuid_hex(): return uuid.uuid4().hex -def rand_name(name='', prefix='tempest'): +def rand_name(name='', prefix='deckhand'): """Generate a random name that includes a random number :param str name: The name that you want to include @@ -51,3 +51,12 @@ def rand_name(name='', prefix='tempest'): if prefix: rand_name = prefix + '-' + rand_name return rand_name + + +def rand_bool(): + """Generate a random boolean value. + + :return: a random boolean value. + :rtype: boolean + """ + return random.choice([True, False]) diff --git a/deckhand/tests/unit/control/test_api.py b/deckhand/tests/unit/control/test_api.py index 2ab5dd4f..a5fe4d85 100644 --- a/deckhand/tests/unit/control/test_api.py +++ b/deckhand/tests/unit/control/test_api.py @@ -18,17 +18,25 @@ import testtools from deckhand.control import api from deckhand.control import base as api_base +from deckhand.control import documents +from deckhand.control import revisions +from deckhand.control import secrets class TestApi(testtools.TestCase): + def setUp(self): + super(TestApi, self).setUp() + for resource in (documents, revisions, secrets): + resource_name = resource.__name__.split('.')[-1] + resource_obj = mock.patch.object( + resource, '%sResource' % resource_name.title()).start() + setattr(self, '%s_resource' % resource_name, resource_obj) @mock.patch.object(api, 'db_api', autospec=True) @mock.patch.object(api, 'config', autospec=True) - @mock.patch.object(api, 'secrets', autospec=True) - @mock.patch.object(api, 'documents', autospec=True) @mock.patch.object(api, 'falcon', autospec=True) - def test_start_api(self, mock_falcon, mock_documents, mock_secrets, + def test_start_api(self, mock_falcon, mock_config, mock_db_api): mock_falcon_api = mock_falcon.API.return_value @@ -38,9 +46,10 @@ class TestApi(testtools.TestCase): mock_falcon.API.assert_called_once_with( request_type=api_base.DeckhandRequest) mock_falcon_api.add_route.assert_has_calls([ - mock.call( - '/api/v1.0/documents', mock_documents.DocumentsResource()), - mock.call('/api/v1.0/secrets', mock_secrets.SecretsResource()) + mock.call('/api/v1.0/documents', self.documents_resource()), + mock.call('/api/v1.0/revisions/{revision_id}/documents', + self.revisions_resource()), + mock.call('/api/v1.0/secrets', self.secrets_resource()) ]) mock_config.parse_args.assert_called_once_with() mock_db_api.setup_db.assert_called_once_with() diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index f2fd6335..2ae5053d 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -12,9 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import mock -import uuid - import testtools from testtools import matchers @@ -33,9 +30,17 @@ class DocumentFixture(object): @staticmethod def get_minimal_fixture(**kwargs): - fixture = {'data': test_utils.rand_name('data'), - 'metadata': {'name': test_utils.rand_name('name')}, - 'schema': test_utils.rand_name('schema', prefix='deckhand')} + fixture = { + 'data': test_utils.rand_name('data'), + 'metadata': { + 'name': test_utils.rand_name('metadata_data'), + 'label': test_utils.rand_name('metadata_label'), + 'layeringDefinition': { + 'abstract': test_utils.rand_bool(), + 'layer': test_utils.rand_name('layer') + } + }, + 'schema': test_utils.rand_name('schema')} fixture.update(kwargs) return fixture @@ -45,7 +50,7 @@ class DocumentFixture(object): for _ in range(count)] -class TestDocumentsApi(base.DeckhandWithDBTestCase): +class TestDocumentsBase(base.DeckhandWithDBTestCase): def _create_documents(self, payload): if not isinstance(payload, list): @@ -66,6 +71,12 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self._validate_revision(revision) return revision + def _get_revision_documents(self, revision_id, **filters): + documents = db_api.revision_get_documents(revision_id, **filters) + for document in documents: + self._validate_document(document) + return documents + def _validate_object(self, obj): for attr in BASE_EXPECTED_FIELDS: if attr.endswith('_at'): @@ -98,6 +109,9 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): for attr in REVISION_EXPECTED_FIELDS: self.assertIn(attr, revision) + +class TestDocuments(TestDocumentsBase): + def test_create_and_get_document(self): payload = DocumentFixture.get_minimal_fixture() documents = self._create_documents(payload) @@ -148,3 +162,21 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): revision = self._get_revision(document['revision_id']) self._validate_revision(revision) self.assertEqual(document['revision_id'], revision['id']) + + def test_get_documents_by_revision_id_and_filters(self): + payload = DocumentFixture.get_minimal_fixture() + document = self._create_documents(payload)[0] + filters = { + 'schema': document['schema'], + 'metadata.name': document['metadata']['name'], + 'metadata.layeringDefinition.abstract': + document['metadata']['layeringDefinition']['abstract'], + 'metadata.layeringDefinition.layer': + document['metadata']['layeringDefinition']['layer'], + 'metadata.label': document['metadata']['label'] + } + + documents = self._get_revision_documents( + document['revision_id'], **filters) + self.assertEqual(1, len(documents)) + self.assertEqual(document, documents[0]) diff --git a/deckhand/tests/unit/db/test_documents_negative.py b/deckhand/tests/unit/db/test_documents_negative.py new file mode 100644 index 00000000..372cb787 --- /dev/null +++ b/deckhand/tests/unit/db/test_documents_negative.py @@ -0,0 +1,44 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import testtools + +from deckhand.db.sqlalchemy import api as db_api +from deckhand.tests import test_utils +from deckhand.tests.unit import base +from deckhand.tests.unit.db import test_documents + + +class TestDocumentsNegative(test_documents.TestDocumentsBase): + + def test_get_documents_by_revision_id_and_wrong_filters(self): + payload = test_documents.DocumentFixture.get_minimal_fixture() + document = self._create_documents(payload)[0] + filters = { + 'schema': 'fake_schema', + 'metadata.name': 'fake_meta_name', + 'metadata.layeringDefinition.abstract': + not document['metadata']['layeringDefinition']['abstract'], + 'metadata.layeringDefinition.layer': 'fake_layer', + 'metadata.label': 'fake_label' + } + + documents = self._get_revision_documents( + document['revision_id'], **filters) + self.assertEmpty(documents) + + for filter_key, filter_val in filters.items(): + documents = self._get_revision_documents( + document['revision_id'], filter_key=filter_val) + self.assertEmpty(documents) diff --git a/deckhand/utils.py b/deckhand/utils.py new file mode 100644 index 00000000..0e44eefd --- /dev/null +++ b/deckhand/utils.py @@ -0,0 +1,47 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +def multi_getattr(multi_key, dict_data): + """Iteratively check for nested attributes in the YAML data. + + Check for nested attributes included in "dest" attributes in the data + section of the YAML file. For example, a "dest" attribute of + ".foo.bar.baz" should mean that the YAML data adheres to: + + .. code-block:: yaml + + --- + foo: + bar: + baz: + + :param multi_key: A multi-part key that references nested data in the + substitutable part of the YAML data, e.g. ".foo.bar.baz". + :param substitutable_data: The section of data in the YAML data that + is intended to be substituted with secrets. + :returns: nested entry in ``dict_data`` if present; else None. + """ + attrs = multi_key.split('.') + # Ignore the first attribute if it is "." as that is a self-reference. + if attrs[0] == '': + attrs = attrs[1:] + + data = dict_data + for attr in attrs: + if attr not in data: + return None + data = data.get(attr) + + return data