Merge pull request #62 from osmlab/dev

Merging dev -> master

Author: mattmanley

examples/challenge_examples.py

@@ -58,4 +58,4 @@
     data = json.loads(data_file.read())
 
 # Printing response:
-print(api.add_tasks_to_challenge(data, challenge_id))
+print(json.dumps(api.add_tasks_to_challenge(data, challenge_id)))

maproulette/__init__.py

@@ -14,4 +14,4 @@
 from .api.task import Task
 from .api.user import User
 
-__version__ = '1.3.0'
+__version__ = '1.4.0'

maproulette/api/challenge.py

@@ -250,7 +250,7 @@ def reset_task_instructions(self, challenge_id):
     def add_tasks_to_challenge(self, data, challenge_id):
         """Method to add tasks to an existing challenge
 
-        :param data: a geojson containing geometry of tasks to be added to a challenge
+        :param data: a GeoJSON containing geometry of tasks to be added to a challenge
         :param challenge_id: the ID corresponding to the challenge that tasks will be added to
         :returns: the API response from the PUT request
         """

maproulette/api/errors.py

@@ -3,34 +3,65 @@
 
 class MapRouletteBaseException(Exception):
     """MapRoulette Base Exception"""
-    def __init__(self, message, status=None, payload=None):
+    def __init__(self, message, status, payload):
         self.message = message
         self.status = status
         self.payload = payload
 
     def __str__(self):
-        return repr({
+        error = {
                 "status": self.status,
                 "message": self.message,
                 "payload": self.payload
-                })
+            }
+        return repr({k: v for (k, v) in error.items() if v is not None})
 
 
 class NotFoundError(MapRouletteBaseException):
     """Resource cannot be found"""
+    def __init__(self, message=None, status=None, payload=None):
+        if message:
+            self.message = message
+        else:
+            self.message = "Resource cannot be found."
+        super().__init__(message=self.message, status=status, payload=payload)
 
 
 class ConnectionUnavailableError(MapRouletteBaseException):
     """A connection error occurred"""
+    def __init__(self, message=None, status=None, payload=None):
+        if message:
+            self.message = message
+        else:
+            self.message = "A connection error occurred."
+        super().__init__(message=self.message, status=status, payload=payload)
 
 
 class UnauthorizedError(MapRouletteBaseException):
     """The user is not authorized to make this request"""
+    def __init__(self, message=None, status=None, payload=None):
+        if message:
+            self.message = message
+        else:
+            self.message = "The user is not authorized to make this request."
+        super().__init__(message=self.message, status=status, payload=payload)
 
 
 class HttpError(MapRouletteBaseException):
     """An HTTP error occurred"""
+    def __init__(self, message=None, status=None, payload=None):
+        if message:
+            self.message = message
+        else:
+            self.message = "An HTTP error occurred."
+        super().__init__(message=self.message, status=status, payload=payload)
 
 
 class InvalidJsonError(MapRouletteBaseException):
     """Errors produced from an invalid JSON object"""
+    def __init__(self, message=None, status=None, payload=None):
+        if message:
+            self.message = message
+        else:
+            self.message = "Invalid JSON payload."
+        super().__init__(message=self.message, status=status, payload=payload)

maproulette/api/maproulette_server.py

@@ -43,6 +43,7 @@ def __check_health(self, retries=3, delay=5):
                     return True
             except requests.exceptions.ConnectionError:
                 print(f"Connection not available. Attempt {str(i+1)} out of {str(retries)}")
+                time.sleep(delay)
 
         raise ConnectionUnavailableError(
             message='Specified server unavailable'
@@ -64,28 +65,25 @@ def get(self, endpoint, params=None):
         except requests.exceptions.HTTPError as e:
             if e.response.status_code == 404:
                 raise NotFoundError(
-                    message='Resource not found',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             else:
                 raise HttpError(
-                    message='An HTTP error occurred',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
         except (requests.ConnectionError, requests.Timeout) as e:
             raise ConnectionUnavailableError(
-                message="Connection Unavailable",
-                status=e.response.status_code,
-                payload=response
+                message=self.parse_response_message(e.response),
+                status=e.response.status_code
             ) from None
         try:
             return {
                 "data": response.json(),
                 "status": response.status_code
             }
-        except json.decoder.JSONDecodeError:
+        except ValueError:
             return {
                 "status": response.status_code
             }
@@ -108,21 +106,18 @@ def post(self, endpoint, body=None, params=None):
         except requests.exceptions.HTTPError as e:
             if e.response.status_code == 400:
                 raise InvalidJsonError(
-                    message='Invalid JSON payload',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             elif e.response.status_code == 401:
                 raise UnauthorizedError(
-                    message='The user is not authorized to make this request',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             else:
                 raise HttpError(
-                    message='An HTTP error occurred',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
         except (requests.ConnectionError, requests.Timeout) as e:
             raise ConnectionUnavailableError(e) from None
@@ -131,7 +126,7 @@ def post(self, endpoint, body=None, params=None):
                 "data": response.json(),
                 "status": response.status_code
             }
-        except json.decoder.JSONDecodeError:
+        except ValueError:
             return {
                 "status": response.status_code
             }
@@ -154,21 +149,18 @@ def put(self, endpoint, body=None, params=None):
         except requests.exceptions.HTTPError as e:
             if e.response.status_code == 400:
                 raise InvalidJsonError(
-                    message='Invalid JSON payload',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             elif e.response.status_code == 401:
                 raise UnauthorizedError(
-                    message='The user is not authorized to make this request',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             else:
                 raise HttpError(
-                    message='An HTTP error occurred',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
         except (requests.ConnectionError, requests.Timeout) as e:
             raise ConnectionUnavailableError(e) from None
@@ -177,7 +169,7 @@ def put(self, endpoint, body=None, params=None):
                 "data": response.json(),
                 "status": response.status_code
             }
-        except json.decoder.JSONDecodeError:
+        except ValueError:
             return {
                 "status": response.status_code
             }
@@ -198,21 +190,18 @@ def delete(self, endpoint, params=None):
         except requests.exceptions.HTTPError as e:
             if e.response.status_code == 401:
                 raise UnauthorizedError(
-                    message='The user is not authorized to make this request',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             elif e.response.status_code == 404:
                 raise NotFoundError(
-                    message='Resource not found',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
             else:
                 raise HttpError(
-                    message='An HTTP error occurred',
-                    status=e.response.status_code,
-                    payload=e.response
+                    message=self.parse_response_message(e.response),
+                    status=e.response.status_code
                 ) from None
         except (requests.ConnectionError, requests.Timeout) as e:
             raise ConnectionUnavailableError(e) from None
@@ -221,7 +210,7 @@ def delete(self, endpoint, params=None):
                 "data": response.json(),
                 "status": response.status_code
             }
-        except json.decoder.JSONDecodeError:
+        except ValueError:
             return {
                 "status": response.status_code
             }
@@ -239,3 +228,15 @@ def is_json(input_object):
             return True
         except ValueError:
             return False
+
+    @staticmethod
+    def parse_response_message(response):
+        """Method to determine the message body from a response object. Will return None if message cannot be parsed.
+
+        :param response: the Requests response object
+        :returns: the response message if parsable, otherwise None
+        """
+        try:
+            return json.loads(response.text)['message']
+        except (ValueError, KeyError):
+            return None

maproulette/api/task.py

@@ -30,15 +30,20 @@ def get_task_history(self, task_id):
             endpoint=f"/task/{task_id}/history")
         return response
 
-    def create_tasks(self, data):
-        """Method to create a batch of tasks
+    def create_tasks(self, data, batch_size=5000):
+        """Method to create a batch of tasks using the specified batch_size.
 
         :param data: a JSON input containing task details
+        :param batch_size: the number of tasks to post per API call. The default is 5000.
+        :type batch_size: int
         :returns: the API response from the POST request
         """
-        response = self.post(
-            endpoint="/tasks",
-            body=data)
+        response = []
+        for batch in self.batch_generator(input_list=data, chunk_size=batch_size):
+            response.append(self.post(
+                endpoint="/tasks",
+                body=batch)
+            )
         return response
 
     def update_tasks(self, data):
@@ -157,3 +162,15 @@ def is_task_model(input_object):
         :returns: True if instance of model
         """
         return bool(isinstance(input_object, TaskModel))
+
+    @staticmethod
+    def batch_generator(input_list, chunk_size):
+        """Method to yield successive n-sized chunks from input_list
+
+        :param input_list: the list to break into chunks
+        :param chunk_size: the number of list items to include per chunk
+        :type chunk_size: int
+        :returns: an iterator for the n-sized chunks of the input_list
+        """
+        for i in range(0, len(input_list), chunk_size):
+            yield input_list[i:i + chunk_size]

maproulette/models/task.py

@@ -190,6 +190,7 @@ def to_dict(self):
             "id": self._id,
             "name": self._name,
             "parent": self._parent,
+            "geometries": self._geometries,
             "instruction": self._instruction,
             "location": self._location,
             "suggestedFix": self._suggested_fix,

tests/test_server.py

@@ -26,7 +26,6 @@ def test_get_not_found_error(self, mock_get, mock_server_get, server_instance=se
             server_instance.get(endpoint='')
         assert context.exception.message == 'Resource not found'
         assert context.exception.status == 404
-        assert context.exception.payload == 'error payload'
 
     @patch('maproulette.api.maproulette_server.MapRouletteServer.get')
     @patch('requests.Session.get')
@@ -265,3 +264,20 @@ def test_delete_connection_error(self, mock_delete, mock_server_delete, server_i
         assert context.exception.message == 'Connection Unavailable'
         assert context.exception.status == 500
         assert context.exception.payload == 'error payload'
+
+    @patch('json.loads')
+    def test_parse_response_message(self, mock_loads, server_instance=server):
+        mock_loads.return_value = {
+            'message': 'some message'
+        }
+        self.assertTrue(server_instance.parse_response_message(mock_loads))
+
+    @patch('json.loads')
+    def test_parse_response_message_value_error(self, mock_loads, server_instance=server):
+        mock_loads.side_effect = ValueError()
+        self.assertIsNone(server_instance.parse_response_message(mock_loads))
+
+    @patch('json.loads')
+    def test_parse_response_message_key_error(self, mock_loads, server_instance=server):
+        mock_loads.side_effect = KeyError()
+        self.assertIsNone(server_instance.parse_response_message(mock_loads))

tests/test_task_api.py

@@ -26,13 +26,16 @@ def test_get_task_history(self, mock_request, api_instance=api):
 
     @patch('maproulette.api.maproulette_server.requests.Session.post')
     def test_create_tasks(self, mock_request, api_instance=api):
+        test_tasks = []
         geometries = test_geojson['features'][0]['geometry']
         test_task_model = maproulette.TaskModel(name='test_task',
                                                 parent='12345',
                                                 geometries=geometries)
+        test_tasks.append(test_task_model.to_dict())
         mock_request.return_value.status_code = '200'
-        response = api_instance.create_tasks(test_task_model)
-        self.assertEqual(response['status'], '200')
+        responses = api_instance.create_tasks(test_tasks)
+        for response in responses:
+            self.assertEqual(response['status'], '200')
 
     @patch('maproulette.api.maproulette_server.requests.Session.put')
     def test_update_tasks(self, mock_request, api_instance=api):
@@ -88,3 +91,15 @@ def test_get_task_comments(self, mock_request, api_instance=api):
         mock_request.return_value.status_code = '200'
         response = api_instance.get_task_comments(task_id)
         self.assertEqual(response['status'], '200')
+
+    def test_batch_generator(self, api_instance=api):
+
+        batch_size = 10
+        test_length = 1234
+        test_list = [i for i in range(test_length)]
+        running_total = 0
+        for chunk in api_instance.batch_generator(test_list, batch_size):
+            running_total += len(chunk)
+            self.assertIsInstance(chunk, list)
+            self.assertLessEqual(len(chunk), batch_size)
+        self.assertEqual(test_length, running_total)