By default, Relation Entity Collector block prevents caching with AutCache
AuthCache debug fails with the following message:

Debug message Canceled: Cached form on the page (likely Ajax enabled). Download and configure the Cache Object API module

See note from #2171129-1: Ajax forms not working for anonymous users on cached pages

Note that Cache Object API will be required in an upcoming release, otherwise pages with ajax forms will not be cached for authenticated users.

After enabling https://drupal.org/project/cacheobject, AuthCache works, but affects default behaviour of Relation Entity Collector block

The solution is to temporary skip AuthCache backend when Entity Collector starts picking entities on page and enable the cache after the collecting operation is done.

Entity Collector plays better with AuthCache comparing to Relation Dummy Field, because there is no need to rebuild the cache by editing the entity.
nocache cookie temporary disables AutCache, but doesn't break the existing cache.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

znerol’s picture

Thanks for the report and the code.

On first sight I thought that this would be solvable using Authcache Block configured with per-user and per-page granularity. Then it turned out that the Relation Entity Collector in fact collects all entities on a page by simply implementing hook_entity_load. Authcache does not restore the whole context of a page during the Ajax callback, thus there is no guarantee that all entities on a page are collected when the Relation Entity Collector block is loaded via Ajax by the Authcache Block module.

Therefore the approach proposed by the OP is the way to go. Posting the module as a patch, such that it is easier to review.

Status: Needs review » Needs work

The last submitted patch, 1: 2270009-relation-entity-collector-block.diff, failed testing.

znerol’s picture

  1. +++ b/modules/authcache_relation/authcache_relation.info
    @@ -0,0 +1,14 @@
    +dependencies[] = cacheobject
    +dependencies[] = relation
    

    There is no need to explicitly depend on relation and cacheobject. Those lines should be removed.

  2. +++ b/modules/authcache_relation/authcache_relation.module
    @@ -0,0 +1,29 @@
    + * Provides integration with Relation Entity Collector block
    +*/
    +
    ...
    + * Extra submit handler for the clear button.
    +*/
    

    Comment is not properly aligned here.

  3. +++ b/modules/authcache_relation/authcache_relation.module
    @@ -0,0 +1,29 @@
    +  setcookie('nocache', '1',time() + (86400 * 30), '/');
    ...
    +   setcookie('nocache', '0', time() - 3600, '/'); // empty value and old timestamp
    

    There should be a space between the comma and the next parameter. Also instead of time() use REQUEST_TIME. The second setcookie call should be intended by two spaces instead of three. And finally, end-of-line comments should not be used.

  4. The test failure from #2 is due to the fact that the code uses Windows-line endings while in Drupal Unix style line endings should be used.

In general I'd prefer if this module would implement hook_authcache_cookie. That way it wouldn't be necessary to manually set the cookie path. Also the expiry time is set to the session cookie lifetime by default. Probably it would just work with the following implementation:

/**
 * Implements hook_authcache_cookie().
 */
function authcache_relation_authcache_cookie($account) {
  return array(
    'nocache' => array(
      'present' => !empty($_SESSION['relation_type']) && !empty($_SESSION['relation_entity_keys']),
      'value' => 1,
    ),
  );
}
Drupa1ish’s picture

There is no need to explicitly depend on relation and cacheobject. Those lines should be removed.

It's ok commenting dependency to relation, since dependencies[] = relation_entity_collector means also dependency to relation
On the other hand, without cacheobject, AuthCache debug fails with the following message:
"Debug message Canceled: Cached form on the page (likely Ajax enabled). Download and configure the Cache Object API module"
(notes in README.txt).

In general I'd prefer if this module would implement hook_authcache_cookie.

Using hook_authcache_cookie means dropping the form_alter approach?
I tested your proposed code and didn't worked for me.

In the first place, I used to test depending on $_SESSION['relation_type'] in hook_boot or hook_init. It was too late and required 2 page refresh in order the Entity Collector block to work properly. This is the reason for using form_alter to setup the cookie very early.

After review, can you submit the proper formatted code ? Thank you. Don't forget authored by :)

znerol’s picture

Normally in Drupal projects the submitter of a patch is responsible of forming it into a good shape, while the maintainer helps with reviews etc. If the submitter is not capable or willing to work on a patch until it is ready for inclusion, chances are that the contribution is not accepted.

I welcome contributions like yours to the project and I'm willing to include the code into Authcache. However, before this can happen the points 1 through 4 noted in the review (comment #3) should be fixed. Authcache code adheres to the Drupal coding standards and contributions should respect those too.

The reason why Authcache does not by default depend on Cache Object API is that this module requires more than just enabling it. Therefore I think it is safer to actively warn the user instead (through Authcache Debug).

Using hook_authcache_cookie means dropping the form_alter approach? I tested your proposed code and didn't worked for me.

What exactly was the problem? To diagnose it examine the following things:

  • Is the hook executed on every page request?
  • Are the correct keys checked in $_SESSION?
  • Using the network inspector / developer tools of the browser, can you see the cookies being set / removed when appropriate?

I'd like to point out that there are automated tests covering hook_authcache_cookie. They are executed whenever I push changes to the code repository, so if that hook did not work in the first place, I likely would have noticed it. Please try again.

Regarding your approach with hook_boot and hook_init: I can confirm that this will not work because cached pages are delivered from within _drupal_bootstrap_page_cache which is called before any hooks are fired.

Thank you for your great work so far!

znerol’s picture

I'd like to point out that there are automated tests covering hook_authcache_cookie. They are executed whenever I push changes to the code repository, so if that hook did not work in the first place, I likely would have noticed it. Please try again.

Ah, wait sorry. I see what's happening. The nocache cookie is in fact taken over by Authcache itself. I will follow up on that in a minute.

znerol’s picture

Ok, the nocache cookie is in fact managed by the preclusion mechanism (see API docs on hook_authcache_preclude).

So the proper way to implement that functionality is this (unlike the hook_authcache_cookie method proposed before, I actually tested that):

/**
 * Implements hook_authcache_preclude().
 */
function authcache_relation_authcache_preclude() {
  if (!empty($_SESSION['relation_type']) || !empty($_SESSION['relation_entity_keys'])) {
    return t('Relation entity collector block contains picked entities');
  }
}

Sorry for the noise. I hope you will submit a proper patch that I can include.

Drupa1ish’s picture

I was just preparing to tell you that I retested hook_authcache_cookie and still didn't worked, when you submitted hook_authcache_preclude() version.
This new approach is by far the most elegant and efficient, because it eliminates burden of maintenance for form altering entity collector module.

I agree that Authcache should not by default depend on Cache Object API , but, as stated previously, the simple presence of Entity Collector Block breaks the authcache without cacheobject enabled.

There is not much left of the original proposal, therefore I drop any claims of authored by :)
I submit the updated version, that works for me.

  • Commit 929ae6f on 7.x-2.x authored by EuroDomenii, committed by znerol:
    Issue #2270009 by EuroDomenii: Authcache integration with Relation...
znerol’s picture

Status: Needs work » Fixed

Fixed, thank you @EuroDomenii.

  • Commit f02c7e2 on 7.x-2.x by znerol:
    Follow-up on Issue #2270009 by EuroDomenii: Add feeds as a test-...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.