Repository: OnroerendErfgoed/pyramid_urireferencer

Issue:
Error message when a resource can't be deleted.
Currently, when the protected resources decorator finds that a resource is still in use, it raise a 409 Conflict with a text string as body.

The current error message looks like:
*Urireferencer: The uri https://id.erfgoed.net/actoren/1 is still in use by other applications: https://inventaris.onroerenderfgoed.be, https://besluiten.onroerenderfgoed.be*

I would like to see this changed to:
*The uri https://id.erfgoed.net/actoren/1 is still in use by other applications. A total of 8 references have been found in the following applications: https://inventaris.onroerenderfgoed.be, https://besluiten.onroerenderfgoed.be*

I would also like a more custom error message when request.accept = 'application/json'

```json
{
 "message": "The uri https://id.erfgoed.net/actoren/1 is still in use by other applications. A total of 8 references have been found.",
 "errors": [
  "https://inventaris.onroerenderfgoed.be: 6 references found, such as https://id.erfgoed.net/erfgoedobjecten/56, https://id.erfgoed.net/aanduidingsobjecten/889, https://id.erfgoed.net/aanduidingsobjecten/12487, https://id.erfgoed.net/gebeurtenissen/965, https://id.erfgoed.net/themas/123.",
  "https://besluiten.onroerenderfgoed.be: 2 references found, such as https://id.erfgoed.net/besluiten/1896, https://id.erfgoed.net/besluiten/23."
 ],
 "registry_response": <Include the response the registry passed so the client can create custom error messages>
}
```

Discussion:

Patch:
diff --git a/pyramid_urireferencer/models.py b/pyramid_urireferencer/models.py
index 8ef53f0..9bbd245 100644
--- a/pyramid_urireferencer/models.py
+++ b/pyramid_urireferencer/models.py
@@ -14,12 +14,13 @@ class RegistryResponse:
     :param int count: How many references were found?
     :param list applications: A list of application results.
     '''
+
     def __init__(self, query_uri, success, has_references, count, applications):
-            self.query_uri = query_uri
-            self.success = success
-            self.has_references = has_references
-            self.count = count
-            self.applications = applications
+        self.query_uri = query_uri
+        self.success = success
+        self.has_references = has_references
+        self.count = count
+        self.applications = applications
 
     @staticmethod
     def load_from_json(data):
@@ -34,9 +35,19 @@ class RegistryResponse:
         r.success = data['success']
         r.has_references = data['has_references']
         r.count = data['count']
-        r.applications = [ApplicationResponse.load_from_json(a) for a in data['applications']] if data['applications'] is not None else None
+        r.applications = [ApplicationResponse.load_from_json(a) for a in data['applications']] if data[
+                                                                                                      'applications'] is not None else None
         return r
 
+    def to_json(self):
+        return {
+            "query_uri": self.query_uri,
+            "success": self.success,
+            "has_references": self.has_references,
+            "count": self.count,
+            "applications": [app.to_json() for app in self.applications]
+        }
+
 
 class ApplicationResponse:
     '''
@@ -52,14 +63,15 @@ class ApplicationResponse:
     :param list items: A list of items that have a reference to the \
         uri under survey. Limited to 5 items for performance reasons.
     '''
+
     def __init__(self, title, uri, service_url, success, has_references, count, items):
-            self.title = title
-            self.uri = uri
-            self.service_url = service_url
-            self.success = success
-            self.has_references = has_references
-            self.count = count
-            self.items = items
+        self.title = title
+        self.uri = uri
+        self.service_url = service_url
+        self.success = success
+        self.has_references = has_references
+        self.count = count
+        self.items = items
 
     @staticmethod
     def load_from_json(data):
@@ -79,6 +91,17 @@ class ApplicationResponse:
         r.items = [Item.load_from_json(a) for a in data['items']] if data['items'] is not None else None
         return r
 
+    def to_json(self):
+        return {
+            "title": self.title,
+            "uri": self.uri,
+            "service_url": self.service_url,
+            "success": self.success,
+            "has_references": self.has_references,
+            "count": self.count,
+            "items": [item.to_json() for item in self.items]
+        }
+
 
 class Item:
     '''
@@ -87,6 +110,7 @@ class Item:
     :param string title: Title of the item.
     :param string uri: Uri of the item.
     '''
+
     def __init__(self, title, uri):
         self.title = title
         self.uri = uri
@@ -103,3 +127,9 @@ class Item:
         i.uri = data['uri']
         i.title = data['title']
         return i
+
+    def to_json(self):
+        return {
+            "title": self.title,
+            "uri": self.uri
+        }
diff --git a/pyramid_urireferencer/protected_resources.py b/pyramid_urireferencer/protected_resources.py
index 2814254..db4556a 100644
--- a/pyramid_urireferencer/protected_resources.py
+++ b/pyramid_urireferencer/protected_resources.py
@@ -8,6 +8,7 @@ that might be used in external applications.
 from pyramid.httpexceptions import (
     HTTPInternalServerError,
     HTTPConflict)
+from webob import Response
 
 import pyramid_urireferencer
 
@@ -26,19 +27,41 @@ def protected_operation(fn):
     :raises pyramid.httpexceptions.HTTPInternalServerError: Raised when we were
         unable to check that the URI is no longer being used.
     '''
+
     def advice(parent_object, *args, **kw):
         id = parent_object.request.matchdict['id']
         referencer = pyramid_urireferencer.get_referencer(parent_object.request.registry)
         uri = parent_object.uri_template.format(id)
         registery_response = referencer.is_referenced(uri)
         if registery_response.has_references:
-            raise HTTPConflict(
-                detail="Urireferencer: The uri {0} is still in use by other applications: {1}".
-                    format(uri, ', '.join([app_response.title for app_response in registery_response.applications
-                                           if app_response.has_references])))
+            if parent_object.request.headers.get("Accept", None) == "application/json":
+                response = Response()
+                response.status_code = 409
+                response_json = {
+                    "message": "The uri {0} is still in use by other applications. A total of {1} references have been found.".format(
+                        uri, registery_response.count),
+                    "errors": [],
+                    "registry_response": registery_response.to_json()
+                }
+                for app_response in registery_response.applications:
+                    if app_response.has_references:
+                        error_string = "{0}: {1} references found, such as {2}"\
+                            .format(app_response.uri,
+                                    app_response.count,
+                                    ', '.join([i.uri for i in app_response.items]))
+                        response_json["errors"].append(error_string)
+                        response.json_body = response_json
+                        response.content_type = 'application/json'
+                return response
+            else:
+                raise HTTPConflict(
+                        detail="Urireferencer: The uri {0} is still in use by other applications. A total of {1} references have been found in the following applications: {2}".
+                            format(uri, registery_response.count,
+                                   ', '.join([app_response.title for app_response in registery_response.applications
+                                              if app_response.has_references])))
         elif not registery_response.success:
             raise HTTPInternalServerError(
-                detail="Urireferencer: Something went wrong while retrieving references of the uri {0}".format(uri))
+                    detail="Urireferencer: Something went wrong while retrieving references of the uri {0}".format(uri))
         return fn(parent_object, *args, **kw)
 
-    return advice
\ No newline at end of file
+    return advice
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 22621e2..10814ca 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -7,6 +7,7 @@ pytest-cov==2.1.0
 webtest==2.0.18
 httpretty==0.8.10
 coveralls
+mock==1.3.0
 
 #wheel
 wheel==0.26.0


Test patch:
diff --git a/tests/test_protected_resources.py b/tests/test_protected_resources.py
index dc47a81..1989235 100644
--- a/tests/test_protected_resources.py
+++ b/tests/test_protected_resources.py
@@ -2,23 +2,34 @@
 import unittest
 from pyramid import testing
 from pyramid_urireferencer.protected_resources import protected_operation
-from pyramid_urireferencer.models import RegistryResponse
+from pyramid_urireferencer.models import RegistryResponse, Item, ApplicationResponse
 from pyramid.httpexceptions import HTTPConflict, HTTPInternalServerError
+
 try:
     from unittest.mock import Mock, patch
 except ImportError:
     from mock import Mock, patch  # pragma: no cover
 
+
 def get_app(nr):
-    class Object(object):
-        pass
-    a = Object()
-    a.title = 'App {0}'.format(nr)
-    a.has_references = True if nr == 1 else False
+    items = []
+    if nr == 1:
+        items.append(Item(uri="https://dev-besluiten.onroerenderfgoed.be/besluiten/152", title="Mijn besluit"))
+        items.append(Item(uri="https://dev-besluiten.onroerenderfgoed.be/besluiten/154",
+                          title="Vaststelling van de inventaris van het Bouwkundig Erfgoed op 28 november 2014"))
+    a = ApplicationResponse(
+            title='App {0}'.format(nr),
+            uri="https://dev-app-{0}.onroerenderfgoed.be/".format(nr),
+            service_url="https://dev-app-{0}.onroerenderfgoed.be/references".format(nr),
+            success=True,
+            has_references=True if nr == 1 else False,
+            count=2 if nr == 1 else 0,
+            items=items
+    )
     return a
 
-class DummyParent(object):
 
+class DummyParent(object):
     def __init__(self):
         self.request = testing.DummyRequest()
         config = testing.setUp(request=self.request)
@@ -37,7 +48,6 @@ class DummyParent(object):
 
 
 class ProtectedTests(unittest.TestCase):
-
     def setUp(self):
         pass
 
@@ -52,7 +62,8 @@ class ProtectedTests(unittest.TestCase):
     @patch('pyramid_urireferencer.protected_resources.pyramid_urireferencer.Referencer.is_referenced')
     def test_protected_operation_409(self, is_referenced_mock):
         dummy = DummyParent()
-        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', True, True, 10, [get_app(1), get_app(2)])
+        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', True, True, 10,
+                                                           [get_app(1), get_app(2)])
         self.assertRaises(HTTPConflict, dummy.protected_dummy)
         is_referenced_call = is_referenced_mock.mock_calls[0]
         self.assertEqual('https://id.erfgoed.net/resources/1', is_referenced_call[1][0])
@@ -60,16 +71,32 @@ class ProtectedTests(unittest.TestCase):
     @patch('pyramid_urireferencer.protected_resources.pyramid_urireferencer.Referencer.is_referenced')
     def test_protected_operation_409_2(self, is_referenced_mock):
         dummy = DummyParent()
-        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', False, True, 10, [get_app(1), get_app(2)])
+        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', False, True, 10,
+                                                           [get_app(1), get_app(2)])
         self.assertRaises(HTTPConflict, dummy.protected_dummy)
         is_referenced_call = is_referenced_mock.mock_calls[0]
         self.assertEqual('https://id.erfgoed.net/resources/1', is_referenced_call[1][0])
 
+    @patch('pyramid_urireferencer.protected_resources.pyramid_urireferencer.Referencer.is_referenced')
+    def test_protected_operation_409_json(self, is_referenced_mock):
+        dummy = DummyParent()
+        dummy.request.headers = {"Accept": "application/json"}
+        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', False, True, 2,
+                                                           [get_app(1), get_app(2)])
+        res = dummy.protected_dummy()
+        self.assertEqual(409, res.status_code)
+        self.assertEqual(res.json_body["message"],
+                         "The uri https://id.erfgoed.net/resources/1 is still in use by other applications. A total of 2 references have been found.")
+        self.assertEqual("application/json", res.content_type)
+
+        is_referenced_call = is_referenced_mock.mock_calls[0]
+        self.assertEqual('https://id.erfgoed.net/resources/1', is_referenced_call[1][0])
+
     @patch('pyramid_urireferencer.protected_resources.pyramid_urireferencer.Referencer.is_referenced')
     def test_protected_operation_500(self, is_referenced_mock):
         dummy = DummyParent()
-        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', False, None, None, None)
+        is_referenced_mock.return_value = RegistryResponse('https://id.erfgoed.net/resources/1', False, None, None,
+                                                           None)
         self.assertRaises(HTTPInternalServerError, dummy.protected_dummy)
         is_referenced_call = is_referenced_mock.mock_calls[0]
         self.assertEqual('https://id.erfgoed.net/resources/1', is_referenced_call[1][0])
-