Pass more descriptive codes and messages back to the user through the…

… API

dmci/api/app.py

@@ -25,7 +25,7 @@
 import uuid
 
 from lxml import etree
-from flask import request, Flask, after_this_request
+from flask import request, Flask
 
 from dmci.api.worker import Worker
 
@@ -55,33 +55,40 @@ def __init__(self):
 
         # Set up api entry points
         @self.route("/v1/insert", methods=["POST"])
-        def base():
+        def post_insert():
             max_permitted_size = self._conf.max_permitted_size
 
             if request.content_length > max_permitted_size:
                 return f"File bigger than permitted size: {max_permitted_size}", 413
 
             data = request.get_data()
 
+            # Cache the job file
             file_uuid = uuid.uuid4()
             path = self._conf.distributor_cache
             full_path = os.path.join(path, f"{file_uuid}.Q")
+            msg, code = self._persist_file(data, full_path)
+            if code != 200:
+                return msg, code
 
+            # Run the validator
             worker = Worker(full_path, self._xsd_obj)
-
-            @after_this_request
-            def dist(response):
-                nonlocal worker
-                worker.distribute()
-                return response
-
-            result, msg = worker.validate(data)
-            if result:
-                return self._persist_file(data, full_path)
+            valid, msg = worker.validate(data)
+            if not valid:
+                return msg, 400
+
+            # Run the distributors
+            err = []
+            status, valid, _, failed, skipped = worker.distribute()
+            if not status:
+                err.append("The following distributors failed: %s" % ", ".join(failed))
+            if not valid:
+                err.append("The following jobs were skipped: %s" % ", ".join(skipped))
+
+            if err:
+                return "\n".join(err), 500
             else:
-                return msg, 500
-
-            # TODO: shouldn't msg be logged?
+                return "Everything is OK", 200
 
         return
 
@@ -99,7 +106,7 @@ def _persist_file(data, full_path):
 
         except Exception as e:
             logger.error(str(e))
-            return "Can't write to file", 507
+            return "Cannot write xml data to cache file", 507
 
         return "", 200
 

dmci/api/worker.py

@@ -111,13 +111,15 @@ def distribute(self):
                 **self._kwargs
             )
             valid &= obj.is_valid()
-            if obj.is_valid:
+            if obj.is_valid():
                 obj_status = obj.run()
                 status &= obj_status
                 if obj_status:
                     called.append(dist)
                 else:
                     failed.append(dist)
+            else:
+                skipped.append(dist)
 
         return status, valid, called, failed, skipped
 

tests/test_api/test_app.py

@@ -108,13 +108,33 @@ def testApiApp_InsertRequests(client, filesDir, monkeypatch):
     tooLargeFile = bytes(3000000)
     assert client.post("/v1/insert", data=tooLargeFile).status_code == 413
 
+    # Fail cahcing the file
+    with monkeypatch.context() as mp:
+        mp.setattr("builtins.open", causeOSError)
+        assert client.post("/v1/insert", data=MOCK_XML).status_code == 507
+
+    # Data is valid
     with monkeypatch.context() as mp:
         mp.setattr("dmci.api.app.Worker.validate", lambda *a: (True, ""))
         assert client.post("/v1/insert", data=MOCK_XML).status_code == 200
 
+    # Data is not valid
     with monkeypatch.context() as mp:
         mp.setattr("dmci.api.app.Worker.validate", lambda *a: (False, ""))
-        assert client.post("/v1/insert", data=MOCK_XML).status_code == 500
+        assert client.post("/v1/insert", data=MOCK_XML).status_code == 400
+
+    # Data is valid, distribute fails
+    with monkeypatch.context() as mp:
+        fail = ["A", "B"]
+        skip = ["C"]
+        mp.setattr("dmci.api.app.Worker.validate", lambda *a: (True, ""))
+        mp.setattr("dmci.api.app.Worker.distribute", lambda *a: (False, False, [], fail, skip))
+        response = client.post("/v1/insert", data=MOCK_XML)
+        assert response.status_code == 500
+        assert response.data == (
+            b"The following distributors failed: A, B\n"
+            b"The following jobs were skipped: C"
+        )
 
 # END Test testApiApp_InsertRequests
 

tests/test_api/test_worker.py

@@ -59,15 +59,34 @@ def testApiWorker_Distributor(tmpDir, tmpConf, mockXml, monkeypatch):
         assert failed == []
         assert skipped == ["blabla"]
 
+    # Same as above, but jobs fail
+    with monkeypatch.context() as mp:
+        mp.setattr(FileDist, "run", lambda *a: False)
+        mp.setattr(PyCSWDist, "run", lambda *a: False)
+
+        tstWorker = Worker(None, None)
+        tstWorker._conf = tmpConf
+        tstWorker._dist_xml_file = mockXml
+
+        status, valid, called, failed, skipped = tstWorker.distribute()
+        assert status is False
+        assert valid is True
+        assert called == []
+        assert failed == ["file", "pycsw"]
+        assert skipped == ["blabla"]
+
     # Call the distributor function with the wrong parameters
     tstWorker = Worker(None, None)
     tstWorker._conf = tmpConf
     tstWorker._dist_cmd = "blabla"
     tstWorker._dist_xml_file = "/path/to/nowhere"
 
-    status, valid, _, _, _ = tstWorker.distribute()
-    assert status is False
-    assert valid is False
+    status, valid, called, failed, skipped = tstWorker.distribute()
+    assert status is True # No jobs were run since all were skipped
+    assert valid is False # All jobs were invalid due to the command
+    assert called == []
+    assert failed == []
+    assert skipped == ["file", "pycsw", "blabla"]
 
 # END Test testApiWorker_Distributor