Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

OriginalRouteExtractor Serialization of 'Closure' is not allowed #332

Open
gomcodoctor opened this issue Sep 6, 2017 · 11 comments
Open
Assignees
Labels

Comments

@gomcodoctor
Copy link

When trying to use OriginalRouteExtractor

there is exception " Serialization of 'Closure' is not allowed " because of router service having a closure

public function serialize() { return serialize([ $this->extractors, $this->resource, $this->createdAt, ]); }

-configCacheFactory: ResourceCheckerConfigCacheFactory {#365 ▼ -resourceCheckers: RewindableGenerator {#363 ▼ -generator: Closure {#364 ▼ class: "appDevDebugProjectContainer" this: appDevDebugProjectContainer {#519 …14} file: "/var/cache/dev/appDevDebugProjectContainer.php" line: "2038 to 2041" } -count: 2 }

@ElectricMaxxx ElectricMaxxx self-assigned this Sep 8, 2017
@ElectricMaxxx
Copy link
Member

Hi @gomcodoctor
thank for reporting that but. Can you give some more information like the Symfony/PHP version and a real stack trace?

@gomcodoctor
Copy link
Author

gomcodoctor commented Sep 8, 2017

@ElectricMaxxx thanks for replying

Serialization of 'Closure' is not allowed,

PHP version 7.1.9
Symfony 3.3.5

in vendor/symfony-cmf/seo-bundle/src/Cache/ExtractorCollection.php (line 88)
SplObjectStorage->serialize()
serialize(array(array(object(TitleExtractor), object(DescriptionExtractor), object(ExtrasExtractor), object(KeywordsExtractor), object(OriginalUrlExtractor), object(OriginalRouteExtractor)),
'/home/*****/sylius/src/Gomco/OfferBundle/Entity/ProductVariant.php', 1504846997))

in vendor/symfony-cmf/seo-bundle/src/Cache/ExtractorCollection.php (line 88)
ExtractorCollection->serialize()
serialize(object(ExtractorCollection))

in vendor/symfony-cmf/seo-bundle/src/Cache/FileCache.php (line 71)
FileCache->putExtractorsInCache('Gomco\OfferBundle\Entity\ProductVariant', object(ExtractorCollection))

in vendor/symfony-cmf/seo-bundle/src/SeoPresentation.php (line 173)
SeoPresentation->getSeoMetadata(object(ProductVariant))

in vendor/symfony-cmf/seo-bundle/src/SeoPresentation.php (line 213)

@gomcodoctor
Copy link
Author

I have debugged it, issue occur when we serialize "$urlGenerator" ( which is router service ) in OriginalRouteExtractor, because router service has Closure {#364 ▼ class: "appDevDebugProjectContainer

@ElectricMaxxx
Copy link
Member

Maybe @wouterj can give some hints as he created the cache. My first idea is to implement Serializeableand to add/remove the dependency, but that isn't that easy.

@gomcodoctor
Copy link
Author

@ElectricMaxxx ya i think Serializeable is only solution or disable cache, lets wait for @wouterj 's reply

@ElectricMaxxx
Copy link
Member

In generall i would ask @wouterj why decided to cache an object that generates something instead of caching the result it had?

@gomcodoctor
Copy link
Author

as per my understanding - he is trying to cache all extractors OBJECT applicable to given object or resource, extractor which has dependency on other service is prone to this issue, instead of caching whole extractor object, caching name of extractor is enough i think, Please correct me if I am wrong, I am not that symfony expert

@wouterj
Copy link
Member

wouterj commented Sep 8, 2017

Yeah, indeed, caching the name is probably enough. We then have to rework SeoPresentation a bit (if the extractor class names are cached, we have to filter all instances based on the cache name before using them), but it's probably the best solution.

@ElectricMaxxx caching values would mean we have to clear the cache each time the value changes. And that value most of the time is a getter, so it can depend on anything out there in the world. Caching here is used to already know which extractors apply to the object, which speed up things a bit already.

@gomcodoctor
Copy link
Author

@wouterj thanks for reply.
There are few more suggestions if you going to edit cache part

  1. currently it do no take care of dev mode of environment, it always cache , cache should be disabled in dev mode.
  2. there should be option to enable and disable cache in config file

@ElectricMaxxx
Copy link
Member

@wouterj but where is the advantage to store a class name and instantiate and call it then, when it normally commes as service defintion from DI, which should be cached at all?

@gomcodoctor
Copy link
Author

@ElectricMaxxx you are right, I think we do not need cache when number of extractors are very small, cache will be useful when there are hundreds of extractors

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

No branches or pull requests

3 participants