freelinks don't redirect properly when content is cached

kvarnelis - May 17, 2009 - 19:40
Project:freelinking
Version:6.x-3.0-alpha1
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:postponed
Description

If I create a new page which has a freelink to a non-existent node, Freelinking gives me the following.

warning: Missing argument 2 for variable_get(), called in /home/.philie/kazys/audc.org/sites/all/modules/freelinking/plugins/freelinking_nodetitle.inc on line 18 and defined in /home/.philie/kazys/audc.org/includes/bootstrap.inc on line 500.
user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"wiki' at line 1 query: SELECT nid FROM node WHERE title = "checkmate" AND type ="wiki in /home/.philie/kazys/audc.org/sites/all/modules/freelinking/plugins/freelinking_nodetitle.inc on line 18.

But that's not all. If I create node x, then add a freelink to a non-existent node y (note this will throw the above error) then create node y, when I revisit node x, the freelink on node y still links to an offer to create node y, not to node y.

FWIW, Wikitools is enabled.

#1

sumpralle - May 17, 2009 - 21:11

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

AttachmentSize
drupal_freelinking_bug.patch 812 bytes

#2

kvarnelis - May 17, 2009 - 21:27

Hi Lars,

Ok, that patch definitely solved the first bug. Awesome!

But I am still getting the second bug, e.g.

If I create node x in which is a freelink to a non-existent node y then click through to the freelink for node y and create node y, when I revisit node x, the freelink to node y still links to an offer to create node y, not to node y, as in

http://networkarchitecturelab.org/node/add/wiki?edit%5Btitle%5D=testhing

where testhing would now resolve to a proper node at http://networkarchitecturelab.org/wiki/testhing

shouldn't that change to

http://networkarchitecturelab.org/wiki/testhing

?

#3

eafarris - May 20, 2009 - 14:01

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.

#4

eafarris - August 27, 2009 - 14:48
Title:freelinks don't redirect properly, also…warning: Missing argument 2 for variable_get() » freelinks don't redirect properly when content is cached

(changing the title to better describe the issue)

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

#5

Grayside - October 23, 2009 - 18:50

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.

#6

acrollet - September 15, 2009 - 22:24
Status:active» needs review

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.

AttachmentSize
freelinking_cache_clear.patch 1.88 KB

#7

Grayside - October 24, 2009 - 07:57

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.

#8

acrollet - October 26, 2009 - 17:06

@#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.

AttachmentSize
freelinking_cache_clear_new.patch 2.13 KB

#9

Grayside - October 26, 2009 - 19:56

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.

#10

acrollet - October 26, 2009 - 21:09

@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.

#11

acrollet - October 26, 2009 - 21:13

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...

#12

Grayside - October 26, 2009 - 23:57
Status:needs review» postponed

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.

#13

acrollet - October 27, 2009 - 15:00

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

 
 

Drupal is a registered trademark of Dries Buytaert.