Add "expose primary keys" option to create_postgis_store (#25)

* Add "expose primary keys" to create_postgis. * Fix tests * Fix tests * Add back 3.9 tests * Add 3.13 to unit test matrix * Enable postgis 3.4 tests to see what breaks * Create postgis_raster in e2e tests * Only add postgis_raster to 3.4 runs * semicolon * matrix.postgis not matrix.py * Enable debug on failing tests * Hack to force GeoServer to use full scan for extents. * Change test data to cover larger geographic scope to see if it solves issue with extent calculation * Disable failing checks in 3.4 * Disable postgis 3.4 tests for now
tethysplatform · Sep 24, 2024 · 34c562c · 34c562c
1 parent 157d2c9
commit 34c562c
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 37 deletions.
diff --git a/.github/workflows/e2e_tests.yml b/.github/workflows/e2e_tests.yml
@@ -15,17 +15,18 @@ jobs:
       fail-fast: false
       matrix:
         py:
-          - "3.10"
+          - "3.12"
         postgis:
           - "9.6-2.5-alpine"
           - "10-2.5-alpine"
           - "11-2.5-alpine"
           - "12-2.5-alpine"
-          # Not working on PostGIS 3.2 yet
-          # - "9.6-3.2-alpine"
-          # - "10-3.2-alpine"
-          # - "11-3.2-alpine"
-          # - "12-3.2-alpine"
+          # Not working on PostGIS 3.4 yet
+          # - "12-3.4-alpine"
+          # - "13-3.4-alpine"
+          # - "14-3.4-alpine"
+          # - "15-3.4-alpine"
+          # - "16-3.4-alpine"
     services:
       geoserver:
         image: tethysplatform/geoserver
@@ -52,9 +53,9 @@ jobs:
         ports:
           - 5432:5432
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.py }} for test
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.py }}
       - name: Install dependencies
@@ -66,6 +67,7 @@ jobs:
         run: |
           PGPASSWORD=mysecretpassword psql -U postgres -h localhost -c "CREATE DATABASE tds_tests WITH OWNER postgres;"
           PGPASSWORD=mysecretpassword psql -U postgres -h localhost -d tds_tests -c  "CREATE EXTENSION postgis;"
+          if [[ ${{ matrix.postgis }} == *"3.4-alpine"* ]]; then PGPASSWORD=mysecretpassword psql -U postgres -h localhost -d tds_tests -c  "CREATE EXTENSION postgis_raster;"; fi
           sleep 10
       - name: Run tests
         run: tox -e e2e_gs_tests
diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml
@@ -19,15 +19,15 @@ jobs:
           - windows
           - macos
         py:
+          # - "3.13"
+          - "3.12"
           - "3.11"
           - "3.10"
           - "3.9"
-          - "3.8"
-          - "3.7"
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.py }} for test
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.py }}
       - name: Install dependencies
@@ -38,7 +38,7 @@ jobs:
       - name: Run tests
         run: tox
       - name: Coveralls
-        if: matrix.os == 'ubuntu' && matrix.py == 3.10
+        if: matrix.os == 'ubuntu' && matrix.py == 3.12
         run: coveralls --service=github
         env: 
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/tests/e2e_tests/geoserver_engine_e2e_tests.py b/tests/e2e_tests/geoserver_engine_e2e_tests.py
@@ -131,7 +131,8 @@ def setup_postgis_table(self):
         insert_sql = "INSERT INTO {table} VALUES ({id}, '{name}', ST_GeomFromText('POINT({lon} {lat})', 4326));"
         rows = [
             {"id": 1, "name": "Aquaveo", "lat": 40.276039, "lon": -111.651120},
-            {"id": 2, "name": "BYU", "lat": 40.252335, "lon": -111.649326},
+            {"id": 2, "name": "Lynker", "lat": 39.111534, "lon": -77.556859},
+            {"id": 3, "name": "CHL", "lat": 32.299343, "lon": -90.866044},
         ]
 
         for r in rows:
@@ -989,10 +990,12 @@ def test_link_and_add_table(self):
         # Execute
         response = self.geoserver_engine.create_layer_from_postgis_store(
             store_id=store_id,
-            table=self.pg_table_name
+            table=self.pg_table_name,
+            debug=True
         )
 
         # Check for success response
+        # TODO: returns an error in PostGIS 3.4: Internal Server Error(500): :java.io.IOException: Error occured calculating bounds for points
         self.assertTrue(response['success'])
 
         # TEST list_stores
@@ -1136,8 +1139,11 @@ def test_create_sql_view_layer(self):
         # Create layer from postgis store
         response = self.geoserver_engine.create_layer_from_postgis_store(
             store_id=store_id,
-            table=self.pg_table_name
+            table=self.pg_table_name,
+            debug=True
         )
+
+        # TODO: returns an error in PostGIS 3.4: Internal Server Error(500): :java.io.IOException: Error occured calculating bounds for points
         self.assertTrue(response['success'])
 
         # Pause to let GeoServer catch up before continuing

diff --git a/tests/unit_tests/test_geoserver_engine.py b/tests/unit_tests/test_geoserver_engine.py
@@ -210,7 +210,7 @@ def test_list_resources(self, mock_catalog):
         for n in self.resource_names:
             self.assertIn(n, result)
 
-        mc.get_resources.called_with(store=None, workspace=None)
+        mc.get_resources.assert_called_with(stores=None, workspaces=None)
 
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerCatalog')
     def test_list_resources_with_properties(self, mock_catalog):
@@ -244,7 +244,7 @@ def test_list_resources_with_properties(self, mock_catalog):
             self.assertIn('store', r)
             self.assertEqual(self.store_name, r['store'])
 
-        mc.get_resources.called_with(store=None, workspace=None)
+        mc.get_resources.assert_called_with(stores=None, workspaces=None)
 
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerCatalog')
     def test_list_resources_ambiguous_error(self, mock_catalog):
@@ -260,7 +260,7 @@ def test_list_resources_ambiguous_error(self, mock_catalog):
         # Success
         self.assertFalse(response['success'])
 
-        mc.get_resources.called_with(store=None, workspace=None)
+        mc.get_resources.assert_called_with(stores=None, workspaces=None)
 
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerCatalog')
     def test_list_resources_multiple_stores_error(self, mock_catalog):
@@ -277,7 +277,7 @@ def test_list_resources_multiple_stores_error(self, mock_catalog):
         self.assertFalse(response['success'])
         self.assertIn('Multiple stores found named', response['error'])
 
-        mc.get_resources.called_with(store=None, workspace=None)
+        mc.get_resources.assert_called_with(stores=None, workspaces=None)
 
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerCatalog')
     def test_list_layers(self, mock_catalog):
@@ -3708,6 +3708,7 @@ def test_create_postgis_store_validate_connection(self, mock_post, _):
                   <entry key="Max connection idle time">{7}</entry>
                   <entry key="Evictor run periodicity">{8}</entry>
                   <entry key="validate connections">true</entry>
+                  <entry key="Expose primary keys">false</entry>
                 </connectionParameters>
               </dataStore>
               """.format('foo', host, port, database, username, password, max_connections, max_connection_idle_time,
@@ -3756,6 +3757,7 @@ def test_create_postgis_store_validate_connection_false(self, mock_workspace, mo
                   <entry key="Max connection idle time">{7}</entry>
                   <entry key="Evictor run periodicity">{8}</entry>
                   <entry key="validate connections">false</entry>
+                  <entry key="Expose primary keys">false</entry>
                 </connectionParameters>
               </dataStore>
               """.format('foo', host, port, database, username, password, max_connections, max_connection_idle_time,
@@ -3774,6 +3776,56 @@ def test_create_postgis_store_validate_connection_false(self, mock_workspace, mo
                                          max_connection_idle_time, evictor_run_periodicity, validate_connections=False)
         mock_post.assert_called_with(url=rest_endpoint, data=xml, headers=expected_headers, auth=self.auth)
 
+    @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerSpatialDatasetEngine.get_store')
+    @mock.patch('tethys_dataset_services.engines.geoserver_engine.requests.post')
+    @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerCatalog.get_default_workspace')
+    def test_create_postgis_store_expose_primary_keys_true(self, mock_workspace, mock_post, _):
+        mock_post.return_value = MockResponse(201)
+        store_id = 'foo'
+        mock_workspace().name = self.workspace_name
+        host = 'localhost'
+        port = '5432'
+        database = 'foo_db'
+        username = 'user'
+        password = 'pass'
+        max_connections = 10
+        max_connection_idle_time = 40
+        evictor_run_periodicity = 60
+
+        xml = """
+              <dataStore>
+                <name>{0}</name>
+                <connectionParameters>
+                  <entry key="host">{1}</entry>
+                  <entry key="port">{2}</entry>
+                  <entry key="database">{3}</entry>
+                  <entry key="user">{4}</entry>
+                  <entry key="passwd">{5}</entry>
+                  <entry key="dbtype">postgis</entry>
+                  <entry key="max connections">{6}</entry>
+                  <entry key="Max connection idle time">{7}</entry>
+                  <entry key="Evictor run periodicity">{8}</entry>
+                  <entry key="validate connections">false</entry>
+                  <entry key="Expose primary keys">true</entry>
+                </connectionParameters>
+              </dataStore>
+              """.format('foo', host, port, database, username, password, max_connections, max_connection_idle_time,
+                         evictor_run_periodicity)
+
+        expected_headers = {
+            "Content-type": "text/xml",
+            "Accept": "application/xml"
+        }
+
+        rest_endpoint = '{endpoint}workspaces/{workspace}/datastores'.format(
+            endpoint=self.endpoint,
+            workspace=self.workspace_name
+        )
+        self.engine.create_postgis_store(store_id, host, port, database, username, password, max_connections, 
+                                         max_connection_idle_time, evictor_run_periodicity, validate_connections=False,
+                                         expose_primary_keys=True)
+        mock_post.assert_called_with(url=rest_endpoint, data=xml, headers=expected_headers, auth=self.auth)
+
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.GeoServerSpatialDatasetEngine.get_store')
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.log')
     @mock.patch('tethys_dataset_services.engines.geoserver_engine.requests.post')
@@ -3803,6 +3855,7 @@ def test_create_postgis_store_not_201(self, mock_post, mock_logger, _):
                   <entry key="Max connection idle time">{7}</entry>
                   <entry key="Evictor run periodicity">{8}</entry>
                   <entry key="validate connections">true</entry>
+                  <entry key="Expose primary keys">false</entry>
                 </connectionParameters>
               </dataStore>
               """.format('foo', host, port, database, username, password, max_connections, max_connection_idle_time,

diff --git a/tethys_dataset_services/engines/geoserver_engine.py b/tethys_dataset_services/engines/geoserver_engine.py
@@ -1250,7 +1250,7 @@ def link_sqlalchemy_db_to_geoserver(self, store_id, sqlalchemy_engine, max_conne
 
     def create_postgis_store(self, store_id, host, port, database, username, password, max_connections=5,
                              max_connection_idle_time=30, evictor_run_periodicity=30, validate_connections=True,
-                             debug=False):
+                             expose_primary_keys=False, debug=False):
         """
         Use this method to link an existing PostGIS database to GeoServer as a feature store. Note that this method only works for data in vector formats.
 
@@ -1265,6 +1265,7 @@ def create_postgis_store(self, store_id, host, port, database, username, passwor
           max_connection_idle_time (int, optional): Number of seconds a connections can stay idle before the evictor considers closing it. Defaults to 30 seconds.
           evictor_run_periodicity (int, optional): Number of seconds between idle connection evictor runs. Defaults to 30 seconds.
           validate_connections (bool, optional): Test connections before using. Defaults to True.
+          expose_primary_keys (bool, optional):
           debug (bool, optional): Pretty print the response dictionary to the console for debugging. Defaults to False.
 
         Returns:
@@ -1283,24 +1284,24 @@ def create_postgis_store(self, store_id, host, port, database, username, passwor
             workspace = self.catalog.get_default_workspace().name
 
         # Create the store
-        xml = """
+        xml = f"""
               <dataStore>
-                <name>{0}</name>
+                <name>{name}</name>
                 <connectionParameters>
-                  <entry key="host">{1}</entry>
-                  <entry key="port">{2}</entry>
-                  <entry key="database">{3}</entry>
-                  <entry key="user">{4}</entry>
-                  <entry key="passwd">{5}</entry>
+                  <entry key="host">{host}</entry>
+                  <entry key="port">{port}</entry>
+                  <entry key="database">{database}</entry>
+                  <entry key="user">{username}</entry>
+                  <entry key="passwd">{password}</entry>
                   <entry key="dbtype">postgis</entry>
-                  <entry key="max connections">{6}</entry>
-                  <entry key="Max connection idle time">{7}</entry>
-                  <entry key="Evictor run periodicity">{8}</entry>
-                  <entry key="validate connections">{9}</entry>
+                  <entry key="max connections">{max_connections}</entry>
+                  <entry key="Max connection idle time">{max_connection_idle_time}</entry>
+                  <entry key="Evictor run periodicity">{evictor_run_periodicity}</entry>
+                  <entry key="validate connections">{str(validate_connections).lower()}</entry>
+                  <entry key="Expose primary keys">{str(expose_primary_keys).lower()}</entry>
                 </connectionParameters>
               </dataStore>
-              """.format(name, host, port, database, username, password, max_connections, max_connection_idle_time,
-                         evictor_run_periodicity, str(validate_connections).lower())
+              """
 
         # Prepare headers
         headers = {

diff --git a/tox.ini b/tox.ini
@@ -1,14 +1,14 @@
 [tox]
 isolated_build = True
-envlist = py37, py38, py39, py310, py311, flake8, clean
+envlist = py39, py310, py311, py312, py313, flake8, clean
 
 [gh-actions]
 python =
-    3.7: py37
-    3.8: py38
     3.9: py39
     3.10: py310
     3.11: py311
+    3.12: py312
+    3.13: py313
 
 [testenv]
 deps =