Jump to content

Problem with hook in modified PageRender


SteveB
 Share

Recommended Posts

I wrote a module which adds a new rendering method to Page using a hook. I wanted it to be able to do caching like the PageRender module does. Various "non-object" issues cropped up. The best I could come up with was to copy the cache related methods from PageRender into my module and fuss around with it.

It works nicely and it does cache the pages but hooks for clearing cache after page save/delete don't work. I can clear cache manually from the module's interface in admin though. I'm happy to have gotten it to this point but I'm doing something wrong in the hook/module/OOP. Tried a few things.

Comparing my module to PageRender module, the methods supporting caching are the the same (except for $this/wire issues explained below) but the renderPage() method is renamed and different. My replacement method keeps the parts about $options and recursion but the whole block of code about $output is replaced.

Clue: There is something funny about $this. It also seems wrong that I've hooked to Page but the cache support I need is in PageRender. I tried hooking PageRender's renderPage and using $event->replace = true; but I didn't get it to work. I could revisit that if anyone thinks it makes sense. I started looking to see how a Page object ends up calling PageRender methods but it's kind of a wild goose chase.

In the module's init():

$this->addHook('Page::foo', $this, 'foo');

$this->pages->addHookAfter('save', $this, 'clearCacheFile');
$this->pages->addHookAfter('delete', $this, 'clearCacheFile');
 

If I do that I get: Error: Call to a member function addHookAfter() on a non-object

If I use wire("pages") instead of $this->pages there's no error but it doesn't do anything:

$this->addHook('Page::foo', $this, 'foo');

wire("pages")->addHookAfter('save', $this, 'clearCacheFile');
wire("pages")->addHookAfter('delete', $this, 'clearCacheFile');
 

Similarly in isCacheAllowed() I had to change $this->user->isGuest() to wire("user")->isGuest().

Also $this->input->pageNum in getCacheFile().

If I try to call PageRender::isCacheAllowed($page) instead of having my own copy I just get a non-object error happening there.

The easiest thing seems to be to make a replacement for the PageRender module but that seems wrong. How do I do this properly?

Link to comment
Share on other sites

Which class does your module extend?

Because if you don't have API variables like $this->pages available, then maybe you do not extend Wire or a sublcass of Wire.

There is also a method ready() which should garantuee that all API variables are loaded. Maybe trying to add your hooks there instead of init().

If you want use functionality of the PageRender module, maybe you could extend the PageRender class itself and overwrite methods for your needs?

Link to comment
Share on other sites

It extends WireData, as do Page and PageRender. I did try (briefly) extending from PageRender. It also has issues with how $this has been defined. What happens is that my method calls $this->isCacheAllowed($page) (a method in PageRender) and then isCacheAllowed does if(!$this->user->isGuest()) and we get Call to a member function isGuest() on a non-object (line 88 of E:\dev\vhosts\pw\wire\modules\PageRender.module).

Link to comment
Share on other sites

I haven't sorted out those questions but I simplified the code. It's working well but cache is still sticky. All my non-standard stuff is done from the PW Template file. I use my module but don't take on any of the caching tasks or hook anything. I'm using template caching but not on any of the sub-renders (there's not much to be gained by that). Works nicely and is simpler but the cache still doesn't clear when the page is saved.

The purpose of the module is to provide a method which recurses out from the current page to other pages following parent/child and possibly other relationships, producing markup and saving it as properties of the pages. It also makes some data about what it found. The pages render their markup differently according to the level of recursion and other parameters passed down to them through $options. Pages involved in this scheme must load the module from their template files. Callback functions customize recursion and layout on a per template basis.

In my test, it's only looking at child pages and only one level down. With 20 child pages speed's okay, 50 feels too slow. Of course it's super fast when cached.

Link to comment
Share on other sites

If your $this->[api variable] isn't working, chances are you've overridden WireData's $this->get() or $this->__get() -- is that the case? If so, using wire('api variable'); is a fine way to go, though you may still want to fix the API variable access in whatever get() method is overridden, just to be safe. Another possibility is that you have a line that says $this->useFuel=false; That tells ProcessWire to cancel access to API variables via $this. 

As for why your hooks aren't being called, that's a mystery as I don't see an obvious problem with the code you posted. Though I would suggest hooking Pages::saved and Pages::deleted rather than Pages::save and Pages::delete. Add a line in your hook function like $this->message("hello!"); so that you can see in your admin that the hook was called. ProcessWire performs a redirect after page saves, which can make it easy to conclude that something isn't happening when it actually is. When you use $this->message("hello"); that gets queued between requests, which makes it useful for this kind of debugging.

  • Like 1
Link to comment
Share on other sites

  • 2 weeks later...

You were right about get().

The hooks work fine but the caches don't clear after editing the page because they get created with the md5 "secondary" kind of id and clearing of the cache this way uses the simple page id cache filename. In PageRender, what do you think about having clearCacheFile() use $cacheFile->removeAll() (instead of $cacheFile->remove()) if the numbered file isn't there?

		if($cacheFile->exists()) {
			$cacheFile->remove();
			if($this->config->debug) $this->message("Cleared cache file: $cacheFile"); 
		}
		else{
			$cacheFile->removeAll(dirname($cacheFile));
			if($this->config->debug) $this->message("Cache file does not exist: $cacheFile, removed all cache files for that page."); 
		}
//Also do same for $cf in the conditional loop of parent pages

FYI, using 2.3 dev, PageRender V102

Wish list: It would be nice if generation of secondary id (in PageRender) was more accessible to the programmer, so we could more easily cache different versions of a page. We might have want to use an md5 of the whole $options array or something like that.

Link to comment
Share on other sites

In PageRender, what do you think about having clearCacheFile() use $cacheFile->removeAll() (instead of $cacheFile->remove()) if the numbered file isn't there?

The $cacheFile->remove() should remove all files in the cache directory for that page. Probably what would be better is just to call $cacheFile->remove() regardless of whether the individual cache file exists -- so your suggestion is a good one (but will use remove rather than removeAll). 

Wish list: It would be nice if generation of secondary id (in PageRender) was more accessible to the programmer, so we could more easily cache different versions of a page.

Secondary ID is a combination of $options (prependFile, appendFile, filename), page number, language and perhaps some other things I'm not thinking of. I can look at opening it up to more options. MarkupCache is also another option, that lets you have full control over the cache ID. 

We might have want to use an md5 of the whole $options array or something like that.

I'll look into this option. Though the MD5 includes things outside the $options array as well, but perhaps the entire $options should be part of it. 

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...