Rename RefExtractor to ExtractsPath

Update docs a bit

6 files changed, 84 insertions, 19 deletions

diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb
index dd5be6dcd58..dd0837ea9ab 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 58e70bc9d04..33387842ec2 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 430876647d4..e6313783d6e 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
index 1db74b9513e..6648ffdc9a3 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 00000000000..8876373dffa
--- /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 1e3d178fbba..8876373dffa 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') }