Add file parameter to blame so it doesn't return all the results… (#988)

Add file parameter to blame so it doesn't return all the results from a commit

Author: ajnavarro

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Add progress for each partition in SHOW PROCESSLIST ([#855](https://github.com/src-d/go-mysql-server/pull/855))
+- Change BLAME to also take a file parameter.
 
 ## [0.24.0-rc3] - 2019-10-23
 

docs/using-gitbase/examples.md

@@ -203,15 +203,15 @@ The output will be similar to this:
 ## Get miscelaneous information about lines with a "// TODO" comment in HEAD
 
 ```sql
-SELECT repository_id,
-       JSON_UNQUOTE(JSON_EXTRACT(bl, "$.file")),
+SELECT repository_id, file_path,
        JSON_UNQUOTE(JSON_EXTRACT(bl, "$.linenum")),
        JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author")),
        JSON_UNQUOTE(JSON_EXTRACT(bl, "$.text"))
-FROM   (SELECT repository_id,
-               EXPLODE(BLAME(repository_id, commit_hash)) AS bl
+FROM   (SELECT repository_id, file_path,
+               EXPLODE(BLAME(repository_id, commit_hash, file_path)) AS bl
         FROM   ref_commits
                NATURAL JOIN blobs
+               NATURAL JOIN commit_files
         WHERE  ref_name = 'HEAD'
                AND NOT IS_BINARY(blob_content)
         ) as p
@@ -225,9 +225,10 @@ SELECT
        JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author")),
        COUNT(JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author")))
 
-FROM   (SELECT EXPLODE(BLAME(repository_id, commit_hash)) AS bl
+FROM   (SELECT EXPLODE(BLAME(repository_id, commit_hash, file_path)) AS bl
         FROM   ref_commits
                NATURAL JOIN blobs
+               NATURAL JOIN commit_files
         WHERE  ref_name = 'HEAD'
                AND NOT IS_BINARY(blob_content)
         ) AS p

docs/using-gitbase/functions.md

@@ -6,7 +6,7 @@ To make some common tasks easier for the user, there are some functions to inter
 
 |     Name     |                                               Description                                                                      |
 |:-------------|:-------------------------------------------------------------------------------------------------------------------------------|
-|`blame(repository, commit)`|Returns an array of lines changes and authorship.                                                                 |
+|`blame(repository, commit, file)`|Returns an array of lines changes and authorship for the specific file and commit.
 |`commit_file_stats(repository_id, [from_commit_hash], to_commit_hash) json array`|returns an array with the stats of each file in `to_commit_hash` since the given `from_commit_hash`. If `from_commit_hash` is not given, the parent commit will be used. Vendored files stats are not included in the result of this function. This function is more thoroughly explained later in this document.|
 |`commit_stats(repository_id, [from_commit_hash], to_commit_hash) json`|returns the stats between two commits for a repository. If `from_commit_hash` is empty, it will compare the given `to_commit_hash` with its parent commit. Vendored files stats are not included in the result of this function. This function is more thoroughly explained later in this document.|
 |`is_remote(reference_name)bool`| checks if the given reference name is from a remote one.                                                         |

integration_test.go

@@ -514,12 +514,12 @@ func TestIntegration(t *testing.T) {
 				SELECT repository_id, JSON_EXTRACT(bl, "$.author"),
 					   COUNT(bl)
 				FROM (
-					SELECT repository_id, EXPLODE(BLAME(repository_id, commit_hash)) as bl
+					SELECT repository_id, EXPLODE(BLAME(repository_id, commit_hash, '.gitignore')) as bl
 					FROM commits
 					WHERE  commit_hash = '918c48b83bd081e863dbe1b80f8998f058cd8294'
 				) as p
 			`,
-			[]sql.Row{{"worktree", "[email protected]", int64(7235)}},
+			[]sql.Row{{"worktree", "[email protected]", int64(12)}},
 		},
 	}
 

internal/function/blame.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"io"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/src-d/gitbase"
 	"github.com/src-d/go-mysql-server/sql"
 	"gopkg.in/src-d/go-git.v4"
@@ -17,55 +15,38 @@ import (
 type BlameGenerator struct {
 	ctx     *sql.Context
 	commit  *object.Commit
-	fIter   *object.FileIter
+	file    string
 	curLine int
-	curFile *object.File
 	lines   []*git.Line
 }
 
-func NewBlameGenerator(ctx *sql.Context, c *object.Commit, f *object.FileIter) (*BlameGenerator, error) {
-	return &BlameGenerator{ctx: ctx, commit: c, fIter: f, curLine: -1}, nil
-}
-
-func (g *BlameGenerator) loadNewFile() error {
-	var err error
-	g.curFile, err = g.fIter.Next()
-	if err != nil {
-		return err
-	}
-
-	result, err := git.Blame(g.commit, g.curFile.Name)
+func NewBlameGenerator(ctx *sql.Context, c *object.Commit, f string) (*BlameGenerator, error) {
+	result, err := git.Blame(c, f)
 	if err != nil {
-		msg := fmt.Sprintf(
-			"Error in BLAME for file %s: %s",
-			g.curFile.Name,
-			err.Error(),
-		)
-		logrus.Warn(msg)
-		g.ctx.Warn(0, msg)
-		return io.EOF
-	}
-
-	if len(result.Lines) == 0 {
-		return g.loadNewFile()
+		return nil, err
 	}
-
-	g.lines = result.Lines
-	g.curLine = 0
-	return nil
+	return &BlameGenerator{
+		ctx:     ctx,
+		commit:  c,
+		file:    f,
+		curLine: 0,
+		lines:   result.Lines,
+	}, nil
 }
 
 func (g *BlameGenerator) Next() (interface{}, error) {
-	if g.curLine == -1 || g.curLine >= len(g.lines) {
-		err := g.loadNewFile()
-		if err != nil {
-			return nil, err
-		}
+	select {
+	case <-g.ctx.Done():
+		return nil, io.EOF
+	default:
+	}
+
+	if len(g.lines) == 0 || g.curLine >= len(g.lines) {
+		return nil, io.EOF
 	}
 
 	l := g.lines[g.curLine]
 	b := BlameLine{
-		File:    g.curFile.Name,
 		LineNum: g.curLine,
 		Author:  l.Author,
 		Text:    l.Text,
@@ -75,7 +56,6 @@ func (g *BlameGenerator) Next() (interface{}, error) {
 }
 
 func (g *BlameGenerator) Close() error {
-	g.fIter.Close()
 	return nil
 }
 
@@ -86,20 +66,20 @@ type (
 	Blame struct {
 		repo   sql.Expression
 		commit sql.Expression
+		file   sql.Expression
 	}
 
 	// BlameLine represents each line of git blame's output
 	BlameLine struct {
-		File    string `json:"file"`
 		LineNum int    `json:"linenum"`
 		Author  string `json:"author"`
 		Text    string `json:"text"`
 	}
 )
 
 // NewBlame constructor
-func NewBlame(repo, commit sql.Expression) sql.Expression {
-	return &Blame{repo, commit}
+func NewBlame(repo, commit, file sql.Expression) sql.Expression {
+	return &Blame{repo, commit, file}
 }
 
 func (b *Blame) String() string {
@@ -112,26 +92,26 @@ func (*Blame) Type() sql.Type {
 }
 
 func (b *Blame) WithChildren(children ...sql.Expression) (sql.Expression, error) {
-	if len(children) != 2 {
+	if len(children) != 3 {
 		return nil, sql.ErrInvalidChildrenNumber.New(b, len(children), 2)
 	}
 
-	return NewBlame(children[0], children[1]), nil
+	return NewBlame(children[0], children[1], children[2]), nil
 }
 
 // Children implements the Expression interface.
 func (b *Blame) Children() []sql.Expression {
-	return []sql.Expression{b.repo, b.commit}
+	return []sql.Expression{b.repo, b.commit, b.file}
 }
 
 // IsNullable implements the Expression interface.
 func (b *Blame) IsNullable() bool {
-	return b.repo.IsNullable() || (b.commit.IsNullable())
+	return b.repo.IsNullable() || (b.commit.IsNullable()) || (b.file.IsNullable())
 }
 
 // Resolved implements the Expression interface.
 func (b *Blame) Resolved() bool {
-	return b.repo.Resolved() && b.commit.Resolved()
+	return b.repo.Resolved() && b.commit.Resolved() && b.file.Resolved()
 }
 
 // Eval implements the sql.Expression interface.
@@ -151,14 +131,16 @@ func (b *Blame) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 		return nil, nil
 	}
 
-	fIter, err := commit.Files()
+	file, err := exprToString(ctx, b.file, row)
 	if err != nil {
-		return nil, err
+		ctx.Warn(0, err.Error())
+		return nil, nil
 	}
 
-	bg, err := NewBlameGenerator(ctx, commit, fIter)
+	bg, err := NewBlameGenerator(ctx, commit, file)
 	if err != nil {
-		return nil, err
+		ctx.Warn(0, err.Error())
+		return nil, nil
 	}
 
 	return bg, nil

internal/function/blame_test.go

@@ -28,6 +28,7 @@ func TestBlameEval(t *testing.T) {
 		name        string
 		repo        sql.Expression
 		commit      sql.Expression
+		file        sql.Expression
 		row         sql.Row
 		expected    BlameLine
 		expectedNil bool
@@ -38,11 +39,11 @@ func TestBlameEval(t *testing.T) {
 			name:       "init commit",
 			repo:       expression.NewGetField(0, sql.Text, "repository_id", false),
 			commit:     expression.NewGetField(1, sql.Text, "commit_hash", false),
-			row:        sql.NewRow("worktree", "b029517f6300c2da0f4b651b8642506cd6aaf45d"),
+			file:       expression.NewGetField(2, sql.Text, "file", false),
+			row:        sql.NewRow("worktree", "b029517f6300c2da0f4b651b8642506cd6aaf45d", ".gitignore"),
 			testedLine: 0,
 			lineCount:  12,
 			expected: BlameLine{
-				".gitignore",
 				0,
 				"[email protected]",
 				"*.class",
@@ -53,11 +54,11 @@ func TestBlameEval(t *testing.T) {
 			name:       "changelog",
 			repo:       expression.NewGetField(0, sql.Text, "repository_id", false),
 			commit:     expression.NewGetField(1, sql.Text, "commit_hash", false),
-			row:        sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47"),
+			file:       expression.NewGetField(2, sql.Text, "file", false),
+			row:        sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47", "CHANGELOG"),
 			testedLine: 0,
 			lineCount:  1,
 			expected: BlameLine{
-				"CHANGELOG",
 				0,
 				"[email protected]",
 				"Initial changelog",
@@ -68,7 +69,8 @@ func TestBlameEval(t *testing.T) {
 			name:        "no repo",
 			repo:        expression.NewGetField(0, sql.Text, "repository_id", false),
 			commit:      expression.NewGetField(1, sql.Text, "commit_hash", false),
-			row:         sql.NewRow("foo", "bar"),
+			file:        expression.NewGetField(2, sql.Text, "file", false),
+			row:         sql.NewRow("foo", "bar", "baz"),
 			testedLine:  0,
 			lineCount:   1,
 			expected:    BlameLine{},
@@ -78,7 +80,19 @@ func TestBlameEval(t *testing.T) {
 			name:        "no commit",
 			repo:        expression.NewGetField(0, sql.Text, "repository_id", false),
 			commit:      expression.NewGetField(1, sql.Text, "commit_hash", false),
-			row:         sql.NewRow("worktree", "foo"),
+			file:        expression.NewGetField(2, sql.Text, "file", false),
+			row:         sql.NewRow("worktree", "foo", "bar"),
+			testedLine:  0,
+			lineCount:   1,
+			expected:    BlameLine{},
+			expectedNil: true,
+		},
+		{
+			name:        "no file",
+			repo:        expression.NewGetField(0, sql.Text, "repository_id", false),
+			commit:      expression.NewGetField(1, sql.Text, "commit_hash", false),
+			file:        expression.NewGetField(2, sql.Text, "file", false),
+			row:         sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47", "foo"),
 			testedLine:  0,
 			lineCount:   1,
 			expected:    BlameLine{},
@@ -88,7 +102,7 @@ func TestBlameEval(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			blame := NewBlame(tc.repo, tc.commit)
+			blame := NewBlame(tc.repo, tc.commit, tc.file)
 			blameGen, err := blame.Eval(ctx, tc.row)
 			require.NoError(t, err)
 
@@ -105,9 +119,6 @@ func TestBlameEval(t *testing.T) {
 			lineCount := 0
 			for i, err := bg.Next(); err == nil; i, err = bg.Next() {
 				i := i.(BlameLine)
-				if i.File != tc.expected.File {
-					continue
-				}
 				if lineCount != tc.testedLine {
 					lineCount++
 					continue

internal/function/registry.go

@@ -17,5 +17,5 @@ var Functions = []sql.Function{
 	sql.Function1{Name: "uast_children", Fn: NewUASTChildren},
 	sql.Function1{Name: "uast_imports", Fn: NewUASTImports},
 	sql.Function1{Name: "is_vendor", Fn: NewIsVendor},
-	sql.Function2{Name: "blame", Fn: NewBlame},
+	sql.Function3{Name: "blame", Fn: NewBlame},
 }