Move API validation into validators.py module

h/api/logic.py

@@ -7,6 +7,7 @@
 
 from h.api.models import Annotation
 from h.api import search as search_lib
+from h.api import validators
 
 
 _ = i18n.TranslationString
@@ -20,18 +21,14 @@
 def create_annotation(fields, user):
     """Create and store an annotation.
 
-    :raises pyramid.httpexceptions.HTTPBadRequest: If the given fields are
-        invalid
+    :raises h.api.validators.Error: If the given fields are invalid
 
     """
     # Some fields are not to be set by the user, ignore them
     for field in PROTECTED_FIELDS:
         fields.pop(field, None)
 
-    if 'document' in fields and 'link' in fields['document']:
-        if not isinstance(fields['document']['link'], list):
-            raise httpexceptions.HTTPBadRequest(
-                "document.link must be an array")
+    validators.Annotation(fields).validate()
 
     # Create Annotation instance
     annotation = Annotation(fields)

h/api/test/logic_test.py

@@ -21,7 +21,7 @@ def _mock_annotation(**kwargs):
 
 # The fixtures required to mock all of create_annotation()'s dependencies.
 create_annotation_fixtures = pytest.mark.usefixtures(
-    'Annotation', 'search_lib')
+    'Annotation', 'search_lib', 'validators')
 
 
 @create_annotation_fixtures
@@ -43,16 +43,15 @@ def test_create_annotation_pops_protected_fields(Annotation):
 
 
 @create_annotation_fixtures
-def test_create_annotation_raises_BadRequest_for_invalid_document_link():
-    with pytest.raises(httpexceptions.HTTPBadRequest):
-        logic.create_annotation(
-            fields={
-                'document': {
-                    'link': None  # Invalid link.
-                }
-            },
-            user=mock.Mock()
-        )
+def test_create_annotation_validates_annotation(validators):
+    """It should validate the annotation by calling the validators module."""
+    fields = mock.MagicMock()
+    validators.Annotation.return_value = mock.Mock()
+
+    logic.create_annotation(fields, mock.Mock())
+
+    validators.Annotation.assert_called_once_with(fields)
+    validators.Annotation.return_value.validate.assert_called_once_with()
 
 
 @create_annotation_fixtures
@@ -369,3 +368,10 @@ def search_lib(request):
     patcher = mock.patch('h.api.logic.search_lib', autospec=True)
     request.addfinalizer(patcher.stop)
     return patcher.start()
+
+
+@pytest.fixture
+def validators(request):
+    patcher = mock.patch('h.api.logic.validators', autospec=True)
+    request.addfinalizer(patcher.stop)
+    return patcher.start()

h/api/test/validators_test.py

@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+import pytest
+
+from h.api import validators
+
+
+def test_Annotation_raises_if_document_link_is_None():
+    annotation = validators.Annotation(
+        data={'document': {'link': None}})  # Invalid link.
+
+    with pytest.raises(validators.Error):
+        annotation.validate()

h/api/test/views_test.py

@@ -8,6 +8,7 @@
 from pyramid import httpexceptions
 
 from h.api import views
+from h.api import validators
 
 
 def _mock_annotation(**kwargs):
@@ -168,8 +169,8 @@ def test_create_calls_create_annotation_once(logic):
 
 
 @create_fixtures
-def test_create_returns_api_error_for_HTTPBadRequest(logic):
-    logic.create_annotation.side_effect = httpexceptions.HTTPBadRequest(
+def test_create_returns_api_error_for_validation_error(logic):
+    logic.create_annotation.side_effect = validators.Error(
         mock.sentinel.reason)
 
     response = views.create(mock.Mock())

h/api/validators.py

@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+"""Classes for validating data passed to the annotations API."""
+
+
+class Error(Exception):
+
+    """Base exception class for all exceptions raised by this module."""
+
+    pass
+
+
+class Annotation(object):
+
+    """An annotation validator."""
+
+    def __init__(self, data):
+        self.data = data
+
+    def validate(self):
+        """Raise h.api.validation.Error if this annotation is invalid."""
+        if 'document' in self.data and 'link' in self.data['document']:
+            if not isinstance(self.data['document']['link'], list):
+                raise Error("document.link must be an array")

h/api/views.py

@@ -13,6 +13,7 @@
 from h.api.events import AnnotationEvent
 from h.api import search as search_lib
 from h.api import logic
+from h.api import validators
 from h.api.resources import Annotation
 from h.api.resources import Annotations
 from h.api.resources import Root
@@ -152,7 +153,7 @@ def create(request):
     try:
         # Create the annotation
         annotation = logic.create_annotation(fields=fields, user=user)
-    except httpexceptions.HTTPBadRequest as err:
+    except validators.Error as err:
         return _api_error(request, err.message, status_code=400)
 
     # Notify any subscribers