Duplicate wp add delete add issue

.github/workflows/tests.yml

@@ -2,9 +2,9 @@ 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 }}
+  TEST_API_PASSWORD: ${{ secrets.TEST_API_PASSWORD }}
 
 concurrency:
   group: ci-${{github.ref}}-autotests
@@ -29,4 +29,5 @@ jobs:
 
       - name: run tests
         run: |
-          python3 -m pytest .
+          python3 -m pytest . --ignore=./workpackages/test/test_with_server.py
+

mergin_work_packages.py

@@ -1,4 +1,3 @@
-
 from workpackages.wp_mergin import run_wp_mergin_with_context, parse_args
 
 if __name__ == "__main__":

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 + '"')

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

workpackages/test/test_basic.py

@@ -393,6 +393,60 @@ 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

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 API_USER and USER_PWD
     return MerginClient(SERVER_URL, login=user, password=pwd)
 
 

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,50 @@ 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()