anatoly/autocad-mcp-pro
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(task-21): one registry entry per file — save/save_as refuse a path another document holds; resolver refuses a shared path instead of guessing

Browse Source 
drawing_save / drawing_save_as onto a file that another open entry holds
used to succeed: the registry then carried two entries for one path, the
resolver's exact-key shortcut picked the stale entry on document_close, and
the file ended up holding the other document's bytes while both rows still
reported saved: True. Both save paths now refuse before doc.saveas (as
AutoCAD refuses SAVEAS onto an open drawing), the _doc_path setter raises
rather than let two entries share a path, and _resolve_document_key refuses
a shared file on every route — exact key, path and basename — naming the
breach instead of advising "pass the full path" into the unchecked shortcut.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
Umut CAN
2026-09-16 21:36:44 +03:00
co-authored by Claude Opus 5
parent 174fb8010c
commit aeab950d2d
2 changed files with 152 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backends/ezdxf_backend.py
+80 -6
View File
@@ -1085,20 +1085,62 @@ class EzdxfBackend(AutoCADBackend):
# `drawing_save` / `drawing_save_as` assign here. A document keyed
# `untitled-N` that is saved to a file becomes keyed by that file, so
# `document_list` shows one identity per document and `document_close`
# can be told the file name. Two entries may end up sharing a path
# (save_as onto a file that is open in another entry); the resolver
# then reports the ambiguity instead of guessing.
state = self._state
state.path = value
# can be told the file name. The registry holds one entry per file:
# `_refuse_path_held_elsewhere` turns a save onto a file that another
# entry holds away *before* the bytes are written (AutoCAD refuses
# SAVEAS onto an open drawing for the same reason), and this setter
# raises rather than let two entries share a path, because the
# resolver could then only guess which one a caller means.
if self._active_key is None or value is None:
self._state.path = value
return
new_key = os.path.abspath(value)
if new_key != self._active_key and new_key not in self._docs:
holder = self._path_holder(new_key, exclude=self._active_key)
if holder is not None:
raise RuntimeError(
f"EzdxfBackend: registry entry {holder!r} already holds {new_key!r}; "
"a file is held by one open document"
)
state = self._state
state.path = value
if new_key != self._active_key:
del self._docs[self._active_key]
state.key = new_key
self._docs[new_key] = state
self._active_key = new_key
def _path_holder(self, abs_path: str, *, exclude: str | None = None) -> str | None:
"""Key of the entry (other than ``exclude``) that holds ``abs_path``, or None.
Compared with ``os.path.normcase`` so that on Windows two spellings of
one file count as the same file; on POSIX that is plain equality.
"""
wanted = os.path.normcase(abs_path)
for key, state in self._docs.items():
if key == exclude:
continue
candidates = [key]
if state.path:
candidates.append(os.path.abspath(state.path))
if any(os.path.normcase(c) == wanted for c in candidates):
return key
return None
def _refuse_path_held_elsewhere(self, op: str, path: str) -> None:
"""Refuse ``op`` onto a file that another open document holds.
Runs before ``doc.saveas`` so a refusal leaves the disk, the registry
and the active document exactly as they were.
"""
holder = self._path_holder(os.path.abspath(path), exclude=self._active_key)
if holder is None:
return
label = self._docs[holder].label
raise ValueError(
f"{op}: {label!r} is already open as document {holder!r}; a file is held by "
"one open document at a time - document_close it first, or save to another path"
)
@property
def name(self) -> str:
return "ezdxf"
@@ -1269,6 +1311,11 @@ class EzdxfBackend(AutoCADBackend):
if not wanted:
raise ValueError("name_or_path must not be empty")
if wanted in self._docs:
# An exact key hit is still refused when a second entry holds the
# same file: the registry is meant to keep one entry per file (the
# save paths enforce it), and if that is ever broken a lookup must
# say so rather than hand back whichever entry the key names.
self._refuse_shared_path(wanted, self._docs[wanted])
return wanted
try:
as_path = os.path.abspath(wanted)
@@ -1280,16 +1327,41 @@ class EzdxfBackend(AutoCADBackend):
if state.path and os.path.abspath(state.path) == as_path
]
if len(by_path) == 1:
self._refuse_shared_path(wanted, self._docs[by_path[0]])
return by_path[0]
if len(by_path) > 1:
raise ValueError(
f"document {wanted!r} is ambiguous: entries {by_path} share the file "
f"{as_path!r}; the registry must hold one entry per file"
)
by_name = [
key for key, state in self._docs.items() if state.label.lower() == wanted.lower()
]
if len(by_name) == 1:
self._refuse_shared_path(wanted, self._docs[by_name[0]])
return by_name[0]
if len(by_name) > 1:
# Same basename in two directories is a real ambiguity the full
# path settles; two entries on one file is the invariant breach,
# and "pass the full path" would send the caller to the exact-key
# shortcut that this resolver refuses for the same reason.
for key in by_name:
self._refuse_shared_path(wanted, self._docs[key])
raise ValueError(f"document {wanted!r} is ambiguous: {by_name}; pass the full path")
raise ValueError(f"no open document named {wanted!r}; open documents: {list(self._docs)}")
def _refuse_shared_path(self, wanted: str, state: _DocState) -> None:
"""Raise if any other entry holds the file ``state`` holds."""
if not state.path:
return
abs_path = os.path.abspath(state.path)
other = self._path_holder(abs_path, exclude=state.key)
if other is not None:
raise ValueError(
f"document {wanted!r} is ambiguous: entries {sorted([state.key, other])} "
f"share the file {abs_path!r}; the registry must hold one entry per file"
)
def _evict_quarantined_document(self) -> None:
"""Drop the quarantined entry before a fresh document is registered.
@@ -1759,6 +1831,7 @@ class EzdxfBackend(AutoCADBackend):
doc = self._require_doc()
if not save_path:
raise RuntimeError("No path specified and no current file path.")
self._refuse_path_held_elsewhere("drawing_save", save_path)
doc.saveas(save_path)
self._doc_path = save_path
self._dirty = False
@@ -1774,6 +1847,7 @@ class EzdxfBackend(AutoCADBackend):
def _sync():
doc = self._require_doc()
self._refuse_path_held_elsewhere("drawing_save_as", path)
doc.saveas(path)
self._doc_path = path
self._dirty = False
tests/test_documents.py
+72
View File
@@ -107,6 +107,78 @@ async def test_saving_rekeys_an_untitled_document_to_its_file(backend, tmp_path)
assert backend._active_key == os.path.abspath(str(path))
async def test_save_refuses_a_path_that_another_open_document_holds(backend, tmp_path):
"""One entry per file: SAVEAS onto an open drawing is refused, as AutoCAD does.
Without the refusal the registry held two entries for one path, the
resolver's exact-key shortcut picked the stale one on close, and the file
ended up holding the *other* document's bytes while ``saved`` stayed True.
"""
import ezdxf
path = tmp_path / "gear.dxf"
await backend.drawing_save(str(path))
key = os.path.abspath(str(path))
await backend.entity_create_line(0, 0, 1, 1) # A: dirty, on disk empty
second = (await backend.drawing_new())["document"]
await backend.entity_create_circle(0, 0, 5)
before = path.read_bytes()
for call in (backend.drawing_save_as, backend.drawing_save):
with pytest.raises(ValueError, match="gear.dxf.*already open.*document_close"):
await call(str(path))
assert path.read_bytes() == before, "a refusal writes nothing"
rows = {r["key"]: r for r in await backend.document_list()}
assert set(rows) == {key, second}, "a refusal re-keys nothing"
assert rows[second]["path"] is None and rows[second]["saved"] is False
assert rows[key]["saved"] is False
# The rejected document is untouched and can still be saved elsewhere.
other = tmp_path / "other.dxf"
await backend.drawing_save_as(str(other))
assert [e.dxftype() for e in ezdxf.readfile(str(other)).modelspace()] == ["CIRCLE"]
# The by-name lookup is unambiguous and closes the entry that owns the file.
await backend.entity_create_line(2, 2, 3, 3)
closed = await backend.document_close("gear.dxf", save=True)
assert closed["closed"] == key and closed["saved"] is True
assert [e.dxftype() for e in ezdxf.readfile(str(path)).modelspace()] == ["LINE"]
async def test_save_to_the_document_own_path_is_not_a_collision(backend, tmp_path):
path = tmp_path / "self.dxf"
await backend.drawing_save(str(path))
await backend.drawing_new()
await backend.document_activate("self.dxf")
await backend.entity_create_circle(0, 0, 1)
assert (await backend.drawing_save())["ok"] is True
await backend.entity_create_circle(0, 0, 2)
assert (await backend.drawing_save(str(path)))["ok"] is True
await backend.entity_create_circle(0, 0, 3)
# A spelling that differs from the key still names the same file.
spelled = str(tmp_path / "sub" / ".." / "self.dxf")
assert (await backend.drawing_save_as(spelled))["ok"] is True
assert len(await backend.document_list()) == 2
assert backend._active_key == os.path.abspath(str(path))
async def test_resolver_refuses_a_shared_path_even_on_an_exact_key_hit(backend, tmp_path):
"""The save refusal keeps the registry one-entry-per-file; if that invariant
is ever broken anyway, every lookup of the pair refuses instead of guessing."""
path = tmp_path / "dup.dxf"
await backend.drawing_save(str(path))
key = os.path.abspath(str(path))
second = (await backend.drawing_new())["document"]
with pytest.raises(RuntimeError, match="already holds"):
backend._doc_path = str(path) # the setter enforces the invariant
backend._docs[second].path = str(path) # corrupt the registry behind its back
for wanted in (key, second, "dup.dxf", str(path)):
with pytest.raises(ValueError, match="share the file"):
backend._resolve_document_key(wanted)
with pytest.raises(ValueError, match="share the file"):
await backend.document_close(key, save=True)
assert len(await backend.document_list()) == 2, "a refusal closes nothing"
async def test_activate_by_basename_full_path_or_key(backend, tmp_path):
path = tmp_path / "gear.dxf"
await backend.drawing_save(str(path))