fix(accounts): anchor duplicate-account detection to a real Open directive (libra-#48)
The existence check matched 'open <name>' anywhere in the chart source,
so a prior account's description metadata or a comment mentioning the
name produced a false 409, while a real directive with an inline comment
and no space ('open X;legacy') was missed → a duplicate Open was appended
and bean-check then rejected the file, breaking every later /api/source
write. Extract the check into a pure _open_directive_exists() anchored to
'^YYYY-MM-DD open <name>' with an account-boundary negative-lookahead, and
unit-test both failure directions plus prefix/child non-matches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
89f0f8ac3a
commit
0ea96cd384
2 changed files with 77 additions and 9 deletions
|
|
@ -60,6 +60,30 @@ def _escape_beancount_string(value: str) -> str:
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _open_directive_exists(source: str, account_name: str) -> bool:
|
||||||
|
"""Return True if `source` already contains an Open directive for exactly
|
||||||
|
`account_name`.
|
||||||
|
|
||||||
|
Anchored to a real `YYYY-MM-DD open <account>` directive line (re.MULTILINE)
|
||||||
|
so the account name can't match text inside another account's description
|
||||||
|
metadata or a comment (false positive → spurious 409). The trailing
|
||||||
|
negative-lookahead `(?![\\w:-])` requires the next char not to be an
|
||||||
|
account-continuation char, so:
|
||||||
|
- a prefix (Expenses:Gas) does not match a longer sibling
|
||||||
|
(Expenses:GasStation / Expenses:Gas:Vehicle), and
|
||||||
|
- a real directive with an inline comment and no space
|
||||||
|
(`open Expenses:Gas;legacy`) is still detected (`;` ends the name),
|
||||||
|
which the previous `(?:\\s|$)` boundary missed → duplicate write.
|
||||||
|
"""
|
||||||
|
return bool(
|
||||||
|
re.search(
|
||||||
|
rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])",
|
||||||
|
source,
|
||||||
|
re.MULTILINE,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class FavaClient:
|
class FavaClient:
|
||||||
"""
|
"""
|
||||||
Async client for Fava REST API.
|
Async client for Fava REST API.
|
||||||
|
|
@ -1644,15 +1668,9 @@ class FavaClient:
|
||||||
source = source_data["source"]
|
source = source_data["source"]
|
||||||
|
|
||||||
# Step 2: Check if account already exists (may have been
|
# Step 2: Check if account already exists (may have been
|
||||||
# created by a concurrent request). Anchor on the Open
|
# created by a concurrent request). See
|
||||||
# directive and require the account to be followed by
|
# _open_directive_exists for the anchoring rationale.
|
||||||
# whitespace/end-of-line so a prefix (Expenses:Gas) does
|
if _open_directive_exists(source, account_name):
|
||||||
# not match a longer sibling (Expenses:GasStation).
|
|
||||||
if re.search(
|
|
||||||
rf"open {re.escape(account_name)}(?:\s|$)",
|
|
||||||
source,
|
|
||||||
re.MULTILINE,
|
|
||||||
):
|
|
||||||
logger.info(f"Account {account_name} already exists in {target_file}")
|
logger.info(f"Account {account_name} already exists in {target_file}")
|
||||||
return {
|
return {
|
||||||
"data": sha256sum,
|
"data": sha256sum,
|
||||||
|
|
|
||||||
|
|
@ -31,9 +31,59 @@ bf = _module("beancount_format")
|
||||||
au = _module("account_utils")
|
au = _module("account_utils")
|
||||||
val = _module("core.validation")
|
val = _module("core.validation")
|
||||||
mdl = _module("models")
|
mdl = _module("models")
|
||||||
|
fc = _module("fava_client")
|
||||||
AccountType = mdl.AccountType
|
AccountType = mdl.AccountType
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# fava_client._open_directive_exists — duplicate-account detection
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_matches_real_directive():
|
||||||
|
src = "2020-01-01 open Expenses:Vehicle:Gas\n"
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_matches_currency_constrained_and_metadata():
|
||||||
|
src = (
|
||||||
|
"2020-01-01 open Expenses:Vehicle:Gas EUR, SATS\n"
|
||||||
|
' added_by: "abc"\n'
|
||||||
|
)
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_matches_inline_comment_without_space():
|
||||||
|
# Valid Beancount: the account token ends at ';'. The old (?:\\s|$) boundary
|
||||||
|
# missed this → duplicate Open written → bean-check breaks.
|
||||||
|
src = "2020-01-01 open Expenses:Vehicle:Gas;legacy-import\n"
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_ignores_name_inside_description():
|
||||||
|
# The name appears only inside another account's description metadata.
|
||||||
|
src = (
|
||||||
|
"2020-01-01 open Expenses:Notes\n"
|
||||||
|
' description: "remember to open Expenses:Vehicle:Gas next month"\n'
|
||||||
|
)
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_ignores_comment_line():
|
||||||
|
src = "; TODO: open Expenses:Vehicle:Gas eventually\n"
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_does_not_match_longer_sibling():
|
||||||
|
src = "2020-01-01 open Expenses:Vehicle:GasStation\n"
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_open_directive_exists_does_not_match_deeper_child():
|
||||||
|
src = "2020-01-01 open Expenses:Vehicle:Gas:Premium\n"
|
||||||
|
assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# beancount_format.sanitize_link
|
# beancount_format.sanitize_link
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue