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

[doctrine/doctrine-bundle] Disable controller resolver auto mapping by default #1299

Merged

Conversation

bobvandevijver
Copy link
Contributor

Q A
License MIT
Doc issue/PR doctrine/DoctrineBundle#1762 & symfony/symfony-docs#19671

Reflects the new default for doctrine.orm.controller_resolver.auto_mapping to ensure new users will not be faced with a deprecation when starting with doctrine bundle 2.12 or higher.

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) March 14, 2024 14:39
Copy link

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1299/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1299/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'doctrine/doctrine-bundle:^2.12'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

doctrine/doctrine-bundle

1.6 vs 1.12
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
index 2f611de..30d710d 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
@@ -10,9 +10,12 @@ doctrine:
         charset: utf8mb4
         default_table_options:
             collate: utf8mb4_unicode_ci
+
+        # backtrace queries in profiler (increases memory usage per request)
+        #profiling_collect_backtrace: '%kernel.debug%'
     orm:
         auto_generate_proxy_classes: true
-        naming_strategy: doctrine.orm.naming_strategy.underscore
+        naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
             App:
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
index 0a7c53b..084f59a 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
@@ -2,26 +2,14 @@ doctrine:
     orm:
         auto_generate_proxy_classes: false
         metadata_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         query_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         result_cache_driver:
-            type: service
-            id: doctrine.result_cache_provider
-
-services:
-    doctrine.result_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.result_cache_pool'
-    doctrine.system_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.system_cache_pool'
+            type: pool
+            pool: doctrine.result_cache_pool
 
 framework:
     cache:
diff --git a/doctrine/doctrine-bundle/1.6/manifest.json b/doctrine/doctrine-bundle/1.12/manifest.json
index 04da3d1..2d3a40a 100644
--- a/doctrine/doctrine-bundle/1.6/manifest.json
+++ b/doctrine/doctrine-bundle/1.12/manifest.json
@@ -11,8 +11,8 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
-        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/db_name?serverVersion=8\"",
+        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/db_name?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
         "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev && \\",
1.12 vs 2.0
diff --git a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
index 30d710d..365fef1 100644
--- a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
@@ -5,14 +5,7 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '16'
-
-        # only needed for MySQL
-        charset: utf8mb4
-        default_table_options:
-            collate: utf8mb4_unicode_ci
-
-        # backtrace queries in profiler (increases memory usage per request)
-        #profiling_collect_backtrace: '%kernel.debug%'
+        use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
diff --git a/doctrine/doctrine-bundle/1.12/manifest.json b/doctrine/doctrine-bundle/2.0/manifest.json
index 2d3a40a..04da3d1 100644
--- a/doctrine/doctrine-bundle/1.12/manifest.json
+++ b/doctrine/doctrine-bundle/2.0/manifest.json
@@ -11,8 +11,8 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/db_name?serverVersion=8\"",
-        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/db_name?serverVersion=16&charset=utf8"
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
+        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
         "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev && \\",
2.0 vs 2.3
diff --git a/doctrine/doctrine-bundle/2.0/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
index 084f59a..17299e2 100644
--- a/doctrine/doctrine-bundle/2.0/config/packages/prod/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
@@ -1,9 +1,6 @@
 doctrine:
     orm:
         auto_generate_proxy_classes: false
-        metadata_cache_driver:
-            type: pool
-            pool: doctrine.system_cache_pool
         query_cache_driver:
             type: pool
             pool: doctrine.system_cache_pool
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
new file mode 100644
index 0000000..2ace640
--- /dev/null
+++ b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
@@ -0,0 +1,4 @@
+doctrine:
+    dbal:
+        # "TEST_TOKEN" is typically set by ParaTest
+        dbname: 'main_test%env(default::TEST_TOKEN)%'
diff --git a/doctrine/doctrine-bundle/2.0/manifest.json b/doctrine/doctrine-bundle/2.3/manifest.json
index 04da3d1..26e52b9 100644
--- a/doctrine/doctrine-bundle/2.0/manifest.json
+++ b/doctrine/doctrine-bundle/2.3/manifest.json
@@ -11,13 +11,13 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8&charset=utf8mb4\"",
         "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
-        "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev && \\",
-        "\tdocker-php-ext-install -j$(nproc) pdo_pgsql && \\",
-        "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5 && \\",
+        "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev; \\",
+        "\tdocker-php-ext-install -j$(nproc) pdo_pgsql; \\",
+        "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5; \\",
         "\tapk del .pgsql-deps"
     ],
     "docker-compose": {
2.3 vs 2.4
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
index 365fef1..e517e07 100644
--- a/doctrine/doctrine-bundle/2.3/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
@@ -13,7 +13,32 @@ doctrine:
         mappings:
             App:
                 is_bundle: false
-                type: annotation
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
                 alias: App
+
+when@test:
+    doctrine:
+        dbal:
+            # "TEST_TOKEN" is typically set by ParaTest
+            dbname_suffix: '_test%env(default::TEST_TOKEN)%'
+
+when@prod:
+    doctrine:
+        orm:
+            auto_generate_proxy_classes: false
+            proxy_dir: '%kernel.build_dir%/doctrine/orm/Proxies'
+            query_cache_driver:
+                type: pool
+                pool: doctrine.system_cache_pool
+            result_cache_driver:
+                type: pool
+                pool: doctrine.result_cache_pool
+
+    framework:
+        cache:
+            pools:
+                doctrine.result_cache_pool:
+                    adapter: cache.app
+                doctrine.system_cache_pool:
+                    adapter: cache.system
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
deleted file mode 100644
index 17299e2..0000000
--- a/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
+++ /dev/null
@@ -1,17 +0,0 @@
-doctrine:
-    orm:
-        auto_generate_proxy_classes: false
-        query_cache_driver:
-            type: pool
-            pool: doctrine.system_cache_pool
-        result_cache_driver:
-            type: pool
-            pool: doctrine.result_cache_pool
-
-framework:
-    cache:
-        pools:
-            doctrine.result_cache_pool:
-                adapter: cache.app
-            doctrine.system_cache_pool:
-                adapter: cache.system
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
deleted file mode 100644
index 2ace640..0000000
--- a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-doctrine:
-    dbal:
-        # "TEST_TOKEN" is typically set by ParaTest
-        dbname: 'main_test%env(default::TEST_TOKEN)%'
diff --git a/doctrine/doctrine-bundle/2.3/manifest.json b/doctrine/doctrine-bundle/2.4/manifest.json
index 26e52b9..88bac7a 100644
--- a/doctrine/doctrine-bundle/2.3/manifest.json
+++ b/doctrine/doctrine-bundle/2.4/manifest.json
@@ -44,5 +44,8 @@
                 "    - \"5432\""
             ]
         }
+    },
+    "conflict": {
+        "symfony/framework-bundle": "<5.3"
     }
 }
2.4 vs 2.8
diff --git a/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.8/config/packages/doctrine.yaml
index e517e07..c00894c 100644
--- a/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.8/config/packages/doctrine.yaml
@@ -8,6 +8,7 @@ doctrine:
         use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
+        enable_lazy_ghost_objects: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
diff --git a/doctrine/doctrine-bundle/2.4/manifest.json b/doctrine/doctrine-bundle/2.8/manifest.json
index 88bac7a..addfd20 100644
--- a/doctrine/doctrine-bundle/2.4/manifest.json
+++ b/doctrine/doctrine-bundle/2.8/manifest.json
@@ -11,12 +11,13 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8&charset=utf8mb4\"",
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8.0.32&charset=utf8mb4\"",
+        "#6": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=10.11.2-MariaDB&charset=utf8mb4\"",
         "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
         "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev; \\",
-        "\tdocker-php-ext-install -j$(nproc) pdo_pgsql; \\",
+        "\tdocker-php-ext-install -j\"$(nproc)\" pdo_pgsql; \\",
         "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5; \\",
         "\tapk del .pgsql-deps"
     ],
@@ -46,6 +47,8 @@
         }
     },
     "conflict": {
+        "doctrine/orm": "<2.14",
+        "symfony/dependency-injection": "<6.2",
         "symfony/framework-bundle": "<5.3"
     }
 }
2.8 vs 2.9
diff --git a/doctrine/doctrine-bundle/2.8/manifest.json b/doctrine/doctrine-bundle/2.9/manifest.json
index addfd20..1d8996a 100644
--- a/doctrine/doctrine-bundle/2.8/manifest.json
+++ b/doctrine/doctrine-bundle/2.9/manifest.json
@@ -16,10 +16,7 @@
         "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
-        "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev; \\",
-        "\tdocker-php-ext-install -j\"$(nproc)\" pdo_pgsql; \\",
-        "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5; \\",
-        "\tapk del .pgsql-deps"
+        "RUN install-php-extensions pdo_pgsql"
     ],
     "docker-compose": {
         "docker-compose.yml": {
2.9 vs 2.10
diff --git a/doctrine/doctrine-bundle/2.9/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
index c00894c..d42c52d 100644
--- a/doctrine/doctrine-bundle/2.9/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
@@ -5,14 +5,19 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '16'
+
+        profiling_collect_backtrace: '%kernel.debug%'
         use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
         enable_lazy_ghost_objects: true
+        report_fields_where_declared: true
+        validate_xml_mapping: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
             App:
+                type: attribute
                 is_bundle: false
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
2.10 vs 2.12
diff --git a/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.12/config/packages/doctrine.yaml
index d42c52d..718692d 100644
--- a/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.12/config/packages/doctrine.yaml
@@ -22,6 +22,8 @@ doctrine:
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
                 alias: App
+        controller_resolver:
+            auto_mapping: false
 
 when@test:
     doctrine:

@bobvandevijver
Copy link
Contributor Author

I do not think the action failure is something I can solve, or is it?

@seb-jean
Copy link
Contributor

Hi,
Since the default value of setting doctrine.orm.controller_resolver.auto_mapping is false, I get the following error:

Cannot autowire argument $user of "App\Controller\ProfileController::show()": it needs an instance of "App\Entity\User" but this type has been excluded in "config/services.yaml".

My controller:

// src/Controller/ProfileController.php
namespace App\Controller;

use App\Entity\User;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Attribute\Route;

class ProfileController
{
    #[Route('/profile/{ulid}')]
    public function show(Request $request, User $user): Response
    {
        // ...
    }
}

@kbond
Copy link
Member

kbond commented Mar 24, 2024

Does changing {ulid} to {user} solve (or changing this config value to true)?

@seb-jean
Copy link
Contributor

With {user}, I have error:

An exception occurred while executing a query: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type integer: "1C9UESfu5Jk1kuhoF1vUh8"
CONTEXT: unnamed portal parameter $1 = '...'

With doctrine.orm.controller_resolver.auto_mapping to true, it works but it is not recommended.

@kbond
Copy link
Member

kbond commented Mar 24, 2024

I think in your case, you'll want to explicitly use the MapEntity attribute.

@seb-jean
Copy link
Contributor

It works with #[MapEntity]

// src/Controller/ProfileController.php
namespace App\Controller;

use App\Entity\User;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Attribute\Route;

class ProfileController
{
    #[Route('/profile/{ulid}')]
    public function show(
        Request $request,
        #[MapEntity(mapping: ['ulid' => 'ulid'])] User $user
    ): Response {
        // ...
    }
}

@fabpot
Copy link
Member

fabpot commented Mar 26, 2024

We have the same issue as described above in the Symfony book.

We now need to use #[MapEntity(mapping: ['slug' => 'slug'])] which seems unnecessary.

I think we should revert this change.

@stof
Copy link
Member

stof commented Mar 26, 2024

The auto-mapping using all fields causes lots of weird cases though (especially when you can have 2 entities in your signature. Having this feature off by default avoids lots of weird issues (which might be silent bugs)

@stof
Copy link
Member

stof commented Mar 26, 2024

Note that we only changed the recipes to change the value for new projects.

@nicolas-grekas
Copy link
Member

I'm proposing to revert this change in #1302: this breaks the DX, we need to find a better way.

@nicolas-grekas
Copy link
Member

FYI, I'm proposing symfony/symfony#54455 for a possible solution to these ambiguities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants