The most pertinent comment from the referenced PR:

This has always been an issue with the JCacheController classes. JCacheController aims to be a usable generic controller and each of the subclasses has a different signature for the get() method. This API needs to be looked at and a way of handling the different method signatures needs to be found.

Just for reference, it would most likely stop emitting a warning if we remove the typehinted callable, but this should really only be a last resort option.

What do you think about join/move JCacheController methods like get, store and a few others to JCache class.

JCacheControler should be tiny.

JCacheControllerView, JCacheControllerCallback, JCacheControllerPage instances then may have to use more reference to public $this->cache->.... instead implementing direct methods.

JCacheController subclasses probably does not need to store any options.

//$cacheController = JFactory::getCache('group', 'callback');
// To be more explicitly maybe we can use `JFactory::getCacheControler`.
$cacheController = JFactory::getCacheControler('callback');
$cacheController->get($callback, $args, 'id', 'group');
// or
$cacheController->cache->get('id', 'group');

IMHO JCache and storages classes should (if possible) remove all JFactory::... stuff.
Config registry can be injected in constructor of JCache as in example.

class JCache
{
// ...
    public function __construct(Registry $config, $options = [])
    {

Probably methods like getWorkarounds, setWorkarounds can be moved to JCacheController class.

I think now that we've got our DI container integrated we can look at either refactoring the existing factory methods or creating new methods that do a better job of building the parameters to be injected and doing just as you said with removing the JFactory references.

Is there a reason to not start with where we got with the 2.x PSR compliant version? https://github.com/joomla-framework/cache/tree/2.0-dev

That work now is mostly PSR spec only with minimal extra features.

PSR-6 doesn't define methods compatible with our existing JCache gc,
getAll, and clean methods. Those would have to be replicated somehow.

PSR-6 doesn't define lock/unlock behaviors. We'll need to reconstruct
those.

PSR-6 doesn't have a grouping construct. We'll need that.

https://github.com/php-cache/cache has a bunch of logic on grouping and
other organizational structure. We could possibly use their high level
abstractions to work with that.

On Wednesday, September 28, 2016, George Wilson notifications@github.com
wrote:

Is there a reason to not start with where we got with the 2.x PSR
compliant version? https://github.com/joomla-framework/cache/tree/2.0-dev

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12133 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoaWcerATN9XE_hNSzVhiQw6Cot9sks5qua4rgaJpZM4KDJzX
.

If there is interest in working with the PSR spec, mbabker#25 is a starting point.

1. Is it worth to deprecate get and store methods in JCacheController and copy it to JCacheControllerOutput for J3.7?
   It is missing only there.
   Is it the right way?
   Then get and store won't have any restriction in subclasses of JCacheController in J4.
   This resolve current issue.

2. I wish to deprecate method getWorkarounds and setWorkarounds in JCache and copy it to
   JCacheController. Am I right?

1. You would have to also make the base controller class abstract and that
   will invalidate any code which passes an empty value to
   JFactory::getCache() or JCache::getInstance() to fetch a controller.

2. Not sure I see the value in moving these methods right now. Care to
   explain more?

On Friday, September 30, 2016, Tomasz Narloch notifications@github.com
wrote:

1.

Is it worth to deprecate get and store methods in JCacheController and
copy it to JCacheControllerOutput for J3.7?
It is missing only there.
Is it the right way?
Then get and store won't have any restriction in subclasses of
JCacheController in J4.
This resolve current issue.
2.

I wish to deprecate method getWorkarounds and setWorkarounds in JCache
and copy it to
JCacheController. Am I right?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12133 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXFIDRtXyTj1F0PKv9LNk-coeGyMks5qvLsCgaJpZM4KDJzX
.

1. Ok, but you say about J4.0. Do you want first make changes in J4 and after that mark code as deprecated in J3.x?

2. For me that methods are not directly related with JCache.
   Only controllers use that methods.
   IMHO this was designed for controllers and controllers should have an option to override parent method 'getWorkarounds' or 'setWorkarounds'.

1. If we're going to do it in 4.0 then by the time 3.7 releases whatever
   annotations need to exist in the 3.x code have to be there otherwise I'd
   suggest we probably can't do it.

2. I can't think of a case where the controllers should have their own
   workarounds implemented personally. Not saying there isn't one, but last I
   remember looking at that code it seemed pretty high level and not something
   I'd expect to be extended.

On Friday, September 30, 2016, Tomasz Narloch notifications@github.com
wrote:

1.

Ok, but you say about J4.0. Do you want first make changes in J4 and
after that mark code as deprecated in J3.x?
2.

For me that methods are not directly related with JCache.
Only controllers use that methods.
IMHO this was designed for controllers and controllers should have an
option to override parent method 'getWorkarounds' or 'setWorkarounds'.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12133 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXtph_BjuVFo2EaKfjn3HW-dKotBks5qvPLxgaJpZM4KDJzX
.

1) If I understand you well I should do changes only on branch 3.7.
What about B/C and change JCacheController to abstract class?

2) I don't want to force any overrides (I should not mentioned it), but only point that modification of cache data structure before save and after load is more related with JCacheController subclasses.

In controller subclasses we can call it like "JCacheController::getWorkaround()" as a static method.

If you do not see my point then I will leave that.

3) [Updated: Ignore this, I see you done it in your branch]
The other issue is where to serialize cache data.
I thing it may go to adapters, but I do not want to touch it now.
http://www.php-fig.org/psr/psr-6/#data

1) Deprecate in JCacheController the methods you want to see removed from 4.0 and comment on the class that it will become abstract using a @note annotation on the class doc block.

2) It's not that I disagree with you necessarily, I'm just not seeing the logic behind the proposal right now is all. They're both static methods so they're pretty much disconnected from the actual cache internals anyway, so moving them just calls for a name change. But if there's an intent that a controller could extend the methods then they probably need to not be static if moved.

3) On the whole serialization, I honestly don't think it's an adapter responsibility either. Then again, after looking at different code examples, I'm not sure what's right. What I do know is our help screen system application isn't explicitly serializing anything with the cache (https://github.com/joomla/help.joomla.org/blob/master/src/Controller/HelpScreenController.php) but since we're using the filesystem for the cache that adapter does serialize before writing the data (in comparison our Memcached adapter doesn't serialize).

3) If joomla in the future will support cache adapter like opcache then serialization must be done in adatpter.

https://blog.graphiq.com/500x-faster-caching-than-redis-memcache-apc-in-php-hhvm-dcd26e8447ad#.klw4pwg7q

For now I've temporarily reverted the typehint. d822a14

But we absolutely need to come up with a better solution for J4 final. So let's keep this issue open with a release blocker tag to track progress :)

As long as JCacheController is NOT abstract and contains its own get() method then there is no other fix than to restore the is_callable() check at the beginning of the method and call this complete.

This part of the API has massive consistency issues and needs a lot more attention than quick "hackjobs" like that commit.

I agree it needs work. Hence why I'm leaving this open with a release blocker tag and not just closing it and pretending the issue has gone away ;)

I have prepared some time ago a PR #12312 for this issue and add a few more improvement.