From 2594f3f889f93196bc9707ca6962f4f773806f10 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Tue, 6 May 2014 14:19:34 +0200
Subject: [PATCH] Odb: use a callback instead of a channel for ForEach

A channel provides no way to specify whether we stopped sending data
because of an error or because there is no more data.

Therefore, make Odb.ForEach() take a callback with which the user is free to
do whatever they need, letting us return en error.
---
 odb.go      | 44 +++++++++++++++++++++++++++-----------------
 odb_test.go | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/odb.go b/odb.go
index abfa1cd..7076e20 100644
--- a/odb.go
+++ b/odb.go
@@ -84,30 +84,40 @@ func (v *Odb) Read(oid *Oid) (obj *OdbObject, err error) {
 	return obj, nil
 }
 
+type OdbForEachCallback func(id *Oid) error
+
+type foreachData struct {
+	callback OdbForEachCallback
+	err error
+}
+
 //export odbForEachCb
 func odbForEachCb(id *C.git_oid, payload unsafe.Pointer) int {
-	ch := *(*chan *Oid)(payload)
-	oid := newOidFromC(id)
-	// Because the channel is unbuffered, we never read our own data. If ch is
-	// readable, the user has sent something on it, which means we should
-	// abort.
-	select {
-	case ch <- oid:
-	case <-ch:
-		return -1
+	data := (*foreachData)(payload)
+
+	err := data.callback(newOidFromC(id))
+	if err != nil {
+		data.err = err
+		return C.GIT_EUSER
 	}
+
 	return 0
 }
 
-func (v *Odb) forEachWrap(ch chan *Oid) {
-	C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&ch))
-	close(ch)
-}
+func (v *Odb) ForEach(callback OdbForEachCallback) error {
+	data := foreachData {
+		callback: callback,
+		err: nil,
+	}
 
-func (v *Odb) ForEach() chan *Oid {
-	ch := make(chan *Oid, 0)
-	go v.forEachWrap(ch)
-	return ch
+	ret := C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&data))
+	if ret == C.GIT_EUSER {
+		return data.err
+	} else if ret < 0 {
+		return MakeGitError(ret)
+	}
+
+	return nil
 }
 
 // Hash determines the object-ID (sha1) of a data buffer.
diff --git a/odb_test.go b/odb_test.go
index 17b3ad2..14a3658 100644
--- a/odb_test.go
+++ b/odb_test.go
@@ -3,6 +3,7 @@ package git
 import (
 	"io"
 	"os"
+	"errors"
 	"testing"
 )
 
@@ -48,7 +49,7 @@ parent 66e1c476199ebcd3e304659992233132c5a52c6c
 author John Doe <john@doe.com> 1390682018 +0000
 committer John Doe <john@doe.com> 1390682018 +0000
 
-Initial commit.`;
+Initial commit.`
 
 	oid, error := odb.Hash([]byte(str), ObjectCommit)
 	checkFatal(t, error)
@@ -60,3 +61,36 @@ Initial commit.`;
 		t.Fatal("Hash and write Oids are different")
 	}
 }
+
+func TestOdbForeach(t *testing.T) {
+	repo := createTestRepo(t)
+	defer os.RemoveAll(repo.Workdir())
+	_, _ = seedTestRepo(t, repo)
+
+	odb, err := repo.Odb()
+	checkFatal(t, err)
+
+	expect := 3
+	count := 0
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return nil
+	})
+
+	checkFatal(t, err)
+	if count != expect {
+		t.Fatalf("Expected %v objects, got %v")
+	}
+
+	expect = 1
+	count = 0
+	to_return := errors.New("not really an error")
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return to_return
+	})
+
+	if err != to_return {
+		t.Fatalf("Odb.ForEach() did not return the expected error, got %v", err)
+	}
+}