Write analysis to the database

This provides the foundations for a web view (discussed in #28),
so users can view the issues without relying on inline or issue
comments.

This also provides a mechanism to time how long the analysis is
taking as well as a per tool timing metric. This allows us to better
understand how much time different repositories or installations spend
checking.

We record which tools were executed, even if no issues were detected
so the user knows which checks passed. This will cause the analysis_tool
table to become very large over time.

Relates to #3.

internal/analyser/analyser.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"log"
 	"strings"
+	"time"
 
 	"github.com/bradleyfalzon/gopherci/internal/db"
 	"github.com/bradleyfalzon/revgrep"
@@ -45,16 +46,6 @@ type Config struct {
 	GoSrcPath string
 }
 
-// Issue contains file, position and string describing a single issue.
-type Issue struct {
-	// File is the relative path name of the file.
-	File string
-	// HunkPos is the position relative to the files first hunk.
-	HunkPos int
-	// Issue is the issue.
-	Issue string
-}
-
 // Executer executes a single command in a contained environment.
 type Executer interface {
 	// Execute executes a command and returns the combined stdout and stderr,
@@ -92,18 +83,23 @@ const (
 )
 
 // Analyse downloads a repository set in config in an environment provided by
-// analyser, running the series of tools. Returns issues from tools that have
-// are likely to have been caused by a change.
-func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Config) ([]Issue, error) {
+// analyser, running the series of tools. Returns analysis from tools that have
+// are likely to have been caused by a change, or an error.
+func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Config) (*db.Analysis, error) {
 	// Get a new executer/environment to execute in
 	exec, err := analyser.NewExecuter(ctx, config.GoSrcPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "analyser could create new executer")
 	}
 
-	// baseRef is the reference to the base branch or before commit, the ref
-	// of the state before this PR/Push.
-	var baseRef string
+	var (
+		// baseRef is the reference to the base branch or before commit, the ref
+		// of the state before this PR/Push.
+		baseRef    string
+		start      = time.Now() // start of entire analysis
+		deltaStart = time.Now() // start of specific analysis
+		analysis   = db.NewAnalysis()
+	)
 	switch config.EventType {
 	case EventTypePullRequest:
 		// clone repo
@@ -141,6 +137,7 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 	default:
 		return nil, errors.Errorf("unknown event type %T", config.EventType)
 	}
+	analysis.CloneDuration = time.Since(deltaStart)
 
 	// create a unified diff for use by revgrep
 	args := []string{"git", "diff", fmt.Sprintf("%v...%v", baseRef, config.HeadRef)}
@@ -150,11 +147,13 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 	}
 
 	// install dependencies, some static analysis tools require building a project
+	deltaStart = time.Now()
 	args = []string{"install-deps.sh"}
 	out, err := exec.Execute(ctx, args)
 	if err != nil {
 		return nil, fmt.Errorf("could not execute %v: %s\n%s", args, err, out)
 	}
+	analysis.DepsDuration = time.Since(deltaStart)
 	log.Printf("install-deps.sh output: %s", bytes.TrimSpace(out))
 
 	// get the base package working directory, used by revgrep to change absolute
@@ -167,8 +166,8 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 	}
 	pwd := string(bytes.TrimSpace(out))
 
-	var issues []Issue
 	for _, tool := range tools {
+		deltaStart = time.Now()
 		args := []string{tool.Path}
 		for _, arg := range strings.Fields(tool.Args) {
 			switch arg {
@@ -199,6 +198,7 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 		}
 		log.Printf("revgrep found %v issues", len(revIssues))
 
+		var issues []db.Issue
 		for _, issue := range revIssues {
 			// Remove issues in generated files, isFileGenereated will return
 			// 0 for file is generated or 1 for file is not generated.
@@ -215,12 +215,18 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 				return nil, fmt.Errorf("could not execute %v: %s\n%s", args, err, out)
 			}
 
-			issues = append(issues, Issue{
-				File:    issue.File,
+			issues = append(issues, db.Issue{
+				Path:    issue.File,
+				Line:    issue.LineNo,
 				HunkPos: issue.HunkPos,
 				Issue:   fmt.Sprintf("%s: %s", tool.Name, issue.Message),
 			})
 		}
+
+		analysis.Tools[tool.ID] = db.AnalysisTool{
+			Duration: time.Since(deltaStart),
+			Issues:   issues,
+		}
 	}
 
 	log.Printf("stopping executer")
@@ -229,5 +235,6 @@ func Analyse(ctx context.Context, analyser Analyser, tools []db.Tool, config Con
 	}
 	log.Printf("finished stopping executer")
 
-	return issues, nil
+	analysis.TotalDuration = time.Since(start)
+	return analysis, nil
 }

internal/analyser/analyser_test.go

@@ -46,9 +46,9 @@ func TestAnalyse_pr(t *testing.T) {
 	}
 
 	tools := []db.Tool{
-		{Name: "Name1", Path: "tool1", Args: "-flag %BASE_BRANCH% ./..."},
-		{Name: "Name2", Path: "tool2"},
-		{Name: "Name2", Path: "tool3"},
+		{ID: 1, Name: "Name1", Path: "tool1", Args: "-flag %BASE_BRANCH% ./..."},
+		{ID: 2, Name: "Name2", Path: "tool2"},
+		{ID: 3, Name: "Name2", Path: "tool3"},
 	}
 
 	diff := []byte(`diff --git a/subdir/main.go b/subdir/main.go
@@ -88,17 +88,23 @@ index 0000000..6362395
 		},
 	}
 
-	issues, err := Analyse(context.Background(), analyser, tools, cfg)
+	analysis, err := Analyse(context.Background(), analyser, tools, cfg)
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	expected := []Issue{
-		{File: "main.go", HunkPos: 1, Issue: "Name1: error1"},
-		{File: "main.go", HunkPos: 1, Issue: "Name2: error2"},
+	want := map[db.ToolID][]db.Issue{
+		1: []db.Issue{{Path: "main.go", Line: 1, HunkPos: 1, Issue: "Name1: error1"}},
+		2: []db.Issue{{Path: "main.go", Line: 1, HunkPos: 1, Issue: "Name2: error2"}},
+		3: nil,
 	}
-	if !reflect.DeepEqual(expected, issues) {
-		t.Errorf("expected issues:\n%+v\ngot:\n%+v", expected, issues)
+	for toolID, issues := range want {
+		if have := analysis.Tools[toolID].Issues; !reflect.DeepEqual(issues, have) {
+			t.Errorf("unexpected issues for toolID %v\nwant: %+v\nhave: %+v", toolID, issues, have)
+		}
+	}
+	if len(analysis.Tools) != len(want) {
+		t.Errorf("analysis has %v tools want %v", len(analysis.Tools), len(want))
 	}
 
 	if !analyser.Stopped {
@@ -134,9 +140,9 @@ func TestAnalyse_push(t *testing.T) {
 	}
 
 	tools := []db.Tool{
-		{Name: "Name1", Path: "tool1", Args: "-flag %BASE_BRANCH% ./..."},
-		{Name: "Name2", Path: "tool2"},
-		{Name: "Name2", Path: "tool3"},
+		{ID: 1, Name: "Name1", Path: "tool1", Args: "-flag %BASE_BRANCH% ./..."},
+		{ID: 2, Name: "Name2", Path: "tool2"},
+		{ID: 3, Name: "Name2", Path: "tool3"},
 	}
 
 	diff := []byte(`diff --git a/subdir/main.go b/subdir/main.go
@@ -176,17 +182,23 @@ index 0000000..6362395
 		},
 	}
 
-	issues, err := Analyse(context.Background(), analyser, tools, cfg)
+	analysis, err := Analyse(context.Background(), analyser, tools, cfg)
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	expected := []Issue{
-		{File: "main.go", HunkPos: 1, Issue: "Name1: error1"},
-		{File: "main.go", HunkPos: 1, Issue: "Name2: error2"},
+	want := map[db.ToolID][]db.Issue{
+		1: []db.Issue{{Path: "main.go", Line: 1, HunkPos: 1, Issue: "Name1: error1"}},
+		2: []db.Issue{{Path: "main.go", Line: 1, HunkPos: 1, Issue: "Name2: error2"}},
+		3: nil,
+	}
+	for toolID, issues := range want {
+		if have := analysis.Tools[toolID].Issues; !reflect.DeepEqual(issues, have) {
+			t.Errorf("unexpected issues for toolID %v\nwant: %+v\nhave: %+v", toolID, issues, have)
+		}
 	}
-	if !reflect.DeepEqual(expected, issues) {
-		t.Errorf("expected issues:\n%+v\ngot:\n%+v", expected, issues)
+	if len(analysis.Tools) != len(want) {
+		t.Errorf("analysis has %v tools want %v", len(analysis.Tools), len(want))
 	}
 
 	if !analyser.Stopped {

internal/db/db.go

@@ -14,9 +14,27 @@ type DB interface {
 	// ListTools returns all tools. Returns nil if no tools were found, error will
 	// be non-nil if an error occurs.
 	ListTools() ([]Tool, error)
+	// StartAnalysis records a new analysis.
+	StartAnalysis(ghInstallationID, repositoryID int) (analysisID int, err error)
+	// FinishAnalysis marks a status as finished.
+	FinishAnalysis(analysisID int, status AnalysisStatus, analysis *Analysis) error
+	// ErrorAnalysis marks a status as an (internal) error.
+	//ErrorAnalysis(analysisID int) error
 }
 
+// AnalysisStatus represents a status in the builds table.
+type AnalysisStatus string
+
+const (
+	AnalysisStatusPending AnalysisStatus = "Pending" // AnalysisStatusPending is a pending/started analysis.
+	AnalysisStatusFailure AnalysisStatus = "Failure" // AnalysisStatusFailure is an analysis marked as failed.
+	AnalysisStatusSuccess AnalysisStatus = "Success" // AnalysisStatusSuccess is an analysis marked as success.
+	AnalysisStatusError   AnalysisStatus = "Error"   // AnalysisStatusError is an analysis with internal error.
+)
+
+// GHInstallation represents a row from the gh_installations table.
 type GHInstallation struct {
+	ID             int
 	InstallationID int
 	AccountID      int
 	SenderID       int
@@ -28,12 +46,63 @@ func (i GHInstallation) IsEnabled() bool {
 	return i.enabledAt.Before(time.Now()) && !i.enabledAt.IsZero()
 }
 
-// Tool represents a single tool
+// ToolID is the primary key on the tools table.
+type ToolID int
+
+// Tool represents a single tool in the tools table.
 type Tool struct {
-	ID     int    `db:"id"`
+	ID     ToolID `db:"id"`
 	Name   string `db:"name"`
 	URL    string `db:"url"`
 	Path   string `db:"path"`
 	Args   string `db:"args"`
 	Regexp string `db:"regexp"`
 }
+
+// Analysis represents a single analysis of a repository at a point in time.
+type Analysis struct {
+	ID               int            `db:"id"`
+	GHInstallationID int            `db:"gh_installation_id"`
+	RepositoryID     int            `db:"repository_id"`
+	Status           AnalysisStatus `db:"status"`
+
+	// When an analysis is finished
+	CloneDuration time.Duration `db:"clone_duration"` // CloneDuration is the wall clock time taken to run clone.
+	DepsDuration  time.Duration `db:"deps_duration"`  // DepsDuration is the wall clock time taken to fetch dependencies.
+	TotalDuration time.Duration `db:"total_duration"` // TotalDuration is the wall clock time taken for the entire analysis.
+	Tools         map[ToolID]AnalysisTool
+}
+
+// NewAnalysis returns a ready to use analysis.
+func NewAnalysis() *Analysis {
+	return &Analysis{
+		Tools: make(map[ToolID]AnalysisTool),
+	}
+}
+
+// Issues returns all the issues by each tool as a slice.
+func (a *Analysis) Issues() []Issue {
+	var issues []Issue
+	for _, tool := range a.Tools {
+		issues = append(issues, tool.Issues...)
+	}
+	return issues
+}
+
+// AnalysisTool contains the timing and result of an individual tool's analysis.
+type AnalysisTool struct {
+	Duration time.Duration // Duration is the wall clock time taken to run the tool.
+	Issues   []Issue       // Issues maybe nil if no issues found.
+}
+
+// Issue contains file, position and string describing a single issue.
+type Issue struct {
+	// Path is the relative path name of the file.
+	Path string
+	// Line is the line number of the file.
+	Line int
+	// HunkPos is the position relative to the files first hunk.
+	HunkPos int
+	// Issue is the issue.
+	Issue string // maybe this should be issue
+}

internal/db/mockdb.go

@@ -60,3 +60,13 @@ func (db *MockDB) GetGHInstallation(installationID int) (*GHInstallation, error)
 func (db *MockDB) ListTools() ([]Tool, error) {
 	return db.Tools, nil
 }
+
+// StartAnalysis implements the DB interface.
+func (db *MockDB) StartAnalysis(ghInstallationID, repositoryID int) (int, error) {
+	return 0, nil
+}
+
+// FinishAnalysis implements the DB interface.
+func (db *MockDB) FinishAnalysis(analysisID int, status AnalysisStatus, analysis *Analysis) error {
+	return nil
+}