From db9b91aecc63657c7f52d68f8ab3146e9839fccd Mon Sep 17 00:00:00 2001 From: Yi Feng Date: Thu, 24 Jun 2021 15:27:51 +0900 Subject: [PATCH] Fix create duplicated subscriptions When creating a duplicated subscription, according to spec SOL003, the api should return 303 See Other instead of 201 Created. To fix this error, the following things is done in this patch: 1.Fix the error when the database virtual field extracts the value from json in table `vnf_lcm_filters`. 2.Fix the error of sql query statement when querying subscription. 3.The attribute VnfProductsFromProviders should be an array, not an object. Fix this error in UT, FT and schemas. 4.Fix typos when writing "object" as "objects" in dict "_versions" in vnf_lcm schemas. 5.Rewrite the throwing and catching of SeeOther exception of this api to fix the following errors: SeeOther should be thrown instead of Exception; the return body should be empty instead of ProblemDetail; the return header should contain location instead of link. Closes-Bug: #1933169 Change-Id: I51c198d60001dba94dd369f495cecef526e79800 --- tacker/api/schemas/vnf_lcm.py | 12 ++--- tacker/api/vnflcm/v1/controller.py | 23 +++------ ...a2_alter_vnfd_ids_vnf_instance_ids_vnf_.py | 51 +++++++++++++++++++ .../alembic_migrations/versions/HEAD | 2 +- tacker/objects/vnf_lcm_subscriptions.py | 48 ++++++++++------- .../functional/sol/vnflcm/fake_vnflcm.py | 4 +- tacker/tests/unit/vnflcm/test_controller.py | 4 +- 7 files changed, 101 insertions(+), 43 deletions(-) create mode 100644 tacker/db/migration/alembic_migrations/versions/70df18f71ba2_alter_vnfd_ids_vnf_instance_ids_vnf_.py diff --git a/tacker/api/schemas/vnf_lcm.py b/tacker/api/schemas/vnf_lcm.py index ab40daf64..ba96cb5ea 100644 --- a/tacker/api/schemas/vnf_lcm.py +++ b/tacker/api/schemas/vnf_lcm.py @@ -187,7 +187,7 @@ _vimConnectionInfo = { _versions = { 'type': 'array', 'items': { - 'type': 'objects', + 'type': 'object', 'properties': { 'vnfSoftwareVersion': {'type': 'string'}, 'vnfdVersions': { @@ -212,15 +212,15 @@ _vnf_products = { } _vnf_products_from_providers = { - 'type': 'object', - 'properties': { + 'type': 'array', + 'items': { 'type': 'object', 'properties': { 'vnfProvider': {'type': 'string'}, 'vnfProducts': _vnf_products - } - }, - 'required': ['vnfProvider'] + }, + 'required': ['vnfProvider'] + } } _lifecycle_change_notifications_filter = { diff --git a/tacker/api/vnflcm/v1/controller.py b/tacker/api/vnflcm/v1/controller.py index 5d3780551..105b63a5c 100644 --- a/tacker/api/vnflcm/v1/controller.py +++ b/tacker/api/vnflcm/v1/controller.py @@ -968,21 +968,14 @@ class VnfLcmController(wsgi.Controller): vnf_lcm_subscription = vnf_lcm_subscription.create(filter) LOG.debug("vnf_lcm_subscription %s" % vnf_lcm_subscription) except exceptions.SeeOther as e: - if re.search("^303", str(e)): - res = self._make_problem_detail( - "See Other", 303, title='See Other') - link = ( - 'LINK', - CONF.vnf_lcm.endpoint_url.rstrip("/") + - "/vnflcm/v1/subscriptions/" + - str(e)[ - 3:]) - res.headerlist.append(link) - return res - else: - LOG.error(traceback.format_exc()) - return self._make_problem_detail( - str(e), 500, title='Internal Server Error') + res = webob.Response(content_type='application/json') + res.status_int = http_client.SEE_OTHER.value + location = ( + 'Location', + CONF.vnf_lcm.endpoint_url.rstrip("/") + + "/vnflcm/v1/subscriptions/" + str(e)) + res.headerlist.append(location) + return res result = self._view_builder.subscription_create(vnf_lcm_subscription, filter) diff --git a/tacker/db/migration/alembic_migrations/versions/70df18f71ba2_alter_vnfd_ids_vnf_instance_ids_vnf_.py b/tacker/db/migration/alembic_migrations/versions/70df18f71ba2_alter_vnfd_ids_vnf_instance_ids_vnf_.py new file mode 100644 index 000000000..ea3b8a128 --- /dev/null +++ b/tacker/db/migration/alembic_migrations/versions/70df18f71ba2_alter_vnfd_ids_vnf_instance_ids_vnf_.py @@ -0,0 +1,51 @@ +# Copyright 2021 OpenStack Foundation +# +# 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. +# + +"""alter vnfd_ids, vnf_instance_ids, vnf_instance_names +columns of vnf_lcm_filters + +Revision ID: 70df18f71ba2 +Revises: a23ebee909a8 +Create Date: 2021-06-16 14:26:51.998033 + +""" + +# revision identifiers, used by Alembic. +revision = '70df18f71ba2' +down_revision = 'a23ebee909a8' + +from alembic import op # noqa: E402 + + +def upgrade(active_plugins=None, options=None): + alter_sql_vnfd_ids = "ALTER TABLE vnf_lcm_filters CHANGE \ + vnfd_ids vnfd_ids mediumtext GENERATED ALWAYS AS \ + (json_unquote(json_extract(`filter`, \ + '$.vnfInstanceSubscriptionFilter.vnfdIds'))) VIRTUAL;" + + alter_sql_vnf_instance_ids = "ALTER TABLE vnf_lcm_filters CHANGE \ + vnf_instance_ids vnf_instance_ids mediumtext GENERATED ALWAYS AS \ + (json_unquote(json_extract(`filter`, \ + '$.vnfInstanceSubscriptionFilter.vnfInstanceIds'))) VIRTUAL;" + + alter_sql_vnf_instance_names = "ALTER TABLE vnf_lcm_filters CHANGE \ + vnf_instance_names vnf_instance_names mediumtext GENERATED \ + ALWAYS AS (json_unquote(json_extract(`filter`, \ + '$.vnfInstanceSubscriptionFilter.vnfInstanceNames'))) \ + VIRTUAL;" + + op.execute(alter_sql_vnfd_ids) + op.execute(alter_sql_vnf_instance_ids) + op.execute(alter_sql_vnf_instance_names) diff --git a/tacker/db/migration/alembic_migrations/versions/HEAD b/tacker/db/migration/alembic_migrations/versions/HEAD index 07b9ae264..c60df71fd 100644 --- a/tacker/db/migration/alembic_migrations/versions/HEAD +++ b/tacker/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -a23ebee909a8 +70df18f71ba2 diff --git a/tacker/objects/vnf_lcm_subscriptions.py b/tacker/objects/vnf_lcm_subscriptions.py index 628023e87..37e25c209 100644 --- a/tacker/objects/vnf_lcm_subscriptions.py +++ b/tacker/objects/vnf_lcm_subscriptions.py @@ -53,19 +53,24 @@ def _get_vnf_subscription_filter_values(vnf_subscription_filter): vnf_instance_names = vnf_subscription_filter.get('vnfInstanceNames', []) vnfd_products_from_providers = vnf_subscription_filter.get( - 'vnfProductsFromProviders', {}) - vnf_provider = vnfd_products_from_providers.get('vnfProvider', "") - vnf_products = vnfd_products_from_providers.get('vnfProducts', []) + 'vnfProductsFromProviders', []) + vnf_provider = "" vnf_product_name = "" vnf_software_version = "" vnfd_versions = [] - if vnf_products: - vnf_product_name = vnf_products[0].get('vnfProductName', "") - versions = vnf_products[0].get('versions', []) - if versions: - vnf_software_version = versions[0].get('vnfSoftwareVersion', "") - vnfd_versions = versions[0].get('vnfdVersions', []) + if vnfd_products_from_providers: + vnfd_products_from_providers = vnfd_products_from_providers[0] + vnf_provider = vnfd_products_from_providers.get('vnfProvider', "") + vnf_products = vnfd_products_from_providers.get('vnfProducts', []) + + if vnf_products: + vnf_product_name = vnf_products[0].get('vnfProductName', "") + versions = vnf_products[0].get('versions', []) + if versions: + vnf_software_version = \ + versions[0].get('vnfSoftwareVersion', "") + vnfd_versions = versions[0].get('vnfdVersions', []) vnf_subscription_array = [ {'vnfdIds': vnfd_ids}, @@ -233,19 +238,26 @@ def _vnf_lcm_subscriptions_id_get(context, column_list = _get_vnf_subscription_filter_values( vnf_instance_subscription_filter) + sql_lst = [sql] for column in column_list: for key in column: if key in VNF_INSTANCE_SUBSCRIPTION_FILTER: value = column[key] if key in VNF_INSTANCE_SUBSCRIPTION_FILTER_LISTS: - value = _make_list(value) + if value: + value = _make_list(value) + sql_lst.append( + " JSON_CONTAINS({}, '{}') and ".format( + convert_string_to_snakecase(key), value)) + else: + sql_lst.append(" {}_len=0 and ".format( + convert_string_to_snakecase(key))) else: - value = '"{}"'.format(value) - sql = (sql + " JSON_CONTAINS({}, '{}') and ".format( - convert_string_to_snakecase(key), - value - )) + sql_lst.append(" {}='{}' and ".format( + convert_string_to_snakecase(key), value)) + included_in_filter.append(key) + sql = ''.join(sql_lst) not_included_in_filter = list( set(VNF_INSTANCE_SUBSCRIPTION_FILTER_LISTS) - @@ -302,6 +314,8 @@ def _add_filter_data(context, subscription_id, filter): vnf_products_from_providers = \ vnf_instance_subscription_filter.get( 'vnfProductsFromProviders') + if vnf_products_from_providers: + vnf_products_from_providers = vnf_products_from_providers[0] new_entries = [] new_entries.append({"subscription_uuid": subscription_id, @@ -387,7 +401,7 @@ def _vnf_lcm_subscriptions_create(context, values, filter): vnf_instance_subscription_filter=subscription_filter) if vnf_lcm_subscriptions_id: - raise Exception("303" + vnf_lcm_subscriptions_id) + raise exceptions.SeeOther(message=vnf_lcm_subscriptions_id.id) _add_filter_data(context, values.id, filter) @@ -396,7 +410,7 @@ def _vnf_lcm_subscriptions_create(context, values, filter): callbackUri) if vnf_lcm_subscriptions_id: - raise Exception("303" + vnf_lcm_subscriptions_id.id) + raise exceptions.SeeOther(message=vnf_lcm_subscriptions_id.id) _add_filter_data(context, values.id, {}) return values diff --git a/tacker/tests/functional/sol/vnflcm/fake_vnflcm.py b/tacker/tests/functional/sol/vnflcm/fake_vnflcm.py index 6fa57314b..482cee18a 100644 --- a/tacker/tests/functional/sol/vnflcm/fake_vnflcm.py +++ b/tacker/tests/functional/sol/vnflcm/fake_vnflcm.py @@ -32,7 +32,7 @@ class Subscription: "filter": { "vnfInstanceSubscriptionFilter": { "vnfdIds": ["b1bb0ce7-ebca-4fa7-95ed-4840d7000000"], - "vnfProductsFromProviders": { + "vnfProductsFromProviders": [{ "vnfProvider": "Company", "vnfProducts": [ { @@ -45,7 +45,7 @@ class Subscription: ] } ] - } + }] }, "notificationTypes": [ "VnfLcmOperationOccurrenceNotification", diff --git a/tacker/tests/unit/vnflcm/test_controller.py b/tacker/tests/unit/vnflcm/test_controller.py index 0956200df..e32707acf 100644 --- a/tacker/tests/unit/vnflcm/test_controller.py +++ b/tacker/tests/unit/vnflcm/test_controller.py @@ -3942,7 +3942,7 @@ class TestController(base.TestCase): 'notificationTypes': ['VnfLcmOperationOccurrenceNotification'], 'vnfInstanceSubscriptionFilter': { "vnfdIds": [], - "vnfProductsFromProviders": { + "vnfProductsFromProviders": [{ "vnfProvider": "Vnf Provider 1", "vnfProducts": [ { @@ -3955,7 +3955,7 @@ class TestController(base.TestCase): ] } ] - } + }] } } }