Merge pull request #106 from MrCreosote/dev-prototype

SCT-1147 Fix get namespaces bug

src/jgikbase/idmapping/service/mapper_service.py

@@ -40,16 +40,6 @@
 # and required less work. Push the bulk implementation further down in the stack as necessitated
 # by peformance needs.
 
-# NOTE 1: I *&*_&*& hate flask. If you do `request.headers.get()`, it parses the data and, if
-#         the content-type header says form data, will set request.data to None
-#         (which means request.get_data() is also None). So you can't get the data if you
-#         don't care about the content type, which you don't if the client is curl, which
-#         always sets content-type to form data if you use --data. That means you always
-#         have to set content-type manually from curl, even if the service only accepts json.
-#         Stupid.
-#
-#         What's even better is that I can't figure out how to replicate this behavior in
-#         the unit tests.
 
 _APP = 'ID_MAPPER'
 
@@ -105,7 +95,7 @@ def _objids_to_jsonable(oids: Set[ObjectID]):
 
 def _get_object_id_dict_from_json(request) -> Dict[str, str]:
     # flask has a built in get_json() method but the errors it throws suck.
-    ids = json.loads(request.data)
+    ids = json.loads(request.get_data())
     if not isinstance(ids, dict):
         raise IllegalParameterError('Expected JSON mapping in request body')
     if not ids:
@@ -126,7 +116,7 @@ def _get_object_id_dict_from_json(request) -> Dict[str, str]:
 
 def _get_object_id_list_from_json(request) -> List[str]:
     # flask has a built in get_json() method but the errors it throws suck.
-    body = json.loads(request.data)
+    body = json.loads(request.get_data())
     if not isinstance(body, dict):
         raise IllegalParameterError('Expected JSON mapping in request body')
     ids = body.get('ids')
@@ -224,7 +214,6 @@ def get_namespaces():
     @app.route('/api/v1/mapping/<admin_ns>/<other_ns>', methods=['PUT', 'POST'])
     def create_mapping(admin_ns, other_ns):
         """ Create a mapping. """
-        request.get_data()  # see note 1) above
         authsource, token = _get_auth(request)
         ids = _get_object_id_dict_from_json(request)
         if len(ids) > 10000:
@@ -238,7 +227,6 @@ def create_mapping(admin_ns, other_ns):
     @app.route('/api/v1/mapping/<admin_ns>/<other_ns>', methods=['DELETE'])
     def remove_mapping(admin_ns, other_ns):
         """ Remove a mapping. """
-        request.get_data()  # see note 1) above
         authsource, token = _get_auth(request)
         ids = _get_object_id_dict_from_json(request)
         if len(ids) > 10000:
@@ -252,7 +240,6 @@ def remove_mapping(admin_ns, other_ns):
     @app.route('/api/v1/mapping/<ns>/', methods=['GET'])
     def get_mappings(ns):
         """ Find mappings. """
-        request.get_data()  # see note 1) above
         ns_filter = request.args.get('namespace_filter')
         separate = request.args.get('separate')
         if ns_filter and ns_filter.strip():

src/jgikbase/idmapping/storage/id_mapping_storage.py

@@ -155,6 +155,8 @@ def get_namespaces(self, nids: Iterable[NamespaceID]=None) -> Set[Namespace]:
 
         :param ids: specific namespaces to get. By default all namespaces are returned.
         :raises TypeError: if nids contains None.
+        :raises NoSuchNamespaceError: if any of the namespaces in the nids parameter do not
+            exist
         """
         raise NotImplementedError()
 

src/jgikbase/idmapping/storage/mongo/id_mapping_mongo_storage.py

@@ -11,7 +11,7 @@
 from jgikbase.idmapping.storage.errors import IDMappingStorageError, StorageInitException
 from jgikbase.idmapping.core.errors import NoSuchUserError, UserExistsError, InvalidTokenError,\
     NamespaceExistsError, NoSuchNamespaceError
-from typing import Set, Iterable, Tuple, Dict, Any  # @UnusedImport pydev gets confused here
+from typing import Set, Iterable, Tuple, Dict, Any, List  # @UnusedImport pydev gets confused here
 from jgikbase.idmapping.core.object_id import NamespaceID, Namespace, ObjectID
 
 # Testing the (many) catch blocks for the general mongo exception is pretty hard, since it
@@ -310,12 +310,18 @@ def set_namespace_publicly_mappable(self, namespace_id: NamespaceID, publicly_ma
 
     def get_namespaces(self, nids: Iterable[NamespaceID]=None) -> Set[Namespace]:
         query = {}
+        nidstr: List[str] = []
         if nids:
             no_Nones_in_iterable(nids, 'nids')
-            query[_FLD_NS_ID] = {'$in': [nid.id for nid in nids]}
+            nidstr = [nid.id for nid in nids]
+            query[_FLD_NS_ID] = {'$in': nidstr}
         try:
             nsdocs = self._db[_COL_NAMESPACES].find(query)
-            return {self._to_ns(nsdoc) for nsdoc in nsdocs}
+            nsobjs = {self._to_ns(nsdoc) for nsdoc in nsdocs}
+            if nidstr and len(nsobjs) != len(nidstr):
+                missing = set(nidstr) - {ns.namespace_id.id for ns in nsobjs}
+                raise NoSuchNamespaceError(str(sorted(missing)))
+            return nsobjs
         except PyMongoError as e:
             raise IDMappingStorageError('Connection to database failed: ' + str(e)) from e
 

src/jgikbase/test/idmapping/storage/mongo/test_id_mapping_mongo_storage.py

@@ -519,10 +519,22 @@ def test_get_namespaces_with_nids(idstorage):
     assert idstorage.get_namespaces(nids=nids) == set([expected[1], expected[2]])
 
 
-def test_get_namespaces_fail(idstorage):
+def test_get_namespaces_fail_None_input(idstorage):
+    fail_get_namespaces(idstorage, {NamespaceID('foo'), None}, TypeError('None item in nids'))
+
+
+def test_get_namespaces_fail_no_such_namepsace(idstorage):
+    idstorage.create_namespace(NamespaceID('foo'))
+    fail_get_namespaces(idstorage, {NamespaceID('zoo'), NamespaceID('foo'), NamespaceID('baz'),
+                                    NamespaceID('aioli_compote_drizzled_on_artisian_tater_tots')},
+                        NoSuchNamespaceError(
+                            "['aioli_compote_drizzled_on_artisian_tater_tots', 'baz', 'zoo']"))
+
+
+def fail_get_namespaces(idstorage, nids, expected):
     with raises(Exception) as got:
-        idstorage.get_namespaces({NamespaceID('foo'), None})
-    assert_exception_correct(got.value, TypeError('None item in nids'))
+        idstorage.get_namespaces(nids)
+    assert_exception_correct(got.value, expected)
 
 
 def test_add_and_get_mapping(idstorage):