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

FEATURE: Add methods getSimpleCache PSR-16 and getCacheItemPool PSR-6 to the cacheManager #2689

Merged
merged 6 commits into from
Mar 18, 2022

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented Feb 1, 2022

The methods create PSR Frontends for the cache with the given identifier. This allows to configure psr caches via
objects and caches yaml. This is an improvement as the included factories are not that easy to use.

The following settings in Objects.yaml allow to inject PSR Caches into Flow Classes:

Vendor\Site\Service:

  properties:
    simpleCacheProperty:
      object:
        factoryObjectName: Neos\Flow\Cache\CacheManager
        factoryMethodName: getSimpleCache
        arguments:
          1:
            value: Cache_Identifier_1

    cachePoolProperty:
      object:
        factoryObjectName: Neos\Flow\Cache\CacheManager
        factoryMethodName: getCacheItemPool
        arguments:
          1:
            value: Cache_Identifier_2

!!! While possible it is not advisible to access the same cache with multiple Interfaces as the storage formats may differ. !!!

Resolves: #2690

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • Docs are adjusted
  • The PR is created against the lowest maintained branch

@mficzel mficzel force-pushed the feature/createPsrCachesViaCacheManager branch 2 times, most recently from b774830 to 26da33d Compare February 1, 2022 22:19
@sorenmalling
Copy link
Contributor

@mficzel Can you create a corresponding issue that this PR can link to?

@sorenmalling
Copy link
Contributor

How is this different from the possibilities today of injecting a configured cache?

@mficzel
Copy link
Member Author

mficzel commented Feb 2, 2022

@sorenmalling it allows to get instances of the psr Cache classes. Those are different from flowCache frontends but external libraries likely need those for caching.

@mficzel mficzel changed the title FEATURE: Add methods getSimpleCache and getCacheItemPool to the cacheManager FEATURE: Add methods getSimpleCache PSR16 and getCacheItemPool PSR6 to the cacheManager Feb 2, 2022
@mficzel mficzel marked this pull request as ready for review February 4, 2022 16:41
@mficzel mficzel requested review from albe and kitsunet February 4, 2022 16:41
@mficzel
Copy link
Member Author

mficzel commented Feb 4, 2022

@kitsunet I remember us discussing how the psr caches should be instantiated back then. I think back then you planned something similar but for some reason we decided to only provide the psr factories back then as a first step. Probably it was my fault back talking you out of this because i feared users would access the same cache via different interfaces.

Had some use cases in the meantime where i would have liked the option to inject those cache-interfaces.

@mficzel mficzel changed the title FEATURE: Add methods getSimpleCache PSR16 and getCacheItemPool PSR6 to the cacheManager FEATURE: Add methods getSimpleCache PSR-16 and getCacheItemPool PSR-6 to the cacheManager Feb 4, 2022
@mficzel mficzel requested a review from fcool February 4, 2022 17:31
@albe
Copy link
Member

albe commented Feb 22, 2022

Just for reference, with the PSR factories of Neos\Cache\Psr one could inject a cache, but needs to define the backend class and options everytime. So the benefit is primarily that our existing cache configurations can be used to configure PSR caches, right?

!!! While possible it is not advisible to access the same cache with multiple Interfaces. !!!

Yeah, that's really a pandora box waiting to be opened. I see no good way around that though

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Generally approve. It's nice to be able to configure PSR caches and inject them like Flow caches.

@sorenmalling
Copy link
Contributor

Can the documentation be extended with a explanation on why it's not a good thing to access the same vacate with multiple interfaces?

@mficzel
Copy link
Member Author

mficzel commented Feb 24, 2022

@sorenmalling changed

!!! While possible it is not advisible to access the same cache with multiple Interfaces. !!!

to

!!! While possible it is not advisible to access the same cache with multiple Interfaces as the storage formats may differ. !!!

As already was in the docs

…Manager

The methods create an PSR Frontend for the cache with the given identifier.

The following settings in Objects.yaml allow to inject PSR Caches into Flow Classes:
```
Vendor\Site\Service:

  properties:
    simpleCacheProperty:
      object:
        factoryObjectName: Neos\Flow\Cache\CacheManager
        factoryMethodName: getSimpleCache
        arguments:
          1:
            value: Cache_Identifier_1

    cachePoolProperty:
      object:
        factoryObjectName: Neos\Flow\Cache\CacheManager
        factoryMethodName: getCacheItemPool
        arguments:
          1:
            value: Cache_Identifier_2
```

!!! While possible it is not advisible to access the same cache with multiple Interfaces !!!
… are used in there

This changes nothing regarding installed packages as those were required by Neos.Cache anyways.
@mficzel mficzel force-pushed the feature/createPsrCachesViaCacheManager branch from 5fe1151 to 2350bfd Compare February 24, 2022 16:10
Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Looks good by reading.

I guess people will shoot themselves in the foot by accessing the same cache from two sides, but… 🤷‍♂️

@mficzel mficzel merged commit 095c44d into neos:master Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEATURE: Add methods to create SimpleCache PSR-16 and CacheItemPool PSR-6 caches to cacheManager
4 participants