097 :: modCacheManager interface with other classes - clarification/debate

In an attempt to solve [MODX-24] http://svn.modxcms.com/jira/browse/MODX-24, I came to the conclusion that modContext was relying too heavily on functionality provided by modCacheManager::generateContext() . OpenGeek had reached the same conclusion, so I forged ahead.

Bear with me while I walk through the background to this.. I will raise an issue that I think there should be clarification/concensus on because many places throughout the system interface with modCacheManager, and currently many of them rely on caching being on by default.

modContext was asking modCacheManager::generateContext() to load up an instance of modContext and do a number of queries to retrieve and build up the following:
->config
->aliasMap
->resourceMap
->documentMap
->documentListing
->eventMap
->pluginCache

It built this up in a string of valid php code, and then saved that string to the appropriate modContext cache file.

The main reason for the bug I was trying to solve is that when global caching is turned off, modCacheManager was not available. Also, modContext was by default including the cache file in order to have those arrays set. So a) modContext was dependent upon the cache manager to generate information that it needed about itself, and b) modContext relied on the cache file in order to load that information.

My solution has been to add a new protected method in modContext which creates all the context-specific stuff that it needs. It either returns the string of php code, if caching is on, or it directly sets the values into the modContext instance if caching is off. modContext::prepare() now handles sending the string of php to the modCacheManager to handle the caching.

I am no oo genius. I would like to make sure that I am clear on
a) the role of modContext in generating it’s own data and interfacing with the modCacheManager
b) the role of modCacheManager in handling modContext’s cacheable data

I guess my general questions on this are
1. how much logic should modCacheManager implement that is specific to other classes
2. how much logic any class that’s interfacing with the modCacheManager should implement that affects the functionality of caching (such as generating file names as one example)

To suggest an alternative approach, maybe modCacheManager should handle everything even when caching is turned off? In this case, modContext could still call on modCacheManager, which would load up an instance of modContext (or reference the calling instance) and use modContext’s new _generateContext() method and control what to do depending on caching being on or off. It’s actually just a subtle change, moving a few lines of code from modContext to modCacheManager, but I feel it’s an important distinction to get right - which class it responsible for handling what.

My proof-of-concept has been committed to the /0.9.7-cache branch as r.3443, and I have started a Crucible review http://svn.modxcms.com/crucible/cru/CR-MODX-1 for those who want make comments in the context of the code.

Please note that r.3444 fixes a small bug in r.3443, and that even with this proof of concept "fix", you should not run Site > Clear Cache yet because other parts of the system, especially in the manager, are still hard-coded to rely on caching of other files. I have found it easiest to simulate turning cachine on/off by
* changing the cache settings flag in the database
* changing the cache settings flag in /core/cache/config.cache.php
* manually deleting /core/cache/web/context.cache.php
* manually deleting /core/cache/mgr/context.cache.php

Comments and all feedback are appreciated.

I’m not that familiar yet with the design of the core, but it seems that if there is data that needs to be available regardless of whether the global cache is on or off, that data should reside in an independent repository object (a context object?) created whenever a new context is necessary and accessed by the various other objects -- including the cache.

It also seems a violation of oo principles to have the cacheManager generate the context, since it should really do nothing other than what its name suggests. In fact, I would think that the design would be cleaner if there were a cache object that managed itself.

Of course, in the real world, performance issues may override the ideals of oo design and it may be a little late in the game to be redesigning the core.

Bob

Actually, backwards compatibility prevented a clean and pure OO implementation at this stage. But we will get there, and in general, it is much easier to "refactor" this stuff now that we have the OO foundation (at least once you comprehend the new implementation, but that takes a little while in itself). There is a modContext object (available as $modx->context); I had just not yet gotten around to implementing solutions to allow the on-demand access to these variables without pre-loading them all, since this will break anything that directly tries to use any of those variables loaded by the context (i.e. $modx->documentMap, $modx->aliasListing, $modx->config for that matter). The ultimate problem is going to be how handle this problem and the only way I see is to force refactoring of the components that do this to use proper accessors (i.e. $modx->getResourceAlias(), $modx->getConfigValue(), or similiar) that abstract the decision of whether to grab from the enabled cache or execute logic to calculate the needed information on demand.

Your doing a good job with this netProphET. I’ll provide some feedback in the code review, but in general you are on the right path with this refactoring; exactly what needed to happen in my perspective, but there are some significant issues to work through, as I have indicated.