Continue extracting URL generation logic from model classes

app/controllers/application.rb

@@ -2,8 +2,8 @@
 # Likewise will all the methods added be available for all controllers.
 class ApplicationController < ActionController::Base
 
-  before_filter :set_utf8_http_header, :connect_to_model
+  before_filter :set_utf8_http_header, :connect_to_model, :setup_url_generator
-  after_filter :remember_location
+  after_filter :remember_location, :teardown_url_generator
 
   # For injecting a different wiki model implementation. Intended for use in tests
   def self.wiki=(the_wiki)
@@ -146,6 +146,14 @@ class ApplicationController < ActionController::Base
     @response.headers['Content-Type'] = 'text/html; charset=UTF-8'
   end
 
+  def setup_url_generator
+    PageRenderer.setup_url_generator(UrlGenerator.new(self))
+  end
+
+  def teardown_url_generator
+    PageRenderer.teardown_url_generator
+  end
+
   def wiki
     self.class.wiki
   end

app/helpers/application_helper.rb

@@ -44,7 +44,12 @@ module ApplicationHelper
   # Creates a hyperlink to a Wiki page, or to a "new page" form if the page doesn't exist yet
   def link_to_page(page_name, web = @web, text = nil, options = {})
     raise 'Web not defined' if web.nil?
-    web.make_link(page_name, text, options.merge(:base_url => "#{base_url}/#{web.address}"))
+    UrlGenerator.new(@controller).make_link(page_name, web, text, 
+        options.merge(:base_url => "#{base_url}/#{web.address}"))
+  end
+
+  def author_link(page, options = {})
+    UrlGenerator.new(@controller).make_link(page.author.name, page.web, nil, options)
   end
 
   def base_url

app/models/page.rb

@@ -79,19 +79,6 @@ class Page < ActiveRecord::Base
     web.brackets_only? ? name : WikiWords.separate(name)
   end
 
-  # used to build chunk ids. 
-  #def id
-  #  @id ||= name.unpack('H*').first
-  #end
-
-  def link(options = {})
-    web.make_link(name, nil, options)
-  end
-
-  def author_link(options = {})
-    web.make_link(author, nil, options)
-  end
-
   LOCKING_PERIOD = 30.minutes
 
   def lock(time, locked_by)

app/models/web.rb

@@ -44,28 +44,6 @@ class Web < ActiveRecord::Base
     read_attribute('markup').to_sym
   end
 
-  # Create a link for the given page name and link text based
-  # on the render mode in options and whether the page exists
-  # in the this web.
-  # The links a relative, and will work only if displayed on another WikiPage.
-  # It should not be used in menus, templates and such - instead, use link_to_page helper
-  def make_link(name, text = nil, options = {})
-    text = CGI.escapeHTML(text || WikiWords.separate(name))
-    mode = options[:mode] || :show
-    base_url = options[:base_url] || '..'
-    link_type = options[:link_type] || :show
-    case link_type.to_sym
-    when :show
-      UrlGenerator.new.make_page_link(mode, name, text, base_url, has_page?(name))
-    when :file
-      UrlGenerator.new.make_file_link(mode, name, text, base_url, has_file?(name))
-    when :pic
-      UrlGenerator.new.make_pic_link(mode, name, text, base_url, has_file?(name))
-    else
-      raise "Unknown link type: #{link_type}"
-    end
-  end
-
   def remove_pages(pages_to_be_removed)
     pages_to_be_removed.each { |p| p.destroy }
   end

app/views/wiki/page.rhtml

@@ -23,7 +23,7 @@
 
 <div class="byline">
   <%= @page.revisions? ? "Revised" : "Created" %> on <%= format_date(@page.revised_at) %> 
-  by <%= @page.author_link %>
+  by <%= author_link(@page) %>
   <%= "(#{@page.author.ip})" if @page.author.respond_to?(:ip) %>
   <% if @web.count_pages? %>
     <% total_chars = @page.content.length %>

app/views/wiki/print.rhtml

@@ -10,5 +10,5 @@
 <div class="byline">
   <%= @page.revisions? ? "Revised" : "Created" %> on <%= format_date(@page.revised_at) %> 
   by
-  <%= @page.author_link({ :mode => (@link_mode || :show) }) %>
+  <%= author_link(@page, { :mode => (@link_mode || :show) }) %>
 </div>

lib/page_renderer.rb

@@ -4,8 +4,17 @@ require 'diff'
 
 class PageRenderer
 
+  def self.setup_url_generator(url_generator)
+    @@url_generator = url_generator
+  end
+
+  def self.teardown_url_generator
+    @@url_generator = nil
+  end
+
   attr_accessor :revision
 
+
   def initialize(revision = nil)
     @revision = revision
   end
@@ -53,7 +62,7 @@ class PageRenderer
   # Ensures new version works with older snapshots.
   def display_content
     unless @display_cache && @display_cache.respond_to?(:chunks_by_type)
-      @display_cache = WikiContent.new(@revision)
+      @display_cache = WikiContent.new(@revision, @@url_generator)
       @display_cache.render!
     end
     @display_cache
@@ -76,28 +85,28 @@ class PageRenderer
 
   def display_published
     unless @published_cache && @published_cache.respond_to?(:chunks_by_type)
-      @published_cache = WikiContent.new(@revision, {:mode => :publish})
+      @published_cache = WikiContent.new(@revision, @@url_generator, {:mode => :publish})
       @published_cache.render!
     end
     @published_cache
   end
 
   def display_content_for_export
-    WikiContent.new(@revision, {:mode => :export} ).render!
+    WikiContent.new(@revision, @@url_generator, {:mode => :export} ).render!
   end
   
   def force_rendering
     begin
       display_content.render!
     rescue => e
-      logger.error "Failed rendering page #{@name}"
+      ActionController::Base.logger.error "Failed rendering page #{@name}"
-      logger.error e
+      ActionController::Base.logger.error e
       message = e.message
       # substitute content with an error message
       @revision.content = <<-EOL
           <p>Markup engine has failed to render this page, raising the following error:</p>
           <p>#{message}</p>
-          <pre>#{self.content}</pre>
+          <pre>#{@revision.content}</pre>
       EOL
       clear_display_cache
       raise e

lib/url_generator.rb

@@ -1,61 +1,121 @@
-class UrlGenerator
+class AbstractUrlGenerator
 
-  def initialize(controller = nil)
+  def initialize(controller)
-    @controller = controller or ControllerStub.new
+    raise 'Controller cannot be nil' if controller.nil?
+    @controller = controller
   end
 
-  def make_file_link(mode, name, text, base_url, known_file)
+  # Create a link for the given page (or file) name and link text based
-    link = CGI.escape(name)
+  # on the render mode in options and whether the page (file) exists
+  # in the web.
+  def make_link(name, web, text = nil, options = {})
+    text = CGI.escapeHTML(text || WikiWords.separate(name))
+    mode = (options[:mode] || :show).to_sym
+    link_type = (options[:link_type] || :show).to_sym
+
+    if (link_type == :show)
+      known_page = web.has_page?(name)
+    else
+      known_page = web.has_file?(name)
+    end
+    
+    case link_type
+    when :show
+      page_link(mode, name, text, web.address, known_page)
+    when :file
+      file_link(mode, name, text, web.address, known_page)
+    when :pic
+      pic_link(mode, name, text, web.address, known_page)
+    else
+      raise "Unknown link type: #{link_type}"
+    end
+  end
+  
+end
+
+class UrlGenerator < AbstractUrlGenerator
+
+  private
+
+  def file_link(mode, name, text, web_address, known_file)
     case mode
     when :export
-      if known_file then "<a class=\"existingWikiWord\" href=\"#{link}.html\">#{text}</a>"
-      else "<span class=\"newWikiWord\">#{text}</span>" end
-    when :publish
-      if known_file then "<a class=\"existingWikiWord\" href=\"#{base_url}/published/#{link}\">#{text}</a>"
-      else "<span class=\"newWikiWord\">#{text}</span>" end
-    else 
       if known_file
-        "<a class=\"existingWikiWord\" href=\"#{base_url}/file/#{link}\">#{text}</a>"
+        %{<a class="existingWikiWord" href="#{CGI.escape(name)}.html">#{text}</a>}
       else 
-        "<span class=\"newWikiWord\">#{text}<a href=\"#{base_url}/file/#{link}\">?</a></span>"
+        %{<span class="newWikiWord">#{text}</span>}
+      end
+    when :publish
+      if known_file 
+        href = @controller.url_for :controller => 'wiki', :web => web_address, :action => 'published', 
+            :id => name
+        %{<a class="existingWikiWord" href="#{href}">#{text}</a>}
+      else 
+        %{<span class="newWikiWord">#{text}</span>}
+      end
+    else 
+      href = @controller.url_for :controller => 'wiki', :web => web_address, :action => 'file', 
+          :id => name
+      if known_file
+        %{<a class="existingWikiWord" href="#{href}">#{text}</a>}
+      else 
+        %{<span class="newWikiWord">#{text}<a href="#{href}">?</a></span>}
       end
     end
   end
 
-  def make_page_link(mode, name, text, base_url, known_page)
+  def page_link(mode, name, text, web_address, known_page)
-    link = CGI.escape(name)
+    case mode
-    case mode.to_sym
     when :export
-      if known_page then %{<a class="existingWikiWord" href="#{link}.html">#{text}</a>}
+      if known_page 
-      else %{<span class="newWikiWord">#{text}</span>} end
+        %{<a class="existingWikiWord" href="#{CGI.escape(name)}.html">#{text}</a>}
+      else 
+        %{<span class="newWikiWord">#{text}</span>} 
+      end
     when :publish
-      if known_page then %{<a class="existingWikiWord" href="#{base_url}/published/#{link}">#{text}</a>}
+      if known_page
-      else %{<span class="newWikiWord">#{text}</span>} end
+        href = @controller.url_for :controller => 'wiki', :web => web_address, :action => 'published', 
+            :id => name
+        %{<a class="existingWikiWord" href="#{href}">#{text}</a>}
+      else 
+        %{<span class="newWikiWord">#{text}</span>} 
+      end
     else 
       if known_page
-        %{<a class="existingWikiWord" href="#{base_url}/show/#{link}">#{text}</a>}
+        href = @controller.url_for :controller => 'wiki', :web => web_address, :action => 'show', 
+            :id => name
+        %{<a class="existingWikiWord" href="#{href}">#{text}</a>}
       else 
-        %{<span class="newWikiWord">#{text}<a href="#{base_url}/show/#{link}">?</a></span>}
+        href = @controller.url_for :controller => 'wiki', :web => web_address, :action => 'new', 
+            :id => name
+        %{<span class="newWikiWord">#{text}<a href="#{href}">?</a></span>}
       end
     end
   end
 
-  def make_pic_link(mode, name, text, base_url, known_pic)
+  def pic_link(mode, name, text, web_address, known_pic)
-    link = CGI.escape(name)
+    case mode
-    case mode.to_sym
     when :export
-      if known_pic then %{<img alt="#{text}" src="#{link}" />}
+      if known_pic 
-      else %{<img alt="#{text}" src="no image" />} end
+        %{<img alt="#{text}" src="#{CGI.escape(name)}" />}
+      else 
+        %{<img alt="#{text}" src="no image" />}
+      end
     when :publish
-      if known_pic then %{<img alt="#{text}" src="#{link}" />}
+      if known_pic 
-      else %{<span class="newWikiWord">#{text}</span>} end
+        %{<img alt="#{text}" src="#{CGI.escape(name)}" />}
+      else 
+        %{<span class="newWikiWord">#{text}</span>} 
+      end
     else 
-      if known_pic then %{<img alt="#{text}" src="#{base_url}/pic/#{link}" />}
+      href = @controller.url_for @controller => 'file', :web => web_address, :action => 'pic', 
-      else %{<span class="newWikiWord">#{text}<a href="#{base_url}/pic/#{link}">?</a></span>} end
+          :id => name
+      if known_pic 
+        %{<img alt="#{text}" src="#{href}" />}
+      else 
+        %{<span class="newWikiWord">#{text}<a href="#{href}">?</a></span>} 
+      end
     end
   end
 
 end
-
-class ControllerStub
-end

lib/wiki_content.rb

@@ -126,8 +126,9 @@ class WikiContent < String
 
   # Create a new wiki content string from the given one.
   # The options are explained at the top of this file.
-  def initialize(revision, options = {})
+  def initialize(revision, url_generator, options = {})
     @revision = revision
+    @url_generator = url_generator
     @web = @revision.page.web
 
     @options = DEFAULT_OPTS.dup.merge(options)
@@ -146,7 +147,7 @@ class WikiContent < String
   # Call @web.page_link using current options.
   def page_link(name, text, link_type)
     @options[:link_type] = (link_type || :show)
-    @web.make_link(name, text, @options)
+    @url_generator.make_link(name, @web, text, @options)
   end
 
   def build_chunks

test/test_helper.rb

@@ -11,6 +11,7 @@ require 'action_controller/test_process'
 require 'action_web_service/test_invoke'
 require 'breakpoint'
 require 'wiki_content'
+require 'url_generator'
 
 # Uncomment these and hang on, because the tests will be FAST
 #Test::Unit::TestCase.pre_loaded_fixtures = false
@@ -43,6 +44,7 @@ class Test::Unit::TestCase
   end
 
   def test_renderer(revision = nil)
+    PageRenderer.setup_url_generator(StubUrlGenerator.new)
     PageRenderer.new(revision)
   end
 
@@ -93,6 +95,64 @@ module ChunkMatch
   end
 end
 
+class StubUrlGenerator < AbstractUrlGenerator
+
+  def initialize
+    super(:doesnt_need_controller)
+  end
+
+  def file_link(mode, name, text, web_name, known_file)
+    link = CGI.escape(name)
+    case mode
+    when :export
+      if known_file then %{<a class="existingWikiWord" href="#{link}.html">#{text}</a>}
+      else %{<span class="newWikiWord">#{text}</span>} end
+    when :publish
+      if known_file then %{<a class="existingWikiWord" href="../published/#{link}">#{text}</a>}
+      else %{<span class=\"newWikiWord\">#{text}</span>} end
+    else 
+      if known_file
+        %{<a class=\"existingWikiWord\" href=\"../file/#{link}\">#{text}</a>}
+      else 
+        %{<span class=\"newWikiWord\">#{text}<a href=\"../file/#{link}\">?</a></span>}
+      end
+    end
+  end
+
+  def page_link(mode, name, text, web_address, known_page)
+    link = CGI.escape(name)
+    case mode.to_sym
+    when :export
+      if known_page then %{<a class="existingWikiWord" href="#{link}.html">#{text}</a>}
+      else %{<span class="newWikiWord">#{text}</span>} end
+    when :publish
+      if known_page then %{<a class="existingWikiWord" href="../published/#{link}">#{text}</a>}
+      else %{<span class="newWikiWord">#{text}</span>} end
+    else 
+      if known_page
+        %{<a class="existingWikiWord" href="../show/#{link}">#{text}</a>}
+      else 
+        %{<span class="newWikiWord">#{text}<a href="../show/#{link}">?</a></span>}
+      end
+    end
+  end
+
+  def pic_link(mode, name, text, web_name, known_pic)
+    link = CGI.escape(name)
+    case mode.to_sym
+    when :export
+      if known_pic then %{<img alt="#{text}" src="#{link}" />}
+      else %{<img alt="#{text}" src="no image" />} end
+    when :publish
+      if known_pic then %{<img alt="#{text}" src="#{link}" />}
+      else %{<span class="newWikiWord">#{text}</span>} end
+    else 
+      if known_pic then %{<img alt="#{text}" src="../pic/#{link}" />}
+      else %{<span class="newWikiWord">#{text}<a href="../pic/#{link}">?</a></span>} end
+    end
+  end
+end
+
 if defined? $validate_xml_in_assert_success and $validate_xml_in_assert_success == true
   module Test
     module Unit

test/unit/page_renderer_test.rb

@@ -25,50 +25,6 @@ class PageRendererTest < Test::Unit::TestCase
         rendered_content(@web.page("SecondPage")))
   end
   
-  def test_make_link
-    add_sample_pages
-    
-    existing_page_wiki_url = 
-        '<a class="existingWikiWord" href="../show/EverBeenInLove">Ever Been In Love</a>'
-    existing_page_published_url = 
-        '<a class="existingWikiWord" href="../published/EverBeenInLove">Ever Been In Love</a>'
-    existing_page_static_url = 
-        '<a class="existingWikiWord" href="EverBeenInLove.html">Ever Been In Love</a>'
-    new_page_wiki_url = 
-        '<span class="newWikiWord">Unknown Word<a href="../show/UnknownWord">?</a></span>'
-    new_page_published_url =  new_page_static_url = '<span class="newWikiWord">Unknown Word</span>'
-    
-    # no options
-    assert_equal existing_page_wiki_url, @web.make_link('EverBeenInLove')
-  
-    # :mode => :export
-    assert_equal existing_page_static_url, @web.make_link('EverBeenInLove', nil, :mode => :export)
-  
-    # :mode => :publish
-    assert_equal existing_page_published_url, 
-        @web.make_link('EverBeenInLove', nil, :mode => :publish)
-  
-    # new page, no options
-    assert_equal new_page_wiki_url, @web.make_link('UnknownWord')
-  
-    # new page, :mode => :export
-    assert_equal new_page_static_url, @web.make_link('UnknownWord', nil, :mode => :export)
-  
-    # new page, :mode => :publish
-    assert_equal new_page_published_url, @web.make_link('UnknownWord', nil, :mode => :publish)
-  
-    # Escaping special characters in the name
-    assert_equal(
-        '<span class="newWikiWord">Smith &amp; Wesson<a href="../show/Smith+%26+Wesson">?</a></span>', 
-        @web.make_link('Smith & Wesson'))
-  
-    # optionally using text as the link text
-    assert_equal(
-      existing_page_published_url.sub(/>Ever Been In Love</, ">Haven't you ever been in love?<"),
-      @web.make_link('EverBeenInLove', "Haven't you ever been in love?", :mode => :publish))
-  
-  end
-
   def test_wiki_words
     assert_equal %w( HisWay MyWay SmartEngine SmartEngineGUI ThatWay ), 
         test_renderer(@revision).wiki_words.sort

test/watir/e2e.rb

@@ -59,7 +59,7 @@ class E2EInstikiTest < Test::Unit::TestCase
   def test_00020_add_a_page
     # Add reference to a non-existant wiki page
     enter_markup('HomePage', '[[Another Wiki Page]]')
-    assert_equal '?', ie.link(:url, url(:show, 'Another Wiki Page')).text
+    assert_equal '?', ie.link(:url, url(:new, 'Another Wiki Page')).text
     
     # Edit the first revision of a page
     enter_markup('Another Wiki Page', 'First revision of Another Wiki Page, linked from HomePage')