I'm working on solution when many sites are linked with each other via tags like

<link rel="alternate" hreflang="en-uk" href="http://example.uk" />
<link rel="alternate" hreflang="ru-ru" href="http://example.ru" />

The data for this tag is get from external storage periodically.
The issue is that metatg module saves everything into permanent cache.

Attached patch introduces one more hook which allows custom modules to modify metatag cache expiration time. Patch will not affect existing installations.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dmitry.kazberovich created an issue. See original summary.

dmitry.kazberovich’s picture

dmitry.kazberovich’s picture

Title: Allow custom modules to modify metatag cache expire time » Allow custom modules to modify metatag cache expiration time
awm’s picture

maybe you can add your tags in hook_page_build

Also see https://www.drupal.org/node/2062379
which fixed the use of permenant cache.

e2thex’s picture

Status: Active » Reviewed & tested by the community

I have tested that this patch allows other modules to change the expiration but does not change the default behavior.

(Note: I am working on the same project as Dmitry, and doing the code review. JnJ has multiple vendors work and reviewing code.)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: metatag_cache_expiration_time-2832427-1.patch, failed testing.

e2thex’s picture

awm,
I did not see anything in that thread that talk about hook_page_build.

awm’s picture

The issue in https://www.drupal.org/node/2062379
Fixes the issue of metatags being set permenant cache.

As op has stated, his issue is

The issue is that metatg module saves everything into permanent cache.

and the patch in that thread fixes this behavior.

From the maintainer of metatag:

@DamienMcKenna: Yeah for anything with output:* we should be not using the permanent cache and setting an expiration. That's not the intended use of the permanent cache.

e2thex’s picture

Update patch to be against latest dev version (I think it is just location that is changed)
I will wait for it to pass before moving to reviewed again.

e2thex’s picture

@awm,

I am not seeing anything in that thread that says the correct way to say you do not want something to cache permanently. Can you point out the comment? Is Dave read saying that we should not cache permanently but I think that is exactly what the module does.

e2thex’s picture

@awm it is worth noting that in https://www.drupal.org/node/2062379#comment-8340685 it is stated the the reason for making a single metatag cache clear function is so that in the future functionality could be added. and that is what we are doing here.

awm’s picture

@e2thex fair enough. I was trying to point out that metatags for an entity with proper cids (i.e. output:..) gets flushed on entity save when the metatag_metatags_cache_clear($entity_type, $entity_id); is called. this function clear cache entries with CACHE_PERMENANT expire flag by passing the cid to cahce_clear_all

/**
 * Clear the cached records for a given entity type or entity ID.
 *
 * @param string $entity_type
 *   The entity type to clear.
 */
function metatag_metatags_cache_clear($entity_type, $entity_ids = NULL) {
  if (empty($entity_ids)) {
    cache_clear_all("output:$entity_type", 'cache_metatag', TRUE);
  }
  else {
    if (!is_array($entity_ids)) {
      $entity_ids = array($entity_ids);
    }
    foreach ($entity_ids as $entity_id) {
      cache_clear_all("output:$entity_type:$entity_id", 'cache_metatag', TRUE);
    }
  }
}
DamienMcKenna’s picture

Version: 7.x-1.17 » 7.x-1.x-dev

Thanks for the updated patch. Don't forget to update the issue status to "needs review" when you upload a patch.

Regardless of what testbot says, the new patch needs some *minor* tweaks - there needs to be an extra space on each of the @param variable definitions so they are underneath the "a" instead of the "p", and the place where the hook is triggered needs to have a comment that says "Triggers hook_metatag_cache_set_expire_alter()."

DamienMcKenna’s picture

Status: Needs work » Needs review
e2thex’s picture

@DamienMcKenna,
Thanks for the response I made the tweaks of which you spoke.

e2thex’s picture

DamienMcKenna’s picture

e2thex’s picture

Thank you @DamienMcKenna

DamienMcKenna’s picture

Status: Needs review » Fixed

Committed. Thanks everyone!

Status: Fixed » Closed (fixed)

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