From: dyh Date: Wed, 19 Feb 2020 01:57:17 +0000 (+0800) Subject: return 404 instead of 500 if subscription does not exist X-Git-Tag: 1.0.5~13^2 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=modeling%2Fetsicatalog.git;a=commitdiff_plain;h=c0fcb2e4cde1cd5338412e5ce83115626d068f7b return 404 instead of 500 if subscription does not exist Change-Id: I88bd100439ad37e7c14ae74e8f93bd9569875a9c Issue-ID: MODELING-313 Signed-off-by: dyh --- diff --git a/catalog/packages/biz/notificationsutil.py b/catalog/packages/biz/notificationsutil.py index 114f922..72afe33 100644 --- a/catalog/packages/biz/notificationsutil.py +++ b/catalog/packages/biz/notificationsutil.py @@ -26,7 +26,6 @@ from django.db.models import Q from catalog.packages.serializers.vnf_pkg_notifications import PkgChangeNotificationSerializer, \ PkgOnboardingNotificationSerializer - logger = logging.getLogger(__name__) diff --git a/catalog/packages/biz/nsdm_subscription.py b/catalog/packages/biz/nsdm_subscription.py index 652e9a7..72eded6 100644 --- a/catalog/packages/biz/nsdm_subscription.py +++ b/catalog/packages/biz/nsdm_subscription.py @@ -15,18 +15,16 @@ import ast import json import logging -import requests import uuid - from collections import Counter +import requests from rest_framework import status from catalog.packages import const from catalog.pub.database.models import NsdmSubscriptionModel from catalog.pub.exceptions import CatalogException, \ - ResourceNotFoundException, \ - NsdmBadRequestException, NsdmDuplicateSubscriptionException + NsdmBadRequestException, NsdmDuplicateSubscriptionException, SubscriptionDoesNotExistsException from catalog.pub.utils.values import ignore_case_get logger = logging.getLogger(__name__) @@ -52,8 +50,8 @@ class NsdmSubscription: NsdmSubscriptionModel.objects.filter( subscriptionid=subscription_id) if not subscription.exists(): - raise ResourceNotFoundException( - "Subscription(%s) doesn't exists" % subscription_id) + raise SubscriptionDoesNotExistsException( + "Subscription(%s) doesn't exist" % subscription_id) logger.debug("Subscription found... ") return self.fill_resp_data(subscription[0]) @@ -63,8 +61,8 @@ class NsdmSubscription: NsdmSubscriptionModel.objects.filter( subscriptionid=subscription_id) if not subscription.exists(): - raise ResourceNotFoundException( - "Subscription(%s) doesn't exists" % subscription_id) + raise SubscriptionDoesNotExistsException( + "Subscription(%s) doesn't exist" % subscription_id) subscription.delete() logger.debug("Deleted Subscription... ") @@ -83,7 +81,7 @@ class NsdmSubscription: else: subscriptions = NsdmSubscriptionModel.objects.all() if not subscriptions.exists(): - raise ResourceNotFoundException("Subscriptions doesn't exist") + raise SubscriptionDoesNotExistsException("Subscriptions doesn't exist") return [self.fill_resp_data(subscription) for subscription in subscriptions] @@ -183,7 +181,7 @@ class NsdmSubscription: def check_valid(self): logger.debug("Create Subscription --> Checking DB if " - "same subscription exists already exists... ") + "same subscription has already existed... ") subscriptions = \ NsdmSubscriptionModel.objects.filter( callback_uri=self.callback_uri) @@ -192,7 +190,7 @@ class NsdmSubscription: for subscription in subscriptions: if self.check_filter_exists(subscription): raise NsdmDuplicateSubscriptionException( - "Already Subscription exists with the " + "Subscription has already existed with the " "same callbackUri and filter") def save_db(self): @@ -201,7 +199,7 @@ class NsdmSubscription: links = { "self": { "href": - const.NSDM_SUBSCRIPTION_ROOT_URI + self.subscription_id + const.NSDM_SUBSCRIPTION_ROOT_URI + self.subscription_id } } subscription_save_db = { diff --git a/catalog/packages/biz/vnf_pkg_subscription.py b/catalog/packages/biz/vnf_pkg_subscription.py index 6abe10e..c457bfe 100644 --- a/catalog/packages/biz/vnf_pkg_subscription.py +++ b/catalog/packages/biz/vnf_pkg_subscription.py @@ -24,11 +24,10 @@ from rest_framework import status from catalog.packages import const from catalog.pub.database.models import VnfPkgSubscriptionModel -from catalog.pub.exceptions import VnfPkgSubscriptionException,\ +from catalog.pub.exceptions import VnfPkgSubscriptionException, \ VnfPkgDuplicateSubscriptionException, SubscriptionDoesNotExistsException from catalog.pub.utils.values import ignore_case_get - logger = logging.getLogger(__name__) ROOT_FILTERS = { @@ -172,7 +171,7 @@ class QuerySubscription(object): subscription_id=subscription_id) if not subscription.exists(): raise SubscriptionDoesNotExistsException("Subscription with ID: %s " - "does not exists" % subscription_id) + "does not exist" % subscription_id) return subscription[0].toDict() @@ -186,5 +185,5 @@ class TerminateSubscription(object): subscription_id=subscription_id) if not subscription.exists(): raise SubscriptionDoesNotExistsException("Subscription with ID: %s " - "does not exists" % subscription_id) + "does not exist" % subscription_id) subscription[0].delete() diff --git a/catalog/packages/tests/test_nsdm_subscription.py b/catalog/packages/tests/test_nsdm_subscription.py index 862054b..0d73afd 100644 --- a/catalog/packages/tests/test_nsdm_subscription.py +++ b/catalog/packages/tests/test_nsdm_subscription.py @@ -158,7 +158,7 @@ class TestNsdmSubscription(TestCase): response.data["callbackUri"]) expected_data = { 'status': 303, - 'detail': 'Already Subscription exists with' + 'detail': 'Subscription has already existed with' ' the same callbackUri and filter' } response = self.client.post("/api/nsd/v1/subscriptions", @@ -465,7 +465,7 @@ class TestNsdmSubscription(TestCase): expected_data = { "status": 404, "detail": "Subscription(" + self.subscription_id + ") " - "doesn't exists" + "doesn't exist" } response = self.client.get('/api/nsd/v1/' 'subscriptions/' + self.subscription_id, diff --git a/catalog/packages/tests/test_vnf_pkg_subscription.py b/catalog/packages/tests/test_vnf_pkg_subscription.py index bc7ee49..d2e2b5b 100644 --- a/catalog/packages/tests/test_vnf_pkg_subscription.py +++ b/catalog/packages/tests/test_vnf_pkg_subscription.py @@ -12,23 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. -import uuid -import mock import json import os +import uuid -from rest_framework.test import APIClient +import mock from django.test import TestCase +from rest_framework import status +from rest_framework.test import APIClient -from catalog.pub.database.models import VnfPkgSubscriptionModel, VnfPackageModel -from .const import vnf_subscription_data, vnfd_data -from catalog.packages.biz.notificationsutil import PkgNotifications +import catalog.pub.utils.timeutil from catalog.packages import const +from catalog.packages.biz.notificationsutil import PkgNotifications +from catalog.packages.biz.vnf_pkg_subscription import QuerySubscription, TerminateSubscription from catalog.pub.config import config as pub_config -import catalog.pub.utils.timeutil -from catalog.pub.utils import toscaparser from catalog.pub.config.config import CATALOG_ROOT_PATH -from rest_framework import status +from catalog.pub.database.models import VnfPkgSubscriptionModel, VnfPackageModel +from catalog.pub.exceptions import SubscriptionDoesNotExistsException +from catalog.pub.utils import toscaparser +from .const import vnf_subscription_data, vnfd_data class TestNfPackageSubscription(TestCase): @@ -196,7 +198,8 @@ class TestNfPackageSubscription(TestCase): @mock.patch("requests.post") @mock.patch("uuid.uuid4") @mock.patch.object(catalog.pub.utils.timeutil, "now_time") - def test_vnfpkg_subscript_notify(self, mock_nowtime, mock_uuid, mock_requests_post, mock_parse_vnfd, mock_requests_get): + def test_vnfpkg_subscript_notify(self, mock_nowtime, mock_uuid, mock_requests_post, mock_parse_vnfd, + mock_requests_get): mock_nowtime.return_value = "2019-02-16 14:41:16" uuid_subscriptid = "99442b18-a5c7-11e8-998c-bf1755941f13" uuid_vnfPackageId = "3fa85f64-5717-4562-b3fc-2c963f66afa6" @@ -247,6 +250,20 @@ class TestNfPackageSubscription(TestCase): mock_requests_post.assert_called_with(vnf_subscription_data["callbackUri"], data=expect_notification, headers={'Connection': 'close'}) + def test_service_query_single_subscription_not_found(self): + try: + subscription_id = "test_not_found" + QuerySubscription().query_single_subscription(subscription_id) + except SubscriptionDoesNotExistsException as e: + self.assertEqual("Subscription with ID: %s does not exist" % subscription_id, e.args[0]) + + def test_service_delete_single_subscription_not_found(self): + try: + subscription_id = "test_not_found" + TerminateSubscription().terminate(subscription_id) + except SubscriptionDoesNotExistsException as e: + self.assertEqual("Subscription with ID: %s does not exist" % subscription_id, e.args[0]) + class NotificationTest(TestCase): def setUp(self): @@ -301,4 +318,5 @@ class NotificationTest(TestCase): } } } - mock_requests_post.assert_called_with(expect_callbackuri, data=expect_notification, headers={'Connection': 'close'}) + mock_requests_post.assert_called_with(expect_callbackuri, data=expect_notification, + headers={'Connection': 'close'}) diff --git a/catalog/packages/views/common.py b/catalog/packages/views/common.py index c074faf..12840a5 100644 --- a/catalog/packages/views/common.py +++ b/catalog/packages/views/common.py @@ -18,7 +18,7 @@ import logging from rest_framework import status from rest_framework.response import Response -from catalog.pub.exceptions import CatalogException +from catalog.pub.exceptions import CatalogException, SubscriptionDoesNotExistsException from catalog.pub.exceptions import BadRequestException from catalog.pub.exceptions import NsdmBadRequestException from catalog.pub.exceptions import PackageNotFoundException @@ -108,6 +108,12 @@ def view_safe_call_with_log(logger): detail=e.args[0], status=status.HTTP_400_BAD_REQUEST ) + except SubscriptionDoesNotExistsException as e: + logger.error(e.args[0]) + return make_error_resp( + detail=e.args[0], + status=status.HTTP_404_NOT_FOUND + ) except VnfPkgSubscriptionException as e: logger.error(e.args[0]) return make_error_resp( @@ -127,5 +133,7 @@ def view_safe_call_with_log(logger): detail='Unexpected exception', status=status.HTTP_500_INTERNAL_SERVER_ERROR ) + return wrapper + return view_safe_call