Fix return codes and messages (#12)

* Use werkzeug Exception classes

Use werkzeug exception classes to return the correct error
codes.
Also raise a ValueError if no item is provided on creation
Fixes #43

* Fix return codes

Fix some return codes on update, delete and end maintenance.
Fix #44

* Upgrade versions and fix linter issues

* Fixed test error

* Do not allow items with empty list on update

main.py

@@ -7,6 +7,7 @@
 
 import pytz
 from flask import jsonify, request
+from werkzeug.exceptions import BadRequest, NotFound, UnsupportedMediaType
 
 from kytos.core import KytosNApp, rest
 from napps.kytos.maintenance.models import MaintenanceWindow as MW
@@ -38,14 +39,12 @@ def execute(self):
 
             self.execute_as_loop(30)  # 30-second interval.
         """
-        pass
 
     def shutdown(self):
         """Run when your napp is unloaded.
 
         If you have some cleanup procedure, insert it here.
         """
-        pass
 
     @rest('/', methods=['GET'])
     @rest('/<mw_id>', methods=['GET'])
@@ -58,23 +57,25 @@ def get_mw(self, mw_id=None):
         try:
             return jsonify(self.maintenances[mw_id].as_dict()), 200
         except KeyError:
-            return jsonify({'response': f'Maintenance with id {mw_id} not '
-                                        f'found'}), 404
+            raise NotFound(f'Maintenance with id {mw_id} not found')
 
     @rest('/', methods=['POST'])
     def create_mw(self):
         """Create a new maintenance window."""
         now = datetime.datetime.now(pytz.utc)
         data = request.get_json()
         if not data:
-            return jsonify("Bad request: The request do not have a json."), 415
-        maintenance = MW.from_dict(data, self.controller)
+            raise UnsupportedMediaType('The request does not have a json.')
+        try:
+            maintenance = MW.from_dict(data, self.controller)
+        except ValueError as err:
+            raise BadRequest(f'{err}')
         if maintenance is None:
-            return jsonify('One or more items are invalid'), 400
+            raise BadRequest('One or more items are invalid')
         if maintenance.start < now:
-            return jsonify('Start in the past not allowed'), 400
+            raise BadRequest('Start in the past not allowed')
         if maintenance.end <= maintenance.start:
-            return jsonify('End before start not allowed'), 400
+            raise BadRequest('End before start not allowed')
         self.scheduler.add(maintenance)
         self.maintenances[maintenance.id] = maintenance
         return jsonify({'mw_id': maintenance.id}), 201
@@ -84,19 +85,17 @@ def update_mw(self, mw_id):
         """Update a maintenance window."""
         data = request.get_json()
         if not data:
-            return jsonify("Bad request: The request do not have a json."), 415
+            raise UnsupportedMediaType('The request does not have a json.')
         try:
             maintenance = self.maintenances[mw_id]
         except KeyError:
-            return jsonify({'response': f'Maintenance with id {mw_id} not '
-                                        f'found'}), 404
+            raise NotFound(f'Maintenance with id {mw_id} not found')
         if maintenance.status == Status.RUNNING:
-            return jsonify({'response': 'Updating a running maintenance is '
-                                        'not allowed'}), 400
+            raise BadRequest('Updating a running maintenance is not allowed')
         try:
             maintenance.update(data)
         except ValueError as error:
-            return jsonify(f'{error}'), 400
+            raise BadRequest(f'{error}')
         self.scheduler.remove(maintenance)
         self.scheduler.add(maintenance)
         return jsonify({'response': f'Maintenance {mw_id} updated'}), 201
@@ -107,11 +106,9 @@ def remove_mw(self, mw_id):
         try:
             maintenance = self.maintenances[mw_id]
         except KeyError:
-            return jsonify({'response': f'Maintenance with id {mw_id} not '
-                                        f'found'}), 404
+            raise NotFound(f'Maintenance with id {mw_id} not found')
         if maintenance.status == Status.RUNNING:
-            return jsonify({'response': 'Deleting a running maintenance is '
-                                        'not allowed'}), 400
+            raise BadRequest('Deleting a running maintenance is not allowed')
         self.scheduler.remove(maintenance)
         del self.maintenances[mw_id]
         return jsonify({'response': f'Maintenance with id {mw_id} '
@@ -123,15 +120,14 @@ def end_mw(self, mw_id):
         try:
             maintenance = self.maintenances[mw_id]
         except KeyError:
-            return jsonify({'response': f'Maintenance with id '
-                                        f'{mw_id} not found'}), 404
+            raise NotFound(f'Maintenance with id {mw_id} not found')
         now = datetime.datetime.now(pytz.utc)
         if now < maintenance.start:
-            return jsonify({'response': f'Maintenance window {mw_id} has not '
-                                        f'yet started.'}), 400
+            raise BadRequest(f'Maintenance window {mw_id} has not yet '
+                             'started.')
         if now > maintenance.end:
-            return jsonify({'response': f'Maintenance window {mw_id} has '
-                                        f'already finished.'}), 400
+            raise BadRequest(f'Maintenance window {mw_id} has already '
+                             'finished.')
         self.scheduler.remove(maintenance)
         maintenance.end_mw()
         return jsonify({'response': f'Maintenance window {mw_id} '

models.py

@@ -96,7 +96,12 @@ def from_dict(cls, mw_dict, controller):
 
         start = cls.str_to_datetime(mw_dict['start'])
         end = cls.str_to_datetime(mw_dict['end'])
-        items = mw_dict['items']
+        try:
+            items = mw_dict['items']
+        except KeyError:
+            raise ValueError('At least one item must be provided')
+        if not items:
+            raise ValueError('At least one item must be provided')
         description = mw_dict.get('description')
         status = mw_dict.get('status', Status.PENDING)
         return cls(start, end, controller, items=items, mw_id=mw_id,
@@ -119,7 +124,7 @@ def update(self, mw_dict):
             raise ValueError('End before start not allowed.')
         self.start = start
         self.end = end
-        if 'items' in mw_dict:
+        if 'items' in mw_dict and mw_dict['items']:
             self.items = mw_dict['items']
         if 'description' in mw_dict:
             self.description = mw_dict['description']

openapi.yml

@@ -155,8 +155,6 @@ paths:
           description: Invalid data.
         '404':
           description: Maintenance window not found.
-        '415':
-          description: No JSON in request.
   '/maintenance/report':
     post:
       tags:

requirements/dev.txt

@@ -7,9 +7,9 @@
 -e git+https://github.com/kytos/kytos.git#egg=kytos
 -e .
 -e git+https://github.com/kytos/python-openflow.git#egg=python-openflow
-astroid==2.0.4            # via pylint
+astroid==2.3.3            # via pylint
 click==7.1.1              # via pip-tools
-coverage==5.0.3
+coverage==5.0.4
 docopt==0.6.2             # via yala
 first==2.0.1              # via pip-tools
 isort==4.3.4              # via pylint, yala
@@ -18,9 +18,9 @@ mccabe==0.6.1             # via pylint
 pip-tools==2.0.2
 pluggy==0.12             # via tox
 py==1.6.0                 # via tox
-pycodestyle==2.4.0        # via yala
-pydocstyle==2.1.1         # via yala
-pylint==2.1.1             # via yala
+pycodestyle==2.5.0        # via yala
+pydocstyle==5.1.1         # via yala
+pylint==2.4.4             # via yala
 pytest==5.4.1             # via pytest
 six==1.15.0               # via astroid, pip-tools, pydocstyle, tox
 snowballstemmer==1.2.1    # via pydocstyle
@@ -29,4 +29,4 @@ typed-ast==1.1.0          # via astroid
 virtualenv==16.0.0        # via tox
 wrapt==1.10.11            # via astroid
 requests==2.21.0
-yala==1.7.0
+yala==2.2.0

setup.py

@@ -155,7 +155,7 @@ def run(self):
         except RuntimeError as error:
             print('Linter check failed. Fix the error(s) above and try again.')
             print(error)
-            exit(-1)
+            sys.exit(-1)
 
 
 class CITest(TestCommand):

tests/unit/test_main.py

@@ -95,7 +95,8 @@ def test_create_mw_case_2(self, from_dict_mock, sched_add_mock):
                                  content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, 'One or more items are invalid')
+        self.assertEqual(current_data['description'],
+                         'One or more items are invalid')
         sched_add_mock.assert_not_called()
 
     @patch('napps.kytos.maintenance.models.Scheduler.add')
@@ -130,7 +131,8 @@ def test_create_mw_case_3(self, from_dict_mock, sched_add_mock):
                                  content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, 'Start in the past not allowed')
+        self.assertEqual(current_data['description'],
+                         'Start in the past not allowed')
         sched_add_mock.assert_not_called()
 
     @patch('napps.kytos.maintenance.models.Scheduler.add')
@@ -165,7 +167,8 @@ def test_create_mw_case_4(self, from_dict_mock, sched_add_mock):
                                  content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, 'End before start not allowed')
+        self.assertEqual(current_data['description'],
+                         'End before start not allowed')
         sched_add_mock.assert_not_called()
 
     @patch('napps.kytos.maintenance.models.MaintenanceWindow.as_dict')
@@ -238,8 +241,8 @@ def test_get_mw_case_3(self, mw_as_dict_mock):
         response = self.api.get(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 404)
-        self.assertEqual(current_data, {'response': 'Maintenance with id 2345 '
-                                                    'not found'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance with id 2345 not found')
         mw_as_dict_mock.assert_not_called()
 
     @patch('napps.kytos.maintenance.models.MaintenanceWindow.as_dict')
@@ -291,8 +294,8 @@ def test_remove_mw_case_1(self):
         response = self.api.delete(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 404)
-        self.assertEqual(current_data, {'response': 'Maintenance with id 2345 '
-                                                    'not found'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance with id 2345 not found')
 
     @patch('napps.kytos.maintenance.models.Scheduler.remove')
     def test_remove_mw_case_2(self, sched_remove_mock):
@@ -336,8 +339,8 @@ def test_remove_mw_case_3(self):
         response = self.api.delete(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, {'response': 'Deleting a running mainte'
-                                                    'nance is not allowed'})
+        self.assertEqual(current_data['description'],
+                         'Deleting a running maintenance is not allowed')
 
     def test_update_mw_case_1(self):
         """Test update non-existent id."""
@@ -361,8 +364,8 @@ def test_update_mw_case_1(self):
                                   content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 404)
-        self.assertEqual(current_data, {'response': 'Maintenance with id 2345 '
-                                                    'not found'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance with id 2345 not found')
 
     def test_update_mw_case_2(self):
         """Test update no data."""
@@ -386,8 +389,8 @@ def test_update_mw_case_2(self):
                                   content_type='text/plain')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 415)
-        self.assertEqual(current_data,
-                         'Bad request: The request do not have a json.')
+        self.assertEqual(current_data['description'],
+                         'The request does not have a json.')
 
     @patch('napps.kytos.maintenance.models.Scheduler.add')
     @patch('napps.kytos.maintenance.models.Scheduler.remove')
@@ -448,7 +451,8 @@ def test_update_mw_case_4(self, mw_update_mock):
                                   content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, 'Start in the past not allowed.')
+        self.assertEqual(current_data['description'],
+                         'Start in the past not allowed.')
         mw_update_mock.assert_called_once_with(payload)
 
     @patch('napps.kytos.maintenance.models.MaintenanceWindow.update')
@@ -479,7 +483,8 @@ def test_update_mw_case_5(self, mw_update_mock):
                                   content_type='application/json')
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data, 'End before start not allowed.')
+        self.assertEqual(current_data['description'],
+                         'End before start not allowed.')
         mw_update_mock.assert_called_once_with(payload)
 
     def test_end_mw_case_1(self):
@@ -500,8 +505,8 @@ def test_end_mw_case_1(self):
         response = self.api.patch(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 404)
-        self.assertEqual(current_data,
-                         {'response': 'Maintenance with id 2345 not found'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance with id 2345 not found')
 
     @patch('napps.kytos.maintenance.models.MaintenanceWindow.end_mw')
     def test_end_mw_case_2(self, end_mw_mock):
@@ -544,9 +549,8 @@ def test_end_mw_case_3(self):
         response = self.api.patch(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data,
-                         {'response': 'Maintenance window 1234 has not yet '
-                                      'started.'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance window 1234 has not yet started.')
 
     def test_end_mw_case_4(self):
         """Test method that finishes the maintenance now."""
@@ -566,6 +570,5 @@ def test_end_mw_case_4(self):
         response = self.api.patch(url)
         current_data = json.loads(response.data)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(current_data,
-                         {'response': 'Maintenance window 1234 has already '
-                                      'finished.'})
+        self.assertEqual(current_data['description'],
+                         'Maintenance window 1234 has already finished.')

tests/unit/test_models.py

@@ -47,6 +47,11 @@ def test_update_start(self):
         self.maintenance.update({'start': start.strftime(TIME_FMT)})
         self.assertEqual(self.maintenance.start, start)
 
+    def test_update_empty_items(self):
+        """Test failed update."""
+        self.maintenance.update({'items': []})
+        self.assertEqual(self.maintenance.items, self.items)
+
     def test_update_items(self):
         """Test update items parameter."""
         items = ["09:87:65:43:21:fe:dc:ba"]