Fixing TODO comments in code /cc @oliveiradan

1. server/login.go:49 (// TODO(bradrydzewski) return an error message instead). Added error message if authorization fails.
2. server/repos.go:178 (TODO(bradrydzewski) verify repo not exists). Added a checking for the repo and return an error in case it does not exist.
3. server/queue.go:170:  // TODO (bradrydzewski) change this interface to accept an io.Reader. All references to the API change been in question SetLogs() have been modified.
4. remote/github/github.go:106  // Fixed a crash in case *repo_.Language is nil , when de-referencing it. This could happen when a repo only has a readme, so github hasn't set the language yet.
5. ./server/queue.go:170:  // TODO (bradrydzewski) change this interface to accept an io.Reader. All references to the API change been in question SetLogs() have been modified.
6. .remote/github/github.go:106  // Fixed a crash in case *repo_.Language is nil , when de-referencing it. This could happen when a repo only has a readme, so github hasn't set the language yet.

datastore/builtin/repo_test.go

@@ -1,6 +1,7 @@
 package builtin
 
 import (
+	"bytes"
 	"github.com/drone/drone/common"
 	. "github.com/franela/goblin"
 	"io/ioutil"
@@ -63,7 +64,7 @@ func TestRepo(t *testing.T) {
 			db.SetBuild(testRepo, &common.Build{State: "success"})
 			db.SetBuild(testRepo, &common.Build{State: "success"})
 			db.SetBuild(testRepo, &common.Build{State: "pending"})
-			db.SetLogs(testRepo, 1, 1, []byte("foo"))
+			db.SetLogs(testRepo, 1, 1, (bytes.NewBuffer([]byte("foo"))))
 
 			// first a little sanity to validate our test conditions
 			_, err = db.BuildLast(testRepo)

datastore/builtin/task.go

@@ -2,19 +2,27 @@ package builtin
 
 import (
 	"bytes"
-	"github.com/boltdb/bolt"
 	"io"
+	"io/ioutil"
 	"strconv"
+
+	"github.com/boltdb/bolt"
 )
 
 // SetLogs inserts or updates a task logs for the
 // named repository and build number.
-func (db *DB) SetLogs(repo string, build int, task int, log []byte) error {
+func (db *DB) SetLogs(repo string, build int, task int, rd io.Reader) error {
 	key := []byte(repo + "/" + strconv.Itoa(build) + "/" + strconv.Itoa(task))
 	t, err := db.Begin(true)
 	if err != nil {
 		return err
 	}
+
+	log, err := ioutil.ReadAll(rd)
+	if err != nil {
+		return err
+	}
+
 	err = t.Bucket(bucketBuildLogs).Put(key, log)
 	if err != nil {
 		t.Rollback()

datastore/builtin/task_test.go

@@ -1,6 +1,7 @@
 package builtin
 
 import (
+	"bytes"
 	"github.com/drone/drone/common"
 	. "github.com/franela/goblin"
 	"io/ioutil"
@@ -30,13 +31,13 @@ func TestTask(t *testing.T) {
 
 		g.It("Should set Logs", func() {
 			db.SetRepo(&common.Repo{FullName: testRepo})
-			err := db.SetLogs(testRepo, testBuild, testTask, testLogInfo)
+			err := db.SetLogs(testRepo, testBuild, testTask, (bytes.NewBuffer(testLogInfo)))
 			g.Assert(err).Equal(nil)
 		})
 
 		g.It("Should get logs", func() {
 			db.SetRepo(&common.Repo{FullName: testRepo})
-			db.SetLogs(testRepo, testBuild, testTask, testLogInfo)
+			db.SetLogs(testRepo, testBuild, testTask, (bytes.NewBuffer(testLogInfo)))
 			buf, err := db.LogReader(testRepo, testBuild, testTask)
 			g.Assert(err).Equal(nil)
 			logInfo, err := ioutil.ReadAll(buf)

datastore/datastore.go

@@ -139,5 +139,6 @@ type Datastore interface {
 
 	// SetLogs inserts or updates a task logs for the
 	// named repository and build number.
-	SetLogs(string, int, int, []byte) error
+	SetLogs(string, int, int, io.Reader) error
 }
+

datastore/mock/mock.go

@@ -291,7 +291,8 @@ func (m *Datastore) LogReader(_a0 string, _a1 int, _a2 int) (io.Reader, error) {
 
 	return r0, r1
 }
-func (m *Datastore) SetLogs(_a0 string, _a1 int, _a2 int, _a3 []byte) error {
+
+func (m *Datastore) SetLogs(_a0 string, _a1 int, _a2 int, _a3 io.Reader) error {
 	ret := m.Called(_a0, _a1, _a2, _a3)
 
 	r0 := ret.Error(0)

remote/github/github.go

@@ -103,7 +103,10 @@ func (g *GitHub) Repo(u *common.User, owner, name string) (*common.Repo, error)
 	repo := &common.Repo{}
 	repo.Owner = owner
 	repo.Name = name
-	repo.Language = *repo_.Language
+
+	if repo_.Language != nil {
+		repo.Language = *repo_.Language
+	}
 	repo.FullName = *repo_.FullName
 	repo.Link = *repo_.HTMLURL
 	repo.Private = *repo_.Private

runner/builtin/updater.go

@@ -3,7 +3,7 @@ package builtin
 import (
 	"encoding/json"
 	"io"
-	"io/ioutil"
+	//"io/ioutil"
 
 	"github.com/drone/drone/common"
 	"github.com/drone/drone/datastore"
@@ -77,10 +77,11 @@ func (u *updater) SetTask(r *common.Repo, b *common.Build, t *common.Task) error
 }
 
 func (u *updater) SetLogs(r *common.Repo, b *common.Build, t *common.Task, rc io.ReadCloser) error {
-	defer rc.Close()
+	//defer rc.Close()
-	out, err := ioutil.ReadAll(rc)
+	//out, err := ioutil.ReadAll(rc)
-	if err != nil {
+	//if err != nil {
-		return err
+	//	return err
-	}
+	//}
-	return u.store.SetLogs(r.FullName, b.Number, t.Number, out)
+	//return u.store.SetLogs(r.FullName, b.Number, t.Number, out)
+	return u.store.SetLogs(r.FullName, b.Number, t.Number, rc)
 }

server/login.go

@@ -45,8 +45,8 @@ func GetLogin(c *gin.Context) {
 	}
 
 	// exit if authorization fails
-	// TODO(bradrydzewski) return an error message instead
 	if c.Writer.Status() != 200 {
+		log.Errorf("authorization failed.")
 		return
 	}
 

server/queue.go

@@ -3,7 +3,6 @@ package server
 import (
 	"encoding/json"
 	"io"
-	"io/ioutil"
 	"net"
 	"strconv"
 
@@ -167,16 +166,8 @@ func PushLogs(c *gin.Context) {
 	bnum, _ := strconv.Atoi(c.Params.ByName("build"))
 	tnum, _ := strconv.Atoi(c.Params.ByName("task"))
 
-	// TODO (bradrydzewski) change this interface to accept an io.Reader
+	const maxBuffToRead int64 = 5000000 // 5MB.
-	// instead of a byte array so that we can buffer the write and so that
+	err := store.SetLogs(repo.FullName, bnum, tnum, io.LimitReader(c.Request.Body, maxBuffToRead))
-	// we avoid unnecessary copies of the data in memory.
-	logs, err := ioutil.ReadAll(io.LimitReader(c.Request.Body, 5000000)) //5MB
-	defer c.Request.Body.Close()
-	if err != nil {
-		c.Fail(500, err)
-		return
-	}
-	err = store.SetLogs(repo.FullName, bnum, tnum, logs)
 	if err != nil {
 		c.Fail(500, err)
 		return

server/repos.go

@@ -171,7 +171,11 @@ func PostRepo(c *gin.Context) {
 	owner := c.Params.ByName("owner")
 	name := c.Params.ByName("name")
 
-	// TODO(bradrydzewski) verify repo not exists
+	_, err := store.Repo(owner + "/" + name)
+	if err == nil {
+		c.String(409, "Repository already exists")
+		return
+	}
 
 	// get the repository and user permissions
 	// from the remote system.