From 0c2bc65e7a25f6bbd8be5cd618c8a24105e749cc Mon Sep 17 00:00:00 2001 From: Jacques Distler Date: Wed, 6 Jan 2010 08:15:34 -0600 Subject: [PATCH] All I want for Christmas ... ... is to settle these encoding issues once and for all. Let's override the accessor methods, which seems to offer a simpler solution. Now with tests (for whatever that helps)... --- app/controllers/wiki_controller.rb | 1 - app/models/page.rb | 10 +++++++--- app/models/revision.rb | 4 ++++ app/models/wiki_reference.rb | 6 +++++- test/functional/wiki_controller_test.rb | 20 +++++++++++--------- test/unit/page_test.rb | 12 ++++++------ 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 07bd54e5..b4d247f2 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -389,7 +389,6 @@ class WikiController < ApplicationController def load_page @page_name = params['id'] ? params['id'].purify : nil @page = @wiki.read_page(@web_name, @page_name) if @page_name - @page.name.as_utf8 if @page end private diff --git a/app/models/page.rb b/app/models/page.rb index c2b4d1aa..28ed497a 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -4,6 +4,10 @@ class Page < ActiveRecord::Base has_many :wiki_references, :order => 'referenced_name' has_one :current_revision, :class_name => 'Revision', :order => 'id DESC' + def name + read_attribute(:name).as_utf8 + end + def revise(content, name, time, author, renderer) revisions_size = new_record? ? 0 : revisions.size if (revisions_size > 0) and content == current_revision.content and name == self.name @@ -65,7 +69,7 @@ class Page < ActiveRecord::Base end def wiki_words - wiki_references.select { |ref| ref.wiki_word? }.map { |ref| ref.referenced_name.as_utf8 } + wiki_references.select { |ref| ref.wiki_word? }.map { |ref| ref.referenced_name } end def linked_from @@ -73,7 +77,7 @@ class Page < ActiveRecord::Base end def redirects - wiki_references.select { |ref| ref.redirected_page? }.map { |ref| ref.referenced_name.as_utf8 } + wiki_references.select { |ref| ref.redirected_page? }.map { |ref| ref.referenced_name } end def included_from @@ -82,7 +86,7 @@ class Page < ActiveRecord::Base # Returns the original wiki-word name as separate words, so "MyPage" becomes "My Page". def plain_name - web.brackets_only? ? CGI.escapeHTML(name) : CGI.escapeHTML(WikiWords.separate(name)) + web.brackets_only? ? name.escapeHTML : WikiWords.separate(name).escapeHTML end LOCKING_PERIOD = 30.minutes diff --git a/app/models/revision.rb b/app/models/revision.rb index 3af2384d..c1a508b6 100644 --- a/app/models/revision.rb +++ b/app/models/revision.rb @@ -1,4 +1,8 @@ class Revision < ActiveRecord::Base belongs_to :page composed_of :author, :mapping => [ %w(author name), %w(ip ip) ] + + def content + read_attribute(:content).as_utf8 + end end diff --git a/app/models/wiki_reference.rb b/app/models/wiki_reference.rb index f94463c2..cbc7bcd8 100644 --- a/app/models/wiki_reference.rb +++ b/app/models/wiki_reference.rb @@ -12,6 +12,10 @@ class WikiReference < ActiveRecord::Base belongs_to :page validates_inclusion_of :link_type, :in => [LINKED_PAGE, WANTED_PAGE, REDIRECTED_PAGE, INCLUDED_PAGE, CATEGORY, AUTHOR, FILE, WANTED_FILE] + def referenced_name + read_attribute(:referenced_name).as_utf8 + end + def self.link_type(web, page_name) if web.has_page?(page_name) || self.page_that_redirects_for(web, page_name) LINKED_PAGE @@ -93,7 +97,7 @@ class WikiReference < ActiveRecord::Base "ON wiki_references.page_id = pages.id " + "WHERE wiki_references.link_type = '#{CATEGORY}' " + "AND pages.web_id = '#{web.id}'" - connection.select_all(query).map { |row| row['referenced_name'] } + connection.select_all(query).map { |row| row['referenced_name'].as_utf8 } end def wiki_word? diff --git a/test/functional/wiki_controller_test.rb b/test/functional/wiki_controller_test.rb index 048bba1f..94ab59d3 100644 --- a/test/functional/wiki_controller_test.rb +++ b/test/functional/wiki_controller_test.rb @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +#coding: utf-8 # Uncomment the line below to enable pdflatex tests; don't forget to comment them again # commiting to SVN @@ -442,15 +443,16 @@ class WikiControllerTest < ActionController::TestCase end def test_recently_revised_with_categorized_page - page2 = @wiki.write_page('wiki1', 'Page2', + page2 = @wiki.write_page('wiki1', "Pagé", "Page2 contents.\n" + - "category: categorized", - Time.now, Author.new('AnotherAuthor', '127.0.0.2'), x_test_renderer) + "category: categorizé", + Time.now, Author.new("André Auteur", '127.0.0.2'), x_test_renderer) r = process('recently_revised', 'web' => 'wiki1') assert_response(:success) - assert_equal %w(animals categorized trees), r.template_objects['categories'] + c = ''.respond_to?(:force_encoding) ? "categoriz\u00E9" : "categoriz\303\251" + assert_equal ['animals', c, '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), @liquor], r.template_objects['pages_in_category'], @@ -676,7 +678,7 @@ class WikiControllerTest < ActionController::TestCase end def test_save_astral_plane_characters - r = process 'save', 'web' => 'wiki1', 'id' => 'NewPage', 'content' => "Double-struck A: \xF0\x9D\x94\xB8", + r = process 'save', 'web' => 'wiki1', 'id' => 'NewPage', 'content' => "Double-struck A: \360\235\224\270", 'author' => "\xF0\x9D\x94\xB8\xC3\xBCthorOfNewPage" assert_redirected_to :web => 'wiki1', :controller => 'wiki', :action => 'show', :id => 'NewPage' @@ -686,7 +688,7 @@ class WikiControllerTest < ActionController::TestCase a = ''.respond_to?(:force_encoding) ? "\u{1D538}\u00FCthorOfNewPage" : "\360\235\224\270\303\274thorOfNewPage" assert_equal a, new_page.author - assert_equal "\360\235\224\270\303\274thorOfNewPage", r.cookies['author'] + assert_equal "\xF0\x9D\x94\xB8\xC3\xBCthorOfNewPage".as_bytes, r.cookies['author'] end def test_save_not_utf8 @@ -972,7 +974,7 @@ class WikiControllerTest < ActionController::TestCase r = process('show', 'id' => 'HomePage', 'web' => 'wiki1') assert_response :success - assert_match /Recursive include detected: HomePage \342\206\222 HomePage<\/em>/, r.body.as_bytes + assert_match /Recursive include detected: HomePage \342\206\222 HomePage<\/em>/, r.body end def test_recursive_include_II @@ -984,7 +986,7 @@ class WikiControllerTest < ActionController::TestCase r = process('show', 'id' => 'HomePage', 'web' => 'wiki1') assert_response :success - assert_match /

Recursive-include:<\/p>\n\n

extra fun Recursive include detected: Foo \342\206\222 Foo<\/em><\/p>/, r.body.as_bytes + assert_match /

Recursive-include:<\/p>\n\n

extra fun Recursive include detected: Foo \342\206\222 Foo<\/em><\/p>/, r.body end def test_recursive_include_III @@ -998,7 +1000,7 @@ class WikiControllerTest < ActionController::TestCase r = process('show', 'id' => 'HomePage', 'web' => 'wiki1') assert_response :success - assert_match /

Recursive-include:<\/p>\n\n

extra fun<\/p>\nRecursive include detected: Bar \342\206\222 Bar<\/em>/, r.body.as_bytes + assert_match /

Recursive-include:<\/p>\n\n

extra fun<\/p>\nRecursive include detected: Bar \342\206\222 Bar<\/em>/, r.body end def test_nonrecursive_include diff --git a/test/unit/page_test.rb b/test/unit/page_test.rb index 6a480001..b0b415de 100644 --- a/test/unit/page_test.rb +++ b/test/unit/page_test.rb @@ -132,19 +132,19 @@ class PageTest < ActiveSupport::TestCase references = new_page.wiki_references(true) assert_equal 2, references.size - assert_equal "H\xC3\xA1ppyPage", references[0].referenced_name + p = ''.respond_to?(:force_encoding) ? "H\u00E1ppyPage" : "H\303\241ppyPage" + assert_equal p, references[0].referenced_name assert_equal WikiReference::WANTED_PAGE, references[0].link_type assert_equal 'WantedPage2', references[1].referenced_name assert_equal WikiReference::WANTED_PAGE, references[1].link_type wanted_pages = web.select.wanted_pages - a = ''.respond_to?(:force_encoding) ? ["HisWay", "H\u00E1ppyPage", "OverThere", "WantedPage2"] : ["HisWay", "H\303\241ppyPage", "OverThere", "WantedPage2"] - assert_equal a, wanted_pages + assert_equal ["HisWay", p, "OverThere", "WantedPage2"], wanted_pages my_page = Page.new(:web => web, :name => 'MyPage') my_page.revise("[[!redirects H\xC3\xA1ppyPage]]\nAnd here it is!", 'MyPage', Time.now, 'AlexeyVerkhovsky', x_test_renderer) my_references = my_page.wiki_references(true) assert_equal 1, my_references.size - assert_equal "H\xC3\xA1ppyPage", my_references[0].referenced_name + assert_equal p, my_references[0].referenced_name assert_equal WikiReference::REDIRECTED_PAGE, my_references[0].link_type wanted_pages = web.select.wanted_pages assert_equal ["HisWay", "OverThere", "WantedPage2"], wanted_pages @@ -157,7 +157,7 @@ class PageTest < ActiveSupport::TestCase assert_match( s, x_test_renderer(new_page.revisions.last).display_content(true) ) assert_equal 2, references.size - assert_equal "H\xC3\xA1ppyPage", references[0].referenced_name + assert_equal p, references[0].referenced_name # Doesn't work, since picking up the change in wiki_references requires a database query. # assert_equal WikiReference::LINKED_PAGE, references[0].link_type assert_equal 'WantedPage2', references[1].referenced_name @@ -176,7 +176,7 @@ class PageTest < ActiveSupport::TestCase x_test_renderer(new_page.revisions.last).display_content(true) ) assert_equal 3, references.size # now it works. - assert_equal "H\xC3\xA1ppyPage", references[0].referenced_name + assert_equal p, references[0].referenced_name assert_equal WikiReference::LINKED_PAGE, references[0].link_type assert_equal 'WantedPage2', references[1].referenced_name assert_equal WikiReference::WANTED_PAGE, references[1].link_type