From cc3621dc8bdc925bdaf348c1a839ff1a96e61236 Mon Sep 17 00:00:00 2001 From: Ali <010109ali@gmail.com> Date: Wed, 20 May 2026 16:14:58 +0000 Subject: [PATCH] fix(master-data): drop file-hash fast-path so sync always reconciles DB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- invoice_az/master_data/sync.py | 104 +++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 44 deletions(-) diff --git a/invoice_az/master_data/sync.py b/invoice_az/master_data/sync.py index 37a89ed..5bc8bd9 100644 --- a/invoice_az/master_data/sync.py +++ b/invoice_az/master_data/sync.py @@ -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 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 -exit when unchanged) and otherwise applies only the diff: insert new rows, -update changed rows, delete rows missing from the file (or disable / skip when -they are referenced by existing documents). +that's a multi-minute hit. This module instead reconciles the DB against the +JSON on every migrate: it reads the current rows in one query and applies only +the delta — insert new rows, update changed rows, delete rows missing from the +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 -import hashlib import json import os from collections.abc import Iterable @@ -20,18 +24,6 @@ import frappe 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]: with open(path, encoding="utf-8") as f: raw = json.load(f) @@ -80,21 +72,30 @@ def _try_remove(doctype: str, name: str, has_disabled: bool) -> str: return "skipped" -def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> dict: - """Sync a doctype from a JSON file via per-record hash-diff. +def sync_master_data( + 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, - return immediately without touching the DB. + Runs a full diff against the current DB state every call — no file-hash + 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): frappe.logger().warning(f"[{LOG_PREFIX}] master data file not found: {json_path}") 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) if not target: 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_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() - # round-trips). Uses db_insert() per doc — skips before_insert/after_insert - # hooks, which is appropriate for static master data. + # Bulk-insert path: avoids thousands of ORM .insert() round-trips, and for + # tree doctypes also sidesteps per-row nested-set recomputation (lft/rgt are + # rebuilt once at the end). db_insert() skips before_insert/after_insert + # hooks — appropriate for static master data. 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: doc = frappe.new_doc(doctype) 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: if field in rec: 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() inserted += 1 if inserted % 500 == 0: @@ -158,16 +165,24 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d frappe.db.set_value(doctype, key, diff, update_modified=False) updated += 1 - for key in set(existing.keys()) - set(target.keys()): - outcome = _try_remove(doctype, key, has_disabled) - if outcome == "deleted": - deleted += 1 - elif outcome == "disabled": - disabled += 1 - else: - skipped += 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()): + outcome = _try_remove(doctype, key, has_disabled) + if outcome == "deleted": + deleted += 1 + elif outcome == "disabled": + disabled += 1 + else: + skipped += 1 + + # 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.set_global(_global_key(doctype), current_hash) frappe.db.commit() summary = { @@ -178,10 +193,11 @@ def sync_master_data(doctype: str, json_path: str, key_field: str = "name") -> d "disabled": disabled, "skipped": skipped, } - print( - f"[{LOG_PREFIX}] {doctype}: " - f"+{inserted} ~{updated} -{deleted} (disabled {disabled}, skipped {skipped})" - ) + if inserted or updated or deleted or disabled or skipped: + print( + f"[{LOG_PREFIX}] {doctype}: " + f"+{inserted} ~{updated} -{deleted} (disabled {disabled}, skipped {skipped})" + ) return summary