10000 [2.7][BC Break][Translation] Undocumented BC break regarding caching · Issue #14689 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[2.7][BC Break][Translation] Undocumented BC break regarding caching #14689

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.

Sign up for GitHub

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

Jump to bottom
Closed
linaori opened this issue May 19, 2015 · 8 comments
Closed

[2.7][BC Break][Translation] Undocumented BC break regarding caching #14689

linaori opened this issue May 19, 2015 · 8 comments
Labels
BC Break Translation

Comments

@linaori
Copy link
Contributor
linaori commented May 19, 2015

I'm using a custom translation loader where the loader is the resource, a database loader like this one. The problem is that as of symfony 2.7, the creating of the cache is slightly changed, it also serializes the resource now. Because my resource is using an entity manager to fetch the translations, it tries to serialize the EM and this of course fails.

A solution is to implement the Serializable interface or to use the spl_object_hash() instead.

https://github.com/symfony/symfony/blob/2.6/src/Symfony/Component/Translation/Translator.php#L376-L379

    // 2.6
    private function getCatalogueCachePath($locale)
    {
        return $this->cacheDir.'/catalogue.'.$locale.'.'.sha1(serialize($this->fallbackLocales)).'.php';
    }

https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Translation/Translator.php#L420-L428

    // 2.7
    private function getCatalogueCachePath($locale)
    {
        $catalogueHash = sha1(serialize(array(
            'resources' => isset($this->resources[$locale]) ? $this->resources[$locale] : array(),
            'fallback_locales' => $this->fallbackLocales,
        )));
        return $this->cacheDir.'/catalogue.'.$locale.'.'.$catalogueHash.'.php';
    }
@fabpot
Copy link
Member
fabpot commented May 19, 2015

ping @aitboudad

@aitboudad
Copy link
Contributor

I didn't introduced anythink I tried to just to improve #13074, we can just use count to solve the serialize issue.

        $catalogueHash = sha1(serialize(array(
            'resources' => isset($this->resources[$locale]) ? count($this->resources[$locale]) : 0,
            'fallback_locales' => $this->fallbackLocales,
        )));

@linaori
Copy link
Contributor Author
linaori commented May 19, 2015

Seems like a good fix to me 👍

@stof
Copy link
Member
stof commented May 20, 2015

@iltar Resources are already serialized in other places (in the .meta file of the ConfigCache for instance). so 2.6 was probably already serializing them, but in a different place.

@linaori
Copy link
Contributor Author
linaori commented May 20, 2015

@stof I don't know about that, but this is since 2.7 as you can see in the changes in the file. In 2.6 this works just fine but in 2.7 and 2.8 it's broken. I don't think the resource was serialized the sa 8000 me way as now in that case or I'm using a different strategy without knowing it.

@aitboudad aitboudad mentioned this issue May 20, 2015
Merged
@aitboudad
Copy link
Contributor

fixed by #14705

@mpdude
Copy link
Contributor
mpdude commented May 31, 2015

@stof Sure we're talking about the same kind of resource here?

@mpdude
Copy link
Contributor
mpdude commented May 31, 2015

The problem that we tried to solve was that the Translator needs to use a different cache file when it is configured differently (think of resources added or removed or configured differently).

Just using the number of resources added will not suffice to cover all the (edge?) cases.

I don't think the resources themselves have to be serializable (they are not necessariliy those put into the ConfigCache), we just need to be able to hash them.

@xabbuh xabbuh mentioned this issue Jun 5, 2015
Closed
fabpot added a commit that referenced this issue Jun 5, 2015
@fabpot
bug #14705 [Translator] avoid serialize unserializable resources. (ai…
34ad82e 
…tboudad)

This PR was merged into the 2.7 branch.

Discussion
----------

[Translator] avoid serialize unserializable resources.

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

Commits
-------

7220f2c [Translator] avoid serialize unserializable resources.
@fabpot fabpot closed this as completed Jun 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BC Break Translation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @stof @mpdude @aitboudad @linaori @xabbuh
0