Fix missing fid col

.github/workflows/tests.yml

@@ -2,9 +2,10 @@ name: Auto Tests
 on: [push]
 
 env:
-  TEST_MERGIN_URL: https://test.dev.merginmaps.com/
-  TEST_API_USERNAME: test_plugin
+  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
 
 concurrency:
   group: ci-${{github.ref}}-autotests

.gitignore

@@ -133,3 +133,6 @@ dmypy.json
 
 # Pyre type checker
 .pyre/
+
+# VS Code
+.vscode

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.2.1
+
+- fix error when the table does not use `fid` column as primary key (#64)
+
 ## 1.2.0
 
 - Improve robustness and speed of sync (#60, #61)

workpackages/remapping.py

@@ -62,7 +62,7 @@ def remap_table_master_to_wp(cursor, table_name, wp_name):
     master_fids_missing = set()
     sql = (
         f"""SELECT {pkey_column_escaped} FROM {table_name_escaped} """
-        f"""LEFT JOIN {remap_table_escaped} AS mapped ON fid = mapped.master_fid WHERE mapped.wp_fid IS NULL"""
+        f"""LEFT JOIN {remap_table_escaped} AS mapped ON {pkey_column_escaped} = mapped.master_fid WHERE mapped.wp_fid IS NULL"""
     )
     for row in cursor.execute(sql):
         master_fids_missing.add(row[0])
@@ -83,7 +83,7 @@ def remap_table_master_to_wp(cursor, table_name, wp_name):
     mapping = []
     sql = (
         f"""SELECT {pkey_column_escaped}, mapped.wp_fid FROM {table_name_escaped} """
-        f"""LEFT JOIN {remap_table_escaped} AS mapped ON fid = mapped.master_fid"""
+        f"""LEFT JOIN {remap_table_escaped} AS mapped ON {pkey_column_escaped} = mapped.master_fid"""
     )
     for row in cursor.execute(sql):
         mapping.append((row[0], row[1]))
@@ -117,7 +117,7 @@ def remap_table_wp_to_master(cursor, table_name, wp_name, new_master_fid):
     wp_fids_missing = set()
     sql = (
         f"""SELECT {pkey_column_escaped} FROM {table_name_escaped} """
-        f"""LEFT JOIN {remap_table} AS mapped ON fid = mapped.wp_fid WHERE mapped.master_fid IS NULL"""
+        f"""LEFT JOIN {remap_table} AS mapped ON {pkey_column_escaped} = mapped.wp_fid WHERE mapped.master_fid IS NULL"""
     )
     for row in cursor.execute(sql):
         wp_fids_missing.add(row[0])
@@ -131,7 +131,7 @@ def remap_table_wp_to_master(cursor, table_name, wp_name, new_master_fid):
     mapping = []  # list of tuples (wp_fid, master_fid)
     sql = (
         f"""SELECT {pkey_column_escaped}, mapped.master_fid FROM {table_name_escaped} """
-        f"""LEFT JOIN {remap_table} AS mapped ON fid = mapped.wp_fid"""
+        f"""LEFT JOIN {remap_table} AS mapped ON {pkey_column_escaped} = mapped.wp_fid"""
     )
     for row in cursor.execute(sql):
         mapping.append((row[0], row[1]))
@@ -141,5 +141,6 @@ def remap_table_wp_to_master(cursor, table_name, wp_name, new_master_fid):
 
     for wp_fid, master_fid in mapping:
         cursor.execute(
-            f"""UPDATE {table_name_escaped} SET {pkey_column_escaped} = ? WHERE fid = ?""", (master_fid, -wp_fid)
+            f"""UPDATE {table_name_escaped} SET {pkey_column_escaped} = ? WHERE {pkey_column_escaped} = ?""",
+            (master_fid, -wp_fid),
         )

workpackages/test/test_basic.py

@@ -397,3 +397,34 @@ def test_delete_row_master_wp():
 # - 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
 # - update_master_update_wp  # one row updated in master and the same row updated in WP
+
+
+def test_not_using_fid_colum():
+    """Special test case where data do not use `fid` column for primary key as usual in GPKG files,
+    but rather custom named column `objectid`."""
+
+    tmp_dir_obj = TemporaryDirectory(prefix="test-mergin-work-packages-")
+    tmp_dir = tmp_dir_obj.name
+    os.makedirs(os.path.join(tmp_dir, "input"))
+
+    # get data
+    shutil.copy(os.path.join(this_dir, "data", "farms_without_fid.gpkg"), os.path.join(tmp_dir, "input", "master.gpkg"))
+
+    # get config
+    wp_config = load_config_from_yaml(os.path.join(this_dir, "config-farm-basic.yml"))
+    # switch from using `fid` (that does not exist) to `objectid`
+    wp_config.wp_tables[0].filter_column_name = "objectid"
+
+    # run alg
+    make_work_packages(tmp_dir, wp_config)
+
+    # run checks
+    output_dir = os.path.join(tmp_dir, "output")
+    output_files = os.listdir(output_dir)
+    assert "Emma.gpkg" in output_files
+    assert "Kyle.gpkg" in output_files
+    assert "master.gpkg" in output_files
+
+    _assert_row_counts(os.path.join(output_dir, "master.gpkg"), expected_farms=4, expected_trees=9)
+    _assert_row_counts(os.path.join(output_dir, "Kyle.gpkg"), expected_farms=1, expected_trees=2)
+    _assert_row_counts(os.path.join(output_dir, "Emma.gpkg"), expected_farms=2, expected_trees=6)

workpackages/test/test_with_server.py

@@ -13,6 +13,7 @@
 SERVER_URL = os.environ.get("TEST_MERGIN_URL")
 API_USER = os.environ.get("TEST_API_USERNAME")
 USER_PWD = os.environ.get("TEST_API_PASSWORD")
+WORKSPACE = os.environ.get("TEST_WORKSPACE")
 
 TMP_DIR = tempfile.gettempdir()
 
@@ -80,9 +81,9 @@ def test_wp_1(mc: MerginClient):
     project_master_name = "farms-master"
     project_kyle_name = "farms-Kyle"
     project_emma_name = "farms-Emma"
-    project_master_full = API_USER + "/" + project_master_name
-    project_kyle_full = API_USER + "/" + project_kyle_name
-    project_emma_full = API_USER + "/" + project_emma_name
+    project_master_full = WORKSPACE + "/" + project_master_name
+    project_kyle_full = WORKSPACE + "/" + project_kyle_name
+    project_emma_full = WORKSPACE + "/" + project_emma_name
     project_dir_master = os.path.join(TMP_DIR, "wp-edits", project_master_name)
     project_dir_kyle = os.path.join(TMP_DIR, "wp-edits", project_kyle_name)
     project_dir_emma = os.path.join(TMP_DIR, "wp-edits", project_emma_name)
@@ -100,7 +101,7 @@ def test_wp_1(mc: MerginClient):
 
     with open(config_yaml, "r") as file:
         filedata = file.read()
-        filedata = filedata.replace("martin/", API_USER + "/")
+        filedata = filedata.replace("martin/", WORKSPACE + "/")
     with open(config_yaml, "w") as file:
         file.write(filedata)
 

workpackages/version.py

@@ -1 +1 @@
-__version__ = "1.2.0"
+__version__ = "1.2.1"

workpackages/wp.py

@@ -13,7 +13,7 @@
 import yaml
 
 from .wp_utils import escape_double_quotes
-from .remapping import remap_table_master_to_wp, remap_table_wp_to_master
+from .remapping import remap_table_master_to_wp, remap_table_wp_to_master, _table_pkey
 
 # Layout of files:
 #
@@ -191,7 +191,9 @@ def _logger_callback(level, text_bytes):
         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 max(fid) FROM {wp_tab_name_esc};""")
+            pkey_column = _table_pkey(c, wp_table_name)
+            pkey_column_escaped = escape_double_quotes(pkey_column)
+            c.execute(f"""SELECT max({pkey_column_escaped}) FROM {wp_tab_name_esc};""")
             new_master_fid = c.fetchone()[0]
             if new_master_fid is None:
                 new_master_fid = 1  # empty table so far