Skip to content

Commit

Permalink
feature #286 [Doctrine] Simplify usage and safety (sstok)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.0-dev branch.
labels: bc-break

Discussion
----------

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes
| Deprecations? | no
| Tickets       | Closes #276 
| License       | MIT

- Remove support for SQLite (not recommended for this kind of usage and internally it was only used for mock-testing)
- Remove support for Doctrine ORM NativeQuery, use the DBAL ConditionGenerator instead to get the correct SQL.
- Reintroduce parameter-binding instead of embedding values directly
- Separate Doctrine DBAL/ORM conversions for more flexibility and reduced complexity -- additionally this might allow for #208 to finally happen!
- Remove support for conversion strategies, it's still possible to use a different conversion for values and columns, but the conversion is no longer cached internally. Which also makes it possible now to handle value/context-specific column conversions (making it possible to add support for a new `DateInterval` type; to be introduced later).



Commits
-------

2256ee0 [Doctrine ORM] Remove support for NativeQuery
f86be19 [Doctrine DBAL] Remove support for SQLite
bdb25e9 [Doctrine] Use parameters instead of embedded
  • Loading branch information
sstok authored Sep 13, 2020
2 parents 71fb578 + bdb25e9 commit c8fa110
Show file tree
Hide file tree
Showing 95 changed files with 2,043 additions and 3,531 deletions.
38 changes: 0 additions & 38 deletions .github/phpunit/mysql.xml

This file was deleted.

38 changes: 0 additions & 38 deletions .github/phpunit/pgsql.xml

This file was deleted.

24 changes: 0 additions & 24 deletions .github/phpunit/sqlite.xml

This file was deleted.

6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ env:
ES_HTTP_PORT: '59200'
ELASTICSEARCH_HOST: 'localhost'
ELASTICSEARCH_PORT: '59200'
DB_HOST: 127.0.0.1

jobs:
test:
Expand Down Expand Up @@ -89,9 +90,8 @@ jobs:
SYMFONY_DEPRECATIONS_HELPER: weak
run: |
vendor/bin/phpunit --verbose
vendor/bin/phpunit --verbose --configuration .github/phpunit/sqlite.xml
vendor/bin/phpunit --verbose --configuration .github/phpunit/pgsql.xml
vendor/bin/phpunit --verbose --configuration .github/phpunit/mysql.xml
vendor/bin/phpunit --verbose --configuration phpunit/pgsql.xml
vendor/bin/phpunit --verbose --configuration phpunit/mysql.xml
lint:
name: PHP-QA
Expand Down
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,11 @@ in-docker-install-lowest:

in-docker-test:
SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/phpunit --verbose
SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/phpunit --verbose --configuration phpunit/sqlite.xml
SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/phpunit --verbose --configuration phpunit/pgsql.xml
SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/phpunit --verbose --configuration phpunit/mysql.xml

in-docker-test-coverage:
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --coverage-php build/cov/coverage-phpunit.cov
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --configuration phpunit/sqlite.xml --coverage-php build/cov/coverage-phpunit-sqlite.cov
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --configuration phpunit/pgsql.xml --coverage-php build/cov/coverage-phpunit-pgsql.cov
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --configuration phpunit/mysql.xml --coverage-php build/cov/coverage-phpunit-mysql.cov

Expand Down
55 changes: 55 additions & 0 deletions UPGRADE-2.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,61 @@ UPGRADE FROM 2.0-ALPHA21 to 2.0-ALPHA22

* The `$forceNew` argument in `SearchConditionBuilder::field()` is deprecated and will
be removed in v2.0.0-BETA1, use `overwriteField()` instead.

### Doctrine DBAL

* Support for SQLite was removed in Doctrine DBAL.

* Values are no longer embedded but are now provided as parameters,
make sure to bind these before executing the query.

Before:

```php
$whereClause = $conditionGenerator->getWhereClause();
$statement = $connection->execute('SELECT * FROM tableName '.$whereClause);

$rows = $statement->fetchAll(\PDO::FETCH_ASSOC);
```

Now:

```php
$whereClause = $conditionGenerator->getWhereClause();
$statement = $connection->prepare('SELECT * FROM tableName '.$whereClause);

$conditionGenerator->bindParameters($statement);

$statement->execute();

$rows = $statement->fetchAll(\PDO::FETCH_ASSOC);
```

* The `Rollerworks\Component\Search\Doctrine\Dbal\ValueConversion::convertValue()` method
now expects a `string` type is returned, and requires a return-type.

* Conversion strategies was changed to return a different column/value
statement rather than keeping all strategies cached.

Use the `ConversionHint` new parameters and helper method to determine
the value for the Column.

### Doctrine ORM

* Support for Doctrine ORM NativeQuery was removed, use the Doctrine DBAL
condition-generator instead for this usage.

* Values are no longer embedded but are now provided as parameters,
make sure to bind these before executing the query.

Note: Using the `updateQuery()` method already performs the binding process.

* Doctrine DBAL conversions are no longer applied, instead the Doctrine ORM
integration now has it's own conversion API with a much more powerful integration.

**Note:** Any functions used in the conversion-generated DQL must be registered
with the EntityManager configuration, refer to the Doctrine ORM manual for details.


UPGRADE FROM 2.0-ALPHA19 to 2.0-ALPHA20
=======================================
Expand Down
7 changes: 4 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
}
],
"require": {
"php": "^7.1",
"php": "^7.2",
"psr/container": "^1.0.0",
"symfony/intl": "^4.4 || ^5.0",
"symfony/options-resolver": "^4.4 || ^5.0",
Expand All @@ -35,9 +35,9 @@
},
"require-dev": {
"api-platform/core": "^2.4.5",
"doctrine/dbal": "^2.8.0",
"doctrine/dbal": "^2.10.3",
"doctrine/doctrine-bundle": "^1.9.1 || ^2.0",
"doctrine/orm": "^2.6.2",
"doctrine/orm": "^2.7.3",
"friendsofsymfony/elastica-bundle": "^5.0 || ^5.2@dev",
"matthiasnoback/symfony-dependency-injection-test": "^3.0 || ^4.1.1",
"moneyphp/money": "^3.2.0",
Expand All @@ -62,6 +62,7 @@
},
"config": {
"preferred-install": {
"api-platform/core": "source",
"doctrine/dbal": "source",
"doctrine/orm": "source",
"*": "dist"
Expand Down
Loading

0 comments on commit c8fa110

Please sign in to comment.