diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb index dd5be6dc..dd0837ea 100644 --- a/app/controllers/blame_controller.rb +++ b/app/controllers/blame_controller.rb @@ -1,7 +1,6 @@ # Controller for viewing a file's blame class BlameController < ApplicationController - - include RefExtractor + include ExtractsPath layout "project" diff --git a/app/controllers/blob_controller.rb b/app/controllers/blob_controller.rb index 58e70bc9..33387842 100644 --- a/app/controllers/blob_controller.rb +++ b/app/controllers/blob_controller.rb @@ -1,9 +1,6 @@ # Controller for viewing a file's blame class BlobController < ApplicationController - # Thrown when given an invalid path - class InvalidPathError < StandardError; end - - include RefExtractor + include ExtractsPath include Gitlab::Encode layout "project" diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb index 43087664..e6313783 100644 --- a/app/controllers/tree_controller.rb +++ b/app/controllers/tree_controller.rb @@ -1,6 +1,6 @@ # Controller for viewing a repository's file structure class TreeController < ApplicationController - include RefExtractor + include ExtractsPath layout "project" diff --git a/lib/ref_extractor.rb b/lib/extracts_path.rb similarity index 77% rename from lib/ref_extractor.rb rename to lib/extracts_path.rb index 1db74b95..6648ffdc 100644 --- a/lib/ref_extractor.rb +++ b/lib/extracts_path.rb @@ -1,14 +1,22 @@ -# Module providing an extract_ref method for controllers working with Git -# tree-ish + path params -module RefExtractor - # Raised when given an invalid path +# Module providing methods for dealing with separating a tree-ish string and a +# file path string when combined in a request parameter +module ExtractsPath + extend ActiveSupport::Concern + + # Raised when given an invalid file path class InvalidPathError < StandardError; end - # Given a string containing both a Git ref - such as a branch or tag - and a - # filesystem path joined by forward slashes, attempts to separate the two. + included do + if respond_to?(:before_filter) + before_filter :assign_ref_vars + end + end + + # Given a string containing both a Git tree-ish, such as a branch or tag, and + # a filesystem path joined by forward slashes, attempts to separate the two. # - # Expects a @project instance variable to contain the active project. Used to - # check the input against a list of valid repository refs. + # Expects a @project instance variable to contain the active project. This is + # used to check the input against a list of valid repository refs. # # Examples # @@ -78,8 +86,11 @@ module RefExtractor # - @commit - A CommitDecorator representing the commit from the given ref # - @tree - A TreeDecorator representing the tree at the given ref/path # - # Automatically renders `not_found!` if a valid tree could not be resolved - # (e.g., when a user inserts an invalid path or ref). + # If the :id parameter appears to be requesting a specific response format, + # that will be handled as well. + # + # Automatically renders `not_found!` if a valid tree path could not be + # resolved (e.g., when a user inserts an invalid path or ref). def assign_ref_vars # Handle formats embedded in the id if params[:id].ends_with?('.atom') diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb new file mode 100644 index 00000000..8876373d --- /dev/null +++ b/spec/lib/extracts_path_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe ExtractsPath do + include ExtractsPath + + let(:project) { double('project') } + + before do + @project = project + project.stub(:branches).and_return(['master', 'foo/bar/baz']) + project.stub(:tags).and_return(['v1.0.0', 'v2.0.0']) + end + + describe '#extract_ref' do + it "returns an empty pair when no @project is set" do + @project = nil + extract_ref('master/CHANGELOG').should == ['', ''] + end + + context "without a path" do + it "extracts a valid branch" do + extract_ref('master').should == ['master', ''] + end + + it "extracts a valid tag" do + extract_ref('v2.0.0').should == ['v2.0.0', ''] + end + + it "extracts a valid commit ref without a path" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', ''] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable').should == ['stable', ''] + end + end + + context "with a path" do + it "extracts a valid branch" do + extract_ref('foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] + end + + it "extracts a valid tag" do + extract_ref('v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] + end + + it "extracts a valid commit SHA" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] + end + end + end +end diff --git a/spec/lib/ref_extractor_spec.rb b/spec/lib/ref_extractor_spec.rb index 1e3d178f..8876373d 100644 --- a/spec/lib/ref_extractor_spec.rb +++ b/spec/lib/ref_extractor_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe RefExtractor do - include RefExtractor +describe ExtractsPath do + include ExtractsPath let(:project) { double('project') }