freelinks don't redirect properly when content is cached

Hi,

this bug only appears if the content type created via freelinking ("freelinking_nodetitle_searchcontenttype") is restricted to a particular type.

The attached patch should fix this problem.

thanks for your time,
Lars

Patch added to -dev. Thanks.

As to the second problem, this is an issue with freelinking3 and Drupal's caching. Drupal is feeding you the cached copy of that node, which includes the link to create the content. Clearing the cache will cause FL to look through it again and should show you the right link. This is not ideal, of course. Previous versions of FL used the freelinking/ urispace and therefore didn't store create links in with the node; instead, the module figured that out at click-time. I'd like to find a way to have that sort of behavior without having to resort to a freelinking/blah uri. And that's why this is alpha-quality code. Suggestions welcome.

(changing the title to better describe the issue)

The patch that was added to -dev is now part of alpha2.

It's not an immediate solution, but if you can leverage #457776: Freelinking Does not Work with Views Backlinks, you could get a list of nodes whose caches to clear when you save the new node.

Unfortunately, that would only be helpful to sites whose caches clear more slowly than the search indexer can run.

I've faked together a patch which uses hook_nodeapi to get around the second problem. Works for me. It shouldn't be too hard on performance, as it adds an admin menu to select which content type to clear the filter cache for, and then does a string comparison on the body, and only clears the filter cache if there is a link (or links) that need to be checked. Hope this can help with a way forward. The only caveat is that if a user selects an alternate input filter, that will revert when the cache is cleared.

First off, let me say my above comments are incorrect. I don't know what I was thinking.

@#6 So the patch resets the cache on every load (or at least view) of a node that has a "dead" freelink and is of a given content type? That seems a little unfocused. In a wiki scenario, well written pages can be both long and riddled with "wanted pages". I'd rather not see them penalized by being on a persistent no-caching list by virtue of the author's helpful syntax choices.

I'm still trying to determine the philosophy in Freelinking 3 on database tables. If one were acceptable I see a solid +1up.

When the nodetitle plugin attempts to identify a node for linking and fails, it updates a two-column table with the nid of the node being processed and the node title it was attempting to link to. Whenever a node is saved, it's title is checked in that table, and if found, the resulting node id's can be targeted for reset. This limits the impact to the nodes that are directly involved, and puts the performance burden on the user who saves content rather than the one who views it, which I find a better bargain.

EDIT: Okay, no databases. Option #2.
A separate module that watches links crafted for the Prepopulate module, when it sees one, it checks the title link for existence. If it does exist, it reroutes the page to the existing page, and attempts to reset the referer. That seems just as kludgy as anything.

@#7: I have no particular attachment to the method I've offered, other than "it works". :) Just to quantify the performance hit a small amount, I did some timing tests. (on a single processor 3.0GHz xeon, otherwise unloaded) The text scan that runs on every node of the selected content type adds a consistent hit of 1.5-2.5 ms. The input filter cache rebuild that runs only on nodes with "dead" links scales with the number of dead links - a very rough rule of thumb would be to take the number of dead links in the node, divide by 2, and add 5-10ms. I was seeing times of 15-20 ms with 25 dead links, and 33-37 ms with 50 dead links. (5-8 ms for one dead link)

If a solution similar to your option #2 was chosen, perhaps an additional GET parameter with the nid of the referring node could be added, to give the code a chance to clear its cache. best of both worlds, perhaps?

I'm attaching a slightly improved version of the patch in #6.

My concern is less for the runtime of the patch on every page load than the continuous cache-clearing of any page with a "dead" link. Effectively, it becomes an uncached page with the extra overhead of caching then clearing the page. The timing test to note is how badly the lack of a cache really affects a complex node.

(Quick search later, it appears that there is no existing functionality to turn off caching for a specific node. Cache Disable being the closest thing to that functionality I can find.)

Caching can always be set for a shorter time period. This is not the only thing that fails to appear promptly within the lifespan of the cache.

@Grayside: my understanding of the drupal caching system is far from complete, but I believe you're talking about the page cache as opposed to the filter cache, which is what my patch is dealing with. check_markup() is the function in question, and it only allows disabling the filter cache per input format. (filter_format_allowcache()) Therefore, it's not possible to disable input filter caching per node, at least according to my understanding. This leaves us with three less than perfect options:

a) entirely disabling input filter caching for freelinking-enabled content types, and taking the performance hit on all enabled nodes.
b) deleting (and therefore re-generating) the cache on each page view of a node with "dead" links
c) being ok with waiting 24 hours (the hard-coded input filter cache expiration period) for "dead" links to be updated. (which I personally am not)

I hope this helps cast some light on the situation - thanks for your work on this module.

addendum: perhaps this is entirely the wrong place to deal with the issue, and the change should be made to the prepopulate module to check for an existing node with the given parameters, and redirect to it if it exists. If that functionality existed, it wouldn't matter that it takes 24 hrs for the "dead" link to point to the right place...

I see, I had assumed the filter cache was controlled by the "minimum cache lifetime" configured under settings/performance.

Having Prepopulate redirect to an existing page and reset the cache on the linking page makes a lot of sense. In this case, you are waiting for the "first click" to fix things. Another possibility is for Prepopulate to run a database-driven check in hook_nodeapi, as the current philosophy in Freelinking 3 is "no database". Hopefully eafarris will further elaborate on that decision.

In the meantime, I am postponing this until we find out if Prepopulate might provide a cleaner fix.

I've created a feature request with a patch for prepopulate here: http://drupal.org/node/615924

this is a "must have" for a consistent behavior (what user expect)
but AFAIK it can't be done without DB,
therefore I'm opening #637842: FL3 DB Functionality Through Separate Project

actually there are other choices I dismissed
but, lets put them here so other can express their opinions

1- DB back-end
2- rendering non-existing targets to point to a proxied path where choice will be made (like old /freelink sub-path)
3- making filters provided by freelinking non-cacheables

Also note that tracking existent/non-existent targets involves more than /node/add being preserved after the content has been created, there is also the reverse problem, where the target existed but was deleted and therefore that link should now point to /node/add instead

1 => DB would be my choice
2 => is an awful partial solution (since it won't fix the inverse problem)
3 => renounce to cache?!

I had started writing a post comparing #1 and #3 a week ago, funny you should post it.

I do not like the idea of blocking filter caching for Freelinking as a whole, and it seems unnecessary. Create link functionality is now contained withing the Freelinking Prepopulate module, thus either the module (or the createnode plugin it provides) could specify that the Cache be turned off. freelinking.module's hook_filter() implementation could check for that, and determine caching as a result.

The sites most likely to want createnode as a missing-node failover are the same sites likely to have many Freelinks in the text. As such, if we attempt the cache-denial approach, the performance loss still seems bad. It might be worth considering these two as options- if you do not enable #637842: FL3 DB Functionality Through Separate Project, the createnode plugin offers an option to turn off caching.

@#16 that might be the best for FL3:
- without a DB-driven sub-mod filter won't be cached
- having a DB-driven sub-mod in brings a lot of extra features, being the most important the ability to bring cache back

did I understand your idea?

A couple more steps in there. The DB option is the best, set it aside. The alternative is as follows:
Starting with what eafarris put together, plugins may have a 'run on view' => TRUE option. This distinction is important, as currently the only plugin this is really important for is createnode, in a separate sub-module.

In the hook_filter() op "no cache", all plugins are loaded, if 'run on view' is true for any of them, it will disable the cache.

I am thinking it might be better if this were run as a configurable option, in case there are sites that would rather not have the performance loss and just accept the 24 hour delay. Perhaps plugins with 'run on view' are automatically given a configuration option to disable the cache.

I've implemented something similar to #8, but for the Drupal 7 version. I've added an option to pass the calling Nid in the url. Once the new node is created, the passed Nid will be used to clear the cache_field. This is because in D7, fields are not using cache_filter.

You can find it in my sandbox here: http://drupal.org/sandbox/baldwinlouie/1761294

Any chance of working on getting this merged back in?

I am unable to reproduce. Can anyone confirm that this bug is still present i 6.x-3.4.

I am unable to reproduce this with the latest version.
Feel free to reopen if you still think this is an issue.