Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions PYEGERIA_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -1121,8 +1121,14 @@ into the entry now, so it isn't rediscovered from scratch later.

### ISSUE-126: `ProductManager` and `DigitalBusiness` never set `collection_command_root`, so every inherited `CollectionManager` method that uses it raises `AttributeError`

**Layer:** pyegeria · **Status:** open, logged only, not fixed (awaiting approval) · **Found:** 2026-10-05,
live cleanup of a throwaway data contract (PR #427's live check).
**Layer:** pyegeria · **Status:** fixed on branch `fix/issue-126-127-collection-manager-subclasses`
(2026-10-05), pending PR/merge · **Found:** 2026-10-05, live cleanup of a throwaway data contract (PR #427's
live check).

**Fix + scope (2026-10-05):** both constructors now call `CollectionManager.__init__`, as `GlossaryManager`
does; that also restores `metadata_expert_command_root`, which the SmartQuery relationship methods use. An AST scan of all 91 classes in
`pyegeria/omvs/` found no other subclass whose `__init__` skips its base's, and
`test_every_omvs_subclass_runs_its_base_constructor` now fails if one ever does.

**What:** `ProductManager.__init__` and `DigitalBusiness.__init__` call `ServerClient.__init__` directly instead
of `CollectionManager.__init__` (`GlossaryManager` does it correctly). `CollectionManager.__init__` is what sets
Expand All @@ -1142,8 +1148,17 @@ constructs each `CollectionManager` subclass and asserts the attribute exists wo

### ISSUE-127: `CollectionManager.delete_collection(cascade=True)` silently never sends `cascadeDelete` -- same shape as ISSUE-62

**Layer:** pyegeria · **Status:** open, logged only, not fixed (awaiting approval) · **Found:** 2026-10-05, found
by capturing the request body while diagnosing a failed cleanup.
**Layer:** pyegeria · **Status:** fixed on branch `fix/issue-126-127-collection-manager-subclasses`
(2026-10-05), pending PR/merge · **Found:** 2026-10-05, found by capturing the request body while diagnosing a
failed cleanup.

**Fix + scope (2026-10-05):** a scan of all 42 callers of `_async_delete_element_request` found exactly two
that pre-fill a default body *and* pass a cascade flag: `CollectionManager._async_delete_collection` and
`ProductManager._async_delete_digital_product` (so `delete_digital_product(cascade=True)` was never cascading
either -- the live check's own product cleanup included). Both now pass `None` so the helper applies the flag.
`delete_digital_product`'s docstring sample also used the wrong key (`cascadedDelete`); corrected. The helper
itself is unchanged: a caller that passes an explicit body dict *and* `cascade=True` still gets the body's
value, which is documented behaviour ("body supersedes").

**What:** `_async_delete_collection` always pre-fills `body = {"class": "DeleteElementRequestBody"}` when none is
given and hands that dict to `_async_delete_element_request(url, body, cascade)`.
Expand Down
4 changes: 2 additions & 2 deletions pyegeria/omvs/collection_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -6567,8 +6567,8 @@ async def _async_delete_collection(self, collection_guid: str, body: dict | Dele


"""
if body is None:
body = {"class": "DeleteElementRequestBody"}
# No default body here: _async_delete_element_request applies `cascade` only when it builds the body
# itself, so a pre-filled dict made `cascade=True` a silent no-op (ISSUE-127).
url = f"{self.collection_command_root}/{collection_guid}/delete"
await self._async_delete_element_request(url, body, cascade)
logger.info(f"Deleted collection {collection_guid} with cascade {cascade}")
Expand Down
2 changes: 1 addition & 1 deletion pyegeria/omvs/digital_business.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def __init__(
user_pwd: Optional[str] = None,
token: Optional[str] = None,
timeout: int = None):
ServerClient.__init__(self, view_server, platform_url, user_id, user_pwd, token, timeout=timeout)
CollectionManager.__init__(self, view_server, platform_url, user_id, user_pwd, token, timeout=timeout)
self.view_server = self.server_name
self.platform_url = self.platform_url
self.user_id = self.user_id
Expand Down
10 changes: 5 additions & 5 deletions pyegeria/omvs/product_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def __init__(
user_pwd: Optional[str] = None,
token: Optional[str] = None,
timeout: int = None):
ServerClient.__init__(self, view_server, platform_url, user_id, user_pwd, token, timeout=timeout)
CollectionManager.__init__(self, view_server, platform_url, user_id, user_pwd, token, timeout=timeout)
self.view_server = self.server_name
self.platform_url = self.platform_url
self.user_id = self.user_id
Expand Down Expand Up @@ -322,12 +322,12 @@ async def _async_delete_digital_product(
```json
{
"class": "DeleteElementRequestBody",
"cascadedDelete": false
"cascadeDelete": false
}
```
"""
if body is None:
body = {"class": "DeleteElementRequestBody"}
# No default body here: _async_delete_element_request applies `cascade` only when it builds the body
# itself, so a pre-filled dict made `cascade=True` a silent no-op (ISSUE-127).
url = f"{self.product_manager_command_root}/collections/{digital_product_guid}/delete"
await self._async_delete_element_request(url, body, cascade)
logger.info(f"Deleted digital product {digital_product_guid} with cascade {cascade}")
Expand Down Expand Up @@ -364,7 +364,7 @@ def delete_digital_product(
```json
{
"class": "DeleteElementRequestBody",
"cascadedDelete": false
"cascadeDelete": false
}
```
"""
Expand Down
118 changes: 118 additions & 0 deletions tests/micro-tests/test_collection_manager_subclasses.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"""
ISSUE-126: ProductManager and DigitalBusiness called ServerClient.__init__ instead of
CollectionManager.__init__, so collection_command_root (used by 27 inherited methods) was never set.
ISSUE-127: delete_collection / delete_digital_product pre-filled a body dict, so the shared helper
silently ignored cascade=True.
"""
import ast
import glob
import json
import os

import pytest

from pyegeria.omvs.collection_manager import CollectionManager
from pyegeria.omvs.digital_business import DigitalBusiness
from pyegeria.omvs.glossary_manager import GlossaryManager
from pyegeria.omvs.product_manager import ProductManager

OMVS_DIR = os.path.join(os.path.dirname(__file__), "..", "..", "pyegeria", "omvs")


def test_every_omvs_subclass_runs_its_base_constructor():
"""Structural guard for the whole bug class: a client that derives from another OMVS client but whose
__init__ never calls that client's __init__ silently misses the attributes it sets."""
classes = {}
for path in sorted(glob.glob(os.path.join(OMVS_DIR, "*.py"))):
for node in ast.parse(open(path).read()).body:
if not isinstance(node, ast.ClassDef):
continue
bases = [b.id for b in node.bases if isinstance(b, ast.Name)]
init = next((m for m in node.body if isinstance(m, ast.FunctionDef) and m.name == "__init__"), None)
called = set()
if init:
for call in ast.walk(init):
if isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute) and call.func.attr == "__init__":
owner = call.func.value
called.add(owner.id if isinstance(owner, ast.Name) else "super()")
classes[node.name] = (bases, init is not None, called)

offenders = [f"{name} (skips {base}.__init__)"
for name, (bases, has_init, called) in classes.items() if has_init
for base in bases
if base in classes and base != "ServerClient" and base not in called and "super()" not in called]
assert offenders == []


@pytest.mark.parametrize("cls", [CollectionManager, GlossaryManager, ProductManager, DigitalBusiness])
def test_collection_manager_subclasses_have_the_collection_roots(cls, monkeypatch):
monkeypatch.setattr(cls, "check_connection", lambda self: "test")
client = cls("view-server", "https://localhost:1", "user", "pwd")

assert client.collection_command_root.endswith("/api/open-metadata/collection-manager/collections")
assert client.metadata_expert_command_root.endswith("/api/open-metadata/metadata-expert")


def test_each_subclass_keeps_its_own_root(monkeypatch):
monkeypatch.setattr(ProductManager, "check_connection", lambda self: "test")
monkeypatch.setattr(DigitalBusiness, "check_connection", lambda self: "test")

assert ProductManager("v", "https://localhost:1", "u", "p").product_manager_command_root.endswith("/product-manager")
assert DigitalBusiness("v", "https://localhost:1", "u", "p").digital_business_command_root.endswith("/digital-business")


# --- ISSUE-127 ------------------------------------------------------------------------------

class _Response:
def json(self):
return {"relatedHTTPCode": 200}


def _capture(cls, monkeypatch):
monkeypatch.setattr(cls, "check_connection", lambda self: "test")
client = cls("view-server", "https://localhost:1", "user", "pwd")
calls: list = []

async def fake_make_request(method, url, payload=None, *args, **kwargs):
calls.append({"url": url, "body": json.loads(payload) if isinstance(payload, str) else payload})
return _Response()

client._async_make_request = fake_make_request
return client, calls


@pytest.mark.asyncio
@pytest.mark.parametrize("cls, method, tail", [
(CollectionManager, "_async_delete_collection", "/collections/g1/delete"),
(ProductManager, "_async_delete_collection", "/collections/g1/delete"), # inherited; was broken by ISSUE-126
(DigitalBusiness, "_async_delete_collection", "/collections/g1/delete"), # likewise
(ProductManager, "_async_delete_digital_product", "/collections/g1/delete"),
])
async def test_cascade_true_reaches_the_server(cls, method, tail, monkeypatch):
client, calls = _capture(cls, monkeypatch)

await getattr(client, method)("g1", cascade=True)

assert calls[0]["url"].endswith(tail)
assert calls[0]["body"]["cascadeDelete"] is True
assert calls[0]["body"]["class"] == "DeleteElementRequestBody"


@pytest.mark.asyncio
@pytest.mark.parametrize("method", ["_async_delete_collection", "_async_delete_digital_product"])
async def test_cascade_default_does_not_cascade(method, monkeypatch):
client, calls = _capture(ProductManager, monkeypatch)

await getattr(client, method)("g1")

assert calls[0]["body"].get("cascadeDelete") in (None, False)


@pytest.mark.asyncio
async def test_an_explicit_body_is_still_honoured(monkeypatch):
client, calls = _capture(CollectionManager, monkeypatch)
body = {"class": "DeleteElementRequestBody", "cascadeDelete": True, "deleteMethod": "PURGE"}

await client._async_delete_collection("g1", body=body)

assert calls[0]["body"]["cascadeDelete"] is True and calls[0]["body"]["deleteMethod"] == "PURGE"
Loading