8000 [Messenger] schema assets filter causes TypeError · Issue #54769 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] schema assets filter causes TypeError #54769

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.

Sign up for GitHub

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

Jump to bottom
Closed
glaubinix opened this issue Apr 29, 2024 · 7 comments
Closed

[Messenger] schema assets filter causes TypeError #54769

glaubinix opened this issue Apr 29, 2024 · 7 comments
Labels
Bug Status: Needs Review

Comments

@glaubinix
Copy link
Contributor
glaubinix commented Apr 29, 2024 •
edited
Loading

Symfony version(s) affected

6.4.7

Description

This commit introduced a new schema asset filter that filters anything that isn't the doctrine messenger table. However, the filter function can receive other types than string based on this doctrine code the filter can be string|AbstractAsset. This can cause a type error in PostgreSQL and potentially other databases.

TypeError: Symfony\Component\Messenger\Bridge\Doctrine\Transport\Connection::Symfony\Component\Messenger\Bridge\Doctrine\Transport\{closure}(): Argument #1 ($tableName) must be of type string, Doctrine\DBAL\Schema\Sequence given
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/Connection.php:301
/home/runner/work/project/vendor/doctrine/dbal/src/Schema/AbstractSchemaManager.php:377
/home/runner/work/project/vendor/doctrine/dbal/src/Schema/AbstractSchemaManager.php:193
/home/runner/work/project/vendor/doctrine/dbal/src/Schema/AbstractSchemaManager.php:1648
/home/runner/work/project/vendor/doctrine/dbal/src/Schema/AbstractSchemaManager.php:1663
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/Connection.php:538
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/Connection.php:302
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/PostgreSqlConnection.php:97
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/Connection.php:434
/home/runner/work/project/vendor/symfony/doctrine-bridge/SchemaListener/AbstractSchemaListener.php:39
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/Connection.php:352
/home/runner/work/project/vendor/symfony/doctrine-messenger/Transport/DoctrineTransport.php:89
/home/runner/work/project/vendor/symfony/doctrine-bridge/SchemaListener/MessengerTransportDoctrineSchemaListener.php:43
/home/runner/work/project/vendor/symfony/doctrine-bridge/ContainerAwareEventManager.php:63
/home/runner/work/project/vendor/doctrine/orm/src/Tools/SchemaTool.php:421
/home/runner/work/project/vendor/doctrine/orm/src/Tools/SchemaTool.php:123
/home/runner/work/project/vendor/doctrine/orm/src/Tools/SchemaTool.php:101

How to reproduce

Have a Symfony doctrine messenger setup on a PostgreSQL database that uses sequences and call the MessengerTransportDoctrineSchemaListener

Possible Solution

Change the typehint to $configuration->setSchemaAssetsFilter(fn (string|AbstractAsset $tableName) => $tableName === $this->configuration['table_name']);

Additional Context

No response
@glaubinix glaubinix added the Bug label Apr 29, 2024
@glaubinix glaubinix changed the title Messenger: schema assets filter causes TypeError [Messenger] schema assets filter causes TypeError Apr 29, 2024
@hhamon
Copy link
Contributor
hhamon commented Apr 29, 2024

Looks like I'm facing a similar issue after just upgrading Symfony to 6.4.7 on one of my clients projects. Our CI fails and the following error got reported to our Sentry:

Error thrown while running command "app:redacted-command-name --no-interaction". Message: "Symfony\Component\Messenger\Bridge\Doctrine\Transport\Connection::Symfony\Component\Messenger\Bridge\Doctrine\Transport{closure}(): Argument #1 ($tableName) must be of type string, Doctrine\DBAL\Schema\Sequence given"

The new patch release introduced a BC break.

@nicolas-grekas
Copy link
Member

A regression maybe, a BC break nope ;)

@hhamon
Copy link
Contributor
hhamon commented Apr 29, 2024

A regression maybe, a BC break nope ;)

You're right! It does sound to be more like a regression than a BC break here with a the callable accepting string|AbstractAsset depending on the configured DBAL context.

@xabbuh xabbuh mentioned this issue Apr 29, 2024
Merged
@xabbuh
Copy link
Member
xabbuh commented Apr 29, 2024

see #54775

@slappyslap
Copy link

Symfony 7.0 also affected

@xabbuh xabbuh mentioned this issue Apr 30, 2024
Closed
@MatTheCat MatTheCat mentioned this issue Apr 30, 2024
Closed
@fabpot fabpot closed this as completed May 1, 2024
fabpot added a commit that referenced this issue May 1, 2024
@fabpot
bug #54775 [Messenger] accept AbstractAsset instances when filtering …
c1ca4ca 
…schemas (xabbuh)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] accept AbstractAsset instances when filtering schemas

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #54769
| License       | MIT

Commits
-------

4d2679d accept AbstractAsset instances when filtering schemas
@99hops
Copy link
99hops commented May 5, 2024

Saw this after 6.4.7 bin/console messenger:setup-transports

console.CRITICAL: Error thrown while running command "messenger:setup-transports". 
Message: "Symfony\Component\Messenger\Bridge\Doctrine\Transport\Connection::Symfony\Component\Messenger\Bridge\Doctrine\Transport\{closure}(): Argument #1 ($tableName) must be of type string, Doctrine\DBAL\Schema\Sequence given"

Should we wait for the next release?

@derrabus
Copy link
Member
derrabus commented May 5, 2024

Yes, or try out the dev branch.

A767

@chriskapp chriskapp mentioned this issue May 9, 2024
Merged
chriskapp added a commit to apioo/fusio-impl that referenced this issue May 10, 2024
@chriskapp
temporary stick to version 6.4.6 since 6.4.7 contains a bug s. symfon…
26582ad 
…y/symfony#54769
@xabbuh xabbuh mentioned this issue May 22, 2024
Closed
fabpot added a commit that referenced this issue Jun 9, 2024
@fabpot
minor #54872 [Messenger] Added postgres asset filter integration test…
fbf8365 
… (chriskapp)

This PR was submitted for the 7.2 branch but it was squashed and merged into the 6.4 branch instead.

Discussion
----------

[Messenger] Added postgres asset filter integration test

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | #54769
| License       | MIT

This PR just adds an additional postgres integration test to test the already fixed issue #54769 regarding the asset filter to prevent those issues in the future.

Commits
-------

fcad142 [Messenger] Added postgres asset filter integration test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@fabpot @hhamon @nicolas-grekas @glaubinix @derrabus @xabbuh @99hops @carsonbot @slappyslap
0