diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 8b970e02..9fcc0111 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -33,7 +33,7 @@ class AdminController < ApplicationController @wiki.create_web(@params['name'], @params['address']) redirect_show('HomePage', @params['address']) else - redirect_to :action => 'index' + redirect_to :controller => 'wiki', :action => 'index' end else # no form submitted -> render template @@ -80,7 +80,7 @@ class AdminController < ApplicationController if wiki.authenticate(@params['system_password_orphaned']) wiki.remove_orphaned_pages(@web_name) flash[:info] = 'Orphaned pages removed' - redirect_to :action => 'list' + redirect_to :controller => 'wiki', :web => @web_name, :action => 'list' else flash[:error] = password_error(@params['system_password']) return_to_last_remembered diff --git a/app/controllers/application.rb b/app/controllers/application.rb index c2648b66..3f4fc67e 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -1,12 +1,7 @@ -require 'url_rewriting_hack' - # The filters added to this controller will be run for all controllers in the application. # Likewise will all the methods added be available for all controllers. class ApplicationController < ActionController::Base - # implements Instiki's legacy URLs - require 'url_rewriting_hack' - before_filter :set_utf8_http_header, :connect_to_model after_filter :remember_location @@ -88,7 +83,7 @@ class ApplicationController < ActionController::Base def remember_location if @response.headers['Status'] == '200 OK' unless @@REMEMBER_NOT.include? action_name or @request.method != :get - @session[:return_to] = url_for + @session[:return_to] = @request.request_uri logger.debug("Session ##{session.object_id}: remembered URL '#{@session[:return_to]}'") end end diff --git a/app/views/file/file.rhtml b/app/views/file/file.rhtml index f4415cf9..63c188eb 100644 --- a/app/views/file/file.rhtml +++ b/app/views/file/file.rhtml @@ -1,10 +1,10 @@ <% @title = "Upload #{@file_name}" - @hide_navigatio = false + @hide_navigation = false %>

-<%= form_tag({}, {:multipart => true}) %> +<%= form_tag({:controller => 'file', :web => @web_name, :action => 'file'}, {:multipart => true}) %>

File to upload:
@@ -14,9 +14,6 @@ as - <% if @page %> - | Cancel (unlocks page) - <% end %>

<%= end_form_tag %>

\ No newline at end of file diff --git a/config/environment.rb b/config/environment.rb index 2c32f084..0cf0efa7 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -47,6 +47,7 @@ end require 'action_controller' require 'active_record_stub' require 'instiki_errors' +require 'routes' unless defined? RAILS_DEFAULT_LOGGER RAILS_DEFAULT_LOGGER = Logger.new(STDERR) diff --git a/config/routes.rb b/config/routes.rb new file mode 100644 index 00000000..24ab21ea --- /dev/null +++ b/config/routes.rb @@ -0,0 +1,19 @@ +ActionController::Routing.draw do |map| + map.connect 'create_system', :controller => 'admin', :action => 'create_system' + map.connect 'create_web', :controller => 'admin', :action => 'create_web' + map.connect 'edit_web', :controller => 'admin', :action => 'edit_web' + map.connect 'remove_orphaned_pages', :controller => 'admin', :action => 'remove_orphaned_pages' + + map.connect ':web/file/:id', :controller => 'file', :action => 'file' + map.connect ':web/pic/:id', :controller => 'file', :action => 'pic' + map.connect ':web/import/:id', :controller => 'file', :action => 'import' + + map.connect 'login', :controller => 'wiki', :action => 'login' + map.connect ':web/login', :controller => 'wiki', :action => 'login' + map.connect 'web_list', :controller => 'wiki', :action => 'web_list' + map.connect ':web/web_list', :controller => 'wiki', :action => 'web_list' + map.connect ':web/:action/:id', :controller => 'wiki' + map.connect ':web/:action', :controller => 'wiki' + map.connect ':web', :controller => 'wiki', :action => 'index' + map.connect '', :controller => 'wiki', :action => 'index' +end diff --git a/libraries/url_rewriting_hack.rb b/libraries/url_rewriting_hack.rb deleted file mode 100644 index 371dc9ef..00000000 --- a/libraries/url_rewriting_hack.rb +++ /dev/null @@ -1,112 +0,0 @@ -# Below are some hacks to Rails internal classes that implement Instiki URLs scheme. -# It is no doubt a bad practice to override internal implementation of anything. -# When Rails implements some way to do it in the framework, this code should be replaced -# with something more legitimate. - -# In Instiki URLs are mapped to the ActionPack actions, possibly performed on a particular -# web (sub-wiki) and page within that web. -# -# 1. Controller is determined by action name (default is 'wiki') -# 2. '/name1/' maps to action 'name1', unspecified web -# Example: http://localhost/create_system/ -# 3. Special case of above, URI '/wiki/' maps to action 'index', because Rails sets this address -# when default controller name is specified as 'wiki', and an application root -# (http://localhost:2500/)is requested. -# 4. '/name1/name2/' maps to web 'name1', action 'name2' -# Example: http://localhost/mywiki/search/ -# 5. '/name1/name2/name3/' maps to web 'name1', action 'name2', -# Example: http://localhost/mywiki/show/HomePage - - -require 'dispatcher' - -# Overrides Rails DispatchServlet.parse_uri -class DispatchServlet - - def self.parse_uri(path) - result = parse_path(path) - if result - result[:controller] = ActionMapper.map_to_controller(result[:action]) - result - else - false - end - end - - def self.parse_path(path) - ApplicationController.logger.debug "Parsing URI '#{path}'" - component = '([-_a-zA-Z0-9]+)' - page_name = '(.*)' - case path.sub(%r{^/(?:fcgi|mruby|cgi)/}, "/") - when '/wiki/' - { :web => nil, :controller => 'wiki', :action => 'index' } - when %r{^/#{component}/?$} - { :web => nil, :controller => 'wiki', :action => $1 } - when %r{^/#{component}/#{component}/?$} - { :web => $1, :controller => 'wiki', :action => $2 } - when %r{^/#{component}/#{component}/(.*)/?$} - { :web => $1, :controller => 'wiki', :action => $2, :id => drop_trailing_slash($3) } - else - false - end - end - - def self.drop_trailing_slash(line) - if line[-1] == ?/ - line.chop - else - line - end - end - - class ActionMapper - - @@action_to_controller_map = { - 'create_system' => 'admin', - 'create_web' => 'admin', - 'edit_web' => 'admin', - 'remove_orphaned_pages' => 'admin', - 'file' => 'file', - 'import' => 'file', - 'pic' => 'file', - } - - def self.map_to_controller(action) - @@action_to_controller_map[action] || 'wiki' - end - - end - -end - - -require 'action_controller/url_rewriter.rb' - -# Overrides parts of AP UrlRewriter to achieve the Instiki's legacy URL scheme -module ActionController - class UrlRewriter - - VALID_OPTIONS << :web unless VALID_OPTIONS.include? :web - - private - - def resolve_aliases(options) - options[:controller_prefix] = options[:web] unless options[:web].nil? - options - end - - def controller_name(options, controller_prefix) - ensure_slash_suffix(options, :controller_prefix) - - controller_name = case options[:controller_prefix] - when String: options[:controller_prefix] - when false : "" - when nil : controller_prefix || "" - end - # In Instiki we don't need the controller name (there is only one comtroller, anyway) - # therefore the below line is commented out - # controller_name << (options[:controller] + "/") if options[:controller] - return controller_name - end - end -end diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb index 89b164c5..bd1fdfcb 100644 --- a/test/functional/admin_controller_test.rb +++ b/test/functional/admin_controller_test.rb @@ -26,32 +26,32 @@ class AdminControllerTest < Test::Unit::TestCase def test_create_system_form_submitted ApplicationController.wiki = WikiServiceWithNoPersistence.new - assert !@controller.wiki.setup? + assert !ApplicationController.wiki.setup? process('create_system', 'password' => 'a_password', 'web_name' => 'My Wiki', 'web_address' => 'my_wiki') assert_redirected_to :web => 'my_wiki', :controller => 'wiki', :action => 'new', :id => 'HomePage' - assert @controller.wiki.setup? - assert_equal 'a_password', @controller.wiki.system[:password] - assert_equal 1, @controller.wiki.webs.size - new_web = @controller.wiki.webs['my_wiki'] + assert ApplicationController.wiki.setup? + assert_equal 'a_password', ApplicationController.wiki.system[:password] + assert_equal 1, ApplicationController.wiki.webs.size + new_web = ApplicationController.wiki.webs['my_wiki'] assert_equal 'My Wiki', new_web.name assert_equal 'my_wiki', new_web.address end def test_create_system_form_submitted_and_wiki_already_initialized - wiki_before = @controller.wiki - assert @controller.wiki.setup? + wiki_before = ApplicationController.wiki + assert ApplicationController.wiki.setup? process 'create_system', 'password' => 'a_password', 'web_name' => 'My Wiki', 'web_address' => 'my_wiki' assert_redirected_to :web => 'wiki1', :action => 'show', :id => 'HomePage' - assert_equal wiki_before, @controller.wiki + assert_equal wiki_before, ApplicationController.wiki # and no new web should be created either - assert_equal 1, @controller.wiki.webs.size + assert_equal 1, ApplicationController.wiki.webs.size assert_flash_has :error end @@ -200,19 +200,19 @@ class AdminControllerTest < Test::Unit::TestCase r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password_orphaned' => 'pswd') - assert_redirected_to :action => 'list' + assert_redirected_to :controller => 'wiki', :web => 'wiki1', :action => 'list' assert_equal [@home, @oak], @web.select.sort, "Pages are not as expected: #{@web.select.sort.map {|p| p.name}.inspect}" # Oak is now orphan, second pass should remove it - r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password' => 'pswd') - assert_redirected_to :action => 'list' + r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password_orphaned' => 'pswd') + assert_redirected_to :controller => 'wiki', :web => 'wiki1', :action => 'list' assert_equal [@home], @web.select.sort, "Pages are not as expected: #{@web.select.sort.map {|p| p.name}.inspect}" # third pass does not destroy HomePage - r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password' => 'pswd') + r = process('remove_orphaned_pages', 'web' => 'wiki1', 'system_password_orphaned' => 'pswd') assert_redirected_to :action => 'list' assert_equal [@home], @web.select.sort, "Pages are not as expected: #{@web.select.sort.map {|p| p.name}.inspect}" diff --git a/test/functional/routes_test.rb b/test/functional/routes_test.rb new file mode 100644 index 00000000..214bc1af --- /dev/null +++ b/test/functional/routes_test.rb @@ -0,0 +1,53 @@ +#!/bin/env ruby -w + +require File.dirname(__FILE__) + '/../test_helper' + +require 'action_controller/routing' + +class RoutesTest < Test::Unit::TestCase + + def test_parse_uri + assert_routing('', :controller => 'wiki', :action => 'index') + assert_routing('x', :controller => 'wiki', :action => 'index', :web => 'x') + assert_routing('x/y', :controller => 'wiki', :web => 'x', :action => 'y') + assert_routing('x/y/z', :controller => 'wiki', :web => 'x', :action => 'y', :id => 'z') + assert_recognizes({:web => 'x', :controller => 'wiki', :action => 'y'}, 'x/y/') + assert_recognizes({:web => 'x', :controller => 'wiki', :action => 'y', :id => 'z'}, 'x/y/z/') + end + + def test_parse_uri_interestng_cases + assert_routing('_veeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeery-long_web_/an_action/HomePage', + :web => '_veeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeery-long_web_', + :controller => 'wiki', + :action => 'an_action', :id => 'HomePage' + ) + assert_recognizes({:controller => 'wiki', :action => 'index'}, '///') + end + + def test_parse_uri_liberal_with_pagenames + + assert_routing('web/show/$HOME_PAGE', + :controller => 'wiki', :web => 'web', :action => 'show', :id => '$HOME_PAGE') + + assert_routing('web/show/HomePage?arg1=value1&arg2=value2', + :controller => 'wiki', :web => 'web', :action => 'show', + :id => 'HomePage?arg1=value1&arg2=value2') + + assert_routing('web/file/abc.zip', + :web => 'web', :controller => 'file', :action => 'file', :id => 'abc.zip') + assert_routing('web/pic/abc.jpg', + :web => 'web', :controller => 'file', :action => 'pic', :id => 'abc.jpg') + assert_routing('web/import', :web => 'web', :controller => 'file', :action => 'import') + # default option is wiki + assert_recognizes({:controller => 'wiki', :web => 'unknown_path', :action => 'index', }, + 'unknown_path') + end + + def test_cases_broken_by_routes + assert_routing('web/show/HomePage/something_else', + :controller => 'wiki', :web => 'web', :action => 'show', :id => 'HomePage/something_else') + assert_routing('web/show/Page+With+Spaces', + :controller => 'wiki', :web => 'web', :action => 'show', :id => 'Page+With+Spaces') + end + +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 66bf8d5c..b321d11f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,9 +14,9 @@ class Test::Unit::TestCase def setup_controller_test(controller_class = nil, host = nil) if controller_class - @controller = controller_class + @controller = controller_class.new elsif self.class.to_s =~ /^(\w+Controller)Test$/ - @controller = Object::const_get($1) + @controller = Object::const_get($1).new else raise "Cannot derive the name of controller under test from class name #{self.class}" end diff --git a/test/unit/url_rewriting_hack_test.rb b/test/unit/url_rewriting_hack_test.rb deleted file mode 100755 index 515ef974..00000000 --- a/test/unit/url_rewriting_hack_test.rb +++ /dev/null @@ -1,94 +0,0 @@ -#!/bin/env ruby -w - -require File.dirname(__FILE__) + '/../test_helper' -require 'url_rewriting_hack' - -class UrlRewritingHackTest < Test::Unit::TestCase - - def test_parse_uri - assert_equal({:controller => 'wiki', :action => 'x', :web => nil}, - DispatchServlet.parse_uri('/x/')) - assert_equal({:web => 'x', :controller => 'wiki', :action => 'y'}, - DispatchServlet.parse_uri('/x/y')) - assert_equal({:web => 'x', :controller => 'wiki', :action => 'y'}, - DispatchServlet.parse_uri('/x/y/')) - assert_equal({:web => 'x', :controller => 'wiki', :action => 'y', :id => 'z'}, - DispatchServlet.parse_uri('/x/y/z')) - assert_equal({:web => 'x', :controller => 'wiki', :action => 'y', :id => 'z'}, - DispatchServlet.parse_uri('/x/y/z/')) - end - - def test_parse_uri_approot - assert_equal({:controller => 'wiki', :action => 'index', :web => nil}, - DispatchServlet.parse_uri('/wiki/')) - end - - def test_parse_uri_interestng_cases - - assert_equal({:web => '_veeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeery-long_web_', - :controller => 'wiki', - :action => 'an_action', :id => 'HomePage' - }, - DispatchServlet.parse_uri( - '/_veeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeery-long_web_/an_action/HomePage') - ) - - assert_equal false, DispatchServlet.parse_uri('') - assert_equal false, DispatchServlet.parse_uri('//') - assert_equal false, DispatchServlet.parse_uri('web') - end - - def test_parse_uri_liberal_with_pagenames - - assert_equal({:controller => 'wiki', :web => 'web', :action => 'show', :id => '$HOME_PAGE'}, - DispatchServlet.parse_uri('/web/show/$HOME_PAGE')) - - assert_equal({:controller => 'wiki', :web => 'web', :action => 'show', - :id => 'HomePage/something_else'}, - DispatchServlet.parse_uri('/web/show/HomePage/something_else')) - - assert_equal({:controller => 'wiki', :web => 'web', :action => 'show', - :id => 'HomePage?arg1=value1&arg2=value2'}, - DispatchServlet.parse_uri('/web/show/HomePage?arg1=value1&arg2=value2')) - - assert_equal({:controller => 'wiki', :web => 'web', :action => 'show', - :id => 'Page+With+Spaces'}, - DispatchServlet.parse_uri('/web/show/Page+With+Spaces')) - end - - def test_url_rewriting - request = ActionController::TestRequest.new - ur = ActionController::UrlRewriter.new(request, 'wiki', 'show') - - assert_equal 'http://test.host/myweb/myaction', - ur.rewrite(:web => 'myweb', :controller => 'wiki', :action => 'myaction') - - assert_equal 'http://test.host/myOtherWeb/', - ur.rewrite(:web => 'myOtherWeb', :controller => 'wiki') - - assert_equal 'http://test.host/myaction', - ur.rewrite(:controller => 'wiki', :action => 'myaction') - - assert_equal 'http://test.host/', - ur.rewrite(:controller => 'wiki') - end - - def test_controller_mapping - request = ActionController::TestRequest.new - ur = ActionController::UrlRewriter.new(request, 'wiki', 'show') - - assert_equal 'http://test.host/file', - ur.rewrite(:controller => 'file', :action => 'file') - assert_equal 'http://test.host/pic/abc.jpg', - ur.rewrite(:controller => 'file', :action => 'pic', :id => 'abc.jpg') - assert_equal 'http://test.host/web/pic/abc.jpg', - ur.rewrite(:web => 'web', :controller => 'file', :action => 'pic', :id => 'abc.jpg') - assert_equal 'http://test.host/web/import', - ur.rewrite(:web => 'web', :controller => 'file', :action => 'import') - - # default option is wiki - assert_equal 'http://test.host/unknown_action', - ur.rewrite(:controller => 'wiki', :action => 'unknown_action') - end - -end \ No newline at end of file