From 1762891c6a15fdc56d69bae5f7bc506c91254644 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Nov 2023 15:44:15 +0000 Subject: [PATCH 1/4] Move creation of `SearchResultMetadata` into `SearchResult` Previously, `SearchResultMetadata` had a static `create_from_uri` method which was being called by `SearchResult`, which then ultimately initialised an instance of `SearchResultMetadata`. However, this is the _only_ place where this method was being called from. This means we can push this method upwards into `SearchResult` and avoid the need for a class acting as its own factory. --- src/caselawclient/responses/search_result.py | 27 +++++------------- tests/responses/test_search_result.py | 30 -------------------- 2 files changed, 7 insertions(+), 50 deletions(-) diff --git a/src/caselawclient/responses/search_result.py b/src/caselawclient/responses/search_result.py index 5bafc45c..ec30f392 100644 --- a/src/caselawclient/responses/search_result.py +++ b/src/caselawclient/responses/search_result.py @@ -2,6 +2,7 @@ import os from datetime import datetime from enum import Enum +from functools import cached_property from typing import Dict, Optional from dateutil import parser as dateparser @@ -44,20 +45,6 @@ def __init__(self, node: etree._Element, last_modified: str): self.node = node self.last_modified = last_modified - @staticmethod - def create_from_uri(uri: DocumentURIString) -> "SearchResultMetadata": - """ - Create a SearchResultMetadata instance from a search result URI. - - :param uri: The URI of the search result - - :return: The created SearchResultMetadata instance - """ - response_text = api_client.get_properties_for_search_results([uri]) - last_modified = api_client.get_last_modified(uri) - root = etree.fromstring(response_text) - return SearchResultMetadata(root, last_modified) - @property def author(self) -> str: """ @@ -259,15 +246,15 @@ def matches(self) -> str: xslt_transform = etree.XSLT(etree.parse(file_path)) return str(xslt_transform(self.node)) - @property + @cached_property def metadata(self) -> SearchResultMetadata: """ - :return: The metadata of the search result + :return: A `SearchResultMetadata` instance representing the metadata of this result """ - - return SearchResultMetadata.create_from_uri( - self.uri, - ) + response_text = api_client.get_properties_for_search_results([self.uri]) + last_modified = api_client.get_last_modified(self.uri) + root = etree.fromstring(response_text) + return SearchResultMetadata(root, last_modified) def _get_xpath_match_string(self, path: str) -> str: return get_xpath_match_string(self.node, path, namespaces=self.NAMESPACES) diff --git a/tests/responses/test_search_result.py b/tests/responses/test_search_result.py index b4e698cd..e77ad3f5 100644 --- a/tests/responses/test_search_result.py +++ b/tests/responses/test_search_result.py @@ -188,36 +188,6 @@ def test_editor_status( assert meta.editor_status == expected_editor_status.value - @patch("caselawclient.responses.search_result.api_client") - def test_create_from_uri(self, mock_api_client): - """ - GIVEN a uri and a mock API client - WHEN SearchResultMetadata.create_from_uri is called with the uri - THEN a SearchResultMetadata object is returned with expected attributes - """ - mock_api_client.get_properties_for_search_results.return_value = ( - "" - "" - "test_assigned_to" - "test_author" - "test_author_email" - "test_consignment_reference" - "false" - "30" - "2023-01-26T14:17:02Z" - "" - "" - ) - mock_api_client.get_last_modified.return_value = "test_last_modified" - - meta = SearchResultMetadata.create_from_uri("test_uri") - - assert ( - etree.tostring(meta.node).decode() - == mock_api_client.get_properties_for_search_results.return_value - ) - assert meta.last_modified == "test_last_modified" - def test_submission_date_is_min_when_transfer_received_at_empty(self): """ GIVEN a node where the `transfer-received-at` element is empty From b9a08eeabe52c32d095bd71476b7405127618a47 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Nov 2023 16:50:03 +0000 Subject: [PATCH 2/4] Remove `SearchResponse.from_response_string` This method was making `SearchResponse` its own factory, but was only ever being used to perform XML parsing. We can move this back up the stack to clarify what is going on here. --- .../client_helpers/search_helpers.py | 12 ++++++++---- src/caselawclient/responses/search_response.py | 9 --------- tests/responses/test_search_response.py | 16 +++++++++------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/caselawclient/client_helpers/search_helpers.py b/src/caselawclient/client_helpers/search_helpers.py index 4e70b200..e149809d 100644 --- a/src/caselawclient/client_helpers/search_helpers.py +++ b/src/caselawclient/client_helpers/search_helpers.py @@ -1,3 +1,5 @@ +from lxml import etree + from caselawclient.Client import MarklogicApiClient from caselawclient.responses.search_response import SearchResponse from caselawclient.search_parameters import SearchParameters @@ -14,8 +16,10 @@ def search_judgments_and_parse_response( :return: The parsed search response as a SearchResponse object """ - return SearchResponse.from_response_string( - api_client.search_judgments_and_decode_response(search_parameters) + return SearchResponse( + etree.fromstring( + api_client.search_judgments_and_decode_response(search_parameters) + ) ) @@ -30,6 +34,6 @@ def search_and_parse_response( :return: The parsed search response as a SearchResponse object """ - return SearchResponse.from_response_string( - api_client.search_and_decode_response(search_parameters) + return SearchResponse( + etree.fromstring(api_client.search_and_decode_response(search_parameters)) ) diff --git a/src/caselawclient/responses/search_response.py b/src/caselawclient/responses/search_response.py index 51c57162..d16f9f79 100644 --- a/src/caselawclient/responses/search_response.py +++ b/src/caselawclient/responses/search_response.py @@ -21,15 +21,6 @@ def __init__(self, node: etree._Element) -> None: """ self.node = node - @staticmethod - def from_response_string(xml: str) -> "SearchResponse": - """ - Constructs a SearchResponse instance from an xml response string. - - :param xml: The XML data as a string - """ - return SearchResponse(etree.fromstring(xml)) - @property def total(self) -> str: """ diff --git a/tests/responses/test_search_response.py b/tests/responses/test_search_response.py index 4227d8eb..160f7b47 100644 --- a/tests/responses/test_search_response.py +++ b/tests/responses/test_search_response.py @@ -13,10 +13,12 @@ def test_total( When calling 'total' on it Then it should return a string representing the total number of results """ - search_response = SearchResponse.from_response_string( - '' # noqa: E501 - "foo" - "" + search_response = SearchResponse( + etree.fromstring( + '' # noqa: E501 + "foo" + "" + ) ) assert search_response.total == "5" @@ -32,8 +34,8 @@ def test_results( Then it should return a list of n SearchResult elements And each element's node attribute should be as expected """ - search_response = SearchResponse.from_response_string( - generate_search_response_xml(2 * valid_search_result_xml) + search_response = SearchResponse( + etree.fromstring(generate_search_response_xml(2 * valid_search_result_xml)) ) results = search_response.results @@ -64,4 +66,4 @@ def test_when_search_namespace_prefix_not_defined_on_response_xml_syntax_error_r etree.XMLSyntaxError, match="Namespace prefix search on response is not defined", ): - SearchResponse.from_response_string(xml_without_namespace) + SearchResponse(etree.fromstring(xml_without_namespace)) From 077b6465d29daa06e9fb57dbf89f10cc8792c6ea Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 28 Nov 2023 22:30:55 +0000 Subject: [PATCH 3/4] Refactor use of static api_client out of search Search, specifically metadata handling, was relying on the static instance of api_client which was initialised as part of loading this API library. This instance is due to be deprecated, which means that instead the dynamic instance which is initialised by the calling code must be passed back down the stack. --- .../client_helpers/search_helpers.py | 6 +- .../responses/search_response.py | 11 +- src/caselawclient/responses/search_result.py | 9 +- .../client/test_search_and_decode_response.py | 116 ++++++++++-------- tests/responses/test_search_response.py | 18 ++- tests/responses/test_search_result.py | 63 ++++++---- 6 files changed, 130 insertions(+), 93 deletions(-) diff --git a/src/caselawclient/client_helpers/search_helpers.py b/src/caselawclient/client_helpers/search_helpers.py index e149809d..cc4726f9 100644 --- a/src/caselawclient/client_helpers/search_helpers.py +++ b/src/caselawclient/client_helpers/search_helpers.py @@ -19,7 +19,8 @@ def search_judgments_and_parse_response( return SearchResponse( etree.fromstring( api_client.search_judgments_and_decode_response(search_parameters) - ) + ), + api_client, ) @@ -35,5 +36,6 @@ def search_and_parse_response( :return: The parsed search response as a SearchResponse object """ return SearchResponse( - etree.fromstring(api_client.search_and_decode_response(search_parameters)) + etree.fromstring(api_client.search_and_decode_response(search_parameters)), + api_client, ) diff --git a/src/caselawclient/responses/search_response.py b/src/caselawclient/responses/search_response.py index d16f9f79..f5cf034c 100644 --- a/src/caselawclient/responses/search_response.py +++ b/src/caselawclient/responses/search_response.py @@ -2,6 +2,7 @@ from lxml import etree +from caselawclient.Client import MarklogicApiClient from caselawclient.responses.search_result import SearchResult @@ -13,13 +14,14 @@ class SearchResponse: NAMESPACES = {"search": "http://marklogic.com/appservices/search"} """ Namespaces used in XPath expressions.""" - def __init__(self, node: etree._Element) -> None: + def __init__(self, node: etree._Element, client: MarklogicApiClient) -> None: """ Initializes a SearchResponse instance from an xml node. :param node: The XML data as an etree element """ self.node = node + self.client = client @property def total(self) -> str: @@ -42,9 +44,4 @@ def results(self) -> List[SearchResult]: results = self.node.xpath( "//search:response/search:result", namespaces=self.NAMESPACES ) - return [ - SearchResult( - result, - ) - for result in results - ] + return [SearchResult(result, self.client) for result in results] diff --git a/src/caselawclient/responses/search_result.py b/src/caselawclient/responses/search_result.py index ec30f392..629409b7 100644 --- a/src/caselawclient/responses/search_result.py +++ b/src/caselawclient/responses/search_result.py @@ -10,7 +10,7 @@ from ds_caselaw_utils.courts import Court, CourtNotFoundException, courts from lxml import etree -from caselawclient.Client import api_client +from caselawclient.Client import MarklogicApiClient from caselawclient.models.documents import DocumentURIString from caselawclient.xml_helpers import get_xpath_match_string @@ -149,12 +149,13 @@ class SearchResult: } """ Namespace mappings used in XPath expressions. """ - def __init__(self, node: etree._Element): + def __init__(self, node: etree._Element, client: MarklogicApiClient): """ :param node: The XML element representing the search result """ self.node = node + self.client = client @property def uri(self) -> DocumentURIString: @@ -251,8 +252,8 @@ def metadata(self) -> SearchResultMetadata: """ :return: A `SearchResultMetadata` instance representing the metadata of this result """ - response_text = api_client.get_properties_for_search_results([self.uri]) - last_modified = api_client.get_last_modified(self.uri) + response_text = self.client.get_properties_for_search_results([self.uri]) + last_modified = self.client.get_last_modified(self.uri) root = etree.fromstring(response_text) return SearchResultMetadata(root, last_modified) diff --git a/tests/client/test_search_and_decode_response.py b/tests/client/test_search_and_decode_response.py index 62a1a515..eb235e91 100644 --- a/tests/client/test_search_and_decode_response.py +++ b/tests/client/test_search_and_decode_response.py @@ -1,61 +1,71 @@ from unittest.mock import patch -from caselawclient.Client import api_client +from caselawclient.Client import MarklogicApiClient from caselawclient.search_parameters import SearchParameters -@patch("caselawclient.Client.api_client.advanced_search") -def test_search_judgments_and_decode_response( - mock_advanced_search, - valid_search_response_xml, - generate_mock_search_response, -): - """ - Given the search parameters for search_judgments_and_decode_response are valid - And a mocked api_client response with search results - When search_judgments_and_decode_response function is called with the mocked API client - and input parameters - Then the API client's advanced_search method should be called once with the - appropriate parameters - And the search_judgments_and_decode_response function should return the response - xml string with all the search results - """ - mock_advanced_search.return_value = generate_mock_search_response( - valid_search_response_xml - ) - - search_response = api_client.search_judgments_and_decode_response( - SearchParameters( - query="test query", - court="test court", - judge="test judge", - party="test party", - neutral_citation="test citation", - specific_keyword="test keyword", - date_from="2022-01-01", - date_to="2022-01-31", - page=1, - page_size=10, +class TestSearchAndDecodeResponse: + def setup_method(self): + self.client = MarklogicApiClient( + host="", + username="", + password="", + use_https=False, + user_agent="marklogic-api-client-test", ) - ) - mock_advanced_search.assert_called_once_with( - SearchParameters( - query="test query", - court="test court", - judge="test judge", - party="test party", - neutral_citation="test citation", - specific_keyword="test keyword", - order=None, - date_from="2022-01-01", - date_to="2022-01-31", - page=1, - page_size=10, - show_unpublished=False, - only_unpublished=False, - collections=["judgment"], - ) - ) + def test_search_judgments_and_decode_response( + self, + valid_search_response_xml, + generate_mock_search_response, + ): + """ + Given the search parameters for search_judgments_and_decode_response are valid + And a mocked MarklogicApiClient.advanced_search response with search results + When search_judgments_and_decode_response function is called with the mocked API client + and input parameters + Then the API client's advanced_search method should be called once with the + appropriate parameters + And the search_judgments_and_decode_response function should return the response + xml string with all the search results + """ + with patch.object(self.client, "advanced_search") as mock_advanced_search: + mock_advanced_search.return_value = generate_mock_search_response( + valid_search_response_xml + ) + + search_response = self.client.search_judgments_and_decode_response( + SearchParameters( + query="test query", + court="test court", + judge="test judge", + party="test party", + neutral_citation="test citation", + specific_keyword="test keyword", + date_from="2022-01-01", + date_to="2022-01-31", + page=1, + page_size=10, + ) + ) + + mock_advanced_search.assert_called_once_with( + SearchParameters( + query="test query", + court="test court", + judge="test judge", + party="test party", + neutral_citation="test citation", + specific_keyword="test keyword", + order=None, + date_from="2022-01-01", + date_to="2022-01-31", + page=1, + page_size=10, + show_unpublished=False, + only_unpublished=False, + collections=["judgment"], + ) + ) - assert search_response == valid_search_response_xml + assert search_response == valid_search_response_xml diff --git a/tests/responses/test_search_response.py b/tests/responses/test_search_response.py index 160f7b47..c99a2369 100644 --- a/tests/responses/test_search_response.py +++ b/tests/responses/test_search_response.py @@ -1,10 +1,20 @@ import pytest from lxml import etree +from caselawclient.Client import MarklogicApiClient from caselawclient.responses.search_response import SearchResponse class TestSearchResponse: + def setup_method(self): + self.client = MarklogicApiClient( + host="", + username="", + password="", + use_https=False, + user_agent="marklogic-api-client-test", + ) + def test_total( self, ): @@ -18,7 +28,8 @@ def test_total( '' # noqa: E501 "foo" "" - ) + ), + self.client, ) assert search_response.total == "5" @@ -35,7 +46,8 @@ def test_results( And each element's node attribute should be as expected """ search_response = SearchResponse( - etree.fromstring(generate_search_response_xml(2 * valid_search_result_xml)) + etree.fromstring(generate_search_response_xml(2 * valid_search_result_xml)), + self.client, ) results = search_response.results @@ -66,4 +78,4 @@ def test_when_search_namespace_prefix_not_defined_on_response_xml_syntax_error_r etree.XMLSyntaxError, match="Namespace prefix search on response is not defined", ): - SearchResponse(etree.fromstring(xml_without_namespace)) + SearchResponse(etree.fromstring(xml_without_namespace), self.client) diff --git a/tests/responses/test_search_result.py b/tests/responses/test_search_result.py index e77ad3f5..3b7c8f0f 100644 --- a/tests/responses/test_search_result.py +++ b/tests/responses/test_search_result.py @@ -5,6 +5,7 @@ from ds_caselaw_utils.courts import Court from lxml import etree +from caselawclient.Client import MarklogicApiClient from caselawclient.responses.search_result import ( EditorPriority, EditorStatus, @@ -14,32 +15,46 @@ class TestSearchResult: - @patch("caselawclient.responses.search_result.api_client") - def test_init(self, mock_api_client, valid_search_result_xml): + def setup_method(self): + self.client = MarklogicApiClient( + host="", + username="", + password="", + use_https=False, + user_agent="marklogic-api-client-test", + ) + + def test_init(self, valid_search_result_xml): """ GIVEN a valid search result XML and a mock API client WHEN initializing a SearchResult object THEN the attributes are set correctly """ - mock_api_client.get_properties_for_search_results.return_value = "" - mock_api_client.get_last_modified.return_value = "bar" - - node = etree.fromstring(valid_search_result_xml) - search_result = SearchResult(node) - - assert search_result.uri == "a/c/2015/20" - assert search_result.neutral_citation == "[2015] A 20 (C)" - assert search_result.name == "Another made up case name" - assert search_result.date == datetime.datetime(2017, 8, 8, 0, 0) - assert search_result.court is None - assert search_result.matches == ( - "

" - "text from the document that matched the search

\n" - ) - assert search_result.content_hash == "test_content_hash" - assert search_result.transformation_date == "2023-04-09T18:05:45" - assert etree.tostring(search_result.metadata.node).decode() == "" - assert search_result.metadata.last_modified == "bar" + with ( + patch.object( + self.client, "get_properties_for_search_results" + ) as mock_get_properties_for_search_results, + patch.object(self.client, "get_last_modified") as mock_get_last_modified, + ): + mock_get_properties_for_search_results.return_value = "" + mock_get_last_modified.return_value = "bar" + + node = etree.fromstring(valid_search_result_xml) + search_result = SearchResult(node, self.client) + + assert search_result.uri == "a/c/2015/20" + assert search_result.neutral_citation == "[2015] A 20 (C)" + assert search_result.name == "Another made up case name" + assert search_result.date == datetime.datetime(2017, 8, 8, 0, 0) + assert search_result.court is None + assert search_result.matches == ( + "

" + "text from the document that matched the search

\n" + ) + assert search_result.content_hash == "test_content_hash" + assert search_result.transformation_date == "2023-04-09T18:05:45" + assert etree.tostring(search_result.metadata.node).decode() == "" + assert search_result.metadata.last_modified == "bar" def test_create_from_node_with_unparsable_date(self): """ @@ -55,7 +70,7 @@ def test_create_from_node_with_unparsable_date(self): "" ) node = etree.fromstring(xml) - search_result = SearchResult(node) + search_result = SearchResult(node, self.client) assert search_result.date is None @@ -73,7 +88,7 @@ def test_create_from_node_with_valid_court_code(self): "" ) node = etree.fromstring(xml) - search_result = SearchResult(node) + search_result = SearchResult(node, self.client) assert isinstance(search_result.court, Court) assert search_result.court.name == "United Kingdom Supreme Court" @@ -92,7 +107,7 @@ def test_create_from_node_with_invalid_court_code(self): "" ) node = etree.fromstring(xml) - search_result = SearchResult(node) + search_result = SearchResult(node, self.client) assert search_result.date is None From 00846dedf145e8463488291cc64d006a85cdb4f6 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 28 Nov 2023 22:38:26 +0000 Subject: [PATCH 4/4] Fully remove deprecated api_client instance This has been deprecated since v13.1.0, and all in-library references to it have been cleaned up. No downstream code should be relying on this instance. --- CHANGELOG.md | 2 ++ src/caselawclient/Client.py | 14 -------------- src/caselawclient/__init__.py | 20 -------------------- 3 files changed, 2 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdc79891..b43d3841 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog 1.0.0]. ## [Unreleased] +- **Breaking:**: Fully remove the deprecated `caselawclient.api_client` instance. + ## [Release 17.3.0] - **Feature:** `Document.enrich()` method will send a message to the announce SNS, requesting that a document be enriched. diff --git a/src/caselawclient/Client.py b/src/caselawclient/Client.py index a1347379..2622fbca 100644 --- a/src/caselawclient/Client.py +++ b/src/caselawclient/Client.py @@ -951,17 +951,3 @@ def get_combined_stats_table(self) -> list[list[Any]]: ) return results - - -api_client = MarklogicApiClient( - host=env("MARKLOGIC_HOST", default=None), - username=env("MARKLOGIC_USER", default=None), - password=env("MARKLOGIC_PASSWORD", default=None), - use_https=env("MARKLOGIC_USE_HTTPS", default=False), -) -""" -An instance of the API client which is automatically initialised on importing the library. - -.. deprecated:: 13.0.1 - You should instead initialise your own instance of `MarklogicApiClient` -""" diff --git a/src/caselawclient/__init__.py b/src/caselawclient/__init__.py index 08e5d701..e9b5ed0b 100644 --- a/src/caselawclient/__init__.py +++ b/src/caselawclient/__init__.py @@ -37,24 +37,4 @@ ``` -## (Deprecated) Use in-library client instance - -This library will automatically initialise an instance of the client. This functionality is deprecated, and will be -removed. - -The client expects the following environment variables to be set or defined in a `.env` file: - -```bash -MARKLOGIC_HOST -MARKLOGIC_USER -MARKLOGIC_PASSWORD -MARKLOGIC_USE_HTTPS # Optional, defaults to False -``` - -Then import `api_client` from `caselawclient.Client`: - -```python -from caselawclient.Client import api_client -``` - """