From a9bb994f15858f6a4820a99bf9ff69dd5ff91f7c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 28 Dec 2023 14:24:45 +0530 Subject: [PATCH] fix!: deterministic fixture import order (#22210) * feat: #20753 fixture export prefix and fixture import order (cherry picked from commit 6a9c56a568e4ccf181fe9cb4153d0b9e4f02ac3d) * refactor: clarify prefix logic, see https://github.com/frappe/frappe/pull/20852/files/c4866921dfaa397bdea9a6c1a01c85d21218b593#r1196249038 (cherry picked from commit cd2519e71e5545bd4c706369df3ea05843a0bfd9) * style: format * refactor: conditionally sort documents when importing * refactor: simplify code * feat: Unittest as requested in https://github.com/frappe/frappe/pull/22210#discussion_r1331587501 --------- Co-authored-by: To Finke Co-authored-by: Ankush Menat --- .../core/doctype/data_import/data_import.py | 4 +- frappe/tests/test_hooks.py | 64 +++++++++++++++++++ frappe/utils/fixtures.py | 18 +++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index f3dca2d5af..ac28f9091f 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -241,9 +241,11 @@ def import_file(doctype, file_path, import_type, submit_after_import=False, cons i.import_data() -def import_doc(path, pre_process=None): +def import_doc(path, pre_process=None, sort=False): if os.path.isdir(path): files = [os.path.join(path, f) for f in os.listdir(path)] + if sort: + files.sort() else: files = [path] diff --git a/frappe/tests/test_hooks.py b/frappe/tests/test_hooks.py index 970699d01c..c1fcd23982 100644 --- a/frappe/tests/test_hooks.py +++ b/frappe/tests/test_hooks.py @@ -95,6 +95,70 @@ class TestHooks(FrappeTestCase): event.delete() + def test_fixture_prefix(self): + import os + import shutil + from frappe import hooks + from frappe.utils.fixtures import export_fixtures + + app = "frappe" + if(os.path.isdir(frappe.get_app_path(app, "fixtures"))): + shutil.rmtree(frappe.get_app_path(app, "fixtures")) + + # use any set of core doctypes for test purposes + hooks.fixtures = [ + {"dt": "User"}, + {"dt": "Contact"}, + {"dt": "Role"}, + ] + hooks.fixture_auto_order = False + # every call to frappe.get_hooks loads the hooks module into cache + # therefor the cache has to be invalidated after every manual overwriting of hooks + # TODO replace with a more elegant solution if there is one or build a util function for this purpose + if frappe._load_app_hooks.__wrapped__ in frappe.local.request_cache.keys(): + del frappe.local.request_cache[frappe._load_app_hooks.__wrapped__] + self.assertEqual([False], frappe.get_hooks("fixture_auto_order", app_name=app)) + self.assertEqual([ + {"dt": "User"}, + {"dt": "Contact"}, + {"dt": "Role"}, + ], frappe.get_hooks("fixtures", app_name=app)) + + export_fixtures(app) + # use assertCountEqual (replaced assertItemsEqual), beacuse os.listdir might return the list in a different order, depending on OS + self.assertCountEqual(["user.json", "contact.json", "role.json"], os.listdir(frappe.get_app_path(app, "fixtures"))) + + + hooks.fixture_auto_order = True + del frappe.local.request_cache[frappe._load_app_hooks.__wrapped__] + self.assertEqual([True], frappe.get_hooks("fixture_auto_order", app_name=app)) + + shutil.rmtree(frappe.get_app_path(app, "fixtures")) + export_fixtures(app) + self.assertCountEqual(["1_user.json", "2_contact.json", "3_role.json"], os.listdir(frappe.get_app_path(app, "fixtures"))) + + + hooks.fixtures = [ + { + "dt": "User", + "prefix": "my_prefix"}, + {"dt": "Contact"}, + {"dt": "Role"}, + ] + hooks.fixture_auto_order = False + + del frappe.local.request_cache[frappe._load_app_hooks.__wrapped__] + shutil.rmtree(frappe.get_app_path(app, "fixtures")) + export_fixtures(app) + self.assertCountEqual(["my_prefix_user.json", "contact.json", "role.json"], os.listdir(frappe.get_app_path(app, "fixtures"))) + + + hooks.fixture_auto_order = True + del frappe.local.request_cache[frappe._load_app_hooks.__wrapped__] + shutil.rmtree(frappe.get_app_path(app, "fixtures")) + export_fixtures(app) + self.assertCountEqual(["1_my_prefix_user.json", "2_contact.json", "3_role.json"], os.listdir(frappe.get_app_path(app, "fixtures"))) + class TestAPIHooks(FrappeAPITestCase): def test_auth_hook(self): diff --git a/frappe/utils/fixtures.py b/frappe/utils/fixtures.py index ddd8650451..2a9fb541b6 100644 --- a/frappe/utils/fixtures.py +++ b/frappe/utils/fixtures.py @@ -38,7 +38,7 @@ def import_fixtures(app): file_path = frappe.get_app_path(app, "fixtures", fname) try: - import_doc(file_path) + import_doc(file_path, sort=True) except (ImportError, frappe.DoesNotExistError) as e: # fixture syncing for missing doctypes print(f"Skipping fixture syncing from the file {fname}. Reason: {e}") @@ -67,20 +67,32 @@ def export_fixtures(app=None): else: apps = frappe.get_installed_apps() for app in apps: - for fixture in frappe.get_hooks("fixtures", app_name=app): + fixture_auto_order = bool(next(iter(frappe.get_hooks("fixture_auto_order", app_name=app)), False)) + fixtures = frappe.get_hooks("fixtures", app_name=app) + for index, fixture in enumerate(fixtures, start=1): filters = None or_filters = None if isinstance(fixture, dict): filters = fixture.get("filters") or_filters = fixture.get("or_filters") + prefix = fixture.get("prefix") fixture = fixture.get("doctype") or fixture.get("dt") print(f"Exporting {fixture} app {app} filters {(filters if filters else or_filters)}") if not os.path.exists(frappe.get_app_path(app, "fixtures")): os.mkdir(frappe.get_app_path(app, "fixtures")) + filename = frappe.scrub(fixture) + if prefix: + filename = f"{prefix}_{filename}" + if fixture_auto_order: + number_of_digits = len(str(len(fixtures))) + # add zero padding so files can be sorted lexicographically with filename. + file_number = str(index).zfill(number_of_digits) + filename = f"{file_number}_{filename}" + export_json( fixture, - frappe.get_app_path(app, "fixtures", frappe.scrub(fixture) + ".json"), + frappe.get_app_path(app, "fixtures", filename + ".json"), filters=filters, or_filters=or_filters, order_by="idx asc, creation asc",