From a5b05d0a6be698ee0302e20922f1e94da45cc101 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:15:51 +0100 Subject: [PATCH 1/8] Added detecting and deleting `remap.db` tables rows for `master_fid`s which are not present in the OUTPUT `master.gpkg`. --- .../test/config-farm-duplicate_wp.yml | 24 ++++++++ workpackages/test/test_basic.py | 55 +++++++++++++++++++ workpackages/wp.py | 45 +++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 workpackages/test/config-farm-duplicate_wp.yml diff --git a/workpackages/test/config-farm-duplicate_wp.yml b/workpackages/test/config-farm-duplicate_wp.yml new file mode 100644 index 0000000..fa564b7 --- /dev/null +++ b/workpackages/test/config-farm-duplicate_wp.yml @@ -0,0 +1,24 @@ +file: farms.gpkg + +work-packages: + - name: Kyle + value: 4 + mergin-project: martin/farms-Kyle + + - name: Kyle_duplicate + value: 4 + mergin-project: martin/farms-Kyle-duplicate + + - name: Emma + value: + - 1 + - 2 + mergin-project: martin/farms-Emma + +tables: + - name: farms + method: filter-column + filter-column-name: fid + - name: trees + method: filter-column + filter-column-name: farm_id diff --git a/workpackages/test/test_basic.py b/workpackages/test/test_basic.py index ad6606e..56b3437 100644 --- a/workpackages/test/test_basic.py +++ b/workpackages/test/test_basic.py @@ -393,6 +393,61 @@ def test_delete_row_master_wp(): _assert_row_missing(os.path.join(output_dir, "Kyle.gpkg"), "trees", 1000001) +def test_wp_add_wp_delete_wp_duplicate_add_feature(): + """ + Test following workflow scenario (with running make_work_packages function after each step): + - we have 2 WPs with a same filter condition (WP1, WP2); + - we are adding a new feature (FID: 1111111) to WP1; + - we are deleting feature (FID: 1111111) from the WP1; + - we are adding a new feature (FID: 1111111) to WP2; + """ + config_file = os.path.join(this_dir, "config-farm-duplicate_wp.yml") + tmp_dir_1 = _make_initial_farm_work_packages(config_file) + output_dir = os.path.join(tmp_dir_1.name, "output") + output_files = os.listdir(output_dir) + assert "Emma.gpkg" in output_files + assert "Kyle.gpkg" in output_files + assert "Kyle_duplicate.gpkg" in output_files + assert "master.gpkg" in output_files + + tmp_dir_2 = _prepare_next_run_work_packages(tmp_dir_1) + + # modify 'Kyle' WP by adding a new 'trees' feature with a FID '1111111' + open_layer_and_create_feature( + os.path.join(tmp_dir_2.name, "input", "Kyle.gpkg"), + "trees", + "POINT(6 16)", + {"tree_species_id": 1, "farm_id": 4}, + fid=1111111 + ) + # run work packaging + wp_config = load_config_from_yaml(config_file) + make_work_packages(tmp_dir_2.name, wp_config) + tmp_dir_3 = _prepare_next_run_work_packages(tmp_dir_2) + # modify 'Kyle' WP by removing 'trees' feature with a FID '1111111' + open_layer_and_delete_feature( + os.path.join(tmp_dir_3.name, "input", "Kyle.gpkg"), "trees", 1111111 + ) + # run work packaging 2nd time + make_work_packages(tmp_dir_3.name, wp_config) + tmp_dir_4 = _prepare_next_run_work_packages(tmp_dir_3) + # modify 'Kyle_duplicate' WP by adding a new (but with previously used FID '1111111') 'trees' feature + open_layer_and_create_feature( + os.path.join(tmp_dir_4.name, "input", "Kyle_duplicate.gpkg"), + "trees", + "POINT(6 16)", + {"tree_species_id": 1, "farm_id": 4}, + fid=1111111 + ) + # run work packaging 3rd time + make_work_packages(tmp_dir_4.name, wp_config) + final_output_dir = os.path.join(tmp_dir_4.name, "output") + final_output_files = os.listdir(final_output_dir) + assert "Emma.gpkg" in final_output_files + assert "Kyle.gpkg" in final_output_files + assert "Kyle_duplicate.gpkg" in final_output_files + assert "master.gpkg" in final_output_files + # TODO: more test cases # - delete_master_update_wp # one row deleted in master while it is updated in WP # - update_master_delete_wp # one row updated in master while it is deleted in WP diff --git a/workpackages/wp.py b/workpackages/wp.py index 627e4e0..9abeec1 100644 --- a/workpackages/wp.py +++ b/workpackages/wp.py @@ -8,6 +8,7 @@ import os import shutil import pygeodiff +from collections import defaultdict from pathlib import Path import yaml @@ -366,3 +367,47 @@ def _logger_callback(level, text_bytes): else: # first time this WP is created... pass # TODO: what to do? + + # STAGE 3: Cleanup remap.db + print("STAGE 3 [remap.db CLEANUP]") + existing_master_fids = defaultdict(set) + # Connect to the OUTPUT master.gpkg and get all FIDs. + # We don't need to look into each WP after all features are mapped back to master. + db = sqlite3.connect(master_gpkg_output) + c = db.cursor() + # Iterate over all tables and get unique FIDs + for wp_table in wp_config.wp_tables: + wp_table_name = wp_table.name + wp_tab_name_esc = escape_double_quotes(wp_table_name) + c.execute(f"""SELECT fid FROM {wp_tab_name_esc};""") + existing_master_fids[wp_table_name] |= set(r[0] for r in c.fetchall()) + db.close() + # Iterate over all remap.db tables and get unique master_fids + existing_remap_master_fids = defaultdict(set) + master_id_column_escaped = escape_double_quotes("master_fid") + db = sqlite3.connect(remap_db_output) + c = db.cursor() + for wp in wp_config.wp_names: + wp_name = wp.name + for wp_table in wp_config.wp_tables: + wp_table_name = wp_table.name + table_wp_name = f"{wp_table_name}_{wp_name}" + table_wp_name_esc = escape_double_quotes(table_wp_name) + c.execute(f"""SELECT {master_id_column_escaped} FROM {table_wp_name_esc};""") + existing_remap_master_fids[wp_table_name] |= set(r[0] for r in c.fetchall()) + # Detect and delete remap.db tables rows for master_ids which are not present in the OUTPUT master.gpkg + c.execute("BEGIN") + for wp in wp_config.wp_names: + wp_name = wp.name + for wp_table in wp_config.wp_tables: + wp_table_name = wp_table.name + missing_master_fids = list(existing_remap_master_fids[wp_table_name] - existing_master_fids[wp_table_name]) + if not missing_master_fids: + continue + table_wp_name = f"{wp_table_name}_{wp_name}" + table_wp_name_esc = escape_double_quotes(table_wp_name) + missing_master_fids_str = ",".join(["?"] * len(missing_master_fids)) + c.execute(f"""DELETE FROM {table_wp_name_esc} WHERE {master_id_column_escaped} IN ({missing_master_fids_str});""", missing_master_fids) + c.execute("COMMIT") + c.execute("VACUUM") + db.close() From 48c81a32954f4823a4bebf9555071873dbc45328 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:19:01 +0100 Subject: [PATCH 2/8] `black` reformatting. --- mergin_work_packages.py | 1 - scripts/update_version.py | 6 +++--- workpackages/test/test_basic.py | 9 ++++----- workpackages/wp.py | 5 ++++- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/mergin_work_packages.py b/mergin_work_packages.py index e06be07..8041353 100644 --- a/mergin_work_packages.py +++ b/mergin_work_packages.py @@ -1,4 +1,3 @@ - from workpackages.wp_mergin import run_wp_mergin_with_context, parse_args if __name__ == "__main__": diff --git a/scripts/update_version.py b/scripts/update_version.py index ee6baad..6c7a2a4 100644 --- a/scripts/update_version.py +++ b/scripts/update_version.py @@ -4,7 +4,7 @@ def replace_in_file(filepath, regex, sub): - with open(filepath, 'r') as f: + with open(filepath, "r") as f: content = f.read() content_new = re.sub(regex, sub, content, flags=re.M) @@ -15,11 +15,11 @@ def replace_in_file(filepath, regex, sub): dir_path = os.path.dirname(os.path.realpath(__file__)) parser = argparse.ArgumentParser() -parser.add_argument('--version', help='version to replace') +parser.add_argument("--version", help="version to replace") args = parser.parse_args() ver = args.version print("using version " + ver) about_file = os.path.join(dir_path, os.pardir, "workpackages", "version.py") print("patching " + about_file) -replace_in_file(about_file, "__version__\s=\s\".*", "__version__ = \"" + ver + "\"") +replace_in_file(about_file, '__version__\s=\s".*', '__version__ = "' + ver + '"') diff --git a/workpackages/test/test_basic.py b/workpackages/test/test_basic.py index 56b3437..339cc41 100644 --- a/workpackages/test/test_basic.py +++ b/workpackages/test/test_basic.py @@ -418,16 +418,14 @@ def test_wp_add_wp_delete_wp_duplicate_add_feature(): "trees", "POINT(6 16)", {"tree_species_id": 1, "farm_id": 4}, - fid=1111111 + fid=1111111, ) # run work packaging wp_config = load_config_from_yaml(config_file) make_work_packages(tmp_dir_2.name, wp_config) tmp_dir_3 = _prepare_next_run_work_packages(tmp_dir_2) # modify 'Kyle' WP by removing 'trees' feature with a FID '1111111' - open_layer_and_delete_feature( - os.path.join(tmp_dir_3.name, "input", "Kyle.gpkg"), "trees", 1111111 - ) + open_layer_and_delete_feature(os.path.join(tmp_dir_3.name, "input", "Kyle.gpkg"), "trees", 1111111) # run work packaging 2nd time make_work_packages(tmp_dir_3.name, wp_config) tmp_dir_4 = _prepare_next_run_work_packages(tmp_dir_3) @@ -437,7 +435,7 @@ def test_wp_add_wp_delete_wp_duplicate_add_feature(): "trees", "POINT(6 16)", {"tree_species_id": 1, "farm_id": 4}, - fid=1111111 + fid=1111111, ) # run work packaging 3rd time make_work_packages(tmp_dir_4.name, wp_config) @@ -448,6 +446,7 @@ def test_wp_add_wp_delete_wp_duplicate_add_feature(): assert "Kyle_duplicate.gpkg" in final_output_files assert "master.gpkg" in final_output_files + # TODO: more test cases # - delete_master_update_wp # one row deleted in master while it is updated in WP # - update_master_delete_wp # one row updated in master while it is deleted in WP diff --git a/workpackages/wp.py b/workpackages/wp.py index 9abeec1..b4559dd 100644 --- a/workpackages/wp.py +++ b/workpackages/wp.py @@ -407,7 +407,10 @@ def _logger_callback(level, text_bytes): table_wp_name = f"{wp_table_name}_{wp_name}" table_wp_name_esc = escape_double_quotes(table_wp_name) missing_master_fids_str = ",".join(["?"] * len(missing_master_fids)) - c.execute(f"""DELETE FROM {table_wp_name_esc} WHERE {master_id_column_escaped} IN ({missing_master_fids_str});""", missing_master_fids) + c.execute( + f"""DELETE FROM {table_wp_name_esc} WHERE {master_id_column_escaped} IN ({missing_master_fids_str});""", + missing_master_fids, + ) c.execute("COMMIT") c.execute("VACUUM") db.close() From dff8071c1f63ba34bba4c9710283464f06f500f7 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:29:08 +0100 Subject: [PATCH 3/8] Fixed test server URL. --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 42cf55f..5db9aad 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,7 +2,7 @@ name: Auto Tests on: [push] env: - TEST_MERGIN_URL: https://test.dev.merginmaps.com/ + TEST_MERGIN_URL: https://app.dev.merginmaps.com/ TEST_API_USERNAME: test_plugin TEST_API_PASSWORD: ${{ secrets.MERGINTEST_API_PASSWORD }} From 68951b14ed6476be49caad1221c0cc92192cd7ce Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:32:35 +0100 Subject: [PATCH 4/8] Changed TEST_API_PASSWORD --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5db9aad..76dc98d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -4,7 +4,7 @@ on: [push] env: TEST_MERGIN_URL: https://app.dev.merginmaps.com/ TEST_API_USERNAME: test_plugin - TEST_API_PASSWORD: ${{ secrets.MERGINTEST_API_PASSWORD }} + TEST_API_PASSWORD: ${{ secrets.TEST_API_PASSWORD }} concurrency: group: ci-${{github.ref}}-autotests From e8ce224b94caf52adc47f210617ba5ffebce0ef7 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:41:43 +0100 Subject: [PATCH 5/8] Changes in test_with_server.py --- workpackages/test/test_with_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workpackages/test/test_with_server.py b/workpackages/test/test_with_server.py index 37737b2..2c72eaa 100644 --- a/workpackages/test/test_with_server.py +++ b/workpackages/test/test_with_server.py @@ -20,7 +20,7 @@ def create_client(user, pwd): - assert SERVER_URL and SERVER_URL.rstrip("/") != "https://app.merginmaps.com" and user and pwd + assert SERVER_URL and SERVER_URL.rstrip("/") != "https://app.dev.merginmaps.com" and user and pwd return MerginClient(SERVER_URL, login=user, password=pwd) From 2e495e77a0180bc7a054040540b6ead422b73396 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 17:57:36 +0100 Subject: [PATCH 6/8] Changes in test_with_server.py --- workpackages/test/test_with_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workpackages/test/test_with_server.py b/workpackages/test/test_with_server.py index 2c72eaa..4451684 100644 --- a/workpackages/test/test_with_server.py +++ b/workpackages/test/test_with_server.py @@ -20,7 +20,7 @@ def create_client(user, pwd): - assert SERVER_URL and SERVER_URL.rstrip("/") != "https://app.dev.merginmaps.com" and user and pwd + assert SERVER_URL and API_USER and USER_PWD return MerginClient(SERVER_URL, login=user, password=pwd) From 36203d96aa113aa125b394c7ae9a03660c410b25 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 18:07:24 +0100 Subject: [PATCH 7/8] Changes in test_with_server.py --- .github/workflows/tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 76dc98d..89316af 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -3,8 +3,9 @@ on: [push] env: TEST_MERGIN_URL: https://app.dev.merginmaps.com/ - TEST_API_USERNAME: test_plugin - TEST_API_PASSWORD: ${{ secrets.TEST_API_PASSWORD }} + TEST_API_USERNAME: test_db_sync + TEST_API_PASSWORD: ${{ secrets.MERGINTEST_API_PASSWORD }} + TEST_WORKSPACE: test-db-sync concurrency: group: ci-${{github.ref}}-autotests From 5b97cd05afc970e8856ba2bed613ac7f0fe50b98 Mon Sep 17 00:00:00 2001 From: "lukasz.debek" Date: Thu, 27 Mar 2025 18:24:53 +0100 Subject: [PATCH 8/8] Changes in test GH action. --- .github/workflows/tests.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 89316af..da86fdd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -3,9 +3,8 @@ on: [push] env: TEST_MERGIN_URL: https://app.dev.merginmaps.com/ - TEST_API_USERNAME: test_db_sync - TEST_API_PASSWORD: ${{ secrets.MERGINTEST_API_PASSWORD }} - TEST_WORKSPACE: test-db-sync + TEST_API_USERNAME: test_plugin + TEST_API_PASSWORD: ${{ secrets.TEST_API_PASSWORD }} concurrency: group: ci-${{github.ref}}-autotests @@ -30,4 +29,5 @@ jobs: - name: run tests run: | - python3 -m pytest . + python3 -m pytest . --ignore=./workpackages/test/test_with_server.py +