Commit tests are now complete. Added the ability to pass a class to f…

…ind, get, post and submit so we can get some good error messages for 404s.

examples/overall.rb

@@ -36,7 +36,7 @@
 # single commit information
 # details is the same as: Commit.find(commit)
 puts "Diff:"
-commit.details.modified.each {|m| puts "#{m['filename']} DIFF: #{m['diff']}" }
+commit.modified.each {|m| puts "#{m['filename']} DIFF: #{m['diff']}" }
 
 # repository search
 repos = Repository.find_all("ruby", "git")

lib/octopi/api.rb

@@ -93,8 +93,8 @@ def save(resource_path, data)
       post("#{resource_path}", { :query => data })
     end
 
-    def find(path, result_key, resource_id)
-      get(path, { :id => resource_id }) 
+    def find(path, result_key, resource_id, klass=nil)
+      get(path, { :id => resource_id }, klass) 
     end
 
 
@@ -106,11 +106,11 @@ def get_raw(path, params)
      get(path, params, 'plain')
     end
 
-    def get(path, params = {}, format = "yaml")
+    def get(path, params = {}, klass=nil, format = "yaml")
       @@retries = 0
       begin
         trace "GET [#{format}]", "/#{format}#{path}", params
-        submit(path, params, format) do |path, params, format|
+        submit(path, params, klass, format) do |path, params, format|
           self.class.get "/#{format}#{path}"
         end
       rescue RetryableAPIError => e
@@ -126,11 +126,11 @@ def get(path, params = {}, format = "yaml")
       end  
     end
 
-    def post(path, params = {}, format = "yaml")
+    def post(path, params = {}, klass=nil, format = "yaml")
       @@retries = 0
       begin
         trace "POST", "/#{format}#{path}", params
-        submit(path, params, format) do |path, params, format|
+        submit(path, params, klass, format) do |path, params, format|
           resp = self.class.post "/#{format}#{path}", :body => params
           resp
         end
@@ -148,7 +148,7 @@ def post(path, params = {}, format = "yaml")
     end
 
     private
-    def submit(path, params = {}, format = "yaml", &block)
+    def submit(path, params = {}, klass=nil, format = "yaml", &block)
       params.each_pair do |k,v|
         if path =~ /:#{k.to_s}/
           params.delete(k)
@@ -177,6 +177,7 @@ def submit(path, params = {}, format = "yaml", &block)
 
 
       raise RetryableAPIError, resp.code.to_i if RETRYABLE_STATUS.include? resp.code.to_i
+      raise NotFound, klass || self.class if resp.code.to_i == 404
       raise APIError, 
         "GitHub returned status #{resp.code}" unless resp.code.to_i == 200
       # FIXME: This fails for showing raw Git data because that call returns

lib/octopi/base.rb

@@ -52,6 +52,11 @@ def save
     end
 
     private
+
+    def self.gather_name(opts)
+      opts[:repository] || opts[:repo] || opts[:name]
+    end
+
     def self.extract_user_repository(*args)
       opts = args.last.is_a?(Hash) ? args.pop : {}
       if opts.empty?

lib/octopi/commit.rb

@@ -4,7 +4,7 @@ class Commit < Base
     find_path "/commits/list/:query"
     resource_path "/commits/show/:id"
 
-    attr_accessor :repository, :message, :parents, :author, :url, :id, :committed_date, :authored_date, :tree, :committer
+    attr_accessor :repository, :message, :parents, :author, :url, :id, :committed_date, :authored_date, :tree, :committer, :added, :removed, :modified
 
 
     # Finds all commits for the given options:
@@ -22,12 +22,7 @@ class Commit < Base
     #   => <Latest 30 commits for lazy branch>
     #
     def self.find_all(opts={})
-      repo = opts[:repository] || opts[:repo] || opts[:name]
-      repo = Repository.find(:user => opts[:user], :name => repo) if !repo.is_a?(Repository)
-      user = repo.owner.to_s
-      user ||= opts[:user].to_s
-      branch = opts[:branch] || "master"
-      self.validate_args(user => :user, repo.name => :repo)
+      repo, user, branch = gather_details(opts)
       commits = super user, repo.name, branch
       # Repository is not passed in from the data, set it manually.
       commits.each { |c| c.repository = repo }
@@ -50,20 +45,8 @@ def self.find_all(opts={})
     #   => <Commit f6609209c3ac0badd004512d318bfaa508ea10ae for branch lazy>
     #
     def self.find(opts={})
-      if args.last.is_a?(Commit)
-        commit = args.pop
-        super "#{commit.repo_identifier}"
-      else
-        user, name, sha = *args
-        user = user.login if user.is_a? User
-        name = repo.name  if name.is_a? Repository
-        self.validate_args(user => :user, name => :repo, sha => :sha)
-        super [user, name, sha]
-      end
-    end
-
-    def details
-      self.class.find(self)
+      repo, user, branch, sha = gather_details(opts)
+      super [user, repo, sha]
     end
 
     def repo_identifier
@@ -76,5 +59,17 @@ def repo_identifier
 
       parts.join('/')
     end
+
+    private
+
+    def self.gather_details(opts)
+      repo = self.gather_name(opts)
+      repo = Repository.find(:user => opts[:user], :name => repo) if !repo.is_a?(Repository)
+      user = repo.owner.to_s
+      user ||= opts[:user].to_s
+      branch = opts[:branch] || "master"
+      self.validate_args(user => :user, repo.name => :repo)
+      [repo, user, branch, opts[:sha]]
+    end
   end
 end

lib/octopi/resource.rb

@@ -38,7 +38,7 @@ def delete_path(path)
 
       def find(*args)
         args = args.join('/') if args.is_a? Array
-        result = Api.api.find(path_for(:resource), @resource_name[:singular], args)
+        result = Api.api.find(path_for(:resource), @resource_name[:singular], args, self)
         key = result.keys.first
 
         if result[key].is_a? Array

test/branch_test.rb

@@ -13,7 +13,7 @@ def setup
     end
 
     should "not find a branch that doesn't exist" do
-      assert_raises NotFound do
+      assert_raise NotFound do
         Branch.all("fcoury", "octopi").find("non-existant")
       end
     end

test/commit_test.rb

@@ -30,6 +30,38 @@ def setup
         assert_equal 30, commits.size
       end
     end
+
+    context "finding a single commit" do
+      should "by strings" do
+        commit = Commit.find(:name => "octopi", :user => "fcoury", :sha => "f6609209c3ac0badd004512d318bfaa508ea10ae")
+        assert_not_nil commit
+      end
+
+      should "by objects" do
+        commit = Commit.find(:name => "octopi", :user => "fcoury", :sha => "f6609209c3ac0badd004512d318bfaa508ea10ae")
+        assert_not_nil commit
+      end
+
+      should "be able to specify a branch" do
+        commit = Commit.find(:name => "octopi", :user => "fcoury", :sha => "f6609209c3ac0badd004512d318bfaa508ea10ae", :branch => "lazy")
+        assert_not_nil commit
+      end
+
+      should "raise an error if not found" do
+        exception = assert_raise Octopi::NotFound do
+          Commit.find(:name => "octopi", :user => "fcoury", :sha => "nothere")
+        end
+
+        assert_equal "The Commit you were looking for could not be found, or is private.", exception.message
+      end
+    end
+
+    context "identifying a repository" do
+      should "be possible" do
+        commit = Commit.find(:name => "octopi", :user => "fcoury", :sha => "f6609209c3ac0badd004512d318bfaa508ea10ae")
+        assert_equal "fcoury/octopi/f6609209c3ac0badd004512d318bfaa508ea10ae", commit.repo_identifier
+      end
+    end
   end
 end
 

test/repository_test.rb

@@ -37,9 +37,11 @@ def setup
     end
 
     should "not return a repository when asked for a private one" do
-      assert_raises NotFound do
+      exception = assert_raise NotFound do
         @user.repository(:name => "rboard")
       end
+
+      assert_equal "The Repository you were looking for could not be found, or is private.", exception.message
     end
 
     should "return a private repository when authed" do

test/stubs/commits/fcoury/octopi/f6609209c3ac0badd004512d318bfaa508ea10ae

@@ -0,0 +1,82 @@
+--- 
+commit: 
+  removed: []
+
+  added: []
+
+  message: |-
+    Fix pulling issues from private repos.
+
+    Signed-off-by: Felipe Coury <felipe.coury@gmail.com>
+  modified: 
+  - diff: |-
+      @@ -23,13 +23,14 @@ module Octopi
+           #   find_all(:user => "fcoury", :repo => "octopi") # state defaults to open
+           #
+           def self.find_all(*args)
+      +      api = (args.length == 4 ? args.pop : nil) || ANONYMOUS_API
+             repo = args.first
+             user, repo_name, opts = extract_user_repository(*args)
+             state = opts[:state] || "open"
+             state.downcase! if state
+             validate_args(user => :user, repo_name => :repo, state => :state)
+
+      -      issues = super user, repo_name, state
+      +      issues = super user, repo_name, state, api
+             issues.each { |i| i.repository = repo } if repo.is_a? Repository
+             issues
+           end
+      @@ -39,7 +40,7 @@ module Octopi
+             if args.length < 2
+               raise "Issue.find needs user, repository and issue number"
+             end
+      -      
+      +      api = (args.length == 3 ? args.pop : nil) || ANONYMOUS_API
+             number = args.pop.to_i if args.last.respond_to?(:to_i)
+             number = args.pop if args.last.is_a?(Integer)
+
+      @@ -55,7 +56,7 @@ module Octopi
+
+             user, repo = extract_names(user, repo)
+             validate_args(user => :user, repo => :repo)
+      -      super user, repo, number
+      +      super user, repo, number, api
+           end
+
+           def self.open(user, repo, params, api = ANONYMOUS_API)
+    filename: lib/octopi/issue.rb
+  - diff: |-
+      @@ -79,7 +79,8 @@ module Octopi
+           end
+
+           def issues(state = "open")
+      -      Issue.find_all(self, :state => state)
+      +      api = self.api || ANONYMOUS_API
+      +      Issue.find_all(self, { :state => state }, api)
+           end
+
+           def all_issues
+      @@ -87,7 +88,8 @@ module Octopi
+           end
+
+           def issue(number)
+      -      Issue.find(self, number)
+      +      api = self.api || ANONYMOUS_API
+      +      Issue.find(self, number, api)
+           end
+
+           def collaborators
+    filename: lib/octopi/repository.rb
+  parents: 
+  - id: 737650490a3cf3689f92644126407b21c4493d9a
+  url: http://github.com/fcoury/octopi/commit/f6609209c3ac0badd004512d318bfaa508ea10ae
+  author: 
+    name: Andrew Moreland
+    email: andy@andymo.org
+  id: f6609209c3ac0badd004512d318bfaa508ea10ae
+  committed_date: "2009-07-27T18:24:59-07:00"
+  authored_date: "2009-07-26T17:37:41-07:00"
+  tree: a59231d4e3b99d6aaf349344ad0306b1bee7622d
+  committer: 
+    name: Felipe Coury
+    email: felipe.coury@gmail.com

test/test_helper.rb

@@ -12,6 +12,8 @@ def stub_file(*path)
 end
 
 def fake_everything
+  # helper variables to make things shorter.
+  sha = "f6609209c3ac0badd004512d318bfaa508ea10ae"
   yaml_api = "github.com/api/v2/yaml"
   plain_api = "github.com:80/api/v2/plain"
   # Set this to true or comment out if you want to test against real data
@@ -20,19 +22,23 @@ def fake_everything
   FakeWeb.allow_net_connect = false
 
   # Public stuff
-  fakes = { 
-        "user/show/fcoury" => File.join("users", "fcoury"),
+  fakes = {
+
+        "blob/show/fcoury/octopi/#{sha}" => File.join("blob", "fcoury", "octopi", "plain", sha),
+        "commits/list/fcoury/octopi/master" => File.join("commits", "fcoury", "octopi", "master"),
+        "commits/list/fcoury/octopi/lazy" => File.join("commits", "fcoury", "octopi", "lazy"),
+        "commits/show/fcoury/octopi/#{sha}" => File.join("commits", "fcoury", "octopi", sha),
 
+        "issues/list/fcoury/octopi/open" => File.join("issues", "fcoury", "octopi", "open"),
+        "issues/show/fcoury/octopi/28" => File.join("issues", "fcoury", "octopi", "28"),
+
+        "repos/show/fcoury" => File.join("repos", "show", "fcoury"),
         "repos/show/fcoury/octopi" => File.join("repos", "fcoury", "octopi", "main"),
         "repos/show/fcoury/octopi/branches" => File.join("repos", "fcoury", "octopi", "branches"),
 
-        "issues/list/fcoury/octopi/open" => File.join("issues", "fcoury", "octopi", "open"),
-        "issues/show/fcoury/octopi/28" => File.join("issues", "fcoury", "octopi", "28"),
 
-        "commits/list/fcoury/octopi/master" => File.join("commits", "fcoury", "octopi", "master"),
-        "commits/list/fcoury/octopi/lazy" => File.join("commits", "fcoury", "octopi", "lazy"),
+        "user/show/fcoury" => File.join("users", "fcoury")
 
-        "repos/show/fcoury" => File.join("repos", "show", "fcoury")
           }
 
   fakes.each do |key, value|
@@ -42,6 +48,8 @@ def fake_everything
   # rboard is a private repository
   FakeWeb.register_uri("http://#{yaml_api}/repos/show/fcoury/rboard", :string => YAML::load_file(stub_file("errors", "repository", "not_found")))
 
+  # nothere is obviously an invalid sha
+  FakeWeb.register_uri("http://#{yaml_api}/commits/show/fcoury/octopi/nothere", :status => ["404", "Not Found"])
   # Personal & Private stuff
 
   secure_fakes = {
@@ -56,9 +64,10 @@ def fake_everything
   end
 
   # And the plain fakes
-  sha = "f6609209c3ac0badd004512d318bfaa508ea10ae"
   FakeWeb.register_uri("http://#{plain_api}/blob/show/fcoury/octopi/#{sha}", 
   :string => File.read(stub_file(File.join("blob", "fcoury", "octopi", "plain", sha))))
+
+
 end