From 9d522af76b13e2ba9d4a9d1ed6e77b9cd4c6b9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 22 May 2024 12:16:42 +0200 Subject: [PATCH] PYL-35 support search by relationship --- src/pylms/__main__.py | 7 +- src/pylms/pylms.py | 140 ++++++++++++++++++++++++++++++++------ tests/pylms/pymls_test.py | 106 ++++++++++++++++++++++++++++- 3 files changed, 227 insertions(+), 26 deletions(-) diff --git a/src/pylms/__main__.py b/src/pylms/__main__.py index 2443358..d1f2236 100644 --- a/src/pylms/__main__.py +++ b/src/pylms/__main__.py @@ -85,14 +85,17 @@ def _command_delete(argv_length: int) -> None: def _command_link(argv_length: int) -> None: if argv_length < 4: print(f"Too few arguments ({argv_length -1})") + return natural_link_request = " ".join(argv[2:]) link_persons(natural_link_request) def _command_search(argv_length: int) -> None: - if argv_length != 3: - print(f"Wrong number of arguments ({argv_length})") + if argv_length < 3: + print(f"Too few arguments ({argv_length})") + return + natural_search_request = " ".join(argv[2:]) search_persons(natural_search_request) diff --git a/src/pylms/pylms.py b/src/pylms/pylms.py index 8b771a2..f4e8af2 100644 --- a/src/pylms/pylms.py +++ b/src/pylms/pylms.py @@ -2,6 +2,7 @@ from pylms.core import Person, PersonIdGenerator from pylms.core import relationship_definitions, RelationshipDefinition, Relationship, RelationshipAlias from pylms.core import resolve_persons +from pylms.python_utils import require_not_none from abc import abstractmethod, ABC import logging @@ -160,39 +161,100 @@ def __init__( self.alias: RelationshipAlias = alias -def _find_relation_ship(natural_language_link_order: str) -> tuple[RelationshipDefinition, RelationshipAlias] | None: - if len(natural_language_link_order) == 0: +class SearchRequest: + def __init__( + self, + *, + pattern: str, + definition: RelationshipDefinition | None = None, + alias: RelationshipAlias | None = None, + ): + self.pattern = require_not_none(pattern, "pattern can't be None") + if (definition is None) != (alias is None): + raise ValueError("Either both definition and alias must be provided or neither of them") + self.definition: RelationshipDefinition | None = definition + self.alias: RelationshipAlias | None = alias + + +class RelationshipPattern: + def __init__( + self, + *, + pattern_before: str | None, + definition: RelationshipDefinition | None = None, + alias: RelationshipAlias | None = None, + pattern_after: str, + ): + """ + Neither pattern_before nor pattern_after are trimmed. + """ + self.pattern_before: str | None = pattern_before + self.definition: RelationshipDefinition | None = definition + self.alias: RelationshipAlias | None = alias + self.pattern_after: str | None = pattern_after + + +def _find_relationship_by_alias( + definition: RelationshipDefinition, alias: RelationshipAlias, request: str +) -> RelationshipPattern | None: + name = alias.name.lower() + try: + index = request.index(name) + return RelationshipPattern( + pattern_before=request[0:index], + definition=definition, + alias=alias, + pattern_after=request[index + len(name) :], + ) + except ValueError: + return None + + +def _find_relationship_pattern(request: str) -> RelationshipPattern | None: + if len(request) == 0: return None - natural_language_link_order: str = natural_language_link_order.lower() + request: str = request.lower() for rl in relationship_definitions: for alias in rl.aliases: - if alias.name.lower() in natural_language_link_order: - return rl, alias + rl_pattern = _find_relationship_by_alias(rl, alias, request) + if rl_pattern: + return rl_pattern return None def _parse_nl_link_request(natural_language_link_request: str) -> LinkRequest | None: - match = _find_relation_ship(natural_language_link_request) - if match is None: + rl_pattern = _find_relationship_pattern(natural_language_link_request) + if rl_pattern is None: return None - definition, alias = match - person_patterns = list( - filter(lambda s: len(s) > 0, map(str.strip, natural_language_link_request.split(alias.name))) - ) - patterns_count = len(person_patterns) - if patterns_count != 2: - logger.error(f"Unsupported link request: wrong number of person patterns ({patterns_count})") + pattern_before = rl_pattern.pattern_before.strip() if rl_pattern.pattern_before else None + pattern_after = rl_pattern.pattern_after.strip() if rl_pattern.pattern_after else None + if not pattern_before or not pattern_after: + logger.error("Unsupported link request: wrong number of person patterns") return None return LinkRequest( - left_person_pattern=person_patterns[0], - right_person_pattern=person_patterns[1], - definition=definition, - alias=alias, + left_person_pattern=pattern_before, + right_person_pattern=pattern_after, + definition=rl_pattern.definition, + alias=rl_pattern.alias, + ) + + +def _parse_search_request(search_request: str) -> SearchRequest | None: + rl_pattern = _find_relationship_pattern(search_request) + if rl_pattern is None: + return SearchRequest(pattern=search_request) + + if rl_pattern.pattern_before: + logger.error(f"Unsupported relationship search request has prefix: {rl_pattern.pattern_before}") + return None + + return SearchRequest( + pattern=rl_pattern.pattern_after.strip(), definition=rl_pattern.definition, alias=rl_pattern.alias ) @@ -241,11 +303,47 @@ def link_persons(natural_language_link_request: str) -> None: storage.store_relationships(relationships + [relationship]) -def search_persons(pattern: str) -> None: - person_hits = _search_persons(pattern) +def _rl_match(search_request: SearchRequest, rl: Relationship) -> bool: + """ + Test whether the provided Relationship match the SearchRequest. + First, a matching relationship must have the same definition as the SearchRequest. + Then, the SearchRequest can have a forward (eg. "Père de Peter") or a reverse alias (eg. "Fils de John"). + In the former case, a matching relationship must have the _right_ person matching the pattern ("Peter"), + in the later case a matching relationship must have the _left_ person matching the pattern ("John" this time). + Finally, the alias in the SearchRequest can have a left person sex. If so, the person must also have the same sex. + """ + if rl.definition != search_request.definition: + return False + + person_to_match_pattern, person_to_match_sex = rl.right, rl.left + if search_request.alias.reverse: + person_to_match_pattern, person_to_match_sex = rl.left, rl.right + if _search_match(search_request.pattern, person_to_match_pattern): + expected_sex = search_request.alias.left_person_sex + if expected_sex: + return person_to_match_sex.sex == expected_sex + return True + + return False + + +def _search_request_relationship(search_request: SearchRequest) -> list[Person]: + persons = storage.read_persons() + relationships = storage.read_relationships(persons) + + return [r.right if search_request.alias.reverse else r.left for r in relationships if _rl_match(search_request, r)] + + +def search_persons(search_request_string: str) -> None: + search_request = _parse_search_request(search_request_string) + + if search_request.definition: + person_hits = _search_request_relationship(search_request) + else: + person_hits = _search_persons(search_request_string) if not person_hits: - logger.info(f'No match for "{pattern}".') + logger.info(f'No match for "{search_request_string}".') return persons = storage.read_persons() diff --git a/tests/pylms/pymls_test.py b/tests/pylms/pymls_test.py index 887cce2..9be20cb 100644 --- a/tests/pylms/pymls_test.py +++ b/tests/pylms/pymls_test.py @@ -1,9 +1,10 @@ -import random - +import pylms.core from pylms.core import Person from pylms.pylms import list_persons, store_person, update_person, search_persons, delete_person, link_persons from pylms.pylms import LinkRequest, Relationship, RelationshipDefinition, RelationshipAlias +from pylms.core import parent_enfant, copain_copine, MALE, FEMALE from unittest.mock import patch, call +from pytest import mark class TestStorePerson: @@ -301,7 +302,7 @@ def __eq__(self, other: Relationship): mock_storage.store_relationships([ExpectedRelationship()]) -class TestSearchPerson: +class TestSearchPersonByWord: @patch("pylms.pylms.logger") @patch("pylms.pylms._search_persons") @@ -347,3 +348,102 @@ def test_list_persons_limited_to_the_person_matching_pattern( mock_storage.read_persons.assert_called_once_with() mock_storage.read_relationships.assert_called_once_with(persons) mock_ios.list_persons.assert_called_once_with([(person_1, [rls[0], rls[4], rls[5]])]) + + +class TestSearchPersonByRelationship: + john = Person(person_id=1, firstname="John") + peter = Person(person_id=2, firstname="Peter") + emma = Person(person_id=3, firstname="Emma", sex=FEMALE) + carine = Person(person_id=4, firstname="Carine") + tom = Person(person_id=5, firstname="Tom", sex=MALE) + bill = Person(person_id=6, firstname="Bill") + dona = Person(person_id=7, firstname="Dona", sex=FEMALE) + elmer = Person(person_id=8, firstname="Elmer", sex=MALE) + princess = Person(person_id=9, firstname="Princess", sex=FEMALE) + persons = [john, peter, emma, carine, tom, bill, dona, elmer, princess] + relationships = [ + Relationship(person_left=john, person_right=peter, definition=parent_enfant), + Relationship(person_left=john, person_right=emma, definition=parent_enfant), + Relationship(person_left=emma, person_right=carine, definition=copain_copine), + Relationship(person_left=john, person_right=tom, definition=parent_enfant), + Relationship(person_left=dona, person_right=bill, definition=parent_enfant), + Relationship(person_left=elmer, person_right=bill, definition=parent_enfant), + Relationship(person_left=princess, person_right=bill, definition=parent_enfant), + ] + + @patch("pylms.pylms.ios") + @patch("pylms.pylms.storage") + @mark.parametrize( + ("search_request", "expected"), + [ + ("parent de emma", [(john, [relationships[0], relationships[1],relationships[3]])]), + ( + "enfant de John", + [(peter, [relationships[0]]), (emma, [relationships[1], relationships[2]]), (tom, [relationships[3]])], + ), + ("fille de John", [(emma, [relationships[1], relationships[2]])]), + ("fils de John", [(tom, [relationships[3]])]), + ("mère de bill", [(dona, [relationships[4]]), (princess, [relationships[6]])]), + ("père de bill", [(elmer, [relationships[5]])]), + ("parent de bill", [(dona, [relationships[4]]), (elmer, [relationships[5]]), (princess, [relationships[6]])]), + ], + ) + def test_search_successful(self, mock_storage, mock_ios, search_request, expected): + mock_storage.read_persons.return_value = self.persons + mock_storage.read_relationships.return_value = self.relationships + + search_persons(search_request) + + # duplicate calls because: 1 for finding the persons, 1 to display them. + # Could be only one call but this optimization is deemed minor + mock_storage.read_persons.assert_has_calls([call(), call()]) + mock_storage.read_relationships.assert_has_calls([call(self.persons), call(self.persons)]) + mock_ios.list_persons.assert_called_once_with(expected) + + @patch("pylms.pylms.ios") + @patch("pylms.pylms.logger") + @patch("pylms.pylms.storage") + @mark.parametrize( + "search_request", + [ + "père de emma", + "mère de peter", + "parent de carine", + "fils de Elmer", + "fils de Dona", + "fils de princess" + ], + ) + def test_no_matching_relationship(self, mock_storage, mock_logger, mock_ios, search_request): + mock_storage.read_persons.return_value = self.persons + mock_storage.read_relationships.return_value = self.relationships + + search_persons(search_request) + + # 1 call to find the persons + mock_storage.read_persons.assert_called_once_with() + mock_storage.read_relationships.assert_called_once_with(self.persons) + mock_logger.info.assert_called_once_with(f'No match for "{search_request}".') + assert mock_ios.list_persons.call_count == 0 + + @patch("pylms.pylms.ios") + @patch("pylms.pylms.logger") + @patch("pylms.pylms.storage") + @mark.parametrize( + "search_request", + [ + "foo bar donut", + "mere de bill", # alias is not yet matched regardless of accentuated chars + ], + ) + def test_no_relationship_found_in_request(self, mock_storage, mock_logger, mock_ios, search_request): + mock_storage.read_persons.return_value = self.persons + mock_storage.read_relationships.return_value = self.relationships + + search_persons(search_request) + + # 1 call to find the persons + mock_storage.read_persons.assert_called_once_with() + assert mock_storage.read_relationships.call_count == 0 + mock_logger.info.assert_called_once_with(f'No match for "{search_request}".') + assert mock_ios.list_persons.call_count == 0