From 8a84a2ef10be51faf50b2880a3a94194d64459ba Mon Sep 17 00:00:00 2001 From: "Zhang Rong(Jon)" Date: Tue, 1 Nov 2022 21:18:06 +0800 Subject: [PATCH] Update error handling; update selector; change delete response code to 200 Issue-ID: INF-302 Issue-ID: INF-300 Signed-off-by: Zhang Rong(Jon) Change-Id: I0e7692fa5f2dd19907ddc75dc134e3500540720d --- o2app/entrypoints/flask_application.py | 3 ++ o2common/views/route.py | 43 +++++------------ o2common/views/route_exception.py | 87 ++++++++++++++++++++++++++++++++++ o2common/views/view.py | 5 +- o2ims/views/__init__.py | 4 +- o2ims/views/alarm_route.py | 15 +++--- o2ims/views/ocloud_dto.py | 27 +++++++---- o2ims/views/ocloud_route.py | 41 ++++++---------- tests/unit/test_alarm.py | 2 +- tests/unit/test_ocloud.py | 2 +- 10 files changed, 149 insertions(+), 80 deletions(-) create mode 100644 o2common/views/route_exception.py diff --git a/o2app/entrypoints/flask_application.py b/o2app/entrypoints/flask_application.py index c1f5c8a..0f9d3f4 100644 --- a/o2app/entrypoints/flask_application.py +++ b/o2app/entrypoints/flask_application.py @@ -18,6 +18,7 @@ from flask_restx import Api from o2app import bootstrap from o2ims.views import configure_namespace as ims_route_configure_namespace +from o2common.views.route_exception import configure_exception from o2ims.adapter.clients.alarm_dict_client import load_alarm_definition from o2common.authmw import authmiddleware @@ -51,12 +52,14 @@ if auth: app.config.SWAGGER_UI_DOC_EXPANSION = 'list' # app.config['RESTX_MASK_HEADER'] = 'fields' app.config['RESTX_MASK_SWAGGER'] = False +app.config['ERROR_INCLUDE_MESSAGE'] = False api = Api(app, version=FLASK_API_VERSION, title='INF O2 Services API', description='Swagger OpenAPI document for the INF O2 Services', ) bus = bootstrap.bootstrap() +configure_exception(api) ims_route_configure_namespace(api) load_alarm_definition(bus.uow) diff --git a/o2common/views/route.py b/o2common/views/route.py index 832e38c..0941f58 100644 --- a/o2common/views/route.py +++ b/o2common/views/route.py @@ -29,7 +29,6 @@ from flask_restx.mask import Mask # , apply as apply_mask from flask_restx.model import Model from flask_restx.fields import List, Nested, String from flask_restx.utils import unpack -from o2common.domain.base import Serializer from o2common.helper import o2logging logger = o2logging.get_logger(__name__) @@ -131,7 +130,9 @@ class o2_marshal_with(marshal_with): all_fields_without_space = kwargs['all_fields'].replace(" ", "") all_fields = all_fields_without_space.lower() if 'true' == all_fields: - mask_val = '' + selector = self.__gen_selector_from_model_with_value( + self.fields) + mask_val = self.__gen_mask_from_selector(selector) elif 'fields' in kwargs and kwargs['fields'] != '': fields_without_space = kwargs['fields'].replace(" ", "") @@ -149,6 +150,7 @@ class o2_marshal_with(marshal_with): selector = {} self.__update_selector_value(selector, fields_without_space, True) + self.__set_default_mask(selector) mask_val = self.__gen_mask_from_selector(selector) @@ -161,6 +163,7 @@ class o2_marshal_with(marshal_with): self.__update_selector_value( selector, exclude_fields_without_space, False) + self.__set_default_mask(selector) mask_val = self.__gen_mask_from_selector(selector) elif 'exclude_default' in kwargs and kwargs['exclude_default'] != '': @@ -201,14 +204,14 @@ class o2_marshal_with(marshal_with): selector[i] = default_val return selector - def __update_selector_value(self, default_selector: dict, filter: str, + def __update_selector_value(self, selector: dict, filter: str, val: bool): fields = filter.split(',') for f in fields: if '/' in f: - self.__update_selector_tree_value(default_selector, f, val) + self.__update_selector_tree_value(selector, f, val) continue - default_selector[f] = val + selector[f] = val def __update_selector_tree_value(self, m: dict, filter: str, val: bool): filter_list = filter.split('/', 1) @@ -232,30 +235,6 @@ class o2_marshal_with(marshal_with): return '{%s}' % ','.join(mask_li) - -class ProblemDetails(Serializer): - def __init__(self, namespace: O2Namespace, code: int, detail: str, - title=None, instance=None - ) -> None: - self.ns = namespace - self.status = code - self.detail = detail - self.type = request.path - self.title = title if title is not None else self.getTitle(code) - self.instance = instance if instance is not None else [] - - def getTitle(self, code): - return HTTPStatus(code).phrase - - def abort(self): - self.ns.abort(self.status, self.detail, **self.serialize()) - - def serialize(self): - details = {} - for key in dir(self): - if key == 'ns' or key.startswith('__') or\ - callable(getattr(self, key)): - continue - else: - details[key] = getattr(self, key) - return details + def __set_default_mask(self, selector: dict, val: bool = True): + default_selector = str(getattr(self.fields, "__mask__"))[1:-1] + self.__update_selector_value(selector, default_selector, val) diff --git a/o2common/views/route_exception.py b/o2common/views/route_exception.py new file mode 100644 index 0000000..07a60a3 --- /dev/null +++ b/o2common/views/route_exception.py @@ -0,0 +1,87 @@ +# Copyright (C) 2021-2022 Wind River Systems, Inc. +# +# 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. +from flask import request +from flask_restx._http import HTTPStatus +from werkzeug.exceptions import ( + BadRequest, + MethodNotAllowed, + NotFound, + InternalServerError, +) + + +from o2common.helper import o2logging +logger = o2logging.get_logger(__name__) + + +class BadRequestException(BadRequest): + def __init__(self, desc=None, resp=None): + super().__init__(description=desc, response=resp) + + +class NotFoundException(NotFound): + def __init__(self, desc=None, resp=None): + super().__init__(description=desc, response=resp) + + +class ProblemDetails(): + def __init__(self, code: int, detail: str, + title=None, instance=None + ) -> None: + self.status = code + self.detail = detail + self.type = request.path + self.title = title if title is not None else self.getTitle(code) + self.instance = instance if instance is not None else [] + + def getTitle(self, code): + return HTTPStatus(code).phrase + + def serialize(self): + details = {} + for key in dir(self): + if key == 'ns' or key.startswith('__') or \ + callable(getattr(self, key)): + continue + else: + details[key] = getattr(self, key) + return details + + +def configure_exception(app): + + @app.errorhandler(BadRequestException) + def handle_badrequest_exception(error): + '''Return a custom message and 400 status code''' + type(error) + problem = ProblemDetails(400, str(error)) + return problem.serialize(), 400 + + @app.errorhandler(NotFoundException) + def handle_notfound_exception(error): + '''Return a custom message and 404 status code''' + problem = ProblemDetails(404, str(error)) + return problem.serialize(), 404 + + @app.errorhandler(MethodNotAllowed) + def handle_methodnotallowed_exception(error): + '''Return a custom message and 405 status code''' + problem = ProblemDetails(405, "Method not allowed") + return problem.serialize(), 405 + + @app.errorhandler(InternalServerError) + def handle_internalservererror_exception(error): + '''Return a custom message and 500 status code''' + problem = ProblemDetails(500, "Internal Server Error") + return problem.serialize(), 500 diff --git a/o2common/views/view.py b/o2common/views/view.py index 7feaf04..6d55b71 100644 --- a/o2common/views/view.py +++ b/o2common/views/view.py @@ -15,6 +15,8 @@ from sqlalchemy.sql.elements import ColumnElement from sqlalchemy import or_ +from o2common.views.route_exception import BadRequestException + from o2common.helper import o2logging logger = o2logging.get_logger(__name__) @@ -47,7 +49,8 @@ def toFilterArgs(operation: str, obj: ColumnElement, key: str, values: list): if not hasattr(obj, key): logger.warning('Filter attrName %s not in Object %s.' % (key, str(obj))) - return [] + raise BadRequestException( + 'Filter attrName {} not in the Object'.format(key)) if operation in ['eq', 'neq', 'gt', 'lt', 'gte', 'lte']: if len(values) != 1: diff --git a/o2ims/views/__init__.py b/o2ims/views/__init__.py index 58d5f69..df2c304 100644 --- a/o2ims/views/__init__.py +++ b/o2ims/views/__init__.py @@ -11,11 +11,9 @@ # 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. - +from . import api_ns, ocloud_route, alarm_route from o2common.config import config -from . import ocloud_route, alarm_route -from . import api_ns from o2common.helper import o2logging logger = o2logging.get_logger(__name__) diff --git a/o2ims/views/alarm_route.py b/o2ims/views/alarm_route.py index 74b8b5f..8b03880 100644 --- a/o2ims/views/alarm_route.py +++ b/o2ims/views/alarm_route.py @@ -17,6 +17,7 @@ from flask_restx import Resource, reqparse from o2common.service.messagebus import MessageBus from o2common.views.pagination_route import link_header, PAGE_PARAM +from o2common.views.route_exception import NotFoundException from o2ims.views import alarm_view from o2ims.views.api_ns import api_ims_monitoring as api_monitoring_v1 from o2ims.views.alarm_dto import AlarmDTO, SubscriptionDTO @@ -123,14 +124,14 @@ class AlarmGetRouter(Resource): model = AlarmDTO.alarm_event_record_get - @api_monitoring_v1.doc('Get resource type') + @api_monitoring_v1.doc('Get AlarmEventRecord') @api_monitoring_v1.marshal_with(model) def get(self, alarmEventRecordId): result = alarm_view.alarm_event_record_one(alarmEventRecordId, bus.uow) if result is not None: return result - api_monitoring_v1.abort( - 404, "Resource type {} doesn't exist".format(alarmEventRecordId)) + raise NotFoundException( + "Alarm Event Record {} doesn't exist".format(alarmEventRecordId)) # ---------- Alarm Subscriptions ---------- # @@ -229,11 +230,11 @@ class SubscriptionGetDelRouter(Resource): alarmSubscriptionID, bus.uow) if result is not None: return result - api_monitoring_v1.abort(404, "Subscription {} doesn't exist".format( - alarmSubscriptionID)) + raise NotFoundException( + "Subscription {} doesn't exist".format(alarmSubscriptionID)) @api_monitoring_v1.doc('Delete subscription by ID') - @api_monitoring_v1.response(204, 'Subscription deleted') + @api_monitoring_v1.response(200, 'Subscription deleted') def delete(self, alarmSubscriptionID): result = alarm_view.subscription_delete(alarmSubscriptionID, bus.uow) - return result, 204 + return result, 200 diff --git a/o2ims/views/ocloud_dto.py b/o2ims/views/ocloud_dto.py index aeb44e4..14d9873 100644 --- a/o2ims/views/ocloud_dto.py +++ b/o2ims/views/ocloud_dto.py @@ -36,7 +36,8 @@ class OcloudDTO: # 'deploymentManagers': fields.String, # 'smoRegistrationService': fields.String 'extensions': fields.String - } + }, + mask='{oCloudId,globalcloudId,name,description,serviceUri}' ) @@ -69,7 +70,8 @@ class ResourceTypeDTO: # 'resourceKind': fields.String, # 'resourceClass': fields.String, 'extensions': fields.String - } + }, + mask='{resourceTypeId,name,description,model,vendor,version}' ) @@ -87,7 +89,8 @@ class ResourcePoolDTO: 'location': fields.String, # 'resources': fields.String, 'extensions': fields.String - } + }, + mask='{resourcePoolId,oCloudId,globalLocationId,name,description}' ) @@ -105,7 +108,8 @@ class ResourceDTO: 'description': fields.String, # 'elements': fields.String, 'extensions': fields.String - } + }, + mask='{resourceId,resourcePoolId,resourceTypeId,description,parentId}' ) def recursive_resource_mapping(iteration_number=2): @@ -126,7 +130,9 @@ class ResourceDTO: fields.Nested(ResourceDTO.recursive_resource_mapping( iteration_number-1)), attribute='children') return api_ims_inventory_v1.model( - 'ResourceGetDto' + str(iteration_number), resource_json_mapping) + 'ResourceGetDto' + str(iteration_number), resource_json_mapping, + mask='{resourceId,resourcePoolId,resourceTypeId,description,' + + 'parentId}') class DeploymentManagerDTO: @@ -151,7 +157,9 @@ class DeploymentManagerDTO: description='Profile support list, use default for the return \ endpoint'), 'extensions': fields.String - } + }, + mask='{deploymentManagerId,name,description,oCloudId,serviceUri,' + \ + 'profileSupportList}' ) profile = api_ims_inventory_v1.model("DeploymentManagerGetDtoProfile", { @@ -187,7 +195,9 @@ class DeploymentManagerDTO: 'profileName': fields.String, 'profileData': fields.Nested(profile, False, True), 'extensions': fields.String - } + }, + mask='{deploymentManagerId,name,description,oCloudId,serviceUri,' +\ + 'profileName,profileData}' ) @@ -201,7 +211,8 @@ class SubscriptionDTO: 'callback': fields.String, 'consumerSubscriptionId': fields.String, 'filter': fields.String, - } + }, + mask='{subscriptionId,callback}' ) subscription = api_ims_inventory_v1.model( diff --git a/o2ims/views/ocloud_route.py b/o2ims/views/ocloud_route.py index 19613f0..570292d 100644 --- a/o2ims/views/ocloud_route.py +++ b/o2ims/views/ocloud_route.py @@ -17,7 +17,7 @@ from flask_restx import Resource, reqparse from o2common.service.messagebus import MessageBus from o2common.views.pagination_route import link_header, PAGE_PARAM -from o2common.views.route import ProblemDetails +from o2common.views.route_exception import NotFoundException from o2ims.views import ocloud_view from o2ims.views.api_ns import api_ims_inventory as api_ims_inventory_v1 from o2ims.views.ocloud_dto import OcloudDTO, ResourceTypeDTO,\ @@ -82,9 +82,7 @@ class OcloudsListRouter(Resource): res = ocloud_view.oclouds(bus.uow) if len(res) > 0: return res[0] - ProblemDetails( - api_ims_inventory_v1, - 404, "oCloud doesn't exist").abort() + raise NotFoundException("oCloud doesn't exist") # ---------- ResourceTypes ---------- # @@ -169,10 +167,8 @@ class ResourceTypeGetRouter(Resource): result = ocloud_view.resource_type_one(resourceTypeID, bus.uow) if result is not None: return result - ProblemDetails( - api_ims_inventory_v1, - 404, "Resource type {} doesn't exist".format( - resourceTypeID)).abort() + raise NotFoundException("Resource type {} doesn't exist".format( + resourceTypeID)) # ---------- ResourcePools ---------- # @@ -257,10 +253,8 @@ class ResourcePoolGetRouter(Resource): result = ocloud_view.resource_pool_one(resourcePoolID, bus.uow) if result is not None: return result - ProblemDetails( - api_ims_inventory_v1, - 404, "Resource pool {} doesn't exist".format( - resourcePoolID)).abort() + raise NotFoundException("Resource pool {} doesn't exist".format( + resourcePoolID)) # ---------- Resources ---------- # @@ -358,10 +352,8 @@ class ResourceGetRouter(Resource): result = ocloud_view.resource_one(resourceID, bus.uow) if result is not None: return result - ProblemDetails( - api_ims_inventory_v1, - 404, "Resource {} doesn't exist".format( - resourceID)).abort() + raise NotFoundException("Resource {} doesn't exist".format( + resourceID)) # ---------- DeploymentManagers ---------- # @@ -456,11 +448,8 @@ class DeploymentManagerGetRouter(Resource): deploymentManagerID, bus.uow, profile) if result is not None: return result - - ProblemDetails( - api_ims_inventory_v1, - 404, "Deployment manager {} doesn't exist".format( - deploymentManagerID)).abort() + raise NotFoundException("Deployment manager {} doesn't exist".format( + deploymentManagerID)) # ---------- Subscriptions ---------- # @@ -559,13 +548,11 @@ class SubscriptionGetDelRouter(Resource): subscriptionID, bus.uow) if result is not None: return result - ProblemDetails( - api_ims_inventory_v1, - 404, "Subscription {} doesn't exist".format( - subscriptionID)).abort() + raise NotFoundException("Subscription {} doesn't exist".format( + subscriptionID)) @api_ims_inventory_v1.doc('Delete subscription by ID') - @api_ims_inventory_v1.response(204, 'Subscription deleted') + @api_ims_inventory_v1.response(200, 'Subscription deleted') def delete(self, subscriptionID): result = ocloud_view.subscription_delete(subscriptionID, bus.uow) - return result, 204 + return result, 200 diff --git a/tests/unit/test_alarm.py b/tests/unit/test_alarm.py index 5309eb5..1cd48b8 100644 --- a/tests/unit/test_alarm.py +++ b/tests/unit/test_alarm.py @@ -195,7 +195,7 @@ def test_flask_delete(mock_flask_uow): subscription_id1 = str(uuid.uuid4()) resp = client.delete(apibase+"/alarmSubscriptions/"+subscription_id1) - assert resp.status_code == 204 + assert resp.status_code == 200 def test_flask_not_allowed(mock_flask_uow): diff --git a/tests/unit/test_ocloud.py b/tests/unit/test_ocloud.py index a3c0f67..01395ee 100644 --- a/tests/unit/test_ocloud.py +++ b/tests/unit/test_ocloud.py @@ -470,7 +470,7 @@ def test_flask_delete(mock_flask_uow): subscription_id1 = str(uuid.uuid4()) resp = client.delete(apibase+"/subscriptions/"+subscription_id1) - assert resp.status_code == 204 + assert resp.status_code == 200 def test_flask_not_allowed(mock_flask_uow): -- 2.16.6