From b7e51db7e286c86d1fef35669c3fdb65833fa066 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:32:47 -0500 Subject: [PATCH 1/9] #82 extract name checks into _is_invalid_name function --- fudgeo/util.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fudgeo/util.py b/fudgeo/util.py index 0a8c0c4..f429dde 100644 --- a/fudgeo/util.py +++ b/fudgeo/util.py @@ -19,12 +19,20 @@ def escape_name(name: str) -> str: """ Escape Name """ - if name.upper() in KEYWORDS or not NAME_MATCHER(name): + if _is_invalid_name(name): name = f'"{name}"' return name # End escape_name function +def _is_invalid_name(name: str) -> bool: + """ + Is invalid name + """ + return name.upper() in KEYWORDS or not NAME_MATCHER(name) +# End _is_invalid_name function + + def now() -> str: """ Formatted Now From af112100d0df6bf75b5b494be19096e72995fff0 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:32:57 -0500 Subject: [PATCH 2/9] #82 add check_geometry_name --- fudgeo/util.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fudgeo/util.py b/fudgeo/util.py index f429dde..8598a21 100644 --- a/fudgeo/util.py +++ b/fudgeo/util.py @@ -33,6 +33,16 @@ def _is_invalid_name(name: str) -> bool: # End _is_invalid_name function +def check_geometry_name(name: str) -> str: + """ + Check geometry name + """ + if _is_invalid_name(name): + raise ValueError(f'Invalid field name for geometry column: {name}') + return name +# End check_geometry_name function + + def now() -> str: """ Formatted Now From 8c98b956ae068ebac1ed76b662f7e250e2976440 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:34:05 -0500 Subject: [PATCH 3/9] #82 rename variable and reflow --- fudgeo/geopkg.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fudgeo/geopkg.py b/fudgeo/geopkg.py index 267c502..797eec3 100644 --- a/fudgeo/geopkg.py +++ b/fudgeo/geopkg.py @@ -575,12 +575,13 @@ def create(cls, geopackage: GeoPackage, name: str, shape_type: str, escaped_name = escape_name(name) has_contents = has_ogr_contents(conn) if overwrite: - geom_name = cls._find_geometry_column_name(geopackage, name) - cls._drop(conn=conn, sql=REMOVE_FEATURE_CLASS, - name=name, escaped_name=escaped_name, - geom_name=geom_name, delete_ogr_contents=has_contents, - delete_metadata=geopackage.is_metadata_enabled, - delete_schema=geopackage.is_schema_enabled) + current_name = cls._find_geometry_column_name(geopackage, name) + cls._drop( + conn=conn, sql=REMOVE_FEATURE_CLASS, name=name, + escaped_name=escaped_name, geom_name=current_name, + delete_ogr_contents=has_contents, + delete_metadata=geopackage.is_metadata_enabled, + delete_schema=geopackage.is_schema_enabled) conn.execute(CREATE_FEATURE_TABLE.format( name=escaped_name, feature_type=shape_type, other_fields=cols)) if not geopackage.check_srs_exists(srs.srs_id): From d1630136bfedf1f1aa5c838b3c96aa1b87103641 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:34:28 -0500 Subject: [PATCH 4/9] #82 add argument for geometry name and check it --- fudgeo/geopkg.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fudgeo/geopkg.py b/fudgeo/geopkg.py index 797eec3..e516b1f 100644 --- a/fudgeo/geopkg.py +++ b/fudgeo/geopkg.py @@ -35,7 +35,7 @@ SELECT_COUNT, SELECT_EXTENT, SELECT_GEOMETRY_COLUMN, SELECT_GEOMETRY_TYPE, SELECT_HAS_ZM, SELECT_PRIMARY_KEY, SELECT_SRS, SELECT_TABLES_BY_TYPE, TABLE_EXISTS, UPDATE_EXTENT) -from fudgeo.util import convert_datetime, escape_name, now +from fudgeo.util import check_geometry_name, convert_datetime, escape_name, now if TYPE_CHECKING: # pragma: no cover @@ -234,7 +234,8 @@ def create_feature_class(self, name: str, srs: 'SpatialReferenceSystem', z_enabled: bool = False, m_enabled: bool = False, fields: FIELDS = (), description: str = '', overwrite: bool = False, - spatial_index: bool = False) -> 'FeatureClass': + spatial_index: bool = False, + geom_name: str = SHAPE) -> 'FeatureClass': """ Creates a feature class in the GeoPackage per the options given. """ @@ -243,7 +244,7 @@ def create_feature_class(self, name: str, srs: 'SpatialReferenceSystem', geopackage=self, name=name, shape_type=shape_type, srs=srs, z_enabled=z_enabled, m_enabled=m_enabled, fields=fields, description=description, overwrite=overwrite, - spatial_index=spatial_index) + spatial_index=spatial_index, geom_name=geom_name) # End create_feature_class method def create_table(self, name: str, fields: FIELDS = (), @@ -566,13 +567,15 @@ def create(cls, geopackage: GeoPackage, name: str, shape_type: str, srs: 'SpatialReferenceSystem', z_enabled: bool = False, m_enabled: bool = False, fields: FIELDS = (), description: str = '', overwrite: bool = False, - spatial_index: bool = False) -> 'FeatureClass': + spatial_index: bool = False, + geom_name: str = SHAPE) -> 'FeatureClass': """ Create Feature Class """ cols = cls._column_names(fields) with geopackage.connection as conn: escaped_name = escape_name(name) + geom_name = check_geometry_name(geom_name) has_contents = has_ogr_contents(conn) if overwrite: current_name = cls._find_geometry_column_name(geopackage, name) From 3602df57eb944b2aba963c1779ce9592038dd5db Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:34:56 -0500 Subject: [PATCH 5/9] #82 use the geometry name --- fudgeo/geopkg.py | 5 +++-- fudgeo/sql.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fudgeo/geopkg.py b/fudgeo/geopkg.py index e516b1f..bc8301e 100644 --- a/fudgeo/geopkg.py +++ b/fudgeo/geopkg.py @@ -586,13 +586,14 @@ def create(cls, geopackage: GeoPackage, name: str, shape_type: str, delete_metadata=geopackage.is_metadata_enabled, delete_schema=geopackage.is_schema_enabled) conn.execute(CREATE_FEATURE_TABLE.format( - name=escaped_name, feature_type=shape_type, other_fields=cols)) + name=escaped_name, geom_name=geom_name, + feature_type=shape_type, other_fields=cols)) if not geopackage.check_srs_exists(srs.srs_id): conn.execute(INSERT_GPKG_SRS, srs.as_record()) conn.execute(INSERT_GPKG_CONTENTS_SHORT, ( name, DataType.features, name, description, now(), srs.srs_id)) conn.execute(INSERT_GPKG_GEOM_COL, - (name, SHAPE, shape_type, srs.srs_id, + (name, geom_name, shape_type, srs.srs_id, int(z_enabled), int(m_enabled))) if has_contents: add_ogr_contents(conn, name=name, escaped_name=escaped_name) diff --git a/fudgeo/sql.py b/fudgeo/sql.py index 5af76ad..d4e27aa 100644 --- a/fudgeo/sql.py +++ b/fudgeo/sql.py @@ -132,7 +132,7 @@ CREATE_FEATURE_TABLE: str = """ CREATE TABLE {name} ( fid INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - SHAPE {feature_type}{other_fields}) + {geom_name} {feature_type}{other_fields}) """ From 419e94435c0d6d1ebcda5c2b7b58af3834bf1bf6 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 11:37:27 -0500 Subject: [PATCH 6/9] #82 use geometry column name property --- tests/extension/test_metadata.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/extension/test_metadata.py b/tests/extension/test_metadata.py index 2a6ad8f..d3b546c 100644 --- a/tests/extension/test_metadata.py +++ b/tests/extension/test_metadata.py @@ -74,10 +74,10 @@ def example1(tmp_path, random_utm_coordinates) -> GeoPackage: records.append((line, 12.3)) with pkg.connection as conn: conn.executemany(f""" - INSERT INTO {roads.name} (SHAPE, overhead_clearance) + INSERT INTO {roads.name} ({roads.geometry_column_name}, overhead_clearance) VALUES (?, ?)""", records) conn.executemany(f""" - INSERT INTO {bridge.name} (SHAPE, overhead_clearance) + INSERT INTO {bridge.name} ({bridge.geometry_column_name}, overhead_clearance) VALUES (?, ?)""", records) metadata = pkg.metadata uri = 'https://www.isotc211.org/2005/gmd' @@ -117,7 +117,7 @@ def example2(tmp_path, random_utm_coordinates) -> GeoPackage: i // 100, easting + northing)) with pkg.connection as conn: conn.executemany(f""" - INSERT INTO {poi.name} (SHAPE, category, point) + INSERT INTO {poi.name} ({poi.geometry_column_name}, category, point) VALUES (?, ?, ?)""", records) metadata = pkg.metadata uri = 'https://schemas.opengis.net/iso/19139/' From d39e5c1a34ff4e8d10ec71c23dc67a3973f90760 Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 12:26:51 -0500 Subject: [PATCH 7/9] #82 update tests to use geometry column name property --- tests/test_geopkg.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_geopkg.py b/tests/test_geopkg.py index fc90daa..f4c1760 100644 --- a/tests/test_geopkg.py +++ b/tests/test_geopkg.py @@ -12,20 +12,20 @@ from pytest import mark, raises +from fudgeo.constant import SHAPE from fudgeo.enumeration import GeometryType, SQLFieldType from fudgeo.geometry import ( LineString, LineStringM, LineStringZ, LineStringZM, MultiLineString, MultiPoint, MultiPolygon, Point, Polygon, PolygonM) from fudgeo.geopkg import ( FeatureClass, Field, GeoPackage, SpatialReferenceSystem, Table) -from fudgeo.constant import SHAPE from fudgeo.extension.ogr import has_ogr_contents from fudgeo.sql import SELECT_SRS from tests.crs import WGS_1984_UTM_Zone_23N # noinspection SqlNoDataSourceInspection INSERT_ROWS = """ - INSERT INTO {} (SHAPE, "int.fld", text_fld, test_fld_size, test_bool) + INSERT INTO {} ({}, "int.fld", text_fld, test_fld_size, test_bool) VALUES (?, ?, ?, ?, ?) """ # noinspection SqlNoDataSourceInspection @@ -33,7 +33,7 @@ # noinspection SqlNoDataSourceInspection,SqlResolve SELECT_RTREE = """SELECT * FROM rtree_{0}_{1} ORDER BY 1""" # noinspection SqlNoDataSourceInspection -INSERT_SHAPE = """INSERT INTO {} (SHAPE) VALUES (?)""" +INSERT_SHAPE = """INSERT INTO {} ({}) VALUES (?)""" def random_points_and_attrs(count, srs_id): @@ -372,7 +372,7 @@ def test_insert_point_rows(setup_geopackage, name, add_index): count = 10000 rows = random_points_and_attrs(count, srs.srs_id) with gpkg.connection as conn: - conn.executemany(INSERT_ROWS.format(fc.escaped_name), rows) + conn.executemany(INSERT_ROWS.format(fc.escaped_name, fc.geometry_column_name), rows) assert fc.count == count # noinspection SqlNoDataSourceInspection cursor = fc.select(limit=10) @@ -385,7 +385,7 @@ def test_insert_point_rows(setup_geopackage, name, add_index): if not add_index: assert fc.add_spatial_index() cursor = gpkg.connection.execute(f""" - SELECT COUNT(1) AS C FROM "rtree_{name}_{SHAPE}" + SELECT COUNT(1) AS C FROM "rtree_{name}_{fc.geometry_column_name}" """) assert cursor.fetchone() == (count,) # End test_insert_point_rows function @@ -393,7 +393,7 @@ def test_insert_point_rows(setup_geopackage, name, add_index): def _insert_shape_and_fetch(gpkg, geom, fc): with gpkg.connection as conn: - conn.execute(INSERT_SHAPE.format(fc.escaped_name), (geom,)) + conn.execute(INSERT_SHAPE.format(fc.escaped_name, fc.geometry_column_name), (geom,)) cursor = fc.select(include_primary=True) return cursor.fetchall() @@ -758,7 +758,7 @@ def test_escaped_columns(setup_geopackage): with fc.geopackage.connection as conn: # noinspection SqlNoDataSourceInspection conn.executemany( - f"""INSERT INTO {fc.name} (SHAPE, {select.escaped_name}, + f"""INSERT INTO {fc.name} ({fc.geometry_column_name}, {select.escaped_name}, {union.escaped_name}, {all_.escaped_name}, {example_dot.escaped_name}) VALUES (?, ?, ?, ?, ?)""", rows) @@ -792,7 +792,7 @@ def test_escaped_table(setup_geopackage): with fc.geopackage.connection as conn: # noinspection SqlNoDataSourceInspection conn.executemany( - f"""INSERT INTO {fc.escaped_name} (SHAPE, {select.escaped_name}, + f"""INSERT INTO {fc.escaped_name} ({fc.geometry_column_name}, {select.escaped_name}, {union.escaped_name}, {all_.escaped_name}) VALUES (?, ?, ?, ?)""", rows) # noinspection SqlNoDataSourceInspection From e8e33c8fae8618fe0c2deb23b94a30d940d5722b Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 12:27:08 -0500 Subject: [PATCH 8/9] #82 better is to use the escaped name --- tests/test_geopkg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_geopkg.py b/tests/test_geopkg.py index f4c1760..a6dd727 100644 --- a/tests/test_geopkg.py +++ b/tests/test_geopkg.py @@ -639,7 +639,7 @@ def test_insert_and_update_lines_zm(setup_geopackage, add_index): gpkg.connection.execute(f""" UPDATE {fc.escaped_name} SET {fc.geometry_column_name} = ? - WHERE {fc.primary_key_field.name} = ?""", (geom, primary)) + WHERE {fc.primary_key_field.escaped_name} = ?""", (geom, primary)) cursor = fc.select(include_primary=True) result = cursor.fetchall() From 85ad24d0cdebd707870dddf29ed9a64b31df56fd Mon Sep 17 00:00:00 2001 From: Jason Humber Date: Sat, 19 Oct 2024 12:27:22 -0500 Subject: [PATCH 9/9] #82 test for non-standard geom name --- tests/test_geopkg.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/test_geopkg.py b/tests/test_geopkg.py index a6dd727..9d60a7f 100644 --- a/tests/test_geopkg.py +++ b/tests/test_geopkg.py @@ -651,6 +651,46 @@ def test_insert_and_update_lines_zm(setup_geopackage, add_index): # End test_insert_and_update_lines_zm function +@mark.parametrize('geom_name, add_index, is_error', [ + ('geom', False, False), + ('geom', True, False), + ('GeOmEtRy', False, False), + ('GeOmEtRy', True, False), + ('aa bb', False, True), + ('cc-dd', True, True), +]) +def test_non_standard_geom_name(setup_geopackage, geom_name, add_index, is_error): + """ + Test non-standard geometry column name + """ + _, gpkg, srs, fields = setup_geopackage + tbl = 'SELECT' + kwargs = dict( + name=tbl, srs=srs, fields=fields, + shape_type=GeometryType.linestring, + z_enabled=True, m_enabled=True, spatial_index=add_index, + geom_name=geom_name) + if is_error: + with raises(ValueError): + gpkg.create_feature_class(**kwargs) + return + fc = gpkg.create_feature_class(**kwargs) + assert fc.has_spatial_index is add_index + assert fc.geometry_column_name == geom_name + if add_index: + assert fc.spatial_index_name == f'rtree_{tbl}_{geom_name}' + coords = [(300000, 1, 10, 0), (300000, 4000000, 20, 1000), + (700000, 4000000, 30, 2000), (700000, 1, 40, 3000)] + geom = LineStringZM(coords, srs_id=srs.srs_id) + result = _insert_shape_and_fetch(gpkg, geom, fc) + assert len(result) == 1 + line, primary = result[0] + assert isinstance(line, LineStringZM) + assert line == geom + assert primary == 1 +# End test_non_standard_geom_name function + + @mark.parametrize('add_index', [ False, True ])