CIM-58 Change return types of delete_person and upsert_person

* Upsert: Return the response

* Delete: Return an i tuple of an enum describing whether person(s)
were deleted or if none was found and the response.

cim_client/client.py

@@ -2,6 +2,7 @@
 import logging
 import urllib.parse
 
+from enum import Enum
 from json import JSONDecodeError
 from typing import Tuple, Union, List, Iterable, Set, Any, Dict, Optional
 
@@ -23,6 +24,11 @@ logger = logging.getLogger(__name__)
 JsonType = Any
 
 
+class DeletePersonReply(Enum):
+    success = "success"
+    no_deletion = "no_deletion"
+
+
 def _serialise_upsert_person(
     person: PersonBase_T,
 ) -> Dict[str, Any]:
@@ -236,7 +242,7 @@ class CimClient(object):
     def upsert_persons(
         self,
         persondata: Union[List[PersonBase], PersonBase, PersonList],
-    ) -> Optional[Tuple[str, bytes]]:
+    ) -> requests.Response:
         """Update one or more persons
 
         Note that updating more than one person at a time may require parsing
@@ -260,14 +266,15 @@ class CimClient(object):
         response = self.post(url, json=data, auth=self.auth, headers=headers)
         if response.status_code == 200:
             logger.debug("import success, response was: %s", response.content)
-            return "Import success", response.content
+            return response
+
         response.raise_for_status()
         raise RuntimeError(f"Unexpected HTTP status: {response.status_code}")
 
     def delete_persons(
         self,
         person_ids: List[str],
-    ) -> Union[Tuple[str, bytes], None]:
+    ) -> Tuple[DeletePersonReply, requests.Response]:
         """Convenience method for deletion
 
         The request body should contain an array of unique identifiers
@@ -284,16 +291,16 @@ class CimClient(object):
 
         if (
             response.status_code == 404
-            and response.text == "No person found: No deletion"
+            and response.content == b"No person found: No deletion"
         ):
             logger.warning(
                 "No persons were deleted, response was: %s", response.content
             )
-            return None
+            return DeletePersonReply.no_deletion, response
 
         if response.status_code == 200:
             logger.debug("Delete success, response was: %s", response.content)
-            return "Delete success", response.content
+            return DeletePersonReply.success, response
 
         response.raise_for_status()
         raise RuntimeError(f"Unexpected HTTP status: {response.status_code}")

tests/test_client.py

@@ -5,6 +5,7 @@ from requests import HTTPError
 from cim_client.client import (
     CimClient,
     CimEndpoints,
+    DeletePersonReply,
     logger as client_logger,
     _find_unknown_parents,
     _serialise_upsert_person,
@@ -94,25 +95,38 @@ def test_get_delete_persons_schema(client, requests_mock):
 
 
 def test_delete_persons_success_200(client, john_doe, requests_mock):
-    """Ensure 404 returns None"""
+    """Ensure 200 returns proper response"""
     requests_mock.post(
         client.endpoints.delete_persons(),
         status_code=200,
         json=["Delete person: johndoe(9999)"],
     )
-    response = client.delete_persons(Person.from_dict(john_doe).username)
-    assert response == ("Delete success", b'["Delete person: johndoe(9999)"]')
+    reply, response = client.delete_persons(Person.from_dict(john_doe).username)
 
+    assert reply == DeletePersonReply.success
+    assert response.status_code == 200
+    assert response.content == b'["Delete person: johndoe(9999)"]'
 
-def test_delete_persons_success_404(client, john_doe, requests_mock):
+
+def test_delete_persons_success_404(client, john_doe, requests_mock, caplog):
     """Ensure 404 returns None when response text is `No person found: No deletion`"""
+    caplog.set_level(level=logging.WARNING, logger=client_logger.name)
     requests_mock.post(
         client.endpoints.delete_persons(),
         status_code=404,
         text="No person found: No deletion",
     )
-    response = client.delete_persons(Person.from_dict(john_doe).username)
-    assert response is None
+    reply, response = client.delete_persons(Person.from_dict(john_doe).username)
+
+    assert reply == DeletePersonReply.no_deletion
+    assert response.status_code == 404
+    assert response.content == b"No person found: No deletion"
+    warning = caplog.records[0]
+    assert warning.levelno == logging.WARNING
+    assert (
+        warning.message
+        == "No persons were deleted, response was: b'No person found: No deletion'"
+    )
 
 
 def test_delete_persons_failure_404(client, john_doe, requests_mock):
@@ -130,7 +144,8 @@ def test_upsert_persons_one(client, john_doe, requests_mock):
     """Ensure upserting one works"""
     requests_mock.post(client.endpoints.upsert_persons())
     response = client.upsert_persons(Person.from_dict(john_doe))
-    assert response == ("Import success", b"")
+
+    assert response.status_code == 200
 
 
 def test_upsert_persons_many(client, john_doe, jane_doe, requests_mock):
@@ -139,7 +154,8 @@ def test_upsert_persons_many(client, john_doe, jane_doe, requests_mock):
 
     requests_mock.post(client.endpoints.upsert_persons())
     response = client.upsert_persons(personlist)
-    assert response == ("Import success", b"")
+
+    assert response.status_code == 200
 
 
 def test_upsert_persons_failure_400(client, john_doe, requests_mock):