See http://drupal.org/node/195773#comment-855978

[workflow-current-state-name] shows the old state instead of the new state.

Proposed patch:

Disable caching in token.module:

function token_replace($original, $type = 'global', $object = NULL, $leading = '[', $trailing = ']', $options = array()) {
  $full = token_get_values($type, $object, FALSE, $options);
  return _token_replace_tokens($original, $full->tokens, $full->values, $leading, $trailing);
}

with

function token_replace($original, $type = 'global', $object = NULL, $leading = '[', $trailing = ']', $options = array()) {
  $full = token_get_values($type, $object, TRUE, $options);
  return _token_replace_tokens($original, $full->tokens, $full->values, $leading, $trailing);
}

Not at all sure of the most appropriate place where to post this issue, or even the category or status. Feel free to move, delete or re-categorize.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

Title: Disable token caching when using actions_token.patch [#195773] » Expose control of token caching to functions that call token_replace

Well, we don't want to be flushing the cache by default. Definitely not. However, I can see how certain modules need to do that and shouldn't have to work too hard to do it.

This is kind of a question for eaton, but in my opinion the real solution is to expose control of flushing the cache to functions that call token_replace

So...I propose something more like:

function token_replace($original, $type = 'global', $object = NULL, $leading = '[', $trailing = ']', $options = array()) {
  $full = token_get_values($type, $object, FALSE, $options);
  return _token_replace_tokens($original, $full->tokens, $full->values, $leading, $trailing);
}

with

function token_replace($original, $type = 'global', $object = NULL, $leading = '[', $trailing = ']', $options = array(), $flush = FALSE) {
  $full = token_get_values($type, $object, $flush, $options);
  return _token_replace_tokens($original, $full->tokens, $full->values, $leading, $trailing);
}

What do you think?

greggles’s picture

Priority: Minor » Normal

I'd say this is normal, btw.

pete-drupal’s picture

Your solution is certainly more open than my original proposal.

The only (minor) issue I have is more conceptual: caching in general is supposed to be a performance optimization which is meant to be as transparent as possible. By exposing the control of the cache, it is no longer transparent; the caller of token_replace() must now decide on whether to enable or disable the caching mechanism. Based on what criteria? How can the caller decide? I'm assuming a black-box approach where the caller doesn't know about the implementation details of tokens.
I guess it would be up to the provider of token entries to document whether or not to clear the token cache for each type of token provided. If at least one non-cacheable token is to be replaced in a string, caching should be cleared or disabled for that token by the caller.

In fact, cache-support could be a property of a token, set by the token provider (maybe with the ability for a caller to override the preset in very tricky cases). But normally speaking, the caller should not have to be aware of all this.

But again, practically speaking, I don't see an issue to go ahead with the proposed change.

greggles’s picture

I'm not sure how uncommon or inappropriate this style is - node_load exposes control over the cache to the caller - http://api.drupal.org/api/function/node_load

I agree that knowing whether or not a call to a function needs to clear the cache can depend on a lot of things. In my experience it tends to depend on which other modules are installed on a site.

Agileware’s picture

Status: Active » Needs review
FileSize
2.97 KB

I came across this issue and needed it changed so I've made a patch against the 5.x-1.11 version.

I am using the auto_nodetitle module and when creating multiple auto node titles from tokens at once you need the cache cleared or you get duplicate (and incorrect) titles.

I think it increases the usefulness of the functions and I can't see any reason why not to make this change.

Agileware’s picture

There is a patch for auto_nodetitle that relies on this patch being submitted before it will work. It is here - http://drupal.org/node/355067#comment-1292104

Agileware’s picture

Does anyone have any objections to adding this functionality?

Here is the patch re-rolled for the current D5 & D6 dev versions (both 2009-Apr-21).

mrjavum’s picture

Thanks, Agileware !!!
This patch helps me with this ussue:
http://drupal.org/node/452802

derhasi’s picture

I would not add a full flush to token, I'd rather add the opportunity to define an additional or custom token id, for example in the $options-array of function token_get_values().

So we could replace line 286 (in D6dev - within token_get_values()) with:

//either set custom ID
if ($options['token_id']) {
  $id = $options['token_id'];
}
//or get default
else {
  $id = _token_get_id($type, $object);
}
//Add suffix and prefix for token
$id = $options['token_id_prefix'].$id.$options['token_id_suffix'];

This would avoid, that a huge amount of data has to be recached, when only one single object was needed to.

derhasi’s picture

Version: 5.x-1.11 » 6.x-1.x-dev
FileSize
1.16 KB

created the patch for current D6-dev alike #9

- added $options['flush'] for firing flush in token_get_values()
- removed $option['token_id_prefix'] to avoid ID duplicates

For example I'm using that for differentiate between "loaded node" and "changed node" with suffixes "node" and "changed".

derhasi’s picture

Status: Needs review » Reviewed & tested by the community

After discussion on IRC with eaton and greggles best practice for that issue would be to use the patch in #7 by Agileware for the current DEVs (D5 & D6). So please ignore #9 & #10.

For a further view on Drupal 7's token, there will be opened an issue for further discussion on the general caching method for token, that will try to inspect the need and necessary complexity.

I also succesfully tested patch in #7 for D6.

MGN’s picture

Same patches as in #7, re-rolled against the latest devs.

It would be great if this could be committed. I've verified that this can be used to fix the problem reported in #543506: Problem with tokens when using a custom_bredcrumbs_paths. That issue is postponed until this issue is resolved.

Please let me know if there is anything else I can do to help this along. Thanks.

MGN’s picture

Any feedback on how to proceed with this?

hadsie’s picture

the patch in #12 is working for me as well.

Agileware’s picture

We just have to wait for one of the module maintainers to commit it I guess.

Hopefully someone will get time to do it soon as I have other issues that depend on this patch.

greggles’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all - now committed to 6.x http://drupal.org/cvs?commit=268510 and 5.x http://drupal.org/cvs?commit=268514

This problem no longer exists in the 7.x version of the module so I think we're as done as we can be with this issue.

Agileware’s picture

Cool thanks

MGN’s picture

Thanks greggles!

Status: Fixed » Closed (fixed)

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