fix(master-data): drop file-hash fast-path so sync always reconciles DB
The hash fast-path compared the JSON file hash to a stored value and exited early when unchanged. But it only tracked the *file*, not the *table*: if rows went missing from the DB while the JSON stayed the same, the sync skipped entirely and never restored them. Always run the full diff now. It costs one SELECT plus an in-memory compare — ~0.1s for 13k E-Taxes Item Group rows — so there's no reason to skip it. The diff is driven by DB state, making the sync self-healing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b9009a0377
commit
cc3621dc8b
|
|
@ -1,16 +1,20 @@
|
||||||
"""Hash-diff sync for master data shipped as JSON.
|
"""Diff-based sync for master data shipped as JSON.
|
||||||
|
|
||||||
The legacy approach (fixtures dir) re-imports every record on every `bench
|
The legacy approach (fixtures dir) re-imports every record on every `bench
|
||||||
migrate` via DELETE+INSERT. For static reference data with thousands of rows
|
migrate` via DELETE+INSERT. For static reference data with thousands of rows
|
||||||
that's a multi-minute hit. This module compares the file hash first (fast-path
|
that's a multi-minute hit. This module instead reconciles the DB against the
|
||||||
exit when unchanged) and otherwise applies only the diff: insert new rows,
|
JSON on every migrate: it reads the current rows in one query and applies only
|
||||||
update changed rows, delete rows missing from the file (or disable / skip when
|
the delta — insert new rows, update changed rows, delete rows missing from the
|
||||||
they are referenced by existing documents).
|
file (or disable / skip when they are referenced by existing documents).
|
||||||
|
|
||||||
|
The diff is intentionally driven by the *DB state*, not a stored file hash:
|
||||||
|
if rows go missing from the table (manual delete, restore from an old dump),
|
||||||
|
the next migrate restores them. A full diff costs one SELECT plus an in-memory
|
||||||
|
comparison — sub-second even for ~13k rows.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import hashlib
|
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
from collections.abc import Iterable
|
from collections.abc import Iterable
|
||||||
|
|
@ -20,18 +24,6 @@ import frappe
|
||||||
LOG_PREFIX = "invoice_az"
|
LOG_PREFIX = "invoice_az"
|
||||||
|
|
||||||
|
|
||||||
def _file_hash(path: str) -> str:
|
|
||||||
h = hashlib.md5(usedforsecurity=False)
|
|
||||||
with open(path, "rb") as f:
|
|
||||||
for chunk in iter(lambda: f.read(8192), b""):
|
|
||||||
h.update(chunk)
|
|
||||||
return h.hexdigest()
|
|
||||||
|
|
||||||
|
|
||||||
def _global_key(doctype: str) -> str:
|
|
||||||
return f"{LOG_PREFIX}:master_data_hash:{frappe.scrub(doctype)}"
|
|
||||||
|
|
||||||
|
|
||||||
def _load_records(path: str, key_field: str) -> dict[str, dict]:
|
def _load_records(path: str, key_field: str) -> dict[str, dict]:
|
||||||
with open(path, encoding="utf-8") as f:
|
with open(path, encoding="utf-8") as f:
|
||||||
raw = json.load(f)
|
raw = json.load(f)
|
||||||
|
|
@ -80,21 +72,30 @@ def _try_remove(doctype: str, name: str, has_disabled: bool) -> str:
|
||||||
return "skipped"
|
return "skipped"
|
||||||
|
|
||||||
|
|
||||||
def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> dict:
|
def sync_master_data(
|
||||||
"""Sync a doctype from a JSON file via per-record hash-diff.
|
doctype: str,
|
||||||
|
json_path: str,
|
||||||
|
key_field: str = "name",
|
||||||
|
*,
|
||||||
|
is_tree: bool = False,
|
||||||
|
manage_deletes: bool = True,
|
||||||
|
) -> dict:
|
||||||
|
"""Reconcile a doctype against a JSON file, applying only the delta.
|
||||||
|
|
||||||
Fast-path: if the file hash matches what was stored on the previous run,
|
Runs a full diff against the current DB state every call — no file-hash
|
||||||
return immediately without touching the DB.
|
fast-path, so rows that go missing from the table are always restored.
|
||||||
|
|
||||||
|
is_tree: doctype is a nested set — insert via db_insert() and rebuild
|
||||||
|
lft/rgt once at the end (avoids per-row tree recomputation and the
|
||||||
|
need to insert parents before children).
|
||||||
|
manage_deletes: when False, rows present in the DB but not in the JSON
|
||||||
|
are left untouched. Use False when the JSON is only a subset of the
|
||||||
|
table.
|
||||||
"""
|
"""
|
||||||
if not os.path.exists(json_path):
|
if not os.path.exists(json_path):
|
||||||
frappe.logger().warning(f"[{LOG_PREFIX}] master data file not found: {json_path}")
|
frappe.logger().warning(f"[{LOG_PREFIX}] master data file not found: {json_path}")
|
||||||
return {"skipped": True}
|
return {"skipped": True}
|
||||||
|
|
||||||
current_hash = _file_hash(json_path)
|
|
||||||
stored_hash = frappe.db.get_global(_global_key(doctype))
|
|
||||||
if stored_hash == current_hash:
|
|
||||||
return {"unchanged": True}
|
|
||||||
|
|
||||||
target = _load_records(json_path, key_field)
|
target = _load_records(json_path, key_field)
|
||||||
if not target:
|
if not target:
|
||||||
return {"empty_file": True}
|
return {"empty_file": True}
|
||||||
|
|
@ -115,11 +116,13 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d
|
||||||
to_insert = [(key, rec) for key, rec in target.items() if key not in existing]
|
to_insert = [(key, rec) for key, rec in target.items() if key not in existing]
|
||||||
to_check = [(key, rec) for key, rec in target.items() if key in existing]
|
to_check = [(key, rec) for key, rec in target.items() if key in existing]
|
||||||
|
|
||||||
# Bulk-insert fast path for the large first-run case (avoid 10k+ ORM .insert()
|
# Bulk-insert path: avoids thousands of ORM .insert() round-trips, and for
|
||||||
# round-trips). Uses db_insert() per doc — skips before_insert/after_insert
|
# tree doctypes also sidesteps per-row nested-set recomputation (lft/rgt are
|
||||||
# hooks, which is appropriate for static master data.
|
# rebuilt once at the end). db_insert() skips before_insert/after_insert
|
||||||
|
# hooks — appropriate for static master data.
|
||||||
BULK_THRESHOLD = 100
|
BULK_THRESHOLD = 100
|
||||||
if len(to_insert) >= BULK_THRESHOLD:
|
use_bulk = is_tree or len(to_insert) >= BULK_THRESHOLD
|
||||||
|
if use_bulk:
|
||||||
for key, rec in to_insert:
|
for key, rec in to_insert:
|
||||||
doc = frappe.new_doc(doctype)
|
doc = frappe.new_doc(doctype)
|
||||||
if key_field == "name":
|
if key_field == "name":
|
||||||
|
|
@ -130,6 +133,10 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d
|
||||||
for field in value_fields:
|
for field in value_fields:
|
||||||
if field in rec:
|
if field in rec:
|
||||||
doc.set(field, rec[field])
|
doc.set(field, rec[field])
|
||||||
|
if is_tree:
|
||||||
|
# placeholder bounds; rebuild_tree() fixes them after all inserts
|
||||||
|
doc.lft = 0
|
||||||
|
doc.rgt = 0
|
||||||
doc.db_insert()
|
doc.db_insert()
|
||||||
inserted += 1
|
inserted += 1
|
||||||
if inserted % 500 == 0:
|
if inserted % 500 == 0:
|
||||||
|
|
@ -158,6 +165,9 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d
|
||||||
frappe.db.set_value(doctype, key, diff, update_modified=False)
|
frappe.db.set_value(doctype, key, diff, update_modified=False)
|
||||||
updated += 1
|
updated += 1
|
||||||
|
|
||||||
|
# Delete rows not present in the JSON. Skipped when the JSON is only a
|
||||||
|
# subset of the table.
|
||||||
|
if manage_deletes:
|
||||||
for key in set(existing.keys()) - set(target.keys()):
|
for key in set(existing.keys()) - set(target.keys()):
|
||||||
outcome = _try_remove(doctype, key, has_disabled)
|
outcome = _try_remove(doctype, key, has_disabled)
|
||||||
if outcome == "deleted":
|
if outcome == "deleted":
|
||||||
|
|
@ -167,7 +177,12 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d
|
||||||
else:
|
else:
|
||||||
skipped += 1
|
skipped += 1
|
||||||
|
|
||||||
frappe.db.set_global(_global_key(doctype), current_hash)
|
# Tree doctypes: recompute lft/rgt once for the whole tree.
|
||||||
|
if is_tree and (inserted or updated or deleted):
|
||||||
|
from frappe.utils.nestedset import rebuild_tree
|
||||||
|
|
||||||
|
rebuild_tree(doctype)
|
||||||
|
|
||||||
frappe.db.commit()
|
frappe.db.commit()
|
||||||
|
|
||||||
summary = {
|
summary = {
|
||||||
|
|
@ -178,6 +193,7 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d
|
||||||
"disabled": disabled,
|
"disabled": disabled,
|
||||||
"skipped": skipped,
|
"skipped": skipped,
|
||||||
}
|
}
|
||||||
|
if inserted or updated or deleted or disabled or skipped:
|
||||||
print(
|
print(
|
||||||
f"[{LOG_PREFIX}] {doctype}: "
|
f"[{LOG_PREFIX}] {doctype}: "
|
||||||
f"+{inserted} ~{updated} -{deleted} (disabled {disabled}, skipped {skipped})"
|
f"+{inserted} ~{updated} -{deleted} (disabled {disabled}, skipped {skipped})"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue