In the commitsymfony/symfony-standard@0080556, we introduced in the standard edition the usage ofserializer.mapping.cache.doctrine.apc instead ofserializer.mapping.cache.apc inconfig_prod.yml comments.

Earlier, we introduced the validator equivalent modification (validator.mapping.cache.doctrine.apc instead ofvalidator.mapping.cache.apc) but while we adapted the validator configuration in the FrameworkBundle in#16822, we did not adapt the FrameworkBundle configuration for the serializer.

I tested the current master of symfony-standard and it's indeed failing:

[Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]                                    The service "serializer" has a dependency on a non-existent service "serializer.mapping.cache.doctrine.apc".

This PR renames the serializer APCu cache service name to fix this issue. However, I'm not sure when the validator cache service modification was merged and released so I'm not sure how this PR should handle this. Is this a bug? Or is this a new feature and we should trigger a depreciation but keep the serviceserializer.mapping.cache.apc usable?

carsonbot added the Status: Needs Review label

Apr 16, 2016

Copy link

ContributorAuthor

tgalopin commentedApr 16, 2016

@dunglas: do you know what happened after your comment:#16795 (comment) ?

javiereguiluz added Serializer FrameworkBundle labels

Apr 17, 2016

Copy link

Member

nicolas-grekas commentedApr 17, 2016

ping@wouterj

Copy link

Member

wouterj commentedApr 17, 2016

The validator service was renamed because the Validator component switched from using their ownApcCache class to Doctrine Cache library in Symfony 2.5. To be able to provide a clean upgrade path, the new service was added in 2.8 (refs#14429).

The serializer service was added in Symfony 2.7 (refs#13107). As it directly used doctrine cache, I think it should have been namedserializer.mapping.cache.doctrine.apc for consistency. However, this was not done.

I think it makes sense to do this renaming now. Just like all other renamings, the old service should be deprecated in favor of the new service and it should be merged into master (probably 3.1). The old service should then be removed in Symfony 4.

Meanwhile, a PR should be submitted to the 2.8 branch of the Standard Edition to revertsymfony/symfony-standard@0080556 This was a mistake (and it shows the need for consistency btw).

tgalopin force-pushed theserializer_apc_cache_service branch frombd84187 to7d5e658Compare

April 18, 2016 16:10

Copy link

ContributorAuthor

tgalopin commentedApr 18, 2016

I update this PR to have the behavior@wouterj recomanded. I'm going to create the 2.8 PR.

tgalopin force-pushed theserializer_apc_cache_service branch 3 times, most recently from3c6016d to6293226Compare

April 18, 2016 17:55

Copy link

ContributorAuthor

tgalopin commentedApr 18, 2016

I think this is ready, targeting 3.1.

Copy link

Member

nicolas-grekas commentedApr 18, 2016

👍 for 3.1

Copy link

Member

wouterj commentedApr 18, 2016

Status: reviewed

Looks good. Can you please also update theCHANGELOG.md file in the FrameworkBundle andUPGRADE-3.1.md andUPGRADE-4.0.md to explain that this service is renamed?

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Apr 18, 2016

Copy link

ContributorAuthor

tgalopin commentedApr 18, 2016

I'll do that tomorrow. Thanks for your review!

tgalopin force-pushed theserializer_apc_cache_service branch from6293226 tod6b1f5dCompare

April 19, 2016 06:24

Copy link

ContributorAuthor

tgalopin commentedApr 19, 2016

I did the change.

xabbuh reviewed

Apr 19, 2016

View reviewed changes

UPGRADE-3.1.md Outdated

		-`"form.type.submit"`
		-`"form.type.reset"`

		* The service`serializer.mapping.cache.apc` has been renamed to

Copy link

Member

xabbuhApr 19, 2016

Choose a reason for hiding this comment

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

[...] has been deprecated in favor of [...]

tgalopin force-pushed theserializer_apc_cache_service branch fromd6b1f5d to5348344Compare

April 19, 2016 06:31

[FrameworkBundle][Serializer] Fix APC cache service name and deprecat…

88ef89c

…e old name

tgalopin force-pushed theserializer_apc_cache_service branch from5348344 to88ef89cCompare

April 19, 2016 06:52

Copy link

ContributorAuthor

tgalopin commentedApr 19, 2016

I'm not sure what the PR in 2.8 should look like. Shouldn't we apply the same PR as this one on 2.8? The validator cache service was renamed in 2.8, we could rename the serializer in 2.8 too couldn't we?

Copy link

Member

xabbuh commentedApr 19, 2016

@tgalopin It's about updating the commented config in the Symfony Standard Edition where we use the wrong service id:https://github.com/symfony/symfony-standard/blob/2.8/app/config/config_prod.yml#L8

Copy link

Member

dunglas commentedApr 19, 2016•
edited
Loading

What do you think about deprecating the class itself and recommending the PSR-6 backend instead?

Copy link

Member

nicolas-grekas commentedApr 19, 2016

@dunglas IMHO it's too early to deprecate for 3.1, but for sure we shouldconsider deprecating once we're used to PSR-6 caching and we're sure this wouldn't prevent any valid use case (which needs some time to me to be confirmed).

Copy link

Member

dunglas commentedApr 19, 2016

So 👍 for this change.

Copy link

ContributorAuthor

tgalopin commentedApr 19, 2016

The failure is unrelated.

Copy link

Member

xabbuh commentedApr 19, 2016

👍

Copy link

Member

nicolas-grekas commentedApr 19, 2016

Thank you@tgalopin.

nicolas-grekas merged commit88ef89c intosymfony:master

Apr 19, 2016

nicolas-grekas added a commit that referenced this pull request

Apr 19, 2016

feature#18567[FrameworkBundle][Serializer] Fix APC cache service na…

d74a4db

…me (tgalopin)This PR was merged into the 3.1-dev branch.Discussion----------[FrameworkBundle][Serializer] Fix APC cache service name| Q             | A| ------------- | ---| Branch?       | master or 3.0 (not sure)| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -In the commitsymfony/symfony-standard@0080556, we introduced in the standard edition the usage of `serializer.mapping.cache.doctrine.apc` instead of `serializer.mapping.cache.apc` in `config_prod.yml` comments.Earlier, we introduced the validator equivalent modification (`validator.mapping.cache.doctrine.apc` instead of `validator.mapping.cache.apc`) but while we adapted the validator configuration in the FrameworkBundle in#16822, we did not adapt the FrameworkBundle configuration for the serializer.I tested the current master of symfony-standard and it's indeed failing:```[Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]The service "serializer" has a dependency on a non-existent service "serializer.mapping.cache.doctrine.apc".```This PR renames the serializer APCu cache service name to fix this issue. However, I'm not sure when the validator cache service modification was merged and released so I'm not sure how this PR should handle this. Is this a bug? Or is this a new feature and we should trigger a depreciation but keep the service `serializer.mapping.cache.apc` usable?Commits-------88ef89c [FrameworkBundle][Serializer] Fix APC cache service name and deprecate old name

Copy link

Member