Simplify the CommitNode API, make it look more like Commit

Signed-off-by: Filip Navara <filip.navara@gmail.com>

3 files changed, 65 insertions, 87 deletions

diff --git a/plumbing/object/commitnode.go b/plumbing/object/commitnode.go
index a613eb4..a7af4a9 100644
--- a/plumbing/object/commitnode.go
+++ b/plumbing/object/commitnode.go
@@ -16,18 +16,17 @@ type CommitNode interface {
 	ID() plumbing.Hash

 	Tree() (*Tree, error)

 	CommitTime() time.Time

+	NumParents() int

+	ParentNodes() CommitNodeIter

+	ParentNode(i int) (CommitNode, error)

+	ParentHashes() []plumbing.Hash

 }

 

 // CommitNodeIndex is generic interface encapsulating an index of CommitNode objects

 // and accessor methods for walking it as a directed graph

 type CommitNodeIndex interface {

-	NumParents(node CommitNode) int

-	ParentNodes(node CommitNode) CommitNodeIter

-	ParentNode(node CommitNode, i int) (CommitNode, error)

-	ParentHashes(node CommitNode) []plumbing.Hash

-

+	// Get returns a commit node from a commit hash

 	Get(hash plumbing.Hash) (CommitNode, error)

-

 	// Commit returns the full commit object from the node

 	Commit(node CommitNode) (*Commit, error)

 }

@@ -67,7 +66,8 @@ type graphCommitNodeIndex struct {
 //

 // objectCommitNode implements the CommitNode interface.

 type objectCommitNode struct {

-	commit *Commit

+	nodeIndex CommitNodeIndex

+	commit    *Commit

 }

 

 // objectCommitNodeIndex is an index that can load CommitNode objects only from the

@@ -93,68 +93,50 @@ func (c *graphCommitNode) CommitTime() time.Time {
 	return c.node.When

 }

 

-func (c *graphCommitNode) String() string {

-	return fmt.Sprintf(

-		"%s %s\nDate:   %s",

-		plumbing.CommitObject, c.ID(),

-		c.CommitTime().Format(DateFormat),

-	)

-}

-

-func NewGraphCommitNodeIndex(commitGraph commitgraph.Index, s storer.EncodedObjectStorer) CommitNodeIndex {

-	return &graphCommitNodeIndex{commitGraph, s}

-}

-

 // NumParents returns the number of parents in a commit.

-func (gci *graphCommitNodeIndex) NumParents(node CommitNode) int {

-	if cgn, ok := node.(*graphCommitNode); ok {

-		return len(cgn.node.ParentIndexes)

-	}

-	co := node.(*objectCommitNode)

-	return co.commit.NumParents()

+func (c *graphCommitNode) NumParents() int {

+	return len(c.node.ParentIndexes)

 }

 

 // ParentNodes return a CommitNodeIter for parents of specified node.

-func (gci *graphCommitNodeIndex) ParentNodes(node CommitNode) CommitNodeIter {

-	return newParentgraphCommitNodeIter(gci, node)

+func (c *graphCommitNode) ParentNodes() CommitNodeIter {

+	return newParentgraphCommitNodeIter(c)

 }

 

 // ParentNode returns the ith parent of a commit.

-func (gci *graphCommitNodeIndex) ParentNode(node CommitNode, i int) (CommitNode, error) {

-	if cgn, ok := node.(*graphCommitNode); ok {

-		if len(cgn.node.ParentIndexes) == 0 || i >= len(cgn.node.ParentIndexes) {

-			return nil, ErrParentNotFound

-		}

-

-		parent, err := gci.commitGraph.GetNodeByIndex(cgn.node.ParentIndexes[i])

-		if err != nil {

-			return nil, err

-		}

-

-		return &graphCommitNode{

-			hash:  cgn.node.ParentHashes[i],

-			index: cgn.node.ParentIndexes[i],

-			node:  parent,

-			gci:   gci,

-		}, nil

+func (c *graphCommitNode) ParentNode(i int) (CommitNode, error) {

+	if i < 0 || i >= len(c.node.ParentIndexes) {

+		return nil, ErrParentNotFound

 	}

 

-	co := node.(*objectCommitNode)

-	if len(co.commit.ParentHashes) == 0 || i >= len(co.commit.ParentHashes) {

-		return nil, ErrParentNotFound

+	parent, err := c.gci.commitGraph.GetNodeByIndex(c.node.ParentIndexes[i])

+	if err != nil {

+		return nil, err

 	}

 

-	parentHash := co.commit.ParentHashes[i]

-	return gci.Get(parentHash)

+	return &graphCommitNode{

+		hash:  c.node.ParentHashes[i],

+		index: c.node.ParentIndexes[i],

+		node:  parent,

+		gci:   c.gci,

+	}, nil

 }

 

 // ParentHashes returns hashes of the parent commits for a specified node

-func (gci *graphCommitNodeIndex) ParentHashes(node CommitNode) []plumbing.Hash {

-	if cgn, ok := node.(*graphCommitNode); ok {

-		return cgn.node.ParentHashes

-	}

-	co := node.(*objectCommitNode)

-	return co.commit.ParentHashes

+func (c *graphCommitNode) ParentHashes() []plumbing.Hash {

+	return c.node.ParentHashes

+}

+

+func (c *graphCommitNode) String() string {

+	return fmt.Sprintf(

+		"%s %s\nDate:   %s",

+		plumbing.CommitObject, c.ID(),

+		c.CommitTime().Format(DateFormat),

+	)

+}

+

+func NewGraphCommitNodeIndex(commitGraph commitgraph.Index, s storer.EncodedObjectStorer) CommitNodeIndex {

+	return &graphCommitNodeIndex{commitGraph, s}

 }

 

 // NodeFromHash looks up a commit node by it's object hash

@@ -181,7 +163,10 @@ func (gci *graphCommitNodeIndex) Get(hash plumbing.Hash) (CommitNode, error) {
 		return nil, err

 	}

 

-	return &objectCommitNode{commit: commit}, nil

+	return &objectCommitNode{

+		nodeIndex: gci,

+		commit:    commit,

+	}, nil

 }

 

 // Commit returns the full Commit object representing the commit graph node.

@@ -194,8 +179,6 @@ func (gci *graphCommitNodeIndex) Commit(node CommitNode) (*Commit, error) {
 }

 

 // CommitTime returns the time when the commit was performed.

-//

-// CommitTime is present to fulfill the CommitNode interface.

 func (c *objectCommitNode) CommitTime() time.Time {

 	return c.commit.Committer.When

 }

@@ -210,35 +193,32 @@ func (c *objectCommitNode) Tree() (*Tree, error) {
 	return c.commit.Tree()

 }

 

-func NewObjectCommitNodeIndex(s storer.EncodedObjectStorer) CommitNodeIndex {

-	return &objectCommitNodeIndex{s}

-}

-

 // NumParents returns the number of parents in a commit.

-func (oci *objectCommitNodeIndex) NumParents(node CommitNode) int {

-	co := node.(*objectCommitNode)

-	return co.commit.NumParents()

+func (c *objectCommitNode) NumParents() int {

+	return c.commit.NumParents()

 }

 

 // ParentNodes return a CommitNodeIter for parents of specified node.

-func (oci *objectCommitNodeIndex) ParentNodes(node CommitNode) CommitNodeIter {

-	return newParentgraphCommitNodeIter(oci, node)

+func (c *objectCommitNode) ParentNodes() CommitNodeIter {

+	return newParentgraphCommitNodeIter(c)

 }

 

 // ParentNode returns the ith parent of a commit.

-func (oci *objectCommitNodeIndex) ParentNode(node CommitNode, i int) (CommitNode, error) {

-	co := node.(*objectCommitNode)

-	parent, err := co.commit.Parent(i)

-	if err != nil {

-		return nil, err

+func (c *objectCommitNode) ParentNode(i int) (CommitNode, error) {

+	if i < 0 || i >= len(c.commit.ParentHashes) {

+		return nil, ErrParentNotFound

 	}

-	return &objectCommitNode{commit: parent}, nil

+

+	return c.nodeIndex.Get(c.commit.ParentHashes[i])

 }

 

 // ParentHashes returns hashes of the parent commits for a specified node

-func (oci *objectCommitNodeIndex) ParentHashes(node CommitNode) []plumbing.Hash {

-	co := node.(*objectCommitNode)

-	return co.commit.ParentHashes

+func (c *objectCommitNode) ParentHashes() []plumbing.Hash {

+	return c.commit.ParentHashes

+}

+

+func NewObjectCommitNodeIndex(s storer.EncodedObjectStorer) CommitNodeIndex {

+	return &objectCommitNodeIndex{s}

 }

 

 // NodeFromHash looks up a commit node by it's object hash

@@ -248,7 +228,10 @@ func (oci *objectCommitNodeIndex) Get(hash plumbing.Hash) (CommitNode, error) {
 		return nil, err

 	}

 

-	return &objectCommitNode{commit: commit}, nil

+	return &objectCommitNode{

+		nodeIndex: oci,

+		commit:    commit,

+	}, nil

 }

 

 // Commit returns the full Commit object representing the commit graph node.

@@ -259,19 +242,18 @@ func (oci *objectCommitNodeIndex) Commit(node CommitNode) (*Commit, error) {
 

 // parentCommitNodeIter provides an iterator for parent commits from associated CommitNodeIndex.

 type parentCommitNodeIter struct {

-	gci  CommitNodeIndex

 	node CommitNode

 	i    int

 }

 

-func newParentgraphCommitNodeIter(gci CommitNodeIndex, node CommitNode) CommitNodeIter {

-	return &parentCommitNodeIter{gci, node, 0}

+func newParentgraphCommitNodeIter(node CommitNode) CommitNodeIter {

+	return &parentCommitNodeIter{node, 0}

 }

 

 // Next moves the iterator to the next commit and returns a pointer to it. If

 // there are no more commits, it returns io.EOF.

 func (iter *parentCommitNodeIter) Next() (CommitNode, error) {

-	obj, err := iter.gci.ParentNode(iter.node, iter.i)

+	obj, err := iter.node.ParentNode(iter.i)

 	if err == ErrParentNotFound {

 		return nil, io.EOF

 	}

diff --git a/plumbing/object/commitnode_test.go b/plumbing/object/commitnode_test.go
index 8f59665..883befc 100644
--- a/plumbing/object/commitnode_test.go
+++ b/plumbing/object/commitnode_test.go
@@ -25,7 +25,6 @@ func testWalker(c *C, nodeIndex CommitNodeIndex) {
 

 	iter := NewCommitNodeIterCTime(

 		head,

-		nodeIndex,

 		nil,

 		nil,

 	)

diff --git a/plumbing/object/commitnode_walker_ctime.go b/plumbing/object/commitnode_walker_ctime.go
index 86b6c57..e55b4ad 100644
--- a/plumbing/object/commitnode_walker_ctime.go
+++ b/plumbing/object/commitnode_walker_ctime.go
@@ -13,7 +13,6 @@ type commitNodeIteratorByCTime struct {
 	heap         *binaryheap.Heap

 	seenExternal map[plumbing.Hash]bool

 	seen         map[plumbing.Hash]bool

-	nodeIndex    CommitNodeIndex

 }

 

 // NewCommitNodeIterCTime returns a CommitNodeIter that walks the commit history,

@@ -26,7 +25,6 @@ type commitNodeIteratorByCTime struct {
 // commits from being iterated.

 func NewCommitNodeIterCTime(

 	c CommitNode,

-	nodeIndex CommitNodeIndex,

 	seenExternal map[plumbing.Hash]bool,

 	ignore []plumbing.Hash,

 ) CommitNodeIter {

@@ -48,7 +46,6 @@ func NewCommitNodeIterCTime(
 		heap:         heap,

 		seenExternal: seenExternal,

 		seen:         seen,

-		nodeIndex:    nodeIndex,

 	}

 }

 

@@ -68,11 +65,11 @@ func (w *commitNodeIteratorByCTime) Next() (CommitNode, error) {
 

 		w.seen[cID] = true

 

-		for i, h := range w.nodeIndex.ParentHashes(c) {

+		for i, h := range c.ParentHashes() {

 			if w.seen[h] || w.seenExternal[h] {

 				continue

 			}

-			pc, err := w.nodeIndex.ParentNode(c, i)

+			pc, err := c.ParentNode(i)

 			if err != nil {

 				return nil, err

 			}