diff --git a/backends/com_backend.py b/backends/com_backend.py index 0218371..fdef6b2 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -394,6 +394,24 @@ def _require_block_defined(doc, name): return block +def _block_names(doc) -> set[str]: + """Every block name the drawing holds, lower-cased. + + ``doc.Blocks.Count`` / ``doc.Blocks.Item(index)`` / ``block.Name`` are the + measured members ``xref_manage`` already walks. The set is lower-cased + because AutoCAD symbol-table names are case-insensitive, so ``BASE`` and + ``Base`` are one block and re-attaching under either name hits the same + definition. + """ + names: set[str] = set() + for index in range(doc.Blocks.Count): + try: + names.add(str(doc.Blocks.Item(index).Name).lower()) + except Exception: # noqa: BLE001 - a block that cannot name itself cannot collide + continue + return names + + _BUILTIN_LINETYPES = {"continuous", "bylayer", "byblock"} #: `drawing_properties_*` field → `IAcadSummaryInfo` property. @@ -1882,6 +1900,17 @@ class ComBackend(AutoCADBackend): def _sync(): doc = _acad_doc() + # The contract and the tool docstring both promise this refusal, + # and the ezdxf engine has always had it (`if name in doc.blocks`). + # Without it a second attach under a name the drawing already holds + # either re-points the existing definition or fails inside AutoCAD, + # while the payload still claims the new `path` was attached. + # Block names are case-insensitive in AutoCAD, so the comparison is. + if name.lower() in _block_names(doc): + raise ValueError( + f"xref_attach: the drawing already holds a block named {name!r}; " + "detach it first with xref_manage(action='detach')" + ) reference = doc.ModelSpace.AttachExternalReference( str(source), name, @@ -1929,8 +1958,17 @@ class ComBackend(AutoCADBackend): return found if action == "list": + # `kind` is null, not "attach". Measured from the seat's own + # registered type library (acax25*.tlb, "AutoCAD 2025 Type + # Library", shipped with AutoCAD 2026): IAcadBlock's xref + # members are exactly IsXRef, Path, Name, Reload, Unload, + # Bind, Detach, XRefDatabase -- there is no overlay indicator + # anywhere on the interface. An overlay (which xref_attach + # itself creates, kind='overlay' -> bOverlay=True) is + # therefore not knowable here, and a constant "attach" would + # read to the caller as measured. Same honesty as `inserts`. rows = [ - {"name": b.Name, "path": str(b.Path or ""), "kind": "attach", "inserts": None} + {"name": b.Name, "path": str(b.Path or ""), "kind": None, "inserts": None} for b in _xref_blocks() ] rows.sort(key=lambda row: row["name"]) diff --git a/backends/contracts/refs.py b/backends/contracts/refs.py index f05faec..e8df4e2 100644 --- a/backends/contracts/refs.py +++ b/backends/contracts/refs.py @@ -54,11 +54,16 @@ class RefsContract(ABC): """One of :data:`XREF_ACTIONS` on one external reference. ``list`` ignores `name` and returns - ``{ok, xrefs: [{name, path, kind, inserts}], backend}``. ``path`` - rewrites the saved path to `new_path` and returns the new one. - ``detach`` removes every insert of the xref and its definition, and - reports ``inserts_removed``. ``reload`` and ``bind`` need a live seat's - xref manager: the headless engine raises + ``{ok, xrefs: [{name, path, kind, inserts}], backend}``. ``kind`` and + ``inserts`` are ``None`` on a live seat -- ActiveX's ``IAcadBlock`` + carries no overlay indicator and no per-xref insert count (measured on + the registered type library), and a constant would read as measured. + ``path`` rewrites the saved path to `new_path` and returns the new one. + ``detach`` removes every insert of the xref -- in *every* entity space, + model space and each paper-space layout, because an insert left behind + with no block definition is a corrupt drawing -- then the definition, + and reports ``inserts_removed``. ``inserts`` counts the same set. + ``reload`` and ``bind`` need a live seat's xref manager: the headless engine raises ``UnsupportedCapabilityError("xref_live", ...)`` rather than pretending, which is why the key exists; ``list``/``detach``/``path`` are *not* behind it. diff --git a/backends/ezdxf_backend.py b/backends/ezdxf_backend.py index 582dac3..5e6335e 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -2455,6 +2455,30 @@ class EzdxfBackend(AutoCADBackend): # misses every overlay. xref_mask = const.BLK_XREF | const.BLK_XREF_OVERLAY + def _inserts_of(key: str): + """Every INSERT of `key`, paired with the layout that owns it. + + Model space is *not* the drawing. `doc.blocks` is the one + iterable that reaches every entity space -- it yields the + block layouts behind model space and each paper-space tab as + well as the real block definitions -- and it is exactly what + ezdxf's own `delete_block(safe=True)` searches through + `doc.query("INSERT[name==...]i")`. Counting or deleting over + `doc.modelspace()` alone under-reports an xref shown on a + sheet and leaves that INSERT behind with no BLOCK definition. + The name comparison is case-insensitive for the same reason: + the `i` on that query string. + """ + wanted = key.lower() + found = [] + for layout in doc.blocks: + for entity in layout: + if entity.dxftype() != "INSERT": + continue + if str(entity.dxf.name).lower() == wanted: + found.append((layout, entity)) + return found + def _rows(): rows = [] for block in doc.blocks: @@ -2466,11 +2490,7 @@ class EzdxfBackend(AutoCADBackend): "name": block.name, "path": str(block.block.dxf.xref_path or ""), "kind": "overlay" if flags & const.BLK_XREF_OVERLAY else "attach", - "inserts": sum( - 1 - for entity in doc.modelspace() - if entity.dxftype() == "INSERT" and entity.dxf.name == block.name - ), + "inserts": len(_inserts_of(block.name)), } ) rows.sort(key=lambda row: row["name"]) @@ -2491,14 +2511,15 @@ class EzdxfBackend(AutoCADBackend): self._mark_dirty() return {"ok": True, "name": key, "path": str(new_path), "backend": self.name} - victims = [ - entity - for entity in doc.modelspace() - if entity.dxftype() == "INSERT" and entity.dxf.name == key - ] - for entity in victims: - doc.modelspace().delete_entity(entity) - doc.blocks.delete_block(key, safe=False) + victims = _inserts_of(key) + for layout, entity in victims: + layout.delete_entity(entity) + # safe=True, as the plan pins it: the in-use check is the only + # thing standing between a missed insert and a saved drawing whose + # INSERT has no BLOCK behind it (ezdxf's recover reports that as + # `FIX 103 UNDEFINED_BLOCK` and throws the geometry away). If the + # scan above ever misses one, this raises instead of corrupting. + doc.blocks.delete_block(key, safe=True) self._mark_dirty() return { "ok": True, diff --git a/server.py b/server.py index b1a4849..50a5ff2 100644 --- a/server.py +++ b/server.py @@ -8635,8 +8635,11 @@ async def xref_manage( `capability: "xref_live"` rather than pretended, and the refusal says which engine does them. Other refusals: an action outside the five, an unknown xref name, a 'path' action with no `new_path`. On the live engine a listed - row's `inserts` is null — ActiveX reports no per-xref insert count and this - server does not invent one. + row's `inserts` and `kind` are both null — ActiveX's block interface + carries neither a per-xref insert count nor an overlay indicator, and this + server does not invent them. Headlessly both are measured: `inserts` + counts every insert in the drawing, model space and paper space alike, and + `detach` removes exactly those. """ await ctx.info(f"Xref {action} {name or '(all)'}") validated = str(validate_path(new_path)) if new_path else None diff --git a/tests/test_refs_contract.py b/tests/test_refs_contract.py index 33512d2..990e47a 100644 --- a/tests/test_refs_contract.py +++ b/tests/test_refs_contract.py @@ -155,6 +155,69 @@ async def test_path_rewrites_the_saved_path_and_detach_removes_the_block( assert (await backend.xref_manage("", "list", None))["xrefs"] == [] +@pytest.mark.asyncio +async def test_an_xref_shown_on_a_sheet_is_counted_and_detached_with_it(backend, external): + """Model space is not the drawing. + + An xref is most often *shown* on a paper-space sheet. Counting or deleting + over `doc.modelspace()` alone reported 1 insert where the drawing held 2 + and then removed the definition with `safe=False`, leaving the sheet's + INSERT with no BLOCK behind it -- which ezdxf's own recover reports as + `FIX 103 UNDEFINED_BLOCK` and throws the geometry away. + """ + await backend.xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + doc = backend._doc + doc.layout("Layout1").add_blockref("BASE", (10, 10)) + + listing = await backend.xref_manage("", "list", None) + assert listing["xrefs"][0]["inserts"] == 2 + + detached = await backend.xref_manage("BASE", "detach", None) + assert detached["inserts_removed"] == 2 + assert "BASE" not in doc.blocks + assert [e for e in doc.layout("Layout1") if e.dxftype() == "INSERT"] == [] + + +@pytest.mark.asyncio +async def test_detach_leaves_a_drawing_that_survives_a_round_trip(backend, external, tmp_path): + """The evidence a caller actually cares about: the saved file audits clean.""" + import ezdxf.recover + + await backend.xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + backend._doc.layout("Layout1").add_blockref("BASE", (10, 10)) + await backend.xref_manage("BASE", "detach", None) + + out = tmp_path / "detached.dxf" + backend._doc.saveas(out) + _, auditor = ezdxf.recover.readfile(str(out)) + assert [f.message for f in auditor.fixes] == [] + assert [e.message for e in auditor.errors] == [] + + +@pytest.mark.asyncio +async def test_the_definition_is_dropped_with_the_in_use_check_on(backend, external): + """`safe=True` is the seatbelt, not decoration. + + The broken version passed `safe=False`, the flag that *suppresses* the + in-use check, so a missed insert became a corrupt file instead of an + error. This pins the check: with the model-space insert gone but the + sheet's still there -- exactly the state the old victim scan produced -- + `delete_block` refuses. + """ + from ezdxf.lldxf.const import DXFBlockInUseError + + await backend.xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + doc = backend._doc + doc.layout("Layout1").add_blockref("BASE", (10, 10)) + msp = doc.modelspace() + for entity in [e for e in msp if e.dxftype() == "INSERT"]: + msp.delete_entity(entity) + + with pytest.raises(DXFBlockInUseError): + doc.blocks.delete_block("BASE", safe=True) + assert "BASE" in doc.blocks + + @pytest.mark.asyncio async def test_reload_and_bind_refuse_headlessly_with_the_xref_live_key(backend, external): await backend.xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") @@ -357,6 +420,74 @@ async def test_com_manage_uses_reload_bind_detach_on_the_block(monkeypatch): assert listing["xrefs"][0]["path"] == "C:/refs/BASE.dwg" +@pytest.mark.asyncio +async def test_com_list_reports_kind_as_unknown_rather_than_inventing_attach(monkeypatch): + """IAcadBlock has no overlay indicator. + + Measured from the seat's own registered type library (acax25*.tlb, + "AutoCAD 2025 Type Library", installed with AutoCAD 2026): the xref + members are IsXRef, Path, Name, Reload, Unload, Bind, Detach, + XRefDatabase -- nothing says attach-vs-overlay. The fake therefore has no + such member either (a fake that models one enshrines the bug), and the + honest row says `None`, the way `inserts` already does. + """ + import backends.com_backend as cb + + doc = _ComDoc([_Block("BASE", path="C:/refs/BASE.dwg")]) + monkeypatch.setattr(cb, "_acad_doc", lambda: doc) + row = (await _com_backend().xref_manage("", "list", None))["xrefs"][0] + assert row["kind"] is None + assert row["inserts"] is None + for invented in ("IsOverlay", "Overlay", "XRefType"): + assert not hasattr(doc.Blocks.Item("BASE"), invented) + + +@pytest.mark.asyncio +async def test_com_attach_refuses_a_name_the_drawing_already_holds(monkeypatch, external): + """The refusal the contract and the tool docstring both promise. + + It has to be on this engine too, and it has to fire *before* + AttachExternalReference is dispatched -- a second attach under a held name + re-points or half-writes the definition while the payload still claims the + new path was attached. + """ + import backends.com_backend as cb + + doc = _ComDoc([_Block("BASE", path="C:/other/BASE.dwg")]) + monkeypatch.setattr(cb, "_acad_doc", lambda: doc) + with pytest.raises(ValueError) as excinfo: + await _com_backend().xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + assert "BASE" in str(excinfo.value) + assert "detach" in str(excinfo.value) + assert doc.ModelSpace.attached == [] + + +@pytest.mark.asyncio +async def test_com_attach_refuses_a_plain_block_of_the_same_name_case_insensitively( + monkeypatch, external +): + """AutoCAD symbol-table names are case-insensitive and a collision is a + collision whether or not the block in the way is itself an xref.""" + import backends.com_backend as cb + + doc = _ComDoc([_Block("Base", is_xref=False)]) + monkeypatch.setattr(cb, "_acad_doc", lambda: doc) + with pytest.raises(ValueError): + await _com_backend().xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + assert doc.ModelSpace.attached == [] + + +@pytest.mark.asyncio +async def test_com_attach_still_works_when_no_block_of_that_name_exists(monkeypatch, external): + import backends.com_backend as cb + + doc = _ComDoc([_Block("OTHER", is_xref=True)]) + monkeypatch.setattr(cb, "_acad_doc", lambda: doc) + result = await _com_backend().xref_attach(external, (0.0, 0.0), 1.0, 0.0, "attach") + assert result["ok"] is True and result["name"] == "BASE" + assert len(doc.ModelSpace.attached) == 1 + + @pytest.mark.asyncio async def test_com_raster_calls_addraster_with_radians(monkeypatch, tmp_path): import math