From 593fd452686e69503dcf68a68bd88d554667a226 Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Wed, 31 Aug 2016 17:25:10 +0200 Subject: [PATCH] Allow to update resource-type (delete attributes) This change extends the resource type patch to allow to delete attributes. Partial-Bug: #1615077 Change-Id: Iea24c075755a0e75f618ba8498515b2f8baa0a2c --- doc/source/rest.j2 | 2 +- doc/source/rest.yaml | 4 + gnocchi/indexer/sqlalchemy.py | 10 +- gnocchi/rest/__init__.py | 25 +++-- .../tests/gabbi/gabbits/resource-type.yaml | 95 +++++++++++++++++-- 5 files changed, 114 insertions(+), 22 deletions(-) diff --git a/doc/source/rest.j2 b/doc/source/rest.j2 index b8ad93626..aeee4a427 100644 --- a/doc/source/rest.j2 +++ b/doc/source/rest.j2 @@ -368,7 +368,7 @@ It can also be deleted if no more resources are associated to it: {{ scenarios['delete-resource-type']['doc'] }} -Attributes can be added: +Attributes can be added or removed: {{ scenarios['patch-resource-type']['doc'] }} diff --git a/doc/source/rest.yaml b/doc/source/rest.yaml index 122fb402c..19871d2dd 100644 --- a/doc/source/rest.yaml +++ b/doc/source/rest.yaml @@ -399,6 +399,10 @@ "op": "add", "path": "/attributes/awesome-stuff", "value": {"type": "bool", "required": false} + }, + { + "op": "remove", + "path": "/attributes/prefix" } ] diff --git a/gnocchi/indexer/sqlalchemy.py b/gnocchi/indexer/sqlalchemy.py index a03ac3902..02f16fb02 100644 --- a/gnocchi/indexer/sqlalchemy.py +++ b/gnocchi/indexer/sqlalchemy.py @@ -390,8 +390,9 @@ class SQLAlchemyIndexer(indexer.IndexerDriver): resource_type.state = "active" return resource_type - def update_resource_type(self, name, add_attributes=None): - if not add_attributes: + def update_resource_type(self, name, add_attributes=None, + del_attributes=None): + if not add_attributes and not del_attributes: return self._set_resource_type_state(name, "updating", "active") @@ -403,6 +404,8 @@ class SQLAlchemyIndexer(indexer.IndexerDriver): ctx = migration.MigrationContext.configure(connection) op = operations.Operations(ctx) with op.batch_alter_table(rt.tablename) as batch_op: + for attr in del_attributes: + batch_op.drop_column(attr) for attr in add_attributes: # TODO(sileht): When attr.required is True, we have # to pass a default. rest layer current protect us, @@ -414,6 +417,9 @@ class SQLAlchemyIndexer(indexer.IndexerDriver): rt.state = "active" rt.updated_at = utils.utcnow() rt.attributes.extend(add_attributes) + for attr in list(rt.attributes): + if attr.name in del_attributes: + rt.attributes.remove(attr) # FIXME(sileht): yeah that's wierd but attributes is a custom # json column and 'extend' doesn't trigger sql update, this # enforce the update. I wonder if sqlalchemy provides something diff --git a/gnocchi/rest/__init__.py b/gnocchi/rest/__init__.py index 70dad8fc4..f30afd195 100644 --- a/gnocchi/rest/__init__.py +++ b/gnocchi/rest/__init__.py @@ -758,11 +758,10 @@ def AttributesPath(value): raise ValueError("Only attributes can be modified") -# TODO(sileht): Implements delete op ResourceTypeJsonPatchSchema = voluptuous.Schema([{ - "op": "add", + "op": voluptuous.Any("add", "remove"), "path": AttributesPath, - "value": dict, + voluptuous.Optional("value"): dict, }]) @@ -811,26 +810,34 @@ class ResourceTypeController(rest.RestController): except voluptuous.Error as e: abort(400, "Invalid input: %s" % e) - # Get only newly formatted attributes - attrs = {k: v for k, v in rt_json_next["attributes"].items() - if k not in rt_json_current["attributes"]} + # Get only newly formatted and deleted attributes + add_attrs = {k: v for k, v in rt_json_next["attributes"].items() + if k not in rt_json_current["attributes"]} + del_attrs = [k for k in rt_json_current["attributes"] + if k not in rt_json_next["attributes"]] + + if not add_attrs and not del_attrs: + # NOTE(sileht): just returns the resource, the asked changes + # just do nothing + return rt try: - attrs = schema.attributes_from_dict(attrs) + add_attrs = schema.attributes_from_dict(add_attrs) except resource_type.InvalidResourceAttributeName as e: abort(400, e) # TODO(sileht): Add a default field on an attribute # to be able to fill non-nullable column on sql side. # And obviousy remove this limitation - for attr in attrs: + for attr in add_attrs: if attr.required: abort(400, ValueError("Adding required attributes is not yet " "possible.")) try: return pecan.request.indexer.update_resource_type( - self._name, add_attributes=attrs) + self._name, add_attributes=add_attrs, + del_attributes=del_attrs) except indexer.NoSuchResourceType as e: abort(400, e) diff --git a/gnocchi/tests/gabbi/gabbits/resource-type.yaml b/gnocchi/tests/gabbi/gabbits/resource-type.yaml index d7c6afe3a..6079f1e19 100644 --- a/gnocchi/tests/gabbi/gabbits/resource-type.yaml +++ b/gnocchi/tests/gabbi/gabbits/resource-type.yaml @@ -334,6 +334,8 @@ tests: required: False min_length: 0 max_length: 255 + - op: remove + path: /attributes/foobar status: 200 response_json_paths: $.name: my_custom_resource @@ -343,11 +345,6 @@ tests: required: True min_length: 2 max_length: 5 - foobar: - type: string - required: False - min_length: 0 - max_length: 255 uuid: type: uuid required: True @@ -380,11 +377,6 @@ tests: required: True min_length: 2 max_length: 5 - foobar: - type: string - required: False - min_length: 0 - max_length: 255 uuid: type: uuid required: True @@ -409,6 +401,73 @@ tests: # Invalid patch + - name: add/delete the same resource attribute + url: /v1/resource_type/my_custom_resource + method: patch + request_headers: + x-roles: admin + content-type: application/json-patch+json + data: + - op: add + path: /attributes/what + value: + type: string + required: False + min_length: 0 + max_length: 255 + - op: remove + path: /attributes/what + status: 200 + response_json_paths: + $.name: my_custom_resource + $.attributes: + name: + type: string + required: True + min_length: 2 + max_length: 5 + uuid: + type: uuid + required: True + int: + type: number + required: False + min: -2 + max: 3 + float: + type: number + required: false + min: -2.3 + max: + bool: + type: bool + required: false + newstuff: + type: string + required: False + min_length: 0 + max_length: 255 + + - name: delete/add the same resource attribute + url: /v1/resource_type/my_custom_resource + method: patch + request_headers: + x-roles: admin + content-type: application/json-patch+json + data: + - op: remove + path: /attributes/what + - op: add + path: /attributes/what + value: + type: string + required: False + min_length: 0 + max_length: 255 + status: 400 + response_strings: + - "can't remove non-existent object 'what'" + - name: patch a resource attribute replace url: /v1/resource_type/my_custom_resource method: patch @@ -424,6 +483,9 @@ tests: min_length: 0 max_length: 255 status: 400 + response_strings: + - "Invalid input: not a valid value for dictionary value @ data[0][" + - "'op']" - name: patch a resource attribute type not exist url: /v1/resource_type/my_custom_resource @@ -441,6 +503,19 @@ tests: max_length: 255 status: 400 + - name: patch a resource attribute type unknown + url: /v1/resource_type/my_custom_resource + method: patch + request_headers: + x-roles: admin + content-type: application/json-patch+json + data: + - op: remove + path: /attributes/unknown + status: 400 + response_strings: + - "can't remove non-existent object 'unknown'" + # Ensure we can't delete the type - name: delete in use resource_type