If you’ve found a mistake in the rake task, please send me a patch.

--- instiki-0.19.4/lib/tasks/fixtures.rake.orig Sat Jun 30 19:40:02 2012
+++ instiki-0.19.4/lib/tasks/fixtures.rake      Wed Jul 18 01:33:06 2012
@@ -74,7 +74,7 @@
     task :import_all => :environment do
       ActiveRecord::Base.establish_connection
       Dir.glob(Rails.root.join('dump','fixtures',"*.yml")).each do |f|
-        table_name = f.gsub( Regexp.escape(Rails.root.join('dump','fixtures').to_s + File::SEPARATOR), '').gsub('.yml', '')
+        table_name = f.gsub( Rails.root.join('dump','fixtures').to_s + File::Separator, '').gsub('.yml', '')
          puts "Importing #{table_name}"
         import_table_fixture(table_name)
       end

That’s what I have anyways, but be careful, because somehow I got File::SEPARATOR in the wrong case in my bumbling around. I guess I got lucky: File::Separator is defined too, so no harm done.

Hmmm. Both look wrong. What I think we want is:

table_name = f.gsub( Regexp.new(Regexp.escape(Rails.root.join('dump','fixtures').to_s + File::SEPARATOR)), '').gsub('.yml', '')

The point being that the output of Regexp.escape is a (properly-escaped) string which is suitable as input to Regexp.new.

I tried a little test case…

$ cat subtest.rb                                                                                                                                                               
#!/usr/bin/env ruby
puts '/some/other/string'.gsub( '/some/other' + File::SEPARATOR, '' )
puts '/some/other/string'.gsub( Regexp.escape( '/some/other' + File::SEPARATOR), '' )
puts '/some/other/string'.gsub( Regexp.new( Regexp.escape( '/some/other' + File::SEPARATOR ) ), '' )
$ ./subtest.rb                                                                                                                                                                 
string
string
string
$ ruby --version
ruby 1.8.7 (2012-06-29 patchlevel 370) [x86_64-openbsd]

I believe that my first and third examples are both correct and equivalent, going by official docs. The second example works, too, in this case, because a forward slash is apparently not one of the characters escaped by Regexp.escape(), which is curious, because now I am unable to reproduce the error that occurred in the rake task before I altered the code. So I’m more and more confused.

I’m unable to run the inbuilt SVG editor on my computer (running Mac OS X, Lion). The window launches but none of the icons are present and although the buttons highlight when I hover over them, nothing happens when I click on one.

This is with Firefox, Chrome, and Safari. Not sure what additional information you would like on this.

And another one, this time in how maruku parses its meta-data. It would seem that not leaving a space at the end of {: #identifier} means that the } gets into the identifier.

Presumably this is the case here as well:

• What’s the id of this element?

I get:

<ul>
<li id='list}'>What’s the <code>id</code> of this element?</li>
</ul>

in the source.

And another one, this time in how maruku parses its meta-data.

This was actually only a problem for IALs attached to <li> elements. Fixed in the latest Maruku.

(This commit gives the complete solution. Its predecessor was only a partial fix.)

(N.b.: Maruku is now unvendored, so a

ruby bundle update

will pull in the latest version from Github.)

I’m unable to run the inbuilt SVG editor on my computer (running Mac OS X, Lion).

Works fine for me under Lion. (I haven’t updated to Mountain Lion, so I can’t make any promises about that. But I’d be surprised if there were any OS dependence of this; it ought to be a function of the Javascript engine in your browser. Or perhaps I misunderstood: were you running the server on Lion, or just the client?)

I know you don’t believe in the Cache Bug …

I had a page on the nLab which I renamed. Looking at the logs, then I see the save command. It ends with:

 "alter_title"=>"1", "new_name"=>"adequate subcategory > history", "author"=>"Andrew Stacey", "web"=>"nlab", "id"=>"adequate subcategory"}

The next log items are:

24808: 2012-10-18 14:22:26 +0400: Reading page 'adequate subcategory' from web '
nlab'
24808: 2012-10-18 14:22:26 +0400: Page 'adequate subcategory'  found
24808: 2012-10-18 14:22:26 +0400: Checking DNSBL 214.7.76.192.bl.spamcop.net
24808: 2012-10-18 14:22:26 +0400: Checking DNSBL 214.7.76.192.sbl-xbl.spamhaus.org
24808: 2012-10-18 14:22:27 +0400: 192.76.7.214 added to DNSBL passed cache
24808: 2012-10-18 14:22:27 +0400: 192.76.7.214
24808: 2012-10-18 14:22:27 +0400: Reading page 'adequate subcategory' from web 'nlab'
24808: 2012-10-18 14:22:27 +0400: Page 'adequate subcategory'  found
24808: 2012-10-18 14:22:27 +0400: Maruku took 0.148016386 seconds.
24808: 2012-10-18 14:22:27 +0400: Maruku took 0.165203678 seconds.
24808: 2012-10-18 14:22:28 +0400: Expired fragment: views/nlab/show/adequate+subcategory+>+history (0.3ms)

There are then a slew of more expirations, the first ones being adequate+subcategory+>+history. The last ones are also adequate+subcategory+>+history.

I just tried on my course wiki. Here are the steps I took:

1. Edit a page and change it’s name.
2. Remove the automatically-inserted redirect.
3. Save the page.

Result: the expiration sweep does not include the old name and includes the new name twice.

I’m wondering if this could be the culprit: app/controllers/revision_sweeper.rb:

  def before_save(record)
    if record.is_a?(Revision)
      expire_cached_page(record.page.web, record.page.name) 
      expire_cached_revisions(record.page)
    end
  end

I notice that in the save post data then the name is the new name and the id is the old name. Could it be that the record object is populated with the new name before this action takes place? I’m not very good at tracing through ruby code, but if my guesses about what happens are right then the before_save action takes place just before the save action is executed in page.revise. If so, then by this time the name is the new name and the old name has been forgotten: in wiki.rb then the call is page.revise(content, new_name, revised_at, author, renderer) and very early on in the revise method then I see self.name = name.

Perhaps the revise should save its name in old_name and then the before_save can use that if it differs from the current name?

Sorry, I follow exactly the steps you outlined, and it’s adequate+subcategory (and its variants) that get cleared from the cache, not adequate+subcategory+>+history.

The scenario you outline (involving the old name being forgotten before the cache gets swept) is exactly what the before_save action is supposed to avoid. Then the cache gets swept again in an after_save action.

Now, the only thing I can think of is that I tested this under SQLite3, rather than MySQL. Perhaps the driver for the latter does something funky. But that seems unlikely…

I just tried on a completely fresh install and found that it happened just as you described: the name used was the old name (and it gets swept twice). That was a sqlite3 database.

I’ve tried to install mysql on my mac to test this there, but get to a crash when I run instiki. Not sure why, seems to be related to the gem not finding my mysql lib files but it’s taken too much of my time already to try to fix it to test further. I can try it on my linux machine later.

However, the evidence certainly suggests that there is a difference between mysql and sqlite3 on this one.

There would appear to be a bug on pages with apostrophes in their names. See http://nforum.mathforge.org/discussion/4757/apostrophes-in-page-titles-lead-to-weird-behaviour for details.

I’m sorry.

Could you please distill that long and rambling discussion in to a set of steps by which one might reproduce the bug?

In the process of migrating my wiki from Textile to Markdown I think I found a bug:

The following snippet of text: punto!). was converted by pandoc as punto![]().. The latter text blows the editing page with the error “NameError in WikiController#save”. Step to reproduce

1. create a new empty page
2. write punto![](). (without apexes)
3. save hitting the “Submit” button

Pandoc is clearly weird. But you have uncovered a regression in Maruku. I’ve fixed that bug in the latest version in my repository on Github.

  ruby bundle update

will fix the problem.

Fixed. Thanks!

Distillation for Distler (sorry, …)

1. Create a page with an apostrophe in the title, say apostrophe's.
2. Edit page.
3. Page magically becomes apostrophe's.
4. Edit page, changing its name back to apostrophe's.
5. Page name is now correct.
6. Edit page.
7. Page name is mangled again.

… and in the time since you asked for clarification, it would appear that you’ve fixed it anyway as it no longer appears having just updated instiki.

Thanks.

I am fairly certain that none of my recent updates would affect this scenario..

But I’m happy to hear that it fixed itself.

I think I’ve killed the cache bug!

I did a fresh install of Instiki+MySQL on a debian virtual machine and … no cache bug. So I went back to my live installs and … cache bug.

Poking around, I discovered that my fresh install was using version 2.9.1 of the mysql gem but the live installs were using 2.8.1. So, on a hunch, I updated to 2.9.1 and tried reproducing the cache bug … and it had gone.

I don’t know if I’ve well and truly killed it, but the steps I put above were reliably showing it for me on the nlab and on mathsnotes and now that I’ve updated the mysql gem then those steps no longer exhibit it so I figure that’s enough for a small celebration.

Poking around, I discovered that my fresh install was using version 2.9.1 of the mysql gem but the live installs were using 2.8.1.

If there’s a minimum version number for the gem that we should be using (no version number is currently specified), then that would be good to know, so that the Gemfile can be updated, accordingly.

Curious, though, that this “cache bug” of yours would seem to have nothing to do with … caching.

I put 2.9.1 in the Gemfile but I don’t know if that’s the minimum value. Interestingly, bundle won’t update to the latest version unless you specify such a minimum.

Comparing the logs, then it would appear that something in the mysql module was returning the updated page name when instiki was expecting the old page name, so when instiki swept the cache using what it thought was the old page name, it was actually using the new one and thus the old cache page wasn’t being deleted.

I put 2.9.1 in the Gemfile but I don’t know if that’s the minimum value.

Might as well assume that 2.9.1 is the minimum (since we know that it works). I updated the instructions accordingly.

Interestingly, bundle won’t update to the latest version unless you specify such a minimum.

Correct. If you don’t specify a version, then any version is supposed to suffice.

Spoke too soon. The cache bug is not dead.

I shoved in a load of logger.info statements to the revise method of page.rb and to the before_save and after_save methods of revision_sweeper.rb to see what was happening. I noticed that the cache sweep only happens when a revision is saved, not when a page is saved. At least, that’s how I interpret the if record.is_a?(Revision) conditional.

Anyway, what I found was interesting. I got different behaviour depending on whether I was creating a new revision or updating an old one (to switch between the two I used two author strings).

Here’s the “same author” sequence of events:

Processing WikiController#edit (for 127.0.0.1 at 2013-05-29 21:56:25) [GET]
Cache Bug Log: Before save page called for previous name.
Cache Bug Log: After save page called for previous name.
Processing WikiController#save (for 127.0.0.1 at 2013-05-29 21:56:32) [POST]
Cache Bug Log: Renaming 'previous name' to 'current name'.
Cache Bug Log: Updating previous revision.
Cache Bug Log: Before save revision called for previous name.
Cache Bug Log: After save revision called for previous name.
Cache Bug Log: Saving new page.
Cache Bug Log: Before save page called for current name.
Cache Bug Log: After save page called for current name.
Cache Bug Log: Before save page called for previous name.
Cache Bug Log: After save page called for previous name.
Processing WikiController#show (for 127.0.0.1 at 2013-05-29 21:56:33) [GET]

As I said above, only the Before save revision and After save revision actually seem to do any cache sweeping. So in this sequence, the previous name and only the previous name gets swept. This is okay, but not optimal: if there is a stale cache file then renaming a page to the name of a stale file means that the stale file doesn’t get swept (I checked this).

But here’s what happens when I change the author name to force a new revision:

Processing WikiController#edit (for 127.0.0.1 at 2013-05-29 21:57:34) [GET]
Cache Bug Log: Before save page called for previous name.
Cache Bug Log: After save page called for previous name.
Processing WikiController#save (for 127.0.0.1 at 2013-05-29 21:57:45) [POST]
Cache Bug Log: Renaming 'previous name' to 'current name'.
Cache Bug Log: Creating new revision.
Cache Bug Log: Saving new page.
Cache Bug Log: Before save page called for current name.
Cache Bug Log: Before save revision called for current name.
Cache Bug Log: After save revision called for current name.
Cache Bug Log: After save page called for current name.
Cache Bug Log: Before save page called for previous name.
Cache Bug Log: After save page called for previous name.
Processing WikiController#show (for 127.0.0.1 at 2013-05-29 21:57:45) [GET]

Notice that the Saving new page message now occurs before the save revision steps. The Saving new page message was inserted just before the save statement in the revise method. So this ought to happen after the new revision is created. However, it would appear that the new revision is not actually saved until the page is saved.

With this sequence then the before_save and after_save functions for the revision are called with the current name meaning that the old name does not get swept. This is “the cache bug”.

Looking at the two different sequences of events, it would seem that the safest place to sweep the cache is when before_save is called when saving a page, not a revision. Moreover, as the before_save and after_save are always called with the same page name then it seems overkill to put the sweep in both.

Of course, my analysis may well be incorrect as instiki (well, ruby-on-rails) is very much a black box to me so I have no idea as to what’s going on under the bonnet.

Incidentally, the above was carried out using sqlite as the database, but I got the same results with mysql with the updated gem.

Since I’ve not had much success reproducing your bug, why don’t we see whether implementing your suggestion fixes it for you?

Apply the following patch:

=== modified file 'app/controllers/revision_sweeper.rb'
--- app/controllers/revision_sweeper.rb 2011-09-21 04:46:36 +0000
+++ app/controllers/revision_sweeper.rb 2013-05-30 05:11:33 +0000
@@ -7,15 +7,15 @@
   observe Revision, Page
   
   def before_save(record)
-    if record.is_a?(Revision)
-      expire_cached_page(record.page.web, record.page.name) 
-      expire_cached_revisions(record.page)
+    if record.is_a?(Page)
+      expire_cached_page(record.web, record.name) 
+      expire_cached_revisions(record)
     end
   end
   
   def after_save(record)
-    if record.is_a?(Revision)
-      expire_caches(record.page)
+    if record.is_a?(Page)
+      expire_caches(record)
     end
   end
 

I’ve put those changes in place and will keep an eye on what happens.

Incidentally, when manually clearing out the cache I note that / in page names gets translated into subfolders. So a page name Sandbox/Set creates a folder Sandbox with a file Set.cache. Should the page name be completely sanitised under such circumstances?

With the above changes, then expire_caches triggers an error when renaming a page. I think it is due to expiring pages that redirect to the given page. This is called in expire_caches in after_save. If the page name has changed, then the function self.pages_redirected_to in wiki_references.rb is called with the old name but that no longer exists in the database. So the page = web.page(page_name) returns null. I’ve put in a conditional to test for this which at least removes the error but I don’t know if there are some caches that aren’t now being expired that ought to be - I’ll check the logs to trace this.

  def self.pages_redirected_to(web, page_name)
    names = []
    redirected_pages = []
    if web.has_page?(page_name)
      page = web.page(page_name)
      redirected_pages.concat page.redirects
      redirected_pages.concat Thread.current[:page_redirects][page] if
            Thread.current[:page_redirects] && Thread.current[:page_redirects][page]
    end
    redirected_pages.uniq.each { |name| names.concat self.pages_that_reference(web, name) }
    names.uniq    
  end

There’s a bug with maruku’s table handling: it doesn’t like whitespace at the end of the line (a previous version did, and it would seem to me that whitespace here should be fine). This causes tables that used to render to no longer do so.

The fix is to modify the regexp for splitting cells: line 515 of lib/maruku/input/parse_block.rb should read:

if (/^[|].*[|]\s*$/ =~ s)

(I notice that this isn’t the same as the version of maruku on github, so don’t know if this would be superseded by updating to the latest version from there.)

I’m working on updating my branch of Maruku, to incorporate various improvements from trunk. In particular, there are improvements to the table code. So you should check it out…