Restore ComponentId from v0.3.x (#287)

This type was removed too soon. v0.4.0 was released but not very widely
adopted, so this problem was never detected. Removing the old
`ComponentId` means no downstream project can have a working version
that supports both v0.3.x and v0.4.x series, which complicates a clean
non-breaking upgrade path.

Author: llucax

RELEASE_NOTES.md

@@ -18,6 +18,8 @@ There are a few intentional hard breaks too, all listed in the Upgrading section
 
 ## Upgrading
 
+* `frequenz.client.common.microgrid.components.ComponentId` is restored as a deprecated compatibility class. It keeps its historical import path and remains a distinct type from `ElectricalComponentId`; importing both classes logs the existing duplicate `CID` prefix warning. This restores only `ComponentId`, not the removed `ComponentCategory`, `ComponentStateCode` or `ComponentErrorCode` symbols. The v0.4.0 release remains incompatible with users of the removed import path. Removal will be coordinated with downstream migration rather than tied automatically to v0.5.0.
+
 * The `frequenz.client.common.microgrid.electrical_components.ElectricalComponentCategory` enum is now deprecated and will be removed in a future release.
 
     Accessing any member of this enum will emit a `DeprecationWarning`. Users are encouraged to switch to the `ElectricalComponent` class hierarchy (using `match` expressions or `isinstance()`) to identify components.

docs/wrapping-guide/deprecation-and-compatibility.md

@@ -40,6 +40,15 @@ with deprecated_call(
     assert thing_from_proto(3) == "3"
 ```
 
+## Check downstream adoption before removal
+
+When possible, check downstream client releases to see if they import or expose
+a deprecated public type in public signatures before removing it. Remove the
+type only after those clients have migrated, or keep a compatibility class
+until the migration is complete. When a migration is expected to last long,
+choose the removal point from downstream adoption and the support policy, not
+from a fixed one-minor-release delay.
+
 ## Add a new converter when its contract changes
 
 When a conversion function's arguments or return type change in an incompatible

src/frequenz/client/common/microgrid/components/__init__.py

@@ -0,0 +1,22 @@
+# License: MIT
+# Copyright © 2022 Frequenz Energy-as-a-Service GmbH
+
+"""Provide the deprecated component ID compatibility import."""
+
+from typing import final
+
+from frequenz.core.id import BaseId
+from typing_extensions import deprecated
+
+
+@deprecated(
+    "frequenz.client.common.microgrid.components.ComponentId is deprecated. "
+    "Use frequenz.client.common.microgrid.electrical_components."
+    "ElectricalComponentId instead."
+)
+@final
+class ComponentId(BaseId, str_prefix="CID"):
+    """A unique identifier for a microgrid component."""
+
+
+__all__ = ["ComponentId"]

tests/microgrid/test_ids.py

@@ -3,13 +3,32 @@
 
 """Tests for microgrid-related IDs."""
 
+import base64
+import importlib
+import os
+import pickle
+import subprocess
+import sys
+from pathlib import Path
+
 import pytest
 from frequenz.core.id import BaseId
 
 from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
+from frequenz.client.common.microgrid.components import ComponentId
 from frequenz.client.common.microgrid.electrical_components import ElectricalComponentId
 from frequenz.client.common.microgrid.sensors import SensorId
 
+# Trusted fixture generated from the released v0.3.8 tag (peeled commit
+# 66b890388c47412006c6767c6c3fcdd69c187c5d) with:
+# uv run --extra dev-pytest python -c 'import base64,pickle; from
+# frequenz.client.common.microgrid.components import ComponentId; print(
+# base64.b64encode(pickle.dumps(ComponentId(123))).decode())'
+_V0_3_8_COMPONENT_ID_PICKLE = base64.b64decode(
+    "gAWVTgAAAAAAAACMK2ZyZXF1ZW56LmNsaWVudC5jb21tb24ubWljcm9ncmlkLmNvbXBvbmVudHOU"
+    "jAtDb21wb25lbnRJZJSTlCmBlH2UjANfaWSUS3tzYi4="
+)
+
 
 @pytest.mark.parametrize(
     "id_class, prefix",
@@ -26,3 +45,143 @@ def test_string_representation(id_class: type[BaseId], prefix: str) -> None:
 
     assert str(_id) == f"{prefix}123"
     assert repr(_id) == f"{id_class.__name__}(123)"
+
+
+def test_component_id_is_a_distinct_deprecated_type() -> None:
+    """Test the restored component ID type remains distinct from its replacement."""
+    with pytest.deprecated_call():
+        component_id = ComponentId(123)
+    electrical_component_id = ElectricalComponentId(123)
+    with pytest.deprecated_call():
+        same_component_id = ComponentId(123)
+
+    assert component_id != electrical_component_id
+    assert not isinstance(component_id, ElectricalComponentId)
+    assert not isinstance(electrical_component_id, ComponentId)
+    assert {component_id, electrical_component_id} == {
+        same_component_id,
+        ElectricalComponentId(123),
+    }
+    assert {component_id: "old", electrical_component_id: "new"} == {
+        same_component_id: "old",
+        ElectricalComponentId(123): "new",
+    }
+
+
+def test_component_id_equality_and_validation() -> None:
+    """Test equality, hashing and validation for component IDs."""
+    with pytest.deprecated_call():
+        first = ComponentId(123)
+    with pytest.deprecated_call():
+        second = ComponentId(123)
+
+    assert first == second
+    assert hash(first) == hash(second)
+    assert int(first) == 123
+    assert str(first) == "CID123"
+    assert repr(first) == "ComponentId(123)"
+
+    with pytest.deprecated_call():
+        with pytest.raises(ValueError, match="ComponentId can't be negative"):
+            ComponentId(-1)
+
+
+@pytest.mark.parametrize(
+    "imports",
+    [
+        (
+            "from frequenz.client.common.microgrid.components import ComponentId\n"
+            "from frequenz.client.common.microgrid.electrical_components "
+            "import ElectricalComponentId"
+        ),
+        (
+            "from frequenz.client.common.microgrid.electrical_components "
+            "import ElectricalComponentId\n"
+            "from frequenz.client.common.microgrid.components import ComponentId"
+        ),
+    ],
+)
+def test_component_id_import_orders_keep_current_prefix_warning(imports: str) -> None:
+    """Test both import orders and acknowledge the current duplicate-prefix warning."""
+    source_root = Path(__file__).resolve().parents[2] / "src"
+    script = f"""
+import logging
+logging.basicConfig(level=logging.WARNING)
+{imports}
+assert ComponentId is not ElectricalComponentId
+"""
+    env = os.environ.copy()
+    env["PYTHONPATH"] = os.pathsep.join(
+        path for path in (str(source_root), env.get("PYTHONPATH")) if path
+    )
+    result = subprocess.run(
+        [sys.executable, "-c", script],
+        check=False,
+        capture_output=True,
+        text=True,
+        env=env,
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert "Prefix 'CID' is already registered" in result.stderr
+
+
+def test_component_id_does_not_restore_legacy_enums() -> None:
+    """Test that only the component ID compatibility symbol is restored."""
+    module = importlib.import_module("frequenz.client.common.microgrid.components")
+
+    assert not hasattr(module, "ComponentCategory")
+    assert not hasattr(module, "ComponentStateCode")
+    assert not hasattr(module, "ComponentErrorCode")
+
+
+def test_component_id_loads_an_old_path_pickle() -> None:
+    """Test loading a trusted pickle carrying the historical import path."""
+    source_root = Path(__file__).resolve().parents[2] / "src"
+    script = """
+import base64
+import pickle
+from frequenz.client.common.microgrid.components import ComponentId
+
+print(base64.b64encode(pickle.dumps(ComponentId(123))).decode())
+"""
+    env = os.environ.copy()
+    env["PYTHONPATH"] = os.pathsep.join(
+        path for path in (str(source_root), env.get("PYTHONPATH")) if path
+    )
+    result = subprocess.run(
+        [sys.executable, "-c", script],
+        check=False,
+        capture_output=True,
+        text=True,
+        env=env,
+    )
+
+    assert result.returncode == 0, result.stderr
+    payload = base64.b64decode(result.stdout.strip())
+    assert b"frequenz.client.common.microgrid.components" in payload
+    assert b"ComponentId" in payload
+
+    with pytest.deprecated_call():
+        loaded = pickle.loads(payload)
+    # Exact type identity is part of the restored compatibility behavior.
+    # pylint: disable-next=unidiomatic-typecheck
+    assert type(loaded) is ComponentId
+    with pytest.deprecated_call():
+        assert loaded == ComponentId(123)
+
+
+def test_component_id_loads_a_v0_3_8_pickle() -> None:
+    """Test loading a pickle generated by the released v0.3.8 class."""
+    payload = _V0_3_8_COMPONENT_ID_PICKLE
+
+    assert b"frequenz.client.common.microgrid.components" in payload
+    assert b"ComponentId" in payload
+    with pytest.deprecated_call():
+        loaded = pickle.loads(payload)
+
+    # Exact type identity is part of the restored compatibility behavior.
+    # pylint: disable-next=unidiomatic-typecheck
+    assert type(loaded) is ComponentId
+    with pytest.deprecated_call():
+        assert loaded == ComponentId(123)