improve commit stats (#864)

improve commit stats

Author: ajnavarro

CHANGELOG.md

@@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Added documentation about `commit_stats`.
+
+### Changed
+
+- internal/function: gracefully handle errors in commit_stats.
+
 ### Fixed
 
 - internal/function: take into account if repository is resolved in commit_stats ([#863](https://github.com/src-d/gitbase/pull/863))

docs/using-gitbase/functions.md

@@ -6,6 +6,7 @@ To make some common tasks easier for the user, there are some functions to inter
 
 |     Name     |                                               Description                                                                      |
 |:-------------|:-------------------------------------------------------------------------------------------------------------------------------|
+|`commit_stats(repository_id, [from_commit_hash], to_commit_hash)`|returns the stats between two commits for a repository. If from is empty, it will compare the given `to_commit_hash` with its parent commit|
 |`is_remote(reference_name)bool`| check if the given reference name is from a remote one                                                          |
 |`is_tag(reference_name)bool`| check if the given reference name is a tag                                                                         |
 |`is_vendor(file_path)bool`| check if the given file name is a vendored file                                                                         |

internal/function/commit_stats.go

@@ -3,6 +3,7 @@ package function
 import (
 	"fmt"
 
+	"github.com/sirupsen/logrus"
 	"github.com/src-d/gitbase"
 	"github.com/src-d/gitbase/internal/commitstats"
 
@@ -81,7 +82,7 @@ func (f *CommitStats) Children() []sql.Expression {
 
 // IsNullable implements the Expression interface.
 func (*CommitStats) IsNullable() bool {
-	return false
+	return true
 }
 
 // Resolved implements the Expression interface.
@@ -98,20 +99,39 @@ func (f *CommitStats) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 
 	r, err := f.resolveRepo(ctx, row)
 	if err != nil {
-		return nil, err
+		ctx.Warn(0, "commit_stats: unable to resolve repository")
+		logrus.WithField("err", err).Error("commit_stats: unable to resolve repository")
+		return nil, nil
 	}
 
+	log := logrus.WithField("repository", r)
+
 	to, err := f.resolveCommit(ctx, r, row, f.To)
 	if err != nil {
-		return nil, err
+		ctx.Warn(0, "commit_stats: unable to resolve 'to' commit of repository: %v", r)
+		log.WithField("err", err).Error("commit_stats: unable to resolve 'to' commit")
+		return nil, nil
 	}
 
 	from, err := f.resolveCommit(ctx, r, row, f.From)
 	if err != nil {
-		return nil, err
+		ctx.Warn(0, "commit_stats: unable to resolve 'from' commit of repository: %v", r)
+		log.WithField("err", err).Error("commit_stats: unable to resolve from commit")
+		return nil, nil
+	}
+
+	result, err := commitstats.Calculate(r.Repository, from, to)
+	if err != nil {
+		ctx.Warn(0, "commit_stats: unable to calculate for repository: %v, from: %v, to: %v", r, from, to)
+		log.WithFields(logrus.Fields{
+			"err":  err,
+			"from": from,
+			"to":   to,
+		}).Error("commit_stats: unable to calculate")
+		return nil, nil
 	}
 
-	return commitstats.Calculate(r.Repository, from, to)
+	return result, nil
 }
 
 func (f *CommitStats) resolveRepo(ctx *sql.Context, r sql.Row) (*gitbase.Repository, error) {

internal/function/commit_stats_test.go

@@ -8,10 +8,10 @@ import (
 	"github.com/src-d/gitbase/internal/commitstats"
 	"github.com/stretchr/testify/require"
 
-	"gopkg.in/src-d/go-git-fixtures.v3"
-	"gopkg.in/src-d/go-git.v4/plumbing/cache"
 	"github.com/src-d/go-mysql-server/sql"
 	"github.com/src-d/go-mysql-server/sql/expression"
+	fixtures "gopkg.in/src-d/go-git-fixtures.v3"
+	"gopkg.in/src-d/go-git.v4/plumbing/cache"
 )
 
 func TestCommitStatsEval(t *testing.T) {
@@ -34,7 +34,7 @@ func TestCommitStatsEval(t *testing.T) {
 		from     sql.Expression
 		to       sql.Expression
 		row      sql.Row
-		expected *commitstats.CommitStats
+		expected interface{}
 	}{
 		{
 			name: "init commit",
@@ -48,6 +48,30 @@ func TestCommitStatsEval(t *testing.T) {
 				Total: commitstats.KindStats{Additions: 22, Deletions: 0},
 			},
 		},
+		{
+			name:     "invalid repository id",
+			repo:     expression.NewGetField(0, sql.Text, "repository_id", false),
+			from:     nil,
+			to:       expression.NewGetField(1, sql.Text, "commit_hash", false),
+			row:      sql.NewRow("foobar", "b029517f6300c2da0f4b651b8642506cd6aaf45d"),
+			expected: nil,
+		},
+		{
+			name:     "invalid to",
+			repo:     expression.NewGetField(0, sql.Text, "repository_id", false),
+			from:     nil,
+			to:       expression.NewGetField(1, sql.Text, "commit_hash", false),
+			row:      sql.NewRow("worktree", "foobar"),
+			expected: nil,
+		},
+		{
+			name:     "invalid from",
+			repo:     expression.NewGetField(0, sql.Text, "repository_id", false),
+			from:     expression.NewGetField(2, sql.Text, "commit_hash", false),
+			to:       expression.NewGetField(1, sql.Text, "commit_hash", false),
+			row:      sql.NewRow("worktree", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "foobar"),
+			expected: nil,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -58,10 +82,7 @@ func TestCommitStatsEval(t *testing.T) {
 			result, err := diff.Eval(ctx, tc.row)
 			require.NoError(t, err)
 
-			stats, ok := result.(*commitstats.CommitStats)
-			require.True(t, ok)
-
-			require.EqualValues(t, tc.expected, stats)
+			require.EqualValues(t, tc.expected, result)
 		})
 	}
 }