perf: optimize the permission check to use an in-memory object

[dkarlovi]

Closes #886.

Comparison current and new, fetching 100 products in a listing:

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[dkarlovi]

I have read the CLA Document and I hereby sign the CLA

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[mattamon]

@dkarlovi tested it and it works fine. My only concern is that the $cacheKey does not get invalidated if you save the configuration. If the permissions change you need to call cache:clear to make it work.
Could you also please invalidate the key on config save.

Also maybe change the key to datahub_workspace_permissions_ to make it more specific.

[dkarlovi]

@mattamon is there a cache tag which gets cleared on workspace save, like asset_ID or object_ID do when they're saved? We can then add a tag to the cache item and it gets automagically invalidated.

[mattamon]

@mattamon is there a cache tag which gets cleared on workspace save, like asset_ID or object_ID do when they're saved? We can then add a tag to the cache item and it gets automagically invalidated.

Hmm, I checked and it does not seem like it. The configs are in general not cached, other than by the locationaware dao.
But this is a static cache.

@fashxp do you have a tag in mind? Otherwise I would introduce a tag and change the Configuration Dao save method to clear that tag.

[dkarlovi]

The configs are in general not cached

IMO that's also an opportunity to improve: why aren't they cached, wouldn't the DAO do that by default? 🤔 This would mean we'd get cache invalidation for free here just by tagging.

[mattamon]

The configs are in general not cached

IMO that's also an opportunity to improve: why aren't they cached, wouldn't the DAO do that by default? 🤔 This would mean we'd get cache invalidation for free here just by tagging.

Yeah I thought the same.
Like I wrote they are kind of cached here https://github.com/pimcore/pimcore/blob/11.x/lib/Model/Dao/PimcoreLocationAwareConfigDao.php but in a static cache.

[fashxp]

@fashxp do you have a tag in mind? Otherwise I would introduce a tag and change the Configuration Dao save method to clear that tag.

nope, no tag in mind... let's introduce a new one.

[dkarlovi]

@mattamon friendly ping, did you maybe get a chance to add the cache tag flush on save by any chance in the meantime?

[mattamon]

@dkarlovi ahhh, I actually thought that you will introduce it, since it can be done in the same scope within your PR.
Sorry maybe there was some miscommunication on my part cause I also asked to change datahub_workspace_permissions_ and for me it was clear that you need to do that update on the PR.

So I guess, I have to ask now, can you please introduce this tag in your PR? Would be awesome!

[dkarlovi]

I would introduce a tag and change the Configuration Dao save method to clear that tag.

Yes, this directly told me you'll be creating a separate PR to do this. :) OK, I'll try to do both here.

[mattamon]

I would introduce a tag and change the Configuration Dao save method to clear that tag.

Yes, this directly told me you'll be creating a separate PR to do this. :) OK, I'll try to do both here.

Thank you! And sorry again for the confusion!

[CLAassistant]

All committers have signed the CLA.

[dkarlovi]

@mattamon ready for review, should be OK now.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[mattamon]

Thank you very much! It works perfectly now!