refactor: refactor pull command, move issue pull and compute to pkg/repo

cmd/cmd_airtable.go

@@ -50,19 +50,26 @@ func (cmd *airtableCommand) ParseFlags(flags *pflag.FlagSet) {
 
 	flags.StringVarP(&cmd.opts.IssuesTableName, "airtable-issues-table-name", "", "Issues and PRs", "Airtable issues table name")
 	cmd.opts.TableNames[airtabledb.IssueIndex] = cmd.opts.IssuesTableName
+
 	flags.StringVarP(&cmd.opts.RepositoriesTableName, "airtable-repositories-table-name", "", "Repositories", "Airtable repositories table name")
 	cmd.opts.TableNames[airtabledb.RepositoryIndex] = cmd.opts.RepositoriesTableName
+
 	flags.StringVarP(&cmd.opts.AccountsTableName, "airtable-accounts-table-name", "", "Accounts", "Airtable accounts table name")
 	cmd.opts.TableNames[airtabledb.AccountIndex] = cmd.opts.AccountsTableName
+
 	flags.StringVarP(&cmd.opts.LabelsTableName, "airtable-labels-table-name", "", "Labels", "Airtable labels table name")
 	cmd.opts.TableNames[airtabledb.LabelIndex] = cmd.opts.LabelsTableName
+
 	flags.StringVarP(&cmd.opts.MilestonesTableName, "airtable-milestones-table-name", "", "Milestones", "Airtable milestones table nfame")
 	cmd.opts.TableNames[airtabledb.MilestoneIndex] = cmd.opts.MilestonesTableName
+
 	flags.StringVarP(&cmd.opts.ProvidersTableName, "airtable-providers-table-name", "", "Providers", "Airtable providers table name")
 	cmd.opts.TableNames[airtabledb.ProviderIndex] = cmd.opts.ProvidersTableName
+
 	flags.StringVarP(&cmd.opts.BaseID, "airtable-base-id", "", "", "Airtable base ID")
 	flags.StringVarP(&cmd.opts.Token, "airtable-token", "", "", "Airtable token")
 	flags.BoolVarP(&cmd.opts.DestroyInvalidRecords, "airtable-destroy-invalid-records", "", false, "Destroy invalid records")
+
 	viper.BindPFlags(flags)
 }
 
@@ -90,6 +97,8 @@ func (cmd *airtableCommand) airtableSyncCommand() *cobra.Command {
 	return cc
 }
 
+// airtableSync pushes issue info to the airtable base specified in opts.
+// Repository info is loaded from the targets specified in opts.
 func airtableSync(opts *airtableOptions) error {
 	if opts.BaseID == "" || opts.Token == "" {
 		return fmt.Errorf("missing token or baseid, check '-h'")
@@ -99,79 +108,77 @@ func airtableSync(opts *airtableOptions) error {
 	// prepare
 	//
 
-	// load issues
-	issues, err := loadIssues(nil)
+	issues, err := repo.LoadIssues(db, nil)
 	if err != nil {
 		return errors.Wrap(err, "failed to load issues")
 	}
-	filtered := issues.FilterByTargets(opts.Targets)
-	zap.L().Debug("fetch db entries", zap.Int("count", len(filtered)))
+	issues = issues.FilterByTargets(opts.Targets)
+	zap.L().Debug("fetch db entries", zap.Int("count", len(issues)))
 
-	// unique entries
-	features := make([]map[string]repo.Feature, airtabledb.NumTables)
-	for i, _ := range features {
-		features[i] = make(map[string]repo.Feature)
+	issueFeatures := make([]map[string]repo.IssueFeature, airtabledb.NumTables)
+	for i, _ := range issueFeatures {
+		issueFeatures[i] = make(map[string]repo.IssueFeature)
 	}
 
-	for _, issue := range filtered {
+	// Parse the loaded issues into the issueFeature map.
+	for _, issue := range issues {
 		// providers
-		features[airtabledb.ProviderIndex][issue.Repository.Provider.ID] = issue.Repository.Provider
+		issueFeatures[airtabledb.ProviderIndex][issue.Repository.Provider.ID] = issue.Repository.Provider
 
 		// labels
 		for _, label := range issue.Labels {
-			features[airtabledb.LabelIndex][label.ID] = label
+			issueFeatures[airtabledb.LabelIndex][label.ID] = label
 		}
 
 		// accounts
 		if issue.Repository.Owner != nil {
-			features[airtabledb.AccountIndex][issue.Repository.Owner.ID] = issue.Repository.Owner
+			issueFeatures[airtabledb.AccountIndex][issue.Repository.Owner.ID] = issue.Repository.Owner
 		}
 
-		features[airtabledb.AccountIndex][issue.Author.ID] = issue.Author
+		issueFeatures[airtabledb.AccountIndex][issue.Author.ID] = issue.Author
 		for _, assignee := range issue.Assignees {
-			features[airtabledb.AccountIndex][assignee.ID] = assignee
+			issueFeatures[airtabledb.AccountIndex][assignee.ID] = assignee
 		}
 		if issue.Milestone != nil && issue.Milestone.Creator != nil {
-			features[airtabledb.AccountIndex][issue.Milestone.Creator.ID] = issue.Milestone.Creator
+			issueFeatures[airtabledb.AccountIndex][issue.Milestone.Creator.ID] = issue.Milestone.Creator
 		}
 
 		// repositories
-		features[airtabledb.RepositoryIndex][issue.Repository.ID] = issue.Repository
+		issueFeatures[airtabledb.RepositoryIndex][issue.Repository.ID] = issue.Repository
 		// FIXME: find external repositories based on depends-on links
 
 		// milestones
 		if issue.Milestone != nil {
-			features[airtabledb.MilestoneIndex][issue.Milestone.ID] = issue.Milestone
+			issueFeatures[airtabledb.MilestoneIndex][issue.Milestone.ID] = issue.Milestone
 		}
 
 		// issue
-		features[airtabledb.IssueIndex][issue.ID] = issue
+		issueFeatures[airtabledb.IssueIndex][issue.ID] = issue
 		// FIXME: find external issues based on depends-on links
 	}
 
-	// init client
-	at := airtable.Client{
+	client := airtable.Client{
 		APIKey:  opts.Token,
 		BaseID:  opts.BaseID,
 		Limiter: airtable.RateLimiter(5),
 	}
 
-	// fetch remote data
+	// cache stores issueFeatures inserted into the airtable base.
 	cache := airtabledb.NewDB()
+
+	// Store already existing issueFeatures into the cache.
 	for tableKind, tableName := range opts.TableNames {
-		table := at.Table(tableName)
+		table := client.Table(tableName)
 		if err := cache.Tables[tableKind].Fetch(table); err != nil {
 			return err
 		}
 	}
 
+	// unmatched stores new issueFeatures (exist in the loaded issues but not the airtable base).
 	unmatched := airtabledb.NewDB()
 
-	//
-	// compute fields
-	//
-
-	for tableKind, featureMap := range features {
+	// Insert new issueFeatures into unmatched and mark altered cache issueFeatures with airtabledb.StateChanged.
+	for tableKind, featureMap := range issueFeatures {
 		for _, dbEntry := range featureMap {
 			matched := false
 			dbRecord := dbEntry.ToRecord(cache)
@@ -194,11 +201,10 @@ func airtableSync(opts *airtableOptions) error {
 		}
 	}
 
-	//
-	// update airtable
-	//
+	// Add new issueFeatures from unmatched to cache.
+	// Then, push new and altered issueFeatures from cache to airtable base.
 	for tableKind, tableName := range opts.TableNames {
-		table := at.Table(tableName)
+		table := client.Table(tableName)
 		ut := unmatched.Tables[tableKind]
 		ct := cache.Tables[tableKind]
 		for i := 0; i < ut.Len(); i++ {
@@ -228,9 +234,6 @@ func airtableSync(opts *airtableOptions) error {
 		}
 	}
 
-	//
-	// debug
-	//
 	for tableKind, tableName := range opts.TableNames {
 		fmt.Println("-------", tableName)
 		ct := cache.Tables[tableKind]

cmd/cmd_db.go

@@ -68,35 +68,4 @@ func dbDump(opts *dbOptions) error {
 	}
 	fmt.Println(string(out))
 	return nil
-}
-
-func loadIssues(targets []string) (repo.Issues, error) {
-	query := db.Model(repo.Issue{}).Order("created_at")
-	if len(targets) > 0 {
-		return nil, fmt.Errorf("not implemented")
-		// query = query.Where("repo_url IN (?)", canonicalTargets(targets))
-		// OR WHERE parents IN ....
-		// etc
-	}
-
-	perPage := 100
-	var issues []*repo.Issue
-	for page := 0; ; page++ {
-		var newIssues []*repo.Issue
-		if err := query.Limit(perPage).Offset(perPage * page).Find(&newIssues).Error; err != nil {
-			return nil, err
-		}
-		issues = append(issues, newIssues...)
-		if len(newIssues) < perPage {
-			break
-		}
-	}
-
-	for _, issue := range issues {
-		issue.PostLoad()
-	}
-
-	return repo.Issues(issues), nil
-}
-
-// FIXME: try to use gorm hooks to auto preload/postload items
+}

cmd/cmd_graph.go

@@ -81,7 +81,7 @@ func (cmd *graphCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Com
 
 func graph(opts *graphOptions) error {
 	zap.L().Debug("graph", zap.Stringer("opts", *opts))
-	issues, err := loadIssues(nil)
+	issues, err := repo.LoadIssues(db, nil)
 	if err != nil {
 		return errors.Wrap(err, "failed to load issues")
 	}

cmd/cmd_pull.go

@@ -2,15 +2,14 @@ package main
 
 import (
 	"encoding/json"
-	"moul.io/depviz/pkg/repo"
 	"os"
-	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"go.uber.org/zap"
+	"moul.io/depviz/pkg/repo"
 )
 
 type pullOptions struct {
@@ -61,51 +60,14 @@ func (cmd *pullCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Comm
 }
 
 func pullAndCompute(opts *pullOptions) error {
+	zap.L().Debug("pull", zap.Stringer("opts", *opts))
 	if os.Getenv("DEPVIZ_NOPULL") != "1" {
-		if err := pull(opts); err != nil {
+		if err := repo.PullAndCompute(opts.GithubToken, opts.GitlabToken, db, opts.Targets); err != nil {
 			return errors.Wrap(err, "failed to pull")
 		}
-	}
-	if err := compute(opts); err != nil {
-		return errors.Wrap(err, "failed to compute")
-	}
-	return nil
-}
-
-func pull(opts *pullOptions) error {
-	// FIXME: handle the special '@me' target
-	zap.L().Debug("pull", zap.Stringer("opts", *opts))
-
-	var (
-		wg        sync.WaitGroup
-		allIssues []*repo.Issue
-		out       = make(chan []*repo.Issue, 100)
-	)
-
-	targets := opts.Targets.UniqueProjects()
-
-	// parallel fetches
-	wg.Add(len(targets))
-	for _, target := range targets {
-		switch target.Driver() {
-		case repo.GithubDriver:
-			go repo.GithubPull(target, &wg, opts.GithubToken, db, out)
-		case repo.GitlabDriver:
-			go repo.GitlabPull(target, &wg, opts.GitlabToken, db, out)
-		default:
-			panic("should not happen")
-		}
-	}
-	wg.Wait()
-	close(out)
-	for issues := range out {
-		allIssues = append(allIssues, issues...)
-	}
-
-	// save
-	for _, issue := range allIssues {
-		if err := db.Save(issue).Error; err != nil {
-			return err
+	} else {
+		if err := repo.Compute(db); err != nil {
+			return errors.Wrap(err, "failed to compute")
 		}
 	}
 	return nil

cmd/cmd_web.go

@@ -63,7 +63,7 @@ func (cmd *webCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Comma
 }
 
 func webListIssues(w http.ResponseWriter, r *http.Request) {
-	issues, err := loadIssues(nil)
+	issues, err := repo.LoadIssues(db, nil)
 	if err != nil {
 		render.Render(w, r, ErrRender(err))
 		return
@@ -92,7 +92,7 @@ func webGraphviz(r *http.Request) (string, error) {
 		Targets:    targets,
 		ShowClosed: r.URL.Query().Get("show-closed") == "1",
 	}
-	issues, err := loadIssues(nil)
+	issues, err := repo.LoadIssues(db, nil)
 	if err != nil {
 		return "", err
 	}