From 2155a3f5bdc3e2c872ecd7f375ee5ef049a1ae6c Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 29 Nov 2017 03:11:51 +0000 Subject: [PATCH] Fix up tags attribute in revisions API Currently the GET /revisions endpoint returns the tags associated with each revision like: tags: foo: name: foo Which is strange, because the name of the tag is recursively referenced also as `name: foo`. Thus, this PS re-formats this response body to be more sensible: tags: foo: {} Where the key is the name of the tag and the value is the data, if any, associated with the tag. If no data is associated, the dict is empty. This PS also makes necessary documentation and functional test updates. Change-Id: I3156a539da11c40fee403806b3bd72b62d5b555d --- deckhand/control/views/revision.py | 11 ++-- deckhand/control/views/revision_tag.py | 8 ++- .../gabbits/revision-tag-success.yaml | 57 ++++++++++++------- .../unit/views/test_revision_tag_views.py | 9 ++- doc/source/api_ref.rst | 10 +++- 5 files changed, 56 insertions(+), 39 deletions(-) diff --git a/deckhand/control/views/revision.py b/deckhand/control/views/revision.py index 569309df..66da661b 100644 --- a/deckhand/control/views/revision.py +++ b/deckhand/control/views/revision.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections + from deckhand.control import common from deckhand import types from deckhand import utils @@ -52,10 +54,7 @@ class ViewBuilder(common.ViewBuilder): Each revision's documents should only be validation policies. """ validation_policies = [] - # TODO(fmontei): For the time being we're only returning the tag name, - # but eventually we'll return data associated with the tag, which is - # why this is a dictionary, not a list. - tags = {} + tags = collections.OrderedDict() success_status = 'success' for vp in [d for d in revision['documents'] @@ -75,7 +74,7 @@ class ViewBuilder(common.ViewBuilder): success_status = 'failed' for tag in revision['tags']: - tags.setdefault(tag['tag'], {'name': tag['tag']}) + tags.setdefault(tag['tag'], tag['data']) buckets = sorted( set([d['bucket_name'] for d in revision['documents']])) @@ -86,6 +85,6 @@ class ViewBuilder(common.ViewBuilder): 'url': self._gen_url(revision), 'validationPolicies': validation_policies, 'status': success_status, - 'tags': tags, + 'tags': dict(tags), 'buckets': buckets } diff --git a/deckhand/control/views/revision_tag.py b/deckhand/control/views/revision_tag.py index 56d90b43..9dab4829 100644 --- a/deckhand/control/views/revision_tag.py +++ b/deckhand/control/views/revision_tag.py @@ -21,13 +21,15 @@ class ViewBuilder(common.ViewBuilder): _collection_name = 'revisions' def list(self, tags): - return [self._show(tag) for tag in tags] + resp = {} + for tag in tags: + resp.update(self._show(tag)) + return resp def show(self, tag): return self._show(tag) def _show(self, tag): return { - 'tag': tag.get('tag', None), - 'data': tag.get('data', {}) + tag['tag']: tag.get('data', {}) } diff --git a/deckhand/tests/functional/gabbits/revision-tag-success.yaml b/deckhand/tests/functional/gabbits/revision-tag-success.yaml index 030b4ee0..d90f8d81 100644 --- a/deckhand/tests/functional/gabbits/revision-tag-success.yaml +++ b/deckhand/tests/functional/gabbits/revision-tag-success.yaml @@ -49,9 +49,8 @@ tests: status: 201 response_multidoc_jsonpaths: - $.`len`: 1 - $[0].data: {} - $[0].tag: foo + $.[0].`len`: 1 + $[0].foo: {} - name: show_tag desc: Verify showing created tag works @@ -60,8 +59,7 @@ tests: response_multidoc_jsonpaths: $.`len`: 1 - $[0].data: {} - $[0].tag: foo + $[0].foo: {} - name: verify_revision_detail_foo desc: Verify showing created tag on revision detail @@ -69,7 +67,9 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].tags.foo.name: foo + $.[0].tags.`len`: 1 + $.[0].tags: + foo: {} - name: verify_revision_list_foo desc: Verify showing created tag on revision list @@ -77,6 +77,7 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 + $.[0].results[0].tags.`len`: 1 $.[0].results[0].tags: [foo] - name: create_tag_with_data @@ -87,9 +88,10 @@ tests: response_multidoc_jsonpaths: $.`len`: 1 - $[0].tag: bar - $[0].data.last: good - $[0].data.random: data + $.[0].bar.`len`: 2 + $[0].bar: + last: good + random: data - name: list_tags desc: Verify listing tags contains created tag @@ -97,12 +99,13 @@ tests: status: 200 response_multidoc_jsonpaths: - $.`len`: 2 - $.[0].tag: bar - $.[0].data.last: good - $.[0].data.random: data - $.[1].tag: foo - $.[1].data: {} + $.`len`: 1 + $.[0].`len`: 2 + $.[0].bar.`len`: 2 + $[0].bar: + last: good + random: data + $[0].foo: {} - name: verify_revision_detail_bar desc: Verify showing created tag on revision detail @@ -110,8 +113,11 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].tags.bar.name: bar - $.[0].tags.foo.name: foo + $.[0].tags.`len`: 2 + $[0].tags.bar: + last: good + random: data + $[0].tags.foo: {} - name: verify_revision_list_bar desc: Verify showing created tag on revision list @@ -119,6 +125,7 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 + $.[0].results[0].tags.`len`: 2 $.[0].results[0].tags: [bar, foo] - name: delete_tag @@ -134,9 +141,10 @@ tests: response_multidoc_jsonpaths: $.`len`: 1 - $.[0].tag: bar - $.[0].data.last: good - $.[0].data.random: data + $.[0].`len`: 1 + $[0].bar: + last: good + random: data - name: verify_revision_detail_deleted_foo desc: Verify not showing deleted tag on revision detail @@ -144,7 +152,10 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].tags.bar.name: bar + $.[0].tags.`len`: 1 + $[0].tags.bar: + last: good + random: data - name: verify_revision_list_deleted_foo desc: Verify not showing deleted tag on revision list @@ -152,6 +163,7 @@ tests: status: 200 response_multidoc_jsonpaths: $.`len`: 1 + $.[0].results[0].tags.`len`: 1 $.[0].results[0].tags: [bar] - name: delete_all_tags @@ -182,4 +194,5 @@ tests: status: 200 response_multidoc_jsonpaths: - $: null + $.`len`: 1 + $.[0]: {} diff --git a/deckhand/tests/unit/views/test_revision_tag_views.py b/deckhand/tests/unit/views/test_revision_tag_views.py index 8ae9054d..47592a5e 100644 --- a/deckhand/tests/unit/views/test_revision_tag_views.py +++ b/deckhand/tests/unit/views/test_revision_tag_views.py @@ -30,7 +30,7 @@ class TestRevisionViews(base.TestDbBase): tag = rand_prefix + '-Tag' data_key = rand_prefix + '-Key' data_val = rand_prefix + '-Val' - expected_view = {'tag': tag, 'data': {data_key: data_val}} + expected_view = {tag: {data_key: data_val}} created_tag = db_api.revision_tag_create( self.revision_id, tag, {data_key: data_val}) @@ -39,7 +39,7 @@ class TestRevisionViews(base.TestDbBase): self.assertEqual(expected_view, actual_view) def test_revision_tag_list_view(self): - expected_view = [] + expected_view = {} # Create 2 revision tags for the same revision. for _ in range(2): @@ -51,10 +51,9 @@ class TestRevisionViews(base.TestDbBase): db_api.revision_tag_create( self.revision_id, tag, {data_key: data_val}) - expected_view.append({'tag': tag, 'data': {data_key: data_val}}) + expected_view.update({tag: {data_key: data_val}}) retrieved_tags = db_api.revision_tag_get_all(self.revision_id) actual_view = self.view_builder.list(retrieved_tags) - self.assertEqual(sorted(expected_view, key=lambda t: t['tag']), - actual_view) + self.assertEqual(expected_view, actual_view) diff --git a/doc/source/api_ref.rst b/doc/source/api_ref.rst index 3cf4dea0..1273debc 100644 --- a/doc/source/api_ref.rst +++ b/doc/source/api_ref.rst @@ -133,7 +133,8 @@ Sample response: url: https://deckhand/api/v1.0/revisions/1 createdAt: 2017-07-14T21:23Z buckets: [mop] - tags: [a, b, c] + tags: + a: {} validationPolicies: site-deploy-validation: status: failure @@ -141,7 +142,10 @@ Sample response: url: https://deckhand/api/v1.0/revisions/2 createdAt: 2017-07-16T01:15Z buckets: [flop, mop] - tags: [b] + tags: + b: + random: stuff + foo: bar validationPolicies: site-deploy-validation: status: success @@ -177,7 +181,7 @@ Sample response: buckets: [mop] tags: a: - name: a + random: stuff url: https://deckhand/api/v1.0/revisions/1/tags/a validationPolicies: site-deploy-validation: