Skip to content
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.

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

Performance: getConfigurableAttributes cache is broken #6999

Closed
paales opened this issue Oct 13, 2016 · 6 comments
Closed

Performance: getConfigurableAttributes cache is broken #6999

paales opened this issue Oct 13, 2016 · 6 comments
Labels
bug report Component: Framework/Cache Fixed in 2.1.x The issue has been fixed in 2.1 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Partner: Reach Digital Pull Request is created by partner Reach Digital

Comments

@paales
Copy link
Contributor

paales commented Oct 13, 2016

Preconditions

  1. 2.1.2
  2. Have configurables

Steps to reproduce

  1. After loading the configurable attributes it stores the result in the cache.
  2. After loading the second time it tries to load it from the cache, but the hasCacheData method always returns false

Expected result

  1. Should use cache

Actual result

  1. Doesn't use cache

The two selected lines should be replaced:

$configurableAttributes = $configurableAttributes ?: unserialize($configurableAttributes);
if (is_array($configurableAttributes) && count($configurableAttributes)) {

With

$configurableAttributes = $configurableAttributes ? unserialize($configurableAttributes) : false;
if (is_object($configurableAttributes) && $configurableAttributes->count()) {
@paales paales changed the title getConfigurableAttributes cache is broken Performance: getConfigurableAttributes cache is broken Oct 14, 2016
@thlassche
Copy link
Contributor

@paales Your proposed check was not sufficient. I opened a pull request (#7376) for this.

@paales
Copy link
Contributor Author

paales commented Nov 9, 2016

Hmm, I haven't experienced that it is still an array.. Maybe Magento could write some tests to check this ;)

magento-team pushed a commit that referenced this issue Jun 27, 2017
…r hit #9809

 - Merge Pull Request #9809 from thlassche/magento2:2.1-develop
@magento-team magento-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Framework/Cache labels Jul 31, 2017
@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-69587

@thlassche
Copy link
Contributor

@magento-team It's already fixed in a merged PR to develop

@korostii
Copy link
Contributor

korostii commented Aug 1, 2017

Never mind that, @magento-team's comment seems to be automatic. The bot is probably walking through all the internal ticket references it has, no matter the status.

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Framework/Cache Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Sep 11, 2017
@magento-engcom-team
Copy link
Contributor

@paales the issue already fixed in 2.1.10 and will be available soon

@magento-engcom-team magento-engcom-team added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label Oct 6, 2017
@paales paales added the Partner: Reach Digital Pull Request is created by partner Reach Digital label May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Framework/Cache Fixed in 2.1.x The issue has been fixed in 2.1 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Partner: Reach Digital Pull Request is created by partner Reach Digital
Projects
None yet
Development

No branches or pull requests

6 participants