From a3b6a6448011c77202909549c59e39a49acf0cb4 Mon Sep 17 00:00:00 2001
From: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date: Mon, 16 Nov 2020 18:51:31 +0100
Subject: [PATCH] Drop backwards-compatibility support for
 RawExtrinsicMetadata.id

All reverse dependencies have been updated to avoid using it now, so it can now
be removed, paving the way to recycle it into an intrinsic identifier.
---
 swh/model/model.py            | 54 +++++++----------------------------
 swh/model/tests/test_model.py | 27 +++---------------
 2 files changed, 14 insertions(+), 67 deletions(-)

diff --git a/swh/model/model.py b/swh/model/model.py
index d3b3971b..6c49c5f9 100644
--- a/swh/model/model.py
+++ b/swh/model/model.py
@@ -8,7 +8,6 @@ import datetime
 from enum import Enum
 from hashlib import sha256
 from typing import Any, Dict, Iterable, Optional, Tuple, TypeVar, Union
-import warnings
 
 import attr
 from attrs_strict import type_validator
@@ -854,26 +853,10 @@ class MetadataTargetType(Enum):
     ORIGIN = "origin"
 
 
-def mangle_raw_extrinsic_metadata_dict(d: Dict[str, Any]) -> Dict[str, Any]:
-    """Rename the raw_extrinsic_metadata `id` field to `target`, throwing a
-    DeprecationWarning if appropriate"""
-    if "id" in d:
-        target = d.pop("id")
-        if d.get("target") is None:
-            warnings.warn(
-                "RawExtrinsicMetadata `id` attribute is now called `target`",
-                DeprecationWarning,
-            )
-            d["target"] = target
-        elif d["target"] != target:
-            raise ValueError(
-                "Different values for `id` and `target` in RawExtrinsicMetadata"
-            )
-    return d
-
-
 @attr.s(frozen=True)
-class _RawExtrinsicMetadata(BaseModel):
+class RawExtrinsicMetadata(BaseModel):
+    object_type: Final = "raw_extrinsic_metadata"
+
     # target object
     type = attr.ib(type=MetadataTargetType, validator=type_validator())
     target = attr.ib(type=Union[str, SWHID], validator=type_validator())
@@ -1041,7 +1024,7 @@ class _RawExtrinsicMetadata(BaseModel):
 
     def to_dict(self):
         d = super().to_dict()
-        d["id"] = d["target"]
+
         context_keys = (
             "origin",
             "visit",
@@ -1058,14 +1041,12 @@ class _RawExtrinsicMetadata(BaseModel):
 
     @classmethod
     def from_dict(cls, d):
-        d = mangle_raw_extrinsic_metadata_dict(
-            {
-                **d,
-                "type": MetadataTargetType(d["type"]),
-                "authority": MetadataAuthority.from_dict(d["authority"]),
-                "fetcher": MetadataFetcher.from_dict(d["fetcher"]),
-            }
-        )
+        d = {
+            **d,
+            "type": MetadataTargetType(d["type"]),
+            "authority": MetadataAuthority.from_dict(d["authority"]),
+            "fetcher": MetadataFetcher.from_dict(d["fetcher"]),
+        }
 
         if d["type"] != MetadataTargetType.ORIGIN:
             d["target"] = parse_swhid(d["target"])
@@ -1087,18 +1068,3 @@ class _RawExtrinsicMetadata(BaseModel):
             "fetcher_name": self.fetcher.name,
             "fetcher_version": self.fetcher.version,
         }
-
-
-class RawExtrinsicMetadata(_RawExtrinsicMetadata):
-    object_type: Final = "raw_extrinsic_metadata"
-
-    def __init__(self, **kwargs):
-        super().__init__(**mangle_raw_extrinsic_metadata_dict(kwargs))
-
-    @property
-    def id(self):
-        warnings.warn(
-            "RawExtrinsicMetadata `id` attribute is now called `target`",
-            DeprecationWarning,
-        )
-        return self.target
diff --git a/swh/model/tests/test_model.py b/swh/model/tests/test_model.py
index 07afe215..902f0df6 100644
--- a/swh/model/tests/test_model.py
+++ b/swh/model/tests/test_model.py
@@ -807,9 +807,7 @@ def test_metadata_valid():
     )
 
 
-@pytest.mark.filterwarnings("ignore: RawExtrinsicMetadata `id`")
-@pytest.mark.parametrize("argument_name", ["id", "target"])
-def test_metadata_to_dict(argument_name):
+def test_metadata_to_dict():
     """Checks valid RawExtrinsicMetadata objects don't raise an error."""
 
     common_fields = {
@@ -821,25 +819,23 @@ def test_metadata_to_dict(argument_name):
     }
 
     m = RawExtrinsicMetadata(
-        type=MetadataTargetType.ORIGIN,
-        **{argument_name: _origin_url, **_common_metadata_fields},
+        type=MetadataTargetType.ORIGIN, target=_origin_url, **_common_metadata_fields,
     )
     assert m.to_dict() == {
         "type": "origin",
         "target": _origin_url,
-        "id": _origin_url,
         **common_fields,
     }
     assert RawExtrinsicMetadata.from_dict(m.to_dict()) == m
 
     m = RawExtrinsicMetadata(
         type=MetadataTargetType.CONTENT,
-        **{argument_name: _content_swhid, **_common_metadata_fields},
+        target=_content_swhid,
+        **_common_metadata_fields,
     )
     assert m.to_dict() == {
         "type": "content",
         "target": "swh:1:cnt:94a9ed024d3859793618152ea559a168bbcbb5e2",
-        "id": "swh:1:cnt:94a9ed024d3859793618152ea559a168bbcbb5e2",
         **common_fields,
     }
     assert RawExtrinsicMetadata.from_dict(m.to_dict()) == m
@@ -1227,18 +1223,3 @@ def test_metadata_validate_context_directory():
             ),
             **_common_metadata_fields,
         )
-
-
-def test_metadata_id_attr():
-    """Checks the legacy id attribute on RawExtrinsicMetadata objects"""
-    # Simplest case
-    meta = RawExtrinsicMetadata(
-        type=MetadataTargetType.ORIGIN, target=_origin_url, **_common_metadata_fields
-    )
-
-    assert meta is not None
-
-    with pytest.deprecated_call() as messages:
-        assert meta.id == _origin_url
-
-    assert "RawExtrinsicMetadata `id`" in str(messages[0].message)
-- 
GitLab