From 8d377072b96265ec36ec3d3b61dca8d255c577fc Mon Sep 17 00:00:00 2001 From: Bernhard Mallinger Date: Wed, 6 Mar 2024 16:24:51 +0100 Subject: [PATCH] Allow retrieving extra properties in oracle provider (#1544) * Implement extra_properties in oracle Provider These can be used to configure additional database-computed fields in the config file which are returned on `get` and `query` calls * Allow mandating properties which are not part of the output Previously, properties which were not requested for the output were not part of `fields`, which means that they were not passed in to the provider as filter properties for e.g. `query()`. This commit adds them there and introduces a new variable `filtered_fields`, which is used for limiting the output of queries. There is also some minor refactoring, but the existing and also the newly written tests should avoid regressions. * Restore previous behavior for default arguments --- .../data-publishing/ogcapi-features.rst | 26 +++++++- pygeoapi/provider/oracle.py | 66 +++++++++++-------- tests/test_oracle_provider.py | 61 ++++++++++++++++- 3 files changed, 122 insertions(+), 31 deletions(-) diff --git a/docs/source/data-publishing/ogcapi-features.rst b/docs/source/data-publishing/ogcapi-features.rst index ec5a2fb..7c98631 100644 --- a/docs/source/data-publishing/ogcapi-features.rst +++ b/docs/source/data-publishing/ogcapi-features.rst @@ -357,13 +357,37 @@ Mandatory properties table: lakes geom_field: geometry title_field: name - manadory_properties: + mandatory_properties: - example_group_id On large tables it could be useful to disallow a query on the complete dataset. For this reason it is possible to configure mandatory properties. When this is activated, the provoder throws an exception when the parameter is not in the query uri. +Extra properties +"""""""""""""""""""" +.. code-block:: yaml + + providers: + - type: feature + name: OracleDB + data: + host: 127.0.0.1 + port: 1521 + service_name: XEPDB1 + user: geo_test + password: geo_test + id_field: id + table: lakes + geom_field: geometry + title_field: name + extra_properties: + - "'Here we have ' || name AS tooltip" + +Extra properties is a list of strings which are added as fields for data retrieval in the SELECT clauses. They +can be used to return expressions computed by the database. + + Custom SQL Manipulator Plugin """"""""""""""""""""""""""""" The provider supports a SQL-Manipulator-Plugin class. With this, the SQL statement could be manipulated. This is diff --git a/pygeoapi/provider/oracle.py b/pygeoapi/provider/oracle.py index 255dc31..a9c2ba2 100644 --- a/pygeoapi/provider/oracle.py +++ b/pygeoapi/provider/oracle.py @@ -204,24 +204,24 @@ class DatabaseConnection: LOGGER.debug("Table: " + table) if self.context == "query": - column_list = self._get_table_columns(schema, table) - - # When self.properties is set, then the result would be filtered - if self.properties: - column_list = [ - col - for col in column_list - if col[0].lower() - in [item.lower() for item in self.properties] - ] - - # Concatenate column names with ', ' - self.columns = ", ".join([item[0].lower() for item in column_list]) + columns = dict(self._get_table_columns(schema, table)) # Populate dictionary for columns with column type - for k, v in dict(column_list).items(): + # NOTE: we want all columns available here because they are + # used for filtering in the where clause, not only + # the ones that are returned to the client. + for k, v in columns.items(): self.fields[k.lower()] = {"type": v} + filtered_columns = set(self.fields) + if self.properties: + filtered_columns &= {k.lower() for k in self.properties} + + # fields which are part of the output + self.filtered_fields = { + k: v for k, v in self.fields.items() if k in filtered_columns + } + return self def __exit__(self, exc_type, exc_val, exc_tb): @@ -327,6 +327,7 @@ class OracleProvider(BaseProvider): self.geom = provider_def["geom_field"] self.properties = [item.lower() for item in self.properties] self.mandatory_properties = provider_def.get("mandatory_properties") + self.extra_properties = provider_def.get("extra_properties", []) # SQL manipulator properties self.sql_manipulator = provider_def.get("sql_manipulator") @@ -496,6 +497,12 @@ class OracleProvider(BaseProvider): return f"ORDER BY {','.join(ret)}" + def _get_extra_columns_expression(self): + """Returns part of SELECT clause for extra properties""" + return "".join( + f", {e_prop}" for e_prop in self.extra_properties + ) + def _output_type_handler( self, cursor, name, default_type, size, precision, scale ): @@ -619,10 +626,10 @@ class OracleProvider(BaseProvider): # Create column list. # Uses columns field that was generated in the Connection class # or the configured columns from the Yaml file. - props = ( - db.columns - if select_properties == [] - else ", ".join([p for p in select_properties]) + props = ", ".join( + select_properties + if select_properties + else db.filtered_fields ) where_dict = self._get_where_clauses( @@ -680,14 +687,16 @@ class OracleProvider(BaseProvider): # SQL manipulation class paging_bind = {} if limit > 0: - sql_query = f"SELECT #HINTS# {props} {geom} \ + sql_query = f"SELECT #HINTS# {props} \ + {self._get_extra_columns_expression()} {geom} \ FROM {self.table} t1 #JOIN# \ {where_dict['clause']} #WHERE# \ {orderby} \ OFFSET :offset ROWS FETCH NEXT :limit ROWS ONLY" paging_bind = {"offset": offset, "limit": limit} else: - sql_query = f"SELECT #HINTS# {props} {geom} \ + sql_query = f"SELECT #HINTS# {props} \ + {self._get_extra_columns_expression()} {geom} \ FROM {self.table} t1 #JOIN# \ {where_dict['clause']} #WHERE# \ {orderby}" @@ -858,7 +867,10 @@ class OracleProvider(BaseProvider): else: geom_sql = f", t1.{self.geom}.get_geojson() AS geometry " - sql_query = f"SELECT {db.columns} {geom_sql} \ + columns = ", ".join(db.filtered_fields) + sql_query = f"SELECT {columns} \ + {self._get_extra_columns_expression()} \ + {geom_sql} \ FROM {self.table} t1 \ WHERE {self.id_field} = :in_id" @@ -971,15 +983,13 @@ class OracleProvider(BaseProvider): columns = [ col for col in columns - if col.lower() in [field.lower() for field in self.fields] + if col.lower() in db.filtered_fields ] # Flter function to get only properties who are # in the column list def filter_binds(pair): - return pair[0].lower() in [ - field.lower() for field in self.fields - ] + return pair[0].lower() in db.filtered_fields # Filter bind variables bind_variables = dict( @@ -1070,15 +1080,13 @@ class OracleProvider(BaseProvider): columns = [ col for col in columns - if col.lower() in [field.lower() for field in self.fields] + if col.lower() in db.filtered_fields ] # Flter function to get only properties who are # in the column list def filter_binds(pair): - return pair[0].lower() in [ - field.lower() for field in self.fields - ] + return pair[0].lower() in db.filtered_fields # Filter bind variables bind_variables = dict( diff --git a/tests/test_oracle_provider.py b/tests/test_oracle_provider.py index 7488231..d807552 100644 --- a/tests/test_oracle_provider.py +++ b/tests/test_oracle_provider.py @@ -32,6 +32,7 @@ import os import pytest +from pygeoapi.provider.base import ProviderInvalidQueryError from pygeoapi.provider.oracle import OracleProvider USERNAME = os.environ.get("PYGEOAPI_ORACLE_USER", "geo_test") @@ -202,6 +203,14 @@ def config_properties(config): } +@pytest.fixture() +def config_extra_properties(config): + return { + **config, + "extra_properties": ["'Here the name is ' || name || '!' as tooltip"], + } + + @pytest.fixture() def create_geojson(): return { @@ -332,15 +341,18 @@ def test_get_fields_properties(config_properties): Test get_fields with subset of columns. Test of property configuration. """ + # NOTE: properties does not influence fields because + # the fields are also used for filtering expected_fields = { "id": {"type": "NUMBER"}, "name": {"type": "VARCHAR2"}, "wiki_link": {"type": "VARCHAR2"}, + "area": {"type": "NUMBER"}, + "volume": {"type": "NUMBER"}, } provider = OracleProvider(config_properties) provided_fields = provider.get_fields() - print(provided_fields) assert provided_fields == expected_fields assert provider.fields == expected_fields @@ -356,6 +368,15 @@ def test_query_with_property_filter(config): assert features[0].get("id") == 12 +def test_query_with_extra_properties(config_extra_properties): + p = OracleProvider(config_extra_properties) + + feature_collection = p.query(properties=[("name", "Aral Sea")]) + features = feature_collection.get("features") + + assert features[0]["properties"]["tooltip"] == "Here the name is Aral Sea!" + + def test_query_bbox(config): """Test query with a specified bounding box""" p = OracleProvider(config) @@ -407,6 +428,17 @@ def test_get(config): assert result.get("next") == 6 +def test_get_with_extra_properties(config_extra_properties): + """Test simple get""" + p = OracleProvider(config_extra_properties) + result = p.get(5) + + assert ( + result["properties"]["tooltip"] == + "Here the name is L. Erie!" + ) + + def test_create(config, create_geojson): """Test simple create""" p = OracleProvider(config) @@ -557,3 +589,30 @@ def test_create_point(config, create_point_geojson): data = p.get(28) assert data.get("geometry").get("type") == "Point" + + +def test_query_can_mandate_properties_which_are_not_returned(config): + config = { + **config, + # 'name' has to be filtered, but only 'wiki_link' is returned + "properties": ["id", "wiki_link"], + "mandatory_properties": ["name"] + } + + p = OracleProvider(config) + result = p.query(properties=[("name", "Aral Sea")]) + + (feature,) = result['features'] + # id is handled separately, so only wiki link and not name must be here + assert feature['properties'].keys() == {"wiki_link"} + + +def test_query_mandatory_properties_must_be_specified(config): + config = { + **config, + "mandatory_properties": ["name"] + } + + p = OracleProvider(config) + with pytest.raises(ProviderInvalidQueryError): + p.query(properties=[("id", "123")])