Merge pull request #198 from metno/update_id_solr_dist

Change metadata_id to UUID in solrDist

dmci/distributors/distributor.py

@@ -120,8 +120,7 @@ def is_valid(self):
     @staticmethod
     def _construct_identifier(namespace, metadata_id):
         """Helper function to construct identifier from namespace and
-        UUID. Currently accepts empty namespaces, but later will only
-        accept correctly formed namespaced UUID
+        UUID. Currently only accepts correctly formed namespaced UUID
 
         Parameters
         ----------
@@ -136,8 +135,6 @@ def _construct_identifier(namespace, metadata_id):
             namespace:UUID or just UUID if namespace is empty.
         """
         if namespace != "":
-            identifier = namespace + ":" + str(metadata_id)
-        else:
-            identifier = str(metadata_id)
-        return identifier
+            return namespace + ":" + str(metadata_id)
+        raise ValueError("Namespace cannot be empty")
 # END Class Distributor

dmci/distributors/solr_dist.py

@@ -39,9 +39,9 @@ class SolRDist(Distributor):
     TOTAL_UPDATED = "total_updated"
     STATUS = [TOTAL_DELETED, TOTAL_INSERTED, TOTAL_UPDATED]
 
-    def __init__(self, cmd, xml_file=None, metadata_id=None, worker=None, **kwargs):
+    def __init__(self, cmd, xml_file=None, metadata_UUID=None, worker=None, **kwargs):
 
-        super().__init__(cmd, xml_file, metadata_id, worker, **kwargs)
+        super().__init__(cmd, xml_file, metadata_UUID, worker, **kwargs)
 
         self.authentication = self._init_authentication()
 
@@ -152,7 +152,6 @@ def _delete(self):
             identifier, commit=self._conf.commit_on_delete)
         logger.info("SolR delete status: %s. With response: %s" %
                     (str(status), str(msg)))
-        # return status, message
         return status, msg
 
 # END Class SolRDist

tests/test_dist/test_pycsw_dist.py

@@ -27,6 +27,15 @@
 from dmci.distributors.pycsw_dist import PyCSWDist
 
 
+class mockResp:
+    text = "Mock response"
+    status_code = 200
+
+
+class mockWorker:
+    _namespace = ""
+
+
 @pytest.mark.dist
 def testDistPyCSW_Init(tmpUUID):
     """Test the PyCSWDist class init."""
@@ -53,9 +62,6 @@ def testDistPyCSW_Run(tmpUUID):
 @pytest.mark.dist
 def testDistPyCSW_Insert(monkeypatch, mockXml, mockXslt):
     """Test insert commands via run()."""
-    class mockResp:
-        text = "Mock response"
-        status_code = 200
 
     # Insert returns True
     with monkeypatch.context() as mp:
@@ -85,9 +91,6 @@ class mockResp:
 @pytest.mark.dist
 def testDistPyCSW_Update(monkeypatch, mockXml, mockXslt, tmpUUID):
     """Test update commands via run()."""
-    class mockResp:
-        text = "Mock response"
-        status_code = 200
 
     tstWorker = Worker("update", None, None)
     tstWorker._file_metadata_id = tmpUUID
@@ -124,12 +127,6 @@ class mockResp:
 @pytest.mark.dist
 def testDistPyCSW_Delete(monkeypatch, mockXml, tmpUUID):
     """Test delete commands via run()."""
-    class mockResp:
-        text = "Mock response"
-        status_code = 200
-
-    class mockWorker:
-        _namespace = ""
 
     assert PyCSWDist("delete").run() == (False, "The run job is invalid")
     assert PyCSWDist("delete", xml_file=mockXml).run() == (False, "The run job is invalid")
@@ -140,20 +137,20 @@ class mockWorker:
             "dmci.distributors.pycsw_dist.requests.post", lambda *a, **k: mockResp
         )
         mp.setattr(PyCSWDist, "_get_transaction_status", lambda *a: True)
-        res = PyCSWDist("delete", metadata_UUID=tmpUUID, worker=mockWorker).run()
-        assert res == (True, "Mock response")
+        with pytest.raises(ValueError):
+            res = PyCSWDist("delete", metadata_UUID=tmpUUID, worker=mockWorker).run()
+
         mockWorker._namespace = "test.no"
         res = PyCSWDist("delete", metadata_UUID=tmpUUID, worker=mockWorker).run()
         assert res == (True, "Mock response")
 
-    mockWorker._namespace = ""
-
     # delete returns false
     with monkeypatch.context() as mp:
         mp.setattr(
             "dmci.distributors.pycsw_dist.requests.post", lambda *a, **k: mockResp
         )
         mp.setattr(PyCSWDist, "_get_transaction_status", lambda *a: False)
+        mockWorker._namespace = "test.no"
         res = PyCSWDist("delete", metadata_UUID=tmpUUID, worker=mockWorker).run()
         assert res == (False, "Mock response")
 

tests/test_dist/test_solr_dist.py

@@ -17,7 +17,7 @@
 limitations under the License.
 """
 import pytest
-
+import uuid
 from tools import causeException
 
 from dmci.distributors import SolRDist
@@ -50,12 +50,17 @@ def check_mmd(self, *args, **kwargs):
 
     def tosolr(self, *args, **kwargs):
         solr_formatted = {
-            'id': 'no-met-dev-250ba38f-1081-4669-a429-f378c569db32',
-            'metadata_identifier': 'no.met.dev:250ba38f-1081-4669-a429-f378c569db32',
+            'id': 'no-test-250ba38f-1081-4669-a429-f378c569db32',
+            'metadata_identifier': 'no.test:250ba38f-1081-4669-a429-f378c569db32',
         }
         return solr_formatted
 
 
+class mockResp:
+    text = "Mock response"
+    status_code = 200
+
+
 class mockWorker:
     _namespace = ""
 
@@ -65,10 +70,10 @@ def testDistSolR_Init(tmpUUID):
     """Test the SolRDist class init."""
     # Check that it initialises properly by running some of the simple
     # Distributor class tests
-    assert SolRDist("insert", metadata_id=tmpUUID).is_valid() is False
-    assert SolRDist("update", metadata_id=tmpUUID).is_valid() is False
-    assert SolRDist("delete", metadata_id=tmpUUID).is_valid() is True
-    assert SolRDist("blabla", metadata_id=tmpUUID).is_valid() is False
+    assert SolRDist("insert", metadata_UUID=tmpUUID).is_valid() is False
+    assert SolRDist("update", metadata_UUID=tmpUUID).is_valid() is False
+    assert SolRDist("delete", metadata_UUID=tmpUUID).is_valid() is True
+    assert SolRDist("blabla", metadata_UUID=tmpUUID).is_valid() is False
 
 
 @pytest.mark.dist
@@ -240,19 +245,24 @@ def testDistSolR_AddDocExists(mockXml, monkeypatch):
 
 
 @pytest.mark.dist
-def testDistSolR_Delete(monkeypatch):
-    """Test the SolRDist class delete actions."""
-    id = "no.met.dev:250ba38f-1081-4669-a429-f378c569db32"
-    tstDist = SolRDist("delete", metadata_id=id, worker=mockWorker)
+def testDistSolR_Delete(monkeypatch, mockXml):
+    """Test the SolRDist class delete via distributor.run()"""
+    md_uuid = uuid.UUID("250ba38f-1081-4669-a429-f378c569db32")
 
-    # Test delete exception Sucess
-    with monkeypatch.context() as mp:
-        mp.setattr("dmci.distributors.solr_dist.IndexMMD.delete",
-                   lambda *a, **k: (True, "Document %s sucessfully deleted" % id))
-        assert tstDist._delete() == (True, "Document %s sucessfully deleted" % id)
-        tstDist._delete()
-    # Test delete exception Fail
+    assert SolRDist("delete").run() == (False, "The run job is invalid")
+    assert SolRDist("delete", xml_file=mockXml).run() == (False, "The run job is invalid")
+
+    assert SolRDist("delete", metadata_UUID=md_uuid, worker=mockWorker).is_valid()
+
+    # mysolr.delete with and without namespace
     with monkeypatch.context() as mp:
-        mp.setattr("dmci.distributors.solr_dist.IndexMMD.delete",
-                   lambda *a, **k: (False, "Document %s not found in index." % id))
-        assert tstDist._delete() == (False, "Document %s not found in index." % id)
+        mp.setattr(
+            "dmci.distributors.solr_dist.IndexMMD.delete", lambda self, my_id, *b, **k:
+            ("Mock Response", my_id)
+        )
+        with pytest.raises(ValueError):
+            res = SolRDist("delete", metadata_UUID=md_uuid, worker=mockWorker).run()
+
+        mockWorker._namespace = "no.test"
+        res = SolRDist("delete", metadata_UUID=md_uuid, worker=mockWorker).run()
+        assert res == ("Mock Response", "no.test:250ba38f-1081-4669-a429-f378c569db32")