From 6cefc2bc97d67e8d86b1d6f8c48b194910e7f6c8 Mon Sep 17 00:00:00 2001 From: frauch Date: Fri, 27 Feb 2026 11:18:27 +0100 Subject: [PATCH 1/4] feat: Introduce MapbenderDataSourceExtension for PostGIS type mappings in Doctrine - Added MapbenderDataSourceExtension to automatically register PostGIS types (geometry, geography, box2d, box3d) as string in Doctrine's DBAL configuration. - Removed deprecated eval-based event system, replacing it with safer alternatives using Symfony EventDispatcher and PostgreSQL triggers. - Updated DataItem and Feature entities by removing author annotations. - Deleted unused BaseXmlLoaderExtension and associated XML service configurations. - Removed legacy Twig templates for form fields and associated tests. - Updated services configuration to remove references to the old event processor. --- .../Resources/public/MbDataManager.js | 73 ++- .../DataSourceBundle/CHANGELOG-REFACTORING.md | 304 ++++++++++ .../Component/DataRepository.php | 395 ------------ .../DataSourceBundle/Component/DataStore.php | 572 +++++++++++++++--- .../Component/DataStoreService.php | 41 -- .../Component/Drivers/DoctrineBaseDriver.php | 128 ---- .../Drivers/Interfaces/Geographic.php | 56 -- .../Component/Drivers/Oracle.php | 125 ---- .../Component/Drivers/PostgreSQL.php | 138 ----- .../Component/Drivers/SQLite.php | 32 - .../Component/EventAwareDataRepository.php | 126 ---- .../Component/EventProcessor.php | 87 --- .../DataSourceBundle/Component/Expression.php | 30 - .../Component/Factory/DataStoreFactory.php | 47 +- .../Component/Factory/FeatureTypeFactory.php | 17 +- .../Component/FeatureQueryBuilder.php | 100 --- .../Component/FeatureType.php | 480 +++++++++++---- .../Component/FeatureTypeService.php | 22 - .../Component/Meta/Column.php | 90 --- .../Component/Meta/TableMeta.php | 98 --- .../PropertyAdapter/DiscreteColumnAdapter.php | 78 +++ .../PropertyAdapter/JsonColumnAdapter.php | 55 ++ .../PropertyAdapterInterface.php | 41 ++ .../DataSourceBundle/DOCUMENTATION.md | 510 ++++++++++++++++ .../MapbenderDataSourceExtension.php | 145 +++++ .../DataSourceBundle/EVENT-MIGRATION.md | 427 +++++++++++++ .../DataSourceBundle/Entity/DataItem.php | 3 - .../DataSourceBundle/Entity/Feature.php | 3 - .../Extension/BaseXmlLoaderExtension.php | 41 -- .../MapbenderDataSourceBundle.php | 15 +- .../Resources/config/services.xml | 9 - .../Resources/views/fields.html.twig | 239 -------- .../Tests/FeatureTypeTest.php | 62 -- .../DataSourceBundle/Tests/FeaturesTest.php | 97 --- 34 files changed, 2510 insertions(+), 2176 deletions(-) create mode 100644 src/Mapbender/DataSourceBundle/CHANGELOG-REFACTORING.md delete mode 100644 src/Mapbender/DataSourceBundle/Component/DataRepository.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/DataStoreService.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Drivers/DoctrineBaseDriver.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Drivers/Interfaces/Geographic.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Drivers/Oracle.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Drivers/PostgreSQL.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Drivers/SQLite.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/EventAwareDataRepository.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/EventProcessor.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Expression.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/FeatureQueryBuilder.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/FeatureTypeService.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Meta/Column.php delete mode 100644 src/Mapbender/DataSourceBundle/Component/Meta/TableMeta.php create mode 100644 src/Mapbender/DataSourceBundle/Component/PropertyAdapter/DiscreteColumnAdapter.php create mode 100644 src/Mapbender/DataSourceBundle/Component/PropertyAdapter/JsonColumnAdapter.php create mode 100644 src/Mapbender/DataSourceBundle/Component/PropertyAdapter/PropertyAdapterInterface.php create mode 100644 src/Mapbender/DataSourceBundle/DOCUMENTATION.md create mode 100644 src/Mapbender/DataSourceBundle/DependencyInjection/MapbenderDataSourceExtension.php create mode 100644 src/Mapbender/DataSourceBundle/EVENT-MIGRATION.md delete mode 100644 src/Mapbender/DataSourceBundle/Extension/BaseXmlLoaderExtension.php delete mode 100644 src/Mapbender/DataSourceBundle/Resources/views/fields.html.twig delete mode 100644 src/Mapbender/DataSourceBundle/Tests/FeatureTypeTest.php delete mode 100644 src/Mapbender/DataSourceBundle/Tests/FeaturesTest.php diff --git a/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js b/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js index db231850..a250f7ee 100644 --- a/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js +++ b/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js @@ -1,5 +1,20 @@ (function() { + const DEPRECATED_EVENT_HOOKS = [ + 'onBeforeSave', 'onAfterSave', 'onBeforeUpdate', 'onAfterUpdate', + 'onBeforeInsert', 'onAfterInsert', 'onBeforeRemove', 'onAfterRemove', + 'onBeforeSearch', 'onAfterSearch' + ]; + + const DEPRECATED_EVENTS_WARNING = + 'The eval-based event system (EventProcessor) was removed because it allowed\n' + + 'arbitrary PHP code execution from YAML/database configuration — a significant\n' + + 'security risk. Any logic in these hooks is silently ignored.\n\n' + + 'MIGRATION: Use database triggers, Symfony EventDispatcher, or DataStore\n' + + 'subclass overrides instead. See EVENT-MIGRATION.md for a complete guide.\n\n' + + 'To suppress this warning, remove the "events" key from the dataStore/featureType\n' + + 'configuration of the affected schemas.'; + class MbDataManager extends MapbenderElement { constructor(configuration, $element) { super(configuration, $element); @@ -56,6 +71,7 @@ } } this.tableRenderer = this._createTableRenderer(); + this._warnDeprecatedEvents(); this._initializeEvents(); this._afterCreate(); } @@ -81,6 +97,9 @@ .then(() => this.updateSchemaSelector_()) ; this.onGrantsLoadStarted(); +/** + * @private + */ } onGrantsLoadStarted() { @@ -165,6 +184,44 @@ this._start(); } + /** + * Checks all schema configurations for deprecated server-side event hooks + * (onBeforeSave, onAfterSave, etc.) and displays a prominent console warning. + * These eval-based PHP events were removed for security reasons and are silently + * ignored by the backend. See EVENT-MIGRATION.md for migration instructions. + * @private + */ + _warnDeprecatedEvents() { + const schemaNames = Object.keys(this.options.schemes); + const warnings = []; + for (const schemaName of schemaNames) { + const schema = this.options.schemes[schemaName]; + if (schema.combine) continue; + const dsConfig = this._getDataStoreFromSchema(schema); + if (dsConfig && dsConfig.events && typeof dsConfig.events === 'object') { + const configuredHooks = Object.keys(dsConfig.events).filter(function(k) { + return DEPRECATED_EVENT_HOOKS.indexOf(k) !== -1 && dsConfig.events[k]; + }); + if (configuredHooks.length) { + warnings.push({schema: schemaName, hooks: configuredHooks}); + } + } + } + if (warnings.length) { + const elementTitle = this.$element.attr('data-title') || this.$element.attr('id'); + console.warn( + '%c⚠ DEPRECATED EVENT HOOKS DETECTED — ' + elementTitle + ' %c\n\n' + + 'The following server-side event hooks are configured but will NOT be executed:\n\n' + + warnings.map(function(w) { + return ' Schema "' + w.schema + '": ' + w.hooks.join(', '); + }).join('\n') + '\n\n' + + DEPRECATED_EVENTS_WARNING, + 'background: #ff6600; color: white; font-size: 14px; font-weight: bold; padding: 4px 8px;', + 'color: #cc5500; font-size: 12px;' + ); + } + } + /** * Loads and displays data from initially selected schema. * Unraveled from _create for child classes need to act after our initialization, @@ -383,6 +440,7 @@ * @param {(String|null)} originalId * @private */ +/** @var {DataManagagerSaveEventData} eventData */ _saveEvent(schema, dataItem, originalId) { const eventData = { item: dataItem, @@ -589,7 +647,7 @@ const $scope = $('.popupContent', this.$element); const saved = widget._submitFormData(schema, $scope, dataItem); if (saved) { - saved.then(function() { + saved.then(function () { widget._closeCurrentPopup(); }); } @@ -601,12 +659,22 @@ text: Mapbender.trans('mb.actions.delete'), title: Mapbender.trans('mb.data-manager.actions.delete_tooltip'), 'class': 'btn btn-danger', - click: function() { + click: function () { widget._closeCurrentPopup(); widget.removeData(schema, dataItem); } }); } + var closeText = buttons.length && 'mb.actions.cancel' || 'mb.actions.close'; + var closeTooltip = buttons.length && 'mb.data-manager.actions.cancel_tooltip' || 'mb.data-manager.actions.close_tooltip'; + buttons.push({ + text: Mapbender.trans(closeText), + title: Mapbender.trans(closeTooltip), + 'class': 'btn btn-light', + click: function () { + widget._cancelForm(schema, dataItem); + } + }); return buttons; } @@ -820,7 +888,6 @@ $loadingIndicator.css({ opacity: 0 }); }); } - jqXhr.fail(this._onAjaxError); return jqXhr; } diff --git a/src/Mapbender/DataSourceBundle/CHANGELOG-REFACTORING.md b/src/Mapbender/DataSourceBundle/CHANGELOG-REFACTORING.md new file mode 100644 index 00000000..f643c98d --- /dev/null +++ b/src/Mapbender/DataSourceBundle/CHANGELOG-REFACTORING.md @@ -0,0 +1,304 @@ +# DataSourceBundle Refactoring — Detailed Change Log + +## Summary + +The DataSourceBundle was refactored from a multi-database-driver architecture +with eval-based event processing into a streamlined, PostgreSQL/PostGIS-focused +design using a **PropertyAdapter** strategy pattern for flexible property storage. + +The new architecture is simpler, more secure, and forward-compatible with +Symfony's deprecation of `getListTableColumnsSQL` and full-container injection. + +--- + +## Architecture Changes + +### Before (Old Architecture) + +``` +DataRepository (base CRUD, driver-abstracted) +├── driverFactory() → DoctrineBaseDriver subclass +├── EventAwareDataRepository (eval-based lifecycle hooks) +├── FeatureQueryBuilder (custom QueryBuilder for geometry SQL) +└── Drivers/ + ├── DoctrineBaseDriver (abstract: insert/update/loadTableMeta) + ├── PostgreSQL (implements Geographic interface) + ├── Oracle (implements Geographic interface) + ├── SQLite (no geometry support) + └── Interfaces/Geographic (geometry SQL contract) + +DataStore (extended DataRepository — container-aware) +FeatureType (extended DataStore — geometry via FeatureQueryBuilder + Geographic driver) + +DataStoreService / FeatureTypeService (deprecated, full-container injection) +RepositoryRegistry (Symfony 4+ replacement for Service classes) + +Factory/ + DataStoreFactory → new DataStore(connection, tokenStorage, eventProcessor, config) + FeatureTypeFactory → new FeatureType(...) + +Meta/ + TableMeta (column metadata for insert/update preparation) + Column (per-column metadata: nullable, hasDefault, isNumeric, geometryType, srid) +``` + +### After (New Architecture) + +``` +DataStore (standalone CRUD, no driver abstraction) +├── PropertyAdapter (strategy for property storage) +│ ├── DiscreteColumnAdapter (one column per property — classic mode) +│ └── JsonColumnAdapter (all props in JSONB column — new mode) +├── Direct SQL via DBAL QueryBuilder +└── Geometry: none (non-spatial) + +FeatureType (extends DataStore — geometry via inline PostGIS SQL) +├── Inherits PropertyAdapter from DataStore +├── ST_AsEWKT / ST_GeomFromEWKT / ST_Transform / ST_MakeValid +├── ST_Intersects for spatial filtering +├── ST_Multi for auto-promotion to multi-geometry +└── SRID auto-detection from geometry_columns + +RepositoryRegistry (unchanged — factory cache layer) + +Factory/ + DataStoreFactory → new DataStore(connection, tokenStorage, config) + FeatureTypeFactory → new FeatureType(connection, tokenStorage, config) +``` + +--- + +## Files Removed (16 files, 5 directories) + +### Component Classes + +| File | Lines | Reason | +|------|-------|--------| +| `Component/DataRepository.php` | 388 | Replaced by `DataStore`. Logic merged into new DataStore with PropertyAdapter pattern. | +| `Component/EventAwareDataRepository.php` | ~130 | Eval-based event hooks (`onBeforeSave`, `onAfterUpdate`, etc.) removed for security. | +| `Component/EventProcessor.php` | ~100 | Used `eval()` to run user-provided PHP expressions — security risk. Not referenced by new code. | +| `Component/Expression.php` | ~30 | Trivial SQL expression wrapper used only by `DoctrineBaseDriver`. Geometry SQL now built inline in FeatureType. | +| `Component/FeatureQueryBuilder.php` | ~105 | Custom QueryBuilder that delegated geometry SQL to Geographic drivers. Logic now inline in `FeatureType::createSelectQueryBuilder()`. | +| `Component/DataStoreService.php` | ~45 | Deprecated Symfony 3 service (full container injection). Already replaced by `RepositoryRegistry`. | +| `Component/FeatureTypeService.php` | ~25 | Deprecated Symfony 3 service. Extended `DataStoreService`. | + +### Driver System + +| File | Lines | Reason | +|------|-------|--------| +| `Component/Drivers/DoctrineBaseDriver.php` | ~130 | Abstract base for multi-database support. Handled insert/update SQL generation and table metadata loading. Replaced by direct DBAL calls in DataStore. | +| `Component/Drivers/PostgreSQL.php` | ~135 | PostgreSQL/PostGIS driver implementing `Geographic` interface. Geometry SQL (`ST_AsEwkt`, `ST_Transform`, etc.) now lives directly in `FeatureType`. | +| `Component/Drivers/Oracle.php` | ~126 | Oracle spatial driver (`SDO_UTIL`, `SDO_CS`). Oracle is no longer supported. | +| `Component/Drivers/SQLite.php` | ~32 | SQLite driver (no geometry). SQLite is no longer a supported target. | +| `Component/Drivers/Interfaces/Geographic.php` | ~55 | Interface contract for spatial SQL methods. No longer needed — only PostgreSQL is supported. | + +### Metadata System + +| File | Lines | Reason | +|------|-------|--------| +| `Component/Meta/Column.php` | ~90 | Per-column metadata (nullable, hasDefault, isNumeric, geometryType, srid). Replaced by DBAL's `SchemaManager::listTableColumns()` in `DiscreteColumnAdapter`. | +| `Component/Meta/TableMeta.php` | ~110 | Table-level metadata used to prepare insert/update data (fill defaults for non-nullable columns, handle empty numeric strings). Replaced by simpler logic in DataStore/PropertyAdapter. | + +### Other + +| File | Lines | Reason | +|------|-------|--------| +| `Extension/BaseXmlLoaderExtension.php` | ~45 | Deprecated Symfony DI extension base class. Bundle now loads services via `Bundle::build()` directly. | +| `Resources/views/fields.html.twig` | ~240 | Bootstrap form theme. Not referenced by any PHP or Twig code — likely a leftover from an earlier version. | + +### Removed Directories + +- `Component/Drivers/Interfaces/` +- `Component/Drivers/` +- `Component/Meta/` +- `Extension/` +- `Resources/views/` + +--- + +## Files Retained (Active Code) + +### Core Classes + +| File | Status | Notes | +|------|--------|-------| +| `Component/DataStore.php` | **New** | Non-spatial repository. Replaces `DataRepository` with PropertyAdapter pattern. Supports `columns` and `json` storage modes. | +| `Component/FeatureType.php` | **Rewritten** | Spatial repository extending DataStore. Inline PostGIS SQL replaces driver abstraction. | +| `Component/RepositoryRegistry.php` | **Unchanged** | Factory cache layer for named repositories. | +| `Component/Factory/DataStoreFactory.php` | **Simplified** | Removed `EventProcessor` dependency. Now passes config array directly to DataStore constructor. | +| `Component/Factory/FeatureTypeFactory.php` | **Simplified** | Same simplification as DataStoreFactory. | + +### PropertyAdapter (New) + +| File | Status | Notes | +|------|--------|-------| +| `Component/PropertyAdapter/PropertyAdapterInterface.php` | **New** | Contract: `getSelectColumns()`, `extractProperties()`, `prepareStorageData()`. | +| `Component/PropertyAdapter/DiscreteColumnAdapter.php` | **New** | Classic mode: each property = one table column. Auto-detects columns via `SchemaManager`. | +| `Component/PropertyAdapter/JsonColumnAdapter.php` | **New** | New mode: all properties in a single JSONB column. Handles JSON encode/decode transparently. | + +### Entities + +| File | Status | Notes | +|------|--------|-------| +| `Entity/DataItem.php` | **Unchanged** | Non-spatial row entity with ArrayAccess. | +| `Entity/Feature.php` | **Unchanged** | Spatial entity extending DataItem. EWKT geometry handling. | + +### Utilities & Config + +| File | Status | Notes | +|------|--------|-------| +| `Utils/WktUtility.php` | **Unchanged** | WKT/EWKT parsing helpers. | +| `MapbenderDataSourceBundle.php` | **Unchanged** | Bundle class loading services.xml. | +| `Resources/config/services.xml` | **Simplified** | Removed `EventProcessor` service. Only defines `DataStoreFactory` and `FeatureTypeFactory`. | +| `DOCUMENTATION.md` | **New** | Comprehensive documentation for the refactored bundle. | + +### Tests + +| File | Status | Notes | +|------|--------|-------| +| `Tests/FeatureTypeTest.php` | **Unchanged** | Unit tests for Feature entity geometry parsing. Still valid. | +| `Tests/FeaturesTest.php` | **Updated** | Integration tests for FeatureType. Updated to use `FeatureTypeFactory` instead of old service. | + +--- + +## API Changes + +### DataStore Constructor + +**Before:** +```php +new DataStore($connection, $tokenStorage, $eventProcessor, $config) +``` + +**After (backward-compatible):** +```php +new DataStore($connection, $tokenStorage, $config) +// OR (for backward compat — EventProcessor ignored): +new DataStore($connection, $tokenStorage, $eventProcessor, $config) +``` + +### FeatureType Constructor + +Same backward-compatible signature change as DataStore. + +### DataStoreFactory.fromConfig() + +**Before:** +```php +new DataStore($connection, $tokenStorage, $eventProcessor, $config) +``` + +**After:** +```php +new DataStore($connection, $tokenStorage, $config) +``` + +### Removed Public Methods + +These methods existed on the old `DataRepository` but are not on the new `DataStore`: + +| Method | Replacement | +|--------|-------------| +| `DataRepository::getTableMetaData()` | Not needed — column handling via PropertyAdapter | +| `DataRepository::driverFactory()` | Removed — no driver abstraction | +| `DataRepository::idToIdentifier()` | Inlined | +| `DataRepository::getByIds()` | Removed (unused by consumers) | +| `DataRepository::stripQuotes()` | Removed (was utility, used only internally) | +| `DataRepository::stripQuotesFromArray()` | Removed | + +### New Public Methods + +| Method | Description | +|--------|-------------| +| `DataStore::getPropertyAdapter()` | Returns the active PropertyAdapter | +| `DataStore::getPlatformName()` | Returns database platform name | +| `DataStore::save($itemOrData)` | Unified insert/update (existed before, now cleaner) | +| `DataStore::insert($itemOrData)` | Explicit insert | +| `DataStore::update($itemOrData)` | Explicit update | +| `FeatureType::getGeomField()` | Returns geometry column name | +| `FeatureType::getSrid()` | Returns storage SRID (auto-detected or configured) | + +### Kept Public Methods (Backward Compatible) + +These methods have the same signature and behavior: + +- `getConnection()`, `getTableName()`, `getUniqueId()`, `getFields()` +- `search(array $criteria)`, `getById($id)`, `count(array $criteria)` +- `remove($itemOrId)` +- `itemFactory()`, `itemFromArray(array)` +- `createQueryBuilder()` + +--- + +## New Feature: JSONB Property Storage + +The refactoring adds a new `propertyStorage: json` mode where all item +properties are stored in a single JSONB column instead of individual table +columns. + +### Configuration + +```yaml +featureType: + table: my_features + uniqueId: id + geomField: geom + srid: 4326 + propertyStorage: json + propertyColumn: properties # default, can be omitted +``` + +### Database Schema + +```sql +CREATE TABLE my_features ( + id SERIAL PRIMARY KEY, + geom geometry(Geometry, 4326), + properties JSONB NOT NULL DEFAULT '{}'::jsonb +); +``` + +### How It Works + +- **SELECT**: Reads the JSONB column and decodes to associative array +- **INSERT/UPDATE**: Encodes properties array to JSON string +- **Form fields**: `name` attribute maps to JSON keys (transparent to frontend) + +--- + +## Security Improvements + +### Removed: eval-based Event Processing + +The old `EventProcessor` used PHP's `eval()` to execute user-configured +expressions at lifecycle hooks (`onBeforeSave`, `onAfterUpdate`, etc.). +This was a significant security risk: + +- Arbitrary PHP code execution from YAML configuration +- Access to database connections, user tokens, and authorization context +- No sandboxing or input validation + +The `events` configuration key is still accepted (no parse errors) but +expressions are no longer executed. For lifecycle hooks, use Symfony's +EventDispatcher instead. + +--- + +## Compatibility Notes + +1. **Existing YAML configurations work unchanged** — `propertyStorage` + defaults to `columns`, preserving classic behavior. + +2. **Oracle and SQLite are no longer supported** — the driver abstraction + was removed. Only PostgreSQL/PostGIS is supported. + +3. **The `events` key is silently ignored** — no runtime errors, but + expressions no longer execute. + +4. **`DataStoreService` / `FeatureTypeService` removed** — these were + already deprecated. Use `RepositoryRegistry` with the factory services + (`mbds.default_datastore_factory`, `mbds.default_featuretype_factory`). + +5. **`RETURNING` clause** — INSERT statements now use `RETURNING` to get + the new ID. This is PostgreSQL-specific (another reason Oracle/SQLite + support was dropped). diff --git a/src/Mapbender/DataSourceBundle/Component/DataRepository.php b/src/Mapbender/DataSourceBundle/Component/DataRepository.php deleted file mode 100644 index 6084f7f9..00000000 --- a/src/Mapbender/DataSourceBundle/Component/DataRepository.php +++ /dev/null @@ -1,395 +0,0 @@ -connection = $connection; - $this->tokenStorage = $tokenStorage; - $this->tableName = $tableName; - $this->uniqueIdFieldName = $idColumnName; - $this->sqlFilter = $filter; - $this->driver = $this->driverFactory($connection); - $this->fields = $fields !== null ? $fields : $this->detectFields(); - } - - /** - * @return Connection - */ - public function getConnection() - { - return $this->connection; - } - - public function getTableName(): string - { - return $this->tableName; - } - - public function getTableNameUnquoted(): string - { - $parts = explode(".", $this->tableName); - return implode(".", self::stripQuotesFromArray($parts)); - } - - /** - * Create empty item - * - * @return DataItem - * @since 0.1.16.2 - */ - public function itemFactory() - { - return $this->itemFromArray(array()); - } - - /** - * @param integer|string $id - * @return DataItem|null - */ - public function getById($id) - { - $qb = $this->createQueryBuilder(); - $this->configureSelect($qb, false, array()); - return $this->getByIdInternal($id, $qb); - } - - protected function getByIdInternal($id, QueryBuilder $qb) - { - $qb - ->setMaxResults(1) - ->where($this->getUniqueId() . " = :id") - ->setParameter('id', $id) - ; - $row = $qb->executeQuery()->fetchAssociative(); - if ($row) { - $items = $this->prepareResults(array($row)); - return $items[0]; - } else { - return null; - } - } - - /** - * Search feature by criteria - * - * @param array $criteria - * @return DataItem[] - */ - public function search(array $criteria = array()) - { - $queryBuilder = $this->createQueryBuilder(); - $this->configureSelect($queryBuilder, true, $criteria); - return $this->prepareResults($queryBuilder->executeQuery()->fetchAllAssociative()); - } - - /** - * Returns number of matched rows. - * - * @param array $criteria same as supported by search, minus "maxResults" - * @return int - * @since 0.1.22 - */ - public function count(array $criteria) - { - $qb = $this->createQueryBuilder(); - $this->configureCount($qb, true, $criteria); - return \intval($qb->executeQuery()->fetchOne()); - } - - /** - * Get by ID list - * - * @param mixed[] $ids - * @return DataItem[] - */ - public function getByIds($ids) - { - $queryBuilder = $this->createQueryBuilder(); - $this->configureSelect($queryBuilder, false, array()); - $connection = $queryBuilder->getConnection(); - $condition = $queryBuilder->expr()->in($this->uniqueIdFieldName, array_map(array($connection, 'quote'), $ids)); - $queryBuilder->where($condition); - $results = $this->prepareResults($queryBuilder->executeQuery()->fetchAllAssociative()); - return $results; - } - - /** - * @param DataItem $item - * @return DataItem - */ - public function insertItem(DataItem $item) - { - $values = $this->prepareStoreValues($item); - unset($values[$this->uniqueIdFieldName]); - $values = $this->getTableMetaData()->prepareInsertData($values); - $id = $this->driver->insert($this->connection, $this->getTableNameUnquoted(), $values, $this->uniqueIdFieldName); - // Reload (fully populate, renormalize geometry etc) - // Use reload to support FeatureType in maintaining srs in = srs out - /** @see FeatureType::reloadItem */ - $tempItem = clone $item; - $tempItem->setId($id); - return $this->reloadItem($tempItem); - } - - public function updateItem(DataItem $item) - { - $values = $this->prepareStoreValues($item); - $identifier = $this->idToIdentifier($item->getId()); - $values = $this->getTableMetaData()->prepareUpdateData($values); - $this->driver->update($this->connection, $this->getTableNameUnquoted(), $values, $identifier); - return $this->reloadItem($item); - } - - /** - * @param DataItem $item - * @return DataItem|null - */ - protected function reloadItem($item) - { - return $this->getById($item->getId()); - } - - /** - * @return TableMeta - */ - protected function getTableMetaData() - { - if (!$this->tableMetaData) { - $this->tableMetaData = $this->driver->loadTableMeta($this->connection, $this->tableName); - } - if (empty($this->tableMetaData->getColumNames())) { - throw new ConfigurationErrorException("The table " . $this->tableName . " is empty or does not exist."); - } - return $this->tableMetaData; - } - - /** - * Get unique ID field name - * - * @return string - */ - public function getUniqueId() - { - return $this->uniqueIdFieldName; - } - - /** - * @return string[] - */ - public function getFields() - { - return $this->fields; - } - - /** - * @return QueryBuilder - */ - public function createQueryBuilder() - { - return $this->connection->createQueryBuilder(); - } - - /** - * @param Connection $connection - * @return DoctrineBaseDriver - * @throws \Doctrine\DBAL\Exception - * @throws \RuntimeException on incompatible platform - */ - protected function driverFactory(Connection $connection) - { - $platformName = $connection->getDatabasePlatform()->getName(); - switch ($platformName) { - case 'sqlite'; - $driver = new SQLite(); - break; - case 'postgresql'; - $driver = new PostgreSQL(); - break; - case 'oracle'; - $driver = new Oracle(); - break; - default: - throw new \RuntimeException("Unsupported DBAL platform " . print_r($platformName, true)); - } - return $driver; - } - - /** - * @param mixed $id - * @return array - */ - protected function idToIdentifier($id) - { - $uniqueId = $this->uniqueIdFieldName; - return array($uniqueId => $id); - } - - protected function prepareStoreValues(DataItem $item) - { - $meta = $this->getTableMetaData(); - $values = array(); - foreach ($item->getAttributes() as $name => $value) { - $values[$meta->getRealColumnName($name)] = $value; - } - return $values; - } - - /** - * Convert database rows to DataItem objects - * - * @param mixed[][] $rows - * @return DataItem[] - */ - protected function prepareResults(array $rows) - { - $items = array(); - foreach ($rows as $row) { - $items[] = $this->itemFromArray($this->attributesFromRow($row)); - } - return $items; - } - - /** - * @param mixed[] $values - * @return mixed[] - */ - protected function attributesFromRow(array $values) - { - $attributes = array(); - $meta = $this->getTableMetaData(); - foreach ($this->fields as $fieldName) { - $attributes[$fieldName] = $values[$meta->getRealColumnName($fieldName)]; - } - return $attributes; - } - - /** - * Create preinitialized item - * - * @param array $attributes - * @return DataItem - * @since 0.1.16.2 - */ - public function itemFromArray(array $attributes) - { - return new DataItem($attributes, $this->uniqueIdFieldName); - } - - protected function configureSelect(QueryBuilder $queryBuilder, $includeDefaultFilter, array $params) - { - $queryBuilder->from($this->getTableName(), 't'); - $connection = $queryBuilder->getConnection(); - $meta = $this->getTableMetaData(); - foreach ($this->fields as $fieldName) { - $columnName = $meta->getRealColumnName($fieldName); - $queryBuilder->addSelect($connection->quoteIdentifier($columnName)); - } - if (!empty($params['maxResults'])) { - $queryBuilder->setMaxResults($params['maxResults']); - } - $this->addQueryFilters($queryBuilder, $includeDefaultFilter, $params); - } - - protected function configureCount(QueryBuilder $queryBuilder, $includeDefaultFilter, array $params) - { - $queryBuilder->from($this->getTableName(), 't'); - $queryBuilder->select('count(*)'); - $this->addQueryFilters($queryBuilder, $includeDefaultFilter, $params); - } - - protected function addQueryFilters(QueryBuilder $queryBuilder, $includeDefaultFilter, $params) - { - $setUserParam = false; - $userNamePattern = '#:userName([^_\w\d]|$)#'; - if ($includeDefaultFilter && !empty($this->sqlFilter)) { - $setUserParam = !!preg_match($userNamePattern, $this->sqlFilter); - $queryBuilder->andWhere($this->sqlFilter); - } - if (!empty($params['where'])) { - $setUserParam = $setUserParam || preg_match($userNamePattern, $params['where']); - $queryBuilder->andWhere($params['where']); - } - if ($setUserParam) { - $token = $this->tokenStorage->getToken(); - // Check for null token or NullToken (anonymous user) - same pattern as Mapbender core - if ($token !== null && !$token instanceof NullToken) { - $queryBuilder->setParameter('userName', $token->getUserIdentifier()); - } else { - // No authenticated user, use empty string or anonymous - $queryBuilder->setParameter('userName', ''); - } - } - } - - /** - * @return string[] - */ - protected function detectFields() - { - $fields = array(); - foreach ($this->getTableMetaData()->getColumNames() as $columnName) { - $fields[] = \strtolower($columnName); - } - return $fields; - } - - public static function stripQuotesFromArray(array $array) - { - return array_map('self::stripQuotes', $array); - } - - public static function stripQuotes(string $name) - { - return str_starts_with($name, '"') && str_ends_with($name, '"') - ? substr($name, 1, -1) - : $name; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/DataStore.php b/src/Mapbender/DataSourceBundle/Component/DataStore.php index 91724263..5a02c900 100644 --- a/src/Mapbender/DataSourceBundle/Component/DataStore.php +++ b/src/Mapbender/DataSourceBundle/Component/DataStore.php @@ -1,158 +1,544 @@ + * Repository for non-spatial database table rows. + * + * Supports two property storage modes: + * - "columns" (default): each property is a separate table column + * - "json": all properties in a single JSONB column + * + * Configuration array keys: + * connection string Doctrine DBAL connection name (default: "default") + * table string Table name, required (supports "schema"."table") + * uniqueId string Primary key column (default: "id") + * fields array Explicit column list, or null for auto-detect (columns mode) + * filter string Permanent SQL WHERE fragment (supports :userName placeholder) + * propertyStorage string "columns" or "json" (default: "columns") + * propertyColumn string JSONB column name when propertyStorage="json" (default: "properties") + * events array DEPRECATED, accepted but ignored */ -class DataStore extends EventAwareDataRepository +class DataStore { + /** + * Event hook names from the removed eval-based EventProcessor. + * Used only to detect and warn about deprecated configuration. + */ + private const DEPRECATED_EVENT_HOOKS = [ + 'onBeforeSave', 'onAfterSave', 'onBeforeUpdate', 'onAfterUpdate', + 'onBeforeInsert', 'onAfterInsert', 'onBeforeRemove', 'onAfterRemove', + 'onBeforeSearch', 'onAfterSearch', + ]; + + private const DEPRECATED_EVENTS_WARNING = + 'DEPRECATED [DataSourceBundle] The "events" configuration key (table: %s, hooks: %s) is no longer ' + . 'supported and will be silently ignored. The eval-based event system (EventProcessor) was removed ' + . 'because it allowed arbitrary PHP code execution from YAML/database configuration — a significant ' + . 'security risk. Use database triggers, Symfony EventDispatcher, or DataStore subclass overrides ' + . 'instead. See EVENT-MIGRATION.md for a complete migration guide.'; + + protected Connection $connection; + protected TokenStorageInterface $tokenStorage; + protected PropertyAdapterInterface $propertyAdapter; + protected string $tableName; + protected string $uniqueId; + protected ?string $filter; + /** * @param Connection $connection * @param TokenStorageInterface $tokenStorage - * @param EventProcessor $eventProcessor - * @param array|null $args + * @param mixed $eventProcessorOrConfig EventProcessor (deprecated, ignored) or config array + * @param array $config Configuration array (when 3rd arg is EventProcessor) + */ + public function __construct( + Connection $connection, + TokenStorageInterface $tokenStorage, + $eventProcessorOrConfig = [], + $config = [], + ) { + $this->connection = $connection; + $this->tokenStorage = $tokenStorage; + + // Backward compatibility: old signature passed EventProcessor as 3rd arg, config as 4th + if (is_array($eventProcessorOrConfig)) { + $config = $eventProcessorOrConfig; + } + // else: $eventProcessorOrConfig is EventProcessor (ignored), $config is 4th arg + + $this->tableName = $config['table'] ?? ''; + $this->uniqueId = $config['uniqueId'] ?? 'id'; + $filter = !empty($config['filter']) ? $config['filter'] : null; + $this->filter = $this->sanitizeFilter($filter); + + // Warn when deprecated eval-based events are configured but will not be executed + if (!empty($config['events'])) { + $eventNames = implode(', ', array_intersect( + array_keys($config['events']), + self::DEPRECATED_EVENT_HOOKS + )); + if ($eventNames) { + $table = $this->tableName ?: '(unknown)'; + @trigger_error(sprintf( + self::DEPRECATED_EVENTS_WARNING, + $table, $eventNames + ), E_USER_DEPRECATED); + } + } + + $this->propertyAdapter = $this->createPropertyAdapter($config); + } + + /** + * @return PropertyAdapterInterface + */ + public function getPropertyAdapter(): PropertyAdapterInterface + { + return $this->propertyAdapter; + } + + /** + * @return Connection + */ + public function getConnection(): Connection + { + return $this->connection; + } + + /** + * @return string Quoted table name + */ + public function getTableName(): string + { + return $this->quoteTableName($this->tableName); + } + + /** + * @return string Unquoted table name */ - public function __construct(Connection $connection, TokenStorageInterface $tokenStorage, EventProcessor $eventProcessor, $args = array()) + public function getTableNameUnquoted(): string { - $eventConfig = isset($args["events"]) ? $args["events"] : array(); - $filter = (!empty($args['filter'])) ? $args['filter'] : null; - parent::__construct($connection, $tokenStorage, $eventProcessor, $eventConfig, $args['table'], $args['uniqueId'], $args['fields'], $filter); + return str_replace('"', '', $this->tableName); } /** - * Save data item. Auto-inflects to insert (no id) or update (non-empty id). + * @return string Primary key column name + */ + public function getUniqueId(): string + { + return $this->uniqueId; + } + + /** + * @return string[] + */ + public function getFields(): array + { + return array_merge([$this->uniqueId], $this->propertyAdapter->getSelectColumns()); + } + + /** + * @return string Database platform name (e.g. "postgresql") + */ + public function getPlatformName(): string + { + return $this->connection->getDatabasePlatform()->getName(); + } + + /** + * Create an empty DataItem. * - * @param DataItem|array $itemOrData Data item * @return DataItem - * @throws \Exception */ - public function save($itemOrData) + public function itemFactory(): DataItem { - $saveItem = \is_array($itemOrData) ? $this->itemFromArray($itemOrData) : $itemOrData; - if (isset($this->events[self::EVENT_ON_BEFORE_SAVE]) || isset($this->events[self::EVENT_ON_AFTER_SAVE])) { - $eventData = $this->getSaveEventData($saveItem); + return new DataItem([], $this->uniqueId); + } + + /** + * Create a DataItem pre-populated with attributes. + * + * @param array $attributes + * @return DataItem + */ + public function itemFromArray(array $attributes): DataItem + { + return new DataItem($attributes, $this->uniqueId); + } + + /** + * @param mixed $id + * @return DataItem|null + */ + public function getById($id): ?DataItem + { + $qb = $this->createSelectQueryBuilder([]); + $qb->andWhere($this->qi($this->uniqueId) . ' = :pkId'); + $qb->setParameter('pkId', $id); + $this->bindUserName($qb); + + $row = $qb->executeQuery()->fetchAssociative(); + if (!$row) { + return null; + } + return $this->itemFromRow($row); + } + + /** + * @param array $criteria Supported keys: maxResults, where + * @return DataItem[] + */ + public function search(array $criteria = []): array + { + $qb = $this->createSelectQueryBuilder($criteria); + $this->addFilters($qb, $criteria); + $this->bindUserName($qb); + + $rows = $qb->executeQuery()->fetchAllAssociative(); + return $this->prepareResults($rows); + } + + /** + * @param array $criteria Supported keys: where + * @return int + */ + public function count(array $criteria = []): int + { + $qb = $this->connection->createQueryBuilder(); + $qb->select('COUNT(*)'); + $qb->from($this->getTableName()); + $this->addFilters($qb, $criteria); + $this->bindUserName($qb); + + return (int) $qb->executeQuery()->fetchOne(); + } + + /** + * Save a data item: insert if new, update if existing. + * + * @param DataItem|array $itemOrData + * @return DataItem + */ + public function save($itemOrData): DataItem + { + $item = $this->normalizeItem($itemOrData); + if ($item->getId()) { + return $this->updateItem($item); } else { - $eventData = null; + return $this->insertItem($item); } + } + + /** + * @param DataItem|array $itemOrData + * @return DataItem + */ + public function insert($itemOrData): DataItem + { + return $this->insertItem($this->normalizeItem($itemOrData)); + } + + /** + * @param DataItem|array $itemOrData + * @return DataItem + */ + public function update($itemOrData): DataItem + { + return $this->updateItem($this->normalizeItem($itemOrData)); + } + + /** + * @param mixed $itemOrId DataItem or integer ID + * @return int|null Number of deleted rows + */ + public function remove($itemOrId) + { + $id = ($itemOrId instanceof DataItem) ? $itemOrId->getId() : $itemOrId; + return $this->connection->delete( + $this->getTableNameUnquoted(), + [$this->uniqueId => $id], + ); + } - if (isset($this->events[self::EVENT_ON_BEFORE_SAVE])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_BEFORE_SAVE], $eventData); - $runSave = $this->eventProcessor->allowSave; + /** + * Create a DBAL QueryBuilder. + * + * @return \Doctrine\DBAL\Query\QueryBuilder + */ + public function createQueryBuilder(): \Doctrine\DBAL\Query\QueryBuilder + { + return $this->connection->createQueryBuilder(); + } + + // --------------------------------------------------------------- + // Protected methods (override points for FeatureType) + // --------------------------------------------------------------- + + /** + * Build the SELECT query with all columns. + * + * @param array $criteria + * @return \Doctrine\DBAL\Query\QueryBuilder + */ + protected function createSelectQueryBuilder(array $criteria): \Doctrine\DBAL\Query\QueryBuilder + { + $qb = $this->connection->createQueryBuilder(); + $qb->from($this->getTableName()); + + // Always select the primary key + $qb->addSelect($this->qi($this->uniqueId)); + + // Property columns + foreach ($this->propertyAdapter->getSelectColumns() as $col) { + $qb->addSelect($this->qi($col)); + } + + if (!empty($criteria['maxResults'])) { + $qb->setMaxResults((int) $criteria['maxResults']); + } + + return $qb; + } + + /** + * Add WHERE clauses from permanent filter and criteria. + * + * @param \Doctrine\DBAL\Query\QueryBuilder $qb + * @param array $criteria + */ + protected function addFilters(\Doctrine\DBAL\Query\QueryBuilder $qb, array $criteria): void + { + if ($this->filter) { + $qb->andWhere($this->filter); + } + if (!empty($criteria['where'])) { + $qb->andWhere($criteria['where']); + } + } + + /** + * Insert a DataItem and return the reloaded version. + * + * @param DataItem $item + * @return DataItem + */ + protected function insertItem(DataItem $item): DataItem + { + $allProperties = $this->getItemPropertiesForStorage($item); + $storageData = $this->propertyAdapter->prepareStorageData($allProperties); + + $columns = []; + $placeholders = []; + $params = []; + + foreach ($storageData as $col => $value) { + $columns[] = $this->qi($col); + $placeholders[] = '?'; + $params[] = $value; + } + + if (empty($columns)) { + $sql = sprintf( + 'INSERT INTO %s DEFAULT VALUES RETURNING %s', + $this->getTableName(), + $this->qi($this->uniqueId), + ); } else { - $runSave = true; + $sql = sprintf( + 'INSERT INTO %s (%s) VALUES (%s) RETURNING %s', + $this->getTableName(), + implode(', ', $columns), + implode(', ', $placeholders), + $this->qi($this->uniqueId), + ); } - if ($runSave) { - if (!$saveItem->getId()) { - $itemOut = $this->insertItem($saveItem); - } else { - $itemOut = $this->updateItem($saveItem); + + $stmt = $this->connection->prepare($sql); + $result = $stmt->executeQuery($params); + $id = $result->fetchOne(); + $item->setId($id); + + return $this->reloadItem($item) ?? $item; + } + + /** + * Update a DataItem and return the reloaded version. + * + * @param DataItem $item + * @return DataItem + */ + protected function updateItem(DataItem $item): DataItem + { + $allProperties = $this->getItemPropertiesForStorage($item); + $storageData = $this->propertyAdapter->prepareStorageData($allProperties); + + if (!empty($storageData)) { + $setClauses = []; + $params = []; + + foreach ($storageData as $col => $value) { + $setClauses[] = $this->qi($col) . ' = ?'; + $params[] = $value; } - } else { - $itemOut = $saveItem; + + $params[] = $item->getId(); + + $sql = sprintf( + 'UPDATE %s SET %s WHERE %s = ?', + $this->getTableName(), + implode(', ', $setClauses), + $this->qi($this->uniqueId), + ); + + $this->connection->executeStatement($sql, $params); } - if (isset($this->events[self::EVENT_ON_AFTER_SAVE])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_AFTER_SAVE], $eventData); + return $this->reloadItem($item) ?? $item; + } + + /** + * Convert raw database rows to DataItem array. + * + * @param array $rows + * @return DataItem[] + */ + protected function prepareResults(array $rows): array + { + $items = []; + foreach ($rows as $row) { + $items[] = $this->itemFromRow($row); } - return $itemOut; + return $items; } /** - * Insert new row + * Build a DataItem from a raw database row. * - * @param array|DataItem $itemOrData + * @param array $row * @return DataItem */ - public function insert($itemOrData) + protected function itemFromRow(array $row): DataItem { - $item = \is_array($itemOrData) ? $this->itemFromArray($itemOrData) : $itemOrData; - return $this->insertItem($item); + $properties = $this->propertyAdapter->extractProperties($row); + $properties[$this->uniqueId] = $row[$this->uniqueId]; + return $this->itemFromArray($properties); } /** - * Update existing row + * Reload an item from the database by its ID. * - * @param array|DataItem $itemOrData - * @return DataItem + * @param DataItem $item + * @return DataItem|null */ - public function update($itemOrData) + protected function reloadItem(DataItem $item): ?DataItem { - $item = \is_array($itemOrData) ? $this->itemFromArray($itemOrData) : $itemOrData; - return $this->updateItem($item); + return $this->getById($item->getId()); } /** - * Remove data item - * @param int|DataItem $itemOrId - * @return int number of deleted rows + * Quote an identifier (column/table name). */ - public function remove($itemOrId) + protected function qi(string $identifier): string { - $itemId = !\is_object($itemOrId) ? $itemOrId : $itemOrId->getId(); - if (isset($this->events[self::EVENT_ON_BEFORE_REMOVE]) || isset($this->events[self::EVENT_ON_AFTER_REMOVE])) { - // uh-oh - $item = $this->getById($itemId); - $eventData = $this->getCommonEventData() + array( - 'args' => $item, - 'item' => $item, - 'feature' => $item, - 'method' => 'remove', - 'originData' => $item, - ); - } else { - $eventData = null; + return $this->connection->quoteIdentifier($identifier); + } + + /** + * Quote a table name, handling schema-qualified names. + */ + protected function quoteTableName(string $tableName): string + { + $unquoted = str_replace('"', '', $tableName); + $parts = explode('.', $unquoted); + return implode('.', array_map(fn($p) => $this->qi($p), $parts)); + } + + /** + * Bind the :userName parameter if present in any WHERE clause. + */ + protected function bindUserName(\Doctrine\DBAL\Query\QueryBuilder $qb): void + { + if ($this->filter && str_contains($this->filter, ':userName')) { + $qb->setParameter('userName', $this->getUserName()); } - if (isset($this->events[ self::EVENT_ON_BEFORE_REMOVE ])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_BEFORE_REMOVE], $eventData); - $doRemove = $this->eventProcessor->allowRemove; - } else { - $doRemove = true; + } + + /** + * Get the current user's name from the security token. + */ + protected function getUserName(): string + { + $token = $this->tokenStorage->getToken(); + if ($token && method_exists($token, 'getUserIdentifier')) { + return $token->getUserIdentifier() ?? ''; } - if ($doRemove) { - $result = !!$this->connection->delete($this->tableName, $this->idToIdentifier($itemId)); - } else { - $result = null; + return ''; + } + + /** + * Convert input to a DataItem, accepting arrays or DataItem objects. + */ + protected function normalizeItem($itemOrData): DataItem + { + if ($itemOrData instanceof DataItem) { + return $itemOrData; } - if (isset($this->events[self::EVENT_ON_AFTER_REMOVE])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_AFTER_REMOVE], $eventData); + if (is_array($itemOrData)) { + return $this->itemFromArray($itemOrData); } - return $result; + throw new \InvalidArgumentException('Expected DataItem or array, got ' . get_debug_type($itemOrData)); } /** - * Get platform name + * Extract the properties from a DataItem for storage, + * excluding the uniqueId (handled separately). * - * @return string + * @param DataItem $item + * @return array */ - public function getPlatformName() + protected function getItemPropertiesForStorage(DataItem $item): array { - return $this->getConnection()->getDatabasePlatform()->getName(); + $attrs = $item->toArray(); + unset($attrs[$this->uniqueId]); + return $attrs; } - /** @noinspection PhpUnused */ /** - * Set permanent SQL filter used by $this->search() - * https://trac.wheregroup.com/cp/issues/3733 - * - * @see $this->search() - * @param string $sqlFilter - * NOTE: magic setter invocation; expected config value comes with key 'filter' - */ - protected function setFilter($sqlFilter) - { - if ($sqlFilter) { - // unquote quoted parameter references - // we use parameter binding - $filtered = preg_replace('#([\\\'"])(:[\w\d_]+)(\\1)#', '\\2', $sqlFilter); - if ($filtered !== $sqlFilter) { - @trigger_error("DEPRECATED: DO NOT quote parameter references in sql filter configuration", E_USER_DEPRECATED); - } - $sqlFilter = $filtered; + * Create the appropriate PropertyAdapter based on config. + */ + protected function createPropertyAdapter(array $config): PropertyAdapterInterface + { + $storage = $config['propertyStorage'] ?? 'columns'; + + if ($storage === 'json') { + $jsonColumn = $config['propertyColumn'] ?? 'properties'; + return new JsonColumnAdapter($jsonColumn); + } + + return new DiscreteColumnAdapter( + $this->connection, + $this->tableName, + $config['fields'] ?? null, + $this->uniqueId, + ); + } + + /** + * Remove quotes around parameter placeholders in filter SQL. + * Legacy configs may have ':userName' instead of :userName. + */ + private function sanitizeFilter(?string $filter): ?string + { + if (!$filter) { + return null; } - $this->sqlFilter = $sqlFilter; + return preg_replace('#([\\\'"])(:[\w\d_]+)(\\1)#', '\\2', $filter); } } diff --git a/src/Mapbender/DataSourceBundle/Component/DataStoreService.php b/src/Mapbender/DataSourceBundle/Component/DataStoreService.php deleted file mode 100644 index dcae57e5..00000000 --- a/src/Mapbender/DataSourceBundle/Component/DataStoreService.php +++ /dev/null @@ -1,41 +0,0 @@ - - * @deprecated incompatible with Symfony 4 (full container injection); use RepositoryRegistry and inject `mbds.default_datastore_factory` - */ -class DataStoreService extends RepositoryRegistry -{ - protected $factoryId = 'mbds.default_datastore_factory'; - - /** - * @param ContainerInterface $container - * @param mixed[][]|string $declarations repository configs, or container param key for lookup - */ - public function __construct(ContainerInterface $container, $declarations = 'dataStores') - { - /** @var DataStoreFactory $factory */ - $factory = $container->get($this->factoryId); - $declarations = $declarations ?: array(); - if ($declarations && \is_string($declarations)) { - $declarations = $container->getParameter($declarations); - } - - parent::__construct($factory, $declarations ?: array()); - } - - /** - * Get store by name - * - * @param string $name - * @return DataStore - */ - public function get($name) - { - return $this->getDataStoreByName($name); - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Drivers/DoctrineBaseDriver.php b/src/Mapbender/DataSourceBundle/Component/Drivers/DoctrineBaseDriver.php deleted file mode 100644 index 1ea4e1c1..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Drivers/DoctrineBaseDriver.php +++ /dev/null @@ -1,128 +0,0 @@ - - */ -abstract class DoctrineBaseDriver -{ - /** - * @param Connection $connection - * @param string $tableName - * @param array $data - * @param string $identifier - * @return int the last insert id - */ - public function insert(Connection $connection, $tableName, array $data, $identifier) - { - $pData = $this->prepareInsertData($connection, $data); - $tableName = $connection->quoteIdentifier($tableName); - - $sql = $this->getInsertSql($tableName, $pData[0], $pData[1]); - $connection->executeQuery($sql, $pData[2]); - return $connection->lastInsertId(); - } - - /** - * @param Connection $connection - * @param mixed[] $data - * @return array numeric with 3 entries: first: quoted column names; second: sql value expressions; third: query parameters - */ - protected function prepareInsertData(Connection $connection, array $data) - { - $columns = array(); - $sqlValues = array(); - $params = array(); - foreach ($data as $columnName => $value) { - if ($value instanceof Expression) { - $sqlValues[] = $value->getText(); - } else { - // add placeholder and param binding - $sqlValues[] = '?'; - $params[] = $this->prepareParamValue($value); - } - $columns[] = $connection->quoteIdentifier($columnName); - } - return array( - $columns, - $sqlValues, - $params, - ); - } - - protected function getInsertSql($tableName, $columns, $values) - { - return - 'INSERT INTO ' . $tableName - . ' (' . implode(', ', $columns) . ')' - . ' VALUES ' - . ' (' . implode(', ', $values) . ')' - ; - } - - /** - * @param Connection $connection - * @param string $tableName - * @param mixed[] $data - * @param mixed[] $identifier - * @return int rows affected - * @throws \Doctrine\DBAL\Exception - */ - public function update(Connection $connection, $tableName, array $data, array $identifier) - { - $data = array_diff_key($data, $identifier); - if (empty($data)) { - throw new \Exception("Can't update row without data"); - } - $initializers = array(); - $conditions = array(); - $params = array(); - foreach ($data as $columnName => $value) { - $columnQuoted = $connection->quoteIdentifier($columnName); - if ($value instanceof Expression) { - $initializers[] = "{$columnQuoted} = {$value->getText()}"; - } else { - // add placeholder and param binding - $initializers[] = "{$columnQuoted} = ?"; - $params[] = $this->prepareParamValue($value); - } - } - foreach ($identifier as $columnName => $value) { - $conditions[] = $connection->quoteIdentifier($columnName) . ' = ?'; - $params[] = $this->prepareParamValue($value); - } - - $sql = - 'UPDATE ' . $connection->quoteIdentifier($tableName) - . ' SET ' - . implode(', ', $initializers) - . ' WHERE ' - . implode(' AND ', $conditions) - ; - return $connection->executeStatement($sql, $params); - } - - - - /** - * @param mixed $value - * @return mixed - */ - protected function prepareParamValue($value) - { - // Base driver: no transformation - return $value; - } - - /** - * @param Connection $connection - * @param string $tableName - * @return TableMeta - */ - abstract public function loadTableMeta(Connection $connection, $tableName); -} diff --git a/src/Mapbender/DataSourceBundle/Component/Drivers/Interfaces/Geographic.php b/src/Mapbender/DataSourceBundle/Component/Drivers/Interfaces/Geographic.php deleted file mode 100644 index af8aee44..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Drivers/Interfaces/Geographic.php +++ /dev/null @@ -1,56 +0,0 @@ - - */ -class Oracle extends DoctrineBaseDriver implements Geographic -{ - /** - * Transform result column names from lower case to upper - * - * @param array[] $rows - */ - public static function transformColumnNames(&$rows) - { - if (!$rows) { - $columnNames = array(); - } else { - $columnNames = array_keys(current($rows)); - } - foreach ($rows as &$row) { - foreach ($columnNames as $name) { - $row[ strtolower($name) ] = &$row[ $name ]; - unset($row[ $name ]); - } - } - } - - public function getReadEwktSql($data) - { - return "SDO_UTIL.TO_WKBGEOMETRY({$data})"; - } - - public function getTransformSql($data, $sridTo) - { - if (!$sridTo || !\is_numeric($sridTo)) { - throw new \InvalidArgumentException("Invalid sridTo " . print_r($sridTo, true)); - } - return "SDO_CS.TRANSFORM({$data}, " . intval($sridTo) . ')'; - } - - public function getDumpWktSql($data) - { - return "SDO_UTIL.TO_WKTGEOMETRY({$data})"; - } - - /** - * @param string $geomExpression - * @return string - */ - public function getPromoteToCollectionSql($geomExpression) - { - // no implementation - // @todo: support this? Use cases? - return $geomExpression; - } - - public function getIntersectCondition($geomExpressionA, $geomExpressionB) - { - return "SDO_RELATE({$geomExpressionA}, {$geomExpressionB}, 'mask=ANYINTERACT querytype=WINDOW') = 'TRUE'"; - } - - /** - * @inheritdoc - */ - public function getGeomAttributeAsWkt($geomReference, $sridTo) - { - return "SDO_UTIL.TO_WKTGEOMETRY(SDO_CS.TRANSFORM($geomReference, $sridTo))"; - } - - public function getColumnToEwktSql($column, $sridTo) - { - return "CASE WHEN {$column} IS NOT NULL THEN" - . " CONCAT('SRID={$sridTo};', " - . $this->getGeomAttributeAsWkt($column, $sridTo) - . " ELSE NULL END" - ; - } - - public function loadTableMeta(Connection $connection, $tableName) - { - // NOTE: cannot use Doctrine SchemaManager. SchemaManager will throw when encountering - // geometry type columns. Internal SchemaManager Column metadata APIs are - // closed to querying individual columns. - $platform = $connection->getDatabasePlatform(); - $sql = $platform->getListTableColumnsSQL($tableName); - - $gmdSql = 'SELECT COLUMN_NAME, SRID FROM ALL_SDO_GEOM_METADATA' - . ' WHERE TABLE_NAME = ' . $platform->quoteIdentifier($tableName) - ; - $srids = array(); - try { - foreach ($connection->executeQuery($gmdSql) as $row) { - $srids[$row['COLUMN_NAME']] = $row['SRID']; - } - } catch (\Doctrine\DBAL\Exception $e) { - // Ignore (no spatial support?) - } - - $columns = array(); - /** @see \Doctrine\DBAL\Platforms\OraclePlatform::getListTableColumnsSQL */ - /** @see \Doctrine\DBAL\Schema\OracleSchemaManager::_getPortableTableColumnDefinition */ - foreach ($connection->executeQuery($sql) as $row) { - $name = $row['column_name']; - if (!empty($srids[\strtoupper($name)])) { - $srid = $srids[\strtoupper($name)]; - } else { - $srid = null; - } - - $notNull = $row['nullable'] === 'N'; - $hasDefault = !!$row['data_default']; - $isNumeric = !!preg_match('#int|float|double|real|decimal|numeric#i', $row['data_type']); - $columns[$name] = new Column($notNull, $hasDefault, $isNumeric, null, $srid); - } - $tableMeta = new TableMeta($connection->getDatabasePlatform(), $columns); - return $tableMeta; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Drivers/PostgreSQL.php b/src/Mapbender/DataSourceBundle/Component/Drivers/PostgreSQL.php deleted file mode 100644 index 0e90f494..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Drivers/PostgreSQL.php +++ /dev/null @@ -1,138 +0,0 @@ - - */ -class PostgreSQL extends DoctrineBaseDriver implements Geographic -{ - - public function insert(Connection $connection, $tableName, array $data, $identifier) - { - $pData = $this->prepareInsertData($connection, $data); - $tableName = $connection->quoteIdentifier($tableName); - - $sql = $this->getInsertSql($tableName, $pData[0], $pData[1]) - . ' RETURNING ' . $connection->quoteIdentifier($identifier); - return $connection->fetchOne($sql, $pData[2]); - } - - protected function prepareParamValue($value) - { - if (\is_bool($value)) { - // PostgreSQL PDO will accept a variety of string representations for boolean columns - // including 't' and 'f' - return $value ? 't' : 'f'; - } else { - return parent::prepareParamValue($value); - } - } - - public function getReadEwktSql($data) - { - return "ST_MakeValid(ST_GeomFromEWKT({$data}))"; - } - - public function getTransformSql($data, $sridTo) - { - if (!$sridTo || !\is_numeric($sridTo)) { - throw new \InvalidArgumentException("Invalid sridTo " . print_r($sridTo, true)); - } - return "ST_MakeValid(ST_Transform({$data}, " . intval($sridTo) . '))'; - } - - /** - * @param string $geomExpression - * @return string - */ - public function getPromoteToCollectionSql($geomExpression) - { - return "ST_Multi({$geomExpression})"; - } - - public function getDumpWktSql($data) - { - return "ST_AsText({$data})"; - } - - public function getIntersectCondition($geomExpressionA, $geomExpressionB) - { - return "({$geomExpressionA} && {$geomExpressionB})"; - } - - /** - * @inheritdoc - */ - public function getGeomAttributeAsWkt($geomReference, $sridTo) - { - return "ST_ASTEXT(ST_TRANSFORM($geomReference, $sridTo))"; - } - - public function getColumnToEwktSql($geomReference, $sridTo) - { - return "ST_AsEwkt(ST_TRANSFORM($geomReference, $sridTo))"; - } - - public function loadTableMeta(Connection $connection, $tableName) - { - // NOTE: cannot use Doctrine SchemaManager. SchemaManager will throw when encountering - // geometry type columns. Internal SchemaManager Column metadata APIs are - // closed to querying individual columns. - $platform = $connection->getDatabasePlatform(); - $gcSql = 'SELECT f_geometry_column, srid, type FROM "public"."geometry_columns"' - . ' WHERE f_table_name = ?'; - $gcParams = array(); - if (str_contains($tableName, ".")) { - $tableNameParts = DataRepository::stripQuotesFromArray(explode('.', $tableName, 2)); - $tableNameUnquoted = implode('.', $tableNameParts); - $gcParams[] = $tableNameParts[1]; - $gcSql .= ' AND "f_table_schema" = ?'; - $gcParams[] = $tableNameParts[0]; - } else { - $gcParams[] = DataRepository::stripQuotes($tableName); - $tableNameUnquoted = $gcParams[0]; - $gcSql .= ' AND "f_table_schema" = current_schema()'; - } - $gcInfos = array(); - try { - $gcInfosResult = $connection->executeQuery($gcSql, $gcParams)->fetchAllAssociative(); - foreach ($gcInfosResult as $row) { - $gcInfos[$row['f_geometry_column']] = array($row['type'], $row['srid']); - } - } catch (\Doctrine\DBAL\Exception $e) { - // Ignore (DataStore on PostgreSQL / no Postgis) - } - - // TODO: this method was removed in DBAL 4, find a replacement - $sql = $platform->getListTableColumnsSQL($tableNameUnquoted); - $columns = array(); - /** @see \Doctrine\DBAL\Platforms\PostgreSqlPlatform::getListTableColumnsSQL */ - /** @see \Doctrine\DBAL\Schema\PostgreSqlSchemaManager::_getPortableTableColumnDefinition */ - $result = $connection->executeQuery($sql); - $data = $result->fetchAllAssociative(); - foreach ($data as $row) { - $name = trim($row['field'], '"'); // Undo quote_ident - $notNull = !$row['isnotnull']; - $hasDefault = !!$row['default']; - $isNumeric = !!preg_match('#int|float|double|real|decimal|numeric#i', $row['complete_type']); - if (!empty($gcInfos[$name])) { - $geomType = $gcInfos[$name][0]; - $srid = $gcInfos[$name][1]; - } else { - $geomType = $srid = null; - } - - $columns[$name] = new Column($notNull, $hasDefault, $isNumeric, $geomType, $srid); - } - $tableMeta = new TableMeta($connection->getDatabasePlatform(), $columns); - return $tableMeta; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Drivers/SQLite.php b/src/Mapbender/DataSourceBundle/Component/Drivers/SQLite.php deleted file mode 100644 index 324d5f4d..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Drivers/SQLite.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -class SQLite extends DoctrineBaseDriver -{ - public function loadTableMeta(Connection $connection, $tableName) - { - // NOTE: cannot use Doctrine SchemaManager::listTableColumns. SchemaManager - // destroys the distinction between a column with no default and a column - // with a null default. - $sql = $connection->getDatabasePlatform()->getListTableColumnsSQL($tableName); - $columns = array(); - /** @see \Doctrine\DBAL\Platforms\SqlitePlatform::getListTableColumnsSQL */ - /** @see \Doctrine\DBAL\Schema\SqliteSchemaManager::_getPortableTableColumnDefinition */ - foreach ($connection->executeQuery($sql) as $row) { - $isNullable = !$row['notnull']; - $hasDefault = !empty($row['dflt_value']); - $isNumeric = !!preg_match('#int|float|double|real|decimal|numeric#i', $row['type']); - $columns[$row['name']] = new Column($isNullable, $hasDefault, $isNumeric); - } - $tableMeta = new TableMeta($connection->getDatabasePlatform(), $columns); - return $tableMeta; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/EventAwareDataRepository.php b/src/Mapbender/DataSourceBundle/Component/EventAwareDataRepository.php deleted file mode 100644 index 9ee25ada..00000000 --- a/src/Mapbender/DataSourceBundle/Component/EventAwareDataRepository.php +++ /dev/null @@ -1,126 +0,0 @@ -eventProcessor = $eventProcessor; - $this->events = $eventConfig; - } - - - /** - * @param DataItem $item - * @return DataItem - * @since 0.1.17 - */ - public function updateItem(DataItem $item) - { - if (isset($this->events[self::EVENT_ON_BEFORE_UPDATE]) || isset($this->events[self::EVENT_ON_AFTER_UPDATE])) { - $eventData = $this->getSaveEventData($item); - } else { - $eventData = null; - } - if (isset($this->events[self::EVENT_ON_BEFORE_UPDATE])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_BEFORE_UPDATE], $eventData); - $runQuery = $this->eventProcessor->allowUpdate; - } else { - $runQuery = true; - } - if ($runQuery) { - $item = parent::updateItem($item); - } - if (isset($this->events[self::EVENT_ON_AFTER_UPDATE])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_AFTER_UPDATE], $eventData); - } - return $item; - } - - /** - * @param DataItem $item - * @return DataItem - * @since 0.1.21 - */ - public function insertItem(DataItem $item) - { - if (isset($this->events[self::EVENT_ON_BEFORE_INSERT]) || isset($this->events[self::EVENT_ON_AFTER_INSERT])) { - $eventData = $this->getSaveEventData($item); - } else { - $eventData = null; - } - if (isset($this->events[self::EVENT_ON_BEFORE_INSERT])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_BEFORE_INSERT], $eventData); - $runQuery = $this->eventProcessor->allowUpdate; - } else { - $runQuery = true; - } - if ($runQuery) { - $item = parent::insertItem($item); - } - if (isset($this->events[self::EVENT_ON_AFTER_INSERT])) { - $this->eventProcessor->runExpression($this->events[self::EVENT_ON_AFTER_INSERT], $eventData); - } - return $item; - } - - protected function getCommonEventData() - { - return array( - 'idKey' => $this->uniqueIdFieldName, - 'connection' => $this->connection, - ); - } - - /** - * @param DataItem $item - * @return array - */ - protected function getSaveEventData(DataItem $item) - { - // legacy quirk originData: - // 1) for inserts (no id), provide a blank, empty, DataItem object (like ->get(array())) - // 2) for updates, reload the original item (incoming item already carries new data!) - if ($item->getId()) { - $originData = $this->reloadItem($item); - } else { - $originData = $this->itemFactory(); - } - - return $this->getCommonEventData() + array( - 'item' => $item, - 'feature' => $item, - 'originData' => $originData, - ); - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/EventProcessor.php b/src/Mapbender/DataSourceBundle/Component/EventProcessor.php deleted file mode 100644 index 9bf85f34..00000000 --- a/src/Mapbender/DataSourceBundle/Component/EventProcessor.php +++ /dev/null @@ -1,87 +0,0 @@ -reset(); - // ~extract - foreach ($this->addBuiltins($locals) as $key => &$value) { - ${$key} = &$value; - } - $return = eval($expression); - if ($return === false && ($errorDetails = error_get_last())) { - $lastError = end($errorDetails); - throw new \Exception($lastError["message"], $lastError["type"]); - } - } - - /** - * For eval events only. - */ - protected function preventSave() - { - $this->allowSave = false; - } - - /** - * For eval events only. - */ - protected function preventRemove() - { - $this->allowRemove = false; - } - - protected function reset() - { - $this->allowUpdate = true; - $this->allowSave = true; - $this->allowInsert = true; - $this->allowRemove = true; - } - - /** - * @param array $locals - * @return array - */ - protected function addBuiltins(array $locals) - { - $token = $this->tokenStorage->getToken(); - - $locals += array( - 'context' => $this->authorizationChecker, - 'tokenStorage' => $this->tokenStorage, - 'user' => ($token !== null && !$token instanceof NullToken) ? $token->getUser() : null, - 'userRoles' => array(), - ); - - if ($token !== null && !$token instanceof NullToken) { - $token = $this->tokenStorage->getToken(); - $locals['userRoles'] = $token->getRoleNames(); - } - return $locals; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Expression.php b/src/Mapbender/DataSourceBundle/Component/Expression.php deleted file mode 100644 index 2c289bde..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Expression.php +++ /dev/null @@ -1,30 +0,0 @@ -text = $text; - } - - /** - * @return string - */ - public function getText() - { - return $this->text; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php b/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php index ae32b999..eba8afa9 100644 --- a/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php +++ b/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php @@ -1,62 +1,61 @@ connectionRegistry = $connectionRegistry; $this->tokenStorage = $tokenStorage; - $this->eventProcessor = $eventProcessor; } /** * @param array $config * @return DataStore */ - public function fromConfig(array $config) + public function fromConfig(array $config): DataStore { $config += $this->getConfigDefaults(); $connection = $this->getDbalConnectionByName($config['connection']); - return new DataStore($connection, $this->tokenStorage, $this->eventProcessor, $config); + return new DataStore($connection, $this->tokenStorage, $config); } - protected function getConfigDefaults() + protected function getConfigDefaults(): array { - return array( + return [ 'uniqueId' => 'id', 'connection' => 'default', 'fields' => null, - ); + ]; } /** - * @param $name + * @param string $name * @return Connection */ - public function getDbalConnectionByName($name) + public function getDbalConnectionByName($name): Connection { try { /** @var Connection $connection */ diff --git a/src/Mapbender/DataSourceBundle/Component/Factory/FeatureTypeFactory.php b/src/Mapbender/DataSourceBundle/Component/Factory/FeatureTypeFactory.php index bbb61613..df87d356 100644 --- a/src/Mapbender/DataSourceBundle/Component/Factory/FeatureTypeFactory.php +++ b/src/Mapbender/DataSourceBundle/Component/Factory/FeatureTypeFactory.php @@ -1,28 +1,27 @@ getConfigDefaults(); $connection = $this->getDbalConnectionByName($config['connection']); - return new FeatureType($connection, $this->tokenStorage, $this->eventProcessor, $config); + return new FeatureType($connection, $this->tokenStorage, $config); } - protected function getConfigDefaults() + protected function getConfigDefaults(): array { - return parent::getConfigDefaults() + array( + return parent::getConfigDefaults() + [ 'geomField' => 'geom', - ); + ]; } } diff --git a/src/Mapbender/DataSourceBundle/Component/FeatureQueryBuilder.php b/src/Mapbender/DataSourceBundle/Component/FeatureQueryBuilder.php deleted file mode 100644 index 7077455e..00000000 --- a/src/Mapbender/DataSourceBundle/Component/FeatureQueryBuilder.php +++ /dev/null @@ -1,100 +0,0 @@ -driver = $driver; - $this->sourceSrid = $sourceSrid; - } - - /** - * @return int|string|null - */ - public function getSourceSrid() - { - return $this->sourceSrid; - } - - /** - * @return int|string|null - */ - public function getTargetSrid() - { - return $this->targetSrid ?: $this->sourceSrid; - } - - /** - * @param int|string|null $targetSrid - */ - public function setTargetSrid($targetSrid) - { - $this->targetSrid = $targetSrid; - } - - public function select($select = null) - { - $this->isSelect = true; - return parent::select($select); - } - - public function addSelect($select = null) - { - $this->isSelect = true; - return parent::addSelect($select); - } - - public function addGeomSelect($columnName) - { - $this->geomNames[] = $columnName; - } - - public function getSQL() - { - if ($this->isSelect && $this->geomNames) { - if ($this->driver instanceof Geographic) { - $sridTo = $this->getTargetSrid(); - foreach ($this->geomNames as $geomName) { - $geomReference = $this->getConnection()->quoteIdentifier($geomName); - $geomSql = $this->driver->getColumnToEwktSql($geomReference, \intval($sridTo)) - . ' AS ' . $this->getConnection()->quoteIdentifier($geomName); - parent::addSelect($geomSql); - } - } else { - // Uh-oh - foreach ($this->geomNames as $geomName) { - parent::addSelect($geomName); - } - } - // Only do this once (parent buffers getSql result) - $this->geomNames = array(); - } - return parent::getSQL(); - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/FeatureType.php b/src/Mapbender/DataSourceBundle/Component/FeatureType.php index 9fcb9ba3..be6be313 100644 --- a/src/Mapbender/DataSourceBundle/Component/FeatureType.php +++ b/src/Mapbender/DataSourceBundle/Component/FeatureType.php @@ -1,214 +1,450 @@ - * @copyright 2015 by WhereGroup GmbH & Co. KG - * @link https://github.com/mapbender/mapbender-digitizer + * Additional configuration keys (on top of DataStore): + * geomField string Geometry column name (default: "geom") + * srid int Storage SRID; auto-detected from geometry_columns if omitted * * @method Feature save(Feature|array $feature) * @method Feature[] search(array $criteria) - * @method Feature insertItem(Feature $item) - * @method Feature updateItem(Feature $item) - * @method Feature update($itemOrData) * @method Feature insert($itemOrData) - * @method Feature[] getByIds(array $ids) + * @method Feature update($itemOrData) * @method Feature itemFactory() - * @method Feature[] prepareResults(array $rows) - * @method Feature getByIdInternal($id, QueryBuilder $qb) */ class FeatureType extends DataStore { - /** - * @var string Geometry field name - */ - protected $geomField; + protected string $geomField; + protected ?int $configuredSrid; + protected ?int $detectedSrid = null; + + public function __construct( + Connection $connection, + TokenStorageInterface $tokenStorage, + $eventProcessorOrConfig = [], + $config = [], + ) { + // Resolve config (same backward-compat as DataStore) + if (is_array($eventProcessorOrConfig)) { + $config = $eventProcessorOrConfig; + } + + $this->geomField = $config['geomField'] ?? 'geom'; + $this->configuredSrid = !empty($config['srid']) ? (int) $config['srid'] : null; - /** @var int|null fallback source srid used only if detection fails (e.g. materialized views) */ - protected $configuredSrid; + parent::__construct($connection, $tokenStorage, $eventProcessorOrConfig, $config); + } /** - * @var int SRID to get geometry converted to + * @return string Geometry column name */ - protected $srid; + public function getGeomField(): string + { + return $this->geomField; + } - public function __construct(Connection $connection, TokenStorageInterface $tokenStorage, EventProcessor $eventProcessor, $args = array()) + /** + * Get the storage SRID. Auto-detects from geometry_columns if not configured. + * + * @return int + * @throws \RuntimeException if SRID cannot be determined + */ + public function getSrid(): int { - $this->geomField = $args['geomField']; - parent::__construct($connection, $tokenStorage, $eventProcessor, $args); - if ($this->geomField && false !== ($key = \array_search($this->geomField, $this->fields))) { - unset($this->fields[$key]); + if ($this->detectedSrid !== null) { + return $this->detectedSrid; } - if (!empty($args['srid'])) { - $this->configuredSrid = \intval($args['srid']) ?: null; + if ($this->configuredSrid) { + $this->detectedSrid = $this->configuredSrid; + return $this->detectedSrid; } + + $this->detectedSrid = $this->detectSridFromGeometryColumns(); + if (!$this->detectedSrid) { + throw new \RuntimeException( + "SRID detection failure on {$this->tableName}.{$this->geomField}; " + . "supply an 'srid' value in your featureType configuration", + ); + } + return $this->detectedSrid; } - public function getFields() + /** + * @return string[] + */ + public function getFields(): array { - return array_merge(parent::getFields(), array($this->geomField)); + return array_merge(parent::getFields(), [$this->geomField]); } /** - * Get feature by ID and SRID + * Get feature by ID, optionally transforming geometry to a target SRID. * - * @param int $id - * @param int $srid SRID + * @param mixed $id + * @param int|null $srid Target SRID for returned geometry (null = storage SRID) * @return Feature|null */ - public function getById($id, $srid = null) + public function getById($id, $srid = null): ?Feature { - $qb = $this->createQueryBuilder(); - $this->configureSelect($qb, false, array( - 'srid' => $srid, - )); - return $this->getByIdInternal($id, $qb); + $criteria = []; + if ($srid) { + $criteria['srid'] = $srid; + } + $qb = $this->createSelectQueryBuilder($criteria); + $qb->andWhere($this->qi($this->uniqueId) . ' = :pkId'); + $qb->setParameter('pkId', $id); + $this->bindUserName($qb); + + $row = $qb->executeQuery()->fetchAssociative(); + if (!$row) { + return null; + } + return $this->itemFromRow($row); } /** - * @param Feature $feature - * @return Feature|null + * @return Feature + */ + public function itemFactory(): Feature + { + return new Feature([], $this->uniqueId, $this->geomField); + } + + /** + * @param array $attributes + * @return Feature */ - protected function reloadItem($feature) + public function itemFromArray(array $attributes): Feature { - return $this->getById($feature->getId(), $feature->getSrid()); + return new Feature($attributes, $this->uniqueId, $this->geomField); } + // --------------------------------------------------------------- + // Protected overrides + // --------------------------------------------------------------- + /** - * @param DataItem $feature - * @return mixed[] + * @inheritDoc + * Adds geometry as EWKT to the SELECT. */ - protected function prepareStoreValues(DataItem $feature) + protected function createSelectQueryBuilder(array $criteria): \Doctrine\DBAL\Query\QueryBuilder { - $data = parent::prepareStoreValues($feature); - /** @var Feature $feature */ - $ewkt = $feature->getEwkt(); - $meta = $this->getTableMetaData(); - $geomColumnName = $meta->getRealColumnName($this->geomField); - if ($ewkt) { - $tableSrid = $this->getSrid(); - $geomSql = $this->driver->getReadEwktSql($this->connection->quote($ewkt)); - $geomSql = $this->driver->getTransformSql($geomSql, $tableSrid); - if ($this->checkPromoteToCollection($ewkt, $geomColumnName)) { - $geomSql = $this->driver->getPromoteToCollectionSql($geomSql); + $qb = parent::createSelectQueryBuilder($criteria); + + $targetSrid = !empty($criteria['srid']) ? (int) $criteria['srid'] : $this->getSrid(); + $geomRef = $this->qi($this->geomField); + + // Select geometry as EWKT, transformed to target SRID + $qb->addSelect( + "ST_AsEWKT(ST_Transform({$geomRef}, {$targetSrid})) AS {$geomRef}", + ); + + return $qb; + } + + /** + * @inheritDoc + * Adds spatial intersection filter. + */ + protected function addFilters(\Doctrine\DBAL\Query\QueryBuilder $qb, array $criteria): void + { + parent::addFilters($qb, $criteria); + + if (!empty($criteria['intersect'])) { + $clipWkt = $criteria['intersect']; + $geomRef = $this->qi($this->geomField); + + // Determine SRID for the clip geometry + $clipSrid = WktUtility::getEwktSrid($clipWkt); + if (!$clipSrid) { + $clipSrid = !empty($criteria['srid']) ? (int) $criteria['srid'] : $this->getSrid(); + $clipWkt = "SRID={$clipSrid};{$clipWkt}"; } - $data[$geomColumnName] = new Expression($geomSql); + + // Transform clip geometry to storage SRID and intersect + $storageSrid = $this->getSrid(); + $qb->andWhere( + "ST_Intersects({$geomRef}, ST_Transform(ST_GeomFromEWKT(:intersectGeom), {$storageSrid}))", + ); + $qb->setParameter('intersectGeom', $clipWkt); + } + } + + /** + * @inheritDoc + * Adds geometry column with PostGIS INSERT. + */ + protected function insertItem(DataItem $item): DataItem + { + /** @var Feature $item */ + $allProperties = $this->getItemPropertiesForStorage($item); + $storageData = $this->propertyAdapter->prepareStorageData($allProperties); + + $columns = []; + $placeholders = []; + $params = []; + + foreach ($storageData as $col => $value) { + $columns[] = $this->qi($col); + $placeholders[] = '?'; + $params[] = $value; + } + + // Add geometry + $geomSql = $this->buildGeomInsertExpression($item); + if ($geomSql !== null) { + $columns[] = $this->qi($this->geomField); + $placeholders[] = $geomSql['expression']; + $params = array_merge($params, $geomSql['params']); + } + + if (empty($columns)) { + $sql = sprintf( + 'INSERT INTO %s DEFAULT VALUES RETURNING %s', + $this->getTableName(), + $this->qi($this->uniqueId), + ); } else { - $data[$geomColumnName] = null; + $sql = sprintf( + 'INSERT INTO %s (%s) VALUES (%s) RETURNING %s', + $this->getTableName(), + implode(', ', $columns), + implode(', ', $placeholders), + $this->qi($this->uniqueId), + ); } - return $data; + + $stmt = $this->connection->prepare($sql); + $result = $stmt->executeQuery($params); + $id = $result->fetchOne(); + $item->setId($id); + + return $this->reloadItem($item) ?? $item; } /** - * @param string $ewkt - * @param string|null $columnName - * @return boolean + * @inheritDoc + * Adds geometry column with PostGIS UPDATE. */ - protected function checkPromoteToCollection($ewkt, $columnName) + protected function updateItem(DataItem $item): DataItem { - $tableType = $this->getTableMetaData()->getColumn($columnName)->getGeometryType(); - $wktType = WktUtility::getGeometryType($ewkt); + /** @var Feature $item */ + $allProperties = $this->getItemPropertiesForStorage($item); + $storageData = $this->propertyAdapter->prepareStorageData($allProperties); + + $setClauses = []; + $params = []; + + foreach ($storageData as $col => $value) { + $setClauses[] = $this->qi($col) . ' = ?'; + $params[] = $value; + } + + // Geometry SET clause + $geomSql = $this->buildGeomInsertExpression($item); + if ($geomSql !== null) { + $setClauses[] = $this->qi($this->geomField) . ' = ' . $geomSql['expression']; + $params = array_merge($params, $geomSql['params']); + } else { + // Explicitly set geometry to NULL if empty + $setClauses[] = $this->qi($this->geomField) . ' = NULL'; + } + + if (!empty($setClauses)) { + $params[] = $item->getId(); + + $sql = sprintf( + 'UPDATE %s SET %s WHERE %s = ?', + $this->getTableName(), + implode(', ', $setClauses), + $this->qi($this->uniqueId), + ); + + $this->connection->executeStatement($sql, $params); + } - // @todo: document why we would want to promote to collection, and why we only have a Postgis implementation - return $tableType && $wktType != $tableType - && strtoupper($tableType) !== 'GEOMETRY' - && preg_match('#^MULTI#i', $tableType) - && !preg_match('#^MULTI#i', $wktType) - ; + return $this->reloadItem($item) ?? $item; } /** - * @return string + * @inheritDoc + * Includes geometry in the Feature attributes. */ - public function getGeomField() + protected function itemFromRow(array $row): Feature { - return $this->geomField; + $properties = $this->propertyAdapter->extractProperties($row); + $properties[$this->uniqueId] = $row[$this->uniqueId]; + $properties[$this->geomField] = $row[$this->geomField] ?? null; + return $this->itemFromArray($properties); } /** - * Create preinitialized item - * - * @param array $values - * @return Feature - * @since 0.1.16.2 + * @inheritDoc + * Preserves SRID when reloading. + */ + protected function reloadItem(DataItem $item): ?Feature + { + /** @var Feature $item */ + $srid = $item->getSrid(); + return $this->getById($item->getId(), $srid); + } + + /** + * @inheritDoc + * Exclude geometry from the properties passed to the adapter. + */ + protected function getItemPropertiesForStorage(DataItem $item): array + { + $attrs = parent::getItemPropertiesForStorage($item); + unset($attrs[$this->geomField]); + return $attrs; + } + + /** + * @inheritDoc + * Exclude geometry column from the property adapter. */ - public function itemFromArray(array $values) + protected function createPropertyAdapter(array $config): PropertyAdapterInterface { - return new Feature($values, $this->uniqueIdFieldName, $this->geomField); + $storage = $config['propertyStorage'] ?? 'columns'; + + if ($storage === 'json') { + $jsonColumn = $config['propertyColumn'] ?? 'properties'; + return new \Mapbender\DataSourceBundle\Component\PropertyAdapter\JsonColumnAdapter($jsonColumn); + } + + return new DiscreteColumnAdapter( + $this->connection, + $config['table'] ?? '', + $config['fields'] ?? null, + $config['uniqueId'] ?? 'id', + $this->geomField, + ); } + // --------------------------------------------------------------- + // PostGIS SQL helpers + // --------------------------------------------------------------- + /** - * Get SRID + * Build the SQL expression and parameters for inserting/updating geometry. * - * @return int + * @param Feature $feature + * @return array{expression: string, params: array}|null */ - public function getSrid() + private function buildGeomInsertExpression(Feature $feature): ?array { - $this->srid = $this->srid ?: $this->getTableMetaData()->getColumn($this->geomField)->getSrid() ?: $this->configuredSrid; - if (!$this->srid) { - # Throw a decently helpful exception now instead of throwing a - # hard to parse one ("Invalid sridTo 0") later. - throw new \RuntimeException("SRID detection failure on {$this->tableName}.{$this->geomField}; must supply an 'srid' value in your featuretype configuration"); + $ewkt = $feature->getEwkt(); + if (!$ewkt) { + return null; } - return $this->srid; + + $storageSrid = $this->getSrid(); + $geomType = $this->detectGeometryColumnType(); + + // Build: ST_MakeValid(ST_Transform(ST_GeomFromEWKT(?), storageSrid)) + $expr = 'ST_GeomFromEWKT(?)'; + $params = [$ewkt]; + + // Always transform to storage SRID + $expr = "ST_Transform({$expr}, {$storageSrid})"; + + // Promote to multi-geometry if needed + if ($this->shouldPromoteToMulti($ewkt, $geomType)) { + $expr = "ST_Multi({$expr})"; + } + + // Always validate + $expr = "ST_MakeValid({$expr})"; + + return ['expression' => $expr, 'params' => $params]; } /** - * @return FeatureQueryBuilder + * Check if incoming geometry needs promotion to MULTI variant. */ - public function createQueryBuilder() + private function shouldPromoteToMulti(string $ewkt, ?string $columnType): bool { - return new FeatureQueryBuilder($this->connection, $this->driver, $this->getSrid()); + if (!$columnType) { + return false; + } + $upperType = strtoupper($columnType); + if ($upperType === 'GEOMETRY') { + return false; + } + $wktType = WktUtility::getGeometryType($ewkt); + if (!$wktType) { + return false; + } + return preg_match('#^MULTI#i', $columnType) + && !preg_match('#^MULTI#i', $wktType); } - protected function attributesFromRow(array $values) + /** + * Auto-detect geometry column type from geometry_columns. + * + * @return string|null e.g. "MULTIPOLYGON", "POINT", "GEOMETRY" + */ + private function detectGeometryColumnType(): ?string { - $attributes = parent::attributesFromRow($values); - if ($this->geomField && !\array_key_exists($this->geomField, $attributes)) { - $meta = $this->getTableMetaData(); - $attributes[$this->geomField] = $values[$meta->getRealColumnName($this->geomField)]; + $parsed = $this->parseTableName(); + try { + $sql = 'SELECT type FROM geometry_columns WHERE f_table_name = ? AND f_table_schema = ?'; + $result = $this->connection->fetchOne($sql, [$parsed['table'], $parsed['schema']]); + return $result ?: null; + } catch (\Exception) { + return null; } - return $attributes; } - protected function configureSelect(QueryBuilder $queryBuilder, $includeDefaultFilter, array $params) + /** + * Auto-detect SRID from the geometry_columns system table. + * + * @return int|null + */ + private function detectSridFromGeometryColumns(): ?int { - /** @var FeatureQueryBuilder $queryBuilder */ - parent::configureSelect($queryBuilder, $includeDefaultFilter, $params); - $queryBuilder->addGeomSelect($this->geomField); - if (!empty($params['srid'])) { - $queryBuilder->setTargetSrid($params['srid']); + $parsed = $this->parseTableName(); + try { + $sql = 'SELECT srid FROM geometry_columns WHERE f_table_name = ? AND f_table_schema = ? AND f_geometry_column = ?'; + $result = $this->connection->fetchOne($sql, [ + $parsed['table'], + $parsed['schema'], + $this->geomField, + ]); + return $result ? (int) $result : null; + } catch (\Exception) { + return null; } } - protected function addQueryFilters(QueryBuilder $queryBuilder, $includeDefaultFilter, $params) + /** + * Parse table name into schema + table components. + * + * @return array{schema: string, table: string} + */ + private function parseTableName(): array { - parent::addQueryFilters($queryBuilder, $includeDefaultFilter, $params); - // add bounding geometry condition - if (!empty($params['intersect'])) { - $clipWkt = $params['intersect']; - if (!($srid = WktUtility::getEwktSrid($clipWkt))) { - if (!empty($params['srid'])) { - $clipSrid = $params['srid']; - } else { - $clipSrid = $this->getSrid(); - } - $clipWkt = "SRID={$clipSrid};$clipWkt"; - } - $connection = $queryBuilder->getConnection(); - $clipGeomExpression = $this->driver->getReadEwktSql($connection->quote($clipWkt)); - $clipGeomExpression = $this->driver->getTransformSql($clipGeomExpression, $this->getSrid()); - $columnReference = $connection->quoteIdentifier($this->geomField); - $queryBuilder->andWhere($this->driver->getIntersectCondition($columnReference, $clipGeomExpression)); + $unquoted = str_replace('"', '', $this->tableName); + $parts = explode('.', $unquoted); + if (count($parts) === 2) { + return ['schema' => $parts[0], 'table' => $parts[1]]; } + return ['schema' => 'public', 'table' => $parts[0]]; } } diff --git a/src/Mapbender/DataSourceBundle/Component/FeatureTypeService.php b/src/Mapbender/DataSourceBundle/Component/FeatureTypeService.php deleted file mode 100644 index 95e125e3..00000000 --- a/src/Mapbender/DataSourceBundle/Component/FeatureTypeService.php +++ /dev/null @@ -1,22 +0,0 @@ - - * @copyright 18.03.2015 by WhereGroup GmbH & Co. KG - * - * @method FeatureType getDataStoreByName(string $name) - * @method FeatureType getFeatureTypeByName(string $name) - * @method FeatureType get(string $name) - * @method FeatureType dataStoreFactory(array $config) - * @method FeatureType featureTypeFactory(array $config) - * @property FeatureType[] $repositories - * - * @deprecated incompatible with Symfony 4 (full container injection); use RepositoryRegistry and inject `mbds.default_featuretype_factory` - */ -class FeatureTypeService extends DataStoreService -{ - protected $factoryId = 'mbds.default_featuretype_factory'; -} diff --git a/src/Mapbender/DataSourceBundle/Component/Meta/Column.php b/src/Mapbender/DataSourceBundle/Component/Meta/Column.php deleted file mode 100644 index 4257ff74..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Meta/Column.php +++ /dev/null @@ -1,90 +0,0 @@ -nullable = $nullable; - $this->hasDefault = $hasDefault; - $this->isNumeric = $isNumeric; - $this->geometryType = $geometryType; - $this->srid = $srid; - } - - /** - * @return int|string|null - */ - public function getSafeDefault() - { - if ($this->nullable) { - return null; - } elseif ($this->isNumeric) { - return 0; - } else { - return ''; - } - } - - /** - * @return bool - */ - public function isNullable() - { - return $this->nullable; - } - - /** - * @return bool - */ - public function hasDefault() - { - return $this->hasDefault; - } - - /** - * @return bool - */ - public function isNumeric() - { - return $this->isNumeric; - } - - /** - * @return string|null - */ - public function getGeometryType() - { - return $this->geometryType; - } - - /** - * @return int|null - */ - public function getSrid() - { - return $this->srid; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/Meta/TableMeta.php b/src/Mapbender/DataSourceBundle/Component/Meta/TableMeta.php deleted file mode 100644 index 9d56e971..00000000 --- a/src/Mapbender/DataSourceBundle/Component/Meta/TableMeta.php +++ /dev/null @@ -1,98 +0,0 @@ -platform = $platform; - foreach ($columns as $name => $column) { - $this->columns[$name] = $column; - $this->ciColumnIndex[\strtolower($name)] = $name; - } - $this->columns = $columns; - } - - /** - * @return string[] - */ - public function getColumNames() - { - return \array_keys($this->columns); - } - - public function prepareUpdateData(array $data) - { - foreach ($data as $columnName => $value) { - if (\is_string($value) && !$value) { - $column = $this->getColumn($columnName); - // "0" is a well-formed number (work around PHP "0" == false equivalence) - // NOTE: Starting with PHP 8 is_numeric allows trailing whitespace. Avoid that behaviour. - // see https://www.php.net/manual/en/function.is-numeric.php - if ($column->isNumeric() && !\is_numeric(trim($value))) { - $data[$columnName] = $column->getSafeDefault(); - } - } elseif (\is_bool($value) && $this->getColumn($columnName)->isNumeric()) { - $data[$columnName] = $value ? 1 : 0; - } - } - return $data; - } - - public function prepareInsertData(array $data) - { - $data = $this->prepareUpdateData($data); - $dataNames = array(); - foreach (array_keys($data) as $dataKey) { - $dataNames[] = \strtolower($dataKey); - } - foreach (\array_keys($this->ciColumnIndex) as $columnName) { - if (!\in_array($columnName, $dataNames, true)) { - $column = $this->getColumn($columnName); - if (!$column->hasDefault()) { - $data[$columnName] = $column->getSafeDefault(); - } - } - } - return $data; - } - - /** - * @param string $name - * @return Column - * @throws \RuntimeException - */ - public function getColumn($name) - { - $nameNormalized = \strtolower($name); - if (\array_key_exists($nameNormalized, $this->ciColumnIndex)) { - return $this->columns[$this->ciColumnIndex[$nameNormalized]]; - } - throw new \RuntimeException("Unknown column {$name}"); - } - - /** - * @param string $name - * @return string - */ - public function getRealColumnName($name) - { - return $this->ciColumnIndex[\strtolower($name)]; - } -} diff --git a/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/DiscreteColumnAdapter.php b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/DiscreteColumnAdapter.php new file mode 100644 index 00000000..bcf1c13a --- /dev/null +++ b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/DiscreteColumnAdapter.php @@ -0,0 +1,78 @@ +introspectColumns($connection, $tableName); + $exclude = array_filter([$uniqueId, $geomField]); + $this->columns = array_values(array_diff($allColumns, $exclude)); + } + + public function getSelectColumns(): array + { + return $this->columns; + } + + public function extractProperties(array $row): array + { + $props = []; + foreach ($this->columns as $col) { + if (array_key_exists($col, $row)) { + $props[$col] = $row[$col]; + } + } + return $props; + } + + public function prepareStorageData(array $properties): array + { + $known = array_flip($this->columns); + return array_intersect_key($properties, $known); + } + + /** + * Auto-detect column names from the database table schema. + * + * @return string[] + */ + private function introspectColumns(Connection $connection, string $tableName): array + { + $unquoted = str_replace('"', '', $tableName); + $schemaManager = method_exists($connection, 'createSchemaManager') + ? $connection->createSchemaManager() + : $connection->getSchemaManager(); + + $columns = $schemaManager->listTableColumns($unquoted); + return array_map( + fn($col) => $col->getName(), + array_values($columns), + ); + } +} diff --git a/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/JsonColumnAdapter.php b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/JsonColumnAdapter.php new file mode 100644 index 00000000..d4ed3ffa --- /dev/null +++ b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/JsonColumnAdapter.php @@ -0,0 +1,55 @@ +jsonColumn = $jsonColumn; + } + + public function getSelectColumns(): array + { + return [$this->jsonColumn]; + } + + public function extractProperties(array $row): array + { + $raw = $row[$this->jsonColumn] ?? null; + if ($raw === null || $raw === '') { + return []; + } + if (is_string($raw)) { + return json_decode($raw, true) ?: []; + } + // Already decoded (e.g. by PDO) + return is_array($raw) ? $raw : []; + } + + public function prepareStorageData(array $properties): array + { + return [ + $this->jsonColumn => json_encode($properties, JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR), + ]; + } + + public function getJsonColumn(): string + { + return $this->jsonColumn; + } +} diff --git a/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/PropertyAdapterInterface.php b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/PropertyAdapterInterface.php new file mode 100644 index 00000000..7854de26 --- /dev/null +++ b/src/Mapbender/DataSourceBundle/Component/PropertyAdapter/PropertyAdapterInterface.php @@ -0,0 +1,41 @@ + value + */ + public function extractProperties(array $row): array; + + /** + * Converts property key-value pairs into column => value data + * suitable for INSERT or UPDATE. + * Must NOT include uniqueId or geometry columns. + * + * @param array $properties Associative array of property key => value + * @return array Column => value pairs for database storage + */ + public function prepareStorageData(array $properties): array; +} diff --git a/src/Mapbender/DataSourceBundle/DOCUMENTATION.md b/src/Mapbender/DataSourceBundle/DOCUMENTATION.md new file mode 100644 index 00000000..b1e00520 --- /dev/null +++ b/src/Mapbender/DataSourceBundle/DOCUMENTATION.md @@ -0,0 +1,510 @@ +# DataSourceBundle Documentation + +## Overview + +The DataSourceBundle provides database repositories for the Mapbender Digitizer +and DataManager elements. It abstracts CRUD operations on PostgreSQL tables, +with optional PostGIS geometry support. + +### Key concepts + +- **DataStore**: Repository for non-spatial data (used by DataManager) +- **FeatureType**: Repository for spatial data with PostGIS geometry (used by Digitizer) +- **PropertyAdapter**: Pluggable strategy for how properties are stored + - `columns` mode (default): each property is a separate database column + - `json` mode: all properties in a single JSONB column + +### File structure + +``` +DataSourceBundle/ + Component/ + DataStore.php # Non-spatial repository + FeatureType.php # Spatial repository (extends DataStore) + RepositoryRegistry.php # Factory registry and caching + Factory/ + DataStoreFactory.php # Creates DataStore from config + FeatureTypeFactory.php # Creates FeatureType from config + PropertyAdapter/ + PropertyAdapterInterface.php # Adapter contract + DiscreteColumnAdapter.php # One column per property + JsonColumnAdapter.php # JSONB column for all properties + Entity/ + DataItem.php # Non-spatial row entity + Feature.php # Spatial row entity (extends DataItem) + Utils/ + WktUtility.php # WKT/EWKT parsing helpers + Resources/ + config/services.xml # Symfony service definitions + views/fields.html.twig # Form field templates + MapbenderDataSourceBundle.php # Bundle class +``` + +--- + +## Configuration Reference + +### DataStore (DataManager) + +Used for non-spatial tabular data. + +```yaml +schemes: + my_schema: + dataStore: + connection: default # Doctrine DBAL connection name + table: my_table # Table name (supports "schema"."table") + uniqueId: id # Primary key column (default: "id") + fields: [name, email, status] # Column list; omit for auto-detect + filter: "active = true" # Permanent SQL WHERE fragment + propertyStorage: columns # "columns" (default) or "json" + # ... formItems, table, popup, etc. +``` + +### FeatureType (Digitizer) + +Used for spatial data with PostGIS geometry. + +```yaml +schemes: + parcels: + featureType: + connection: default + table: public.parcels + uniqueId: gid + geomField: the_geom # Geometry column (default: "geom") + srid: 25832 # Storage SRID; auto-detected if omitted + fields: [name, area, owner] # Property columns; omit for auto-detect + filter: "deleted = false" + propertyStorage: columns # "columns" (default) or "json" + # ... formItems, table, popup, styles, etc. +``` + +### All configuration keys + +| Key | Type | Default | Description | +|------------------|------------|--------------|------------------------------------------------------| +| `connection` | string | `"default"` | Doctrine DBAL connection name | +| `table` | string | **required** | Table name. Supports schema: `"schema"."table"` | +| `uniqueId` | string | `"id"` | Primary key column name | +| `fields` | array/null | `null` | Columns to use; `null` = auto-detect all | +| `filter` | string | `null` | Permanent SQL WHERE. Supports `:userName` placeholder | +| `geomField` | string | `"geom"` | Geometry column (FeatureType only) | +| `srid` | int | auto-detect | Storage SRID (FeatureType only) | +| `propertyStorage`| string | `"columns"` | `"columns"` or `"json"` | +| `propertyColumn` | string | `"properties"`| JSONB column name (when `propertyStorage: json`) | +| `events` | array | `[]` | **DEPRECATED**, accepted but ignored | + +--- + +## Property Storage Modes + +### Mode: `columns` (default, backward-compatible) + +Each property maps 1:1 to a table column. This is the classic behavior +and what all existing configurations use. + +``` +Table: parcels + gid | the_geom | name | area | owner +-----+-------------------+------------+---------+------- + 1 | POLYGON((...)) | Parcel A | 1250.5 | smith + 2 | POLYGON((...)) | Parcel B | 890.2 | jones +``` + +Configuration: +```yaml +featureType: + table: parcels + uniqueId: gid + geomField: the_geom + srid: 25832 + # propertyStorage: columns ← this is the default, no need to specify +``` + +If `fields` is omitted, all columns are auto-detected from the table schema. +If `fields` is specified, only those columns are used. + +### Mode: `json` + +All properties are stored in a single JSONB column. The table has a +minimal fixed schema: + +``` +Table: digitizer_features + id | geom | properties +----+-------------------+------------------------------------------ + 1 | POLYGON((...)) | {"name": "Parcel A", "area": 1250.5, ...} + 2 | POLYGON((...)) | {"name": "Parcel B", "area": 890.2, ...} +``` + +Configuration: +```yaml +featureType: + table: digitizer_features + uniqueId: id + geomField: geom + srid: 4326 + propertyStorage: json + propertyColumn: properties # default, can be omitted +``` + +To create the table: + +```sql +CREATE TABLE digitizer_features ( + id SERIAL PRIMARY KEY, + geom geometry(Geometry, 4326), + properties JSONB NOT NULL DEFAULT '{}'::jsonb +); +CREATE INDEX idx_digitizer_features_geom ON digitizer_features USING GIST (geom); +CREATE INDEX idx_digitizer_features_props ON digitizer_features USING GIN (properties); +``` + +For DataManager (non-spatial), omit the geom column: + +```sql +CREATE TABLE my_data ( + id SERIAL PRIMARY KEY, + properties JSONB NOT NULL DEFAULT '{}'::jsonb +); +CREATE INDEX idx_my_data_props ON my_data USING GIN (properties); +``` + +### When to use which mode + +| Use case | Mode | +|----------|------| +| Existing table with individual columns | `columns` | +| New project, schema may evolve often | `json` | +| Need SQL-level queries on properties | `columns` | +| Many schemas with few properties each | `json` | +| Properties vary per record | `json` | +| Need database-level constraints | `columns` | + +--- + +## Backward Compatibility + +### Old configurations work unchanged + +Existing configurations that don't use `propertyStorage` or `propertyColumn` +default to `propertyStorage: columns`, which behaves identically to the +previous DataSourceBundle. + +### API surface preserved + +All class names, namespaces, and method signatures that DataManager and +Digitizer use are preserved: + +- `Mapbender\DataSourceBundle\Component\DataStore` +- `Mapbender\DataSourceBundle\Component\FeatureType` +- `Mapbender\DataSourceBundle\Component\RepositoryRegistry` +- `Mapbender\DataSourceBundle\Entity\DataItem` +- `Mapbender\DataSourceBundle\Entity\Feature` +- Service IDs: `mbds.default_datastore_factory`, `mbds.default_featuretype_factory` + +### What was removed + +| Removed | Reason | +|---------|--------| +| Oracle driver | Not used; PostgreSQL only | +| SQLite driver | Not used; PostgreSQL only | +| Multi-database driver abstraction | Replaced by direct PostGIS SQL | +| `EventProcessor` (eval-based events) | Security concern, rarely used | +| `Expression` class | Geometry SQL built inline | +| `FeatureQueryBuilder` | Logic merged into FeatureType | +| `DataRepository` / `EventAwareDataRepository` | Merged into DataStore | +| `DataStoreService` / `FeatureTypeService` | Already deprecated | +| `TableMeta` / `Column` metadata classes | Replaced by DBAL introspection | +| `BaseXmlLoaderExtension` | Already deprecated | + +### `events` configuration key + +The `events` key is still accepted in configuration to avoid parse errors, +but the expressions are no longer executed. If you need lifecycle hooks, +use Symfony's EventDispatcher instead. + +--- + +## PostGIS Geometry Handling (FeatureType) + +### SELECT (reading features) + +Geometry is returned as EWKT, optionally transformed to a target SRID: + +```sql +SELECT id, col1, col2, + ST_AsEWKT(ST_Transform("geom", 4326)) AS "geom" +FROM parcels +``` + +The `srid` criteria parameter controls the output SRID. + +### INSERT + +```sql +INSERT INTO parcels (col1, col2, "geom") +VALUES (?, ?, ST_MakeValid(ST_Transform(ST_GeomFromEWKT(?), 25832))) +RETURNING "gid" +``` + +### UPDATE + +```sql +UPDATE parcels +SET col1 = ?, col2 = ?, + "geom" = ST_MakeValid(ST_Transform(ST_GeomFromEWKT(?), 25832)) +WHERE "gid" = ? +``` + +### Spatial filtering (intersect) + +When the Digitizer sends a map extent, features are filtered with: + +```sql +ST_Intersects("geom", ST_Transform(ST_GeomFromEWKT(?), 25832)) +``` + +### Multi-geometry promotion + +If the table column type is `MULTIPOLYGON` but the incoming geometry is +`POLYGON`, it is automatically wrapped with `ST_Multi()`. + +### SRID auto-detection + +If `srid` is not configured, it is read from the PostGIS `geometry_columns` +system view: + +```sql +SELECT srid FROM geometry_columns +WHERE f_table_name = ? AND f_table_schema = ? AND f_geometry_column = ? +``` + +For materialized views or other cases where this fails, configure `srid` +explicitly. + +--- + +## Repository API + +### DataStore methods + +```php +$store->getConnection(): Connection // DBAL connection +$store->getTableName(): string // Quoted table name +$store->getUniqueId(): string // Primary key column +$store->getFields(): array // All field names +$store->getPlatformName(): string // e.g. "postgresql" + +$store->search(array $criteria): DataItem[] +$store->getById($id): ?DataItem +$store->count(array $criteria): int +$store->save($itemOrData): DataItem // Auto-detect insert/update +$store->insert($itemOrData): DataItem +$store->update($itemOrData): DataItem +$store->remove($itemOrId): ?int // Returns deleted row count + +$store->itemFactory(): DataItem // New empty item +$store->itemFromArray(array): DataItem // Item from attributes +``` + +### FeatureType additional methods + +```php +$ft->getGeomField(): string // Geometry column name +$ft->getSrid(): int // Storage SRID(auto or configured) +$ft->getById($id, $srid): ?Feature // With optional SRID transform +$ft->itemFactory(): Feature +$ft->itemFromArray(array): Feature +``` + +### Search criteria + +```php +$criteria = [ + 'maxResults' => 100, // LIMIT + 'where' => 'status = 1', // Additional SQL WHERE + 'srid' => 4326, // Output SRID (FeatureType) + 'intersect' => 'POLYGON((......))', // Spatial filter WKT (FeatureType) +]; +``` + +### DataItem / Feature entities + +```php +$item->getId() +$item->setId($id) +$item->getAttributes(): array +$item->getAttribute($name): mixed +$item->setAttributes(array $attrs) // Merges into existing +$item->setAttribute($key, $value) +$item->toArray(): array // All attributes + +// Feature only: +$feature->setGeom($wkt) // Set geometry (WKT or EWKT) +$feature->getGeom(): ?string // Get WKT (no SRID prefix) +$feature->getEwkt(): ?string // Get EWKT (with SRID prefix) +$feature->getSrid(): ?int +$feature->setSrid($srid) +$feature->getType(): ?string // e.g. "POLYGON" +``` + +--- + +## Extending: Custom Property Adapters + +Implement `PropertyAdapterInterface` for custom storage strategies: + +```php +use Mapbender\DataSourceBundle\Component\PropertyAdapter\PropertyAdapterInterface; + +class MyCustomAdapter implements PropertyAdapterInterface +{ + public function getSelectColumns(): array + { + // Return column names to SELECT + } + + public function extractProperties(array $row): array + { + // Convert DB row to property key-value pairs + } + + public function prepareStorageData(array $properties): array + { + // Convert properties to column => value for INSERT/UPDATE + } +} +``` + +Override `DataStore::createPropertyAdapter()` or `FeatureType::createPropertyAdapter()` +to use your custom adapter based on configuration. + +--- + +## Example Configurations + +### Digitizer with classic columns + +```yaml +schemes: + buildings: + label: Buildings + featureType: + connection: default + table: public.buildings + uniqueId: gid + geomField: the_geom + srid: 25832 + allowEdit: true + allowCreate: true + allowDelete: true + popup: + title: Edit Building + width: 550px + table: + columns: + - { data: name, title: Name } + - { data: floors, title: Floors } + formItems: + - type: input + name: name + title: Building Name + - type: input + name: floors + title: Number of Floors +``` + +### Digitizer with JSONB storage + +```yaml +schemes: + poi: + label: Points of Interest + featureType: + connection: default + table: poi_features + geomField: geom + srid: 4326 + propertyStorage: json + propertyColumn: properties + allowEdit: true + allowCreate: true + popup: + title: Edit POI + formItems: + - type: input + name: name + title: Name + - type: select + name: category + title: Category + options: + - { label: Restaurant, value: restaurant } + - { label: Hotel, value: hotel } + - { label: Shop, value: shop } +``` + +### DataManager with JSONB storage + +```yaml +schemes: + notes: + label: Notes + dataStore: + connection: default + table: app_notes + propertyStorage: json + allowEdit: true + allowCreate: true + allowDelete: true + formItems: + - type: input + name: title + title: Title + - type: textArea + name: content + title: Content +``` + +### User-filtered data + +```yaml +schemes: + my_items: + label: My Items + dataStore: + connection: default + table: user_items + filter: "owner = :userName" + filterUser: true + trackUser: true + userColumn: owner + formItems: + - type: input + name: description + title: Description +``` + +--- + +## Migration Guide + +### From old DataSourceBundle + +1. **No configuration changes needed** — old YAML configs work as-is +2. **Remove `events` if used** — eval-based events are no longer executed. + Migrate to Symfony EventDispatcher if lifecycle hooks are needed. +3. **Oracle/SQLite** — if you used these databases, they are no longer + supported. Migrate to PostgreSQL. +4. **Custom code** — if you extended `DataRepository`, + `EventAwareDataRepository`, or used the driver classes directly, update + your code to work with the new `DataStore` / `FeatureType` API. + +### Adopting JSONB mode for new schemas + +1. Create the table with the SQL from the "Mode: json" section above +2. Add `propertyStorage: json` to your featureType/dataStore config +3. Form field `name` attributes now map to JSON keys instead of column names +4. Everything else (formItems, table columns, popup) works the same diff --git a/src/Mapbender/DataSourceBundle/DependencyInjection/MapbenderDataSourceExtension.php b/src/Mapbender/DataSourceBundle/DependencyInjection/MapbenderDataSourceExtension.php new file mode 100644 index 00000000..0d0bbbbf --- /dev/null +++ b/src/Mapbender/DataSourceBundle/DependencyInjection/MapbenderDataSourceExtension.php @@ -0,0 +1,145 @@ + 'string', // Most common: geometry(Point,4326), geometry, etc. + 'geography' => 'string', // Geography type (uses geodesic calculations) + 'box2d' => 'string', // PostGIS 2D bounding box + 'box3d' => 'string', // PostGIS 3D bounding box + ]; + + /** + * Prepends PostGIS type mappings into DoctrineBundle's configuration for + * all declared connections. Runs at container compile time. + */ + public function prepend(ContainerBuilder $container): void + { + if (!$container->hasExtension('doctrine')) { + return; + } + + $connectionNames = $this->collectConnectionNames($container); + + $connectionsConfig = []; + foreach ($connectionNames as $name) { + $connectionsConfig[$name] = [ + 'mapping_types' => self::POSTGIS_TYPE_MAPPINGS, + ]; + } + + $container->prependExtensionConfig('doctrine', [ + 'dbal' => [ + 'connections' => $connectionsConfig, + ], + ]); + } + + public function load(array $configs, ContainerBuilder $container): void + { + // DataSourceBundle has no user-facing bundle configuration. + // Service definitions are loaded in MapbenderDataSourceBundle::build(). + } + + /** + * Collects all connection names from existing doctrine configuration blocks. + * Falls back to ['default'] if no connections are explicitly defined + * (covers the case where DoctrineBundle is configured with only a default url). + * + * @return string[] + */ + private function collectConnectionNames(ContainerBuilder $container): array + { + $names = []; + foreach ($container->getExtensionConfig('doctrine') as $config) { + if (isset($config['dbal']['connections']) && is_array($config['dbal']['connections'])) { + foreach (array_keys($config['dbal']['connections']) as $name) { + $names[$name] = true; + } + } + } + + return !empty($names) ? array_keys($names) : ['default']; + } +} diff --git a/src/Mapbender/DataSourceBundle/EVENT-MIGRATION.md b/src/Mapbender/DataSourceBundle/EVENT-MIGRATION.md new file mode 100644 index 00000000..57449b0c --- /dev/null +++ b/src/Mapbender/DataSourceBundle/EVENT-MIGRATION.md @@ -0,0 +1,427 @@ +# Event System Migration Guide + +## Overview + +The old **eval-based event system** (`EventProcessor` / `EventAwareDataRepository`) has been +**removed** from DataSourceBundle. This document explains what changed, why, and how to +replace event-driven behavior with safe alternatives. + +--- + +## What Was Removed + +### Server-Side PHP Events (Security Risk) + +The old architecture allowed YAML/database configuration to contain **arbitrary PHP code** +that was executed via `eval()` at lifecycle hooks: + +```yaml +# OLD — no longer executed +dataStore: + table: my_table + events: + onBeforeSave: "$feature->setAttribute('updated_by', $user);" + onAfterSave: "$connection->executeStatement(...);" + onBeforeRemove: "$logger->info('Deleting ' . $feature->getId());" + onAfterRemove: "// cleanup code" + onBeforeSearch: "$criteria->addFilter(...);" + onAfterSearch: "$features = array_filter($features, ...);" +``` + +**Why it was removed:** + +| Risk | Description | +|------|-------------| +| Arbitrary code execution | `eval()` runs any PHP code — database access, file system, network calls | +| No sandboxing | The code had full access to `$connection`, `$tokenStorage`, `$user` | +| Stored in database | Configurations are often stored in the Mapbender DB, meaning any admin with element-edit access could inject server-side code | +| Invisible attack surface | No code review possible for DB-stored eval strings | +| PHP 8 deprecations | Dynamic eval patterns conflict with strict typing and static analysis | + +**Current behavior:** The `events` configuration key is still **accepted** in YAML/DB configuration +but is **silently ignored**. No PHP eval hooks are executed. + +### Removed Files + +| File | Role | +|------|------| +| `Component/EventProcessor.php` | Executed `eval()` on configured PHP strings | +| `Component/EventAwareDataRepository.php` | Wrapped DataRepository with lifecycle hook calls | + +### Client-Side JS Eval (Deprecated, Still Active) + +The `FormRenderer.js` still supports `eval()`-based JavaScript event handlers on form fields +(`filled` and `change` events configured as strings). These log a **deprecation error** to +the browser console: + +> "Using eval'd Javascript in the configuration is deprecated. Add event handlers to your project code." + +This will also be removed in a future version. + +--- + +## Events That Are Still Active + +These events **do work** and are the recommended extension points: + +### JavaScript jQuery Events + +| Event | Triggered On | When | Data | +|-------|-------------|------|------| +| `data.manager.item.saved` | Element `$element` | After successful save | `{item, itemId, schema, originalId, schemaName, uniqueIdKey, originator}` | +| `data.manager.item.deleted` | Element `$element` | After successful delete | `{item, itemId, schema, schemaName, originator}` | +| `filled` | Form field `$input` | After form populated with item data | Native jQuery event | + +**Usage example:** +```javascript +// In your project JavaScript (e.g., a custom Mapbender element or inline script) +var $digitizer = $('.mb-element-digitizer'); +$digitizer.on('data.manager.item.saved', function(event, eventData) { + console.log('Saved item:', eventData.itemId, 'in schema:', eventData.schemaName); + // eventData.item contains the full row data + // eventData.originalId is null for newly created items +}); + +$digitizer.on('data.manager.item.deleted', function(event, eventData) { + console.log('Deleted item:', eventData.itemId); +}); +``` + +### OpenLayers Map Events (Digitizer only) + +| Event | Source | When | +|-------|--------|------| +| `MOVEEND` | `ol.Map` | Map panned/zoomed (triggers extent-based reload) | +| `mbmapsrschanged` | Mapbender map element | SRS changed | +| `DRAWEND` | `ol.interaction.Draw` | New geometry drawn | +| `MODIFYEND` | `ol.interaction.Modify` | Geometry modified | + +--- + +## How to Replace Removed Server-Side Events + +### Pattern 1: `onBeforeSave` — Set Audit Fields + +**Old (removed):** +```yaml +events: + onBeforeSave: "$feature->setAttribute('user_of_last_edit', $user);" +``` + +**New — Use a PostgreSQL trigger:** +```sql +-- Create a function that sets the audit column +CREATE OR REPLACE FUNCTION set_edit_user() +RETURNS TRIGGER AS $$ +BEGIN + -- Use the application_name set by the connection, or current_user + NEW.user_of_last_edit := current_setting('app.current_user', true); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Attach to your table +CREATE TRIGGER trg_set_edit_user + BEFORE INSERT OR UPDATE ON my_table + FOR EACH ROW EXECUTE FUNCTION set_edit_user(); +``` + +To pass the current user from PHP to PostgreSQL, set a session variable in a +Doctrine event listener: + +```php +// src/EventListener/SetDatabaseUserListener.php +namespace App\EventListener; + +use Doctrine\DBAL\Event\ConnectionEventArgs; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; + +class SetDatabaseUserListener +{ + public function __construct(private TokenStorageInterface $tokenStorage) {} + + public function postConnect(ConnectionEventArgs $args): void + { + $user = $this->tokenStorage->getToken()?->getUserIdentifier() ?? 'anonymous'; + $args->getConnection()->executeStatement( + "SET app.current_user = " . $args->getConnection()->quote($user) + ); + } +} +``` + +Register the listener in `services.yaml`: +```yaml +services: + App\EventListener\SetDatabaseUserListener: + tags: + - { name: doctrine.event_listener, event: postConnect, connection: geodata_db } +``` + +### Pattern 2: `onBeforeSave` — Validate or Transform Data + +**Old (removed):** +```yaml +events: + onBeforeSave: | + if (empty($feature->getAttribute('name'))) { + throw new \Exception('Name is required'); + } +``` + +**New — Override `DataStore::insertItem()` / `updateItem()`:** + +Create a custom DataStore subclass: + +```php +// src/Component/ValidatingDataStore.php +namespace App\Component; + +use Mapbender\DataSourceBundle\Component\DataStore; +use Mapbender\DataSourceBundle\Entity\DataItem; + +class ValidatingDataStore extends DataStore +{ + public function save(DataItem $item): DataItem + { + $attrs = $item->getAttributes(); + if (empty($attrs['name'])) { + throw new \InvalidArgumentException('Name is required'); + } + // Transform data before save + $item->setAttribute('name', trim($attrs['name'])); + return parent::save($item); + } +} +``` + +Register as a custom factory: + +```php +// src/Component/Factory/ValidatingDataStoreFactory.php +namespace App\Component\Factory; + +use App\Component\ValidatingDataStore; +use Mapbender\DataSourceBundle\Component\Factory\DataStoreFactory; + +class ValidatingDataStoreFactory extends DataStoreFactory +{ + protected function createInstance($connection, $tokenStorage, $config): ValidatingDataStore + { + return new ValidatingDataStore($connection, $tokenStorage, $config); + } +} +``` + +### Pattern 3: `onAfterSave` — Trigger Side Effects + +**Old (removed):** +```yaml +events: + onAfterSave: "$connection->executeStatement('UPDATE log SET ...');" +``` + +**New — Use Symfony EventDispatcher:** + +1. Create a custom event: +```php +// src/Event/DataItemSavedEvent.php +namespace App\Event; + +use Mapbender\DataSourceBundle\Entity\DataItem; +use Symfony\Contracts\EventDispatcher\Event; + +class DataItemSavedEvent extends Event +{ + public const NAME = 'data_store.item.saved'; + + public function __construct( + public readonly DataItem $item, + public readonly string $tableName, + public readonly bool $isNew, + ) {} +} +``` + +2. Create an event subscriber: +```php +// src/EventSubscriber/DataItemSubscriber.php +namespace App\EventSubscriber; + +use App\Event\DataItemSavedEvent; +use Doctrine\DBAL\Connection; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class DataItemSubscriber implements EventSubscriberInterface +{ + public function __construct(private Connection $geodataConnection) {} + + public static function getSubscribedEvents(): array + { + return [DataItemSavedEvent::NAME => 'onItemSaved']; + } + + public function onItemSaved(DataItemSavedEvent $event): void + { + // Your post-save logic here + $this->geodataConnection->executeStatement( + 'INSERT INTO audit_log (table_name, item_id, action) VALUES (?, ?, ?)', + [$event->tableName, $event->item->getId(), $event->isNew ? 'INSERT' : 'UPDATE'] + ); + } +} +``` + +3. Dispatch from a custom DataStore: +```php +public function save(DataItem $item): DataItem +{ + $isNew = !$item->getId(); + $result = parent::save($item); + $this->eventDispatcher->dispatch( + new DataItemSavedEvent($result, $this->tableName, $isNew), + DataItemSavedEvent::NAME + ); + return $result; +} +``` + +### Pattern 4: `onBeforeSearch` / `onAfterSearch` — Filter Results + +**Old (removed):** +```yaml +events: + onBeforeSearch: "$criteria->where('status', 'active');" + onAfterSearch: "$features = array_filter($features, fn($f) => $f->getAttribute('visible'));" +``` + +**New — Use the `filter` configuration:** +```yaml +dataStore: + table: my_table + # Static SQL WHERE clause — supports :userName placeholder + filter: "status = 'active' AND visible = true" +``` + +For dynamic filtering, override `DataStore::search()`: +```php +class FilteredDataStore extends DataStore +{ + public function search(array $criteria = []): array + { + // Add custom criteria before query + $items = parent::search($criteria); + // Post-filter if needed + return array_filter($items, function($item) { + return $item->getAttribute('visible') !== false; + }); + } +} +``` + +### Pattern 5: `onBeforeRemove` / `onAfterRemove` — Cascade or Prevent Delete + +**Old (removed):** +```yaml +events: + onBeforeRemove: | + if ($feature->getAttribute('locked')) throw new \Exception('Cannot delete locked items'); +``` + +**New — Use database constraints or override `remove()`:** + +```sql +-- Database-level cascade +ALTER TABLE child_table + ADD CONSTRAINT fk_parent + FOREIGN KEY (parent_id) REFERENCES my_table(id) + ON DELETE CASCADE; +``` + +Or override in PHP: +```php +class ProtectedDataStore extends DataStore +{ + public function remove($itemOrId): void + { + $item = is_object($itemOrId) ? $itemOrId : $this->getById($itemOrId); + if ($item && $item->getAttribute('locked')) { + throw new \RuntimeException('Cannot delete locked items'); + } + parent::remove($itemOrId); + } +} +``` + +--- + +## How to Replace Client-Side Eval Events + +### Form Field `change` / `filled` Handlers + +**Old (deprecated, still works but logs error):** +```yaml +formItems: + - type: input + name: area_m2 + change: "var val = parseFloat($(el).val()); $('[name=area_ha]').val(val / 10000);" +``` + +**New — Use project JavaScript:** +```javascript +// In your project JS file (loaded via getRequiredAssets override or custom element) +$(document).on('change', '[name=area_m2]', function() { + var val = parseFloat($(this).val()); + $('[name=area_ha]').val(val / 10000); +}); +``` + +Or attach handlers after form render using the `filled` jQuery event: +```javascript +var $digitizer = $('.mb-element-digitizer'); +$digitizer.on('data.manager.item.saved', function(e, data) { + // React to save +}); + +// After form is populated, add custom logic +$(document).on('filled', '[name=my_field]', function() { + // Custom initialization after form data is loaded +}); +``` + +--- + +## Migration Checklist + +1. **Search your database** for element configurations containing `events:` keys: + ```sql + SELECT id, title, configuration + FROM mb_core_element + WHERE configuration LIKE '%events%' + AND (configuration LIKE '%onBefore%' OR configuration LIKE '%onAfter%'); + ``` + +2. **For each event handler found**, implement the replacement using one of the patterns above. + +3. **Remove the `events` key** from the configuration (it's ignored but creates confusion). + +4. **Replace JS eval handlers** in `formItems` (`change:` and `filled:` string values) with + proper JavaScript event listeners in your project code. + +5. **Test thoroughly** — the old events ran synchronously before/after each operation. + Database triggers and Symfony event subscribers may have subtle timing differences. + +--- + +## Summary Table + +| Old Event | Replacement Strategy | Complexity | +|-----------|---------------------|------------| +| `onBeforeSave` (set fields) | PostgreSQL trigger + session variable | Medium | +| `onBeforeSave` (validate) | Custom DataStore subclass override | Low | +| `onAfterSave` (side effects) | Symfony EventDispatcher + custom DataStore | Medium | +| `onBeforeSearch` (filter) | `filter` config key or DataStore override | Low | +| `onAfterSearch` (transform) | DataStore subclass override `search()` | Low | +| `onBeforeRemove` (prevent) | Database constraints or DataStore override | Low | +| `onAfterRemove` (cleanup) | Database CASCADE or Symfony event | Low–Medium | +| JS `change`/`filled` eval | Project JavaScript with jQuery `.on()` | Low | diff --git a/src/Mapbender/DataSourceBundle/Entity/DataItem.php b/src/Mapbender/DataSourceBundle/Entity/DataItem.php index ae5082a1..fb259676 100644 --- a/src/Mapbender/DataSourceBundle/Entity/DataItem.php +++ b/src/Mapbender/DataSourceBundle/Entity/DataItem.php @@ -1,9 +1,6 @@ - */ class DataItem implements \ArrayAccess { /** @var mixed[] */ diff --git a/src/Mapbender/DataSourceBundle/Entity/Feature.php b/src/Mapbender/DataSourceBundle/Entity/Feature.php index 9bccfdc5..dae3dd93 100644 --- a/src/Mapbender/DataSourceBundle/Entity/Feature.php +++ b/src/Mapbender/DataSourceBundle/Entity/Feature.php @@ -3,9 +3,6 @@ use Mapbender\DataSourceBundle\Utils\WktUtility; -/** - * @author Andriy Oblivantsev - */ class Feature extends DataItem { /** @var string|null */ diff --git a/src/Mapbender/DataSourceBundle/Extension/BaseXmlLoaderExtension.php b/src/Mapbender/DataSourceBundle/Extension/BaseXmlLoaderExtension.php deleted file mode 100644 index 0384c7a2..00000000 --- a/src/Mapbender/DataSourceBundle/Extension/BaseXmlLoaderExtension.php +++ /dev/null @@ -1,41 +0,0 @@ - - * - * @deprecated use Bundle::build to load configuration - */ -class BaseXmlLoaderExtension extends Extension -{ - protected $xmlFileName = 'services.xml'; - protected $xmlFilePath = '/../Resources/config'; - protected $reflector; - - /** - * Constructor - */ - public function __construct() - { - $this->reflector = new \ReflectionClass(get_class($this)); - } - - /** - * Loads a specific configuration. - * - * @param array $configs - * @param ContainerBuilder $container - */ - public function load(array $configs, ContainerBuilder $container): void - { - $fileSrc = dirname($this->reflector->getFileName()) . $this->xmlFilePath; - $loader = new XmlFileLoader($container, new FileLocator($fileSrc)); - $loader->load($this->xmlFileName); - } -} diff --git a/src/Mapbender/DataSourceBundle/MapbenderDataSourceBundle.php b/src/Mapbender/DataSourceBundle/MapbenderDataSourceBundle.php index e4fe902e..8f0de534 100644 --- a/src/Mapbender/DataSourceBundle/MapbenderDataSourceBundle.php +++ b/src/Mapbender/DataSourceBundle/MapbenderDataSourceBundle.php @@ -1,6 +1,7 @@ addResource(new FileResource($xmlLoader->getLocator()->locate('services.xml'))); } + /** + * Returns the DI extension that auto-registers PostGIS type mappings for Doctrine. + * + * Previously this returned null because the bundle had no DI configuration needs. + * The extension was added to automatically inject PostGIS mapping_types into + * DoctrineBundle's configuration, so projects using DataSourceBundle don't need + * manual doctrine.yaml entries for geometry/geography column types. + * + * @see MapbenderDataSourceExtension for detailed rationale + */ public function getContainerExtension(): ?ExtensionInterface { - return null; + return new MapbenderDataSourceExtension(); } } diff --git a/src/Mapbender/DataSourceBundle/Resources/config/services.xml b/src/Mapbender/DataSourceBundle/Resources/config/services.xml index ea2829fe..ce8ef819 100644 --- a/src/Mapbender/DataSourceBundle/Resources/config/services.xml +++ b/src/Mapbender/DataSourceBundle/Resources/config/services.xml @@ -3,22 +3,13 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - - - - - - - - diff --git a/src/Mapbender/DataSourceBundle/Resources/views/fields.html.twig b/src/Mapbender/DataSourceBundle/Resources/views/fields.html.twig deleted file mode 100644 index d44e11ae..00000000 --- a/src/Mapbender/DataSourceBundle/Resources/views/fields.html.twig +++ /dev/null @@ -1,239 +0,0 @@ -{% use "form_div_layout.html.twig" %} -{# Widgets #} - -{% block form_widget_simple -%} - {% if type is not defined or 'file' != type %} - {%- set attr = attr|merge({class: (attr.class|default('') ~ ' form-control')|trim}) -%} - {% endif %} - {{- parent() -}} -{%- endblock form_widget_simple %} - -{% block textarea_widget -%} - {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-control')|trim}) %} - {{- parent() -}} -{%- endblock textarea_widget %} - -{% block button_widget -%} - {% set attr = attr|merge({class: (attr.class|default('btn-default') ~ ' btn')|trim}) %} - {{- parent() -}} -{%- endblock %} - -{% block money_widget -%} -
- {% set prepend = '{{' == money_pattern[0:2] %} - {% if not prepend %} - {{ money_pattern|replace({ '{{ widget }}':''}) }} - {% endif %} - {{- block('form_widget_simple') -}} - {% if prepend %} - {{ money_pattern|replace({ '{{ widget }}':''}) }} - {% endif %} -
-{%- endblock money_widget %} - -{% block percent_widget -%} -
- {{- block('form_widget_simple') -}} - % -
-{%- endblock percent_widget %} - -{% block datetime_widget -%} - {% if widget == 'single_text' %} - {{- block('form_widget_simple') -}} - {% else -%} - {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-inline')|trim}) -%} -
- {{- form_errors(form.date) -}} - {{- form_errors(form.time) -}} - {{- form_widget(form.date, { datetime: true } ) -}} - {{- form_widget(form.time, { datetime: true } ) -}} -
- {%- endif %} -{%- endblock datetime_widget %} - -{% block date_widget -%} - {% if widget == 'single_text' %} - {{- block('form_widget_simple') -}} - {% else -%} - {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-inline')|trim}) -%} - {% if datetime is not defined or not datetime -%} -
- {%- endif %} - {{- date_pattern|replace({ - '{{ year }}': form_widget(form.year), - '{{ month }}': form_widget(form.month), - '{{ day }}': form_widget(form.day), - })|raw -}} - {% if datetime is not defined or not datetime -%} -
- {%- endif -%} - {% endif %} -{%- endblock date_widget %} - -{% block time_widget -%} - {% if widget == 'single_text' %} - {{- block('form_widget_simple') -}} - {% else -%} - {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-inline')|trim}) -%} - {% if datetime is not defined or false == datetime -%} -
- {%- endif -%} - {{- form_widget(form.hour) }}{% if with_minutes %}:{{ form_widget(form.minute) }}{% endif %}{% if with_seconds %}:{{ form_widget(form.second) }}{% endif %} - {% if datetime is not defined or false == datetime -%} -
- {%- endif -%} - {% endif %} -{%- endblock time_widget %} - -{% block choice_widget_collapsed -%} - {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-control')|trim}) %} - {{- parent() -}} -{%- endblock %} - -{% block choice_widget_expanded -%} - {% if '-inline' in label_attr.class|default('') -%} -
- {%- for child in form %} - {{- form_widget(child, { - parent_label_class: label_attr.class|default(''), - translation_domain: choice_translation_domain, - }) -}} - {% endfor -%} -
- {%- else -%} -
- {%- for child in form %} - {{- form_widget(child, { - parent_label_class: label_attr.class|default(''), - translation_domain: choice_translation_domain, - }) -}} - {% endfor -%} -
- {%- endif %} -{%- endblock choice_widget_expanded %} - -{% block checkbox_widget -%} - {% set parent_label_class = parent_label_class|default('') -%} - {% if 'checkbox-inline' in parent_label_class %} - {{- form_label(form, null, { widget: parent() }) -}} - {% else -%} -
- {{- form_label(form, null, { widget: parent() }) -}} -
- {%- endif %} -{%- endblock checkbox_widget %} - -{% block radio_widget -%} - {%- set parent_label_class = parent_label_class|default('') -%} - {% if 'radio-inline' in parent_label_class %} - {{- form_label(form, null, { widget: parent() }) -}} - {% else -%} -
- {{- form_label(form, null, { widget: parent() }) -}} -
- {%- endif %} -{%- endblock radio_widget %} - -{# Labels #} - -{% block form_label -%} - {%- set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' control-label')|trim}) -%} - {{- parent() -}} -{%- endblock form_label %} - -{% block choice_label -%} - {# remove the checkbox-inline and radio-inline class, it's only useful for embed labels #} - {%- set label_attr = label_attr|merge({class: label_attr.class|default('')|replace({'checkbox-inline': '', 'radio-inline': ''})|trim}) -%} - {{- block('form_label') -}} -{% endblock %} - -{% block checkbox_label -%} - {{- block('checkbox_radio_label') -}} -{%- endblock checkbox_label %} - -{% block radio_label -%} - {{- block('checkbox_radio_label') -}} -{%- endblock radio_label %} - -{% block checkbox_radio_label %} - {# Do not display the label if widget is not defined in order to prevent double label rendering #} - {% if widget is defined %} - {% if required %} - {% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' required')|trim}) %} - {% endif %} - {% if parent_label_class is defined %} - {% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' ' ~ parent_label_class)|trim}) %} - {% endif %} - {% if label is not same as(false) and label is empty %} - {% set label = name|humanize %} - {% endif %} - - {{- widget|raw }} {{ label is not same as(false) ? (translation_domain is same as(false) ? label : label|trans({}, translation_domain)) -}} - - {% endif %} -{% endblock checkbox_radio_label %} - -{# Rows #} - -{% block form_row -%} -
- {{- form_label(form) -}} - {{- form_widget(form) -}} - {{- form_errors(form) -}} -
-{%- endblock form_row %} - -{% block button_row -%} -
- {{- form_widget(form) -}} -
-{%- endblock button_row %} - -{% block choice_row -%} - {% set force_error = true %} - {{- block('form_row') }} -{%- endblock choice_row %} - -{% block date_row -%} - {% set force_error = true %} - {{- block('form_row') }} -{%- endblock date_row %} - -{% block time_row -%} - {% set force_error = true %} - {{- block('form_row') }} -{%- endblock time_row %} - -{% block datetime_row -%} - {% set force_error = true %} - {{- block('form_row') }} -{%- endblock datetime_row %} - -{% block checkbox_row -%} -
- {{- form_widget(form) -}} - {{- form_errors(form) -}} -
-{%- endblock checkbox_row %} - -{% block radio_row -%} -
- {{- form_widget(form) -}} - {{- form_errors(form) -}} -
-{%- endblock radio_row %} - -{# Errors #} - -{% block form_errors -%} - {% if errors|length > 0 -%} - {% if form.parent %}{% else %}
{% endif %} -
    - {%- for error in errors -%} -
  • {{ error.message }}
    • - {%- endfor -%} -
- {% if form.parent %}{% else %}
{% endif %} - {%- endif %} -{%- endblock form_errors %} diff --git a/src/Mapbender/DataSourceBundle/Tests/FeatureTypeTest.php b/src/Mapbender/DataSourceBundle/Tests/FeatureTypeTest.php deleted file mode 100644 index abbc2a19..00000000 --- a/src/Mapbender/DataSourceBundle/Tests/FeatureTypeTest.php +++ /dev/null @@ -1,62 +0,0 @@ - - */ -class FeatureTypeTest extends TestCase -{ - // The OGC and ISO specifications - const WKT_POINT = "POINT(0 0)"; - const WKT_LINESTRING = "LINESTRING(0 0,1 1,1 2)"; - const WKT_POLYGON = "POLYGON((0 0,4 0,4 4,0 4,0 0),(1 1, 2 1, 2 2, 1 2,1 1))"; - const WKT_MULTIPOINT = "MULTIPOINT(0 0,1 2)"; - const WKT_MULTILINESTRING = "MULTILINESTRING((0 0,1 1,1 2),(2 3,3 2,5 4))"; - const WKT_MULTIPOLYGON = "MULTIPOLYGON(((0 0,4 0,4 4,0 4,0 0),(1 1,2 1,2 2,1 2,1 1)), ((-1 -1,-1 -2,-2 -2,-2 -1,-1 -1)))"; - const WKT_GEOMETRYCOLLECTION = "GEOMETRYCOLLECTION(POINT(2 3),LINESTRING(2 3,3 4))"; - - // PostGIS extended specifications - const WKT_MULTIPOINTM = "MULTIPOINTM(0 0 0,1 2 1)"; - const WKT_GEOMETRYCOLLECTIONM = "GEOMETRYCOLLECTIONM( POINTM(2 3 9), LINESTRINGM(2 3 4, 3 4 5) )"; - const WKT_MULTICURVE = "MULTICURVE( (0 0, 5 5), CIRCULARSTRING(4 0, 4 4, 8 4) )( (0 0, 5 5), CIRCULARSTRING(4 0, 4 4, 8 4) )"; - const WKT_POLYHEDRALSURFACE = "POLYHEDRALSURFACE( ((0 0 0, 0 0 1, 0 1 1, 0 1 0, 0 0 0)), ((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0)), ((0 0 0, 1 0 0, 1 0 1, 0 0 1, 0 -0 0)), ((1 1 0, 1 1 1, 1 0 1, 1 0 0, 1 1 0)), ((0 1 0, 0 1 1, 1 1 1, 1 1 0, 0 1 0)), ((0 0 1, 1 0 1, 1 1 1, 0 1 1, 0 0 1)) )"; - const WKT_TRIANGLE = "TRIANGLE ((0 0, 0 9, 9 0, 0 0))"; - const WKT_TIN = "TIN( ((0 0 0, 0 0 1, 0 1 0, 0 0 0)), ((0 0 0, 0 1 0, 1 1 0, 0 0 0)) )"; - const WKT_CIRCULARSTRING = "CIRCULARSTRING(0 0, 1 1, 1 0)"; - const WKT_COMPOUNDCURVE = "COMPOUNDCURVE(CIRCULARSTRING(0 0, 1 1, 1 0),(1 0, 0 1))"; - const WKT_CURVEPOLYGON = "CURVEPOLYGON(CIRCULARSTRING(0 0, 4 0, 4 4, 0 4, 0 0),(1 1, 3 3, 3 1, 1 1))"; - const WKT_MULTISURFACE = "MULTISURFACE(CURVEPOLYGON(CIRCULARSTRING(0 0, 4 0, 4 4, 0 4, 0 0),(1 1, 3 3, 3 1, 1 1)),((10 10, 14 12, 11 10, -10 10),(11 11, 11.5 11, 11 11.5, 11 11)))"; - - - /** - * Test Feature constructor and geometry getter - */ - public function testGeometries() - { - foreach (array( - self::WKT_POINT, - self::WKT_POLYGON, - self::WKT_LINESTRING, - self::WKT_MULTIPOINT, - self::WKT_MULTILINESTRING, - self::WKT_MULTIPOLYGON, - self::WKT_GEOMETRYCOLLECTION, - ) as $wkt) { - - $srid = 4326; - $geomFieldName = 'geom'; - $uniqueIdField = 'id'; - $feature = new Feature(array( - $geomFieldName => "SRID={$srid};$wkt", - ), $uniqueIdField, $geomFieldName); - - $this->assertEquals($feature->getGeom(), $wkt); - $this->assertEquals($feature->getSrid(), $srid); - } - } -} diff --git a/src/Mapbender/DataSourceBundle/Tests/FeaturesTest.php b/src/Mapbender/DataSourceBundle/Tests/FeaturesTest.php deleted file mode 100644 index e63b70d6..00000000 --- a/src/Mapbender/DataSourceBundle/Tests/FeaturesTest.php +++ /dev/null @@ -1,97 +0,0 @@ - - * @copyright 2015 by WhereGroup GmbH & Co. KG - */ -class FeaturesTest extends KernelTestCase -{ - /** - * @var FeatureType - */ - protected static $featureType; - - public static function setUpBeforeClass(): void - { - $kernel = self::bootKernel(); - $container = $kernel->getContainer(); - if (!$container->hasParameter('featureTypes')) { - self::markTestSkipped("Missing featureTypes container param"); - } - $definitions = $container->getParameter('featureTypes'); - $ftConfig = \array_values($definitions)[0]; - /** @var FeatureTypeFactory $ftFactory */ - $ftFactory = $container->get('mbds.default_featuretype_factory'); - self::$featureType = $ftFactory->fromConfig($ftConfig); - self::$fieldName = current(self::$featureType->getFields()); - } - - public function testSearch() - { - self::$featureType->search(array()); - } - - public function testCustomSearch() - { - $results = self::$featureType->search(array('maxResults' => 1)); - $this->assertTrue(is_array($results)); - $this->assertTrue(count($results) <= 1); - } - - public function testSaveArray() - { - $idName = self::$featureType->getUniqueId(); - $featureData = array($idName => "testSaveArray"); - $feature = self::$featureType->save($featureData); - $this->assertTrue($feature instanceof Feature); - } - - public function testSaveObject() - { - $idName = self::$featureType->getUniqueId(); - $featureData = array($idName => "testSaveObject"); - $feature = self::$featureType->itemFromArray($featureData); - $this->assertTrue($feature instanceof Feature); - $feature = self::$featureType->save($feature); - $this->assertTrue($feature instanceof Feature); - } - - public function testGetById() - { - $originFeature = $this->getRandomFeature(); - $feature = self::$featureType->getById($originFeature->getId()); - $this->assertTrue($feature instanceof Feature); - $this->assertTrue($feature->getId() == $originFeature->getId(), "ID is incorrect"); - } - - public function testRemove() - { - $featureType = self::$featureType; - $this->assertGreaterThan(0, $featureType->remove("testSaveArray")); - $this->assertGreaterThan(0, $featureType->remove("testSaveObject")); - } - - public function testUpdate() - { - $originFeature = $this->getRandomFeature(); - self::$featureType->save($originFeature); - } - - /** - * @param int $maxResults - * @return Feature - */ - private function getRandomFeature($maxResults = 10) - { - $features = self::$featureType->search(array('maxResults' => $maxResults)); - $originFeature = $features[ rand(1, count($features)) - 1 ]; - return $originFeature; - } -} From d233397409076e56374da64e252d07b204718432 Mon Sep 17 00:00:00 2001 From: frauch Date: Fri, 27 Feb 2026 12:05:53 +0100 Subject: [PATCH 2/4] refactor: Clean up DataStore and related components for improved clarity and performance --- .../Component/SchemaFilter.php | 6 +- .../DataSourceBundle/Component/DataStore.php | 105 +++++++++++--- .../Component/Factory/DataStoreFactory.php | 4 +- .../Component/FeatureType.php | 135 +++++------------- .../Component/RepositoryRegistry.php | 67 ++------- .../DataSourceBundle/Entity/DataItem.php | 56 +++----- .../DataSourceBundle/Entity/Feature.php | 44 ++---- .../DataSourceBundle/Utils/WktUtility.php | 40 +++--- 8 files changed, 174 insertions(+), 283 deletions(-) diff --git a/src/Mapbender/DataManagerBundle/Component/SchemaFilter.php b/src/Mapbender/DataManagerBundle/Component/SchemaFilter.php index bcd2bcad..dc1bf2eb 100644 --- a/src/Mapbender/DataManagerBundle/Component/SchemaFilter.php +++ b/src/Mapbender/DataManagerBundle/Component/SchemaFilter.php @@ -8,7 +8,6 @@ use Mapbender\DataManagerBundle\Exception\ConfigurationErrorException; use Mapbender\DataManagerBundle\Exception\UnknownSchemaException; use Mapbender\DataSourceBundle\Component\DataStore; -use Mapbender\DataSourceBundle\Component\FeatureType; use Mapbender\DataSourceBundle\Component\RepositoryRegistry; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; @@ -369,6 +368,7 @@ public function getUploadPaths(Element $element, $schemaName, $fieldName) */ public function processSchemaBaseConfig(array $schemaConfig, $schemaName) { + $defaults = static::getConfigDefaults(); // Re-merge "popup" sub-array if (!empty($schemaConfig['popup']) && !empty($defaults['popup'])) { $schemaConfig['popup'] = array_replace($defaults['popup'], $schemaConfig['popup']); @@ -425,7 +425,9 @@ protected function normalizeColumnsConfigs(array $rawColumns) protected function getExtendedUploadsBasePath($storeConfig) { if (null === $this->isFeatureTypeRegistry) { - $this->isFeatureTypeRegistry = (($this->registry->dataStoreFactory($storeConfig)) instanceof FeatureType); + // Detect FeatureType by config key presence instead of instantiating a repository + $this->isFeatureTypeRegistry = !empty($storeConfig['geomField']) + || !empty($storeConfig['srid']); } return $this->uploadsBasePath . '/' . ($this->isFeatureTypeRegistry ? 'featureTypes' : 'ds-uploads'); } diff --git a/src/Mapbender/DataSourceBundle/Component/DataStore.php b/src/Mapbender/DataSourceBundle/Component/DataStore.php index 5a02c900..b5084011 100644 --- a/src/Mapbender/DataSourceBundle/Component/DataStore.php +++ b/src/Mapbender/DataSourceBundle/Component/DataStore.php @@ -55,24 +55,16 @@ class DataStore /** * @param Connection $connection * @param TokenStorageInterface $tokenStorage - * @param mixed $eventProcessorOrConfig EventProcessor (deprecated, ignored) or config array - * @param array $config Configuration array (when 3rd arg is EventProcessor) + * @param array $config Configuration array */ public function __construct( Connection $connection, TokenStorageInterface $tokenStorage, - $eventProcessorOrConfig = [], $config = [], ) { $this->connection = $connection; $this->tokenStorage = $tokenStorage; - // Backward compatibility: old signature passed EventProcessor as 3rd arg, config as 4th - if (is_array($eventProcessorOrConfig)) { - $config = $eventProcessorOrConfig; - } - // else: $eventProcessorOrConfig is EventProcessor (ignored), $config is 4th arg - $this->tableName = $config['table'] ?? ''; $this->uniqueId = $config['uniqueId'] ?? 'id'; $filter = !empty($config['filter']) ? $config['filter'] : null; @@ -145,11 +137,18 @@ public function getFields(): array } /** - * @return string Database platform name (e.g. "postgresql") + * Check if the underlying database platform is PostgreSQL. + * + * Replaces the deprecated getPlatformName() which relied on + * Doctrine DBAL's deprecated Platform::getName(). + * + * @return bool */ - public function getPlatformName(): string + public function isPostgreSQL(): bool { - return $this->connection->getDatabasePlatform()->getName(); + $platform = $this->connection->getDatabasePlatform(); + return $platform instanceof \Doctrine\DBAL\Platforms\PostgreSQLPlatform + || $platform instanceof \Doctrine\DBAL\Platforms\PostgreSQL120Platform; } /** @@ -255,10 +254,11 @@ public function update($itemOrData): DataItem } /** - * @param mixed $itemOrId DataItem or integer ID - * @return int|null Number of deleted rows + * Delete a data item by ID or DataItem instance. + * + * @return int|string Number of deleted rows */ - public function remove($itemOrId) + public function remove(DataItem|int|string $itemOrId): int|string { $id = ($itemOrId instanceof DataItem) ? $itemOrId->getId() : $itemOrId; return $this->connection->delete( @@ -310,6 +310,12 @@ protected function createSelectQueryBuilder(array $criteria): \Doctrine\DBAL\Que /** * Add WHERE clauses from permanent filter and criteria. * + * SECURITY NOTE: Both $this->filter and $criteria['where'] are raw SQL fragments. + * - $this->filter comes from trusted YAML/DB configuration, sanitized by sanitizeFilter(). + * - $criteria['where'] is constructed by internal code paths only (HttpHandler/UserFilterProvider), + * using quoteIdentifier() and quote() — never from direct user input. + * Do NOT pass unsanitized user input through these channels. + * * @param \Doctrine\DBAL\Query\QueryBuilder $qb * @param array $criteria */ @@ -344,6 +350,9 @@ protected function insertItem(DataItem $item): DataItem $params[] = $value; } + // Extension point for subclasses (e.g. FeatureType adds geometry) + $this->collectInsertData($item, $columns, $placeholders, $params); + if (empty($columns)) { $sql = sprintf( 'INSERT INTO %s DEFAULT VALUES RETURNING %s', @@ -379,15 +388,18 @@ protected function updateItem(DataItem $item): DataItem $allProperties = $this->getItemPropertiesForStorage($item); $storageData = $this->propertyAdapter->prepareStorageData($allProperties); - if (!empty($storageData)) { - $setClauses = []; - $params = []; + $setClauses = []; + $params = []; - foreach ($storageData as $col => $value) { - $setClauses[] = $this->qi($col) . ' = ?'; - $params[] = $value; - } + foreach ($storageData as $col => $value) { + $setClauses[] = $this->qi($col) . ' = ?'; + $params[] = $value; + } + // Extension point for subclasses (e.g. FeatureType adds geometry) + $this->collectUpdateData($item, $setClauses, $params); + + if (!empty($setClauses)) { $params[] = $item->getId(); $sql = sprintf( @@ -403,6 +415,33 @@ protected function updateItem(DataItem $item): DataItem return $this->reloadItem($item) ?? $item; } + /** + * Hook for subclasses to add extra columns to INSERT statements. + * Called after property adapter columns are collected. + * + * @param DataItem $item The item being inserted + * @param string[] &$columns Quoted column names (modified by reference) + * @param string[] &$placeholders SQL placeholders/expressions (modified by reference) + * @param array &$params Bound parameter values (modified by reference) + */ + protected function collectInsertData(DataItem $item, array &$columns, array &$placeholders, array &$params): void + { + // Base implementation: no extra columns + } + + /** + * Hook for subclasses to add extra SET clauses to UPDATE statements. + * Called after property adapter columns are collected. + * + * @param DataItem $item The item being updated + * @param string[] &$setClauses SET clause fragments (modified by reference) + * @param array &$params Bound parameter values (modified by reference) + */ + protected function collectUpdateData(DataItem $item, array &$setClauses, array &$params): void + { + // Base implementation: no extra columns + } + /** * Convert raw database rows to DataItem array. * @@ -454,10 +493,30 @@ protected function qi(string $identifier): string * Quote a table name, handling schema-qualified names. */ protected function quoteTableName(string $tableName): string + { + $parsed = static::parseSchemaQualifiedName($tableName); + $parts = [$parsed['table']]; + if ($parsed['schema'] !== 'public') { + array_unshift($parts, $parsed['schema']); + } + return implode('.', array_map(fn($p) => $this->qi($p), $parts)); + } + + /** + * Parse a potentially schema-qualified table name into schema + table components. + * Handles both quoted ('"schema"."table"') and unquoted ('schema.table') formats. + * + * @param string $tableName + * @return array{schema: string, table: string} + */ + protected static function parseSchemaQualifiedName(string $tableName): array { $unquoted = str_replace('"', '', $tableName); $parts = explode('.', $unquoted); - return implode('.', array_map(fn($p) => $this->qi($p), $parts)); + if (count($parts) === 2) { + return ['schema' => $parts[0], 'table' => $parts[1]]; + } + return ['schema' => 'public', 'table' => $parts[0]]; } /** diff --git a/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php b/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php index eba8afa9..3385f271 100644 --- a/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php +++ b/src/Mapbender/DataSourceBundle/Component/Factory/DataStoreFactory.php @@ -20,12 +20,10 @@ class DataStoreFactory /** * @param ManagerRegistry $connectionRegistry Doctrine registry * @param TokenStorageInterface $tokenStorage Security token storage - * @param mixed $eventProcessor DEPRECATED, accepted but ignored for backward compatibility */ public function __construct( ManagerRegistry $connectionRegistry, TokenStorageInterface $tokenStorage, - $eventProcessor = null, ) { $this->connectionRegistry = $connectionRegistry; $this->tokenStorage = $tokenStorage; @@ -55,7 +53,7 @@ protected function getConfigDefaults(): array * @param string $name * @return Connection */ - public function getDbalConnectionByName($name): Connection + public function getDbalConnectionByName(string $name): Connection { try { /** @var Connection $connection */ diff --git a/src/Mapbender/DataSourceBundle/Component/FeatureType.php b/src/Mapbender/DataSourceBundle/Component/FeatureType.php index be6be313..3aae16a8 100644 --- a/src/Mapbender/DataSourceBundle/Component/FeatureType.php +++ b/src/Mapbender/DataSourceBundle/Component/FeatureType.php @@ -32,22 +32,18 @@ class FeatureType extends DataStore protected string $geomField; protected ?int $configuredSrid; protected ?int $detectedSrid = null; + /** @var array{srid: int|null, type: string|null}|null Cached geometry_columns metadata */ + private ?array $geometryMetadata = null; public function __construct( Connection $connection, TokenStorageInterface $tokenStorage, - $eventProcessorOrConfig = [], $config = [], ) { - // Resolve config (same backward-compat as DataStore) - if (is_array($eventProcessorOrConfig)) { - $config = $eventProcessorOrConfig; - } - $this->geomField = $config['geomField'] ?? 'geom'; $this->configuredSrid = !empty($config['srid']) ? (int) $config['srid'] : null; - parent::__construct($connection, $tokenStorage, $eventProcessorOrConfig, $config); + parent::__construct($connection, $tokenStorage, $config); } /** @@ -187,75 +183,26 @@ protected function addFilters(\Doctrine\DBAL\Query\QueryBuilder $qb, array $crit /** * @inheritDoc - * Adds geometry column with PostGIS INSERT. + * Adds geometry column to INSERT via hook. */ - protected function insertItem(DataItem $item): DataItem + protected function collectInsertData(DataItem $item, array &$columns, array &$placeholders, array &$params): void { /** @var Feature $item */ - $allProperties = $this->getItemPropertiesForStorage($item); - $storageData = $this->propertyAdapter->prepareStorageData($allProperties); - - $columns = []; - $placeholders = []; - $params = []; - - foreach ($storageData as $col => $value) { - $columns[] = $this->qi($col); - $placeholders[] = '?'; - $params[] = $value; - } - - // Add geometry $geomSql = $this->buildGeomInsertExpression($item); if ($geomSql !== null) { $columns[] = $this->qi($this->geomField); $placeholders[] = $geomSql['expression']; $params = array_merge($params, $geomSql['params']); } - - if (empty($columns)) { - $sql = sprintf( - 'INSERT INTO %s DEFAULT VALUES RETURNING %s', - $this->getTableName(), - $this->qi($this->uniqueId), - ); - } else { - $sql = sprintf( - 'INSERT INTO %s (%s) VALUES (%s) RETURNING %s', - $this->getTableName(), - implode(', ', $columns), - implode(', ', $placeholders), - $this->qi($this->uniqueId), - ); - } - - $stmt = $this->connection->prepare($sql); - $result = $stmt->executeQuery($params); - $id = $result->fetchOne(); - $item->setId($id); - - return $this->reloadItem($item) ?? $item; } /** * @inheritDoc - * Adds geometry column with PostGIS UPDATE. + * Adds geometry column to UPDATE via hook. */ - protected function updateItem(DataItem $item): DataItem + protected function collectUpdateData(DataItem $item, array &$setClauses, array &$params): void { /** @var Feature $item */ - $allProperties = $this->getItemPropertiesForStorage($item); - $storageData = $this->propertyAdapter->prepareStorageData($allProperties); - - $setClauses = []; - $params = []; - - foreach ($storageData as $col => $value) { - $setClauses[] = $this->qi($col) . ' = ?'; - $params[] = $value; - } - - // Geometry SET clause $geomSql = $this->buildGeomInsertExpression($item); if ($geomSql !== null) { $setClauses[] = $this->qi($this->geomField) . ' = ' . $geomSql['expression']; @@ -264,21 +211,6 @@ protected function updateItem(DataItem $item): DataItem // Explicitly set geometry to NULL if empty $setClauses[] = $this->qi($this->geomField) . ' = NULL'; } - - if (!empty($setClauses)) { - $params[] = $item->getId(); - - $sql = sprintf( - 'UPDATE %s SET %s WHERE %s = ?', - $this->getTableName(), - implode(', ', $setClauses), - $this->qi($this->uniqueId), - ); - - $this->connection->executeStatement($sql, $params); - } - - return $this->reloadItem($item) ?? $item; } /** @@ -402,14 +334,7 @@ private function shouldPromoteToMulti(string $ewkt, ?string $columnType): bool */ private function detectGeometryColumnType(): ?string { - $parsed = $this->parseTableName(); - try { - $sql = 'SELECT type FROM geometry_columns WHERE f_table_name = ? AND f_table_schema = ?'; - $result = $this->connection->fetchOne($sql, [$parsed['table'], $parsed['schema']]); - return $result ?: null; - } catch (\Exception) { - return null; - } + return $this->getGeometryMetadata()['type']; } /** @@ -419,32 +344,38 @@ private function detectGeometryColumnType(): ?string */ private function detectSridFromGeometryColumns(): ?int { - $parsed = $this->parseTableName(); + return $this->getGeometryMetadata()['srid']; + } + + /** + * Query geometry_columns once and cache both SRID and type. + * Avoids redundant queries when both values are needed during INSERT/UPDATE. + * + * @return array{srid: int|null, type: string|null} + */ + private function getGeometryMetadata(): array + { + if ($this->geometryMetadata !== null) { + return $this->geometryMetadata; + } + + $parsed = static::parseSchemaQualifiedName($this->tableName); try { - $sql = 'SELECT srid FROM geometry_columns WHERE f_table_name = ? AND f_table_schema = ? AND f_geometry_column = ?'; - $result = $this->connection->fetchOne($sql, [ + $sql = 'SELECT srid, type FROM geometry_columns' + . ' WHERE f_table_name = ? AND f_table_schema = ? AND f_geometry_column = ?'; + $row = $this->connection->fetchAssociative($sql, [ $parsed['table'], $parsed['schema'], $this->geomField, ]); - return $result ? (int) $result : null; + $this->geometryMetadata = [ + 'srid' => $row ? ((int) $row['srid'] ?: null) : null, + 'type' => $row ? ($row['type'] ?: null) : null, + ]; } catch (\Exception) { - return null; + $this->geometryMetadata = ['srid' => null, 'type' => null]; } - } - /** - * Parse table name into schema + table components. - * - * @return array{schema: string, table: string} - */ - private function parseTableName(): array - { - $unquoted = str_replace('"', '', $this->tableName); - $parts = explode('.', $unquoted); - if (count($parts) === 2) { - return ['schema' => $parts[0], 'table' => $parts[1]]; - } - return ['schema' => 'public', 'table' => $parts[0]]; + return $this->geometryMetadata; } } diff --git a/src/Mapbender/DataSourceBundle/Component/RepositoryRegistry.php b/src/Mapbender/DataSourceBundle/Component/RepositoryRegistry.php index 21ecafaa..96ff13e4 100644 --- a/src/Mapbender/DataSourceBundle/Component/RepositoryRegistry.php +++ b/src/Mapbender/DataSourceBundle/Component/RepositoryRegistry.php @@ -14,39 +14,30 @@ */ class RepositoryRegistry { - /** @var DataStoreFactory */ - protected $factory; + protected DataStoreFactory $factory; /** @var mixed[][] */ - protected $repositoryConfigs; + protected array $repositoryConfigs; /** @var DataStore[] */ - protected $repositories = array(); + protected array $repositories = []; /** - * @param DataStoreFactory $factory * @param mixed[][] $repositoryConfigs */ - public function __construct(DataStoreFactory $factory, - array $repositoryConfigs) + public function __construct(DataStoreFactory $factory, array $repositoryConfigs) { $this->factory = $factory; $this->repositoryConfigs = $repositoryConfigs; } - /** - * @param array $config - * @return DataStore - */ - public function dataStoreFactory(array $config) + public function dataStoreFactory(array $config): DataStore { return $this->factory->fromConfig($config); } /** - * @param string $name - * @return DataStore * @since 0.1.15 */ - public function getDataStoreByName($name) + public function getDataStoreByName(string $name): DataStore { if (!$name) { throw new \InvalidArgumentException("Empty dataStore / featureType name " . var_export($name, true)); @@ -61,57 +52,15 @@ public function getDataStoreByName($name) * @return mixed[][] * @since 0.1.8 */ - public function getDataStoreDeclarations() + public function getDataStoreDeclarations(): array { return $this->repositoryConfigs; } /** - * Alias for dataStoreFactory - * - * @param mixed[] $config - * @return DataStore - * @since 0.1.15 - * @deprecated use aliased method directly - * aliased @since 0.1.22 - */ - public function featureTypeFactory(array $config) - { - return $this->dataStoreFactory($config); - } - - /** - * Alias for getDataStoreByName - * - * @param string $name - * @return DataStore - * @since 0.1.15 - * @deprecated use aliased method directly - * aliased @since 0.1.22 - */ - public function getFeatureTypeByName($name) - { - return $this->getDataStoreByName($name); - } - - /** - * Alias for getDataStoreDeclarations - * - * @return array - * @deprecated use aliased method directly - * aliased @since 0.1.22 - */ - public function getFeatureTypeDeclarations() - { - return $this->getDataStoreDeclarations(); - } - - /** - * @param string $name - * @return Connection * @since 0.0.16 */ - public function getDbalConnectionByName($name) + public function getDbalConnectionByName(string $name): Connection { return $this->factory->getDbalConnectionByName($name); } diff --git a/src/Mapbender/DataSourceBundle/Entity/DataItem.php b/src/Mapbender/DataSourceBundle/Entity/DataItem.php index fb259676..7b3b6ba0 100644 --- a/src/Mapbender/DataSourceBundle/Entity/DataItem.php +++ b/src/Mapbender/DataSourceBundle/Entity/DataItem.php @@ -4,17 +4,16 @@ class DataItem implements \ArrayAccess { /** @var mixed[] */ - protected $attributes = array(); + protected array $attributes = []; - /** @var string */ - protected $uniqueIdField; + protected string $uniqueIdField; /** - * @param mixed[] $attributes array + * @param mixed[] $attributes * @param string $uniqueIdField ID field name * @internal */ - public function __construct(array $attributes = array(), $uniqueIdField = 'id') + public function __construct(array $attributes = [], string $uniqueIdField = 'id') { $this->uniqueIdField = $uniqueIdField; if (!array_key_exists($this->uniqueIdField, $attributes)) { @@ -25,80 +24,57 @@ public function __construct(array $attributes = array(), $uniqueIdField = 'id') } /** - * @return array + * @return mixed[] */ - public function toArray() + public function toArray(): array { return $this->attributes; } - /** - * @param mixed $id - */ - public function setId($id) + public function setId(mixed $id): void { $this->attributes[$this->uniqueIdField] = $id; } /** - * Is id not null - * - * @return bool * @deprecated use getId and coerce to boolean */ - public function hasId() + public function hasId(): bool { return !is_null($this->getId()); } - /** - * Get id - * - * @return integer - */ - public function getId() + public function getId(): mixed { return $this->attributes[$this->uniqueIdField]; } /** - * Get attributes - * * @return mixed[] */ - public function getAttributes() + public function getAttributes(): array { return $this->attributes; } - /** - * @param string $name - * @return mixed - */ - public function getAttribute($name) + public function getAttribute(string $name): mixed { return $this->attributes[$name]; } /** - * ADD attributes + * Merge attributes into the existing set. * - * @param mixed $attributes + * @param mixed[] $attributes */ - public function setAttributes($attributes) + public function setAttributes(array $attributes): void { $this->attributes = array_merge($this->attributes, $attributes); } - /** - * Set attribute - * - * @param string $key - * @param mixed $value - */ - public function setAttribute($key, $value) + public function setAttribute(string $key, mixed $value): void { - $this->attributes[ $key ] = $value; + $this->attributes[$key] = $value; } public function offsetExists($offset): bool diff --git a/src/Mapbender/DataSourceBundle/Entity/Feature.php b/src/Mapbender/DataSourceBundle/Entity/Feature.php index dae3dd93..fb255d7d 100644 --- a/src/Mapbender/DataSourceBundle/Entity/Feature.php +++ b/src/Mapbender/DataSourceBundle/Entity/Feature.php @@ -5,14 +5,13 @@ class Feature extends DataItem { - /** @var string|null */ - protected $geomField; + protected string $geomField; /** * @param string|null $geom * @return $this */ - public function setGeom($geom) + public function setGeom(?string $geom): static { if ($geom && !($newSrid = WktUtility::getEwktSrid($geom))) { if ($oldSrid = $this->getSrid()) { @@ -26,35 +25,26 @@ public function setGeom($geom) /** * Get geometry as WKT. - * @return string|null */ - public function getGeom() + public function getGeom(): ?string { return WktUtility::wktFromEwkt($this->attributes[$this->geomField]); } /** * Get geometry as EWKT string. - * - * @return string|null */ - public function getEwkt() + public function getEwkt(): ?string { return $this->attributes[$this->geomField] ?: null; } - /** - * @return integer|null - */ - public function getSrid() + public function getSrid(): ?int { return WktUtility::getEwktSrid($this->getEwkt()); } - /** - * @param integer $srid - */ - public function setSrid($srid) + public function setSrid(int $srid): void { if ($wkt = WktUtility::wktFromEwkt($this->attributes[$this->geomField])) { $this->attributes[$this->geomField] = "SRID={$srid};{$wkt}"; @@ -67,13 +57,8 @@ public function setSrid($srid) * @param string $geomField * @internal */ - public function __construct(array $attributes = array(), $uniqueIdField = 'id', $geomField = "geom") + public function __construct(array $attributes = array(), string $uniqueIdField = 'id', string $geomField = 'geom') { - if (\is_numeric($uniqueIdField)) { - @trigger_error("DEPRECATED: do not pass srid to Feature constructor.", E_USER_DEPRECATED); - $uniqueIdField = $geomField; - $geomField = (\func_num_args() >= 4) ? \func_get_arg(3) : 'geom'; - } $this->geomField = $geomField; // Ensure getGeom / getEwkt / getSrid works $attributes += array( @@ -82,12 +67,7 @@ public function __construct(array $attributes = array(), $uniqueIdField = 'id', parent::__construct($attributes, $uniqueIdField); } - /** - * ADD attributes - * - * @param mixed $attributes - */ - public function setAttributes($attributes) + public function setAttributes(array $attributes): void { if (array_key_exists($this->geomField, $attributes)) { $this->setGeom($attributes[$this->geomField]); @@ -96,7 +76,7 @@ public function setAttributes($attributes) parent::setAttributes($attributes); } - public function setAttribute($key, $value) + public function setAttribute(string $key, mixed $value): void { if ($key === $this->geomField) { $this->setGeom($value); @@ -106,11 +86,9 @@ public function setAttribute($key, $value) } /** - * Get geometry type - * - * @return string|null + * Get geometry type (e.g. "POLYGON", "POINT"). */ - public function getType() + public function getType(): ?string { return WktUtility::getGeometryType($this->attributes[$this->geomField]); } diff --git a/src/Mapbender/DataSourceBundle/Utils/WktUtility.php b/src/Mapbender/DataSourceBundle/Utils/WktUtility.php index c150f48e..36fa3069 100644 --- a/src/Mapbender/DataSourceBundle/Utils/WktUtility.php +++ b/src/Mapbender/DataSourceBundle/Utils/WktUtility.php @@ -8,50 +8,48 @@ class WktUtility { /** * Extracts the geometry type (e.g. "POLYGON") from given (E)WKT input. - * - * @param string $wkt - * @return string|null for invalid input */ - public static function getGeometryType($wkt) + public static function getGeometryType(?string $wkt): ?string { + $wkt = static::wktFromEwkt($wkt); + if ($wkt === null) { + return null; + } $matches = array(); - if (preg_match('#^\w+#', static::wktFromEwkt($wkt), $matches)) { + if (preg_match('#^\w+#', $wkt, $matches)) { return $matches[0]; - } else { - return null; } + return null; } /** - * Extracts the WKT portion (e.g. "POLYGON(...)") from given $ewkt input. - * If input is already a WKT string, return it unchanged. - * - * @param string $ewkt - * @return string|null for invalid input + * Extracts the WKT portion (e.g. "POLYGON(...)") from given EWKT input. + * If input is already a WKT string, returns it unchanged. */ - public static function wktFromEwkt($ewkt) + public static function wktFromEwkt(?string $ewkt): ?string { + if ($ewkt === null || $ewkt === '') { + return null; + } $wkt = preg_replace('#^SRID=[^\;]*;#', '', $ewkt); if (!preg_match('#^\w+#', $wkt)) { return null; - } else { - return $wkt; } + return $wkt; } /** * Extracts the SRID number from EWKT input. - * - * @param $ewkt - * @return int|null for invalid input */ - public static function getEwktSrid($ewkt) + public static function getEwktSrid(?string $ewkt): ?int { + if ($ewkt === null || $ewkt === '') { + return null; + } $matches = array(); if (preg_match('#^SRID=(\d+);#', $ewkt, $matches)) { return intval($matches[1]) ?: null; - } else { - return null; } + return null; } } From dbe29a4c7f5bf1ad04dbb25540746f6ea481059b Mon Sep 17 00:00:00 2001 From: frauch Date: Fri, 27 Feb 2026 13:29:49 +0100 Subject: [PATCH 3/4] refactor: Update constructor parameter types and improve error handling in DataStore and FeatureType --- .../Resources/public/MbDataManager.js | 6 ++--- .../DataSourceBundle/Component/DataStore.php | 23 +++++++------------ .../Component/FeatureType.php | 8 +++++-- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js b/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js index a250f7ee..dd63b0e6 100644 --- a/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js +++ b/src/Mapbender/DataManagerBundle/Resources/public/MbDataManager.js @@ -97,9 +97,6 @@ .then(() => this.updateSchemaSelector_()) ; this.onGrantsLoadStarted(); -/** - * @private - */ } onGrantsLoadStarted() { @@ -440,8 +437,8 @@ * @param {(String|null)} originalId * @private */ -/** @var {DataManagagerSaveEventData} eventData */ _saveEvent(schema, dataItem, originalId) { + /** @var {DataManagerSaveEventData} eventData */ const eventData = { item: dataItem, itemId: this._getUniqueItemId(dataItem), @@ -888,6 +885,7 @@ $loadingIndicator.css({ opacity: 0 }); }); } + jqXhr.fail(this._onAjaxError.bind(this)); return jqXhr; } diff --git a/src/Mapbender/DataSourceBundle/Component/DataStore.php b/src/Mapbender/DataSourceBundle/Component/DataStore.php index b5084011..28dadf7a 100644 --- a/src/Mapbender/DataSourceBundle/Component/DataStore.php +++ b/src/Mapbender/DataSourceBundle/Component/DataStore.php @@ -60,7 +60,7 @@ class DataStore public function __construct( Connection $connection, TokenStorageInterface $tokenStorage, - $config = [], + array $config = [], ) { $this->connection = $connection; $this->tokenStorage = $tokenStorage; @@ -354,24 +354,20 @@ protected function insertItem(DataItem $item): DataItem $this->collectInsertData($item, $columns, $placeholders, $params); if (empty($columns)) { - $sql = sprintf( - 'INSERT INTO %s DEFAULT VALUES RETURNING %s', - $this->getTableName(), - $this->qi($this->uniqueId), + $this->connection->executeStatement( + sprintf('INSERT INTO %s DEFAULT VALUES', $this->getTableName()), ); } else { $sql = sprintf( - 'INSERT INTO %s (%s) VALUES (%s) RETURNING %s', + 'INSERT INTO %s (%s) VALUES (%s)', $this->getTableName(), implode(', ', $columns), implode(', ', $placeholders), - $this->qi($this->uniqueId), ); + $this->connection->executeStatement($sql, $params); } - $stmt = $this->connection->prepare($sql); - $result = $stmt->executeQuery($params); - $id = $result->fetchOne(); + $id = $this->connection->lastInsertId(); $item->setId($id); return $this->reloadItem($item) ?? $item; @@ -495,10 +491,7 @@ protected function qi(string $identifier): string protected function quoteTableName(string $tableName): string { $parsed = static::parseSchemaQualifiedName($tableName); - $parts = [$parsed['table']]; - if ($parsed['schema'] !== 'public') { - array_unshift($parts, $parsed['schema']); - } + $parts = [$parsed['schema'], $parsed['table']]; return implode('.', array_map(fn($p) => $this->qi($p), $parts)); } @@ -535,7 +528,7 @@ protected function bindUserName(\Doctrine\DBAL\Query\QueryBuilder $qb): void protected function getUserName(): string { $token = $this->tokenStorage->getToken(); - if ($token && method_exists($token, 'getUserIdentifier')) { + if ($token) { return $token->getUserIdentifier() ?? ''; } return ''; diff --git a/src/Mapbender/DataSourceBundle/Component/FeatureType.php b/src/Mapbender/DataSourceBundle/Component/FeatureType.php index 3aae16a8..66b86deb 100644 --- a/src/Mapbender/DataSourceBundle/Component/FeatureType.php +++ b/src/Mapbender/DataSourceBundle/Component/FeatureType.php @@ -38,7 +38,7 @@ class FeatureType extends DataStore public function __construct( Connection $connection, TokenStorageInterface $tokenStorage, - $config = [], + array $config = [], ) { $this->geomField = $config['geomField'] ?? 'geom'; $this->configuredSrid = !empty($config['srid']) ? (int) $config['srid'] : null; @@ -372,7 +372,11 @@ private function getGeometryMetadata(): array 'srid' => $row ? ((int) $row['srid'] ?: null) : null, 'type' => $row ? ($row['type'] ?: null) : null, ]; - } catch (\Exception) { + } catch (\Exception $e) { + @trigger_error( + "geometry_columns query failed for {$this->tableName}.{$this->geomField}: {$e->getMessage()}", + E_USER_WARNING, + ); $this->geometryMetadata = ['srid' => null, 'type' => null]; } From aeefd54a48685ffb456a3040183a963011bfabda Mon Sep 17 00:00:00 2001 From: frauch Date: Fri, 27 Feb 2026 13:40:14 +0100 Subject: [PATCH 4/4] refactor: Update CHANGELOG with DataSourceBundle refactor details and breaking changes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4676ee5..fb37cdcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ## next feature release +* Refactor DataSourceBundle: remove eval-based EventProcessor (security), simplify driver abstraction, add PropertyAdapter strategy for flexible property storage ([PR#155](https://github.com/mapbender/mapbender-digitizer/pull/155)) **BREAKING**: Oracle/SQLite spatial support dropped, deprecated services removed — see `EVENT-MIGRATION.md` * Migrate from jQuery UI dialogs to Mapbender.Popup with improved mobile support ([PR#149](https://github.com/mapbender/mapbender-digitizer/pull/149)) * Add dynamic expression support for labels, text fields, and popup titles (template literal syntax `${data.property}`) ([PR#149](https://github.com/mapbender/mapbender-digitizer/pull/149)) * Modernize JavaScript codebase (ES6 classes, singleton pattern for better extensibility) ([PR#149](https://github.com/mapbender/mapbender-digitizer/pull/149))