[FrameworkBundle] Remove dependency on Doctrine cache #23131
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xabbuh
reviewed
Jun 11, 2017
UPGRADE-3.3.md
Outdated
| @@ -165,6 +165,9 @@ Form | |||
| FrameworkBundle | |||
| --------------- | |||
|
|
|||
| * The `doctrine/cache` dependency has been removed; require it via `composer | |||
| require doctrine/cache` if you are using Doctrine cache in your project. | |||
There was a problem hiding this comment.
this should be part of the UPGRADE-3.4.md file
|
I think we also need check for the existence of the Doctrine Cache package when annotation support is enabled in the config as by default we use a caching annotation reader. |
8 tasks
stof
requested changes
Jun 12, 2017
| ->beforeNormalization() | ||
| // Can be removed in 4.0, when validator.mapping.cache.apc is removed | ||
| ->ifString()->then(function ($v) { | ||
| if ('validator.mapping.cache.apc' === $v && !class_exists('Doctrine\Common\Cache\ApcCache')) { |
There was a problem hiding this comment.
shouldn't this be validator.mapping.cache.doctrine.apc ?
|
@xabbuh is right about the annotation reader |
1161555 to
1c33f02
Compare
|
check on annotations added |
stof
requested changes
Jun 12, 2017
| @@ -1217,6 +1217,10 @@ private function registerAnnotationsConfiguration(array $config, ContainerBuilde | |||
| throw new LogicException('Annotations cannot be enabled as the Doctrine Annotation library is not installed.'); | |||
| } | |||
|
|
|||
| if (!class_exists('Doctrine\Common\Cache\ApcCache')) { | |||
There was a problem hiding this comment.
I would check the existence of the interface rather than loading a particular implementation (which will not be the one we will use).
| @@ -1217,6 +1217,10 @@ private function registerAnnotationsConfiguration(array $config, ContainerBuilde | |||
| throw new LogicException('Annotations cannot be enabled as the Doctrine Annotation library is not installed.'); | |||
| } | |||
|
|
|||
| if (!class_exists('Doctrine\Common\Cache\ApcCache')) { | |||
| throw new LogicException('Annotations cannot be enabled as the Doctrine Cache library is not installed.'); | |||
There was a problem hiding this comment.
this exception must be inside the if ('none' !== $config['cache']) {. We don't need the doctrine/cache library when disabling the annotation caching.
1c33f02 to
a4e336e
Compare
fabpot
added a commit
that referenced
this pull request
Jun 12, 2017
…(fabpot) This PR was squashed before being merged into the 3.4 branch (closes #23131). Discussion ---------- [FrameworkBundle] Remove dependency on Doctrine cache | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | yes | Tests pass? | yes | Fixed tickets | related to symfony/flex#14 | License | MIT | Doc PR | n/a In our quest to remove hard dependencies, I propose to remove `doctrine/cache` from the default dependencies on the Framework bundle. That's possible now as we have PSR6 cache support in Symfony and because Doctrine cache is only used for the validator mapping cache. The two other occurrences are for the serializer (already deprecated in 3.3) and for annotations, where we need to keep it, but as Doctrine annotations is already optional, that's not an issue. Commits ------- a4e336e [FrameworkBundle] removed doctrine/cache as a dependency b57895c [FrameworkBundle] deprecated validator.mapping.cache.doctrine.apc
fabpot
added a commit
that referenced
this pull request
Jun 14, 2017
… failing unit tests (derrabus) This PR was merged into the 3.4 branch. Discussion ---------- [TwigBundle] Add Doctrine Cache to dev dependencies to fix failing unit tests | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | None | License | MIT | Doc PR | N/A Currently, the unit tests of TwigBundle are failing because Doctrine Cache is missing. Before the changes of #23131, this dependency was pulled via FrameworkBundle. This PR adds Doctrine Cache to the dev dependencies of TwigBundle, so the tests pass again. ``` There were 3 errors: 1) Symfony\Bundle\TwigBundle\Tests\CacheWarmingTest::testCacheIsProperlyWarmedWhenTemplatingIsAvailable Symfony\Component\DependencyInjection\Exception\LogicException: Annotations cannot be enabled as the Doctrine Cache library is not installed. 2) Symfony\Bundle\TwigBundle\Tests\CacheWarmingTest::testCacheIsProperlyWarmedWhenTemplatingIsDisabled Symfony\Component\DependencyInjection\Exception\LogicException: Annotations cannot be enabled as the Doctrine Cache library is not installed. 3) Symfony\Bundle\TwigBundle\Tests\NoTemplatingEntryTest::test Symfony\Component\DependencyInjection\Exception\LogicException: Annotations cannot be enabled as the Doctrine Cache library is not installed. ``` Commits ------- 2dfde58 Add Doctrine Cache to dev dependencies to fix failing unit tests.
This was referenced Oct 18, 2017
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In our quest to remove hard dependencies, I propose to remove
doctrine/cachefrom the default dependencies on the Framework bundle. That's possible now as we have PSR6 cache support in Symfony and because Doctrine cache is only used for the validator mapping cache.The two other occurrences are for the serializer (already deprecated in 3.3) and for annotations, where we need to keep it, but as Doctrine annotations is already optional, that's not an issue.