fix(blocks): block_define refuses a non-finite base point and a missing layer unless create_layers
base_x/base_y were only float()-cast, so a NaN base point was written into the BLOCK record; a primitive on a layer the drawing lacks was written as a dangling reference doc.audit() never flags (and on COM would fail mid-loop after Blocks.Add). validate_base_point names the key; the layer table is probed before the first write and the missing names are refused, or created and reported as layers_created when create_layers=true (new optional kwarg, contract + both engines + tool). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5
parent
7f79497928
commit
375c9f4c70
@@ -33,7 +33,10 @@ _POSITIVE = {"r", "height"}
|
|||||||
def _scalar(value, where: str, *, positive: bool = False) -> float:
|
def _scalar(value, where: str, *, positive: bool = False) -> float:
|
||||||
if isinstance(value, bool) or not isinstance(value, (int, float)):
|
if isinstance(value, bool) or not isinstance(value, (int, float)):
|
||||||
raise TypeError(f"{where} must be a number, got {value!r}")
|
raise TypeError(f"{where} must be a number, got {value!r}")
|
||||||
|
try:
|
||||||
number = float(value)
|
number = float(value)
|
||||||
|
except OverflowError as exc: # a 400-digit JSON integer is an int, not a float
|
||||||
|
raise TypeError(f"{where} must be finite") from exc
|
||||||
if not math.isfinite(number):
|
if not math.isfinite(number):
|
||||||
raise TypeError(f"{where} must be finite")
|
raise TypeError(f"{where} must be finite")
|
||||||
if positive and number <= 0:
|
if positive and number <= 0:
|
||||||
@@ -204,6 +207,18 @@ def validate_attdef_specs(attdefs) -> list[dict]:
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def validate_base_point(base_x, base_y) -> tuple[float, float]:
|
||||||
|
"""``block_define``'s base point as finite floats; ``TypeError`` names the key.
|
||||||
|
|
||||||
|
Only ``float()`` was applied before, so ``float("nan")`` was written into
|
||||||
|
the BLOCK record headlessly and would reach AutoCAD as a NaN VARIANT.
|
||||||
|
"""
|
||||||
|
return (
|
||||||
|
_scalar(base_x, "block_define: 'base_x'"),
|
||||||
|
_scalar(base_y, "block_define: 'base_y'"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def validate_attribute_values(attributes, where: str = "attributes") -> dict[str, str]:
|
def validate_attribute_values(attributes, where: str = "attributes") -> dict[str, str]:
|
||||||
"""``{TAG: value}`` for ``block_insert`` / ``block_set_attributes`` as the
|
"""``{TAG: value}`` for ``block_insert`` / ``block_set_attributes`` as the
|
||||||
strings that will be written, refused before any write.
|
strings that will be written, refused before any write.
|
||||||
|
|||||||
+27
-3
@@ -3880,18 +3880,24 @@ class ComBackend(AutoCADBackend):
|
|||||||
base_x=0.0,
|
base_x=0.0,
|
||||||
base_y=0.0,
|
base_y=0.0,
|
||||||
overwrite=False,
|
overwrite=False,
|
||||||
|
create_layers=False,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""A block definition with ATTDEFs from typed specs, through ActiveX.
|
"""A block definition with ATTDEFs from typed specs, through ActiveX.
|
||||||
|
|
||||||
``Blocks.Add`` creates the definition; each primitive is one ``Add*``
|
``Blocks.Add`` creates the definition; each primitive is one ``Add*``
|
||||||
call on the Block object, each ATTDEF one ``AddAttribute``. Overwrite
|
call on the Block object, each ATTDEF one ``AddAttribute``. Overwrite
|
||||||
deletes the existing definition's members and re-adds — the name and
|
deletes the existing definition's members and re-adds — the name and
|
||||||
its INSERTs survive. Unit-tested against a fake ActiveX surface;
|
its INSERTs survive. The base point must be finite (a NaN would reach
|
||||||
executed live by ``scripts/smoke_pid_com.py``.
|
AutoCAD as a VARIANT) and every primitive layer must exist
|
||||||
|
(``Layers.Item`` probe before ``Blocks.Add`` — ``obj.Layer = name`` for
|
||||||
|
an unknown layer would otherwise fail mid-loop with the definition half
|
||||||
|
written) unless ``create_layers`` adds them first. Unit-tested against
|
||||||
|
a fake ActiveX surface; executed live by ``scripts/smoke_pid_com.py``.
|
||||||
"""
|
"""
|
||||||
from backends.block_specs import (
|
from backends.block_specs import (
|
||||||
solid_vertices,
|
solid_vertices,
|
||||||
validate_attdef_specs,
|
validate_attdef_specs,
|
||||||
|
validate_base_point,
|
||||||
validate_entity_specs,
|
validate_entity_specs,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -3903,6 +3909,8 @@ class ComBackend(AutoCADBackend):
|
|||||||
clean_name = sanitize_symbol_name(name, kind="block")
|
clean_name = sanitize_symbol_name(name, kind="block")
|
||||||
ents = validate_entity_specs(entities)
|
ents = validate_entity_specs(entities)
|
||||||
atts = validate_attdef_specs(attdefs or [])
|
atts = validate_attdef_specs(attdefs or [])
|
||||||
|
base_xy = validate_base_point(base_x, base_y)
|
||||||
|
wanted_layers = sorted({spec["layer"] for spec in ents if spec["layer"] != "0"})
|
||||||
# acAlignmentLeft / Center / Right / MiddleCenter
|
# acAlignmentLeft / Center / Right / MiddleCenter
|
||||||
align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10}
|
align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10}
|
||||||
|
|
||||||
@@ -3923,7 +3931,22 @@ class ComBackend(AutoCADBackend):
|
|||||||
"pass overwrite=true to replace its contents"
|
"pass overwrite=true to replace its contents"
|
||||||
)
|
)
|
||||||
replaced = existing is not None
|
replaced = existing is not None
|
||||||
base = _apoint(float(base_x), float(base_y), 0.0)
|
missing_layers = []
|
||||||
|
for layer in wanted_layers:
|
||||||
|
try:
|
||||||
|
doc.Layers.Item(layer)
|
||||||
|
except Exception:
|
||||||
|
missing_layers.append(layer)
|
||||||
|
if missing_layers and not create_layers:
|
||||||
|
raise ValueError(
|
||||||
|
"block_define: layer(s) "
|
||||||
|
+ ", ".join(repr(layer) for layer in missing_layers)
|
||||||
|
+ " do not exist in the drawing; create them with layer_create "
|
||||||
|
"or pass create_layers=true"
|
||||||
|
)
|
||||||
|
for layer in missing_layers:
|
||||||
|
doc.Layers.Add(layer)
|
||||||
|
base = _apoint(base_xy[0], base_xy[1], 0.0)
|
||||||
if existing is None:
|
if existing is None:
|
||||||
block = doc.Blocks.Add(base, clean_name)
|
block = doc.Blocks.Add(base, clean_name)
|
||||||
else:
|
else:
|
||||||
@@ -3989,6 +4012,7 @@ class ComBackend(AutoCADBackend):
|
|||||||
"entity_count": len(ents),
|
"entity_count": len(ents),
|
||||||
"attdef_count": len(atts),
|
"attdef_count": len(atts),
|
||||||
"replaced": replaced,
|
"replaced": replaced,
|
||||||
|
"layers_created": missing_layers,
|
||||||
"backend": "com",
|
"backend": "com",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -58,9 +58,19 @@ class BlockContract(ABC):
|
|||||||
base_x: float = 0.0,
|
base_x: float = 0.0,
|
||||||
base_y: float = 0.0,
|
base_y: float = 0.0,
|
||||||
overwrite: bool = False,
|
overwrite: bool = False,
|
||||||
|
create_layers: bool = False,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Create (or with ``overwrite`` replace the contents of) a block
|
"""Create (or with ``overwrite`` replace the contents of) a block
|
||||||
definition from typed primitive specs and ATTDEF specs — see
|
definition from typed primitive specs and ATTDEF specs — see
|
||||||
``backends/block_specs.py`` for the spec vocabulary. Returns
|
``backends/block_specs.py`` for the spec vocabulary.
|
||||||
``{ok, name, entity_count, attdef_count, replaced, backend}``."""
|
|
||||||
|
Refusals, all before any write: a malformed spec (``TypeError`` naming
|
||||||
|
``entities[i]``/``attdefs[i]`` and the key), a non-finite or non-numeric
|
||||||
|
``base_x``/``base_y`` (``TypeError`` naming it), a name clash without
|
||||||
|
``overwrite`` (``ValueError``), and a primitive ``layer`` that does not
|
||||||
|
exist in the drawing (``ValueError`` listing the missing names) unless
|
||||||
|
``create_layers`` is true, in which case the missing layers are created
|
||||||
|
with the engine's defaults and listed in ``layers_created``. Returns
|
||||||
|
``{ok, name, entity_count, attdef_count, replaced, layers_created,
|
||||||
|
backend}``."""
|
||||||
...
|
...
|
||||||
|
|||||||
@@ -4976,20 +4976,26 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
base_x=0.0,
|
base_x=0.0,
|
||||||
base_y=0.0,
|
base_y=0.0,
|
||||||
overwrite=False,
|
overwrite=False,
|
||||||
|
create_layers=False,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""A block definition with ATTDEFs from typed specs.
|
"""A block definition with ATTDEFs from typed specs.
|
||||||
|
|
||||||
Validation runs before any write, so a malformed entry leaves no
|
Validation runs before any write, so a malformed entry leaves no
|
||||||
definition behind. ``overwrite`` replaces the *contents* of an existing
|
definition behind: specs, the base point (``base_x``/``base_y`` must be
|
||||||
definition rather than deleting it: existing INSERTs keep pointing at
|
finite numbers — ``float("nan")`` used to be written into the BLOCK
|
||||||
the name and show the new geometry (deleting a referenced block is
|
record) and the layer table (a primitive's ``layer`` must exist, or
|
||||||
refused by both engines; replacing contents works on both).
|
``create_layers`` must be true — a dangling layer reference passes
|
||||||
|
``doc.audit()`` silently). ``overwrite`` replaces the *contents* of an
|
||||||
|
existing definition rather than deleting it: existing INSERTs keep
|
||||||
|
pointing at the name and show the new geometry (deleting a referenced
|
||||||
|
block is refused by both engines; replacing contents works on both).
|
||||||
"""
|
"""
|
||||||
from ezdxf.enums import TextEntityAlignment
|
from ezdxf.enums import TextEntityAlignment
|
||||||
|
|
||||||
from backends.block_specs import (
|
from backends.block_specs import (
|
||||||
solid_vertices,
|
solid_vertices,
|
||||||
validate_attdef_specs,
|
validate_attdef_specs,
|
||||||
|
validate_base_point,
|
||||||
validate_entity_specs,
|
validate_entity_specs,
|
||||||
)
|
)
|
||||||
from security import sanitize_symbol_name
|
from security import sanitize_symbol_name
|
||||||
@@ -5002,6 +5008,8 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
clean_name = sanitize_symbol_name(name, kind="block")
|
clean_name = sanitize_symbol_name(name, kind="block")
|
||||||
ents = validate_entity_specs(entities)
|
ents = validate_entity_specs(entities)
|
||||||
atts = validate_attdef_specs(attdefs or [])
|
atts = validate_attdef_specs(attdefs or [])
|
||||||
|
base = (*validate_base_point(base_x, base_y), 0.0)
|
||||||
|
wanted_layers = sorted({spec["layer"] for spec in ents if spec["layer"] != "0"})
|
||||||
align_map = {
|
align_map = {
|
||||||
"left": TextEntityAlignment.LEFT,
|
"left": TextEntityAlignment.LEFT,
|
||||||
"center": TextEntityAlignment.CENTER,
|
"center": TextEntityAlignment.CENTER,
|
||||||
@@ -5018,7 +5026,16 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
"pass overwrite=true to replace its contents"
|
"pass overwrite=true to replace its contents"
|
||||||
)
|
)
|
||||||
replaced = existing is not None
|
replaced = existing is not None
|
||||||
base = (float(base_x), float(base_y), 0.0)
|
missing_layers = [layer for layer in wanted_layers if layer not in doc.layers]
|
||||||
|
if missing_layers and not create_layers:
|
||||||
|
raise ValueError(
|
||||||
|
"block_define: layer(s) "
|
||||||
|
+ ", ".join(repr(layer) for layer in missing_layers)
|
||||||
|
+ " do not exist in the drawing; create them with layer_create "
|
||||||
|
"or pass create_layers=true"
|
||||||
|
)
|
||||||
|
for layer in missing_layers:
|
||||||
|
doc.layers.add(layer)
|
||||||
if existing is None:
|
if existing is None:
|
||||||
blk = doc.blocks.new(name=clean_name, base_point=base)
|
blk = doc.blocks.new(name=clean_name, base_point=base)
|
||||||
else:
|
else:
|
||||||
@@ -5085,6 +5102,7 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
"entity_count": len(ents),
|
"entity_count": len(ents),
|
||||||
"attdef_count": len(atts),
|
"attdef_count": len(atts),
|
||||||
"replaced": replaced,
|
"replaced": replaced,
|
||||||
|
"layers_created": missing_layers,
|
||||||
"backend": "ezdxf",
|
"backend": "ezdxf",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3160,19 +3160,30 @@ async def block_define(
|
|||||||
overwrite: Annotated[
|
overwrite: Annotated[
|
||||||
bool, "Replace the contents of an existing definition of this name"
|
bool, "Replace the contents of an existing definition of this name"
|
||||||
] = False,
|
] = False,
|
||||||
|
create_layers: Annotated[
|
||||||
|
bool, "Create primitive layers that do not exist yet (default: refuse and name them)"
|
||||||
|
] = False,
|
||||||
ctx: Context = None,
|
ctx: Context = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Create a block definition with attribute definitions from typed specs.
|
"""Create a block definition with attribute definitions from typed specs.
|
||||||
|
|
||||||
The whole request is validated before anything is written: one malformed
|
The whole request is validated before anything is written: one malformed
|
||||||
entry refuses the call and leaves no definition behind. `overwrite=true`
|
entry refuses the call and leaves no definition behind. Refusals: a
|
||||||
replaces the *contents* of an existing block so its INSERTs keep pointing
|
malformed primitive or ATTDEF (names `entities[i]`/`attdefs[i]` and the
|
||||||
at the name and show the new geometry; `replaced` reports it. Geometry
|
key), a non-finite base point (names `base_x`/`base_y`), a name clash
|
||||||
inside the block is ByBlock so the INSERT's layer supplies colour and
|
without `overwrite`, an anonymous `*` name, and a primitive `layer` that
|
||||||
lineweight. Insert with `block_insert(attributes={TAG: value})`.
|
is not in the drawing's layer table (names the missing layers) unless
|
||||||
|
`create_layers=true`, which creates them and lists them in
|
||||||
|
`layers_created`. `overwrite=true` replaces the *contents* of an existing
|
||||||
|
block so its INSERTs keep pointing at the name and show the new geometry;
|
||||||
|
`replaced` reports it. Geometry inside the block is ByBlock so the
|
||||||
|
INSERT's layer supplies colour and lineweight. Insert with
|
||||||
|
`block_insert(attributes={TAG: value})`.
|
||||||
"""
|
"""
|
||||||
await ctx.info(f"Defining block '{name}' from {len(entities)} primitives")
|
await ctx.info(f"Defining block '{name}' from {len(entities)} primitives")
|
||||||
return await _backend(ctx).block_define(name, entities, attdefs, base_x, base_y, overwrite)
|
return await _backend(ctx).block_define(
|
||||||
|
name, entities, attdefs, base_x, base_y, overwrite, create_layers
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@cad_tool(summary="Find every place a given block is inserted.", cost="read")
|
@cad_tool(summary="Find every place a given block is inserted.", cost="read")
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ async def test_defines_geometry_and_attdefs(backend):
|
|||||||
"entity_count": 7,
|
"entity_count": 7,
|
||||||
"attdef_count": 2,
|
"attdef_count": 2,
|
||||||
"replaced": False,
|
"replaced": False,
|
||||||
|
"layers_created": [],
|
||||||
"backend": "ezdxf",
|
"backend": "ezdxf",
|
||||||
}
|
}
|
||||||
blk = backend._doc.blocks.get("PID_VALVE_TEST")
|
blk = backend._doc.blocks.get("PID_VALVE_TEST")
|
||||||
|
|||||||
@@ -1589,3 +1589,72 @@ async def test_com_set_attributes_refuses_before_touching_the_reference(com):
|
|||||||
"updated_tags": ["TAG"],
|
"updated_tags": ["TAG"],
|
||||||
}
|
}
|
||||||
assert attr.TextString == "2.5"
|
assert attr.TextString == "2.5"
|
||||||
|
|
||||||
|
|
||||||
|
# ── Task 4: block_define validates the base point and the layer table ────────
|
||||||
|
|
||||||
|
CIRC = [{"type": "circle", "cx": 0, "cy": 0, "r": 1}]
|
||||||
|
ON_LAYER = [{"type": "line", "x1": 0, "y1": 0, "x2": 1, "y2": 1, "layer": "NOT_A_LAYER"}]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"kwargs, key",
|
||||||
|
[
|
||||||
|
({"base_x": float("nan")}, "base_x"),
|
||||||
|
({"base_y": float("inf")}, "base_y"),
|
||||||
|
({"base_x": "3"}, "base_x"),
|
||||||
|
({"base_y": True}, "base_y"),
|
||||||
|
({"base_x": 10**400}, "base_x"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
async def test_define_refuses_a_bad_base_point_before_writing(backend, kwargs, key):
|
||||||
|
with pytest.raises(TypeError, match=f"'{key}'"):
|
||||||
|
await backend.block_define("PID_BASE", CIRC, **kwargs)
|
||||||
|
assert "PID_BASE" not in backend._doc.blocks
|
||||||
|
|
||||||
|
|
||||||
|
async def test_define_refuses_a_missing_layer_and_names_it(backend):
|
||||||
|
with pytest.raises(ValueError, match="'NOT_A_LAYER' do not exist") as exc:
|
||||||
|
await backend.block_define("PID_LYR", ON_LAYER)
|
||||||
|
assert "create_layers=true" in str(exc.value)
|
||||||
|
assert "PID_LYR" not in backend._doc.blocks
|
||||||
|
assert "NOT_A_LAYER" not in backend._doc.layers
|
||||||
|
|
||||||
|
|
||||||
|
async def test_define_creates_the_layer_on_request_and_reports_it(backend):
|
||||||
|
result = await backend.block_define("PID_LYR", ON_LAYER, create_layers=True)
|
||||||
|
assert result["layers_created"] == ["NOT_A_LAYER"] and result["ok"] is True
|
||||||
|
assert "NOT_A_LAYER" in backend._doc.layers
|
||||||
|
member = next(iter(backend._doc.blocks.get("PID_LYR")))
|
||||||
|
assert member.dxf.layer == "NOT_A_LAYER"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_define_on_an_existing_layer_reports_nothing_created(backend):
|
||||||
|
await backend.layer_create("PID-EQUIP")
|
||||||
|
spec = [{**ON_LAYER[0], "layer": "PID-EQUIP"}]
|
||||||
|
result = await backend.block_define("PID_OK", spec)
|
||||||
|
assert result["layers_created"] == []
|
||||||
|
result = await backend.block_define("PID_ZERO", CIRC)
|
||||||
|
assert result["layers_created"] == [], "layer 0 always exists"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_com_define_probes_layers_before_blocks_add(com):
|
||||||
|
backend, document, space = com
|
||||||
|
with pytest.raises(ValueError, match="'NOT_A_LAYER' do not exist"):
|
||||||
|
await backend.block_define("PID_LYR", ON_LAYER)
|
||||||
|
assert "PID_LYR" not in document.Blocks.blocks and document.Layers.added == []
|
||||||
|
|
||||||
|
result = await backend.block_define("PID_LYR", ON_LAYER, create_layers=True)
|
||||||
|
assert result["layers_created"] == ["NOT_A_LAYER"] and result["backend"] == "com"
|
||||||
|
assert document.Layers.added == ["NOT_A_LAYER"]
|
||||||
|
assert [name for name, _ in document.Blocks.blocks["PID_LYR"].calls] == ["AddLine"]
|
||||||
|
|
||||||
|
|
||||||
|
async def test_com_define_refuses_a_bad_base_point_before_blocks_add(com):
|
||||||
|
backend, document, space = com
|
||||||
|
with pytest.raises(TypeError, match="'base_y'"):
|
||||||
|
await backend.block_define("PID_BASE", CIRC, base_y=float("nan"))
|
||||||
|
assert "PID_BASE" not in document.Blocks.blocks
|
||||||
|
result = await backend.block_define("PID_BASE", CIRC, base_x=1.5, base_y=2.5)
|
||||||
|
assert document.Blocks.blocks["PID_BASE"].base_point == (1.5, 2.5, 0.0)
|
||||||
|
assert result["layers_created"] == []
|
||||||
|
|||||||
Reference in New Issue
Block a user