Skip to content

Rely on defaults for doctrine mapping types #971

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

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Rely on defaults for doctrine mapping types #971

merged 1 commit into from
Dec 22, 2021

Conversation

nicolas-grekas
Copy link
Member

Q A
License MIT
Doc issue/PR -

Needs symfony/symfony#42054 to be released before merging.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

@github-actions
Copy link

github-actions bot commented Jul 11, 2021

Diff between recipe versions

Thanks for the PR 😍

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 bea3648..9faf8e2 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:
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 9faf8e2..c319176 100644
--- a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
@@ -5,14 +5,6 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '13'
-
-        # 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%'
     orm:
         auto_generate_proxy_classes: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
diff --git a/doctrine/doctrine-bundle/1.12/post-install.txt b/doctrine/doctrine-bundle/2.0/post-install.txt
index 99a8825..0e3d418 100644
--- a/doctrine/doctrine-bundle/1.12/post-install.txt
+++ b/doctrine/doctrine-bundle/2.0/post-install.txt
@@ -1,4 +1,4 @@
   * Modify your DATABASE_URL config in .env
 
-  * Configure the driver (mysql) and
-    server_version (5.7) in config/packages/doctrine.yaml
+  * Configure the driver (postgresql) and
+    server_version (13) in config/packages/doctrine.yaml
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 062aed0..995a953 100644
--- a/doctrine/doctrine-bundle/2.0/manifest.json
+++ b/doctrine/doctrine-bundle/2.3/manifest.json
@@ -15,9 +15,9 @@
         "DATABASE_URL": "postgresql://db_user:[email protected]:5432/db_name?serverVersion=13&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 c319176..4d59967 100644
--- a/doctrine/doctrine-bundle/2.3/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
@@ -12,7 +12,6 @@ doctrine:
         mappings:
             App:
                 is_bundle: false
-                type: annotation
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
                 alias: App
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.4/config/packages/test/doctrine.yaml
index 2ace640..34c2ebc 100644
--- a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.4/config/packages/test/doctrine.yaml
@@ -1,4 +1,4 @@
 doctrine:
     dbal:
         # "TEST_TOKEN" is typically set by ParaTest
-        dbname: 'main_test%env(default::TEST_TOKEN)%'
+        dbname_suffix: '_test%env(default::TEST_TOKEN)%'

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold-on, that's going to break things (the book for instance). But also, the maker bundle does not support attributes yet.

@jrushlow
Copy link
Contributor

We're wrapping up attribute support in MakerBundle now in symfony/maker-bundle#920 We should have this merged and released pretty quick.

@fabpot
Copy link
Member

fabpot commented Sep 23, 2021

@jrushlow Correct if I'm wrong, but this change is not a requirement. I understand that it would be better, but as I said, I'm afraid that it will break a lot of things. I know for instance that the Symfony book will break instanenaously.
I would recommend waiting for 5.4 to be released so that we can have a version of the book with support for the new maker bundle.

@weaverryan
Copy link
Member

I would recommend waiting for 5.4 to be released so that we can have a version of the book with support for the new maker bundle.

That makes sense to me. Indeed, this recipe is not a blocker for MakerBundle. Once it is merged, we'll just need to update some MakerBundle tests - no big deal :).

@weaverryan
Copy link
Member

Is it safe to merge this PR now? Without it, if you start a brand new Symfony 6 project / PHP 8, etc, you still get annotation-powered entities instead of attributes.

@fabpot fabpot merged commit b16ba4a into symfony:master Dec 22, 2021
weaverryan added a commit to symfony/maker-bundle that referenced this pull request Jan 3, 2022
…ushlow)

This PR was merged into the 1.0-dev branch.

Discussion
----------

[tests] handle missing type config value for doctrine

handles recipe change from symfony/recipes#971 for `DoctrineBundle`

Commits
-------

df382a3 [tests] handle missing type config value for doctrine
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.

7 participants