I'm stoked to the work done here. There has been great work done to support caching blocks in various scenarios; context and panels. This post proof-of-concept is ready for some enhancements. I think with a few more features Drupal we can refine the caching to the point it becomes the standard of implementation. Feature requested - region handling. I have an almost completly functional fork of the module that supports regions. I'm looking how to do a quick and dirty patch. I've had to edit several files, but have largely kept my code separate.

Features:
* Extension of admin interface with a list of regions and default caching choices similar to blocks.
* Integration with context. Panels not yet supported as I don't have to support that scenarion
* Some custom handling. We use some blocks that are super blocks of sort - blocks w/in blocks. There are a few other modules that do this and easy to extend support for variable rendering if desired.
* AJAX works, Varnish initial test was good for unauth'd traffinc, but I think there is some VCL tweaking for auth'd to be fully functional.
* Support for nested ESI - Rendered regions check their blocks for . Scenario - Cache $right region but have UberCart's block to be cached for no caching.

Desired features - might open different issue for these

* ESI Implementation in batch / queue. All ESI tags that have identical TTL can be gathered and handled with a single. Say you want the header and footer for one TTL and the right rail for another, along with a block from a separate region. I want to group the requests by TTL in a sort of queue to reduce the number of requests.
* GUI interface of statistic data. Don't know quite what this would do...

I'll have some code to post soon. Anybody interested in helping out to refactor?

CommentFileSizeAuthor
#1 esi-added_region_support_for_esi-1907054-1.patch25.05 KBstevenmhouse
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stevenmhouse’s picture

stevenmhouse CreditAttribution: stevenmhouse commented
FileSize
esi-added_region_support_for_esi-1907054-1.patch25.05 KB

Here'a a patch that should allow for regions to be served via ESI. The configuration URI is the same, with added region area. - [drupal-root]/admin/settings/esi

This is my first-ever patch and was created quick-and-dirty style to provide code to those who could use it and hopefully for those that can help extend it. Please ask any questions you have. I have documented my code well and it should be pretty easy to run through.

Issues:

  • If any of the rendered data has document.write (used for some google ads), I have stripped out the surrounding
    tag but left the tag> - this needs AJAX differentiation and should remove the and tag while leaving in the content beneath it. Another way to handle this would be to indeed output the advertisement at load time, then figure out how to put back in right space - perhaps having a placeholder there.
  • I had to rely on preprocessing and there is wasted time where other preprocessing is happening that we could get rid of. If using ESI on regions, look at the preprocessing functions and remove / rearrange those so that it's not necessary to invoke the f(n)s that will be overwritten by ESI
Features I'd like to add:
  • Separate page for configuring regions?
  • Conditional debugging for devs - maybe a wrapper of drupal_set_message with a check for debug mode - esi_debug($msg) { if (ESI_DEBUG_MODE) { drupal_set_message($msg);}
  • An admin ESI page that shows all blocks and regions that are currently setup to use ESI
  • A way to bundle esi request into one call... esi:src='esi/multi/[TTL]/region:right/block:block_id'
  • A GUI or layout manager for above item
    • Any constructive criticism would be much appreciated, especially with patches!
mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Get this, it helps with reviewing patches
http://drupal.org/project/dreditor
(greasemonkey script)

First thing this shows is that you have some trailing white space issues with the patch.

Have you looked at the Ajaxify Regions module? You might be able to steal some ideas from it.

I see you've done some changes of the code with cookies and the usage of the url function. The usage of url came from this issue #1512818: AJAX fallback URI does not take language into consideration (via git blame). Cookies: #1491782: ESI roles/user cookies do not use session.cookie_domain. If you didn't make these changes, its important to start with the latest dev from SVN when modifying code.

I'm open to the idea of less backend requests for ESI. Patch needs more work but you got the right idea :)

stevenmhouse’s picture

stevenmhouse CreditAttribution: stevenmhouse commented

Thanks for the quick response. I will take all of your advice. I wish I would have checked out Ajaxify Regions before... but perhaps something can be learned by comparing my solution with theirs. It looks like there have been about 20 commits to that module, and none for a few years, but I understand that it is for D6 which has much different support for regions than D7.

Your assumption about the beginning code base was correct. When creating the patch I did indeed get (git) the current dev module, but then I just copied over my code from our existing module which had been changed.

I will get it together and submit another patch. Should I submit a monster patch like this if it is adressing a single issue - or should I do smaller patches with a different methodology? Thanks again for the response!

EDIT: There's also the possiblity of being more modular.... utilize a plugin architecture with minimal changes needed to the base module. Learning about Varnish and ESI in the last month gave me some cool ideas that might be 'fresh'.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Smaller patches make it easier to review. One thing you can do is create a sandbox that is a fork of esi. This allows for us to review the changes one commit at a time and we can then create a big patch once it's ready to be integrated.

stevenmhouse’s picture

stevenmhouse CreditAttribution: stevenmhouse commented

Roger. I will do that, good for experience. I have been using Drupal for so long now, I need to get up to speed about giving back to the community. I refactored the code yesterday to OO, with proper functional decomposition, and code is a lot easier to understand now, as well as to port to D7. I'm pretty stoked about D8 using parts of Symfony2. There is some pretty cool ESI support there! Thanks again for your input / guidance!

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Here is my old ESI sandbox
http://drupal.org/sandbox/mikeytown2/1328648
If you want we can update this sandbox to the latest esi version and then you can start to apply the changes

stevenmhouse’s picture

stevenmhouse CreditAttribution: stevenmhouse commented

Sounds grand! I'd appreciate that.

On a side note I have a (hopefully) very easy request: http://drupal.org/node/1909858

Thanks!

dstuart’s picture

dstuart CreditAttribution: dstuart at Axis12 Limited commented
Status: Needs review » Closed (outdated)

No longer supported