Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure correct json default value normalization #6358

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

sbuerk
Copy link
Contributor

@sbuerk sbuerk commented Apr 16, 2024

Q A
Type bug
Fixed issues #6357

Summary

PostgreSQLSchemaManager::_getPortableTableColumnDefinition() is modified to call the private method parseDefaultExpression() on default values for JSON field to correctly normalize doubled single-quotes in json string value.

A test to cover this case is added.

`PostgreSQLSchemaManager::_getPortableTableColumnDefinition()` is
modified to call the private method `parseDefaultExpression()` on
default values for `JSON` field to correctly normalize doubled
single-quotes in json string value.

A test to cover this case is added.

Fixes doctrine#6357
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Apr 16, 2024
Default value support for TEXT, JSON and BLOB fields
has been added with #103578 by implementing the use
of default value expression for MySQL. That required
to add custom normalization on data schema reads to
be comparable.

MySQL requires to use a single-quote to quote a single
quote in a value string, and due to the expression way
this needs to be properly decoded now in two steps:

* Revert escape sequences in the retrieved default value
* Unquote the unescaped retrieved default value

JSON field defaults shows a similar issue for double
quotes in the json value and can be fixed in the same
way.

Added test revealed, that Doctrine DBAL has an issue
with double single-quotes for PostgreSQL too. To fix
this the issue has been reported [1] and a pull-request
provided [2].

This change ensure correct unescaping and unquoting of
the retrieved column default value for TEXT, JSON and
BLOB column types for MySQL connections, enriched with
further tests.

The extended PostreSQLSchemaManager now clones method
`_getPortableTableColumnDefinition()` to incorporate
the bugfix directly until a fixed Doctrine DBAL version
has been released.

[1] doctrine/dbal#6357
[2] doctrine/dbal#6358

Resolves: #103610
Related: #103578
Releases: main
Change-Id: Icb39cdb8c87ae7907f84e5c38adcde4ef545ed1b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83745
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Garvin Hicking <gh@faktor-e.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Garvin Hicking <gh@faktor-e.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Apr 16, 2024
Default value support for TEXT, JSON and BLOB fields
has been added with #103578 by implementing the use
of default value expression for MySQL. That required
to add custom normalization on data schema reads to
be comparable.

MySQL requires to use a single-quote to quote a single
quote in a value string, and due to the expression way
this needs to be properly decoded now in two steps:

* Revert escape sequences in the retrieved default value
* Unquote the unescaped retrieved default value

JSON field defaults shows a similar issue for double
quotes in the json value and can be fixed in the same
way.

Added test revealed, that Doctrine DBAL has an issue
with double single-quotes for PostgreSQL too. To fix
this the issue has been reported [1] and a pull-request
provided [2].

This change ensure correct unescaping and unquoting of
the retrieved column default value for TEXT, JSON and
BLOB column types for MySQL connections, enriched with
further tests.

The extended PostreSQLSchemaManager now clones method
`_getPortableTableColumnDefinition()` to incorporate
the bugfix directly until a fixed Doctrine DBAL version
has been released.

[1] doctrine/dbal#6357
[2] doctrine/dbal#6358

Resolves: #103610
Related: #103578
Releases: main
Change-Id: Icb39cdb8c87ae7907f84e5c38adcde4ef545ed1b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83745
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Garvin Hicking <gh@faktor-e.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Garvin Hicking <gh@faktor-e.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
@derrabus derrabus added this to the 3.8.4 milestone Apr 16, 2024
@derrabus derrabus merged commit e2a6621 into doctrine:3.8.x Apr 16, 2024
92 checks passed
derrabus added a commit that referenced this pull request Apr 16, 2024
Follow-up to #6358 (@sbuerk): `getName()` is gone on 4.0.x. This change
will make merging this test up easier for us.
derrabus added a commit to derrabus/dbal that referenced this pull request Apr 16, 2024
* 3.8.x:
  Avoid calling deprecated Type::getName() (doctrine#6359)
  Ensure correct json default value normalization (doctrine#6358)
  Document how to run integration tests locally
  Remove unused script
derrabus added a commit that referenced this pull request Apr 16, 2024
* 4.0.x:
  Avoid calling deprecated Type::getName() (#6359)
  Ensure correct json default value normalization (#6358)
  Document how to run integration tests locally
  Remove unused script
derrabus added a commit that referenced this pull request Apr 16, 2024
* 4.1.x:
  Avoid calling deprecated Type::getName() (#6359)
  Ensure correct json default value normalization (#6358)
  Document how to run integration tests locally
  Remove unused script
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Apr 26, 2024
Doctrine DBAL released a new bugfix version [1]
of the `doctrine/dbal` package, including a
required bugfix for TYPO3 v13 [2].

This change upgrades to `4.0.2` to ensure that
the bugfix [2] is included and all changes from
`3.8.1` up to `3.8.4`[3][4][5].

Used command(s):

> \
  composer req --no-update --no-install \
    -d typo3/sysext/redirects \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/core \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/install \
    "doctrine/dbal":"^4.0.2" ; \
  composer req \
    "doctrine/dbal":"^4.0.2"

Note: The intermediate fix in our extended Platform
classes will be removed with a follow up commit.

[1] https://github.com/doctrine/dbal/releases/tag/4.0.2
[2] doctrine/dbal#6358
[3] https://github.com/doctrine/dbal/releases/tag/3.8.4
[4] https://github.com/doctrine/dbal/releases/tag/3.8.3
[5] https://github.com/doctrine/dbal/releases/tag/3.8.2

Resolves: #103729
Releases: main
Change-Id: Ie64a373f623ae102533661a44247aaac65e5a4d9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84024
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Benjamin Franzke <ben@bnf.dev>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Apr 26, 2024
Doctrine DBAL released a new bugfix version [1]
of the `doctrine/dbal` package, including a
required bugfix for TYPO3 v13 [2].

This change upgrades to `4.0.2` to ensure that
the bugfix [2] is included and all changes from
`3.8.1` up to `3.8.4`[3][4][5].

Used command(s):

> \
  composer req --no-update --no-install \
    -d typo3/sysext/redirects \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/core \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/install \
    "doctrine/dbal":"^4.0.2" ; \
  composer req \
    "doctrine/dbal":"^4.0.2"

Note: The intermediate fix in our extended Platform
classes will be removed with a follow up commit.

[1] https://github.com/doctrine/dbal/releases/tag/4.0.2
[2] doctrine/dbal#6358
[3] https://github.com/doctrine/dbal/releases/tag/3.8.4
[4] https://github.com/doctrine/dbal/releases/tag/3.8.3
[5] https://github.com/doctrine/dbal/releases/tag/3.8.2

Resolves: #103729
Releases: main
Change-Id: Ie64a373f623ae102533661a44247aaac65e5a4d9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84024
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Benjamin Franzke <ben@bnf.dev>
TYPO3IncTeam pushed a commit to TYPO3-CMS/install that referenced this pull request Apr 26, 2024
Doctrine DBAL released a new bugfix version [1]
of the `doctrine/dbal` package, including a
required bugfix for TYPO3 v13 [2].

This change upgrades to `4.0.2` to ensure that
the bugfix [2] is included and all changes from
`3.8.1` up to `3.8.4`[3][4][5].

Used command(s):

> \
  composer req --no-update --no-install \
    -d typo3/sysext/redirects \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/core \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/install \
    "doctrine/dbal":"^4.0.2" ; \
  composer req \
    "doctrine/dbal":"^4.0.2"

Note: The intermediate fix in our extended Platform
classes will be removed with a follow up commit.

[1] https://github.com/doctrine/dbal/releases/tag/4.0.2
[2] doctrine/dbal#6358
[3] https://github.com/doctrine/dbal/releases/tag/3.8.4
[4] https://github.com/doctrine/dbal/releases/tag/3.8.3
[5] https://github.com/doctrine/dbal/releases/tag/3.8.2

Resolves: #103729
Releases: main
Change-Id: Ie64a373f623ae102533661a44247aaac65e5a4d9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84024
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Benjamin Franzke <ben@bnf.dev>
TYPO3IncTeam pushed a commit to TYPO3-CMS/redirects that referenced this pull request Apr 26, 2024
Doctrine DBAL released a new bugfix version [1]
of the `doctrine/dbal` package, including a
required bugfix for TYPO3 v13 [2].

This change upgrades to `4.0.2` to ensure that
the bugfix [2] is included and all changes from
`3.8.1` up to `3.8.4`[3][4][5].

Used command(s):

> \
  composer req --no-update --no-install \
    -d typo3/sysext/redirects \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/core \
    "doctrine/dbal":"^4.0.2" ; \
  composer req --no-update --no-install \
    -d typo3/sysext/install \
    "doctrine/dbal":"^4.0.2" ; \
  composer req \
    "doctrine/dbal":"^4.0.2"

Note: The intermediate fix in our extended Platform
classes will be removed with a follow up commit.

[1] https://github.com/doctrine/dbal/releases/tag/4.0.2
[2] doctrine/dbal#6358
[3] https://github.com/doctrine/dbal/releases/tag/3.8.4
[4] https://github.com/doctrine/dbal/releases/tag/3.8.3
[5] https://github.com/doctrine/dbal/releases/tag/3.8.2

Resolves: #103729
Releases: main
Change-Id: Ie64a373f623ae102533661a44247aaac65e5a4d9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84024
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Benjamin Franzke <ben@bnf.dev>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Apr 26, 2024
Database field default values for TEXT, BLOB and JSON fields
has been added with #103578 and default value normalization
fixed with #103610 also including a direct bugfix for the
reported and contributed `PostgreSQLSchemaManager` in Doctrine
DBAL [1].

The Doctrine DBAL Team released `4.0.2` containing this
bugfix, which has been ensured with #103729 and is now
removed again to clean up the Core code base.

[1] doctrine/dbal#6358

Resolves: #103730
Related: #103729
Related: #103610
Related: #103578
Releases: main
Change-Id: I77d8f595a18d648ddf14ddba98156f4cc337dd9f
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84025
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Apr 26, 2024
Database field default values for TEXT, BLOB and JSON fields
has been added with #103578 and default value normalization
fixed with #103610 also including a direct bugfix for the
reported and contributed `PostgreSQLSchemaManager` in Doctrine
DBAL [1].

The Doctrine DBAL Team released `4.0.2` containing this
bugfix, which has been ensured with #103729 and is now
removed again to clean up the Core code base.

[1] doctrine/dbal#6358

Resolves: #103730
Related: #103729
Related: #103610
Related: #103578
Releases: main
Change-Id: I77d8f595a18d648ddf14ddba98156f4cc337dd9f
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84025
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants