fix(task-4): block_define dedupes requested layers case-insensitively so one layer in two spellings is created and reported once
DXF and ActiveX layer tables are case-insensitive, but block_define collected the primitives' layers with a case-sensitive set. A request naming 'Alpha' and 'ALPHA' with create_layers=True made ezdxf add the first, then raise DXFTableEntryError on the second after the write — layer left behind, no block, surfaced as an internal error instead of a named refusal. COM ran Layers.Add twice, got the existing layer back the second time, and reported two layers created for one that appeared; the two engines diverged and the refusal message without create_layers named the same layer twice. referenced_layers() in backends/block_specs.py keys the set on the folded name and keeps the first spelling; both engines consume it. _FakeLayers in the hardening tests now matches Item/Add case-insensitively and returns the existing record on a duplicate Add, as the ActiveX table does, so the COM path is pinned. 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
375c9f4c70
commit
b464bcdaf5
@@ -219,6 +219,24 @@ def validate_base_point(base_x, base_y) -> tuple[float, float]:
|
||||
)
|
||||
|
||||
|
||||
def referenced_layers(entities: list[dict]) -> list[str]:
|
||||
"""The distinct layers ``block_define``'s primitives reference, other than ``0``.
|
||||
|
||||
DXF and ActiveX layer tables are case-insensitive, so the set is keyed on
|
||||
the folded name and keeps the first spelling seen: a request naming
|
||||
``Alpha`` and ``ALPHA`` wants *one* layer. Deduping case-sensitively let
|
||||
ezdxf write ``ALPHA`` and then raise ``DXFTableEntryError`` on ``Alpha``
|
||||
mid-call, and let COM report two layers created for one ``Layers.Add``
|
||||
that took effect.
|
||||
"""
|
||||
seen: dict[str, str] = {}
|
||||
for spec in entities:
|
||||
layer = spec["layer"]
|
||||
if layer != "0" and layer.lower() not in seen:
|
||||
seen[layer.lower()] = layer
|
||||
return sorted(seen.values())
|
||||
|
||||
|
||||
def validate_attribute_values(attributes, where: str = "attributes") -> dict[str, str]:
|
||||
"""``{TAG: value}`` for ``block_insert`` / ``block_set_attributes`` as the
|
||||
strings that will be written, refused before any write.
|
||||
|
||||
@@ -3895,6 +3895,7 @@ class ComBackend(AutoCADBackend):
|
||||
a fake ActiveX surface; executed live by ``scripts/smoke_pid_com.py``.
|
||||
"""
|
||||
from backends.block_specs import (
|
||||
referenced_layers,
|
||||
solid_vertices,
|
||||
validate_attdef_specs,
|
||||
validate_base_point,
|
||||
@@ -3910,7 +3911,7 @@ class ComBackend(AutoCADBackend):
|
||||
ents = validate_entity_specs(entities)
|
||||
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"})
|
||||
wanted_layers = referenced_layers(ents)
|
||||
# acAlignmentLeft / Center / Right / MiddleCenter
|
||||
align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10}
|
||||
|
||||
@@ -3932,6 +3933,7 @@ class ComBackend(AutoCADBackend):
|
||||
)
|
||||
replaced = existing is not None
|
||||
missing_layers = []
|
||||
# ``Layers.Item`` matches case-insensitively, like the table itself.
|
||||
for layer in wanted_layers:
|
||||
try:
|
||||
doc.Layers.Item(layer)
|
||||
|
||||
@@ -4993,6 +4993,7 @@ class EzdxfBackend(AutoCADBackend):
|
||||
from ezdxf.enums import TextEntityAlignment
|
||||
|
||||
from backends.block_specs import (
|
||||
referenced_layers,
|
||||
solid_vertices,
|
||||
validate_attdef_specs,
|
||||
validate_base_point,
|
||||
@@ -5009,7 +5010,7 @@ class EzdxfBackend(AutoCADBackend):
|
||||
ents = validate_entity_specs(entities)
|
||||
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"})
|
||||
wanted_layers = referenced_layers(ents)
|
||||
align_map = {
|
||||
"left": TextEntityAlignment.LEFT,
|
||||
"center": TextEntityAlignment.CENTER,
|
||||
@@ -5026,6 +5027,7 @@ class EzdxfBackend(AutoCADBackend):
|
||||
"pass overwrite=true to replace its contents"
|
||||
)
|
||||
replaced = existing is not None
|
||||
# ``in doc.layers`` is case-insensitive, like the table itself.
|
||||
missing_layers = [layer for layer in wanted_layers if layer not in doc.layers]
|
||||
if missing_layers and not create_layers:
|
||||
raise ValueError(
|
||||
|
||||
@@ -119,19 +119,25 @@ class _FakeBlocks:
|
||||
|
||||
|
||||
class _FakeLayers:
|
||||
"""The ActiveX layer table: ``Item`` and ``Add`` match names case-insensitively,
|
||||
and ``Add`` of an existing name hands the existing record back instead of
|
||||
creating a second one — so ``added`` records only the layers that really
|
||||
appeared."""
|
||||
|
||||
def __init__(self, *names: str):
|
||||
self.names = {"0", *names}
|
||||
self.names: dict[str, str] = {n.lower(): n for n in ("0", *names)}
|
||||
self.added: list[str] = []
|
||||
|
||||
def Item(self, name):
|
||||
if name not in self.names:
|
||||
if name.lower() not in self.names:
|
||||
raise RuntimeError(f"no layer {name}")
|
||||
return types.SimpleNamespace(Name=name)
|
||||
return types.SimpleNamespace(Name=self.names[name.lower()])
|
||||
|
||||
def Add(self, name):
|
||||
self.names.add(name)
|
||||
self.added.append(name)
|
||||
return types.SimpleNamespace(Name=name)
|
||||
if name.lower() not in self.names:
|
||||
self.names[name.lower()] = name
|
||||
self.added.append(name)
|
||||
return types.SimpleNamespace(Name=self.names[name.lower()])
|
||||
|
||||
|
||||
class _FakeSpace:
|
||||
@@ -1650,6 +1656,59 @@ async def test_com_define_probes_layers_before_blocks_add(com):
|
||||
assert [name for name, _ in document.Blocks.blocks["PID_LYR"].calls] == ["AddLine"]
|
||||
|
||||
|
||||
TWO_SPELLINGS = [{**ON_LAYER[0], "layer": "Alpha"}, {**ON_LAYER[0], "layer": "ALPHA"}]
|
||||
|
||||
|
||||
async def test_define_names_a_case_folded_missing_layer_once(backend):
|
||||
"""DXF layer names are case-insensitive: 'Alpha' and 'ALPHA' are one missing
|
||||
layer, refused once, under the first spelling, before anything is written."""
|
||||
with pytest.raises(ValueError, match=r"layer\(s\) 'Alpha' do not exist"):
|
||||
await backend.block_define("PID_CASE", TWO_SPELLINGS)
|
||||
assert "PID_CASE" not in backend._doc.blocks
|
||||
assert "Alpha" not in backend._doc.layers
|
||||
|
||||
|
||||
async def test_define_creates_a_case_folded_layer_once(backend):
|
||||
"""Without the case fold the second ``layers.add`` raised DXFTableEntryError
|
||||
after the first one had written — the layer stayed, the block never came."""
|
||||
result = await backend.block_define("PID_CASE", TWO_SPELLINGS, create_layers=True)
|
||||
assert result["layers_created"] == ["Alpha"] and result["entity_count"] == 2
|
||||
assert "PID_CASE" in backend._doc.blocks
|
||||
spellings = [
|
||||
layer.dxf.name for layer in backend._doc.layers if layer.dxf.name.lower() == "alpha"
|
||||
]
|
||||
assert spellings == ["Alpha"]
|
||||
members = [member.dxf.layer for member in backend._doc.blocks.get("PID_CASE")]
|
||||
assert members == ["Alpha", "ALPHA"], "each primitive keeps the spelling it was given"
|
||||
|
||||
|
||||
async def test_define_treats_an_existing_layer_case_insensitively(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"] == []
|
||||
|
||||
|
||||
async def test_com_define_creates_a_case_folded_layer_once(com):
|
||||
backend, document, space = com
|
||||
with pytest.raises(ValueError, match=r"layer\(s\) 'Alpha' do not exist"):
|
||||
await backend.block_define("PID_CASE", TWO_SPELLINGS)
|
||||
assert "PID_CASE" not in document.Blocks.blocks and document.Layers.added == []
|
||||
|
||||
result = await backend.block_define("PID_CASE", TWO_SPELLINGS, create_layers=True)
|
||||
assert result["layers_created"] == ["Alpha"] and result["entity_count"] == 2
|
||||
assert document.Layers.added == ["Alpha"]
|
||||
assert [name for name, _ in document.Blocks.blocks["PID_CASE"].calls] == ["AddLine", "AddLine"]
|
||||
|
||||
|
||||
async def test_com_define_treats_an_existing_layer_case_insensitively(com):
|
||||
backend, document, space = com
|
||||
document.Layers.Add("PID-EQUIP")
|
||||
spec = [{**ON_LAYER[0], "layer": "pid-equip"}]
|
||||
result = await backend.block_define("PID_OK", spec)
|
||||
assert result["layers_created"] == [] and document.Layers.added == ["PID-EQUIP"]
|
||||
|
||||
|
||||
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'"):
|
||||
|
||||
Reference in New Issue
Block a user