Another Bug-fix and some tests

Man, but this needs more tests ... !

app/models/page_set.rb

@@ -67,8 +67,8 @@ class PageSet < Array
       if never_orphans.include? page.name
         false
       else
-        references = WikiReference.pages_that_reference(@web, page.name) +
+        references = (WikiReference.pages_that_reference(@web, page.name) +
-                     WikiReference.pages_redirected_to(@web, page.name)
+                      WikiReference.pages_redirected_to(@web, page.name)).uniq
         references.empty? or references == [page.name]
       end
     }

app/models/wiki_reference.rb

@@ -63,9 +63,9 @@ class WikiReference < ActiveRecord::Base
     redirected_pages = []
     page = web.page(page_name)
     redirected_pages.concat page.redirects
-    redirected_pages.concat Thread.current[:page_redirects][page.name] if
+    redirected_pages.concat Thread.current[:page_redirects][page] if
-            Thread.current[:page_redirects] && Thread.current[:page_redirects][page.name]
+            Thread.current[:page_redirects] && Thread.current[:page_redirects][page]
-    redirected_pages.uniq.each { |name| names = names + self.pages_that_reference(web, name) }
+    redirected_pages.uniq.each { |name| names.concat self.pages_that_reference(web, name) }
     names.uniq    
   end
 

lib/page_renderer.rb

@@ -182,8 +182,8 @@ class PageRenderer
     
     # ugly hack: store these in a thread-local variable, so that the cache-sweeper has access to it.
     Thread.current[:page_redirects] ?
-      Thread.current[:page_redirects].update({ @revision.page.name => redirects}) :
+      Thread.current[:page_redirects].update({ @revision.page => redirects}) :
-      Thread.current[:page_redirects] = { @revision.page.name => redirects}
+      Thread.current[:page_redirects] = { @revision.page => redirects}
     
     categories = rendering_result.find_chunks(Category).map { |cat| cat.list }.flatten
     categories.each do |category|

lib/url_generator.rb

@@ -18,7 +18,7 @@ class AbstractUrlGenerator
       page_exists = web.has_page?(asked_name)
       known_page = page_exists || web.has_redirect_for?(asked_name)
       if known_page && !page_exists
-        name = web.page_that_redirects_for(asked_name)
+        name = web.page_that_redirects_for(asked_name).name
       else
         name = asked_name
       end

test/fixtures/pages.yml

@@ -52,4 +52,12 @@ elephant:
   created_at: <%= 10.minutes.ago.to_formatted_s(:db) %>
   updated_at: <%= 10.minutes.ago.to_formatted_s(:db) %>
   web_id: 1
-  name: Elephant
+  name: Elephant
+
+liquor:
+  id: 9
+  created_at: <%= 40.minutes.ago.to_formatted_s(:db) %>
+  updated_at: <%= 40.minutes.ago.to_formatted_s(:db) %>
+  web_id: 1
+  name: liquor
+  

test/fixtures/revisions.yml

@@ -81,3 +81,13 @@ elephant_first_revision:
   content: "All about elephants.\ncategory: animals"
   author: Guest
   ip: 127.0.0.2
+
+liquor_first_revision:
+  id: 10
+  created_at: <%= 40.minutes.ago.to_formatted_s(:db) %>
+  updated_at: <%= 40.minutes.ago.to_formatted_s(:db) %>
+  revised_at: <%= 40.minutes.ago.to_formatted_s(:db) %>
+  page_id: 9
+  content: "[[!redirects booze]]\n Drinky-poo."
+  author: Guest
+  ip: 127.0.0.2

test/fixtures/wiki_references.yml

@@ -109,4 +109,11 @@ elephant_1:
   link_type: C
   created_at: <%= Time.now.to_formatted_s(:db) %>
   updated_at: <%= Time.now.to_formatted_s(:db) %>
-  
+
+liquor_1:
+  id: 15
+  page_id: 9
+  referenced_name: booze
+  link_type: R
+  created_at: <%= Time.now.to_formatted_s(:db) %>
+  updated_at: <%= Time.now.to_formatted_s(:db) %>

test/functional/admin_controller_test.rb

@@ -21,6 +21,7 @@ class AdminControllerTest < ActionController::TestCase
     @request.session.dbman = FakeSessionDbMan
     @wiki = Wiki.new
     @oak = pages(:oak)
+    @liquor = pages(:liquor)
     @elephant = pages(:elephant)
     @web = webs(:test_wiki)
     @home = @page = pages(:home_page)
@@ -211,12 +212,13 @@ class AdminControllerTest < ActionController::TestCase
 
   def test_remove_orphaned_pages
     @wiki.system.update_attribute(:password, 'pswd')
-    page_order = [@home, pages(:my_way), @oak, pages(:smart_engine), pages(:that_way)]
+    page_order = [@home, pages(:my_way), @oak, pages(:smart_engine), pages(:that_way), @liquor]
-    orphan_page_linking_to_oak = @wiki.write_page('wiki1', 'Pine',
+    test_renderer(@web.page('liquor').revisions.last).display_content(true)
-        "Refers to [[Oak]].\n" +
+    orphan_page_linking_to_oak_and_redirecting_to_liquor = @wiki.write_page('wiki1', 'Pine',
+        "Refers to [[Oak]] and to [[booze]].\n" +
         "category: trees", 
         Time.now, Author.new('TreeHugger', '127.0.0.2'), test_renderer)
-
+    
     r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password_orphaned' => 'pswd')
 
     assert_redirected_to :controller => 'wiki', :web => 'wiki1', :action => 'list'
@@ -229,6 +231,7 @@ class AdminControllerTest < ActionController::TestCase
     assert_redirected_to :controller => 'wiki', :web => 'wiki1', :action => 'list'
     @web.pages(true)
     page_order.delete(@oak)
+    page_order.delete(@liquor)
     assert_equal page_order, @web.select.sort,
         "Pages are not as expected: #{@web.select.sort.map {|p| p.name}.inspect}"
 
@@ -242,7 +245,8 @@ class AdminControllerTest < ActionController::TestCase
 
   def test_remove_orphaned_pages_in_category
     @wiki.system.update_attribute(:password, 'pswd')
-    page_order = [pages(:elephant), pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), @oak, pages(:smart_engine), pages(:that_way)]
+    page_order = [pages(:elephant), pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word),
+       @oak, pages(:smart_engine), pages(:that_way), @liquor]
     orphan_page_linking_to_oak = @wiki.write_page('wiki1', 'Pine',
         "Refers to [[Oak]].\n" +
         "category: trees", 

test/functional/wiki_controller_test.rb

@@ -29,6 +29,7 @@ class WikiControllerTest < ActionController::TestCase
     @web = webs(:test_wiki)
     @home = @page = pages(:home_page)
     @oak = pages(:oak)
+    @liquor = pages(:liquor)
     @elephant = pages(:elephant)
     @eternity = Regexp.new('author=.*; path=/; expires=' + Time.utc(2030).strftime("%a, %d-%b-%Y %H:%M:%S GMT"))
     set_tex_header
@@ -130,7 +131,7 @@ class WikiControllerTest < ActionController::TestCase
     begin 
       File.open(@tempfile_path, 'wb') { |f| f.write(r.body); @exported_file = f.path }
       Zip::ZipFile.open(@exported_file) do |zip| 
-        assert_equal %w(Elephant.xhtml FirstPage.xhtml HomePage.xhtml MyWay.xhtml NoWikiWord.xhtml Oak.xhtml SmartEngine.xhtml ThatWay.xhtml index.xhtml), zip.dir.entries('.').sort
+        assert_equal %w(Elephant.xhtml FirstPage.xhtml HomePage.xhtml MyWay.xhtml NoWikiWord.xhtml Oak.xhtml SmartEngine.xhtml ThatWay.xhtml index.xhtml liquor.xhtml), zip.dir.entries('.').sort
         assert_match /.*<html .*All about elephants.*<\/html>/, 
             zip.file.read('Elephant.xhtml').gsub(/\s+/, ' ')
         assert_match /.*<html .*All about oak.*<\/html>/, 
@@ -161,7 +162,7 @@ class WikiControllerTest < ActionController::TestCase
     begin 
       File.open(@tempfile_path, 'wb') { |f| f.write(r.body); @exported_file = f.path }
       Zip::ZipFile.open(@exported_file) do |zip| 
-        assert_equal %w(Elephant.html FirstPage.html HomePage.html MyWay.html NoWikiWord.html Oak.html SmartEngine.html ThatWay.html index.html), zip.dir.entries('.').sort
+        assert_equal %w(Elephant.html FirstPage.html HomePage.html MyWay.html NoWikiWord.html Oak.html SmartEngine.html ThatWay.html index.html liquor.html), zip.dir.entries('.').sort
         assert_match /.*<html .*All about elephants.*<\/html>/, 
             zip.file.read('Elephant.html').gsub(/\s+/, ' ')
         assert_match /.*<html .*All about oak.*<\/html>/, 
@@ -260,7 +261,7 @@ class WikiControllerTest < ActionController::TestCase
     assert_equal ['animals', 'trees'], r.template_objects['categories']
     assert_nil r.template_objects['category']
     assert_equal [@elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), 
-                  @oak, pages(:smart_engine), pages(:that_way)], 
+                  @oak, pages(:smart_engine), pages(:that_way), @liquor], 
                  r.template_objects['pages_in_category']
   end
 
@@ -356,7 +357,7 @@ class WikiControllerTest < ActionController::TestCase
     assert_equal %w(animals trees), r.template_objects['categories']
     assert_nil r.template_objects['category']
     all_pages = @elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), 
-                @oak, pages(:smart_engine), pages(:that_way)
+                @oak, pages(:smart_engine), pages(:that_way), @liquor
     assert_equal all_pages, r.template_objects['pages_in_category']
     
     pages_by_day = r.template_objects['pages_by_day']
@@ -383,7 +384,7 @@ class WikiControllerTest < ActionController::TestCase
     assert_equal %w(animals categorized trees), r.template_objects['categories']
     # no category is specified in params
     assert_nil r.template_objects['category']
-    assert_equal [@elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), @oak, page2, pages(:smart_engine), pages(:that_way)], r.template_objects['pages_in_category'],
+    assert_equal [@elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), @oak, page2, pages(:smart_engine), pages(:that_way), @liquor], r.template_objects['pages_in_category'],
         "Pages are not as expected: " +
         r.template_objects['pages_in_category'].map {|p| p.name}.inspect
     assert_equal 'the web', r.template_objects['set_name']
@@ -396,7 +397,7 @@ class WikiControllerTest < ActionController::TestCase
     assert_equal ['animals', 'trees'], r.template_objects['categories']
     # no category is specified in params
     assert_nil r.template_objects['category']
-    assert_equal [@elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), @oak, pages(:smart_engine), pages(:that_way)], r.template_objects['pages_in_category'], 
+    assert_equal [@elephant, pages(:first_page), @home, pages(:my_way), pages(:no_wiki_word), @oak, pages(:smart_engine), pages(:that_way), @liquor], r.template_objects['pages_in_category'], 
         "Pages are not as expected: " +
         r.template_objects['pages_in_category'].map {|p| p.name}.inspect
     assert_equal 'the web', r.template_objects['set_name']
@@ -438,7 +439,8 @@ class WikiControllerTest < ActionController::TestCase
 
     assert_response(:success)
     pages = r.template_objects['pages_by_revision']
-    assert_equal [@elephant, @oak, pages(:no_wiki_word), pages(:that_way), pages(:smart_engine), pages(:my_way), pages(:first_page), @home], pages,
+    assert_equal [@elephant, @liquor, @oak, pages(:no_wiki_word), pages(:that_way), pages(:smart_engine),
+          pages(:my_way), pages(:first_page), @home], pages,
         "Pages are not as expected: #{pages.map {|p| p.name}.inspect}"
     assert !r.template_objects['hide_description']
   end
@@ -464,7 +466,7 @@ class WikiControllerTest < ActionController::TestCase
 
     assert_response(:success)
     pages = r.template_objects['pages_by_revision']
-    assert_equal [@elephant, @title_with_spaces, @oak, pages(:no_wiki_word), pages(:that_way), pages(:smart_engine), pages(:my_way), pages(:first_page), @home], pages, "Pages are not as expected: #{pages.map {|p| p.name}.inspect}"
+    assert_equal [@elephant, @liquor, @title_with_spaces, @oak, pages(:no_wiki_word), pages(:that_way), pages(:smart_engine), pages(:my_way), pages(:first_page), @home], pages, "Pages are not as expected: #{pages.map {|p| p.name}.inspect}"
     assert r.template_objects['hide_description']
     
     xml = REXML::Document.new(r.body)
@@ -703,6 +705,22 @@ class WikiControllerTest < ActionController::TestCase
     assert !@home.locked?(Time.now), 'HomePage should be unlocked if an edit was unsuccessful'
   end
 
+  def test_save_new_revision_identical_to_last_but_new_name
+    revisions_before = @liquor.revisions.size
+    @liquor.lock(Time.now, 'AnAuthor')
+  
+    r = process 'save', {'web' => 'wiki1', 'id' => 'liquor', 
+        'content' => @liquor.revisions.last.content.dup, 'new_name' => 'booze',
+        'author' => 'SomeOtherAuthor'}, {:return_to => '/wiki1/show/booze'}
+
+    assert_redirected_to :action => 'show', :web => 'wiki1', :id => 'booze'
+
+    revisions_after = @liquor.revisions.size
+    assert_equal revisions_before + 1, revisions_after
+    @booze = Page.find(@liquor.id)
+    assert !@booze.locked?(Time.now), 'booze should be unlocked if an edit was unsuccessful'
+  end
+
   def test_save_blank_author
     r = process 'save', 'web' => 'wiki1', 'id' => 'NewPage', 'content' => 'Contents of a new page', 
       'author' => ''
@@ -716,6 +734,18 @@ class WikiControllerTest < ActionController::TestCase
     assert_equal 'AnonymousCoward', another_page.author
   end
 
+  def test_save_invalid_author_name
+    r = process 'save', 'web' => 'wiki1', 'id' => 'NewPage', 'content' => 'Contents of a new page', 
+      'author' => 'foo.bar'
+    assert_redirected_to :action => 'new', :web => 'wiki1', :id => 'NewPage'
+    assert r.flash[:error].to_s == 'Your name cannot contain a "."'
+
+    r = process 'save', 'web' => 'wiki1', 'id' => 'AnotherPage', 'content' => 'Contents of a new page', 
+      'author' => "\000"
+
+    assert_redirected_to :action => 'new', :web => 'wiki1', :id => 'AnotherPage'
+    assert r.flash[:error].to_s == "Your name was not valid utf-8"
+  end
 
   def test_search
     r = process 'search', 'web' => 'wiki1', 'query' => '\s[A-Z]ak'

test/test_helper.rb

@@ -127,7 +127,7 @@ class StubUrlGenerator < AbstractUrlGenerator
 
   def page_link(mode, name, text, web_address, known_page)
     link = CGI.escape(name)
-    case mode.to_sym
+    case mode
     when :export
       if known_page then %{<a class="existingWikiWord" href="#{link}.html">#{text}</a>}
       else %{<span class="newWikiWord">#{text}</span>} end

test/unit/page_renderer_test.rb

@@ -326,6 +326,14 @@ END_THM
         '_should we go ThatWay or ThisWay _')
   end
   
+  def test_content_with_redirected_link
+    assert_markup_parsed_as(
+        "<p>This is a redirected link: <a class='existingWikiWord' href='../show/liquor'>" +
+	    "booze</a>. This is not: <span class='newWikiWord'>hooch<a href='../show/hooch'>?</a>" +
+	    '</span></p>', 
+        'This is a redirected link: [[booze]]. This is not: [[hooch]]')
+  end
+
   def test_content_with_wikiword_in_equations
     assert_markup_parsed_as(
         "<p>should we go <a class='existingWikiWord' href='../show/ThatWay'>" +
@@ -524,7 +532,6 @@ END_THM
   end
   
   def test_revisions_diff
-    Thread.current[:page_redirects] = { @page.name => []}
     Revision.create(:page => @page, :content => 'What a blue and lovely morning', 
         :author => Author.new('DavidHeinemeierHansson'), :revised_at => Time.now)
     Revision.create(:page => @page, :content => 'What a red and lovely morning today', 
@@ -703,7 +710,7 @@ END_THM
 
   def assert_markup_parsed_as(expected_output, input)
     revision = Revision.new(:page => @page, :content => input, :author => Author.new('AnAuthor'))
-    assert_equal expected_output, test_renderer(revision).display_content, 'Rendering output not as expected'
+    assert_equal expected_output, test_renderer(revision).display_content(true), 'Rendering output not as expected'
   end
 
   def assert_match_markup_parsed_as(expected_output, input)