Fix/lookup error 404

src/nsls2api/api/v1/user_api.py

@@ -9,59 +9,68 @@
     bnlpeople_service,
     person_service,
 )
+from nsls2api.services.bnlpeople_service import AmbiguousPersonLookupError
 from nsls2api.services.ldap_service import get_user_info, shape_ldap_response
 
 router = fastapi.APIRouter()
 
 
 @router.get("/person/username/{username}", response_model=Person)
 async def get_person_from_username(username: str):
-    bnl_person = await bnlpeople_service.get_person_by_username(username)
-    print(bnl_person)
-    if bnl_person:
-        person = Person(
-            firstname=bnl_person.FirstName,
-            lastname=bnl_person.LastName,
-            email=bnl_person.BNLEmail,
-            bnl_id=bnl_person.EmployeeNumber,
-            institution=bnl_person.Institution,
-            username=bnl_person.ActiveDirectoryName,
-            cyber_agreement_signed=bnl_person.CyberAgreementSigned,
-        )
-        # If the person is an Employee then set their institution to BNL
-        if (
-            bnl_person.EmployeeStatus == "Active"
-            and bnl_person.EmployeeType == "Employee"
-        ):
-            person.bnl_employee = True
-            person.institution = "Brookhaven National Laboratory"
-        return person
-    else:
-        return fastapi.responses.JSONResponse(
-            {"error": f"No people with username {username} found."},
+    try:
+        bnl_person = await bnlpeople_service.get_person_by_username(username)
+    except LookupError as e:
+        raise HTTPException(
             status_code=404,
-        )
+            detail=f"No person with username {username} was found.",
+        ) from None
+
+    person = Person(
+        firstname=bnl_person.FirstName,
+        lastname=bnl_person.LastName,
+        email=bnl_person.BNLEmail,
+        bnl_id=bnl_person.EmployeeNumber,
+        institution=bnl_person.Institution,
+        username=bnl_person.ActiveDirectoryName,
+        cyber_agreement_signed=bnl_person.CyberAgreementSigned,
+    )
+    # If the person is an Employee then set their institution to BNL
+    if (
+        bnl_person.EmployeeStatus == "Active"
+        and bnl_person.EmployeeType == "Employee"
+    ):
+        person.bnl_employee = True
+        person.institution = "Brookhaven National Laboratory"
+    return person
 
 
-@router.get("/person/email/{email}")
+@router.get("/person/email/{email}", response_model=Person)
 async def get_person_from_email(email: str):
-    bnl_person = await bnlpeople_service.get_person_by_email(email)
-    if bnl_person:
-        person = Person(
-            firstname=bnl_person.FirstName,
-            lastname=bnl_person.LastName,
-            email=bnl_person.BNLEmail,
-            bnl_id=bnl_person.EmployeeNumber,
-            institution=bnl_person.Institution,
-            username=bnl_person.ActiveDirectoryName,
-            cyber_agreement_signed=bnl_person.CyberAgreementSigned,
-        )
-        return person
-    else:
-        return fastapi.responses.JSONResponse(
-            {"error": f"No people with username {email} found."},
+    try:
+        bnl_person = await bnlpeople_service.get_person_by_email(email)
+    except LookupError as e:
+        raise HTTPException(
             status_code=404,
-        )
+            detail=f"No person with email {email} was found.",
+        ) from None
+
+    person = Person(
+        firstname=bnl_person.FirstName,
+        lastname=bnl_person.LastName,
+        email=bnl_person.BNLEmail,
+        bnl_id=bnl_person.EmployeeNumber,
+        institution=bnl_person.Institution,
+        username=bnl_person.ActiveDirectoryName,
+        cyber_agreement_signed=bnl_person.CyberAgreementSigned,
+    )
+    # If the person is an Employee then set their institution to BNL
+    if (
+        bnl_person.EmployeeStatus == "Active"
+        and bnl_person.EmployeeType == "Employee"
+    ):
+        person.bnl_employee = True
+        person.institution = "Brookhaven National Laboratory"
+    return person
 
 
 # TODO: Add back into schema if we decide to use this endpoint.

src/nsls2api/services/bnlpeople_service.py

@@ -9,6 +9,11 @@
 base_url = "https://api.bnl.gov/BNLPeople"
 
 
+class AmbiguousPersonLookupError(Exception):
+    """Raised when a person lookup returns multiple results (data integrity issue)."""
+    pass
+
+
 async def _call_bnlpeople_webservice(url: str):
     return await _call_async_webservice_with_client(url, client=httpx_client_wrapper())
 
@@ -19,12 +24,20 @@ async def get_all_people():
     return people
 
 
-async def get_person_by_username(username: str) -> BNLPerson | None:
+async def get_person_by_username(username: str) -> BNLPerson:
     url = f"{base_url}/api/BNLPeople?accountName={username}"
     person = await _call_bnlpeople_webservice(url)
-    if len(person) == 0 or len(person) > 1:
-        raise LookupError(
-            f"BNL People could not find a person with a username of '{username}'"
+    if len(person) == 0:
+        logger.warning(
+            f"BNL People API could not find a person with a username of '{username}'"
+        )
+        raise LookupError(f"BNL People API could not find a person with a username of '{username}'")
+    if len(person) > 1:
+        logger.error(
+            f"BNL People API returned {len(person)} people for username '{username}' - ambiguous result"
+        )
+        raise AmbiguousPersonLookupError(
+            f"BNL People API returned {len(person)} people for username '{username}' - ambiguous result"
         )
     return BNLPerson(**person[0])
 
@@ -44,7 +57,7 @@ async def get_username_by_id(lifenumber: str) -> str | None:
     # logger.debug(person)
     if len(person) == 0 or len(person) > 1:
         logger.warning(
-            f"BNL People could not find a person with an employee/life number of '{lifenumber}'"
+            f"BNL People API could not find a person with an employee/life number of '{lifenumber}'"
         )
         return None
 
@@ -67,17 +80,25 @@ async def get_person_by_id(lifenumber: str) -> BNLPerson | None:
 
     if len(person) == 0 or len(person) > 1:
         raise LookupError(
-            f"BNL People could not find a person with an employee/life number of '{lifenumber}'"
+            f"BNL People API could not find a person with an employee/life number of '{lifenumber}'"
         )
     return BNLPerson(**person[0])
 
 
-async def get_person_by_email(email: str) -> BNLPerson | None:
+async def get_person_by_email(email: str) -> BNLPerson:
     url = f"{base_url}/api/BNLPeople?email={email}"
     person = await _call_bnlpeople_webservice(url)
-    if len(person) == 0 or len(person) > 1:
-        raise LookupError(
-            f"BNL People could not find a person with an email of '{email}'"
+    if len(person) == 0:
+        logger.warning(
+            f"BNL People API could not find a person with an email of '{email}'"
+        )
+        raise LookupError(f"BNL People API could not find a person with an email of '{email}'")
+    if len(person) > 1:
+        logger.error(
+            f"BNL People API returned {len(person)} people for email '{email}' - ambiguous result"
+        )
+        raise AmbiguousPersonLookupError(
+            f"BNL People API returned {len(person)} people for email '{email}' - ambiguous result"
         )
     return BNLPerson(**person[0])
 
@@ -89,7 +110,7 @@ async def get_people_by_department(
     people = await _call_bnlpeople_webservice(url)
     if len(people) == 0:
         raise LookupError(
-            f"BNL People could not find a person with the department code of '{department_code}'"
+            f"BNL People API could not find a person with the department code of '{department_code}'"
         )
     people_in_department = [BNLPerson(**p) for p in people]
     return people_in_department

src/nsls2api/services/person_service.py

@@ -13,6 +13,7 @@
     n2sn_service,
     proposal_service,
 )
+from nsls2api.services.bnlpeople_service import AmbiguousPersonLookupError
 from nsls2api.services.pass_service import get_proposals_by_person
 
 
@@ -37,22 +38,11 @@ async def diagnostic_details_by_username(username: str) -> Person | None:
         )
         ad_groups = await n2sn_service.get_groups_by_username(username)
         proposals = await get_proposals_by_person(bnl_person.EmployeeNumber)
-    except LookupError as error:
+    except (LookupError, AmbiguousPersonLookupError) as error:
         raise LookupError(
             f"Error obtaining diagnostic details for username of {username}"
         ) from error
 
-    print(bnl_person)
-    print("-------")
-
-    print(ad_person)
-    print("-------")
-
-    print(ad_groups)
-    print("-------")
-
-    print(proposals)
-    print("-------")
 
     person = Person(
         firstname=bnl_person.FirstName,

src/nsls2api/services/proposal_service.py

@@ -28,6 +28,7 @@
     facility_service,
     pass_service,
 )
+from nsls2api.services.bnlpeople_service import AmbiguousPersonLookupError
 
 
 async def get_locked_proposals(
@@ -810,8 +811,7 @@ async def generate_fake_test_proposal(
                     is_pi=True,
                 )
                 user_list.append(user)
-        except LookupError:
-            logger.error(f"Could not find user {add_specific_user} in BNLPeople.")
+        except (LookupError, AmbiguousPersonLookupError):
             return None
 
     fake_proposal_id = await generate_fake_proposal_id()