Refactor to reduce code complexity and increase test coverage

Author: jermnelson

ils_middleware/tasks/sinopia/metadata_check.py

@@ -5,7 +5,7 @@
 import rdflib
 import requests  # type: ignore
 
-from typing import Optional
+from typing import Optional, Union
 
 logger = logging.getLogger(__name__)
 
@@ -33,6 +33,16 @@ def _query_for_ils_info(graph_jsonld: str, uri: str) -> dict:
     return output
 
 
+def _get_relationships(resource_uri: str) -> Union[list, None]:
+    result = requests.get(f"{resource_uri}/relationships")
+    if result.status_code > 399:
+        msg = f"Failed to retrieve {resource_uri}: {result.status_code}\n{result.text}"
+        logging.error(msg)
+        return None
+
+    return result.json().get("sinopiaHasLocalAdminMetadataInferredRefs")
+
+
 def _get_retrieve_metadata_resource(uri: str) -> Optional[dict]:
     """Retrieves AdminMetadata resource and extracts any ILS identifiers"""
     metadata_result = requests.get(uri)
@@ -76,13 +86,10 @@ def _retrieve_all_resource_refs(
         if target_resource_id:
             retrieved_resources[resource_uri] = [{default_ils: target_resource_id}]
             continue
-        result = requests.get(f"{resource_uri}/relationships")
-        if result.status_code > 399:
-            msg = f"Failed to retrieve {resource_uri}: {result.status_code}\n{result.text}"
-            logging.error(msg)
-            continue
 
-        metadata_uris = result.json().get("sinopiaHasLocalAdminMetadataInferredRefs")
+        metadata_uris = _get_relationships(resource_uri)
+        if metadata_uris is None:
+            continue
 
         ils_info = _retrieve_all_metadata(metadata_uris)
         if ils_info:

tests/tasks/sinopia/test_metadata_check.py

@@ -16,6 +16,7 @@
 from ils_middleware.tasks.sinopia.metadata_check import (
     existing_metadata_check,
     _get_retrieve_metadata_resource,
+    _get_relationships,
     _retrieve_all_metadata,
 )
 
@@ -184,3 +185,11 @@ def test_dups_in_retrieve_all_metadata(mock_requests):
     )
 
     assert result == [{"SIRSI": "13704749"}]
+
+
+def test_get_relationships_no_resource(mock_requests, caplog):
+    missing_result = _get_relationships("https://sinopia.io/resource/no-resource")
+    assert missing_result is None
+    assert (
+        "Failed to retrieve https://sinopia.io/resource/no-resource: 401" in caplog.text
+    )