Merge pull request #706 from nationalarchives/chore/remove-overwrite-…

…document

Breaking: Remove unused overwrite methods

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog 1.0.0].
 
 ### BREAKING CHANGE
 
+- Remove `Document.overwrite` and `MarkLogicApiClient.overwrite`
 - The `models.documents.body.CourtIdentifierString` type has been replaced with the more specific `courts.CourtCode` type from ds-caselaw-utils.
 
 ### Fix
@@ -17,10 +18,10 @@ The format is based on [Keep a Changelog 1.0.0].
 - **deps**: update dependency ds-caselaw-utils to v1.7.0
 - **deps**: update dependency boto3 to v1.35.28
 - **deps**: update dependency ds-caselaw-utils to v1.5.7
-- **deps**: update dependency boto3 to v1.35.25
 
 ### Refactor
 
+- **Document**: remove unused overwrite method
 - **DocumentBody**: replace CourtIdentifierString with utils.courts.CourtCode
 
 ## v26.0.0 (2024-09-25)

src/caselawclient/Client.py

@@ -1035,12 +1035,7 @@ def search_judgments_and_decode_response(
         search_parameters.collections = [DOCUMENT_COLLECTION_URI_JUDGMENT]
         return self.search_and_decode_response(search_parameters)
 
-    def overwrite_document(self, old_uri: str, new_citation: str) -> str:
-        """Move the judgment at old_uri on top of the new citation, which must already exist
-        Compare to update_document_uri"""
-        return move.overwrite_document(old_uri, new_citation, api_client=self)
-
-    def update_document_uri(self, old_uri: str, new_citation: str) -> str:
+    def update_document_uri(self, old_uri: DocumentURIString, new_citation: str) -> DocumentURIString:
         """
         Move the document at old_uri to the correct location based on the neutral citation
         The new neutral citation *must* not already exist (that is handled elsewhere)

src/caselawclient/models/documents/__init__.py

@@ -441,9 +441,6 @@ def delete(self) -> None:
         else:
             raise DocumentNotSafeForDeletion
 
-    def overwrite(self, new_citation: str) -> None:
-        self.api_client.overwrite_document(self.uri, new_citation)
-
     def move(self, new_citation: str) -> None:
         self.api_client.update_document_uri(self.uri, new_citation)
 

src/caselawclient/models/utilities/move.py

@@ -1,79 +1,26 @@
-from typing import Any, Optional
+from typing import TYPE_CHECKING, Any, Optional
 
 import ds_caselaw_utils as caselawutils
 
 from caselawclient.errors import MarklogicAPIError
+from caselawclient.models.documents import DocumentURIString
 from caselawclient.models.utilities.aws import copy_assets
 
-
-class NeutralCitationToUriError(Exception):
-    pass
+if TYPE_CHECKING:
+    from caselawclient.Client import MarklogicApiClient
 
 
-class OverwriteJudgmentError(Exception):
+class NeutralCitationToUriError(Exception):
     pass
 
 
 class MoveJudgmentError(Exception):
     pass
 
 
-def overwrite_document(
-    source_uri: str,
-    target_citation: str,
-    api_client: Any,
-) -> str:
-    """Move the document at source_uri on top of the new citation, which must already exist
-    Compare to update_document_uri
-
-    :param source_uri: The URI with the contents of the document to be written. (possibly a failure url)
-    :param target_citation: The NCN (implying a URL) whose contents will be overwritten
-    :param api_client: An instance of MarklogicApiClient used to make the search request
-    :return: The URL associated with the `target_citation`
-    """
-
-    new_uri: Optional[str] = caselawutils.neutral_url(target_citation.strip())
-
-    if new_uri == source_uri:
-        raise RuntimeError(
-            f"Attempted to overwrite document {source_uri} with itself, which is not permitted.",
-        )
-    if new_uri is None:
-        raise NeutralCitationToUriError(
-            f"Unable to form new URI for {source_uri} from neutral citation: {target_citation}",
-        )
-    if not api_client.document_exists(new_uri):
-        raise OverwriteJudgmentError(
-            f"The URI {new_uri} generated from {target_citation} does not already exist, so cannot be overwritten",
-        )
-    old_doc = api_client.get_document_by_uri_or_404(source_uri)
-    try:
-        old_doc_xml = old_doc.content_as_xml
-        api_client.update_document_xml(
-            new_uri,
-            old_doc_xml,
-            annotation=f"overwritten from {source_uri}",
-        )
-        set_metadata(source_uri, new_uri, api_client)
-        # TODO: consider deleting existing public assets at that location
-        copy_assets(source_uri, new_uri)
-        api_client.set_judgment_this_uri(new_uri)
-    except MarklogicAPIError as e:
-        raise OverwriteJudgmentError(
-            f"Failure when attempting to copy judgment from {source_uri} to {new_uri}: {e}",
-        )
-
-    try:
-        api_client.delete_judgment(source_uri)
-    except MarklogicAPIError as e:
-        raise OverwriteJudgmentError(
-            f"Failure when attempting to delete judgment from {source_uri}: {e}",
-        )
-
-    return new_uri
-
-
-def update_document_uri(source_uri: str, target_citation: str, api_client: Any) -> str:
+def update_document_uri(
+    source_uri: DocumentURIString, target_citation: str, api_client: "MarklogicApiClient"
+) -> DocumentURIString:
     """
     Move the document at source_uri to the correct location based on the neutral citation
     The new neutral citation *must* not already exist (that is handled elsewhere)
@@ -83,7 +30,8 @@ def update_document_uri(source_uri: str, target_citation: str, api_client: Any)
     :param api_client: An instance of MarklogicApiClient used to make the search request
     :return: The URL associated with the `target_citation`
     """
-    new_uri: Optional[str] = caselawutils.neutral_url(target_citation.strip())
+    new_ncn_based_uri = caselawutils.neutral_url(target_citation.strip())
+    new_uri: Optional[DocumentURIString] = DocumentURIString(new_ncn_based_uri) if new_ncn_based_uri else None
     if new_uri is None:
         raise NeutralCitationToUriError(
             f"Unable to form new URI for {source_uri} from neutral citation: {target_citation}",

tests/models/utilities/test_utilities.py

@@ -1,10 +1,9 @@
 import io
 import os
-from unittest.mock import ANY, MagicMock, Mock, patch
+from unittest.mock import MagicMock, Mock, patch
 
 import boto3
 import ds_caselaw_utils
-import pytest
 from moto import mock_aws
 
 from caselawclient.models.utilities import extract_version, move, render_versions
@@ -14,8 +13,6 @@
     copy_assets,
 )
 
-from ...factories import JudgmentFactory
-
 
 class TestVersionUtils:
     def test_extract_version_uri(self):
@@ -83,45 +80,6 @@ def test_copy_assets(self, client):
         )
 
 
-class TestOverwrite:
-    @patch.dict(os.environ, {"PRIVATE_ASSET_BUCKET": "MY_BUCKET"})
-    @patch("boto3.session.Session.client")
-    def test_overwrite_success(self, fake_boto3_client):
-        """Given the target judgment does not exist,
-        we continue to move the judgment to the new location
-        (where moving is copy + delete)"""
-        ds_caselaw_utils.neutral_url = MagicMock(return_value="new/uri")
-        fake_api_client = MagicMock()
-        fake_api_client.judgment_exists.return_value = True
-        fake_api_client.copy_judgment.return_value = True
-        fake_api_client.delete_judgment.return_value = True
-        fake_boto3_client.list_objects.return_value = []
-
-        fake_judgment = JudgmentFactory.build()
-        fake_judgment.return_value.content_as_xml = "<xml>b</xml>"
-        fake_api_client.get_document_by_uri_or_404.return_value = fake_judgment
-
-        result = move.overwrite_document("old/uri", "[2002] EAT 1", fake_api_client)
-        fake_api_client.update_document_xml.assert_called_with(
-            "new/uri",
-            ANY,
-            annotation="overwritten from old/uri",
-        )
-        fake_api_client.delete_judgment.assert_called_with("old/uri")
-        assert result == "new/uri"
-
-    def test_overwrite_unparseable_citation(self):
-        ds_caselaw_utils.neutral_url = MagicMock(return_value=None)
-        fake_api_client = MagicMock()
-
-        with pytest.raises(move.NeutralCitationToUriError):
-            move.overwrite_document(
-                "old/uri",
-                "Wrong neutral citation",
-                fake_api_client,
-            )
-
-
 class TestMove:
     @patch.dict(os.environ, {"PRIVATE_ASSET_BUCKET": "MY_BUCKET"})
     @patch("boto3.session.Session.client")