From d8129bf4a336bcf75374310efa487dc0b4340e91 Mon Sep 17 00:00:00 2001 From: Thomas Thorogood Date: Wed, 8 Sep 2021 17:15:14 -0700 Subject: [PATCH] Optimization and fine-tuning (#101) * Move all gunicorn config into gunicorn.conf.py and update configuration to, like, work. * Remove unused, empty directory. * Blacken gunicorn.conf.py * Optimize search output import and translation - Make `_GLOBAL_CONSTRAINTS` a constant instead of generating on each request - Don't import the raw data to the `ListPersonsOutput` model until AFTER it has been filtered - Simplify RecordConstraint logic by only operating on Dicts, instead of Generic Types (which also speeds things up quite a lot) - Stop duplicate filtering in translator.py that is already handled by PWS. - Update tests to work with new logic. * Fix minor typo in pws model documentation * [Bot] Update version to 2.0.2 Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- docker/development-server.dockerfile | 3 - husky_directory/gunicorn.conf.py | 27 ++- husky_directory/models/common.py | 33 ++-- husky_directory/models/pws.py | 4 +- husky_directory/services/pws.py | 174 +++++++++++--------- husky_directory/services/query_generator.py | 30 ++-- husky_directory/services/translator.py | 15 +- pyproject.toml | 2 +- tests/blueprints/test_search_blueprint.py | 2 +- tests/conftest.py | 11 +- tests/models/test_common_models.py | 3 - tests/services/test_pws.py | 2 + tests/services/test_search.py | 24 +-- tests/services/test_translator.py | 9 +- tests/services/test_vcard.py | 7 +- web-tests | 0 16 files changed, 182 insertions(+), 164 deletions(-) delete mode 100644 web-tests diff --git a/docker/development-server.dockerfile b/docker/development-server.dockerfile index f6fcc785..6bd252f1 100644 --- a/docker/development-server.dockerfile +++ b/docker/development-server.dockerfile @@ -22,8 +22,5 @@ ENV FLASK_PORT=8000 \ # 0.0.0.0 binding is necessary for the EXPOSE above to have any effect. CMD poetry run gunicorn -b 0.0.0.0:${FLASK_PORT} \ - --log-level ${GUNICORN_LOG_LEVEL} \ -c "/app/husky_directory/gunicorn.conf.py" \ - --reload \ - --capture-output \ "husky_directory.app:create_app()" diff --git a/husky_directory/gunicorn.conf.py b/husky_directory/gunicorn.conf.py index 64e08624..d6d1a33f 100644 --- a/husky_directory/gunicorn.conf.py +++ b/husky_directory/gunicorn.conf.py @@ -1,9 +1,26 @@ -import gevent.monkey - -gevent.monkey.patch_all() - -worker_class = "gevent" +import os +from multiprocessing import cpu_count def worker_exit(worker, server): worker.log.info(f"Server {server} shutting down . . .") + + +def max_workers(): + default_max = 2 * cpu_count() + 1 + return os.environ.get("GUNICORN_MAX_WORKERS", default_max) + + +def max_threads(): + default_max = 4 + return os.environ.get("GUNICORN_MAX_THREADS", default_max) + + +max_requests = 1000 +bind = "0.0.0.0:8000" +worker_class = "gthread" +workers = max_workers() +threads = max_threads() +loglevel = os.environ.get("GUNICORN_LOG_LEVEL", "DEBUG") +reload = os.environ.get("FLASK_ENV") == "development" +preload_app = os.environ.get("FLASK_ENV") != "development" diff --git a/husky_directory/models/common.py b/husky_directory/models/common.py index b2bf5010..4bde970f 100644 --- a/husky_directory/models/common.py +++ b/husky_directory/models/common.py @@ -7,13 +7,10 @@ """ from __future__ import annotations -import json import re -from types import SimpleNamespace -from typing import Any, Callable, Generic, Optional, TypeVar, Union +from typing import Any, Callable, Dict, Optional, Union from pydantic import BaseModel, validator -from pydantic.generics import GenericModel class UWDepartmentRole(BaseModel): @@ -43,10 +40,7 @@ def validate(cls, v) -> RecordNamespace: return v -T = TypeVar("T") - - -class RecordConstraint(GenericModel, Generic[T]): +class RecordConstraint(BaseModel): """ This model helps us apply additional client-side filters to result records. A record can be an object or a dict. @@ -92,13 +86,11 @@ def callback(record: PersonOutput): assert data_2['foo']['bar'] == 'temp' """ - namespace: Union[ - RecordNamespace, str - ] # Will always be converted to a RecordNamespace - predicate: Callable[[T], bool] = lambda _: False - failure_callback: Callable[[T], bool] = lambda _: False + namespace: Optional[Any] + predicate: Callable[[Any], bool] = lambda _: False + failure_callback: Callable[[Dict], bool] = lambda _: False - def resolve_namespace(self, record: Any) -> Optional[Any]: + def resolve_namespace(self, record: Dict) -> Optional[Any]: """ Given a record, resolves the value of the namespace. The record can be anything. If it is a dict, the dict will be evaluated as namespace for the scope of this method. @@ -109,21 +101,18 @@ def resolve_namespace(self, record: Any) -> Optional[Any]: :param record: The thing you want to test; it can be anything! :return: The value of the resolved namespace, or None. """ - if isinstance(record, dict): - # Converts the dict into an object. - record = json.loads( - json.dumps(record), object_hook=lambda item: SimpleNamespace(**item) - ) - resolved = record for token in self.namespace.split("."): - resolved = getattr(resolved, token, None) + try: + resolved = resolved.get(token) + except AttributeError: + resolved = None if not resolved: break return resolved - def matches(self, record: T) -> bool: + def matches(self, record: Dict) -> bool: """ Given a thing, returns whether the thing matches the predicate or the failure callback. diff --git a/husky_directory/models/pws.py b/husky_directory/models/pws.py index edfb2f25..ec6a3649 100644 --- a/husky_directory/models/pws.py +++ b/husky_directory/models/pws.py @@ -164,7 +164,7 @@ class PersonAffiliations(PWSBaseModel): class NamedIdentity(PWSBaseModel): - display_name: Optional[str] = Field(..., alias="DisplayName") + display_name: Optional[str] = Field(...) registered_name: Optional[str] registered_surname: Optional[str] registered_first_middle_name: Optional[str] @@ -201,7 +201,7 @@ class PersonOutput(NamedIdentity): @validator("href", always=True) def populate_href(cls, href: Optional[str], values: Dict): """ - While abbreviated surch results include the href, the full + While abbreviated search results include the href, the full results do not, so it has to be generated. """ if not href: diff --git a/husky_directory/services/pws.py b/husky_directory/services/pws.py index 58ac9efc..d75620e3 100644 --- a/husky_directory/services/pws.py +++ b/husky_directory/services/pws.py @@ -2,7 +2,7 @@ from collections import namedtuple from functools import partial from logging import Logger -from typing import Any, Dict, List, Optional, Type, TypeVar, Union, cast +from typing import Any, Dict, List, Optional, Type, TypeVar, Union import requests from flask_injector import request @@ -27,8 +27,76 @@ ) +def clear_namespace(namespace, record: Any) -> bool: + """ + This callback allows attributes to be cleared if they + do not match the criteria, instead of throwing away the entire record. + Practically: if the user has opted out of employee or student publication, + then we null the respective attributes. + """ + ns = namespace.split(".") + attr = ns.pop( + -1 + ) # Allows us to resolve to the level _above_ the field being cleared + constraint = RecordConstraint.construct(namespace=".".join(ns)) + field = constraint.resolve_namespace(record) + if field and attr in field: + del field[attr] + return True + + @request class PersonWebServiceClient: + _GLOBAL_CONSTRAINTS = [ + RecordConstraint( + # If the identity has no netid, we are not interested + # in the record. + namespace="UWNetID", + predicate=bool, + ), + RecordConstraint( + # If the identity is not eligible for publication, + # we are not interested in the record. + namespace="WhitepagesPublish", + predicate=bool, + ), + RecordConstraint( + # TODO: Support including test entities here, + # which will need a minor refactor of how that param + # is passed around. For now, we simply invert + # the bool representation: if we have a test entity, + # then we return False (and exclude the record). + namespace="IsTestEntity", + predicate=lambda v: not bool(v), + ), + RecordConstraint( + # If there is a student affiliation and the affiliation's + # publication flag is true, then accept it; + # otherwise clear the affiliation from the record and + # continue processing. + namespace=( + "PersonAffiliations.StudentPersonAffiliation.StudentWhitePages.PublishInDirectory" + ), + predicate=bool, + failure_callback=partial( + clear_namespace, "PersonAffiliations.StudentPersonAffiliation" + ), + ), + RecordConstraint( + # If there is an employee affiliation and the affiliation's + # publication flag is true, then accept it; + # otherwise clear the affiliation from the record and + # continue processing. + namespace=( + "PersonAffiliations.EmployeePersonAffiliation.EmployeeWhitePages.PublishInDirectory" + ), + predicate=bool, + failure_callback=partial( + clear_namespace, "PersonAffiliations.EmployeePersonAffiliation" + ), + ), + ] + @inject def __init__( self, @@ -75,46 +143,56 @@ def _get_sanitized_request_output( :return: The output; always returns a value, unless PWS raises an error. """ constraints = constraints or self.global_constraints - output = self._get_search_request_output(url, params, output_type) + output = self._get_search_request_output(url, params) if output_type is ListPersonsOutput: - output = cast(ListPersonsOutput, output) - # Post-process the output to remove items that do not meet + # Pre-process the output to remove items that do not meet # the provided client-side constraints. - output.persons = list( - filter(partial(self._filter_output_item, constraints), output.persons) + output["Persons"] = list( + filter( + partial(self._filter_output_item, constraints), output["Persons"] + ) ) elif output_type is PersonOutput: - output = cast(PersonOutput, output) if not self._filter_output_item(constraints, output): raise NotFound + output = output_type.parse_obj(output) return output def _get_search_request_output( self, url: str, params: Optional[Dict] = None, - output_type: Type[OutputReturnType] = ListPersonsOutput, - ) -> OutputReturnType: + ) -> Dict: response = requests.get( url, cert=self.cert, params=params, headers={"Accept": "application/json"} ) self.logger.info(f"[GET] {response.url} : {response.status_code}") response.raise_for_status() data = response.json() - output = output_type.parse_obj(data) - return output + return data def _filter_output_item( - self, constraints: List[RecordConstraint], target: PersonOutput + self, + constraints: List[RecordConstraint], + target: Dict, ) -> bool: - if target.affiliations.student and not self.auth.request_is_authenticated: - target.affiliations.student = None + affiliations: Dict = target.get("PersonAffiliations") + + if affiliations and not self.auth.request_is_authenticated: + # If the request is not authenticated, trash student + # data here and now, before it gets exported into + # a model. + affiliations.pop("StudentPersonAffiliation", None) + if not affiliations.get("EmployeePersonAffiliation"): + affiliations.pop("EmployeePersonAffiliation", None) - for constraint in constraints: - if not constraint.matches(target): - return False + # Don't use elif here, because the first + # predicate (above) may modify the dictionary, so we + # must re-check that the dict is populated. + if not affiliations: + return False - return bool(target.affiliations.student) or bool(target.affiliations.employee) + return all(constraint.matches(target) for constraint in constraints) def get_explicit_href( self, page_url: str, output_type: Type[OutputReturnType] = ListPersonsOutput @@ -138,65 +216,7 @@ def global_constraints(self) -> List[RecordConstraint]: """ These constraints are applied to every public request method in this application. """ - - def clear_namespace(namespace, record: Any) -> bool: - """ - This callback allows attributes to be cleared if they - do not match the criteria, instead of throwing away the entire record. - Practically: if the user has opted out of employee or student publication, - then we null the respective attributes. - """ - ns = namespace.split(".") - attr = ns.pop( - -1 - ) # Allows us to resolve to the level _above_ the field being cleared - constraint = RecordConstraint.construct(namespace=".".join(ns)) - field = constraint.resolve_namespace(record) - if field: - setattr(field, attr, None) - return True - - return [ - RecordConstraint( - # If the identity has no netid, we are not interested - # in the record. - namespace="netid", - predicate=bool, - ), - RecordConstraint( - # If the identity is not eligible for publication, - # we are not interested in the record. - namespace="whitepages_publish", - predicate=bool, - ), - RecordConstraint( - # TODO: Support including test entities here, - # which will need a minor refactor of how that param - # is passed around. For now, we simply invert - # the bool representation: if we have a test entity, - # then we return False (and exclude the record). - namespace="is_test_entity", - predicate=lambda v: not bool(v), - ), - RecordConstraint( - # If there is a student affiliation and the affiliation's - # publication flag is true, then accept it; - # otherwise clear the affiliation from the record and - # continue processing. - namespace="affiliations.student.directory_listing.publish_in_directory", - predicate=bool, - failure_callback=partial(clear_namespace, "affiliations.student"), - ), - RecordConstraint( - # If there is an employee affiliation and the affiliation's - # publication flag is true, then accept it; - # otherwise clear the affiliation from the record and - # continue processing. - namespace="affiliations.employee.directory_listing.publish_in_directory", - predicate=bool, - failure_callback=partial(clear_namespace, "affiliations.employee"), - ), - ] + return self._GLOBAL_CONSTRAINTS def list_persons(self, request_input: ListPersonsInput) -> ListPersonsOutput: """ diff --git a/husky_directory/services/query_generator.py b/husky_directory/services/query_generator.py index 7176d08c..0308875e 100644 --- a/husky_directory/services/query_generator.py +++ b/husky_directory/services/query_generator.py @@ -136,7 +136,7 @@ class SearchQueryGenerator: last_name=name, constraints=[ RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_matches, name ), @@ -150,7 +150,7 @@ class SearchQueryGenerator: last_name=ArgFmt.begins_with(name), constraints=[ RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_begins_with, name ), @@ -164,7 +164,7 @@ class SearchQueryGenerator: first_name=name, constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_matches, name ), @@ -178,7 +178,7 @@ class SearchQueryGenerator: display_name=ArgFmt.contains(name), constraints=[ RecordConstraint( - namespace="display_name", + namespace="DisplayName", predicate=partial( ConstraintPredicates.null_or_includes, name ), @@ -195,13 +195,13 @@ class SearchQueryGenerator: last_name=last, constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_matches, first ), ), RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_matches, last ), @@ -216,13 +216,13 @@ class SearchQueryGenerator: last_name=last, constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_begins_with, first ), ), RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_matches, last ), @@ -237,13 +237,13 @@ class SearchQueryGenerator: last_name=ArgFmt.begins_with(last), constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_matches, first ), ), RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_begins_with, last ), @@ -258,13 +258,13 @@ class SearchQueryGenerator: last_name=ArgFmt.begins_with(last), constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_begins_with, first ), ), RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_begins_with, last ), @@ -278,7 +278,7 @@ class SearchQueryGenerator: last_name=ArgFmt.begins_with(*args), constraints=[ RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_begins_with, " ".join(args) ), @@ -316,13 +316,13 @@ def _generate_query(*args: str): last_name=last_name, constraints=[ RecordConstraint( - namespace="preferred_first_name", + namespace="PreferredFirstName", predicate=partial( ConstraintPredicates.null_or_begins_with, first_name ), ), RecordConstraint( - namespace="preferred_last_name", + namespace="PreferredSurname", predicate=partial( ConstraintPredicates.null_or_begins_with, last_name ), diff --git a/husky_directory/services/translator.py b/husky_directory/services/translator.py index 9594ccd9..f55a64d6 100644 --- a/husky_directory/services/translator.py +++ b/husky_directory/services/translator.py @@ -7,7 +7,6 @@ from husky_directory.models.pws import ( EmployeePersonAffiliation, ListPersonsOutput, - PersonOutput, StudentPersonAffiliation, ) from husky_directory.models.search import ( @@ -83,18 +82,10 @@ def translate_scenario( ), } - def filter_(person_: PersonOutput) -> bool: - """ - Ignores entities we've already catalogued [as they may have been returned - in both the student search and the employee search], and entities - who have had all of their guts filtered out by the output - constraint filtering. - """ - return person_.netid not in netid_tracker and ( - person_.affiliations.student or person_.affiliations.employee - ) + for person in request_output.persons: + if person.netid in netid_tracker: + continue - for person in filter(filter_, request_output.persons): student = person.affiliations.student employee = person.affiliations.employee diff --git a/pyproject.toml b/pyproject.toml index 5f9916f1..f4f65d7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "uw-husky-directory" -version = "2.0.1" +version = "2.0.2" description = "An updated version of the UW Directory" authors = ["Thomas Thorogood "] license = "MIT" diff --git a/tests/blueprints/test_search_blueprint.py b/tests/blueprints/test_search_blueprint.py index a9e4bdef..8b900758 100644 --- a/tests/blueprints/test_search_blueprint.py +++ b/tests/blueprints/test_search_blueprint.py @@ -422,7 +422,7 @@ def test_get_person_vcard(self): """ person = self.mock_people.published_employee href = base64.b64encode("foo".encode("UTF-8")).decode("UTF-8") - self.mock_send_request.return_value = person + self.mock_send_request.return_value = person.dict(by_alias=True) response = self.flask_client.get(f"/search/person/{href}/vcard") assert response.status_code == 200 assert response.mimetype == "text/vcard" diff --git a/tests/conftest.py b/tests/conftest.py index a04dfee7..1f4e5275 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -74,10 +74,10 @@ def inner(**attrs: Any) -> PersonOutput: registered_name="Ada Lovelace", registered_surname="Lovelace", registered_first_middle_name="Ada", - whitepages_publish=True, is_test_entity=False, netid=netid, href=f"person/{netid}", + whitepages_publish=True, ) return default.copy(update=attrs) @@ -108,7 +108,7 @@ class People: ], ), ) - ) + ), ) unpublished_employee = generate_person( affiliations=PersonAffiliations( @@ -166,15 +166,16 @@ class People: @staticmethod def as_search_output( *people: PersonOutput, next_: Optional[str] = None - ) -> ListPersonsOutput: - return ListPersonsOutput( + ) -> Dict: + result = ListPersonsOutput( persons=list(people), current=ListPersonsInput(), # Not used page_size=len(people), page_start=1, total_count=len(people), next=next_, - ) + ).dict(by_alias=True) + return result @property def published_student_employee(self) -> PersonOutput: diff --git a/tests/models/test_common_models.py b/tests/models/test_common_models.py index be06d257..f42d9517 100644 --- a/tests/models/test_common_models.py +++ b/tests/models/test_common_models.py @@ -1,5 +1,3 @@ -from types import SimpleNamespace - import pytest from pydantic import ValidationError @@ -11,7 +9,6 @@ class TestRecordConstraint: "record", [ {"foo": "bar", "baz": {"bop": "buzz"}}, - SimpleNamespace(foo="bar", baz=SimpleNamespace(bop="buzz")), ], ) @pytest.mark.parametrize( diff --git a/tests/services/test_pws.py b/tests/services/test_pws.py index cdabbd53..35214d24 100644 --- a/tests/services/test_pws.py +++ b/tests/services/test_pws.py @@ -23,12 +23,14 @@ def test_pws_url(self): def test_get_explicit_href(self): expected_url = f"{self.client.pws_url}/foobar" + self.mock_send_request.return_value = self.mock_people.as_search_output() self.client.get_explicit_href("/identity/v2/foobar") self.mock_send_request.assert_called_once() assert self.mock_send_request.call_args[0][0] == expected_url def test_list_persons(self): request_input = ListPersonsInput(display_name="test") + self.mock_send_request.return_value = self.mock_people.as_search_output() expected_url = f"{self.client.pws_url}/person" self.client.list_persons(request_input) self.mock_send_request.assert_called_once() diff --git a/tests/services/test_search.py b/tests/services/test_search.py index c5f452b3..18390a7a 100644 --- a/tests/services/test_search.py +++ b/tests/services/test_search.py @@ -1,4 +1,5 @@ from contextlib import ExitStack +from typing import Dict from unittest import mock import pytest @@ -7,7 +8,6 @@ from husky_directory.models.common import UWDepartmentRole from husky_directory.models.enum import PopulationType from husky_directory.models.pws import ( - ListPersonsInput, ListPersonsOutput, ) from husky_directory.models.search import Person, SearchDirectoryInput @@ -36,7 +36,7 @@ def configure_base(self, injector, mock_people, mock_injected): self.pws = injector.get(PersonWebServiceClient) stack.enter_context(mock_injected(PersonWebServiceClient, self.pws)) self.mock_list_persons = stack.enter_context( - mock.patch.object(self.pws, "list_persons") + mock.patch.object(self.pws, "_get_search_request_output") ) self.mock_get_explicit_href = stack.enter_context( mock.patch.object(self.pws, "get_explicit_href") @@ -48,7 +48,7 @@ def configure_base(self, injector, mock_people, mock_injected): self.client: DirectorySearchService = injector.get(DirectorySearchService) yield - def set_list_persons_output(self, output: ListPersonsOutput): + def set_list_persons_output(self, output: Dict): self.list_persons_output = output self.mock_list_persons.return_value = output @@ -65,11 +65,11 @@ def test_search_directory_happy(self): assert output.scenarios[0].populations["employees"].people def test_search_removes_duplicates(self): - dupe = self.mock_people.as_search_output(self.mock_people.published_employee) - self.set_list_persons_output( - dupe.copy(update={"next": ListPersonsInput(href="foo")}) - ) - self.set_get_next_output(dupe) + orig = self.mock_people.as_search_output(self.mock_people.published_employee) + dupe = orig.copy() + orig["Next"] = {"Href": "foo"} + self.set_list_persons_output(orig) + self.set_get_next_output(ListPersonsOutput.parse_obj(dupe)) request_input = SearchDirectoryInput(name="foo") output = self.client.search_directory(request_input) @@ -78,7 +78,7 @@ def test_search_removes_duplicates(self): def test_output_includes_phones(self): person = self.mock_people.contactable_person - self.list_persons_output.persons = [person] + self.list_persons_output["Persons"] = [person.dict(by_alias=True)] self.session["uwnetid"] = "foo" request_input = SearchDirectoryInput(name="foo") output = self.client.search_directory(request_input) @@ -161,7 +161,7 @@ def test_output_includes_department(self, person: str, expected_departments): input_population = PopulationType(input_population) person = getattr(self.mock_people, person) - self.list_persons_output.persons = [person] + self.list_persons_output["Persons"] = [person.dict(by_alias=True)] request_input = SearchDirectoryInput( name="Lovelace", population=input_population ) @@ -176,7 +176,7 @@ def test_output_includes_department(self, person: str, expected_departments): def test_department_ignores_invalid_data(self): person = self.mock_people.published_employee person.affiliations.employee.directory_listing.positions[0].department = None - self.list_persons_output.persons = [person] + self.list_persons_output["Persons"] = [person.dict(by_alias=True)] request_input = SearchDirectoryInput( name="whatever", population=PopulationType.employees ) @@ -186,7 +186,7 @@ def test_department_ignores_invalid_data(self): def test_output_includes_box_number(self): person = self.mock_people.published_employee - self.list_persons_output.persons = [person] + self.list_persons_output["Persons"] = [person.dict(by_alias=True)] output = self.client.search_directory( SearchDirectoryInput(name="*blah", population=PopulationType.employees) ) diff --git a/tests/services/test_translator.py b/tests/services/test_translator.py index 986c9f22..5848b652 100644 --- a/tests/services/test_translator.py +++ b/tests/services/test_translator.py @@ -2,6 +2,7 @@ from werkzeug.local import LocalProxy from husky_directory.models.enum import PopulationType +from husky_directory.models.pws import ListPersonsOutput from husky_directory.services.translator import ListPersonsOutputTranslator @@ -20,9 +21,11 @@ def translator(self) -> ListPersonsOutputTranslator: return self.injector.get(ListPersonsOutputTranslator) def test_translate_scenario(self, generate_person): - pws_output = self.mock_people.as_search_output( - self.mock_people.contactable_person, - generate_person(), # This empty person should be ignored + pws_output = ListPersonsOutput.parse_obj( + self.mock_people.as_search_output( + self.mock_people.contactable_person, + generate_person(), # This empty person should be ignored + ) ) netid_tracker = set() diff --git a/tests/services/test_vcard.py b/tests/services/test_vcard.py index 4adea911..da4974af 100644 --- a/tests/services/test_vcard.py +++ b/tests/services/test_vcard.py @@ -103,8 +103,9 @@ def test_set_student_vcard_attrs(self, student): class TestVCardServiceVCardGeneration: @pytest.fixture(autouse=True) - def initialize(self, injector: Injector, mock_injected): + def initialize(self, injector: Injector, mock_injected, mock_people): self.session = cast(LocalProxy, {}) + self.mock_people = mock_people with mock_injected(LocalProxy, self.session): self.service = injector.get(VCardService) @@ -120,7 +121,7 @@ def prepare_pws(self): mock_pws_get.return_value = self.mock_pws_person def get_vcard_result(self, person: PersonOutput) -> List[str]: - self.mock_pws_person = person + self.mock_pws_person = person.dict(by_alias=True) self.prepare_pws() result = self.service.get_vcard("foo") @@ -147,7 +148,7 @@ def expected_employee_vcard(self) -> List[str]: ] def test_employee_vcard(self, employee): - self.mock_pws_person = employee + self.mock_pws_person = self.mock_people.as_search_output(employee) self.prepare_pws() result = self.get_vcard_result(employee) diff --git a/web-tests b/web-tests deleted file mode 100644 index e69de29b..00000000