Merge pull request #603 from alphagov/ldeb-fix-duns-not-found-error

Rewrite DirectPlusClient.get_organization_by_duns_number()

CHANGELOG.md

@@ -2,6 +2,17 @@
 
 Records breaking changes from major version bumps
 
+## 57.0.0
+
+### Breaking changes
+
+The Direct+ client no longer returns `None` if a DUNS number is in valid or not
+found.
+
+You must change calls of `DirectPlusClient.get_organization_by_duns_number()`
+to catch `DirectPlusError` exceptions or these errors will be propogated to
+users.
+
 ## 56.0.0
 
 Flask Redis session type is enabled by default.

dmutils/__init__.py

@@ -2,4 +2,4 @@
 from .flask_init import init_app
 
 
-__version__ = '56.1.2'
+__version__ = '57.0.0'

dmutils/direct_plus_client.py

@@ -1,12 +1,21 @@
 import logging
-from typing import Optional, cast
+from typing import Union, cast
 
 import requests
 from requests import HTTPError
 
+
+class DirectPlusError(Exception):
+    pass
+
+
 # See https://directplus.documentation.dnb.com/errorsAndInformationMessages.html
-DUNS_NUMBER_NOT_FOUND = 404, "10001"
-DUNS_NUMBER_INVALID = 400, "10003"
+class DUNSNumberNotFound(DirectPlusError):
+    status_code = 404
+
+
+class DUNSNumberInvalid(DirectPlusError):
+    status_code = 400
 
 
 class DirectPlusClient(object):
@@ -78,15 +87,18 @@ def _direct_plus_request(
             )
         return response
 
-    def get_organization_by_duns_number(self, duns_number) -> Optional[dict]:
+    def get_organization_by_duns_number(self, duns_number: Union[str, int]) -> dict:
         """
-        Request a supplier by duns number from the Direct Plus API
+        Request a organization by D-U-N-S number from the Direct+ API
 
-        :return the organisation corresponding to the DUNS number; or `None` if the number is invalid or no
-                corresponding organisation exists.
-        :raises KeyError on unexpected failure if the response body is JSON.
-        :raises ValueError on unexpected failure if the response body is not valid JSON.
+        :return: the organization details corresponding to the DUNS number
+        :raises DUNSNumberNotFound: if there is no organization for the DUNS number
+        :raises DUNSNumberInvalid: if `duns_number` is not a valid DUNS number
+        :raises DirectPlusError: if there is an unexpected error response from the API
         """
+
+        # request the company profile, linkage, and executives (v2)
+        # https://directplus.documentation.dnb.com/openAPI.html?apiID=cmpelkv2
         response = self._direct_plus_request(
             f'data/duns/{duns_number}', payload={'productId': 'cmpelk', 'versionId': 'v2'}
         )
@@ -95,12 +107,34 @@ def get_organization_by_duns_number(self, duns_number) -> Optional[dict]:
             response.raise_for_status()
         except HTTPError as exception:
             try:
-                error = response.json()['error']
+                error = response.json()["error"]
+            except (ValueError, KeyError) as e:
+                error_message = f"Unable to parse Direct+ API response: {response}: {e}"
+                raise DirectPlusError(error_message, exception)
 
-                if (response.status_code, error["errorCode"]) in [DUNS_NUMBER_INVALID, DUNS_NUMBER_NOT_FOUND]:
-                    return None
+            error_message = (
+                f"Unable to get supplier for DUNS number '{duns_number}' (HTTP error {response.status_code}): {error}"
+            )
+
+            if response.status_code in [DUNSNumberInvalid.status_code, DUNSNumberNotFound.status_code]:
+                self.logger.warning(error_message)
+
+                if response.status_code == DUNSNumberInvalid.status_code:
+                    raise DUNSNumberInvalid(error_message, exception)
+                if response.status_code == DUNSNumberNotFound.status_code:
+                    raise DUNSNumberNotFound(error_message, exception)
+
+            else:
+                self.logger.error(error_message)
+                raise DirectPlusError(error_message, exception)
+
+        try:
+            organization = cast(dict, response.json()['organization'])
+        except (ValueError, KeyError) as e:
+            # this should never happen, so let's propogate it so it is noisy
+            self.logger.error(
+                f"Unable to parse Direct+ API response: {response}: {e}"
+            )
+            raise
 
-                self.logger.error(f"Unable to get supplier by DUNS number: {error}")
-            except (ValueError, KeyError):
-                self.logger.error(f"Unable to get supplier by DUNS number: {exception}")
-        return cast(dict, response.json()['organization'])
+        return organization

tests/test_direct_plus_client.py

@@ -2,7 +2,12 @@
 import pytest
 import requests_mock
 
-from dmutils.direct_plus_client import DirectPlusClient
+from dmutils.direct_plus_client import (
+    DirectPlusClient,
+    DirectPlusError,
+    DUNSNumberInvalid,
+    DUNSNumberNotFound,
+)
 
 
 @pytest.fixture
@@ -52,29 +57,36 @@ def test_requests_call(self, duns_number, direct_plus_client, r_mock_with_token_
             params={'productId': 'cmpelk', 'versionId': 'v2'}
         )
 
-    def test_404_returns_none(self, direct_plus_client, r_mock_with_token_request):
+    @pytest.mark.parametrize("json_error", [
+        {"errorMessage": "DUNS not found", "errorCode": "10001"},
+        {"errorMessage": "Requested product not available due to insufficient data",
+         "errorCode": "40105", "errorDetails": []},
+    ])
+    def test_404_raises_duns_number_not_found(self, direct_plus_client, r_mock_with_token_request, json_error):
         r_mock_with_token_request.get(
-            'https://plus.dnb.com/v1/data/duns/123456789',
-            json={'error': {"errorMessage": 'DUNS not found', "errorCode": '10001'}},
+            "https://plus.dnb.com/v1/data/duns/123456789",
+            json={"error": json_error},
             status_code=404
         )
-        assert direct_plus_client.get_organization_by_duns_number(123456789) is None
+        with pytest.raises(DUNSNumberNotFound):
+            direct_plus_client.get_organization_by_duns_number(123456789)
 
-    def test_400_returns_none(self, direct_plus_client, r_mock_with_token_request):
+    def test_400_raises_duns_number_invalid(self, direct_plus_client, r_mock_with_token_request):
         r_mock_with_token_request.get(
             'https://plus.dnb.com/v1/data/duns/123456789',
             json={'error': {"errorMessage": 'Supplied DUNS number format is invalid', "errorCode": '10003'}},
             status_code=400
         )
-        assert direct_plus_client.get_organization_by_duns_number(123456789) is None
+        with pytest.raises(DUNSNumberInvalid):
+            direct_plus_client.get_organization_by_duns_number(123456789)
 
     def test_500_raises(self, direct_plus_client, r_mock_with_token_request):
         r_mock_with_token_request.get(
             'https://plus.dnb.com/v1/data/duns/123456789',
             json={'error': {"errorMessage": 'Server Error'}},
             status_code=500
         )
-        with pytest.raises(KeyError):
+        with pytest.raises(DirectPlusError):
             direct_plus_client.get_organization_by_duns_number(123456789)