From b40a344c495c4317b2aadc32cdf7a5358c3d5ef7 Mon Sep 17 00:00:00 2001 From: Dawa Ometto Date: Sat, 4 May 2019 01:19:59 +0200 Subject: [PATCH] Remove page file dir from frontend. Resolves #1052 #1241 (#1369) * Remove all references to page_file_dir. Use new method signature for Wiki#page. Refactor helpers. * Use write_file for uploads * Refactor /pages route --- Rakefile | 1 + lib/gollum/app.rb | 139 +++++++++++----------------- lib/gollum/helpers.rb | 32 ------- lib/gollum/templates/pages.mustache | 2 +- lib/gollum/views/layout.rb | 6 +- lib/gollum/views/pages.rb | 27 +++--- test/test_app.rb | 113 ++++++++++++++-------- test/test_app_helpers.rb | 25 ----- test/test_pages_view.rb | 6 ++ 9 files changed, 154 insertions(+), 197 deletions(-) delete mode 100644 test/test_app_helpers.rb diff --git a/Rakefile b/Rakefile index d42ebd29..fbe44492 100644 --- a/Rakefile +++ b/Rakefile @@ -71,6 +71,7 @@ Rake::TestTask.new(:test) do |test| test.libs << 'lib' << 'test' << '.' test.pattern = 'test/**/test_*.rb' test.verbose = true + test.warning = false end desc "Generate RCov test coverage and open in your browser" diff --git a/lib/gollum/app.rb b/lib/gollum/app.rb index a5547a77..2c3156a8 100644 --- a/lib/gollum/app.rb +++ b/lib/gollum/app.rb @@ -9,6 +9,7 @@ require 'json' require 'sprockets' require 'sprockets-helpers' require 'sass' +require 'pathname' require 'gollum' require 'gollum/assets' @@ -88,8 +89,10 @@ module Precious forbid unless @allow_editing || request.request_method == "GET" Precious::App.set(:mustache, {:templates => settings.wiki_options[:template_dir]}) if settings.wiki_options[:template_dir] + @base_url = url('/', false).chomp('/').force_encoding('utf-8') @page_dir = settings.wiki_options[:page_file_dir].to_s + # above will detect base_path when it's used with map in a config.ru settings.wiki_options.merge!({ :base_path => @base_url }) @css = settings.wiki_options[:css] @@ -112,7 +115,7 @@ module Precious end get '/' do - redirect clean_url(::File.join(@base_url, @page_dir, wiki_new.index_page)) + redirect clean_url(::File.join(@base_url, wiki_new.index_page)) end namespace '/gollum' do @@ -166,8 +169,9 @@ module Precious get '/edit/*' do forbid unless @allow_editing wikip = wiki_page(params[:splat].first) - @name = join_page_name(wikip.name, wikip.ext) + @name = wikip.fullname @path = wikip.path + @upload_dest = find_upload_dest(wikip.fullpath) wiki = wikip.wiki @allow_uploads = wiki.allow_uploads @@ -194,28 +198,12 @@ module Precious end halt 500 unless tempfile.is_a? Tempfile - if wiki.per_page_uploads - # remove base_url and gollum/* subpath if necessary - dir = request.referer. - sub(request.base_url, ''). - sub(/.*gollum\/[-\w]+\//, '') - # remove file extension - dir = dir.sub(::File.extname(dir), '') - dir = ::File.join("uploads", dir) - else - # Remove page file dir prefix from upload path if necessary -- committer handles this itself - dir = ::File.join([wiki.page_file_dir, 'uploads'].compact) - end - halt 500 if dir.include?('..') - halt 500 unless Pathname(dir).relative? - + dir = wiki.per_page_uploads ? params[:upload_dest] : 'uploads' ext = ::File.extname(fullname) format = ext.split('.').last || 'txt' filename = ::File.basename(fullname, ext) contents = ::File.read(tempfile) - reponame = "#{filename}.#{format}" - - head = wiki.repo.head + reponame = "#{dir}/#{filename}.#{format}" options = { :message => "Uploaded file to #{dir}/#{reponame}", @@ -229,16 +217,11 @@ module Precious normalize = Gollum::Page.valid_extension?(fullname) begin - committer = Gollum::Committer.new(wiki, options) - committer.add_to_index(dir, filename, format, contents, {normalize: normalize}) - committer.after_commit do |committer, sha| - wiki.clear_cache - committer.update_working_dir(dir, filename, format) - end - committer.commit + wiki.write_file(reponame, contents, options) redirect to(request.referer) - rescue Gollum::DuplicatePageError => e - halt 409 # Signal conflict + rescue Gollum::DuplicatePageError, Gollum::IllegalDirectoryPath => e + @message = e.message + mustache :error end end @@ -247,7 +230,7 @@ module Precious wikip = wiki_page(params[:splat].first) halt 500 if wikip.nil? wiki = wikip.wiki - page = wiki.paged(join_page_name(wikip.name, wikip.ext), wikip.path, exact = true) + page = wikip.page rename = params[:rename] halt 500 if page.nil? halt 500 if rename.nil? or rename.empty? @@ -274,7 +257,7 @@ module Precious committer.commit wikip = wiki_page(rename) - page = wiki.paged(join_page_name(wikip.name, wikip.ext), wikip.path, exact = true) + page = wikip.page return if page.nil? redirect to("/#{page.escaped_url_path}") end @@ -284,8 +267,8 @@ module Precious path = "/#{clean_url(sanitize_empty_params(params[:path]))}" page_name = CGI.unescape(params[:page]) wiki = wiki_new - page = wiki.paged(page_name, path, exact = true) - + page = wiki.page(::File.join(path, page_name)) + return if page.nil? if etag != page.sha # Signal edit collision and return the page's most recent version @@ -303,7 +286,6 @@ module Precious end - post '/delete/*' do forbid unless @allow_editing wiki = wiki_new @@ -313,8 +295,7 @@ module Precious commit[:message] = "Deleted #{filepath}" wiki.delete_file(filepath, commit) end - end - + end get '/create/*' do forbid unless @allow_editing @@ -327,20 +308,11 @@ module Precious @ext = wikip.ext @path = wikip.path @allow_uploads = wikip.wiki.allow_uploads - - page_dir = settings.wiki_options[:page_file_dir].to_s - unless page_dir.empty? - # --page-file-dir docs - # /docs/Home should be created in /Home - # not /docs/Home because write_page will append /docs - @path = @path.sub(page_dir, '/') if @path.start_with? page_dir - end - @path = clean_path(@path) + @upload_dest = find_upload_dest(wikip.fullpath) page = wikip.page if page - page_dir = settings.wiki_options[:page_file_dir].to_s - redirect to("/#{clean_url(::File.join(page_dir, page.escaped_url_path))}") + redirect to("/#{clean_url(page.escaped_url_path)}") else unless Gollum::Page.format_for("#{@name}#{@ext}") @name = "#{@name}#{@ext}" @@ -359,12 +331,11 @@ module Precious path.gsub!(/^\//, '') begin - wiki.write_page(name, format, params[:content], commit_message, path) + wiki.write_page(::File.join(path, name), format, params[:content], commit_message) - page_dir = settings.wiki_options[:page_file_dir].to_s - redirect to("/#{clean_url(::File.join(encodeURIComponent(page_dir), encodeURIComponent(path), encodeURIComponent(wiki.page_file_name(name, format))))}") - rescue Gollum::DuplicatePageError => e - @message = "Duplicate page: #{e.message}" + redirect to("/#{clean_url(::File.join(encodeURIComponent(path), encodeURIComponent(wiki.page_file_name(name, format))))}") + rescue Gollum::DuplicatePageError, Gollum::IllegalDirectoryPath => e + @message = e.message mustache :error end end @@ -372,9 +343,9 @@ module Precious post '/revert/*/:sha1/:sha2' do wikip = wiki_page(params[:splat].first) @path = wikip.path - @name = join_page_name(wikip.name, wikip.ext) + @name = wiki.fullname wiki = wikip.wiki - @page = wiki.paged(@name, @path) + @page = wikip.page sha1 = params[:sha1] sha2 = params[:sha2] @@ -448,7 +419,7 @@ module Precious }x do |path, start_version, end_version| wikip = wiki_page(path) @path = wikip.path - @name = join_page_name(wikip.name, wikip.ext) + @name = wikip.fullname @versions = [start_version, end_version] wiki = wikip.wiki @page = wikip.page @@ -472,12 +443,17 @@ module Precious /(.+) # capture any path after the "/pages" excluding the leading slash )? # end the optional non-capturing group }x do |path| - @path = extract_path(path) if path - wiki_options = settings.wiki_options.merge({ :page_file_dir => @path }) - wiki = Gollum::Wiki.new(settings.gollum_path, wiki_options) - @results = wiki.pages - @results += wiki.files - @results = @results.sort_by { |p| p.name.downcase } # Sort Results alphabetically, fixes 922 + wiki = wiki_new + @results = wiki.tree_list + + if path + @path = Pathname.new(path).cleanpath.to_s + check_path = wiki.page_file_dir ? ::File.join(wiki.page_file_dir, @path, '/') : "#{@path}/" + @results.select! {|result| result.path.start_with?(check_path) } + end + + @results.sort_by! {|result| result.name.downcase} + @ref = wiki.ref mustache :pages end @@ -486,8 +462,8 @@ module Precious get %r{/(.+?)/([0-9a-f]{40})} do file_path = params[:captures][0] version = params[:captures][1] - wikip = wiki_page(file_path, file_path, version) - name = join_page_name(wikip.name, wikip.ext) + wikip = wiki_page(file_path, version) + name = wikip.fullname path = wikip.path if page = wikip.page @page = page @@ -496,7 +472,7 @@ module Precious @version = version @bar_side = wikip.wiki.bar_side mustache :page - elsif file = wikip.wiki.file("#{file_path}", version, true) + elsif file = wikip.wiki.file(file_path, version, true) show_file(file) else halt 404 @@ -511,13 +487,11 @@ module Precious def show_page_or_file(fullpath) wiki = wiki_new - - name, ext = extract_name(fullpath) || wiki.index_page - path = extract_path(fullpath) || '/' - if page = wiki.paged(join_page_name(name, ext), path, exact = true) + if page = wiki.page(fullpath) @page = page - @name = name + @name = page.filename_stripped @content = page.formatted_data + @upload_dest = find_upload_dest(Pathname.new(fullpath).cleanpath.to_s) # Extensions and layout data @editable = true @@ -532,8 +506,8 @@ module Precious show_file(file) else if @allow_editing - page_path = [path, join_page_name(name, ext)].compact.join('/') - redirect to("/gollum/create/#{clean_url(encodeURIComponent(page_path))}") + path = fullpath[-1] == '/' ? "#{fullpath}#{wiki.index_page}" : fullpath # Append default index page if no page name is supplied + redirect to("/gollum/create/#{clean_url(encodeURIComponent(path))}") else @message = "The requested page does not exist." status 404 @@ -561,19 +535,11 @@ module Precious wiki.update_page(page, name, format, content.to_s, commit) end - # path is set to name if path is nil. - # if path is 'a/b' and a and b are dirs, then - # path must have a trailing slash 'a/b/' or - # extract_path will trim path to 'a' - def wiki_page(name, path = nil, version = nil, exact = true) + def wiki_page(path, version = nil) + pathname = (Pathname.new('/') + path).cleanpath wiki = wiki_new - path = name if path.nil? - name, ext = extract_name(name) || wiki.index_page - path = extract_path(path) - path = '/' if exact && path.nil? - - OpenStruct.new(:wiki => wiki, :page => wiki.paged(join_page_name(name, ext), path, exact, version), - :name => name, :path => path, :ext => ext) + OpenStruct.new(:wiki => wiki, :page => wiki.page(pathname.to_s, version = version), + :name => pathname.basename.sub_ext('').to_s, :path => pathname.dirname.to_s, :ext => pathname.extname, :fullname => pathname.basename.to_s, :fullpath => pathname.to_s) end def wiki_new @@ -594,5 +560,12 @@ module Precious commit_message end + def find_upload_dest(path) + settings.wiki_options[:allow_uploads] ? + (settings.wiki_options[:per_page_uploads] ? + path : 'uploads' + ) : '' + end + end end diff --git a/lib/gollum/helpers.rb b/lib/gollum/helpers.rb index 034bc469..e967dbb9 100644 --- a/lib/gollum/helpers.rb +++ b/lib/gollum/helpers.rb @@ -6,42 +6,10 @@ module Precious EMOJI_PATHNAME = Pathname.new(Gemojione.images_path).freeze - def join_page_name(name, ext) - "#{name}#{ext}" - end - - # Extract the path string that Gollum::Wiki expects - def extract_path(file_path) - return nil if file_path.nil? - last_slash = file_path.rindex("/") - if last_slash - file_path[0, last_slash] - end - end - - # Extract the 'page' name from the file_path - def extract_name(file_path) - if file_path[-1, 1] == "/" - return nil - end - - # File.basename is too eager to please and will return the last - # component of the path even if it ends with a directory separator. - ext = ::File.extname(file_path) - return ::File.basename(file_path, ext), ext - end - def sanitize_empty_params(param) [nil, ''].include?(param) ? nil : CGI.unescape(param) end - # Ensure path begins with a single leading slash - def clean_path(path) - if path - (path[0] != '/' ? path.insert(0, '/') : path).gsub(/\/{2,}/, '/') - end - end - # Remove all slashes from the start of string. # Remove all double slashes def clean_url url diff --git a/lib/gollum/templates/pages.mustache b/lib/gollum/templates/pages.mustache index 7cfde7a4..9f85e382 100644 --- a/lib/gollum/templates/pages.mustache +++ b/lib/gollum/templates/pages.mustache @@ -28,7 +28,7 @@ {{#no_results}}

- There are no pages in {{ref}}. + There are no pages in {{current_path}} on {{ref}}.

{{/no_results}} diff --git a/lib/gollum/views/layout.rb b/lib/gollum/views/layout.rb index efa2c80b..8cd75688 100644 --- a/lib/gollum/views/layout.rb +++ b/lib/gollum/views/layout.rb @@ -24,16 +24,12 @@ module Precious !@path.nil? end - def page_dir - @page_dir - end - def base_url @base_url end def custom_path - "#{@base_url}#{@page_dir.empty? ? '' : '/'}#{@page_dir}" + "#{@base_url}" end def css # custom css diff --git a/lib/gollum/views/pages.rb b/lib/gollum/views/pages.rb index 51efe802..cc7eee86 100644 --- a/lib/gollum/views/pages.rb +++ b/lib/gollum/views/pages.rb @@ -6,7 +6,11 @@ module Precious attr_reader :results, :ref, :allow_editing def title - "All pages in #{@ref}" + "Overview of #{@ref}" + end + + def current_path + @path ? @path : '/' end def breadcrumb @@ -23,15 +27,15 @@ module Precious end end - breadcrumb.join(" / ") + breadcrumb.join(' / ') else - "Home" + 'Home' end end - def delete_file(url) - %Q(
) + def delete_file(path) + %Q(
) end def files_folders @@ -41,9 +45,8 @@ module Precious # 1012: Folders and Pages need to be separated @results.each do |result| - result_path = result.path - result_path = result_path.sub(/^#{Regexp.escape(@path)}\//, '') unless @path.nil? - + result_path = result.url_path + result_path = result_path.sub(/^#{Regexp.escape(@path)}\//, '') unless @path.nil? if result_path.include?('/') # result contains a folder @@ -52,12 +55,12 @@ module Precious folder_link = %{
  • #{folder}
    • } folders[folder] = folder_link unless folders.key?(folder) - elsif result_path != ".gitkeep" + elsif result_path != '.gitkeep' # result is either a valid gollum page or another type of file - klass = (defined? result.format) ? "page" : "file" + klass = (defined? result.format) ? 'page' : 'file' url = "#{@base_url}/#{result.escaped_url_path}" - file_link = %{
  • #{result.filename}#{delete_file(result.escaped_url_path) if @allow_editing}
    • } + file_link = %{
  • #{result.filename}#{delete_file(result.url_path) if @allow_editing}
    • } files[result.name] = file_link end end @@ -68,7 +71,7 @@ module Precious result else - "" + '' end end diff --git a/test/test_app.rb b/test/test_app.rb index e4832283..8e553401 100644 --- a/test/test_app.rb +++ b/test/test_app.rb @@ -196,7 +196,7 @@ context "Frontend" do test "renames page in subdirectory" do - page_1 = @wiki.paged("H", "G") + page_1 = @wiki.page("G/H") assert_not_equal page_1, nil post "/gollum/rename/G/H", :rename => "/I/C", :message => 'def' @@ -205,15 +205,15 @@ context "Frontend" do assert last_response.ok? @wiki.clear_cache - assert_nil @wiki.paged("H", "G") - page_2 = @wiki.paged('C', 'I') + assert_nil @wiki.page("G/H") + page_2 = @wiki.page('I/C') assert_equal "INITIAL\n\nSPAM2\n", page_2.raw_data assert_equal 'def', page_2.version.message assert_not_equal page_1.version.sha, page_2.version.sha end test "renames page relative in subdirectory" do - page_1 = @wiki.paged("H", "G") + page_1 = @wiki.page("G/H") assert_not_equal page_1, nil post "/gollum/rename/G/H", :rename => "K/C", :message => 'def' @@ -222,8 +222,8 @@ context "Frontend" do assert last_response.ok? @wiki.clear_cache - assert_nil @wiki.paged("H", "G") - page_2 = @wiki.paged('C', 'G/K') + assert_nil @wiki.page("G/H") + page_2 = @wiki.page('G/K/C') assert_equal "INITIAL\n\nSPAM2\n", page_2.raw_data assert_equal 'def', page_2.version.message assert_not_equal page_1.version.sha, page_2.version.sha @@ -306,7 +306,6 @@ context "Frontend" do :path => '/', :format => 'markdown', :message => '' follow_redirect! assert last_response.ok? - #puts last_response @wiki.clear_cache get "/gollum/create/TT" assert last_response.ok? @@ -320,25 +319,14 @@ context "Frontend" do assert last_response.ok? Precious::App.set(:wiki_options, { :template_page => false }) end - - test "create sets the correct path for a relative path subdirectory with the page file directory set" do - Precious::App.set(:wiki_options, { :page_file_dir => "foo" }) - dir = "bardir" - name = "#{dir}/baz" - get "/gollum/create/foo/#{name}" - assert_match(/\/#{dir}/, last_response.body) - assert_no_match(/[^\/]#{dir}/, last_response.body) - # reset page_file_dir - Precious::App.set(:wiki_options, { :page_file_dir => nil }) - end test "edit returns nil for non-existant page" do # post '/edit' fails. post '/edit/' works. page = 'not-real-page' path = '/' post '/gollum/edit/', :content => 'edit_msg', - :page => page, :path => path, :message => '' - page_e = @wiki.paged(page, path) + :page => page, :path => path, :message => '' + page_e = @wiki.page(::File.join(path,page)) assert_equal nil, page_e end @@ -347,8 +335,8 @@ context "Frontend" do path = 'a/b/' # path must end with / post '/gollum/create', :content => 'create_msg', :page => page, - :path => path, :format => 'markdown', :message => '' - page_c = @wiki.paged(page, path) + :path => path, :format => 'markdown', :message => '' + page_c = @wiki.page(File.join(path, page)) assert_equal 'create_msg', page_c.raw_data # must clear or create_msg will be returned @@ -356,8 +344,8 @@ context "Frontend" do # post '/edit' fails. post '/edit/' works. post '/gollum/edit/', :content => 'edit_msg', - :page => page, :path => path, :message => '', :etag => page_c.sha - page_e = @wiki.paged(page, path) + :page => page, :path => path, :message => '', :etag => page_c.sha + page_e = @wiki.page(File.join(path, page)) assert_equal 'edit_msg', page_e.raw_data @wiki.clear_cache @@ -545,25 +533,12 @@ context "Frontend" do Precious::App.set(:wiki_options, { :js => nil }) end - test "change custom.css path if page-file-dir is set" do - Precious::App.set(:wiki_options, { :css => true, :page_file_dir => 'docs'}) - page = 'docs/yaycustom' - text = 'customized!' - - @wiki.write_page(page, :markdown, text, - { :name => 'user1', :email => 'user1' }); - - get page - assert_match /"\/docs\/custom.css"/, last_response.body - Precious::App.set(:wiki_options, { :css => nil, :page_file_dir => nil }) - end - test "show edit page with header and footer and sidebar of multibyte" do post "/gollum/create", :content => 'りんご', :page => 'Multibyte', :format => :markdown, :message => 'mesg' - page = @wiki.paged('Multibyte') + page = @wiki.page('Multibyte') post "/gollum/edit/Multibyte", :content => 'りんご', :header => 'みかん', :footer => 'バナナ', :sidebar => 'スイカ', @@ -716,7 +691,7 @@ context "Frontend with lotr" do assert_equal 'http://example.org/Mordor/Orc.md', last_response.headers['Location'] - page = @wiki.paged('Orc', 'Mordor') + page = @wiki.page('Mordor/Orc') post "/gollum/edit/Mordor/Orc", :content => 'not so big smelly creatures', :page => 'Orc', :path => 'Mordor', :message => 'minor edit', :etag => page.sha assert last_response.ok? @@ -754,3 +729,63 @@ context "Frontend with lotr" do Precious::App end end + +context "Frontend with page-file-dir" do + include Rack::Test::Methods + + setup do + @path = cloned_testpath("examples/revert.git") + @wiki = Gollum::Wiki.new(@path) + Precious::App.set(:gollum_path, @path) + Precious::App.set(:wiki_options, {allow_editing: true}) + Precious::App.set(:wiki_options, { :css => true, :page_file_dir => 'docs'}) + end + + teardown do + Precious::App.set(:wiki_options, { :css => nil, :page_file_dir => nil}) + FileUtils.rm_rf(@path) + end + + test "create sets the correct path for a relative path subdirectory with the page file directory set" do + dir = "bardir" + name = "#{dir}/baz" + get "/gollum/create/#{name}" + assert_match(/\/#{dir}/, last_response.body) + assert_no_match(/[^\/]#{dir}/, last_response.body) + end + + test "use custom.css from page-file-dir path if page-file-dir is set" do + page = 'docs/yaycustom' + text = 'customized!' + + @wiki.write_page(page, :markdown, text, + { :name => 'user1', :email => 'user1' }) + + get 'yaycustom' + assert_match /"\/custom.css"/, last_response.body + end + + test "custom.css with page-file-dir" do + custom_content = 'customized for page-file-dir' + options = { + :message => "Uploaded file", + :parent => @wiki.repo.head.commit, + :author => "Bilbo Baggins" + } + + committer = Gollum::Committer.new(@wiki, options) + committer.add_to_index('docs/custom.css', custom_content, {normalize: false}) + committer.after_commit do |committer, sha| + @wiki.clear_cache + committer.update_working_dir('docs/custom.css') + end + committer.commit + get 'custom.css' + + assert_equal custom_content, last_response.body + end + + def app + Precious::App + end +end diff --git a/test/test_app_helpers.rb b/test/test_app_helpers.rb deleted file mode 100644 index 549afff2..00000000 --- a/test/test_app_helpers.rb +++ /dev/null @@ -1,25 +0,0 @@ -# ~*~ encoding: utf-8 ~*~ -require File.expand_path(File.join(File.dirname(__FILE__), "helper")) - -context "Precious::Helpers" do - include Precious::Helpers - - test "extracting paths from URLs" do - assert_nil extract_path('Eye-Of-Sauron') - assert_equal 'Mordor', extract_path('Mordor/Sauron') - assert_equal 'Mordor/Sauron', extract_path('Mordor/Sauron/Evil') - end - - test "clean path without leading slash" do - assert_equal '/Mordor', clean_path('Mordor') - end - - test "clean path with leading slash" do - assert_equal '/Mordor', clean_path('/Mordor') - end - - test "clean path with double leading slash" do - assert_equal '/Mordor', clean_path('//Mordor') - end -end - diff --git a/test/test_pages_view.rb b/test/test_pages_view.rb index 78d5f2c2..522cf5fe 100644 --- a/test/test_pages_view.rb +++ b/test/test_pages_view.rb @@ -10,6 +10,9 @@ FakePageResult = Struct.new(:path) do def escaped_url_path CGI.escape(path).gsub(/\..+$/, "").gsub("%2F", "/") end + def url_path + path + end def format true end @@ -20,6 +23,9 @@ FakeFileResult = Struct.new(:path) do File.basename(path).gsub("-", " ") end alias filename name + def url_path + path + end def escaped_url_path result = path.sub(/\/[^\/]+$/, '/') result = result << name if result.include?('/')