From b7159b0cd4b25ee3b1a8eb9e0d4991d297487a36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Tue, 4 Aug 2015 14:47:10 +0200
Subject: [PATCH] Move from an Object interface to a type

An Object should be about representing a libgit2 object rather than
showing which methods it should support.

Change any return of Object to *Object and provide methods to convert
between this and the particular type.
---
 blob.go          |   2 +-
 commit.go        |   6 +-
 describe.go      |   2 +-
 index_test.go    |   5 +-
 object.go        | 173 +++++++++++++++++++++++++++++++++--------------
 object_test.go   |  24 +++----
 reference.go     |   2 +-
 repository.go    |  12 ++--
 reset.go         |   2 +-
 revparse.go      |  16 ++---
 revparse_test.go |   2 +-
 tag.go           |   8 +--
 tree.go          |   2 +-
 13 files changed, 163 insertions(+), 93 deletions(-)

diff --git a/blob.go b/blob.go
index b1fc78a..16ec183 100644
--- a/blob.go
+++ b/blob.go
@@ -18,7 +18,7 @@ import (
 )
 
 type Blob struct {
-	gitObject
+	Object
 	cast_ptr *C.git_blob
 }
 
diff --git a/commit.go b/commit.go
index 52f7c01..6830da3 100644
--- a/commit.go
+++ b/commit.go
@@ -14,7 +14,7 @@ import (
 
 // Commit
 type Commit struct {
-	gitObject
+	Object
 	cast_ptr *C.git_commit
 }
 
@@ -37,7 +37,7 @@ func (c Commit) Tree() (*Tree, error) {
 		return nil, MakeGitError(err)
 	}
 
-	return allocObject((*C.git_object)(ptr), c.repo).(*Tree), nil
+	return allocTree(ptr, c.repo), nil
 }
 
 func (c Commit) TreeId() *Oid {
@@ -61,7 +61,7 @@ func (c *Commit) Parent(n uint) *Commit {
 		return nil
 	}
 
-	return allocObject((*C.git_object)(cobj), c.repo).(*Commit)
+	return allocCommit(cobj, c.repo)
 }
 
 func (c *Commit) ParentId(n uint) *Oid {
diff --git a/describe.go b/describe.go
index c6f9a79..d75dbcb 100644
--- a/describe.go
+++ b/describe.go
@@ -127,7 +127,7 @@ func (c *Commit) Describe(opts *DescribeOptions) (*DescribeResult, error) {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_describe_commit(&resultPtr, c.gitObject.ptr, cDescribeOpts)
+	ecode := C.git_describe_commit(&resultPtr, c.ptr, cDescribeOpts)
 	if ecode < 0 {
 		return nil, MakeGitError(ecode)
 	}
diff --git a/index_test.go b/index_test.go
index 7c65f4f..5f6b375 100644
--- a/index_test.go
+++ b/index_test.go
@@ -32,10 +32,11 @@ func TestIndexReadTree(t *testing.T) {
 	ref, err := repo.Head()
 	checkFatal(t, err)
 
-	obj, err := ref.Peel(ObjectTree);
+	obj, err := ref.Peel(ObjectTree)
 	checkFatal(t, err)
 
-	tree := obj.(*Tree)
+	tree, err := obj.AsTree()
+	checkFatal(t, err)
 
 	idx, err := NewIndex()
 	checkFatal(t, err)
diff --git a/object.go b/object.go
index 6ecebf8..1981980 100644
--- a/object.go
+++ b/object.go
@@ -4,7 +4,11 @@ package git
 #include <git2.h>
 */
 import "C"
-import "runtime"
+import (
+	"errors"
+	"fmt"
+	"runtime"
+)
 
 type ObjectType int
 
@@ -17,15 +21,7 @@ const (
 	ObjectTag    ObjectType = C.GIT_OBJ_TAG
 )
 
-type Object interface {
-	Free()
-	Id() *Oid
-	Type() ObjectType
-	Owner() *Repository
-	Peel(t ObjectType) (Object, error)
-}
-
-type gitObject struct {
+type Object struct {
 	ptr  *C.git_object
 	repo *Repository
 }
@@ -49,23 +45,128 @@ func (t ObjectType) String() string {
 	return ""
 }
 
-func (o gitObject) Id() *Oid {
+func (o *Object) Id() *Oid {
 	return newOidFromC(C.git_object_id(o.ptr))
 }
 
-func (o gitObject) Type() ObjectType {
+func (o *Object) Type() ObjectType {
 	return ObjectType(C.git_object_type(o.ptr))
 }
 
 // Owner returns a weak reference to the repository which owns this
-// object
-func (o gitObject) Owner() *Repository {
+// object. This won't keep the underlying repository alive.
+func (o *Object) Owner() *Repository {
 	return &Repository{
 		ptr: C.git_object_owner(o.ptr),
 	}
 }
 
-func (o *gitObject) Free() {
+func dupObject(obj *Object, kind ObjectType) (*C.git_object, error) {
+	if obj.Type() != kind {
+		return nil, errors.New(fmt.Sprintf("object is not a %v", kind))
+	}
+
+	var cobj *C.git_object
+
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	if err := C.git_object_dup(&cobj, obj.ptr); err < 0 {
+		return nil, MakeGitError(err)
+	}
+
+	return cobj, nil
+}
+
+func allocTree(ptr *C.git_tree, repo *Repository) *Tree {
+	tree := &Tree{
+		Object: Object{
+			ptr:  (*C.git_object)(ptr),
+			repo: repo,
+		},
+		cast_ptr: ptr,
+	}
+	runtime.SetFinalizer(tree, (*Tree).Free)
+
+	return tree
+}
+
+func (o *Object) AsTree() (*Tree, error) {
+	cobj, err := dupObject(o, ObjectTree)
+	if err != nil {
+		return nil, err
+	}
+
+	return allocTree((*C.git_tree)(cobj), o.repo), nil
+}
+
+func allocCommit(ptr *C.git_commit, repo *Repository) *Commit {
+	commit := &Commit{
+		Object: Object{
+			ptr:  (*C.git_object)(ptr),
+			repo: repo,
+		},
+		cast_ptr: ptr,
+	}
+	runtime.SetFinalizer(commit, (*Commit).Free)
+
+	return commit
+}
+
+func (o *Object) AsCommit() (*Commit, error) {
+	cobj, err := dupObject(o, ObjectCommit)
+	if err != nil {
+		return nil, err
+	}
+
+	return allocCommit((*C.git_commit)(cobj), o.repo), nil
+}
+
+func allocBlob(ptr *C.git_blob, repo *Repository) *Blob {
+	blob := &Blob{
+		Object: Object{
+			ptr:  (*C.git_object)(ptr),
+			repo: repo,
+		},
+		cast_ptr: ptr,
+	}
+	runtime.SetFinalizer(blob, (*Blob).Free)
+
+	return blob
+}
+
+func (o *Object) AsBlob() (*Blob, error) {
+	cobj, err := dupObject(o, ObjectBlob)
+	if err != nil {
+		return nil, err
+	}
+
+	return allocBlob((*C.git_blob)(cobj), o.repo), nil
+}
+
+func allocTag(ptr *C.git_tag, repo *Repository) *Tag {
+	tag := &Tag{
+		Object: Object{
+			ptr:  (*C.git_object)(ptr),
+			repo: repo,
+		},
+		cast_ptr: ptr,
+	}
+	runtime.SetFinalizer(tag, (*Tag).Free)
+
+	return tag
+}
+
+func (o *Object) AsTag() (*Tag, error) {
+	cobj, err := dupObject(o, ObjectTag)
+	if err != nil {
+		return nil, err
+	}
+
+	return allocTag((*C.git_tag)(cobj), o.repo), nil
+}
+
+func (o *Object) Free() {
 	runtime.SetFinalizer(o, nil)
 	C.git_object_free(o.ptr)
 }
@@ -82,7 +183,7 @@ func (o *gitObject) Free() {
 //
 // If peeling a tag we discover an object which cannot be peeled to the target
 // type due to the object model, an error will be returned.
-func (o *gitObject) Peel(t ObjectType) (Object, error) {
+func (o *Object) Peel(t ObjectType) (*Object, error) {
 	var cobj *C.git_object
 
 	runtime.LockOSThread()
@@ -95,44 +196,12 @@ func (o *gitObject) Peel(t ObjectType) (Object, error) {
 	return allocObject(cobj, o.repo), nil
 }
 
-func allocObject(cobj *C.git_object, repo *Repository) Object {
-	obj := gitObject{
+func allocObject(cobj *C.git_object, repo *Repository) *Object {
+	obj := &Object{
 		ptr:  cobj,
 		repo: repo,
 	}
+	runtime.SetFinalizer(obj, (*Object).Free)
 
-	switch ObjectType(C.git_object_type(cobj)) {
-	case ObjectCommit:
-		commit := &Commit{
-			gitObject: obj,
-			cast_ptr:  (*C.git_commit)(cobj),
-		}
-		runtime.SetFinalizer(commit, (*Commit).Free)
-		return commit
-
-	case ObjectTree:
-		tree := &Tree{
-			gitObject: obj,
-			cast_ptr:  (*C.git_tree)(cobj),
-		}
-		runtime.SetFinalizer(tree, (*Tree).Free)
-		return tree
-
-	case ObjectBlob:
-		blob := &Blob{
-			gitObject: obj,
-			cast_ptr:  (*C.git_blob)(cobj),
-		}
-		runtime.SetFinalizer(blob, (*Blob).Free)
-		return blob
-	case ObjectTag:
-		tag := &Tag{
-			gitObject: obj,
-			cast_ptr:  (*C.git_tag)(cobj),
-		}
-		runtime.SetFinalizer(tag, (*Tag).Free)
-		return tag
-	}
-
-	return nil
+	return obj
 }
diff --git a/object_test.go b/object_test.go
index ef6c5a1..2ae2a6a 100644
--- a/object_test.go
+++ b/object_test.go
@@ -10,12 +10,12 @@ func TestObjectPoymorphism(t *testing.T) {
 
 	commitId, treeId := seedTestRepo(t, repo)
 
-	var obj Object
+	var obj *Object
 
 	commit, err := repo.LookupCommit(commitId)
 	checkFatal(t, err)
 
-	obj = commit
+	obj = &commit.Object
 	if obj.Type() != ObjectCommit {
 		t.Fatalf("Wrong object type, expected commit, have %v", obj.Type())
 	}
@@ -27,13 +27,13 @@ func TestObjectPoymorphism(t *testing.T) {
 	tree, err := repo.LookupTree(treeId)
 	checkFatal(t, err)
 
-	obj = tree
+	obj = &tree.Object
 	if obj.Type() != ObjectTree {
 		t.Fatalf("Wrong object type, expected tree, have %v", obj.Type())
 	}
 
-	tree2, ok := obj.(*Tree)
-	if !ok {
+	tree2, err := obj.AsTree()
+	if err != nil {
 		t.Fatalf("Converting back to *Tree is not ok")
 	}
 
@@ -46,16 +46,16 @@ func TestObjectPoymorphism(t *testing.T) {
 		t.Fatal("Wrong filemode for \"README\"")
 	}
 
-	_, ok = obj.(*Commit)
-	if ok {
+	_, err = obj.AsCommit()
+	if err == nil {
 		t.Fatalf("*Tree is somehow the same as *Commit")
 	}
 
 	obj, err = repo.Lookup(tree.Id())
 	checkFatal(t, err)
 
-	_, ok = obj.(*Tree)
-	if !ok {
+	_, err = obj.AsTree()
+	if err != nil {
 		t.Fatalf("Lookup creates the wrong type")
 	}
 
@@ -99,8 +99,8 @@ func TestObjectOwner(t *testing.T) {
 	tree, err := repo.LookupTree(treeId)
 	checkFatal(t, err)
 
-	checkOwner(t, repo, commit)
-	checkOwner(t, repo, tree)
+	checkOwner(t, repo, commit.Object)
+	checkOwner(t, repo, tree.Object)
 }
 
 func TestObjectPeel(t *testing.T) {
@@ -109,7 +109,7 @@ func TestObjectPeel(t *testing.T) {
 
 	commitID, treeID := seedTestRepo(t, repo)
 
-	var obj Object
+	var obj *Object
 
 	commit, err := repo.LookupCommit(commitID)
 	checkFatal(t, err)
diff --git a/reference.go b/reference.go
index 140082f..463f2fc 100644
--- a/reference.go
+++ b/reference.go
@@ -263,7 +263,7 @@ func (v *Reference) Delete() error {
 	return nil
 }
 
-func (v *Reference) Peel(t ObjectType) (Object, error) {
+func (v *Reference) Peel(t ObjectType) (*Object, error) {
 	var cobj *C.git_object
 
 	runtime.LockOSThread()
diff --git a/repository.go b/repository.go
index 62fde6d..2e05780 100644
--- a/repository.go
+++ b/repository.go
@@ -145,7 +145,7 @@ func (v *Repository) Index() (*Index, error) {
 	return newIndexFromC(ptr), nil
 }
 
-func (v *Repository) lookupType(id *Oid, t ObjectType) (Object, error) {
+func (v *Repository) lookupType(id *Oid, t ObjectType) (*Object, error) {
 	var ptr *C.git_object
 
 	runtime.LockOSThread()
@@ -159,7 +159,7 @@ func (v *Repository) lookupType(id *Oid, t ObjectType) (Object, error) {
 	return allocObject(ptr, v), nil
 }
 
-func (v *Repository) Lookup(id *Oid) (Object, error) {
+func (v *Repository) Lookup(id *Oid) (*Object, error) {
 	return v.lookupType(id, ObjectAny)
 }
 
@@ -169,7 +169,7 @@ func (v *Repository) LookupTree(id *Oid) (*Tree, error) {
 		return nil, err
 	}
 
-	return obj.(*Tree), nil
+	return obj.AsTree()
 }
 
 func (v *Repository) LookupCommit(id *Oid) (*Commit, error) {
@@ -178,7 +178,7 @@ func (v *Repository) LookupCommit(id *Oid) (*Commit, error) {
 		return nil, err
 	}
 
-	return obj.(*Commit), nil
+	return obj.AsCommit()
 }
 
 func (v *Repository) LookupBlob(id *Oid) (*Blob, error) {
@@ -187,7 +187,7 @@ func (v *Repository) LookupBlob(id *Oid) (*Blob, error) {
 		return nil, err
 	}
 
-	return obj.(*Blob), nil
+	return obj.AsBlob()
 }
 
 func (v *Repository) LookupTag(id *Oid) (*Tag, error) {
@@ -196,7 +196,7 @@ func (v *Repository) LookupTag(id *Oid) (*Tag, error) {
 		return nil, err
 	}
 
-	return obj.(*Tag), nil
+	return obj.AsTag()
 }
 
 func (v *Repository) Head() (*Reference, error) {
diff --git a/reset.go b/reset.go
index b5b7435..9da7625 100644
--- a/reset.go
+++ b/reset.go
@@ -17,7 +17,7 @@ const (
 func (r *Repository) ResetToCommit(commit *Commit, resetType ResetType, opts *CheckoutOpts) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
-	ret := C.git_reset(r.ptr, commit.gitObject.ptr, C.git_reset_t(resetType), opts.toC())
+	ret := C.git_reset(r.ptr, commit.ptr, C.git_reset_t(resetType), opts.toC())
 
 	if ret < 0 {
 		return MakeGitError(ret)
diff --git a/revparse.go b/revparse.go
index 7eb04f1..950932b 100644
--- a/revparse.go
+++ b/revparse.go
@@ -20,16 +20,16 @@ const (
 )
 
 type Revspec struct {
-	to    Object
-	from  Object
+	to    *Object
+	from  *Object
 	flags RevparseFlag
 }
 
-func (rs *Revspec) To() Object {
+func (rs *Revspec) To() *Object {
 	return rs.to
 }
 
-func (rs *Revspec) From() Object {
+func (rs *Revspec) From() *Object {
 	return rs.from
 }
 
@@ -38,8 +38,8 @@ func (rs *Revspec) Flags() RevparseFlag {
 }
 
 func newRevspecFromC(ptr *C.git_revspec, repo *Repository) *Revspec {
-	var to Object
-	var from Object
+	var to *Object
+	var from *Object
 
 	if ptr.to != nil {
 		to = allocObject(ptr.to, repo)
@@ -73,7 +73,7 @@ func (r *Repository) Revparse(spec string) (*Revspec, error) {
 	return newRevspecFromC(&crevspec, r), nil
 }
 
-func (v *Repository) RevparseSingle(spec string) (Object, error) {
+func (v *Repository) RevparseSingle(spec string) (*Object, error) {
 	cspec := C.CString(spec)
 	defer C.free(unsafe.Pointer(cspec))
 
@@ -90,7 +90,7 @@ func (v *Repository) RevparseSingle(spec string) (Object, error) {
 	return allocObject(ptr, v), nil
 }
 
-func (r *Repository) RevparseExt(spec string) (Object, *Reference, error) {
+func (r *Repository) RevparseExt(spec string) (*Object, *Reference, error) {
 	cspec := C.CString(spec)
 	defer C.free(unsafe.Pointer(cspec))
 
diff --git a/revparse_test.go b/revparse_test.go
index 091a76b..75e9ffd 100644
--- a/revparse_test.go
+++ b/revparse_test.go
@@ -46,7 +46,7 @@ func TestRevparseExt(t *testing.T) {
 	}
 }
 
-func checkObject(t *testing.T, obj Object, id *Oid) {
+func checkObject(t *testing.T, obj *Object, id *Oid) {
 	if obj == nil {
 		t.Fatalf("bad object")
 	}
diff --git a/tag.go b/tag.go
index ca85156..8957430 100644
--- a/tag.go
+++ b/tag.go
@@ -13,7 +13,7 @@ import (
 
 // Tag
 type Tag struct {
-	gitObject
+	Object
 	cast_ptr *C.git_tag
 }
 
@@ -30,7 +30,7 @@ func (t Tag) Tagger() *Signature {
 	return newSignatureFromC(cast_ptr)
 }
 
-func (t Tag) Target() Object {
+func (t Tag) Target() *Object {
 	var ptr *C.git_object
 	ret := C.git_tag_target(&ptr, t.cast_ptr)
 
@@ -70,7 +70,7 @@ func (c *TagsCollection) Create(
 	}
 	defer C.git_signature_free(taggerSig)
 
-	ctarget := commit.gitObject.ptr
+	ctarget := commit.ptr
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
@@ -102,7 +102,7 @@ func (c *TagsCollection) CreateLightweight(name string, commit *Commit, force bo
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 
-	ctarget := commit.gitObject.ptr
+	ctarget := commit.ptr
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
diff --git a/tree.go b/tree.go
index f543c11..8288176 100644
--- a/tree.go
+++ b/tree.go
@@ -23,7 +23,7 @@ const (
 )
 
 type Tree struct {
-	gitObject
+	Object
 	cast_ptr *C.git_tree
 }