From 0b80b5b4e7aa28a18b967ce9a9d59f354b9af8e2 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 21 Jul 2017 23:00:58 +0100 Subject: [PATCH 01/23] Revisions database and API implementation This commit implements the initial design for the revisions database schema and API. --- deckhand/db/sqlalchemy/api.py | 76 +++++++++++++++++++++++++++++++- deckhand/db/sqlalchemy/models.py | 33 +++++++++++--- 2 files changed, 101 insertions(+), 8 deletions(-) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 8491e1d0..fc6ca7e9 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -15,6 +15,7 @@ """Defines interface for DB access.""" +import copy import datetime import threading @@ -117,11 +118,82 @@ def document_create(values, session=None): values = values.copy() values['doc_metadata'] = values.pop('metadata') values['schema_version'] = values.pop('schemaVersion') - session = session or get_session() - document = models.Document() + + existing_document = document_get( + as_dict=False, + **{c: values[c] for c in models.Document.UNIQUE_CONSTRAINTS}) + + def _document_changed(): + other_document = copy.deepcopy(existing_document) + other_document = other_document.to_dict() + # The document has changed if at least one value in ``values`` differs. + for key, val in values.items(): + if val != other_document[key]: + return True + return False + + if existing_document: + # Only generate a new revision for the document if anything has + # changed. + if _document_changed(): + revision_index = revision_update( + revision_index=existing_document['revision_index'])['id'] + values['revision_index'] = revision_index + else: + revision_index = revision_create()['id'] + values['revision_index'] = revision_index + + document = existing_document or models.Document() with session.begin(): document.update(values) document.save(session=session) return document.to_dict() + + +def document_get(session=None, as_dict=True, **filters): + unqiue_constraints = models.Document.UNIQUE_CONSTRAINTS + session = session or get_session() + + if 'document_id' in filters: + # Get the document by `document_id`. + document = session.query(models.Document).get(filters['document_id']) + elif all([c in filters for c in unqiue_constraints]): + # Get the document by its unique constraints. + document = session.query(models.Document) + document = document.filter_by(**{c: filters[c] + for c in unqiue_constraints}).first() + + if as_dict: + return document.to_dict() if document else {} + return document + + +#################### + + +def revision_create(session=None): + session = session or get_session() + revision = models.Revision() + with session.begin(): + revision.save(session=session) + + return revision.to_dict() + + +def revision_update(session=None, revision_index=None): + session = session or get_session() + previous_revision = session.query(models.Revision).get(revision_index) + + new_revision = models.Revision() + with session.begin(): + # Create the new revision with a reference to the previous revision. + new_revision.update({'previous': revision_index}) + new_revision.save(session=session) + + # Update the previous revision with a reference to the new revision. + previous_revision.update({'next': new_revision.id}) + previous_revision.save(session=session) + + return new_revision.to_dict() diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index e8e2a868..0afdd4a0 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -22,6 +22,7 @@ from sqlalchemy import Boolean from sqlalchemy import Column from sqlalchemy import DateTime from sqlalchemy.ext import declarative +from sqlalchemy import ForeignKey from sqlalchemy import Integer from sqlalchemy import orm from sqlalchemy import schema @@ -107,17 +108,23 @@ class DeckhandBase(models.ModelBase, models.TimestampMixin): return d + @staticmethod + def gen_unqiue_contraint(self, *fields): + constraint_name = 'ix_' + self.__class__.__name__.lower() + '_' + for field in fields: + constraint_name = constraint_name + '_%s' % field + return schema.UniqueConstraint(*fields, name=constraint_name) + class Document(BASE, DeckhandBase): - __tablename__ = 'document' - __table_args__ = (schema.UniqueConstraint('schema_version', 'kind', - name='ix_documents_schema_version_kind'),) + UNIQUE_CONSTRAINTS = ('schema_version', 'kind') + __tablename__ = 'documents' + __table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) - # TODO: the revision_index will be a foreign key to a Revision table. - revision_index = Column(String(36), nullable=False, - default=lambda: str(uuid.uuid4())) + revision_index = Column(Integer, ForeignKey('revisions.id'), + nullable=False) schema_version = Column(String(64), nullable=False) kind = Column(String(64), nullable=False) # NOTE: Do not define a maximum length for these JSON data below. However, @@ -127,6 +134,20 @@ class Document(BASE, DeckhandBase): data = Column(JSONEncodedDict(), nullable=False) +class Revision(BASE, DeckhandBase): + """Revision history for a ``Document``. + + Each ``Revision`` will have a unique ID along with a previous and next + pointer to each ``Revision`` that comprises the revision history for a + ``Document``. + """ + __tablename__ = 'revisions' + + id = Column(String(36), primary_key=True, + default=lambda: str(uuid.uuid4())) + previous = Column(Integer, ForeignKey('revisions.id'), nullable=True) + next = Column(Integer, ForeignKey('revisions.id'), nullable=True) + def register_models(engine): """Create database tables for all models with the given engine.""" models = [Document] From f230d9b364dc3ac8dd6170693c776519bfc59451 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 22 Jul 2017 00:03:08 +0100 Subject: [PATCH 02/23] temp --- deckhand/db/sqlalchemy/api.py | 49 ++++++++++++++++---------------- deckhand/db/sqlalchemy/models.py | 33 +++++++++++---------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index fc6ca7e9..9c8d2260 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -29,6 +29,7 @@ import six from six.moves import range import sqlalchemy from sqlalchemy.ext.compiler import compiles +from sqlalchemy import desc from sqlalchemy import MetaData, Table import sqlalchemy.orm as sa_orm from sqlalchemy import sql @@ -120,9 +121,9 @@ def document_create(values, session=None): values['schema_version'] = values.pop('schemaVersion') session = session or get_session() - existing_document = document_get( - as_dict=False, - **{c: values[c] for c in models.Document.UNIQUE_CONSTRAINTS}) + filters = copy.copy(models.Document.UNIQUE_CONSTRAINTS) + filters = [f for f in filters if f != 'revision_index'] + existing_document = document_get(**{c: values[c] for c in filters}) def _document_changed(): other_document = copy.deepcopy(existing_document) @@ -133,41 +134,41 @@ def document_create(values, session=None): return True return False + def _document_create(): + document = models.Document() + with session.begin(): + document.update(values) + document.save(session=session) + return document + + created_document = {} if existing_document: - # Only generate a new revision for the document if anything has - # changed. + # Only generate a new revision and entirely new document if anything + # was changed. if _document_changed(): revision_index = revision_update( revision_index=existing_document['revision_index'])['id'] values['revision_index'] = revision_index + created_document = _document_create().to_dict() + # TODO: indicate that now document was actually created. else: revision_index = revision_create()['id'] values['revision_index'] = revision_index + created_document = _document_create().to_dict() - document = existing_document or models.Document() - with session.begin(): - document.update(values) - document.save(session=session) - - return document.to_dict() + return created_document -def document_get(session=None, as_dict=True, **filters): - unqiue_constraints = models.Document.UNIQUE_CONSTRAINTS +def document_get(session=None, **filters): session = session or get_session() - if 'document_id' in filters: - # Get the document by `document_id`. - document = session.query(models.Document).get(filters['document_id']) - elif all([c in filters for c in unqiue_constraints]): - # Get the document by its unique constraints. - document = session.query(models.Document) - document = document.filter_by(**{c: filters[c] - for c in unqiue_constraints}).first() + document = session.query(models.Document)\ + .filter_by(**filters)\ + .options(sa_orm.joinedload("revision_index"))\ + .order_by(desc(models.Revision.created_at))\ + .first() - if as_dict: - return document.to_dict() if document else {} - return document + return document.to_dict() if document else {} #################### diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 0afdd4a0..9eba2402 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -25,6 +25,7 @@ from sqlalchemy.ext import declarative from sqlalchemy import ForeignKey from sqlalchemy import Integer from sqlalchemy import orm +from sqlalchemy.orm import backref, relationship from sqlalchemy import schema from sqlalchemy import String from sqlalchemy import Text @@ -116,8 +117,23 @@ class DeckhandBase(models.ModelBase, models.TimestampMixin): return schema.UniqueConstraint(*fields, name=constraint_name) +class Revision(BASE, DeckhandBase): + """Revision history for a ``Document``. + + Like a doubly linked list, each ``Revision`` will have a unique ID along + with a previous and next pointer to each ``Revision`` that comprises the + revision history for a ``Document``. + """ + __tablename__ = 'revisions' + + id = Column(String(36), primary_key=True, + default=lambda: str(uuid.uuid4())) + previous = Column(Integer, ForeignKey('revisions.id'), nullable=True) + next = Column(Integer, ForeignKey('revisions.id'), nullable=True) + + class Document(BASE, DeckhandBase): - UNIQUE_CONSTRAINTS = ('schema_version', 'kind') + UNIQUE_CONSTRAINTS = ('schema_version', 'kind', 'revision_index') __tablename__ = 'documents' __table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) @@ -125,6 +141,7 @@ class Document(BASE, DeckhandBase): default=lambda: str(uuid.uuid4())) revision_index = Column(Integer, ForeignKey('revisions.id'), nullable=False) + revision = relationship(Revision, backref=backref('revisions')) schema_version = Column(String(64), nullable=False) kind = Column(String(64), nullable=False) # NOTE: Do not define a maximum length for these JSON data below. However, @@ -134,20 +151,6 @@ class Document(BASE, DeckhandBase): data = Column(JSONEncodedDict(), nullable=False) -class Revision(BASE, DeckhandBase): - """Revision history for a ``Document``. - - Each ``Revision`` will have a unique ID along with a previous and next - pointer to each ``Revision`` that comprises the revision history for a - ``Document``. - """ - __tablename__ = 'revisions' - - id = Column(String(36), primary_key=True, - default=lambda: str(uuid.uuid4())) - previous = Column(Integer, ForeignKey('revisions.id'), nullable=True) - next = Column(Integer, ForeignKey('revisions.id'), nullable=True) - def register_models(engine): """Create database tables for all models with the given engine.""" models = [Document] From 8b79789425014dc9588ce73c9479f9f4e55cf83e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 23 Jul 2017 01:50:57 +0100 Subject: [PATCH 03/23] Update revision and document tables and add more unit tests. --- deckhand/control/base.py | 2 +- deckhand/db/sqlalchemy/api.py | 90 +++++++++++++++--------- deckhand/db/sqlalchemy/models.py | 17 +++-- deckhand/tests/unit/db/test_documents.py | 57 ++++++++++++++- 4 files changed, 124 insertions(+), 42 deletions(-) diff --git a/deckhand/control/base.py b/deckhand/control/base.py index f8fbfe90..f88a8b2b 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -72,7 +72,7 @@ class BaseResource(object): def return_error(self, resp, status_code, message="", retry=False): resp.body = json.dumps( - {'type': 'error', 'message': message, 'retry': retry}) + {'type': 'error', 'message': str(message), 'retry': retry}) resp.status = status_code diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 9c8d2260..39704218 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -121,16 +121,15 @@ def document_create(values, session=None): values['schema_version'] = values.pop('schemaVersion') session = session or get_session() - filters = copy.copy(models.Document.UNIQUE_CONSTRAINTS) - filters = [f for f in filters if f != 'revision_index'] + filters = models.Document.UNIQUE_CONSTRAINTS existing_document = document_get(**{c: values[c] for c in filters}) + created_document = {} + def _document_changed(): - other_document = copy.deepcopy(existing_document) - other_document = other_document.to_dict() # The document has changed if at least one value in ``values`` differs. for key, val in values.items(): - if val != other_document[key]: + if val != existing_document[key]: return True return False @@ -139,62 +138,87 @@ def document_create(values, session=None): with session.begin(): document.update(values) document.save(session=session) - return document + return document.to_dict() - created_document = {} if existing_document: # Only generate a new revision and entirely new document if anything # was changed. if _document_changed(): - revision_index = revision_update( - revision_index=existing_document['revision_index'])['id'] - values['revision_index'] = revision_index - created_document = _document_create().to_dict() - # TODO: indicate that now document was actually created. + created_document = _document_create() + revision_update(created_document['id'], existing_document['id']) else: - revision_index = revision_create()['id'] - values['revision_index'] = revision_index - created_document = _document_create().to_dict() + created_document = _document_create() + revision_create(created_document['id']) return created_document def document_get(session=None, **filters): session = session or get_session() - - document = session.query(models.Document)\ - .filter_by(**filters)\ - .options(sa_orm.joinedload("revision_index"))\ - .order_by(desc(models.Revision.created_at))\ - .first() - + document = session.query(models.Document).filter_by(**filters).first() return document.to_dict() if document else {} #################### -def revision_create(session=None): +def revision_create(document_id, session=None): session = session or get_session() revision = models.Revision() with session.begin(): + revision.update({'document_id': document_id}) revision.save(session=session) return revision.to_dict() -def revision_update(session=None, revision_index=None): +def revision_get(document_id, session=None): session = session or get_session() - previous_revision = session.query(models.Revision).get(revision_index) + revision = session.query(models.Revision)\ + .filter_by(document_id=document_id).first() + return revision.to_dict() - new_revision = models.Revision() + +def revision_update(document_id, child_document_id, session=None): + """Create a parent revision and update the child revision. + + The ``document_id`` references the newly created document that is a more + up-to-date revision. Create a new (parent) revision that references + ``document_id`` and whose ``child_id`` is ``child_document_id``. + + Set the ``parent_id`` for ``child_revision`` to ``document_id``. + + After this function has executed, the following relationship is true: + + parent_document <-- parent_revision + ^ / + \ (has child) + \ / + \ / + \ / + / \ + / \ + / \ + / (has parent) + v \ + child_document <-- child_revision + + :param document_id: The ID corresponding to the up-to-date document. + :param child_document_id: The ID corresponding tothe out-of-date document. + :param session: The database session. + :returns: The dictionary representation of the newly created revision. + """ + session = session or get_session() + parent_revision = models.Revision() with session.begin(): - # Create the new revision with a reference to the previous revision. - new_revision.update({'previous': revision_index}) - new_revision.save(session=session) + parent_revision.update({'document_id': document_id, + 'child_id': child_document_id}) + parent_revision.save(session=session) - # Update the previous revision with a reference to the new revision. - previous_revision.update({'next': new_revision.id}) - previous_revision.save(session=session) + child_revision = session.query(models.Revision)\ + .filter_by(document_id=child_document_id).first() + with session.begin(): + child_revision.update({'parent_id': document_id}) + child_revision.save(session=session) - return new_revision.to_dict() + return parent_revision.to_dict() diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 9eba2402..c5245c55 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -128,20 +128,20 @@ class Revision(BASE, DeckhandBase): id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) - previous = Column(Integer, ForeignKey('revisions.id'), nullable=True) - next = Column(Integer, ForeignKey('revisions.id'), nullable=True) + document_id = Column(Integer, ForeignKey('documents.id'), nullable=True) + parent_id = Column(Integer, ForeignKey('documents.id'), nullable=True) + child_id = Column(Integer, ForeignKey('documents.id'), nullable=True) + document = relationship("Document", back_populates="revision", + foreign_keys=[document_id]) class Document(BASE, DeckhandBase): - UNIQUE_CONSTRAINTS = ('schema_version', 'kind', 'revision_index') + UNIQUE_CONSTRAINTS = ('schema_version', 'kind') __tablename__ = 'documents' - __table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) + #__table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) - revision_index = Column(Integer, ForeignKey('revisions.id'), - nullable=False) - revision = relationship(Revision, backref=backref('revisions')) schema_version = Column(String(64), nullable=False) kind = Column(String(64), nullable=False) # NOTE: Do not define a maximum length for these JSON data below. However, @@ -149,6 +149,9 @@ class Document(BASE, DeckhandBase): # "metadata" is reserved, so use "doc_metadata" instead. doc_metadata = Column(JSONEncodedDict(), nullable=False) data = Column(JSONEncodedDict(), nullable=False) + revision = relationship("Revision", uselist=False, + back_populates="document", + foreign_keys="[Revision.document_id]") def register_models(engine): diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 4c72a960..d2256a27 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -13,8 +13,10 @@ # limitations under the License. import mock +import uuid import testtools +from testtools import matchers from deckhand.db.sqlalchemy import api as db_api from deckhand.tests.unit import base @@ -24,7 +26,7 @@ class DocumentFixture(object): def get_minimal_fixture(self, **kwargs): fixture = {'data': 'fake document data', - 'metadata': 'fake meta', + 'metadata': 'fake metadata', 'kind': 'FakeConfigType', 'schemaVersion': 'deckhand/v1'} fixture.update(kwargs) @@ -43,7 +45,60 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self.assertIn(key, actual) self.assertEqual(val, actual[key]) + def _validate_revision(self, revision): + expected_attrs = ('id', 'document_id', 'child_id', 'parent_id') + for attr in expected_attrs: + self.assertIn(attr, revision) + self.assertThat(revision[attr], matchers.MatchesAny( + matchers.Is(None), matchers.IsInstance(unicode))) + def test_create_document(self): fixture = DocumentFixture().get_minimal_fixture() document = db_api.document_create(fixture) self._validate_document(fixture, document) + + revision = db_api.revision_get(document['id']) + self._validate_revision(revision) + self.assertEqual(document['id'], revision['document_id']) + + def test_create_and_update_document(self): + """ + Check that the following relationship is true: + + parent_document <-- parent_revision + ^ / + \ (has child) + \ / + \ / + \ / + / \ + / \ + / \ + / (has parent) + v \ + child_document <-- child_revision + """ + fixture = DocumentFixture().get_minimal_fixture() + child_document = db_api.document_create(fixture) + + fixture['metadata'] = 'modified fake metadata' + parent_document = db_api.document_create(fixture) + self._validate_document(fixture, parent_document) + + # Validate that the new document was created. + self.assertEqual('modified fake metadata', + parent_document['doc_metadata']) + self.assertNotEqual(child_document['id'], parent_document['id']) + + # Validate that the parent document has a different revision and + # that the revisions and document links are correct. + child_revision = db_api.revision_get(child_document['id']) + parent_revision = db_api.revision_get(parent_document['id']) + for revision in (child_revision, parent_revision): + self._validate_revision(revision) + + self.assertNotEqual(child_revision['id'], parent_revision['id']) + self.assertEqual(parent_document['id'], + parent_revision['document_id']) + self.assertEqual(child_document['id'], parent_revision['child_id']) + self.assertEqual(parent_document['id'], child_revision['parent_id']) From dbc80fbfae94d600cb2f7695266b4cc73abe4de7 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 23 Jul 2017 18:04:14 +0100 Subject: [PATCH 04/23] Update documents/revisions relationship/tables. --- deckhand/db/sqlalchemy/api.py | 80 +++++++++++++----------- deckhand/db/sqlalchemy/models.py | 19 +++--- deckhand/tests/unit/db/test_documents.py | 35 +++++------ 3 files changed, 68 insertions(+), 66 deletions(-) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 39704218..f1dc69b9 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -122,7 +122,8 @@ def document_create(values, session=None): session = session or get_session() filters = models.Document.UNIQUE_CONSTRAINTS - existing_document = document_get(**{c: values[c] for c in filters}) + existing_document = document_get(**{c: values[c] for c in filters + if c != 'revision_id'}) created_document = {} @@ -144,11 +145,14 @@ def document_create(values, session=None): # Only generate a new revision and entirely new document if anything # was changed. if _document_changed(): + revision = revision_create_parent(existing_document) + values['revision_id'] = revision['id'] created_document = _document_create() - revision_update(created_document['id'], existing_document['id']) + revision_update_child(existing_document, created_document) else: + revision = revision_create() + values['revision_id'] = revision['id'] created_document = _document_create() - revision_create(created_document['id']) return created_document @@ -162,63 +166,69 @@ def document_get(session=None, **filters): #################### -def revision_create(document_id, session=None): +def revision_create(session=None): session = session or get_session() revision = models.Revision() with session.begin(): - revision.update({'document_id': document_id}) revision.save(session=session) return revision.to_dict() -def revision_get(document_id, session=None): +def revision_get(revision_id, session=None): session = session or get_session() - revision = session.query(models.Revision)\ - .filter_by(document_id=document_id).first() + revision = session.query(models.Revision).get(revision_id) return revision.to_dict() -def revision_update(document_id, child_document_id, session=None): - """Create a parent revision and update the child revision. +def revision_create_parent(child_document, session=None): + """Create a parent revision. - The ``document_id`` references the newly created document that is a more - up-to-date revision. Create a new (parent) revision that references - ``document_id`` and whose ``child_id`` is ``child_document_id``. - - Set the ``parent_id`` for ``child_revision`` to ``document_id``. + Create a new (parent) revision that references whose ``child_id`` is + the ID of ``child_document``. After this function has executed, the following relationship is true: - parent_document <-- parent_revision - ^ / - \ (has child) - \ / - \ / - \ / - / \ - / \ - / \ - / (has parent) - v \ - child_document <-- child_revision + parent_document --> parent_revision + | + (has child) + v + child_document --> child_revision - :param document_id: The ID corresponding to the up-to-date document. - :param child_document_id: The ID corresponding tothe out-of-date document. + :param child_document: The out-of-date document. :param session: The database session. :returns: The dictionary representation of the newly created revision. """ session = session or get_session() parent_revision = models.Revision() with session.begin(): - parent_revision.update({'document_id': document_id, - 'child_id': child_document_id}) + parent_revision.update({'child_id': child_document['revision_id']}) parent_revision.save(session=session) - child_revision = session.query(models.Revision)\ - .filter_by(document_id=child_document_id).first() + return parent_revision.to_dict() + + +def revision_update_child(child_document, parent_document, session=None): + """Update the child revision for an out-of-date document. + + After this function has executed, the following relationship is true: + + parent_document --> parent_revision + | ^ + (has child) (has parent) + v | + child_document --> child_revision + + :param child_document: The out-of-date document. + :param parent_document: The up-to-date document. + :param session: The database session. + :returns: The dictionary representation of the ``child_revision``. + """ + session = session or get_session() + child_revision = session.query(models.Revision).get( + child_document['revision_id']) with session.begin(): - child_revision.update({'parent_id': document_id}) + child_revision.update({'parent_id': parent_document['revision_id']}) child_revision.save(session=session) - return parent_revision.to_dict() + return child_revision.to_dict() diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index c5245c55..8ccffe06 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -128,17 +128,14 @@ class Revision(BASE, DeckhandBase): id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) - document_id = Column(Integer, ForeignKey('documents.id'), nullable=True) - parent_id = Column(Integer, ForeignKey('documents.id'), nullable=True) - child_id = Column(Integer, ForeignKey('documents.id'), nullable=True) - document = relationship("Document", back_populates="revision", - foreign_keys=[document_id]) + parent_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) + child_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) class Document(BASE, DeckhandBase): - UNIQUE_CONSTRAINTS = ('schema_version', 'kind') + UNIQUE_CONSTRAINTS = ('schema_version', 'kind', 'revision_id') __tablename__ = 'documents' - #__table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) + __table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) @@ -149,9 +146,11 @@ class Document(BASE, DeckhandBase): # "metadata" is reserved, so use "doc_metadata" instead. doc_metadata = Column(JSONEncodedDict(), nullable=False) data = Column(JSONEncodedDict(), nullable=False) - revision = relationship("Revision", uselist=False, - back_populates="document", - foreign_keys="[Revision.document_id]") + revision_id = Column(Integer, ForeignKey('revisions.id'), nullable=False) + + revision = relationship("Revision", + foreign_keys=[revision_id], + cascade="all, delete") def register_models(engine): diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index d2256a27..424b25dc 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -46,7 +46,7 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self.assertEqual(val, actual[key]) def _validate_revision(self, revision): - expected_attrs = ('id', 'document_id', 'child_id', 'parent_id') + expected_attrs = ('id', 'child_id', 'parent_id') for attr in expected_attrs: self.assertIn(attr, revision) self.assertThat(revision[attr], matchers.MatchesAny( @@ -57,26 +57,19 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): document = db_api.document_create(fixture) self._validate_document(fixture, document) - revision = db_api.revision_get(document['id']) + revision = db_api.revision_get(document['revision_id']) self._validate_revision(revision) - self.assertEqual(document['id'], revision['document_id']) + self.assertEqual(document['revision_id'], revision['id']) def test_create_and_update_document(self): """ Check that the following relationship is true: - parent_document <-- parent_revision - ^ / - \ (has child) - \ / - \ / - \ / - / \ - / \ - / \ - / (has parent) - v \ - child_document <-- child_revision + parent_document --> parent_revision + | ^ + (has child) (has parent) + v | + child_document --> child_revision """ fixture = DocumentFixture().get_minimal_fixture() child_document = db_api.document_create(fixture) @@ -92,13 +85,13 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): # Validate that the parent document has a different revision and # that the revisions and document links are correct. - child_revision = db_api.revision_get(child_document['id']) - parent_revision = db_api.revision_get(parent_document['id']) + child_revision = db_api.revision_get(child_document['revision_id']) + parent_revision = db_api.revision_get(parent_document['revision_id']) for revision in (child_revision, parent_revision): self._validate_revision(revision) self.assertNotEqual(child_revision['id'], parent_revision['id']) - self.assertEqual(parent_document['id'], - parent_revision['document_id']) - self.assertEqual(child_document['id'], parent_revision['child_id']) - self.assertEqual(parent_document['id'], child_revision['parent_id']) + self.assertEqual(parent_document['revision_id'], + parent_revision['id']) + self.assertEqual(child_document['revision_id'], child_revision['id']) + self.assertEqual(parent_document['revision_id'], parent_revision['id']) From 7f6788db89dad12a1b29abd44dacc876ee3f8c2b Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 24 Jul 2017 16:47:58 +0100 Subject: [PATCH 05/23] Update schema/db model/db api to align with design document. --- deckhand/db/sqlalchemy/api.py | 5 +- deckhand/db/sqlalchemy/models.py | 8 +- deckhand/engine/document_validation.py | 15 +- deckhand/engine/schema/v1_0/default_schema.py | 60 ++++---- deckhand/engine/secret_substitution.py | 138 ------------------ deckhand/tests/unit/db/test_documents.py | 14 +- .../unit/engine/test_document_validation.py | 3 +- .../unit/engine/test_secret_substitution.py | 123 ---------------- deckhand/tests/unit/resources/sample.yaml | 57 ++++---- 9 files changed, 79 insertions(+), 344 deletions(-) delete mode 100644 deckhand/engine/secret_substitution.py delete mode 100644 deckhand/tests/unit/engine/test_secret_substitution.py diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index f1dc69b9..da440773 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -117,8 +117,9 @@ def documents_create(documents, session=None): def document_create(values, session=None): """Create a document.""" values = values.copy() - values['doc_metadata'] = values.pop('metadata') - values['schema_version'] = values.pop('schemaVersion') + values['_metadata'] = values.pop('metadata') + print(values) + values['name'] = values['_metadata']['name'] session = session or get_session() filters = models.Document.UNIQUE_CONSTRAINTS diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 8ccffe06..88f9ab2b 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -133,18 +133,18 @@ class Revision(BASE, DeckhandBase): class Document(BASE, DeckhandBase): - UNIQUE_CONSTRAINTS = ('schema_version', 'kind', 'revision_id') + UNIQUE_CONSTRAINTS = ('schema', 'name', 'revision_id') __tablename__ = 'documents' __table_args__ = (DeckhandBase.gen_unqiue_contraint(*UNIQUE_CONSTRAINTS),) id = Column(String(36), primary_key=True, default=lambda: str(uuid.uuid4())) - schema_version = Column(String(64), nullable=False) - kind = Column(String(64), nullable=False) + schema = Column(String(64), nullable=False) + name = Column(String(64), nullable=False) # NOTE: Do not define a maximum length for these JSON data below. However, # this approach is not compatible with all database types. # "metadata" is reserved, so use "doc_metadata" instead. - doc_metadata = Column(JSONEncodedDict(), nullable=False) + _metadata = Column(JSONEncodedDict(), nullable=False) data = Column(JSONEncodedDict(), nullable=False) revision_id = Column(Integer, ForeignKey('revisions.id'), nullable=False) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index ea7947c1..8c6e1abb 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -44,9 +44,8 @@ class DocumentValidation(object): schema_versions_info = [{'version': 'v1', 'kind': 'default', 'schema': default_schema}] - def __init__(self, schema_version, kind): + def __init__(self, schema_version): self.schema_version = schema_version - self.kind = kind @property def schema(self): @@ -60,21 +59,15 @@ class DocumentValidation(object): # TODO(fm577c): Query Deckhand API to validate "src" values. - @property - def doc_name(self): - return (self.data['schemaVersion'] + self.data['kind'] + - self.data['metadata']['name']) - def _validate_with_schema(self): # Validate the document using the schema defined by the document's # `schemaVersion` and `kind`. try: - schema_version = self.data['schemaVersion'].split('/')[-1] - doc_kind = self.data['kind'] - doc_schema_version = self.SchemaVersion(schema_version, doc_kind) + schema_version = self.data['schema'].split('/')[-1] + doc_schema_version = self.SchemaVersion(schema_version) except (AttributeError, IndexError, KeyError) as e: raise errors.InvalidFormat( - 'The provided schemaVersion is invalid or missing. Exception: ' + 'The provided schema is invalid or missing. Exception: ' '%s.' % e) try: jsonschema.validate(self.data, doc_schema_version.schema) diff --git a/deckhand/engine/schema/v1_0/default_schema.py b/deckhand/engine/schema/v1_0/default_schema.py index 7d888223..7c2e13fe 100644 --- a/deckhand/engine/schema/v1_0/default_schema.py +++ b/deckhand/engine/schema/v1_0/default_schema.py @@ -18,8 +18,7 @@ substitution_schema = { 'dest': { 'type': 'object', 'properties': { - 'path': {'type': 'string'}, - 'replacePattern': {'type': 'string'} + 'path': {'type': 'string'} }, 'additionalProperties': False, # 'replacePattern' is not required. @@ -28,12 +27,12 @@ substitution_schema = { 'src': { 'type': 'object', 'properties': { - 'kind': {'type': 'string'}, + 'schema': {'type': 'string'}, 'name': {'type': 'string'}, 'path': {'type': 'string'} }, 'additionalProperties': False, - 'required': ['kind', 'name', 'path'] + 'required': ['schema', 'name', 'path'] } }, 'additionalProperties': False, @@ -43,45 +42,46 @@ substitution_schema = { schema = { 'type': 'object', 'properties': { - 'schemaVersion': { + 'schema': { 'type': 'string', - 'pattern': '^([A-Za-z]+\/v[0-9]{1})$' + 'pattern': '^(.*\/v[0-9]{1})$' }, - # TODO: The kind should be an enum. - 'kind': {'type': 'string'}, 'metadata': { 'type': 'object', 'properties': { - 'metadataVersion': { + 'schema': { 'type': 'string', - 'pattern': '^([A-Za-z]+\/v[0-9]{1})$' + 'pattern': '^(.*/v[0-9]{1})$' }, 'name': {'type': 'string'}, + 'storagePolicy': {'type': 'string'}, 'labels': { - 'type': 'object', - 'properties': { - 'component': {'type': 'string'}, - 'hostname': {'type': 'string'} - }, - 'additionalProperties': False, - 'required': ['component', 'hostname'] + 'type': 'object' }, - 'layerDefinition': { + 'layeringDefinition': { 'type': 'object', 'properties': { - 'layer': {'enum': ['global', 'region', 'site']}, + 'layer': {'type': 'string'}, 'abstract': {'type': 'boolean'}, - 'childSelector': { - 'type': 'object', - 'properties': { - 'label': {'type': 'string'} - }, - 'additionalProperties': False, - 'required': ['label'] + 'parentSelector': { + 'type': 'object' + }, + 'actions': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'method': {'enum': ['merge', 'delete', + 'replace']}, + 'path': {'type': 'string'} + }, + 'additionalProperties': False, + 'required': ['method', 'path'] + } } }, 'additionalProperties': False, - 'required': ['layer', 'abstract', 'childSelector'] + 'required': ['layer', 'abstract', 'parentSelector'] }, 'substitutions': { 'type': 'array', @@ -89,13 +89,13 @@ schema = { } }, 'additionalProperties': False, - 'required': ['metadataVersion', 'name', 'labels', - 'layerDefinition', 'substitutions'] + 'required': ['schema', 'name', 'storagePolicy', 'labels', + 'layeringDefinition', 'substitutions'] }, 'data': { 'type': 'object' } }, 'additionalProperties': False, - 'required': ['schemaVersion', 'kind', 'metadata', 'data'] + 'required': ['schema', 'metadata', 'data'] } diff --git a/deckhand/engine/secret_substitution.py b/deckhand/engine/secret_substitution.py deleted file mode 100644 index 38229547..00000000 --- a/deckhand/engine/secret_substitution.py +++ /dev/null @@ -1,138 +0,0 @@ -# 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 yaml - -import jsonschema - -from deckhand.engine.schema.v1_0 import default_schema -from deckhand import errors - - -class SecretSubstitution(object): - """Class for secret substitution logic for YAML files. - - This class is responsible for parsing, validating and retrieving secret - values for values stored in the YAML file. Afterward, secret values will be - substituted or "forward-repalced" into the YAML file. The end result is a - YAML file containing all necessary secrets to be handed off to other - services. - - :param data: YAML data that requires secrets to be validated, merged and - consolidated. - """ - - def __init__(self, data): - try: - self.data = yaml.safe_load(data) - except yaml.YAMLError: - raise errors.InvalidFormat( - 'The provided YAML file cannot be parsed.') - - self.pre_validate_data() - - class SchemaVersion(object): - """Class for retrieving correct schema for pre-validation on YAML. - - Retrieves the schema that corresponds to "apiVersion" in the YAML - data. This schema is responsible for performing pre-validation on - YAML data. - """ - - # TODO: Update kind according to requirements. - schema_versions_info = [{'version': 'v1', 'kind': 'default', - 'schema': default_schema}] - - def __init__(self, schema_version, kind): - self.schema_version = schema_version - self.kind = kind - - @property - def schema(self): - # TODO: return schema based on version and kind. - return [v['schema'] for v in self.schema_versions_info - if v['version'] == self.schema_version][0].schema - - def pre_validate_data(self): - """Pre-validate that the YAML file is correctly formatted.""" - self._validate_with_schema() - - # Validate that each "dest" field exists in the YAML data. - # FIXME(fm577c): Dest fields will be injected if not present - the - # validation below needs to be updated or removed. - substitutions = self.data['metadata']['substitutions'] - destinations = [s['dest'] for s in substitutions] - sub_data = self.data['data'] - - for dest in destinations: - result, missing_attr = self._multi_getattr(dest['path'], sub_data) - if not result: - raise errors.InvalidFormat( - 'The attribute "%s" included in the "dest" field "%s" is ' - 'missing from the YAML data: "%s".' % ( - missing_attr, dest, sub_data)) - - # TODO(fm577c): Query Deckhand API to validate "src" values. - - def _validate_with_schema(self): - # Validate the document using the schema defined by the document's - # `schemaVersion` and `kind`. - try: - schema_version = self.data['schemaVersion'].split('/')[-1] - doc_kind = self.data['kind'] - doc_schema_version = self.SchemaVersion(schema_version, doc_kind) - except (AttributeError, IndexError, KeyError) as e: - raise errors.InvalidFormat( - 'The provided schemaVersion is invalid or missing. Exception: ' - '%s.' % e) - try: - jsonschema.validate(self.data, doc_schema_version.schema) - 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/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 424b25dc..97a6777a 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -26,9 +26,8 @@ class DocumentFixture(object): def get_minimal_fixture(self, **kwargs): fixture = {'data': 'fake document data', - 'metadata': 'fake metadata', - 'kind': 'FakeConfigType', - 'schemaVersion': 'deckhand/v1'} + 'metadata': {'name': 'fake metadata'}, + 'schema': 'deckhand/v1'} fixture.update(kwargs) return fixture @@ -36,8 +35,7 @@ class DocumentFixture(object): class TestDocumentsApi(base.DeckhandWithDBTestCase): def _validate_document(self, expected, actual): - expected['doc_metadata'] = expected.pop('metadata') - expected['schema_version'] = expected.pop('schemaVersion') + expected['_metadata'] = expected.pop('metadata') # TODO: Validate "status" fields, like created_at. self.assertIsInstance(actual, dict) @@ -74,13 +72,13 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): fixture = DocumentFixture().get_minimal_fixture() child_document = db_api.document_create(fixture) - fixture['metadata'] = 'modified fake metadata' + fixture['metadata'] = {'name': 'modified fake metadata'} parent_document = db_api.document_create(fixture) self._validate_document(fixture, parent_document) # Validate that the new document was created. - self.assertEqual('modified fake metadata', - parent_document['doc_metadata']) + self.assertEqual({'name': 'modified fake metadata'}, + parent_document['_metadata']) self.assertNotEqual(child_document['id'], parent_document['id']) # Validate that the parent document has a different revision and diff --git a/deckhand/tests/unit/engine/test_document_validation.py b/deckhand/tests/unit/engine/test_document_validation.py index 0f0e5fb1..835b531e 100644 --- a/deckhand/tests/unit/engine/test_document_validation.py +++ b/deckhand/tests/unit/engine/test_document_validation.py @@ -81,8 +81,7 @@ class TestDocumentValidation(testtools.TestCase): invalid_data = [ (self._corrupt_data('data'), 'data'), (self._corrupt_data('metadata'), 'metadata'), - (self._corrupt_data('metadata.metadataVersion'), - 'metadataVersion'), + (self._corrupt_data('metadata.schema'), 'schema'), (self._corrupt_data('metadata.name'), 'name'), (self._corrupt_data('metadata.substitutions'), 'substitutions'), (self._corrupt_data('metadata.substitutions.0.dest'), 'dest'), diff --git a/deckhand/tests/unit/engine/test_secret_substitution.py b/deckhand/tests/unit/engine/test_secret_substitution.py deleted file mode 100644 index df017948..00000000 --- a/deckhand/tests/unit/engine/test_secret_substitution.py +++ /dev/null @@ -1,123 +0,0 @@ -# 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 copy -import os -import testtools -import yaml - -import six - -from deckhand.engine import secret_substitution -from deckhand import errors - - -class TestSecretSubtitution(testtools.TestCase): - - def setUp(self): - super(TestSecretSubtitution, self).setUp() - dir_path = os.path.dirname(os.path.realpath(__file__)) - test_yaml_path = os.path.abspath(os.path.join( - dir_path, os.pardir, 'resources', 'sample.yaml')) - - with open(test_yaml_path, 'r') as yaml_file: - yaml_data = yaml_file.read() - self.data = yaml.safe_load(yaml_data) - - def _corrupt_data(self, key, data=None): - """Corrupt test data to check that pre-validation works. - - Corrupt data by removing a key from the document. Each key must - correspond to a value that is a dictionary. - - :param key: The document key to be removed. The key can have the - following formats: - * 'data' => document.pop('data') - * 'metadata.name' => document['metadata'].pop('name') - * 'metadata.substitutions.0.dest' => - document['metadata']['substitutions'][0].pop('dest') - :returns: Corrupted YAML data. - """ - if data is None: - data = self.data - corrupted_data = copy.deepcopy(data) - - if '.' in key: - _corrupted_data = corrupted_data - nested_keys = key.split('.') - for nested_key in nested_keys: - if nested_key == nested_keys[-1]: - break - if nested_key.isdigit(): - _corrupted_data = _corrupted_data[int(nested_key)] - else: - _corrupted_data = _corrupted_data[nested_key] - _corrupted_data.pop(nested_keys[-1]) - else: - corrupted_data.pop(key) - - return self._format_data(corrupted_data) - - def _format_data(self, data=None): - """Re-formats dict data as YAML to pass to ``SecretSubstitution``.""" - if data is None: - data = self.data - return yaml.safe_dump(data) - - def test_initialization(self): - sub = secret_substitution.SecretSubstitution(self._format_data()) - self.assertIsInstance(sub, secret_substitution.SecretSubstitution) - - def test_initialization_missing_sections(self): - expected_err = ("The provided YAML file is invalid. Exception: '%s' " - "is a required property.") - invalid_data = [ - (self._corrupt_data('data'), 'data'), - (self._corrupt_data('metadata'), 'metadata'), - (self._corrupt_data('metadata.metadataVersion'), 'metadataVersion'), - (self._corrupt_data('metadata.name'), 'name'), - (self._corrupt_data('metadata.substitutions'), 'substitutions'), - (self._corrupt_data('metadata.substitutions.0.dest'), 'dest'), - (self._corrupt_data('metadata.substitutions.0.src'), 'src') - ] - - for invalid_entry, missing_key in invalid_data: - with six.assertRaisesRegex(self, errors.InvalidFormat, - expected_err % missing_key): - secret_substitution.SecretSubstitution(invalid_entry) - - def test_initialization_bad_substitutions(self): - expected_err = ('The attribute "%s" included in the "dest" field "%s" ' - 'is missing from the YAML data') - invalid_data = [] - - data = copy.deepcopy(self.data) - data['metadata']['substitutions'][0]['dest'] = {'path': 'foo'} - invalid_data.append(self._format_data(data)) - - data = copy.deepcopy(self.data) - data['metadata']['substitutions'][0]['dest'] = { - 'path': 'tls_endpoint.bar'} - invalid_data.append(self._format_data(data)) - - def _test(invalid_entry, field, dest): - _expected_err = expected_err % (field, dest) - with six.assertRaisesRegex(self, errors.InvalidFormat, - _expected_err): - secret_substitution.SecretSubstitution(invalid_entry) - - # Verify that invalid body dest reference is invalid. - _test(invalid_data[0], "foo", {'path': 'foo'}) - # Verify that nested invalid body dest reference is invalid. - _test(invalid_data[1], "bar", {'path': 'tls_endpoint.bar'}) diff --git a/deckhand/tests/unit/resources/sample.yaml b/deckhand/tests/unit/resources/sample.yaml index ec083907..d1e83e0d 100644 --- a/deckhand/tests/unit/resources/sample.yaml +++ b/deckhand/tests/unit/resources/sample.yaml @@ -1,33 +1,38 @@ -# Sample YAML file for testing forward replacement. --- -schemaVersion: promenade/v1 -kind: SomeConfigType +schema: some-service/ResourceType/v1 metadata: - metadataVersion: deckhand/v1 - name: a-unique-config-name-12345 + schema: metadata/Document/v1 + name: unique-name-given-schema + storagePolicy: cleartext labels: - component: apiserver - hostname: server0 - layerDefinition: - layer: global - abstract: True - childSelector: - label: value + genesis: enabled + master: enabled + layeringDefinition: + abstract: true + layer: region + parentSelector: + required_key_a: required_label_a + required_key_b: required_label_b + actions: + - method: merge + path: .path.to.merge.into.parent + - method: delete + path: .path.to.delete substitutions: - dest: - path: .tls_endpoint.certificate - replacePattern: 'test.pattern' + path: .substitution.target src: - kind: Certificate - name: some-certificate-asdf-1234 - path: .cert - - dest: - path: .tls_endpoint.key - src: - kind: CertificateKey - name: some-certificate-asdf-1234 - path: .key + schema: another-service/SourceType/v1 + name: name-of-source-document + path: .source.path data: - tls_endpoint: - certificate: '.cert' - key: deckhand/v1:some-certificate-asdf-1234 + path: + to: + merge: + into: + parent: + foo: bar + ignored: # Will not be part of the resultant document after layering. + data: here + substitution: + target: null # Paths do not need to exist to be specified as substitution destinations. From cb29a3f0ba576054dfe35a997d0cb6b7d26420a8 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 24 Jul 2017 16:55:17 +0100 Subject: [PATCH 06/23] Update schema validation to be internal validation. --- deckhand/db/sqlalchemy/api.py | 1 - deckhand/engine/document_validation.py | 19 +++++++++++++------ .../schema/v1_0/default_policy_validation.py | 13 +++++++++++++ ...schema.py => default_schema_validation.py} | 0 4 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 deckhand/engine/schema/v1_0/default_policy_validation.py rename deckhand/engine/schema/v1_0/{default_schema.py => default_schema_validation.py} (100%) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index da440773..ce9bc3e4 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -118,7 +118,6 @@ def document_create(values, session=None): """Create a document.""" values = values.copy() values['_metadata'] = values.pop('metadata') - print(values) values['name'] = values['_metadata']['name'] session = session or get_session() diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 8c6e1abb..d97e38bd 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -14,7 +14,8 @@ import jsonschema -from deckhand.engine.schema.v1_0 import default_schema +from deckhand.engine.schema.v1_0 import default_policy_validation +from deckhand.engine.schema.v1_0 import default_schema_validation from deckhand import errors @@ -38,11 +39,17 @@ class DocumentValidation(object): Retrieves the schema that corresponds to "apiVersion" in the YAML data. This schema is responsible for performing pre-validation on YAML data. - """ - # TODO: Update kind according to requirements. - schema_versions_info = [{'version': 'v1', 'kind': 'default', - 'schema': default_schema}] + The built-in validation schemas that are always executed include: + + - `deckhand-document-schema-validation` + - `deckhand-policy-validation` + """ + internal_validations = [ + {'version': 'v1', 'fqn': 'deckhand-document-schema-validation', + 'schema': default_schema_validation}, + {'version': 'v1', 'fqn': 'deckhand-policy-validation', + 'schema': default_policy_validation}] def __init__(self, schema_version): self.schema_version = schema_version @@ -50,7 +57,7 @@ class DocumentValidation(object): @property def schema(self): # TODO: return schema based on version and kind. - return [v['schema'] for v in self.schema_versions_info + return [v['schema'] for v in self.internal_validations if v['version'] == self.schema_version][0].schema def pre_validate_data(self): diff --git a/deckhand/engine/schema/v1_0/default_policy_validation.py b/deckhand/engine/schema/v1_0/default_policy_validation.py new file mode 100644 index 00000000..f10bbbf6 --- /dev/null +++ b/deckhand/engine/schema/v1_0/default_policy_validation.py @@ -0,0 +1,13 @@ +# 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. diff --git a/deckhand/engine/schema/v1_0/default_schema.py b/deckhand/engine/schema/v1_0/default_schema_validation.py similarity index 100% rename from deckhand/engine/schema/v1_0/default_schema.py rename to deckhand/engine/schema/v1_0/default_schema_validation.py From a0df0c459de614bdfa2f0df8a95790c9172e1333 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 24 Jul 2017 17:16:20 +0100 Subject: [PATCH 07/23] Skip validation for abstract documents & add unit tests. --- deckhand/engine/document_validation.py | 30 ++++++++++++++----- deckhand/errors.py | 2 +- .../unit/engine/test_document_validation.py | 28 ++++++++++++++++- deckhand/tests/unit/resources/sample.yaml | 2 +- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index d97e38bd..c9f24961 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -13,17 +13,20 @@ # limitations under the License. import jsonschema +from oslo_log import log as logging +import six from deckhand.engine.schema.v1_0 import default_policy_validation from deckhand.engine.schema.v1_0 import default_schema_validation from deckhand import errors +LOG = logging.getLogger(__name__) + class DocumentValidation(object): """Class for document validation logic for YAML files. - This class is responsible for parsing, validating and retrieving secret - values for values stored in the YAML file. + This class is responsible for performing built-in validations on Documents. :param data: YAML data that requires secrets to be validated, merged and consolidated. @@ -45,6 +48,8 @@ class DocumentValidation(object): - `deckhand-document-schema-validation` - `deckhand-policy-validation` """ + + # TODO: Use the correct validation based on the Document's schema. internal_validations = [ {'version': 'v1', 'fqn': 'deckhand-document-schema-validation', 'schema': default_schema_validation}, @@ -56,7 +61,7 @@ class DocumentValidation(object): @property def schema(self): - # TODO: return schema based on version and kind. + # TODO: return schema based on Document's schema. return [v['schema'] for v in self.internal_validations if v['version'] == self.schema_version][0].schema @@ -64,11 +69,22 @@ class DocumentValidation(object): """Pre-validate that the YAML file is correctly formatted.""" self._validate_with_schema() - # TODO(fm577c): Query Deckhand API to validate "src" values. - def _validate_with_schema(self): - # Validate the document using the schema defined by the document's - # `schemaVersion` and `kind`. + # Validate the document using the document's ``schema``. Only validate + # concrete documents. + try: + abstract = self.data['metadata']['layeringDefinition'][ + 'abstract'] + is_abstract = six.text_type(abstract).lower() == 'true' + except KeyError as e: + raise errors.InvalidFormat( + "Could not find 'abstract' property from document.") + + if is_abstract: + LOG.info( + "Skipping validation for the document because it is abstract") + return + try: schema_version = self.data['schema'].split('/')[-1] doc_schema_version = self.SchemaVersion(schema_version) diff --git a/deckhand/errors.py b/deckhand/errors.py index 9ef701f8..f79341d3 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -14,7 +14,7 @@ class DeckhandException(Exception): - """Base Nova Exception + """Base Deckhand Exception To correctly use this class, inherit from it and define a 'msg_fmt' property. That msg_fmt will get printf'd with the keyword arguments provided to the constructor. diff --git a/deckhand/tests/unit/engine/test_document_validation.py b/deckhand/tests/unit/engine/test_document_validation.py index 835b531e..eeaee958 100644 --- a/deckhand/tests/unit/engine/test_document_validation.py +++ b/deckhand/tests/unit/engine/test_document_validation.py @@ -17,6 +17,7 @@ import os import testtools import yaml +import mock import six from deckhand.engine import document_validation @@ -80,7 +81,6 @@ class TestDocumentValidation(testtools.TestCase): "is a required property.") invalid_data = [ (self._corrupt_data('data'), 'data'), - (self._corrupt_data('metadata'), 'metadata'), (self._corrupt_data('metadata.schema'), 'schema'), (self._corrupt_data('metadata.name'), 'name'), (self._corrupt_data('metadata.substitutions'), 'substitutions'), @@ -92,3 +92,29 @@ class TestDocumentValidation(testtools.TestCase): with six.assertRaisesRegex(self, errors.InvalidFormat, expected_err % missing_key): document_validation.DocumentValidation(invalid_entry) + + def test_initialization_missing_abstract_section(self): + expected_err = ("Could not find 'abstract' property from document.") + + invalid_data = [ + self._corrupt_data('metadata'), + self._corrupt_data('metadata.layeringDefinition'), + self._corrupt_data('metadata.layeringDefinition.abstract'), + ] + + for invalid_entry in invalid_data: + with six.assertRaisesRegex(self, errors.InvalidFormat, + expected_err): + document_validation.DocumentValidation(invalid_entry) + + @mock.patch.object(document_validation, 'LOG', autospec=True) + def test_initialization_with_abstract_document(self, mock_log): + abstract_data = copy.deepcopy(self.data) + + for true_val in (True, 'true', 'True'): + abstract_data['metadata']['layeringDefinition']['abstract'] = True + + document_validation.DocumentValidation(abstract_data) + mock_log.info.assert_called_once_with( + "Skipping validation for the document because it is abstract") + mock_log.info.reset_mock() diff --git a/deckhand/tests/unit/resources/sample.yaml b/deckhand/tests/unit/resources/sample.yaml index d1e83e0d..a5ebaa6a 100644 --- a/deckhand/tests/unit/resources/sample.yaml +++ b/deckhand/tests/unit/resources/sample.yaml @@ -8,7 +8,7 @@ metadata: genesis: enabled master: enabled layeringDefinition: - abstract: true + abstract: false layer: region parentSelector: required_key_a: required_label_a From adca9575b69d5b68f5a16e9e61ebbab6d0d211d8 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 29 Jul 2017 21:24:53 +0100 Subject: [PATCH 08/23] More tests for revisions-api. Fix minor bugs. --- deckhand/control/base.py | 6 +- deckhand/control/documents.py | 13 -- deckhand/db/sqlalchemy/api.py | 9 +- deckhand/db/sqlalchemy/models.py | 3 + deckhand/engine/document_validation.py | 1 + deckhand/tests/unit/db/test_documents.py | 207 +++++++++++++++++------ 6 files changed, 172 insertions(+), 67 deletions(-) diff --git a/deckhand/control/base.py b/deckhand/control/base.py index f88a8b2b..f885d463 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -17,6 +17,7 @@ import uuid import falcon from falcon import request +import six from deckhand import errors @@ -72,7 +73,8 @@ class BaseResource(object): def return_error(self, resp, status_code, message="", retry=False): resp.body = json.dumps( - {'type': 'error', 'message': str(message), 'retry': retry}) + {'type': 'error', 'message': six.text_type(message), + 'retry': retry}) resp.status = status_code @@ -80,7 +82,7 @@ class DeckhandRequestContext(object): def __init__(self): self.user = None - self.roles = ['anyone'] + self.roles = ['*'] self.request_id = str(uuid.uuid4()) def set_user(self, user): diff --git a/deckhand/control/documents.py b/deckhand/control/documents.py index cede8094..c4669e77 100644 --- a/deckhand/control/documents.py +++ b/deckhand/control/documents.py @@ -32,16 +32,6 @@ LOG = logging.getLogger(__name__) class DocumentsResource(api_base.BaseResource): """API resource for realizing CRUD endpoints for Documents.""" - def __init__(self, **kwargs): - super(DocumentsResource, self).__init__(**kwargs) - self.authorized_roles = ['user'] - - def on_get(self, req, resp): - pass - - def on_head(self, req, resp): - pass - def on_post(self, req, resp): """Create a document. Accepts YAML data only.""" if req.content_type != 'application/x-yaml': @@ -73,6 +63,3 @@ class DocumentsResource(api_base.BaseResource): resp.status = falcon.HTTP_201 resp.body = json.dumps(created_documents) - - def _check_document_exists(self): - pass diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index ce9bc3e4..c071cae2 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -122,8 +122,13 @@ def document_create(values, session=None): session = session or get_session() filters = models.Document.UNIQUE_CONSTRAINTS - existing_document = document_get(**{c: values[c] for c in filters - if c != 'revision_id'}) + try: + existing_document = document_get(**{c: values[c] for c in filters + if c != 'revision_id'}) + except db_exception.DBError: + # Ignore bad data at this point. Allow creation to bubble up the error + # related to bad data. + existing_document = None created_document = {} diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 88f9ab2b..6b97b736 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -103,6 +103,9 @@ class DeckhandBase(models.ModelBase, models.TimestampMixin): # CircularReference. d.pop("_sa_instance_state") + if 'deleted_at' not in d: + d.setdefault('deleted_at', None) + for k in ["created_at", "updated_at", "deleted_at", "deleted"]: if k in d and d[k]: d[k] = d[k].isoformat() diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index c9f24961..2d9eed68 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -80,6 +80,7 @@ class DocumentValidation(object): raise errors.InvalidFormat( "Could not find 'abstract' property from document.") + # TODO: This should be done inside a different module. if is_abstract: LOG.info( "Skipping validation for the document because it is abstract") diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 97a6777a..c3d8842e 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -24,7 +24,12 @@ from deckhand.tests.unit import base class DocumentFixture(object): - def get_minimal_fixture(self, **kwargs): + EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted", + "id", "schema", "name", "_metadata", "data", + "revision_id") + + @staticmethod + def get_minimal_fixture(**kwargs): fixture = {'data': 'fake document data', 'metadata': {'name': 'fake metadata'}, 'schema': 'deckhand/v1'} @@ -34,62 +39,164 @@ class DocumentFixture(object): class TestDocumentsApi(base.DeckhandWithDBTestCase): - def _validate_document(self, expected, actual): - expected['_metadata'] = expected.pop('metadata') + def _create_document(self, payload): + doc = db_api.document_create(payload) + self._validate_document(expected=payload, actual=doc) + return doc - # TODO: Validate "status" fields, like created_at. - self.assertIsInstance(actual, dict) - for key, val in expected.items(): - self.assertIn(key, actual) - self.assertEqual(val, actual[key]) + def _get_document(self, **fields): + doc = db_api.document_get(**fields) + self._validate_document(actual=doc) + return doc - def _validate_revision(self, revision): - expected_attrs = ('id', 'child_id', 'parent_id') - for attr in expected_attrs: - self.assertIn(attr, revision) - self.assertThat(revision[attr], matchers.MatchesAny( - matchers.Is(None), matchers.IsInstance(unicode))) + def _get_revision(self, revision_id): + revision = db_api.revision_get(revision_id) + self._validate_revision(revision) + return revision - def test_create_document(self): - fixture = DocumentFixture().get_minimal_fixture() - document = db_api.document_create(fixture) - self._validate_document(fixture, document) + def _validate_document(self, actual, expected=None, is_deleted=False): + # Validate that the document has all expected fields and is a dict. + expected_fields = list(DocumentFixture.EXPECTED_FIELDS) + if not is_deleted: + expected_fields.remove('deleted_at') - revision = db_api.revision_get(document['revision_id']) - self._validate_revision(revision) - self.assertEqual(document['revision_id'], revision['id']) + self.assertIsInstance(actual, dict) + for field in expected_fields: + self.assertIn(field, actual) - def test_create_and_update_document(self): - """ - Check that the following relationship is true: + # ``_metadata`` is used in the DB schema as ``metadata`` is reserved. + actual['metadata'] = actual.pop('_metadata') - parent_document --> parent_revision - | ^ - (has child) (has parent) - v | - child_document --> child_revision - """ - fixture = DocumentFixture().get_minimal_fixture() - child_document = db_api.document_create(fixture) + if expected: + # Validate that the expected values are equivalent to actual + # values. + for key, val in expected.items(): + self.assertEqual(val, actual[key]) - fixture['metadata'] = {'name': 'modified fake metadata'} - parent_document = db_api.document_create(fixture) - self._validate_document(fixture, parent_document) + def _validate_revision(self, revision): + expected_attrs = ('id', 'child_id', 'parent_id') + for attr in expected_attrs: + self.assertIn(attr, revision) + self.assertThat(revision[attr], matchers.MatchesAny( + matchers.Is(None), matchers.IsInstance(unicode))) - # Validate that the new document was created. - self.assertEqual({'name': 'modified fake metadata'}, - parent_document['_metadata']) - self.assertNotEqual(child_document['id'], parent_document['id']) + def _validate_revision_connections(self, parent_document, parent_revision, + child_document, child_revision, + parent_child_connected=True): + self.assertNotEqual(child_revision['id'], parent_revision['id']) + self.assertEqual(parent_document['revision_id'], parent_revision['id']) + self.assertEqual(child_document['revision_id'], child_revision['id']) - # Validate that the parent document has a different revision and - # that the revisions and document links are correct. - child_revision = db_api.revision_get(child_document['revision_id']) - parent_revision = db_api.revision_get(parent_document['revision_id']) - for revision in (child_revision, parent_revision): - self._validate_revision(revision) + # Validate that the revisions are distinct and connected together. + if parent_child_connected: + self.assertEqual(parent_revision['child_id'], child_revision['id']) + self.assertEqual( + child_revision['parent_id'], parent_revision['id']) + # Validate that the revisions are distinct but unconnected. + else: + self.assertIsNone(parent_revision['child_id']) + self.assertIsNone(child_revision['parent_id']) - self.assertNotEqual(child_revision['id'], parent_revision['id']) - self.assertEqual(parent_document['revision_id'], - parent_revision['id']) - self.assertEqual(child_document['revision_id'], child_revision['id']) - self.assertEqual(parent_document['revision_id'], parent_revision['id']) + def test_create_and_get_document(self): + payload = DocumentFixture.get_minimal_fixture() + document = self._create_document(payload) + retrieved_document = self._get_document(id=document['id']) + self.assertEqual(document, retrieved_document) + + def test_create_document_and_get_revision(self): + payload = DocumentFixture.get_minimal_fixture() + document = self._create_document(payload) + + revision = self._get_revision(document['revision_id']) + self._validate_revision(revision) + self.assertEqual(document['revision_id'], revision['id']) + + def test_create_and_update_document(self): + """ + Check that the following relationship is true: + + parent_document --> parent_revision + | ^ + (has child) (has parent) + v | + child_document --> child_revision + """ + child_payload = DocumentFixture.get_minimal_fixture() + child_document = self._create_document(child_payload) + + parent_payload = DocumentFixture.get_minimal_fixture() + parent_payload['data'] = 'fake updated document data' + parent_document = self._create_document(parent_payload) + + # Validate that the new document was created. + self.assertEqual('fake updated document data', parent_document['data']) + self.assertNotEqual(child_document['id'], parent_document['id']) + + # Validate that the parent document has a different revision and + # that the revisions and document links are correct. + child_revision = self._get_revision(child_document['revision_id']) + parent_revision = self._get_revision(parent_document['revision_id']) + + self._validate_revision_connections( + parent_document, parent_revision, child_document, child_revision) + + def test_create_and_update_document_schema(self): + """ + Check that the following relationship is true: + + parent_document --> parent_revision + child_document --> child_revision + + "schema" is unique so changing it results in a new document being + created. + """ + child_payload = DocumentFixture.get_minimal_fixture() + child_document = self._create_document(child_payload) + + parent_payload = DocumentFixture.get_minimal_fixture() + parent_payload['schema'] = 'deckhand/v2' + parent_document = self._create_document(parent_payload) + + # Validate that the new document was created. + self.assertEqual('deckhand/v2', parent_document['schema']) + self.assertNotEqual(child_document['id'], parent_document['id']) + + # Validate that the parent document has a different revision and + # that the revisions and document links are correct. + child_revision = self._get_revision(child_document['revision_id']) + parent_revision = self._get_revision(parent_document['revision_id']) + + self._validate_revision_connections( + parent_document, parent_revision, child_document, child_revision, + False) + + def test_create_and_update_document_metadata_name(self): + """ + Check that the following relationship is true: + + parent_document --> parent_revision + child_document --> child_revision + + "metadata.name" is unique so changing it results in a new document + being created. + """ + child_payload = DocumentFixture.get_minimal_fixture() + child_document = self._create_document(child_payload) + + parent_payload = DocumentFixture.get_minimal_fixture() + parent_payload['metadata'] = {'name': 'fake updated metadata'} + parent_document = self._create_document(parent_payload) + + # Validate that the new document was created. + self.assertEqual({'name': 'fake updated metadata'}, + parent_document['metadata']) + self.assertNotEqual(child_document['id'], parent_document['id']) + + # Validate that the parent document has a different revision and + # that the revisions and document links are correct. + child_revision = self._get_revision(child_document['revision_id']) + parent_revision = self._get_revision(parent_document['revision_id']) + + self._validate_revision_connections( + parent_document, parent_revision, child_document, child_revision, + False) From b44392bcb44a1e6d90cdc8634af5faf310974f06 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 29 Jul 2017 22:32:08 +0100 Subject: [PATCH 09/23] Add Revision resource. --- deckhand/control/api.py | 2 ++ deckhand/control/revisions.py | 45 ++++++++++++++++++++++++ deckhand/db/sqlalchemy/api.py | 9 ++--- deckhand/db/sqlalchemy/models.py | 21 +++++++++-- deckhand/tests/unit/db/test_documents.py | 43 +++++++++++++++------- 5 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 deckhand/control/revisions.py diff --git a/deckhand/control/api.py b/deckhand/control/api.py index e18939e8..5bb28e18 100644 --- a/deckhand/control/api.py +++ b/deckhand/control/api.py @@ -21,6 +21,7 @@ from oslo_log import log as logging from deckhand.conf import config from deckhand.control import base as api_base from deckhand.control import documents +from deckhand.control import revisions from deckhand.control import secrets from deckhand.db.sqlalchemy import api as db_api @@ -68,6 +69,7 @@ def start_api(state_manager=None): v1_0_routes = [ ('documents', documents.DocumentsResource()), + ('revisions/{revision_id}/documents', revisions.RevisionsResource()), ('secrets', secrets.SecretsResource()) ] diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py new file mode 100644 index 00000000..a182c63d --- /dev/null +++ b/deckhand/control/revisions.py @@ -0,0 +1,45 @@ +# 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 copy +import yaml + +import falcon + +from oslo_db import exception as db_exc +from oslo_log import log as logging +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 + +LOG = logging.getLogger(__name__) + + +class RevisionsResource(api_base.BaseResource): + """API resource for realizing CRUD endpoints for Document Revisions.""" + + def on_get(self, req, resp, revision_id): + """Returns all documents for a `revision_id`. + + Returns a multi-document YAML response containing all the documents + matching the filters specified via query string parameters. Returned + 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'] diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index c071cae2..8e52c0ff 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -123,8 +123,9 @@ def document_create(values, session=None): filters = models.Document.UNIQUE_CONSTRAINTS try: - existing_document = document_get(**{c: values[c] for c in filters - if c != 'revision_id'}) + existing_document = document_get( + raw_dict=True, + **{c: values[c] for c in filters if c != 'revision_id'}) except db_exception.DBError: # Ignore bad data at this point. Allow creation to bubble up the error # related to bad data. @@ -162,10 +163,10 @@ def document_create(values, session=None): return created_document -def document_get(session=None, **filters): +def document_get(session=None, raw_dict=False, **filters): session = session or get_session() document = session.query(models.Document).filter_by(**filters).first() - return document.to_dict() if document else {} + return document.to_dict(raw_dict=raw_dict) if document else {} #################### diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 6b97b736..7c938911 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -134,6 +134,13 @@ class Revision(BASE, DeckhandBase): parent_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) child_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) + documents = relationship("Document") + + def to_dict(self): + d = super(Revision, self).to_dict() + d['documents'] = [doc.to_dict() for doc in self.documents] + return d + class Document(BASE, DeckhandBase): UNIQUE_CONSTRAINTS = ('schema', 'name', 'revision_id') @@ -151,10 +158,18 @@ class Document(BASE, DeckhandBase): data = Column(JSONEncodedDict(), nullable=False) revision_id = Column(Integer, ForeignKey('revisions.id'), nullable=False) - revision = relationship("Revision", - foreign_keys=[revision_id], - cascade="all, delete") + def to_dict(self, raw_dict=False): + """Convert the ``Document`` object into a dictionary format. + :param raw_dict: if True, returns unmodified data; else returns data + expected by users. + :returns: dictionary format of ``Document`` object. + """ + d = super(Document, self).to_dict() + # ``_metadata`` is used in the DB schema as ``metadata`` is reserved. + if not raw_dict: + d['metadata'] = d.pop('_metadata') + return d def register_models(engine): """Create database tables for all models with the given engine.""" diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index c3d8842e..9f1ff63c 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -21,13 +21,15 @@ from testtools import matchers from deckhand.db.sqlalchemy import api as db_api from deckhand.tests.unit import base +BASE_EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted") +DOCUMENT_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( + "id", "schema", "name", "metadata", "data", "revision_id") +REVISION_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( + "id", "child_id", "parent_id", "documents") + class DocumentFixture(object): - EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted", - "id", "schema", "name", "_metadata", "data", - "revision_id") - @staticmethod def get_minimal_fixture(**kwargs): fixture = {'data': 'fake document data', @@ -54,9 +56,19 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self._validate_revision(revision) return revision + def _validate_object(self, obj): + for attr in BASE_EXPECTED_FIELDS: + if attr.endswith('_at'): + self.assertThat(obj[attr], matchers.MatchesAny( + matchers.Is(None), matchers.IsInstance(str))) + else: + self.assertIsInstance(obj[attr], bool) + def _validate_document(self, actual, expected=None, is_deleted=False): + self._validate_object(actual) + # Validate that the document has all expected fields and is a dict. - expected_fields = list(DocumentFixture.EXPECTED_FIELDS) + expected_fields = list(DOCUMENT_EXPECTED_FIELDS) if not is_deleted: expected_fields.remove('deleted_at') @@ -64,9 +76,6 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): for field in expected_fields: self.assertIn(field, actual) - # ``_metadata`` is used in the DB schema as ``metadata`` is reserved. - actual['metadata'] = actual.pop('_metadata') - if expected: # Validate that the expected values are equivalent to actual # values. @@ -74,11 +83,10 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self.assertEqual(val, actual[key]) def _validate_revision(self, revision): - expected_attrs = ('id', 'child_id', 'parent_id') - for attr in expected_attrs: + self._validate_object(revision) + + for attr in REVISION_EXPECTED_FIELDS: self.assertIn(attr, revision) - self.assertThat(revision[attr], matchers.MatchesAny( - matchers.Is(None), matchers.IsInstance(unicode))) def _validate_revision_connections(self, parent_document, parent_revision, child_document, child_revision, @@ -200,3 +208,14 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): self._validate_revision_connections( parent_document, parent_revision, child_document, child_revision, False) + + def test_get_documents_by_revision_id(self): + payload = DocumentFixture.get_minimal_fixture() + document = self._create_document(payload) + + revision = self._get_revision(document['revision_id']) + self.assertEqual(1, len(revision['documents'])) + self.assertEqual(document, revision['documents'][0]) + + def test_get_multiple_documents_by_revision_id(self): + # TODO From 2d36f866a141ef49c16a9c37ab665ef32c658ae9 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 29 Jul 2017 23:37:25 +0100 Subject: [PATCH 10/23] Test and DB API changes. --- deckhand/db/sqlalchemy/api.py | 61 +++++---- deckhand/tests/test_utils.py | 53 ++++++++ deckhand/tests/unit/base.py | 3 + deckhand/tests/unit/db/test_documents.py | 160 ++++++++--------------- 4 files changed, 141 insertions(+), 136 deletions(-) create mode 100644 deckhand/tests/test_utils.py diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 8e52c0ff..976c0f90 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -114,53 +114,56 @@ def documents_create(documents, session=None): return created_docs -def document_create(values, session=None): +def documents_create(values_list, session=None): """Create a document.""" - values = values.copy() - values['_metadata'] = values.pop('metadata') - values['name'] = values['_metadata']['name'] + values_list = copy.deepcopy(values_list) session = session or get_session() - filters = models.Document.UNIQUE_CONSTRAINTS - try: - existing_document = document_get( - raw_dict=True, - **{c: values[c] for c in filters if c != 'revision_id'}) - except db_exception.DBError: - # Ignore bad data at this point. Allow creation to bubble up the error - # related to bad data. - existing_document = None - created_document = {} + do_create = False + documents_created = [] - def _document_changed(): + def _document_changed(existing_document): # The document has changed if at least one value in ``values`` differs. for key, val in values.items(): if val != existing_document[key]: return True return False - def _document_create(): + def _document_create(values): document = models.Document() with session.begin(): document.update(values) document.save(session=session) return document.to_dict() - if existing_document: - # Only generate a new revision and entirely new document if anything - # was changed. - if _document_changed(): - revision = revision_create_parent(existing_document) - values['revision_id'] = revision['id'] - created_document = _document_create() - revision_update_child(existing_document, created_document) - else: - revision = revision_create() - values['revision_id'] = revision['id'] - created_document = _document_create() + for values in values_list: + values['_metadata'] = values.pop('metadata') + values['name'] = values['_metadata']['name'] + + try: + existing_document = document_get( + raw_dict=True, + **{c: values[c] for c in filters if c != 'revision_id'}) + except db_exception.DBError: + # Ignore bad data at this point. Allow creation to bubble up the + # error related to bad data. + existing_document = None - return created_document + if not existing_document: + do_create = True + elif existing_document and _document_changed(existing_document): + do_create = True + + if do_create: + revision = revision_create() + + for values in values_list: + values['revision_id'] = revision['id'] + doc = _document_create(values) + documents_created.append(doc) + + return documents_created def document_get(session=None, raw_dict=False, **filters): diff --git a/deckhand/tests/test_utils.py b/deckhand/tests/test_utils.py new file mode 100644 index 00000000..cefb16fd --- /dev/null +++ b/deckhand/tests/test_utils.py @@ -0,0 +1,53 @@ +# 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 random +import uuid + + +def rand_uuid(): + """Generate a random UUID string + + :return: a random UUID (e.g. '1dc12c7d-60eb-4b61-a7a2-17cf210155b6') + :rtype: string + """ + return uuidutils.generate_uuid() + + +def rand_uuid_hex(): + """Generate a random UUID hex string + + :return: a random UUID (e.g. '0b98cf96d90447bda4b46f31aeb1508c') + :rtype: string + """ + return uuid.uuid4().hex + + +def rand_name(name='', prefix='tempest'): + """Generate a random name that includes a random number + + :param str name: The name that you want to include + :param str prefix: The prefix that you want to include + :return: a random name. The format is + '--'. + (e.g. 'prefixfoo-namebar-154876201') + :rtype: string + """ + randbits = str(random.randint(1, 0x7fffffff)) + rand_name = randbits + if name: + rand_name = name + '-' + rand_name + if prefix: + rand_name = prefix + '-' + rand_name + return rand_name diff --git a/deckhand/tests/unit/base.py b/deckhand/tests/unit/base.py index 7350af00..ad69e0a6 100644 --- a/deckhand/tests/unit/base.py +++ b/deckhand/tests/unit/base.py @@ -36,6 +36,9 @@ class DeckhandTestCase(testtools.TestCase): CONF.set_override(name, override, group) self.addCleanup(CONF.clear_override, name, group) + def assertEmpty(self, list): + self.assertEqual(0, len(list)) + class DeckhandWithDBTestCase(DeckhandTestCase): diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 9f1ff63c..80467993 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -19,6 +19,7 @@ import testtools from testtools import matchers from deckhand.db.sqlalchemy import api as db_api +from deckhand.tests import test_utils from deckhand.tests.unit import base BASE_EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted") @@ -32,19 +33,28 @@ class DocumentFixture(object): @staticmethod def get_minimal_fixture(**kwargs): - fixture = {'data': 'fake document data', - 'metadata': {'name': 'fake metadata'}, - 'schema': 'deckhand/v1'} + fixture = {'data': test_utils.rand_name('data'), + 'metadata': {'name': test_utils.rand_name('name')}, + 'schema': test_utils.rand_name('schema', prefix='deckhand')} fixture.update(kwargs) return fixture + @staticmethod + def get_minimal_multi_fixture(count=2, **kwargs): + return [DocumentFixture.get_minimal_fixture(**kwargs) + for _ in range(count)] + class TestDocumentsApi(base.DeckhandWithDBTestCase): - def _create_document(self, payload): - doc = db_api.document_create(payload) - self._validate_document(expected=payload, actual=doc) - return doc + def _create_documents(self, payload): + if not isinstance(payload, list): + payload = [payload] + + docs = db_api.documents_create(payload) + for idx, doc in enumerate(docs): + self._validate_document(expected=payload[idx], actual=doc) + return docs def _get_document(self, **fields): doc = db_api.document_get(**fields) @@ -107,115 +117,51 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): def test_create_and_get_document(self): payload = DocumentFixture.get_minimal_fixture() - document = self._create_document(payload) - retrieved_document = self._get_document(id=document['id']) - self.assertEqual(document, retrieved_document) + documents = self._create_documents(payload) + + self.assertIsInstance(documents, list) + self.assertEqual(1, len(documents)) + + for document in documents: + retrieved_document = self._get_document(id=document['id']) + self.assertEqual(document, retrieved_document) + + def test_create_document_again_with_no_changes(self): + payload = DocumentFixture.get_minimal_fixture() + self._create_documents(payload) + documents = self._create_documents(payload) + + self.assertIsInstance(documents, list) + self.assertEmpty(documents) def test_create_document_and_get_revision(self): payload = DocumentFixture.get_minimal_fixture() - document = self._create_document(payload) + documents = self._create_documents(payload) - revision = self._get_revision(document['revision_id']) - self._validate_revision(revision) - self.assertEqual(document['revision_id'], revision['id']) + self.assertIsInstance(documents, list) + self.assertEqual(1, len(documents)) - def test_create_and_update_document(self): - """ - Check that the following relationship is true: - - parent_document --> parent_revision - | ^ - (has child) (has parent) - v | - child_document --> child_revision - """ - child_payload = DocumentFixture.get_minimal_fixture() - child_document = self._create_document(child_payload) - - parent_payload = DocumentFixture.get_minimal_fixture() - parent_payload['data'] = 'fake updated document data' - parent_document = self._create_document(parent_payload) - - # Validate that the new document was created. - self.assertEqual('fake updated document data', parent_document['data']) - self.assertNotEqual(child_document['id'], parent_document['id']) - - # Validate that the parent document has a different revision and - # that the revisions and document links are correct. - child_revision = self._get_revision(child_document['revision_id']) - parent_revision = self._get_revision(parent_document['revision_id']) - - self._validate_revision_connections( - parent_document, parent_revision, child_document, child_revision) - - def test_create_and_update_document_schema(self): - """ - Check that the following relationship is true: - - parent_document --> parent_revision - child_document --> child_revision - - "schema" is unique so changing it results in a new document being - created. - """ - child_payload = DocumentFixture.get_minimal_fixture() - child_document = self._create_document(child_payload) - - parent_payload = DocumentFixture.get_minimal_fixture() - parent_payload['schema'] = 'deckhand/v2' - parent_document = self._create_document(parent_payload) - - # Validate that the new document was created. - self.assertEqual('deckhand/v2', parent_document['schema']) - self.assertNotEqual(child_document['id'], parent_document['id']) - - # Validate that the parent document has a different revision and - # that the revisions and document links are correct. - child_revision = self._get_revision(child_document['revision_id']) - parent_revision = self._get_revision(parent_document['revision_id']) - - self._validate_revision_connections( - parent_document, parent_revision, child_document, child_revision, - False) - - def test_create_and_update_document_metadata_name(self): - """ - Check that the following relationship is true: - - parent_document --> parent_revision - child_document --> child_revision - - "metadata.name" is unique so changing it results in a new document - being created. - """ - child_payload = DocumentFixture.get_minimal_fixture() - child_document = self._create_document(child_payload) - - parent_payload = DocumentFixture.get_minimal_fixture() - parent_payload['metadata'] = {'name': 'fake updated metadata'} - parent_document = self._create_document(parent_payload) - - # Validate that the new document was created. - self.assertEqual({'name': 'fake updated metadata'}, - parent_document['metadata']) - self.assertNotEqual(child_document['id'], parent_document['id']) - - # Validate that the parent document has a different revision and - # that the revisions and document links are correct. - child_revision = self._get_revision(child_document['revision_id']) - parent_revision = self._get_revision(parent_document['revision_id']) - - self._validate_revision_connections( - parent_document, parent_revision, child_document, child_revision, - False) + for document in documents: + 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(self): payload = DocumentFixture.get_minimal_fixture() - document = self._create_document(payload) + documents = self._create_documents(payload) - revision = self._get_revision(document['revision_id']) + revision = self._get_revision(documents[0]['revision_id']) self.assertEqual(1, len(revision['documents'])) - self.assertEqual(document, revision['documents'][0]) + self.assertEqual(documents[0], revision['documents'][0]) def test_get_multiple_documents_by_revision_id(self): - # TODO + payload = DocumentFixture.get_minimal_multi_fixture(count=3) + documents = self._create_documents(payload) + + self.assertIsInstance(documents, list) + self.assertEqual(3, len(documents)) + + for document in documents: + revision = self._get_revision(document['revision_id']) + self._validate_revision(revision) + self.assertEqual(document['revision_id'], revision['id']) From fdab717350decebd45ef1cc8be9edc2f757a2f77 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 29 Jul 2017 23:43:44 +0100 Subject: [PATCH 11/23] Clean up. --- deckhand/db/sqlalchemy/api.py | 60 +++--------------------- deckhand/tests/unit/db/test_documents.py | 17 ------- 2 files changed, 6 insertions(+), 71 deletions(-) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 976c0f90..7d42fda9 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -115,7 +115,12 @@ def documents_create(documents, session=None): def documents_create(values_list, session=None): - """Create a document.""" + """Create a set of documents and associated schema. + + If no changes are detected, a new revision will not be created. This + allows services to periodically re-register their schemas without + creating unnecessary revisions. + """ values_list = copy.deepcopy(values_list) session = session or get_session() filters = models.Document.UNIQUE_CONSTRAINTS @@ -188,56 +193,3 @@ def revision_get(revision_id, session=None): session = session or get_session() revision = session.query(models.Revision).get(revision_id) return revision.to_dict() - - -def revision_create_parent(child_document, session=None): - """Create a parent revision. - - Create a new (parent) revision that references whose ``child_id`` is - the ID of ``child_document``. - - After this function has executed, the following relationship is true: - - parent_document --> parent_revision - | - (has child) - v - child_document --> child_revision - - :param child_document: The out-of-date document. - :param session: The database session. - :returns: The dictionary representation of the newly created revision. - """ - session = session or get_session() - parent_revision = models.Revision() - with session.begin(): - parent_revision.update({'child_id': child_document['revision_id']}) - parent_revision.save(session=session) - - return parent_revision.to_dict() - - -def revision_update_child(child_document, parent_document, session=None): - """Update the child revision for an out-of-date document. - - After this function has executed, the following relationship is true: - - parent_document --> parent_revision - | ^ - (has child) (has parent) - v | - child_document --> child_revision - - :param child_document: The out-of-date document. - :param parent_document: The up-to-date document. - :param session: The database session. - :returns: The dictionary representation of the ``child_revision``. - """ - session = session or get_session() - child_revision = session.query(models.Revision).get( - child_document['revision_id']) - with session.begin(): - child_revision.update({'parent_id': parent_document['revision_id']}) - child_revision.save(session=session) - - return child_revision.to_dict() diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 80467993..f2fd6335 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -98,23 +98,6 @@ class TestDocumentsApi(base.DeckhandWithDBTestCase): for attr in REVISION_EXPECTED_FIELDS: self.assertIn(attr, revision) - def _validate_revision_connections(self, parent_document, parent_revision, - child_document, child_revision, - parent_child_connected=True): - self.assertNotEqual(child_revision['id'], parent_revision['id']) - self.assertEqual(parent_document['revision_id'], parent_revision['id']) - self.assertEqual(child_document['revision_id'], child_revision['id']) - - # Validate that the revisions are distinct and connected together. - if parent_child_connected: - self.assertEqual(parent_revision['child_id'], child_revision['id']) - self.assertEqual( - child_revision['parent_id'], parent_revision['id']) - # Validate that the revisions are distinct but unconnected. - else: - self.assertIsNone(parent_revision['child_id']) - self.assertIsNone(child_revision['parent_id']) - def test_create_and_get_document(self): payload = DocumentFixture.get_minimal_fixture() documents = self._create_documents(payload) From 8e43f917510dc38635814a89325309b0d00064dd Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 30 Jul 2017 04:11:32 +0100 Subject: [PATCH 12/23] 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 From 3bc589e7fc7a8bd78d90246732ef50db8b16a45c Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 30 Jul 2017 22:25:47 +0100 Subject: [PATCH 13/23] Use built-in oslo_db types for Columns serialized as dicts This commit makes the following changes: * removes unncessary code (timeutils, oslo_utils.timeutils can be used instead) * oslo_db.types.JsonEncodedDict can be used instead of a custom JSONEncodedDict (forces Deckhand to save an actual dict in the DB as well) * oslo_db.types.JsonEncodedList used for new `results` Column in Revisions table --- deckhand/common/__init__.py | 0 deckhand/common/timeutils.py | 88 ------------------------ deckhand/db/sqlalchemy/models.py | 34 ++------- deckhand/tests/unit/db/test_documents.py | 4 +- 4 files changed, 7 insertions(+), 119 deletions(-) delete mode 100644 deckhand/common/__init__.py delete mode 100644 deckhand/common/timeutils.py diff --git a/deckhand/common/__init__.py b/deckhand/common/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/deckhand/common/timeutils.py b/deckhand/common/timeutils.py deleted file mode 100644 index 37a2eace..00000000 --- a/deckhand/common/timeutils.py +++ /dev/null @@ -1,88 +0,0 @@ -# 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. - -""" -Time related utilities and helper functions. -""" - -import datetime - -import iso8601 -from monotonic import monotonic as now # noqa -from oslo_utils import encodeutils - -# ISO 8601 extended time format with microseconds -_ISO8601_TIME_FORMAT_SUBSECOND = '%Y-%m-%dT%H:%M:%S.%f' -_ISO8601_TIME_FORMAT = '%Y-%m-%dT%H:%M:%S' -PERFECT_TIME_FORMAT = _ISO8601_TIME_FORMAT_SUBSECOND - - -def isotime(at=None, subsecond=False): - """Stringify time in ISO 8601 format.""" - if not at: - at = utcnow() - st = at.strftime(_ISO8601_TIME_FORMAT - if not subsecond - else _ISO8601_TIME_FORMAT_SUBSECOND) - tz = at.tzinfo.tzname(None) if at.tzinfo else 'UTC' - st += ('Z' if tz == 'UTC' else tz) - return st - - -def parse_isotime(timestr): - """Parse time from ISO 8601 format.""" - try: - return iso8601.parse_date(timestr) - except iso8601.ParseError as e: - raise ValueError(encodeutils.exception_to_unicode(e)) - except TypeError as e: - raise ValueError(encodeutils.exception_to_unicode(e)) - - -def utcnow(with_timezone=False): - """Overridable version of utils.utcnow that can return a TZ-aware datetime. - """ - if utcnow.override_time: - try: - return utcnow.override_time.pop(0) - except AttributeError: - return utcnow.override_time - if with_timezone: - return datetime.datetime.now(tz=iso8601.iso8601.UTC) - return datetime.datetime.utcnow() - - -def normalize_time(timestamp): - """Normalize time in arbitrary timezone to UTC naive object.""" - offset = timestamp.utcoffset() - if offset is None: - return timestamp - return timestamp.replace(tzinfo=None) - offset - - -def iso8601_from_timestamp(timestamp, microsecond=False): - """Returns an iso8601 formatted date from timestamp.""" - return isotime(datetime.datetime.utcfromtimestamp(timestamp), microsecond) - -utcnow.override_time = None - - -def delta_seconds(before, after): - """Return the difference between two timing objects. - - Compute the difference in seconds between two date, time, or - datetime objects (as a float, to microsecond resolution). - """ - delta = after - before - return datetime.timedelta.total_seconds(delta) diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 7c938911..ccd1a2dc 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -15,8 +15,7 @@ import uuid from oslo_db.sqlalchemy import models -from oslo_log import log as logging -from oslo_serialization import jsonutils as json +from oslo_db.sqlalchemy import types as oslo_types from oslo_utils import timeutils from sqlalchemy import Boolean from sqlalchemy import Column @@ -31,38 +30,12 @@ from sqlalchemy import String from sqlalchemy import Text from sqlalchemy.types import TypeDecorator -from deckhand.common import timeutils - -LOG = logging.getLogger(__name__) # Declarative base class which maintains a catalog of classes and tables # relative to that base. BASE = declarative.declarative_base() -class JSONEncodedDict(TypeDecorator): - """Represents an immutable structure as a json-encoded string. - - Usage:: - - JSONEncodedDict(255) - - """ - - impl = Text - - def process_bind_param(self, value, dialect): - if value is not None: - value = json.dumps(value) - - return value - - def process_result_value(self, value, dialect): - if value is not None: - value = json.loads(value) - return value - - class DeckhandBase(models.ModelBase, models.TimestampMixin): """Base class for Deckhand Models.""" @@ -133,6 +106,7 @@ class Revision(BASE, DeckhandBase): default=lambda: str(uuid.uuid4())) parent_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) child_id = Column(Integer, ForeignKey('revisions.id'), nullable=True) + results = Column(oslo_types.JsonEncodedList(), nullable=True) documents = relationship("Document") @@ -154,8 +128,8 @@ class Document(BASE, DeckhandBase): # NOTE: Do not define a maximum length for these JSON data below. However, # this approach is not compatible with all database types. # "metadata" is reserved, so use "doc_metadata" instead. - _metadata = Column(JSONEncodedDict(), nullable=False) - data = Column(JSONEncodedDict(), nullable=False) + _metadata = Column(oslo_types.JsonEncodedDict(), nullable=False) + data = Column(oslo_types.JsonEncodedDict(), nullable=False) revision_id = Column(Integer, ForeignKey('revisions.id'), nullable=False) def to_dict(self, raw_dict=False): diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 2ae5053d..4fa1dd4f 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -31,7 +31,9 @@ class DocumentFixture(object): @staticmethod def get_minimal_fixture(**kwargs): fixture = { - 'data': test_utils.rand_name('data'), + 'data': { + test_utils.rand_name('key'): test_utils.rand_name('value') + }, 'metadata': { 'name': test_utils.rand_name('metadata_data'), 'label': test_utils.rand_name('metadata_label'), From 0608801376a863a23b421dbd63c9c0f476b4d91b Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 30 Jul 2017 23:28:25 +0100 Subject: [PATCH 14/23] Add endpoint for GET /revisions. --- deckhand/control/api.py | 5 +- deckhand/control/revision_documents.py | 50 ++++++++ deckhand/control/revisions.py | 28 +---- deckhand/db/sqlalchemy/api.py | 14 +++ deckhand/tests/unit/control/test_api.py | 9 +- deckhand/tests/unit/db/base.py | 115 ++++++++++++++++++ deckhand/tests/unit/db/test_documents.py | 113 ++--------------- .../tests/unit/db/test_documents_negative.py | 11 +- deckhand/tests/unit/db/test_revisions.py | 28 +++++ 9 files changed, 234 insertions(+), 139 deletions(-) create mode 100644 deckhand/control/revision_documents.py create mode 100644 deckhand/tests/unit/db/base.py create mode 100644 deckhand/tests/unit/db/test_revisions.py diff --git a/deckhand/control/api.py b/deckhand/control/api.py index 5bb28e18..15d3257f 100644 --- a/deckhand/control/api.py +++ b/deckhand/control/api.py @@ -21,6 +21,7 @@ from oslo_log import log as logging from deckhand.conf import config from deckhand.control import base as api_base from deckhand.control import documents +from deckhand.control import revision_documents from deckhand.control import revisions from deckhand.control import secrets from deckhand.db.sqlalchemy import api as db_api @@ -69,7 +70,9 @@ def start_api(state_manager=None): v1_0_routes = [ ('documents', documents.DocumentsResource()), - ('revisions/{revision_id}/documents', revisions.RevisionsResource()), + ('revisions', revisions.RevisionsResource()), + ('revisions/{revision_id}/documents', + revision_documents.RevisionDocumentsResource()), ('secrets', secrets.SecretsResource()) ] diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py new file mode 100644 index 00000000..22b12b82 --- /dev/null +++ b/deckhand/control/revision_documents.py @@ -0,0 +1,50 @@ +# 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 copy +import yaml + +import falcon + +from oslo_db import exception as db_exc +from oslo_log import log as logging +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 import errors + +LOG = logging.getLogger(__name__) + + +class RevisionDocumentsResource(api_base.BaseResource): + """API resource for realizing CRUD endpoints for Document Revisions.""" + + def on_get(self, req, resp, revision_id): + """Returns all documents for a `revision_id`. + + Returns a multi-document YAML response containing all the documents + matching the filters specified via query string parameters. Returned + documents will be as originally posted with no substitutions or + layering applied. + """ + params = req.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 + # TODO: return YAML-encoded body + resp.body = json.dumps(documents) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 77b1fdec..9b6f89ff 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -12,39 +12,23 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy -import yaml - import falcon -from oslo_db import exception as db_exc -from oslo_log import log as logging -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 import errors - -LOG = logging.getLogger(__name__) class RevisionsResource(api_base.BaseResource): """API resource for realizing CRUD endpoints for Document Revisions.""" def on_get(self, req, resp, revision_id): - """Returns all documents for a `revision_id`. + """Returns list of existing revisions. - Returns a multi-document YAML response containing all the documents - matching the filters specified via query string parameters. Returned - documents will be as originally posted with no substitutions or - layering applied. + Lists existing revisions and reports basic details including a summary + of validation status for each `deckhand/ValidationPolicy` that is part + of each revision. """ - 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) + revisions = db_api.revision_get_all() resp.status = falcon.HTTP_200 - resp.body = json.dumps(documents) + resp.body = json.dumps(revisions) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 172e16d1..809b87cf 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -207,6 +207,20 @@ def revision_get(revision_id, session=None): return revision +def revision_get_all(session=None): + """Return list of all revisions.""" + session = session or get_session() + revisions = session.query(models.Revision).all() + + revisions_resp = [] + for revision in revisions: + revision_dict = revision.to_dict() + revision['count'] = len(revision_dict.pop('documents')) + revisions_resp.append(revision) + + return revisions_resp + + def revision_get_documents(revision_id, session=None, **filters): """Return the documents that match filters for the specified `revision_id`. diff --git a/deckhand/tests/unit/control/test_api.py b/deckhand/tests/unit/control/test_api.py index a5fe4d85..ddbac81c 100644 --- a/deckhand/tests/unit/control/test_api.py +++ b/deckhand/tests/unit/control/test_api.py @@ -19,6 +19,7 @@ import testtools from deckhand.control import api from deckhand.control import base as api_base from deckhand.control import documents +from deckhand.control import revision_documents from deckhand.control import revisions from deckhand.control import secrets @@ -27,10 +28,11 @@ class TestApi(testtools.TestCase): def setUp(self): super(TestApi, self).setUp() - for resource in (documents, revisions, secrets): + for resource in (documents, revisions, revision_documents, secrets): resource_name = resource.__name__.split('.')[-1] resource_obj = mock.patch.object( - resource, '%sResource' % resource_name.title()).start() + resource, '%sResource' % resource_name.title().replace('_', '') + ).start() setattr(self, '%s_resource' % resource_name, resource_obj) @mock.patch.object(api, 'db_api', autospec=True) @@ -47,8 +49,9 @@ class TestApi(testtools.TestCase): request_type=api_base.DeckhandRequest) mock_falcon_api.add_route.assert_has_calls([ mock.call('/api/v1.0/documents', self.documents_resource()), + mock.call('/api/v1.0/revisions', self.revisions_resource()), mock.call('/api/v1.0/revisions/{revision_id}/documents', - self.revisions_resource()), + self.revision_documents_resource()), mock.call('/api/v1.0/secrets', self.secrets_resource()) ]) mock_config.parse_args.assert_called_once_with() diff --git a/deckhand/tests/unit/db/base.py b/deckhand/tests/unit/db/base.py new file mode 100644 index 00000000..e155339f --- /dev/null +++ b/deckhand/tests/unit/db/base.py @@ -0,0 +1,115 @@ +# 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 testtools import matchers + +from deckhand.db.sqlalchemy import api as db_api +from deckhand.tests import test_utils +from deckhand.tests.unit import base + +BASE_EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted") +DOCUMENT_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( + "id", "schema", "name", "metadata", "data", "revision_id") +REVISION_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( + "id", "child_id", "parent_id", "documents") + + +class DocumentFixture(object): + + @staticmethod + def get_minimal_fixture(**kwargs): + fixture = { + 'data': { + test_utils.rand_name('key'): test_utils.rand_name('value') + }, + '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 + + @staticmethod + def get_minimal_multi_fixture(count=2, **kwargs): + return [DocumentFixture.get_minimal_fixture(**kwargs) + for _ in range(count)] + + +class TestDbBase(base.DeckhandWithDBTestCase): + + def _create_documents(self, payload): + if not isinstance(payload, list): + payload = [payload] + + docs = db_api.documents_create(payload) + for idx, doc in enumerate(docs): + self._validate_document(expected=payload[idx], actual=doc) + return docs + + def _get_document(self, **fields): + doc = db_api.document_get(**fields) + self._validate_document(actual=doc) + return doc + + def _get_revision(self, revision_id): + revision = db_api.revision_get(revision_id) + 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 _list_revisions(self): + return db_api.revision_get_all() + + def _validate_object(self, obj): + for attr in BASE_EXPECTED_FIELDS: + if attr.endswith('_at'): + self.assertThat(obj[attr], matchers.MatchesAny( + matchers.Is(None), matchers.IsInstance(str))) + else: + self.assertIsInstance(obj[attr], bool) + + def _validate_document(self, actual, expected=None, is_deleted=False): + self._validate_object(actual) + + # Validate that the document has all expected fields and is a dict. + expected_fields = list(DOCUMENT_EXPECTED_FIELDS) + if not is_deleted: + expected_fields.remove('deleted_at') + + self.assertIsInstance(actual, dict) + for field in expected_fields: + self.assertIn(field, actual) + + if expected: + # Validate that the expected values are equivalent to actual + # values. + for key, val in expected.items(): + self.assertEqual(val, actual[key]) + + def _validate_revision(self, revision): + self._validate_object(revision) + + for attr in REVISION_EXPECTED_FIELDS: + self.assertIn(attr, revision) diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 4fa1dd4f..b1baea1f 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -12,110 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import testtools -from testtools import matchers - -from deckhand.db.sqlalchemy import api as db_api -from deckhand.tests import test_utils -from deckhand.tests.unit import base - -BASE_EXPECTED_FIELDS = ("created_at", "updated_at", "deleted_at", "deleted") -DOCUMENT_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( - "id", "schema", "name", "metadata", "data", "revision_id") -REVISION_EXPECTED_FIELDS = BASE_EXPECTED_FIELDS + ( - "id", "child_id", "parent_id", "documents") +from deckhand.tests.unit.db import base -class DocumentFixture(object): - - @staticmethod - def get_minimal_fixture(**kwargs): - fixture = { - 'data': { - test_utils.rand_name('key'): test_utils.rand_name('value') - }, - '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 - - @staticmethod - def get_minimal_multi_fixture(count=2, **kwargs): - return [DocumentFixture.get_minimal_fixture(**kwargs) - for _ in range(count)] - - -class TestDocumentsBase(base.DeckhandWithDBTestCase): - - def _create_documents(self, payload): - if not isinstance(payload, list): - payload = [payload] - - docs = db_api.documents_create(payload) - for idx, doc in enumerate(docs): - self._validate_document(expected=payload[idx], actual=doc) - return docs - - def _get_document(self, **fields): - doc = db_api.document_get(**fields) - self._validate_document(actual=doc) - return doc - - def _get_revision(self, revision_id): - revision = db_api.revision_get(revision_id) - 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'): - self.assertThat(obj[attr], matchers.MatchesAny( - matchers.Is(None), matchers.IsInstance(str))) - else: - self.assertIsInstance(obj[attr], bool) - - def _validate_document(self, actual, expected=None, is_deleted=False): - self._validate_object(actual) - - # Validate that the document has all expected fields and is a dict. - expected_fields = list(DOCUMENT_EXPECTED_FIELDS) - if not is_deleted: - expected_fields.remove('deleted_at') - - self.assertIsInstance(actual, dict) - for field in expected_fields: - self.assertIn(field, actual) - - if expected: - # Validate that the expected values are equivalent to actual - # values. - for key, val in expected.items(): - self.assertEqual(val, actual[key]) - - def _validate_revision(self, revision): - self._validate_object(revision) - - for attr in REVISION_EXPECTED_FIELDS: - self.assertIn(attr, revision) - - -class TestDocuments(TestDocumentsBase): +class TestDocuments(base.TestDbBase): def test_create_and_get_document(self): - payload = DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() documents = self._create_documents(payload) self.assertIsInstance(documents, list) @@ -126,7 +29,7 @@ class TestDocuments(TestDocumentsBase): self.assertEqual(document, retrieved_document) def test_create_document_again_with_no_changes(self): - payload = DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() self._create_documents(payload) documents = self._create_documents(payload) @@ -134,7 +37,7 @@ class TestDocuments(TestDocumentsBase): self.assertEmpty(documents) def test_create_document_and_get_revision(self): - payload = DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() documents = self._create_documents(payload) self.assertIsInstance(documents, list) @@ -146,7 +49,7 @@ class TestDocuments(TestDocumentsBase): self.assertEqual(document['revision_id'], revision['id']) def test_get_documents_by_revision_id(self): - payload = DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() documents = self._create_documents(payload) revision = self._get_revision(documents[0]['revision_id']) @@ -154,7 +57,7 @@ class TestDocuments(TestDocumentsBase): self.assertEqual(documents[0], revision['documents'][0]) def test_get_multiple_documents_by_revision_id(self): - payload = DocumentFixture.get_minimal_multi_fixture(count=3) + payload = base.DocumentFixture.get_minimal_multi_fixture(count=3) documents = self._create_documents(payload) self.assertIsInstance(documents, list) @@ -166,7 +69,7 @@ class TestDocuments(TestDocumentsBase): self.assertEqual(document['revision_id'], revision['id']) def test_get_documents_by_revision_id_and_filters(self): - payload = DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() document = self._create_documents(payload)[0] filters = { 'schema': document['schema'], diff --git a/deckhand/tests/unit/db/test_documents_negative.py b/deckhand/tests/unit/db/test_documents_negative.py index 372cb787..f7d2eaa3 100644 --- a/deckhand/tests/unit/db/test_documents_negative.py +++ b/deckhand/tests/unit/db/test_documents_negative.py @@ -12,18 +12,13 @@ # 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 +from deckhand.tests.unit.db import base -class TestDocumentsNegative(test_documents.TestDocumentsBase): +class TestDocumentsNegative(base.TestDbBase): def test_get_documents_by_revision_id_and_wrong_filters(self): - payload = test_documents.DocumentFixture.get_minimal_fixture() + payload = base.DocumentFixture.get_minimal_fixture() document = self._create_documents(payload)[0] filters = { 'schema': 'fake_schema', diff --git a/deckhand/tests/unit/db/test_revisions.py b/deckhand/tests/unit/db/test_revisions.py new file mode 100644 index 00000000..9bb8f6e0 --- /dev/null +++ b/deckhand/tests/unit/db/test_revisions.py @@ -0,0 +1,28 @@ +# 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. + +from deckhand.tests.unit.db import base + + +class TestRevisions(base.TestDbBase): + + def test_list_revisions(self): + payload = [base.DocumentFixture.get_minimal_fixture() + for _ in range(4)] + self._create_documents(payload) + + revisions = self._list_revisions() + self.assertIsInstance(revisions, list) + self.assertEqual(1, len(revisions)) + self.assertEqual(4, revisions[0]["count"]) From 1a4c0751d19a7ef762b77576da8243593169dea7 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 15:36:39 +0100 Subject: [PATCH 15/23] Return YAML response body. --- deckhand/control/base.py | 11 ++++++++++- deckhand/control/documents.py | 2 +- deckhand/control/revision_documents.py | 13 ++----------- deckhand/control/revisions.py | 4 ++-- deckhand/db/sqlalchemy/api.py | 8 +++----- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/deckhand/control/base.py b/deckhand/control/base.py index f885d463..949f0759 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import uuid +import yaml import falcon from falcon import request +from oslo_serialization import jsonutils as json import six from deckhand import errors @@ -77,6 +78,14 @@ class BaseResource(object): 'retry': retry}) resp.status = status_code + def to_yaml_body(self, dict_body): + """Converts dictionary into YAML response body. + + :dict_body: response body to be converted to YAML. + :returns: YAML encoding of `dict_body`. + """ + return yaml.safe_dump_all(dict_body) + class DeckhandRequestContext(object): diff --git a/deckhand/control/documents.py b/deckhand/control/documents.py index c4669e77..cb032ddc 100644 --- a/deckhand/control/documents.py +++ b/deckhand/control/documents.py @@ -62,4 +62,4 @@ class DocumentsResource(api_base.BaseResource): return self.return_error(resp, falcon.HTTP_500, message=e) resp.status = falcon.HTTP_201 - resp.body = json.dumps(created_documents) + resp.body = self.to_yaml_body(created_documents) diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 22b12b82..e3509c88 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -12,21 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy -import yaml - import falcon - from oslo_db import exception as db_exc -from oslo_log import log as logging -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 import errors -LOG = logging.getLogger(__name__) - class RevisionDocumentsResource(api_base.BaseResource): """API resource for realizing CRUD endpoints for Document Revisions.""" @@ -43,8 +35,7 @@ class RevisionDocumentsResource(api_base.BaseResource): 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) + return self.return_error(resp, falcon.HTTP_404, message=e) resp.status = falcon.HTTP_200 - # TODO: return YAML-encoded body - resp.body = json.dumps(documents) + resp.body = self.to_yaml_body(documents) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 9b6f89ff..655f5772 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -21,7 +21,7 @@ from deckhand.db.sqlalchemy import api as db_api class RevisionsResource(api_base.BaseResource): """API resource for realizing CRUD endpoints for Document Revisions.""" - def on_get(self, req, resp, revision_id): + def on_get(self, req, resp): """Returns list of existing revisions. Lists existing revisions and reports basic details including a summary @@ -31,4 +31,4 @@ class RevisionsResource(api_base.BaseResource): revisions = db_api.revision_get_all() resp.status = falcon.HTTP_200 - resp.body = json.dumps(revisions) + resp.body = self.to_yaml_body(revisions) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 809b87cf..2820fab1 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -211,12 +211,10 @@ def revision_get_all(session=None): """Return list of all revisions.""" session = session or get_session() revisions = session.query(models.Revision).all() + revisions_resp = [r.to_dict() for r in revisions] - revisions_resp = [] - for revision in revisions: - revision_dict = revision.to_dict() - revision['count'] = len(revision_dict.pop('documents')) - revisions_resp.append(revision) + for revision in revisions_resp: + revision['count'] = len(revision.pop('documents')) return revisions_resp From 714b2d4e7142cf44544b00297e91f2ff2132965f Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 15:50:55 +0100 Subject: [PATCH 16/23] Update control README (with current response bodies, even though they're a WIP. --- deckhand/control/README.rst | 139 ++++++++++++++++++++++++- deckhand/control/documents.py | 1 + deckhand/control/revision_documents.py | 1 + deckhand/control/revisions.py | 1 + 4 files changed, 137 insertions(+), 5 deletions(-) diff --git a/deckhand/control/README.rst b/deckhand/control/README.rst index 3f13ade6..8b7a575e 100644 --- a/deckhand/control/README.rst +++ b/deckhand/control/README.rst @@ -7,13 +7,142 @@ Deckhand-managed data. v1.0 Endpoints -------------- -/api/v1.0/documents -~~~~~~~~~~~~~~~~~~~ +POST `/documents` +~~~~~~~~~~~~~~~~~ -POST - Create a new YAML document and return a revision number. If the YAML -document already exists, then the document will be replaced and a new -revision number will be returned. +Accepts a multi-document YAML body and creates a new revision which adds +those documents. Updates are detected based on exact match to an existing +document of `schema` + `metadata.name`. Documents are "deleted" by including +documents with the tombstone metadata schema, such as: +```yaml +--- +schema: any-namespace/AnyKind/v1 +metadata: + schema: metadata/Tombstone/v1 + name: name-to-delete +... +``` + +This endpoint is the only way to add, update, and delete documents. This +triggers Deckhand's internal schema validations for all documents. + +If no changes are detected, a new revision should not be created. This allows +services to periodically re-register their schemas without creating +unnecessary revisions. + +Sample response: + +```yaml +--- +created_at: '2017-07-31T14:46:46.119853' +data: + path: + to: + merge: + into: + ignored: {data: here} + parent: {foo: bar} + substitution: {target: null} +deleted: false +deleted_at: null +id: f99630d9-a89c-4aad-9aaa-7c44462047c1 +metadata: + labels: {genesis: enabled, master: enabled} + layeringDefinition: + abstract: false + actions: + - {method: merge, path: .path.to.merge.into.parent} + - {method: delete, path: .path.to.delete} + layer: region + parentSelector: {required_key_a: required_label_a, required_key_b: required_label_b} + name: unique-name-given-schema + schema: metadata/Document/v1 + storagePolicy: cleartext + substitutions: + - dest: {path: .substitution.target} + src: {name: name-of-source-document, path: .source.path, schema: another-service/SourceType/v1} +name: unique-name-given-schema +revision_id: 0206088a-c9e9-48e1-8725-c9bdac15d6b7 +schema: some-service/ResourceType/v1 +updated_at: '2017-07-31T14:46:46.119858' +``` + +GET `/revisions` +~~~~~~~~~~~~~~~~ + +Lists existing revisions and reports basic details including a summary of +validation status for each `deckhand/ValidationPolicy` that is part of that +revision. + +Sample response: + +```yaml +--- +child_id: null +count: 2 +created_at: '2017-07-31T14:36:00.348967' +deleted: false +deleted_at: null +id: d3428d6a-d8c4-4a5b-8006-aba974cc36a2 +parent_id: null +results: [] +updated_at: '2017-07-31T14:36:00.348973' +``` + +GET `/revisions/{revision_id}/documents` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Returns a multi-document YAML response containing all the documents matching +the filters specified via query string parameters. Returned documents will be +as originally posted with no substitutions or layering applied. + +Supported query string parameters: + +* `schema` - string, optional - The top-level `schema` field to select. This + may be partially specified by section, e.g., `schema=promenade` would select all + `kind` and `version` schemas owned by promenade, or `schema=promenade/Node` + which would select all versions of `promenade/Node` documents. One may not + partially specify the namespace or kind, so `schema=promenade/No` would not + select `promenade/Node/v1` documents, and `schema=prom` would not select + `promenade` documents. +* `metadata.name` - string, optional +* `metadata.layeringDefinition.abstract` - string, optional - Valid values are + the "true" and "false". +* `metadata.layeringDefinition.layer` - string, optional - Only return documents from + the specified layer. +* `metadata.label` - string, optional, repeatable - Uses the format + `metadata.label=key=value`. Repeating this parameter indicates all + requested labels must apply (AND not OR). + +Sample response: + +```yaml +created_at: '2017-07-31T14:36:00.352701' +data: {foo: bar} +deleted: false +deleted_at: null +id: ffba233a-326b-4eed-9b21-079ebd2a53f0 +metadata: + labels: {genesis: enabled, master: enabled} + layeringDefinition: + abstract: false + actions: + - {method: merge, path: .path.to.merge.into.parent} + - {method: delete, path: .path.to.delete} + layer: region + parentSelector: {required_key_a: required_label_a, required_key_b: required_label_b} + name: foo-name-given-schema + schema: metadata/Document/v1 + storagePolicy: cleartext + substitutions: + - dest: {path: .substitution.target} + src: {name: name-of-source-document, path: .source.path, schema: another-service/SourceType/v1} +name: foo-name-given-schema +revision_id: d3428d6a-d8c4-4a5b-8006-aba974cc36a2 +schema: some-service/ResourceType/v1 +updated_at: '2017-07-31T14:36:00.352705' +``` Testing ------- diff --git a/deckhand/control/documents.py b/deckhand/control/documents.py index cb032ddc..3e9e90f0 100644 --- a/deckhand/control/documents.py +++ b/deckhand/control/documents.py @@ -62,4 +62,5 @@ class DocumentsResource(api_base.BaseResource): return self.return_error(resp, falcon.HTTP_500, message=e) resp.status = falcon.HTTP_201 + resp.append_header('Content-Type', 'application/x-yaml') resp.body = self.to_yaml_body(created_documents) diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index e3509c88..d4394869 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -38,4 +38,5 @@ class RevisionDocumentsResource(api_base.BaseResource): return self.return_error(resp, falcon.HTTP_404, message=e) resp.status = falcon.HTTP_200 + resp.append_header('Content-Type', 'application/x-yaml') resp.body = self.to_yaml_body(documents) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 655f5772..e8032944 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -31,4 +31,5 @@ class RevisionsResource(api_base.BaseResource): revisions = db_api.revision_get_all() resp.status = falcon.HTTP_200 + resp.append_header('Content-Type', 'application/x-yaml') resp.body = self.to_yaml_body(revisions) From 8ff4639c6c8962534baebc6e1d2c68124f988e24 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 16:00:09 +0100 Subject: [PATCH 17/23] Remove old docstring. --- deckhand/db/sqlalchemy/models.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index ccd1a2dc..ca4bcf5b 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -94,12 +94,6 @@ class DeckhandBase(models.ModelBase, models.TimestampMixin): class Revision(BASE, DeckhandBase): - """Revision history for a ``Document``. - - Like a doubly linked list, each ``Revision`` will have a unique ID along - with a previous and next pointer to each ``Revision`` that comprises the - revision history for a ``Document``. - """ __tablename__ = 'revisions' id = Column(String(36), primary_key=True, From 841906a4353cd0fc88d23107b28be075dc98005e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 17:01:40 +0100 Subject: [PATCH 18/23] Updated /GET revisions response body. --- deckhand/control/README.rst | 26 ++++++++++++------- deckhand/control/base.py | 12 +++++++-- deckhand/control/documents.py | 5 ++-- deckhand/control/revisions.py | 2 +- deckhand/db/sqlalchemy/api.py | 14 ++++++++-- deckhand/db/sqlalchemy/models.py | 2 +- deckhand/engine/document_validation.py | 3 +-- deckhand/tests/unit/db/test_revisions.py | 6 ++++- .../unit/engine/test_document_validation.py | 19 ++++++++------ deckhand/utils.py | 7 +++++ 10 files changed, 68 insertions(+), 28 deletions(-) diff --git a/deckhand/control/README.rst b/deckhand/control/README.rst index 8b7a575e..243c4c70 100644 --- a/deckhand/control/README.rst +++ b/deckhand/control/README.rst @@ -79,15 +79,23 @@ Sample response: ```yaml --- -child_id: null -count: 2 -created_at: '2017-07-31T14:36:00.348967' -deleted: false -deleted_at: null -id: d3428d6a-d8c4-4a5b-8006-aba974cc36a2 -parent_id: null -results: [] -updated_at: '2017-07-31T14:36:00.348973' +count: 7 +next: https://deckhand/api/v1.0/revisions?limit=2&offset=2 +prev: null +results: + - id: 0 + url: https://deckhand/api/v1.0/revisions/0 + createdAt: 2017-07-14T21:23Z + validationPolicies: + site-deploy-validation: + status: failed + - id: 1 + url: https://deckhand/api/v1.0/revisions/1 + createdAt: 2017-07-16T01:15Z + validationPolicies: + site-deploy-validation: + status: succeeded +... ``` GET `/revisions/{revision_id}/documents` diff --git a/deckhand/control/base.py b/deckhand/control/base.py index 949f0759..d2975f20 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -17,11 +17,14 @@ import yaml import falcon from falcon import request +from oslo_log import log as logging from oslo_serialization import jsonutils as json import six from deckhand import errors +LOG = logging.getLogger(__name__) + class BaseResource(object): """Base resource class for implementing API resources.""" @@ -79,12 +82,17 @@ class BaseResource(object): resp.status = status_code def to_yaml_body(self, dict_body): - """Converts dictionary into YAML response body. + """Converts JSON body into YAML response body. :dict_body: response body to be converted to YAML. :returns: YAML encoding of `dict_body`. """ - return yaml.safe_dump_all(dict_body) + if isinstance(dict_body, dict): + return yaml.safe_dump(dict_body) + elif isinstance(dict_body, list): + return yaml.safe_dump_all(dict_body) + return TypeError('Unrecognized dict_body type when converting response' + ' body to YAML format.') class DeckhandRequestContext(object): diff --git a/deckhand/control/documents.py b/deckhand/control/documents.py index 3e9e90f0..b1d12910 100644 --- a/deckhand/control/documents.py +++ b/deckhand/control/documents.py @@ -47,10 +47,11 @@ class DocumentsResource(api_base.BaseResource): LOG.error(error_msg) return self.return_error(resp, falcon.HTTP_400, message=error_msg) - # Validate the document before doing anything with it. + # All concrete documents in the payload must successfully pass their + # JSON schema validations. Otherwise raise an error. try: for doc in documents: - document_validation.DocumentValidation(doc) + document_validation.DocumentValidation(doc).pre_validate() except deckhand_errors.InvalidFormat as e: return self.return_error(resp, falcon.HTTP_400, message=e) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index e8032944..4956c193 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -19,7 +19,7 @@ from deckhand.db.sqlalchemy import api as db_api class RevisionsResource(api_base.BaseResource): - """API resource for realizing CRUD endpoints for Document Revisions.""" + """API resource for realizing CRUD endpoints for Revisions.""" def on_get(self, req, resp): """Returns list of existing revisions. diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 2820fab1..3b3ce523 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -212,11 +212,21 @@ def revision_get_all(session=None): session = session or get_session() revisions = session.query(models.Revision).all() revisions_resp = [r.to_dict() for r in revisions] + resp_body = { + 'count': len(revisions_resp), + 'next': None, + 'prev': None, + 'revisions': [] + } for revision in revisions_resp: - revision['count'] = len(revision.pop('documents')) + result = {} + for attr in ('id', 'created_at'): + result[utils.to_camel_case(attr)] = revision[attr] + result['count'] = len(revision.pop('documents')) + resp_body['revisions'].append(result) - return revisions_resp + return resp_body def revision_get_documents(revision_id, session=None, **filters): diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index ca4bcf5b..348fe6ba 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -121,7 +121,7 @@ class Document(BASE, DeckhandBase): name = Column(String(64), nullable=False) # NOTE: Do not define a maximum length for these JSON data below. However, # this approach is not compatible with all database types. - # "metadata" is reserved, so use "doc_metadata" instead. + # "metadata" is reserved, so use "_metadata" instead. _metadata = Column(oslo_types.JsonEncodedDict(), nullable=False) data = Column(oslo_types.JsonEncodedDict(), nullable=False) revision_id = Column(Integer, ForeignKey('revisions.id'), nullable=False) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 44494fb1..7dc5ae0c 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -34,7 +34,6 @@ class DocumentValidation(object): def __init__(self, data): self.data = data - self.pre_validate_data() class SchemaVersion(object): """Class for retrieving correct schema for pre-validation on YAML. @@ -65,7 +64,7 @@ class DocumentValidation(object): return [v['schema'] for v in self.internal_validations if v['version'] == self.schema_version][0].schema - def pre_validate_data(self): + def pre_validate(self): """Pre-validate that the YAML file is correctly formatted.""" self._validate_with_schema() diff --git a/deckhand/tests/unit/db/test_revisions.py b/deckhand/tests/unit/db/test_revisions.py index 9bb8f6e0..1bfd4fe2 100644 --- a/deckhand/tests/unit/db/test_revisions.py +++ b/deckhand/tests/unit/db/test_revisions.py @@ -23,6 +23,10 @@ class TestRevisions(base.TestDbBase): self._create_documents(payload) revisions = self._list_revisions() - self.assertIsInstance(revisions, list) + self.assertIsInstance(revisions, dict) + self.assertIn('revisions', revisions) + self.assertIsInstance(revisions['revisions'], list) + + revisions = revisions['revisions'] self.assertEqual(1, len(revisions)) self.assertEqual(4, revisions[0]["count"]) diff --git a/deckhand/tests/unit/engine/test_document_validation.py b/deckhand/tests/unit/engine/test_document_validation.py index eeaee958..2b2ff1a6 100644 --- a/deckhand/tests/unit/engine/test_document_validation.py +++ b/deckhand/tests/unit/engine/test_document_validation.py @@ -71,10 +71,8 @@ class TestDocumentValidation(testtools.TestCase): return corrupted_data def test_initialization(self): - doc_validation = document_validation.DocumentValidation( - self.data) - self.assertIsInstance(doc_validation, - document_validation.DocumentValidation) + doc_validation = document_validation.DocumentValidation(self.data) + doc_validation.pre_validate() # Should not raise any errors. def test_initialization_missing_sections(self): expected_err = ("The provided YAML file is invalid. Exception: '%s' " @@ -91,11 +89,12 @@ class TestDocumentValidation(testtools.TestCase): for invalid_entry, missing_key in invalid_data: with six.assertRaisesRegex(self, errors.InvalidFormat, expected_err % missing_key): - document_validation.DocumentValidation(invalid_entry) + doc_validation = document_validation.DocumentValidation( + invalid_entry) + doc_validation.pre_validate() def test_initialization_missing_abstract_section(self): expected_err = ("Could not find 'abstract' property from document.") - invalid_data = [ self._corrupt_data('metadata'), self._corrupt_data('metadata.layeringDefinition'), @@ -105,7 +104,9 @@ class TestDocumentValidation(testtools.TestCase): for invalid_entry in invalid_data: with six.assertRaisesRegex(self, errors.InvalidFormat, expected_err): - document_validation.DocumentValidation(invalid_entry) + doc_validation = document_validation.DocumentValidation( + invalid_entry) + doc_validation.pre_validate() @mock.patch.object(document_validation, 'LOG', autospec=True) def test_initialization_with_abstract_document(self, mock_log): @@ -114,7 +115,9 @@ class TestDocumentValidation(testtools.TestCase): for true_val in (True, 'true', 'True'): abstract_data['metadata']['layeringDefinition']['abstract'] = True - document_validation.DocumentValidation(abstract_data) + doc_validation = document_validation.DocumentValidation( + abstract_data) + doc_validation.pre_validate() mock_log.info.assert_called_once_with( "Skipping validation for the document because it is abstract") mock_log.info.reset_mock() diff --git a/deckhand/utils.py b/deckhand/utils.py index 0e44eefd..18fba501 100644 --- a/deckhand/utils.py +++ b/deckhand/utils.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import string + def multi_getattr(multi_key, dict_data): """Iteratively check for nested attributes in the YAML data. @@ -45,3 +47,8 @@ def multi_getattr(multi_key, dict_data): data = data.get(attr) return data + + +def to_camel_case(s): + return (s[0].lower() + string.capwords(s, sep='_').replace('_', '')[1:] + if s else s) From 1c942b23e36f837a8f3d198816d72536104f6237 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 17:22:58 +0100 Subject: [PATCH 19/23] Raise exception instead of return. --- deckhand/control/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deckhand/control/base.py b/deckhand/control/base.py index d2975f20..0d0fc5ee 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -91,8 +91,8 @@ class BaseResource(object): return yaml.safe_dump(dict_body) elif isinstance(dict_body, list): return yaml.safe_dump_all(dict_body) - return TypeError('Unrecognized dict_body type when converting response' - ' body to YAML format.') + raise TypeError('Unrecognized dict_body type when converting response ' + 'body to YAML format.') class DeckhandRequestContext(object): From 6299c4b123f5b2185fe84c01a4b0d3bdf760063e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 20:08:38 +0100 Subject: [PATCH 20/23] Add view abstraction layer for modifying DB data into view data. --- deckhand/control/base.py | 2 +- deckhand/control/common.py | 26 ++++++++++ deckhand/control/revisions.py | 2 + deckhand/control/views/__init__.py | 0 deckhand/control/views/revision.py | 38 ++++++++++++++ deckhand/db/sqlalchemy/api.py | 18 +------ deckhand/tests/test_utils.py | 18 +++---- deckhand/tests/unit/db/test_revisions.py | 12 ++--- deckhand/tests/unit/views/__init__.py | 0 deckhand/tests/unit/views/test_views.py | 66 ++++++++++++++++++++++++ deckhand/utils.py | 7 --- 11 files changed, 147 insertions(+), 42 deletions(-) create mode 100644 deckhand/control/common.py create mode 100644 deckhand/control/views/__init__.py create mode 100644 deckhand/control/views/revision.py create mode 100644 deckhand/tests/unit/views/__init__.py create mode 100644 deckhand/tests/unit/views/test_views.py diff --git a/deckhand/control/base.py b/deckhand/control/base.py index 0d0fc5ee..c9907603 100644 --- a/deckhand/control/base.py +++ b/deckhand/control/base.py @@ -99,7 +99,7 @@ class DeckhandRequestContext(object): def __init__(self): self.user = None - self.roles = ['*'] + self.roles = [] self.request_id = str(uuid.uuid4()) def set_user(self, user): diff --git a/deckhand/control/common.py b/deckhand/control/common.py new file mode 100644 index 00000000..a2c26dd5 --- /dev/null +++ b/deckhand/control/common.py @@ -0,0 +1,26 @@ +# 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 string + + +def to_camel_case(s): + return (s[0].lower() + string.capwords(s, sep='_').replace('_', '')[1:] + if s else s) + + +class ViewBuilder(object): + """Model API responses as dictionaries.""" + + _collection_name = None diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 4956c193..59321d34 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -15,6 +15,7 @@ import falcon from deckhand.control import base as api_base +from deckhand.control.views import revision as revision_view from deckhand.db.sqlalchemy import api as db_api @@ -29,6 +30,7 @@ class RevisionsResource(api_base.BaseResource): of each revision. """ revisions = db_api.revision_get_all() + resp = revision_view.ViewBuilder().list(revisions) resp.status = falcon.HTTP_200 resp.append_header('Content-Type', 'application/x-yaml') diff --git a/deckhand/control/views/__init__.py b/deckhand/control/views/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/deckhand/control/views/revision.py b/deckhand/control/views/revision.py new file mode 100644 index 00000000..32270072 --- /dev/null +++ b/deckhand/control/views/revision.py @@ -0,0 +1,38 @@ +# 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. + +from deckhand.control import common + + +class ViewBuilder(common.ViewBuilder): + """Model revision API responses as a python dictionary.""" + + _collection_name = 'revisions' + + def list(self, revisions): + resp_body = { + 'count': len(revisions), + 'next': None, + 'prev': None, + 'results': [] + } + + for revision in revisions: + result = {} + for attr in ('id', 'created_at'): + result[common.to_camel_case(attr)] = revision[attr] + result['count'] = len(revision.pop('documents')) + resp_body['results'].append(result) + + return resp_body diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 3b3ce523..ccb1bc5d 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -203,7 +203,6 @@ def revision_get(revision_id, session=None): id=revision_id).one().to_dict() except sa_orm.exc.NoResultFound: raise errors.RevisionNotFound(revision=revision_id) - return revision @@ -211,22 +210,7 @@ def revision_get_all(session=None): """Return list of all revisions.""" session = session or get_session() revisions = session.query(models.Revision).all() - revisions_resp = [r.to_dict() for r in revisions] - resp_body = { - 'count': len(revisions_resp), - 'next': None, - 'prev': None, - 'revisions': [] - } - - for revision in revisions_resp: - result = {} - for attr in ('id', 'created_at'): - result[utils.to_camel_case(attr)] = revision[attr] - result['count'] = len(revision.pop('documents')) - resp_body['revisions'].append(result) - - return resp_body + return [r.to_dict() for r in revisions] def revision_get_documents(revision_id, session=None, **filters): diff --git a/deckhand/tests/test_utils.py b/deckhand/tests/test_utils.py index f3812548..8a0e9f39 100644 --- a/deckhand/tests/test_utils.py +++ b/deckhand/tests/test_utils.py @@ -16,15 +16,6 @@ import random import uuid -def rand_uuid(): - """Generate a random UUID string - - :return: a random UUID (e.g. '1dc12c7d-60eb-4b61-a7a2-17cf210155b6') - :rtype: string - """ - return uuidutils.generate_uuid() - - def rand_uuid_hex(): """Generate a random UUID hex string @@ -60,3 +51,12 @@ def rand_bool(): :rtype: boolean """ return random.choice([True, False]) + + +def rand_int(min, max): + """Generate a random integer value between range (`min`, `max`). + + :return: a random integer between the range(`min`, `max`). + :rtype: integer + """ + return random.randint(min, max) diff --git a/deckhand/tests/unit/db/test_revisions.py b/deckhand/tests/unit/db/test_revisions.py index 1bfd4fe2..91f0dc14 100644 --- a/deckhand/tests/unit/db/test_revisions.py +++ b/deckhand/tests/unit/db/test_revisions.py @@ -15,18 +15,14 @@ from deckhand.tests.unit.db import base -class TestRevisions(base.TestDbBase): +class TestRevisionViews(base.TestDbBase): - def test_list_revisions(self): + def test_list(self): payload = [base.DocumentFixture.get_minimal_fixture() for _ in range(4)] self._create_documents(payload) revisions = self._list_revisions() - self.assertIsInstance(revisions, dict) - self.assertIn('revisions', revisions) - self.assertIsInstance(revisions['revisions'], list) - - revisions = revisions['revisions'] + self.assertIsInstance(revisions, list) self.assertEqual(1, len(revisions)) - self.assertEqual(4, revisions[0]["count"]) + self.assertEqual(4, len(revisions[0]['documents'])) diff --git a/deckhand/tests/unit/views/__init__.py b/deckhand/tests/unit/views/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/deckhand/tests/unit/views/test_views.py b/deckhand/tests/unit/views/test_views.py new file mode 100644 index 00000000..e186b0a6 --- /dev/null +++ b/deckhand/tests/unit/views/test_views.py @@ -0,0 +1,66 @@ +# 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. + +from deckhand.control.views import revision +from deckhand.tests.unit.db import base +from deckhand.tests import test_utils + + +class TestRevisionViews(base.TestDbBase): + + def setUp(self): + super(TestRevisionViews, self).setUp() + self.view_builder = revision.ViewBuilder() + + def test_list_revisions(self): + payload = [base.DocumentFixture.get_minimal_fixture() + for _ in range(4)] + self._create_documents(payload) + revisions = self._list_revisions() + revisions_view = self.view_builder.list(revisions) + + expected_attrs = ('next', 'prev', 'results', 'count') + for attr in expected_attrs: + self.assertIn(attr, revisions_view) + # Validate that only 1 revision was returned. + self.assertEqual(1, revisions_view['count']) + # Validate that the first revision has 4 documents. + self.assertIn('id', revisions_view['results'][0]) + self.assertIn('count', revisions_view['results'][0]) + self.assertEqual(4, revisions_view['results'][0]['count']) + + def test_list_many_revisions(self): + docs_count = [] + for _ in range(3): + doc_count = test_utils.rand_int(3, 9) + docs_count.append(doc_count) + + payload = [base.DocumentFixture.get_minimal_fixture() + for _ in range(doc_count)] + self._create_documents(payload) + revisions = self._list_revisions() + revisions_view = self.view_builder.list(revisions) + + expected_attrs = ('next', 'prev', 'results', 'count') + for attr in expected_attrs: + self.assertIn(attr, revisions_view) + # Validate that only 1 revision was returned. + self.assertEqual(3, revisions_view['count']) + + # Validate that each revision has correct number of documents. + for idx, doc_count in enumerate(docs_count): + self.assertIn('count', revisions_view['results'][idx]) + self.assertIn('id', revisions_view['results'][idx]) + self.assertEqual(doc_count, revisions_view['results'][idx][ + 'count']) diff --git a/deckhand/utils.py b/deckhand/utils.py index 18fba501..0e44eefd 100644 --- a/deckhand/utils.py +++ b/deckhand/utils.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import string - def multi_getattr(multi_key, dict_data): """Iteratively check for nested attributes in the YAML data. @@ -47,8 +45,3 @@ def multi_getattr(multi_key, dict_data): data = data.get(attr) return data - - -def to_camel_case(s): - return (s[0].lower() + string.capwords(s, sep='_').replace('_', '')[1:] - if s else s) From 441f3afd9f7f170b6b12dcb12c43bfd8484dd334 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 20:10:32 +0100 Subject: [PATCH 21/23] Fix naming conflict error. --- deckhand/control/revisions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 59321d34..9c73e146 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -30,8 +30,8 @@ class RevisionsResource(api_base.BaseResource): of each revision. """ revisions = db_api.revision_get_all() - resp = revision_view.ViewBuilder().list(revisions) + revisions_view = revision_view.ViewBuilder().list(revisions) resp.status = falcon.HTTP_200 resp.append_header('Content-Type', 'application/x-yaml') - resp.body = self.to_yaml_body(revisions) + resp.body = self.to_yaml_body(revisions_view) From cbb09bd1edd0d29e35430898c32c80c01953d320 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 21:13:39 +0100 Subject: [PATCH 22/23] Add endpoint/tests for GET /revisions/{revision_id} --- deckhand/control/api.py | 1 + deckhand/control/common.py | 5 +++++ deckhand/control/revisions.py | 23 ++++++++++++++++++++--- deckhand/control/views/revision.py | 9 +++++++++ deckhand/tests/unit/control/test_api.py | 18 ++++++++++++++---- deckhand/tests/unit/views/test_views.py | 12 ++++++++++++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/deckhand/control/api.py b/deckhand/control/api.py index 15d3257f..24b6a608 100644 --- a/deckhand/control/api.py +++ b/deckhand/control/api.py @@ -71,6 +71,7 @@ def start_api(state_manager=None): v1_0_routes = [ ('documents', documents.DocumentsResource()), ('revisions', revisions.RevisionsResource()), + ('revisions/{revision_id}', revisions.RevisionResource()), ('revisions/{revision_id}/documents', revision_documents.RevisionDocumentsResource()), ('secrets', secrets.SecretsResource()) diff --git a/deckhand/control/common.py b/deckhand/control/common.py index a2c26dd5..67c214b7 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -24,3 +24,8 @@ class ViewBuilder(object): """Model API responses as dictionaries.""" _collection_name = None + + def _gen_url(self, revision): + # TODO: Use a config-based url for the base url below. + base_url = 'https://deckhand/api/v1.0/%s/%s' + return base_url % (self._collection_name, revision.get('id')) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 9c73e146..5b0fcae6 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -20,7 +20,7 @@ from deckhand.db.sqlalchemy import api as db_api class RevisionsResource(api_base.BaseResource): - """API resource for realizing CRUD endpoints for Revisions.""" + """API resource for realizing GET /revisions.""" def on_get(self, req, resp): """Returns list of existing revisions. @@ -30,8 +30,25 @@ class RevisionsResource(api_base.BaseResource): of each revision. """ revisions = db_api.revision_get_all() - revisions_view = revision_view.ViewBuilder().list(revisions) + revisions_resp = revision_view.ViewBuilder().list(revisions) resp.status = falcon.HTTP_200 resp.append_header('Content-Type', 'application/x-yaml') - resp.body = self.to_yaml_body(revisions_view) + resp.body = self.to_yaml_body(revisions_resp) + + +class RevisionResource(api_base.BaseResource): + """API resource for realizing GET /revisions/{revision_id}.""" + + def on_get(self, req, resp, revision_id): + """Returns detailed description of a particular revision. + + The status of each ValidationPolicy belonging to the revision is also + included. + """ + revision = db_api.revision_get(revision_id) + revision_resp = revision_view.ViewBuilder().show(revision) + + resp.status = falcon.HTTP_200 + resp.append_header('Content-Type', 'application/x-yaml') + resp.body = self.to_yaml_body(revision_resp) diff --git a/deckhand/control/views/revision.py b/deckhand/control/views/revision.py index 32270072..095b2c88 100644 --- a/deckhand/control/views/revision.py +++ b/deckhand/control/views/revision.py @@ -36,3 +36,12 @@ class ViewBuilder(common.ViewBuilder): resp_body['results'].append(result) return resp_body + + def show(self, revision): + return { + 'id': revision.get('id'), + 'createdAt': revision.get('created_at'), + 'url': self._gen_url(revision), + # TODO: Not yet implemented. + 'validationPolicies': [], + } diff --git a/deckhand/tests/unit/control/test_api.py b/deckhand/tests/unit/control/test_api.py index ddbac81c..0346974c 100644 --- a/deckhand/tests/unit/control/test_api.py +++ b/deckhand/tests/unit/control/test_api.py @@ -30,10 +30,18 @@ class TestApi(testtools.TestCase): super(TestApi, self).setUp() for resource in (documents, revisions, revision_documents, secrets): resource_name = resource.__name__.split('.')[-1] - resource_obj = mock.patch.object( - resource, '%sResource' % resource_name.title().replace('_', '') - ).start() - setattr(self, '%s_resource' % resource_name, resource_obj) + + # Mock the singular/plural version. + resource_names = [resource_name, resource_name[:-1]] + for resource_name in resource_names: + try: + resource_obj = mock.patch.object( + resource, '%sResource' % resource_name.title().replace( + '_', '')).start() + setattr(self, '%s_resource' % resource_name, resource_obj) + except AttributeError: + # If a resource doesn't exist, ignore. + pass @mock.patch.object(api, 'db_api', autospec=True) @mock.patch.object(api, 'config', autospec=True) @@ -50,6 +58,8 @@ class TestApi(testtools.TestCase): mock_falcon_api.add_route.assert_has_calls([ mock.call('/api/v1.0/documents', self.documents_resource()), mock.call('/api/v1.0/revisions', self.revisions_resource()), + mock.call('/api/v1.0/revisions/{revision_id}', + self.revision_resource()), mock.call('/api/v1.0/revisions/{revision_id}/documents', self.revision_documents_resource()), mock.call('/api/v1.0/secrets', self.secrets_resource()) diff --git a/deckhand/tests/unit/views/test_views.py b/deckhand/tests/unit/views/test_views.py index e186b0a6..99d0d80c 100644 --- a/deckhand/tests/unit/views/test_views.py +++ b/deckhand/tests/unit/views/test_views.py @@ -64,3 +64,15 @@ class TestRevisionViews(base.TestDbBase): self.assertIn('id', revisions_view['results'][idx]) self.assertEqual(doc_count, revisions_view['results'][idx][ 'count']) + + def test_show_revision(self): + payload = [base.DocumentFixture.get_minimal_fixture() + for _ in range(4)] + documents = self._create_documents(payload) + revision = self._get_revision(documents[0]['revision_id']) + revision_view = self.view_builder.show(revision) + + expected_attrs = ('id', 'url', 'createdAt', 'validationPolicies') + for attr in expected_attrs: + self.assertIn(attr, revision_view) + self.assertIsInstance(revision_view['validationPolicies'], list) From fa7a84884723cba7bde83f094b5bfa2e82ce2ac3 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 31 Jul 2017 21:42:41 +0100 Subject: [PATCH 23/23] Refactor some code. --- deckhand/control/api.py | 2 +- deckhand/control/revisions.py | 44 ++++++++++++++----------- deckhand/tests/unit/control/test_api.py | 18 +++------- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/deckhand/control/api.py b/deckhand/control/api.py index 24b6a608..e36815ec 100644 --- a/deckhand/control/api.py +++ b/deckhand/control/api.py @@ -71,7 +71,7 @@ def start_api(state_manager=None): v1_0_routes = [ ('documents', documents.DocumentsResource()), ('revisions', revisions.RevisionsResource()), - ('revisions/{revision_id}', revisions.RevisionResource()), + ('revisions/{revision_id}', revisions.RevisionsResource()), ('revisions/{revision_id}/documents', revision_documents.RevisionDocumentsResource()), ('secrets', secrets.SecretsResource()) diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 5b0fcae6..e69031d2 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -17,38 +17,44 @@ import falcon from deckhand.control import base as api_base from deckhand.control.views import revision as revision_view from deckhand.db.sqlalchemy import api as db_api +from deckhand import errors class RevisionsResource(api_base.BaseResource): - """API resource for realizing GET /revisions.""" + """API resource for realizing CRUD operations for revisions.""" - def on_get(self, req, resp): + def on_get(self, req, resp, revision_id=None): """Returns list of existing revisions. Lists existing revisions and reports basic details including a summary of validation status for each `deckhand/ValidationPolicy` that is part of each revision. """ + if revision_id: + self._show_revision(req, resp, revision_id=revision_id) + else: + self._list_revisions(req, resp) + + def _show_revision(self, req, resp, revision_id): + """Returns detailed description of a particular revision. + + The status of each ValidationPolicy belonging to the revision is also + included. + """ + try: + revision = db_api.revision_get(revision_id) + except errors.RevisionNotFound as e: + return self.return_error(resp, falcon.HTTP_404, message=e) + + revision_resp = revision_view.ViewBuilder().show(revision) + resp.status = falcon.HTTP_200 + resp.append_header('Content-Type', 'application/x-yaml') + resp.body = self.to_yaml_body(revision_resp) + + def _list_revisions(self, req, resp): revisions = db_api.revision_get_all() revisions_resp = revision_view.ViewBuilder().list(revisions) resp.status = falcon.HTTP_200 resp.append_header('Content-Type', 'application/x-yaml') resp.body = self.to_yaml_body(revisions_resp) - - -class RevisionResource(api_base.BaseResource): - """API resource for realizing GET /revisions/{revision_id}.""" - - def on_get(self, req, resp, revision_id): - """Returns detailed description of a particular revision. - - The status of each ValidationPolicy belonging to the revision is also - included. - """ - revision = db_api.revision_get(revision_id) - revision_resp = revision_view.ViewBuilder().show(revision) - - resp.status = falcon.HTTP_200 - resp.append_header('Content-Type', 'application/x-yaml') - resp.body = self.to_yaml_body(revision_resp) diff --git a/deckhand/tests/unit/control/test_api.py b/deckhand/tests/unit/control/test_api.py index 0346974c..026c52c5 100644 --- a/deckhand/tests/unit/control/test_api.py +++ b/deckhand/tests/unit/control/test_api.py @@ -30,18 +30,10 @@ class TestApi(testtools.TestCase): super(TestApi, self).setUp() for resource in (documents, revisions, revision_documents, secrets): resource_name = resource.__name__.split('.')[-1] - - # Mock the singular/plural version. - resource_names = [resource_name, resource_name[:-1]] - for resource_name in resource_names: - try: - resource_obj = mock.patch.object( - resource, '%sResource' % resource_name.title().replace( - '_', '')).start() - setattr(self, '%s_resource' % resource_name, resource_obj) - except AttributeError: - # If a resource doesn't exist, ignore. - pass + resource_obj = mock.patch.object( + resource, '%sResource' % resource_name.title().replace( + '_', '')).start() + setattr(self, '%s_resource' % resource_name, resource_obj) @mock.patch.object(api, 'db_api', autospec=True) @mock.patch.object(api, 'config', autospec=True) @@ -59,7 +51,7 @@ class TestApi(testtools.TestCase): mock.call('/api/v1.0/documents', self.documents_resource()), mock.call('/api/v1.0/revisions', self.revisions_resource()), mock.call('/api/v1.0/revisions/{revision_id}', - self.revision_resource()), + self.revisions_resource()), mock.call('/api/v1.0/revisions/{revision_id}/documents', self.revision_documents_resource()), mock.call('/api/v1.0/secrets', self.secrets_resource())