diff --git a/backends/com_backend.py b/backends/com_backend.py index b0aa28d..244822b 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -193,6 +193,30 @@ def _msp(): return doc.ModelSpace +def _require_block_defined(doc, name): + """``doc.Blocks.Item(name)``, or a ``ValueError`` before ``InsertBlock`` runs. + + ``InsertBlock`` with an unknown name raises a bare COM error after the call + has already been dispatched; refusing here names the block, costs one + ``Blocks.Item`` probe and matches the headless engine's message. A layout + block (``IsLayout``) cannot be inserted into itself — same refusal by name. + Shared by ``block_insert`` and ``entity_create_block_ref``. + """ + if not isinstance(name, str) or not name.strip(): + raise TypeError("block name must be a non-empty string") + try: + block = doc.Blocks.Item(name) + except Exception as exc: + raise ValueError(f"block {name!r} is not defined") from exc + try: + is_layout = bool(block.IsLayout) + except Exception: + is_layout = False + if is_layout: + raise ValueError(f"block {name!r} is a layout block and cannot be inserted") + return block + + _BUILTIN_LINETYPES = {"continuous", "bylayer", "byblock"} @@ -2479,6 +2503,7 @@ class ComBackend(AutoCADBackend): layer=None, ) -> EntityInfo: def _sync(): + _require_block_defined(_acad_doc(), name) mspace = _msp() ref = mspace.InsertBlock( _apoint(x, y), @@ -3248,6 +3273,7 @@ class ComBackend(AutoCADBackend): layer=None, ) -> EntityInfo: def _sync(): + _require_block_defined(_acad_doc(), name) mspace = _msp() ref = mspace.InsertBlock( _apoint(x, y), diff --git a/backends/ezdxf_backend.py b/backends/ezdxf_backend.py index 465d1d8..f8e3fda 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -1087,6 +1087,28 @@ class EzdxfBackend(AutoCADBackend): raise RuntimeError("No document open. Call drawing_new() or drawing_open() first.") return self._doc + def _require_block(self, name): + """The block definition ``name`` names, or a refusal before any write. + + ``add_blockref`` never checks the name and ``add_auto_attribs`` skips + autofill when ``Insert.block()`` resolves to None, so a typo'd name + used to yield a dangling INSERT that dropped its attribute values and + that ``doc.audit()`` later deletes. A layout block (``*Model_Space``, + ``*Paper_Space``) is in ``doc.blocks`` too, but inserting one is the + reference cycle AutoCAD's INSERT refuses, so it is refused here by + name. Shared by ``block_insert`` and ``entity_create_block_ref`` so the + two tools that place a block reference agree on what a typo does. + """ + doc = self._require_doc() + if not isinstance(name, str) or not name.strip(): + raise TypeError("block name must be a non-empty string") + if name not in doc.blocks: + raise ValueError(f"block {name!r} is not defined") + blk = doc.blocks.get(name) + if blk.block_record.is_any_layout: + raise ValueError(f"block {name!r} is a layout block and cannot be inserted") + return blk + def _msp(self): """The space new geometry goes into — model space, or the current layout. @@ -3002,6 +3024,7 @@ class EzdxfBackend(AutoCADBackend): ) -> EntityInfo: def _sync(): msp = self._msp() + self._require_block(name) ent = msp.add_blockref( name, (float(x), float(y)), @@ -4485,13 +4508,7 @@ class EzdxfBackend(AutoCADBackend): } if layer: attribs["layer"] = layer - # `add_blockref` never checks the name, and `add_auto_attribs` - # silently skips autofill when `Insert.block()` resolves to None, - # so a typo'd name used to yield a dangling INSERT that dropped its - # attribute values and that `doc.audit()` later deletes. Refuse - # before writing, as COM's `InsertBlock` does for an unknown name. - if name not in self._doc.blocks: - raise ValueError(f"block {name!r} is not defined") + self._require_block(name) ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs) if attributes: # `add_auto_blockref` wraps the INSERT in an anonymous *U block, diff --git a/server.py b/server.py index 27a06b2..5c3e095 100644 --- a/server.py +++ b/server.py @@ -1656,7 +1656,14 @@ async def entity_create_block_ref( layer: Annotated[str | None, "Layer name"] = None, ctx: Context = None, ) -> dict: - """Insert a block reference (instance of an existing block definition).""" + """Insert a block reference (instance of an existing block definition). + + Refused before any write, on both engines: a name that is not a block + definition in this drawing (`block 'X' is not defined` — check + `block_list`), a layout block (`*Model_Space` / `*Paper_Space`, a + reference cycle) and a non-string name. The same gate as `block_insert`; + this tool is the no-attributes form. + """ await ctx.debug(f"Inserting block '{name}' at ({x},{y})") result = await _backend(ctx).entity_create_block_ref( name, x, y, scale_x, scale_y, rotation, layer diff --git a/tests/test_track_a_hardening.py b/tests/test_track_a_hardening.py new file mode 100644 index 0000000..32f278d --- /dev/null +++ b/tests/test_track_a_hardening.py @@ -0,0 +1,220 @@ +"""Track A hardening (v1.6 track E, wave 0): the block-tool defects the Track A +review found, fixed on both engines. + +Headless tests run against a real ezdxf document; the live engine runs against +a fake ActiveX surface that records every call, so the tests assert the call +shape (what is dispatched, in which order, and — for a refusal — that nothing +is dispatched at all). +""" + +from __future__ import annotations + +import types + +import pytest + +from backends.base import EntityInfo + +pytestmark = pytest.mark.asyncio + + +def _define_tagged_block(backend, name: str = "TB") -> None: + blk = backend._doc.blocks.new(name=name) + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5}) + + +# ── fake ActiveX surface ────────────────────────────────────────────────────── + + +class _FakeObject: + """An ActiveX object with only the members it is given; property writes are recorded.""" + + def __init__(self, object_name: str, handle: str, **members): + object.__setattr__(self, "ObjectName", object_name) + object.__setattr__(self, "Handle", handle) + object.__setattr__(self, "writes", []) + object.__setattr__(self, "deleted", False) + for name, value in members.items(): + object.__setattr__(self, name, value) + + def __setattr__(self, name, value): + self.writes.append((name, value)) + object.__setattr__(self, name, value) + + def __getattr__(self, name): + raise AttributeError(name) + + def Delete(self): + object.__setattr__(self, "deleted", True) + + +class _FakeBlock: + def __init__(self, name: str, is_layout: bool = False): + self.Name = name + self.IsLayout = is_layout + self.Count = 0 + self.calls: list[tuple[str, tuple]] = [] + + def __getattr__(self, attr): + if attr.startswith("Add"): + + def _call(*args): + self.calls.append((attr, args)) + return _FakeObject("AcDbEntity", f"M{len(self.calls)}") + + return _call + raise AttributeError(attr) + + +class _FakeBlocks: + def __init__(self, *names: str): + self.blocks: dict[str, _FakeBlock] = { + "*Model_Space": _FakeBlock("*Model_Space", is_layout=True), + } + for name in names: + self.blocks[name] = _FakeBlock(name) + + def Add(self, base_point, name): + self.blocks[name] = _FakeBlock(name) + self.blocks[name].base_point = tuple(base_point.value) + return self.blocks[name] + + def Item(self, name): + if name not in self.blocks: + raise RuntimeError(f"no block {name}") + return self.blocks[name] + + +class _FakeLayers: + def __init__(self, *names: str): + self.names = {"0", *names} + self.added: list[str] = [] + + def Item(self, name): + if name not in self.names: + raise RuntimeError(f"no layer {name}") + return types.SimpleNamespace(Name=name) + + def Add(self, name): + self.names.add(name) + self.added.append(name) + return types.SimpleNamespace(Name=name) + + +class _FakeSpace: + """The active layout block: records InsertBlock / AddText and hands back styled objects.""" + + def __init__(self, attributes: dict[str, str] | None = None): + self.calls: list[tuple[str, tuple]] = [] + self.attributes = attributes or {} + self.inserted: list[_FakeObject] = [] + + def InsertBlock(self, point, name, sx, sy, sz, rotation): + self.calls.append(("InsertBlock", (tuple(point.value), name, sx, sy, sz, rotation))) + attrs = tuple( + _FakeObject("AcDbAttribute", f"A{i}", TagString=tag, TextString=value) + for i, (tag, value) in enumerate(self.attributes.items()) + ) + ref = _FakeObject( + "AcDbBlockReference", f"R{len(self.inserted)}", Name=name, GetAttributes=lambda: attrs + ) + self.inserted.append(ref) + return ref + + def AddText(self, text, point, height): + self.calls.append(("AddText", (text, tuple(point.value), height))) + return _FakeObject("AcDbText", f"T{len(self.calls)}") + + +@pytest.fixture +def com(monkeypatch): + """A ComBackend over a recording fake: ``document`` and ``space`` are inspectable.""" + from backends import com_backend as module + + document = types.SimpleNamespace(Blocks=_FakeBlocks("TB"), Layers=_FakeLayers(), objects={}) + document.HandleToObject = lambda handle: document.objects[handle] + space = _FakeSpace() + monkeypatch.setattr(module, "_acad_doc", lambda: document) + monkeypatch.setattr(module, "_msp", lambda: space) + monkeypatch.setattr(module, "_regen", lambda: None) + # The real converter reads a dozen ActiveX members (bounding box, block + # geometry, ...); the fixture only needs the handle and the name back. + monkeypatch.setattr( + module, + "_entity_info", + lambda ent: EntityInfo( + handle=ent.Handle, + type="INSERT", + layer="0", + color=256, + linetype="ByLayer", + visible=True, + properties={"block_name": ent.Name}, + ), + ) + backend = module.ComBackend() + + async def _run_inline(func, *args, **kwargs): + return func(*args, **kwargs) + + monkeypatch.setattr(backend, "_run", _run_inline) + return backend, document, space + + +# ── Task 1: entity_create_block_ref refuses an undefined block ──────────────── + + +async def test_block_ref_of_an_undefined_block_is_refused_before_writing(backend): + before = len(list(backend._msp())) + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2) + assert len(list(backend._msp())) == before, "a refused insert must not write" + assert "NO_SUCH_BLOCK" not in backend._doc.blocks + + +async def test_block_ref_of_a_defined_block_still_inserts(backend): + _define_tagged_block(backend) + ref = await backend.entity_create_block_ref("TB", 5, 5, rotation=90) + info = await backend.entity_get(ref.handle) + assert info.type == "INSERT" and info.properties["block_name"] == "TB" + assert info.properties["rotation_deg"] == pytest.approx(90.0) + + +@pytest.mark.parametrize("name", ["*Model_Space", "*Paper_Space"]) +async def test_layout_blocks_are_refused_by_both_placing_tools(backend, name): + with pytest.raises(ValueError, match="layout block"): + await backend.entity_create_block_ref(name, 0, 0) + with pytest.raises(ValueError, match="layout block"): + await backend.block_insert(name, 0, 0) + assert backend._doc.audit().errors == [], "nothing was written, nothing to fix" + + +async def test_a_non_string_block_name_is_a_type_error_on_both_placing_tools(backend): + with pytest.raises(TypeError, match="non-empty string"): + await backend.entity_create_block_ref(None, 0, 0) + with pytest.raises(TypeError, match="non-empty string"): + await backend.block_insert("", 0, 0) + + +async def test_com_block_ref_probes_the_block_table_before_insert_block(com): + backend, document, space = com + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2) + assert space.calls == [], "InsertBlock must not be dispatched for an unknown name" + with pytest.raises(ValueError, match="layout block"): + await backend.entity_create_block_ref("*Model_Space", 0, 0) + assert space.calls == [] + + ref = await backend.entity_create_block_ref("TB", 1, 2, 2.0, 2.0, 90.0) + assert ref.properties["block_name"] == "TB" + assert space.calls == [ + ("InsertBlock", ((1.0, 2.0, 0.0), "TB", 2.0, 2.0, 1.0, pytest.approx(1.5708, abs=1e-4))) + ] + + +async def test_com_block_insert_shares_the_same_gate(com): + backend, document, space = com + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.block_insert("NO_SUCH_BLOCK", 1, 2, attributes={"TAG": "P-1"}) + assert space.calls == []