[FrameworkBundle][Validator] Fix apc cache service & config #16795

ogizanagi · 2015-12-02T02:34:21Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#16793
License	MIT
Doc PR	-

Keep track of #16794 (comment)

NOTE: This PR is on standby. If #16822 is merged, this one might probably be closed, as everything will be done during the merge.

jderusse · 2015-12-02T06:11:58Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

-                        ->scalarNode('cache')
-                            ->beforeNormalization()
-                                // Can be removed in 3.0, once ApcCache support is dropped
-                                ->ifString()->then(function ($v) { return 'apc' === $v ? 'validator.mapping.cache.apc' : $v; })


The previous deprecated notice says, the class ApcCache will be removed, not that the alias "APC" will be discontinued.
This is a new BC break

Three options then:

trigger a deprecation in 2.8, now released ? (as a "bugfix": a missing deprecation notice)

keep the ability to pass "apc", but trigger a new deprecation in 3.0. Then, remove this ability in 3.1

keep the ability to pass "apc"

Given Symfony 3 had been released, the options 1 is not valid
Given it's a BC break, the option 2 is not valid. => Have to wait Symfony 4
IMO option 3 is the only one available

ping @nicolas-grekas

@jderusse I don't agree. There is no BC break in 3.0 if we remove the apc logic. Configuring apc in 3.0 does not work anyway. ApcCache does not exist anymore.

So we should go with option 1 as planned: Add missing deprecation in 2.8 and remove validator.mapping.cache.apc service in 3.0

@Tobion : Thanks. See #16822 about this.

Removing the feature because it's not documented, mays be a point.
BUT, If someone use the configuration cache: apc with symfony 2.7, he never been warned about the deprecation. Upgrading to Symfony 3 lead to an error due to the missing class.
He may expect we fix the bug, instead of removing the feature.

Just my 2 cents and playing the devil's advocate. I understand the point, to cleanup this code 😉

In order to upgrade from symfony 2.7 to 3.0, he'll need to update to 2.8 first. Then adding the deprecation in 2.8 in a bugfix is the right thing to do, and no one should expect having this service anymore in 3.0.

I removed the BC flag from the PR description accordingly. The existence of the validator.mapping.cache.apc service and the removal of the validator.mapping.cache.doctrine.apc one in 3.0 (as well as its deprecation in 2.8 instead of first one) is a bug and should be fixed in both branches.

Tobion · 2015-12-03T12:44:38Z

Status: Needs Work

jderusse · 2015-12-03T22:24:23Z

Notice that service validator.mapping.cache.doctrine.apchad been previously declared and merged (https://github.com/symfony/symfony/pull/14429/files) and documented (http://symfony.com/doc/current/reference/configuration/framework.html#reference-validation-cache) but then removed by @nicolas-grekas 5aa4a3b

ogizanagi · 2015-12-03T22:59:58Z

Yes, we can also notice that the validator.mapping.cache.doctrine.apc was a feature of 2.8. It would have been really strange to make this feature and deprecate it directly.
So everything seems to confirm this line was unintentional and logically targeting the validator.mapping.cache.apc service instead.

dunglas · 2015-12-07T21:07:52Z

👍 but the serializer.mapping.cache.apc service should have be deprecated has well and new serializer.mapping.cache.doctrine.apc created.

As the serializer and the validator only support Doctrine Cache, I don't see what the doctrine in the service name brings.

jakzal · 2015-12-08T13:46:55Z

Something went wrong here. The validator.mapping.cache.doctrine.apc service exists in 2.8. Looks like it was deprecated in 2.8 instead of validator.mapping.cache.apc, and then removed in master.

ogizanagi · 2015-12-08T14:12:38Z

@jakzal : Yes, it's exactly what happened. But let's forget about this PR for now.
Instead, a decision should be taken first on #16822 where we should either:

Fix the deprecation on the right service (first commit)
Or let validator.mapping.cache.doctrine.apc deprecated in 2.8 and removed in 3.0 as @Tobion suggested in [FrameworkBundle][Validator] Fix apc cache service deprecation #16822 (comment) (second commit)

What bothers me however is to deprecate a service in the same version it appeared. Maybe we should only fix the deprecation on the right service in 2.8, and then deprecate validator.mapping.cache.doctrine.apc as well as serializer.mapping.cache.doctrine.apc in 3.1, for a complete removal in 4.0...

dunglas · 2015-12-10T07:32:22Z

👍 to merge this one.

dunglas · 2015-12-15T07:45:34Z

Any update regarding this PR? This is a blocker to use Symfony 3 in prod.

/cc @symfony/deciders

xabbuh · 2015-12-15T07:54:22Z

I would be in favour of merging this one and trigger a deprecation in 2.8.

ogizanagi · 2015-12-15T10:13:02Z

I suggest removing the second commit in #16822 (I'll do it right now) and merge it first. The deprecation should be fixed in 2.8 anyway. Then merge this one if necessary, but everything should be properly updated during the merge of 2.8 into 3.0.

jonathanbull · 2015-12-27T14:13:47Z

Any update regarding this PR? It looks like a fresh Symfony project still features an invalid validator.mapping.cache.apc setting.

wouterj · 2015-12-27T14:36:24Z

Isn't it better to just quickly fix this error by merging this or mine PRs and then later decide on whether more actions should be taken? Currently, Symfony 3 with validator mapping cache just doesn't work as it should: bad dev experience...

peterrehm · 2016-01-15T10:36:54Z

Any update on this?

…tion (ogizanagi) This PR was merged into the 2.8 branch. Discussion ---------- [FrameworkBundle][Validator] Fix apc cache service deprecation | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Related to #16795 I guess the deprecation was on the wrong service. Also, no deprecation notice was triggered about using `"apc"` as the value of the `framework.validation.cache` configuration option. This PR adds the missing deprecation. > 📝 _NOTE_: The standard edition will need to be updated [here](https://github.com/symfony/symfony-standard/blob/2.8/app/config/config_prod.yml#L6). Commits ------- 907bbec [FrameworkBundle][Validator] Fix apc cache service deprecation

ogizanagi · 2016-01-25T18:27:13Z

@fabpot : Since #16822 is now merged, this one can be merged too in order to remove the last leftover of the framework.validation.cache.apc service :)

fabpot · 2016-01-25T18:30:36Z

Thank you @ogizanagi.

…g (ogizanagi) This PR was merged into the 3.0 branch. Discussion ---------- [FrameworkBundle][Validator] Fix apc cache service & config | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16793 | License | MIT | Doc PR | - _Keep track of #16794 (comment) _NOTE_: This PR is on standby. If #16822 is merged, this one might probably be closed, as everything will be done during the merge. Commits ------- 94a1728 [FrameworkBundle][Validator] Fix apc cache service & config

carsonbot added the Status: Needs Review label Dec 2, 2015

ogizanagi mentioned this pull request Dec 2, 2015

[FrameworkBundle][Validator] Fix apc cache service & config #16794

Closed

jderusse reviewed Dec 2, 2015
View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Dec 3, 2015

ogizanagi mentioned this pull request Dec 3, 2015

[FrameworkBundle][Validator] Fix apc cache service deprecation #16822

Merged

This was referenced Dec 5, 2015

[PropertyAccess] Add PSR-6 cache #16838

Closed

[3.0] Revert removal of correct APC cache service & remove deprecated one #16866

Closed

[FrameworkBundle][Validator] Fix apc cache service & config

94a1728

fabpot merged commit 94a1728 into symfony:3.0 Jan 25, 2016

ogizanagi deleted the 16793_validator_apc_cache_service branch January 25, 2016 18:35

fabpot mentioned this pull request Feb 3, 2016

Release v3.0.2 #17665

Merged

raziel057 mentioned this pull request Feb 4, 2016

[3.0.2] The service "validator.builder" has a dependency on a non-existent service "apc" #17680

Closed

tgalopin mentioned this pull request Apr 16, 2016

[FrameworkBundle][Serializer] Fix APC cache service name #18567

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle][Validator] Fix apc cache service & config #16795

[FrameworkBundle][Validator] Fix apc cache service & config #16795

ogizanagi commented Dec 2, 2015

jderusse Dec 2, 2015

ogizanagi Dec 2, 2015

jderusse Dec 2, 2015

Tobion Dec 3, 2015

ogizanagi Dec 3, 2015

jderusse Dec 3, 2015

ogizanagi Dec 3, 2015

Tobion commented Dec 3, 2015

jderusse commented Dec 3, 2015

ogizanagi commented Dec 3, 2015

dunglas commented Dec 7, 2015

jakzal commented Dec 8, 2015

ogizanagi commented Dec 8, 2015

dunglas commented Dec 10, 2015

dunglas commented Dec 15, 2015

xabbuh commented Dec 15, 2015

ogizanagi commented Dec 15, 2015

jonathanbull commented Dec 27, 2015

wouterj commented Dec 27, 2015

peterrehm commented Jan 15, 2016

ogizanagi commented Jan 25, 2016

fabpot commented Jan 25, 2016

[FrameworkBundle][Validator] Fix apc cache service & config #16795

[FrameworkBundle][Validator] Fix apc cache service & config #16795

Conversation

ogizanagi commented Dec 2, 2015

jderusse Dec 2, 2015

Choose a reason for hiding this comment

ogizanagi Dec 2, 2015

Choose a reason for hiding this comment

jderusse Dec 2, 2015

Choose a reason for hiding this comment

Tobion Dec 3, 2015

Choose a reason for hiding this comment

ogizanagi Dec 3, 2015

Choose a reason for hiding this comment

jderusse Dec 3, 2015

Choose a reason for hiding this comment

ogizanagi Dec 3, 2015

Choose a reason for hiding this comment

Tobion commented Dec 3, 2015

jderusse commented Dec 3, 2015

ogizanagi commented Dec 3, 2015

dunglas commented Dec 7, 2015

jakzal commented Dec 8, 2015

ogizanagi commented Dec 8, 2015

dunglas commented Dec 10, 2015

dunglas commented Dec 15, 2015

xabbuh commented Dec 15, 2015

ogizanagi commented Dec 15, 2015

jonathanbull commented Dec 27, 2015

wouterj commented Dec 27, 2015

peterrehm commented Jan 15, 2016

ogizanagi commented Jan 25, 2016

fabpot commented Jan 25, 2016