Merge "return 404 instead of 500 if subscription does not exist"

6 files changed, 53 insertions, 31 deletions

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