From 82a957a459c9133d6f01709e27bf3e1dc9e10065 Mon Sep 17 00:00:00 2001 From: Manfred Touron Date: Tue, 3 Sep 2019 19:18:14 +0200 Subject: [PATCH] feat: optimize issues fetching --- compute/db.go | 64 ++++++++++++++++++++++++++++++++++++ github/github.go | 6 ++-- github/model.go | 79 ++++++++++++++++++++++++++++++++------------- go.mod | 2 +- go.sum | 4 +-- graph/graph.go | 6 +--- model/models.go | 26 ++++++++------- model/targets.go | 5 +++ sql/cmd_sql_info.go | 2 +- sql/helpers.go | 2 ++ 10 files changed, 151 insertions(+), 45 deletions(-) create mode 100644 compute/db.go diff --git a/compute/db.go b/compute/db.go new file mode 100644 index 000000000..8f55f9dc4 --- /dev/null +++ b/compute/db.go @@ -0,0 +1,64 @@ +package compute + +import ( + "github.com/jinzhu/gorm" + "go.uber.org/zap" + "moul.io/depviz/sql" + "moul.io/multipmuri" +) + +type multipmuriRepo interface { + RepoEntity() multipmuri.Entity +} + +type multipmuriOwner interface { + OwnerEntity() multipmuri.Entity +} + +type multipmuriService interface { + ServiceEntity() multipmuri.Entity +} + +// FIXME: loadIssuesByAuthor +// FIXME: handle github search + +func LoadIssuesByTargets(db *gorm.DB, targets []multipmuri.Entity) (*Computed, error) { + byRepo := []string{} + byOwner := []string{} + byService := []string{} + useFilters := true + for _, target := range targets { + switch v := target.(type) { + case multipmuriRepo: + byRepo = append(byRepo, v.RepoEntity().String()) + case multipmuriOwner: + byOwner = append(byOwner, v.OwnerEntity().String()) + case multipmuriService: + byService = append(byService, v.ServiceEntity().String()) + default: + zap.L().Warn("unsupported target filter", zap.Any("target", target)) + useFilters = false + } + } + + // FIXME: add a owner field on issue + filteredDB := db + if useFilters { + filteredDB = filteredDB.Where( + "repository_id IN (?) OR repository_owner_id IN (?) OR service_id IN (?)", + byRepo, + byOwner, + byService, + ) + } + + allIssues, err := sql.LoadAllIssues(filteredDB) + if err != nil { + return nil, err + } + + computed := Compute(allIssues) + computed.FilterByTargets(targets) // in most cases, this step is optional as we are already filtering by targets when querying the database + + return &computed, nil +} diff --git a/github/github.go b/github/github.go index 57a944888..f92bf4c89 100644 --- a/github/github.go +++ b/github/github.go @@ -17,14 +17,14 @@ import ( func Pull(input multipmuri.Entity, wg *sync.WaitGroup, token string, db *gorm.DB, out chan<- []*model.Issue) { defer wg.Done() type multipmuriMinimalInterface interface { - RepoEntity() *multipmuri.GitHubRepo + Repo() *multipmuri.GitHubRepo } target, ok := input.(multipmuriMinimalInterface) if !ok { zap.L().Warn("invalid input", zap.String("input", fmt.Sprintf("%v", input.String()))) return } - repo := target.RepoEntity() + repo := target.Repo() // create client ctx := context.Background() @@ -43,7 +43,7 @@ func Pull(input multipmuri.Entity, wg *sync.WaitGroup, token string, db *gorm.DB } for { - issues, resp, err := client.Issues.ListByRepo(ctx, repo.Owner(), repo.Repo(), callOpts) + issues, resp, err := client.Issues.ListByRepo(ctx, repo.OwnerID(), repo.RepoID(), callOpts) if err != nil { log.Fatal(err) return diff --git a/github/model.go b/github/model.go index e92226bb7..01e2e083f 100644 --- a/github/model.go +++ b/github/model.go @@ -5,9 +5,15 @@ import ( "github.com/google/go-github/github" "moul.io/depviz/model" + "moul.io/multipmuri" ) func FromUser(input *github.User) *model.Account { + entity, err := model.ParseTarget(input.GetHTMLURL()) + if err != nil { + panic(err) + } + name := input.GetName() if name == "" { name = input.GetLogin() @@ -19,7 +25,8 @@ func FromUser(input *github.User) *model.Account { UpdatedAt: input.GetUpdatedAt().Time, URL: input.GetURL(), }, - Provider: githubProvider(), + // Type: "user" + Provider: FromServiceURL(multipmuri.ServiceEntity(entity).String()), Location: input.GetLocation(), Company: input.GetCompany(), Blog: input.GetBlog(), @@ -31,12 +38,32 @@ func FromUser(input *github.User) *model.Account { } func FromRepositoryURL(input string) *model.Repository { + entity, err := model.ParseTarget(input) + if err != nil { + panic(err) + } + owner := multipmuri.OwnerEntity(entity) return &model.Repository{ Base: model.Base{ ID: input, URL: input, }, - Provider: githubProvider(), + Provider: FromServiceURL(multipmuri.ServiceEntity(entity).String()), + Owner: FromOwnerURL(owner.String()), + } +} + +func FromOwnerURL(input string) *model.Account { + entity, err := model.ParseTarget(input) + if err != nil { + panic(err) + } + return &model.Account{ + Base: model.Base{ + ID: input, + URL: input, + }, + Provider: FromServiceURL(multipmuri.ServiceEntity(entity).String()), } } @@ -77,30 +104,37 @@ func FromLabel(input *github.Label) *model.Label { } func FromIssue(input *github.Issue) *model.Issue { - parts := strings.Split(input.GetHTMLURL(), "/") - url := strings.Replace(input.GetHTMLURL(), "/pull/", "/issues/", -1) + entity, err := model.ParseTarget(input.GetHTMLURL()) + if err != nil { + panic(err) + } + repo := multipmuri.RepoEntity(entity) + owner := multipmuri.OwnerEntity(entity) + service := multipmuri.ServiceEntity(entity) issue := &model.Issue{ Base: model.Base{ - ID: url, - URL: url, + ID: entity.String(), + URL: entity.String(), CreatedAt: input.GetCreatedAt(), UpdatedAt: input.GetUpdatedAt(), }, - CompletedAt: input.GetClosedAt(), - Repository: FromRepositoryURL(strings.Join(parts[0:len(parts)-2], "/")), - Title: input.GetTitle(), - State: input.GetState(), - Body: input.GetBody(), - IsPR: input.PullRequestLinks != nil, - IsLocked: input.GetLocked(), - NumComments: input.GetComments(), - NumUpvotes: *input.Reactions.PlusOne, - NumDownvotes: *input.Reactions.MinusOne, - Labels: make([]*model.Label, 0), - Assignees: make([]*model.Account, 0), - Author: FromUser(input.User), - Milestone: FromMilestone(input.Milestone), + CompletedAt: input.GetClosedAt(), + Repository: FromRepositoryURL(repo.String()), + RepositoryOwner: FromOwnerURL(owner.String()), + Service: FromServiceURL(service.String()), + Title: input.GetTitle(), + State: input.GetState(), + Body: input.GetBody(), + IsPR: input.PullRequestLinks != nil, + IsLocked: input.GetLocked(), + NumComments: input.GetComments(), + NumUpvotes: *input.Reactions.PlusOne, + NumDownvotes: *input.Reactions.MinusOne, + Labels: make([]*model.Label, 0), + Assignees: make([]*model.Account, 0), + Author: FromUser(input.User), + Milestone: FromMilestone(input.Milestone), } for _, label := range input.Labels { issue.Labels = append(issue.Labels, FromLabel(&label)) @@ -111,10 +145,11 @@ func FromIssue(input *github.Issue) *model.Issue { return issue } -func githubProvider() *model.Provider { +func FromServiceURL(input string) *model.Provider { return &model.Provider{ Base: model.Base{ - ID: "github", // FIXME: support multiple github instances + ID: input, + URL: input, }, Driver: string(model.GithubDriver), } diff --git a/go.mod b/go.mod index a7262923e..7ad7cd562 100644 --- a/go.mod +++ b/go.mod @@ -35,6 +35,6 @@ require ( gopkg.in/yaml.v2 v2.2.2 moul.io/graphman v1.5.0 moul.io/graphman/viz v0.0.0-20190830152634-1bb4245b0456 - moul.io/multipmuri v1.6.0 + moul.io/multipmuri v1.8.0 moul.io/zapgorm v0.0.0-20190706070406-8138918b527b ) diff --git a/go.sum b/go.sum index 84c8c243b..8ca46c306 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ moul.io/graphman v1.5.0 h1:m2kVB06uI82UEEWSOtR58sKwB2yahhY4eDu3wmIDTCE= moul.io/graphman v1.5.0/go.mod h1:A34tMYPJRjkFBWPIsmrVFb481r+dsPyP2Ee83RuObwI= moul.io/graphman/viz v0.0.0-20190830152634-1bb4245b0456 h1:hZfuflz4kSOV21yDBTqtQdgB2GHitLfaCb8cS7tD4xs= moul.io/graphman/viz v0.0.0-20190830152634-1bb4245b0456/go.mod h1:TSPOyvLb1/0QPhaOzlW936GMtFxpgDeLGAXPjkBJNxM= -moul.io/multipmuri v1.6.0 h1:lfRCGN58BMCXDq2lZOwb/FBHhvJdeRaJYBV2C6TY5Kk= -moul.io/multipmuri v1.6.0/go.mod h1:xTE1AUMMTNRFyNRNwnM9E7UgOalny3SNb3B4h5CYv4Q= +moul.io/multipmuri v1.8.0 h1:KOQ/Nu9pe+5HyjJ8XUO0wxcGvLgrOo4bkxYyb9G0ZnY= +moul.io/multipmuri v1.8.0/go.mod h1:xTE1AUMMTNRFyNRNwnM9E7UgOalny3SNb3B4h5CYv4Q= moul.io/zapgorm v0.0.0-20190706070406-8138918b527b h1:7A2qUMck+Ikop+5+Ar6wyweFOHTwXEgHbNayY2mEB6Q= moul.io/zapgorm v0.0.0-20190706070406-8138918b527b/go.mod h1:JDE3xz5BQ1ccnAijE5+T8Qin6T256Bw2Cpdi+qMfWgw= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/graph/graph.go b/graph/graph.go index 6e5bba13f..57de1765f 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -62,14 +62,10 @@ func Graph(opts *Options) (string, error) { return "", err } - issues, err := sql.LoadAllIssues(db) + computed, err := compute.LoadIssuesByTargets(db, opts.Targets) if err != nil { return "", err } - - // compute and filter issues - computed := compute.Compute(issues) - computed.FilterByTargets(opts.Targets) // FIXME: if !opts.ShowOrphans { computed.FilterOrphans() } // FIXME: if !opts.ShowAllRelated { computed.FilterAllRelated() // FIXME: if !opts.ShowPRs { computed.FilterPRs() diff --git a/model/models.go b/model/models.go index 6e17eeca8..30ab7804f 100644 --- a/model/models.go +++ b/model/models.go @@ -151,17 +151,21 @@ type Issue struct { IsHidden bool `json:"is-hidden"` // relationships - Repository *Repository `json:"repository"` - RepositoryID string `json:"repository_id"` - Milestone *Milestone `json:"milestone"` - MilestoneID string `json:"milestone_id"` - Author *Account `json:"author"` - AuthorID string `json:"author_id"` - Labels []*Label `gorm:"many2many:issue_labels" json:"labels"` - Assignees []*Account `gorm:"many2many:issue_assignees" json:"assignees"` - Parents []*Issue `json:"-" gorm:"many2many:issue_parents;association_jointable_foreignkey:parent_id"` - Children []*Issue `json:"-" gorm:"many2many:issue_children;association_jointable_foreignkey:child_id"` - Related []*Issue `json:"-" gorm:"many2many:issue_related;association_jointable_foreignkey:related_id"` + Repository *Repository `json:"repository"` + RepositoryID string `json:"repository-id"` + RepositoryOwner *Account `json:"repository-owner"` + RepositoryOwnerID string `json:"repository-owner-id"` + Service *Provider `json:"service"` + ServiceID string `json:"service-id"` + Milestone *Milestone `json:"milestone"` + MilestoneID string `json:"milestone-id"` + Author *Account `json:"author"` + AuthorID string `json:"author-id"` + Labels []*Label `gorm:"many2many:issue_labels" json:"labels"` + Assignees []*Account `gorm:"many2many:issue_assignees" json:"assignees"` + Parents []*Issue `json:"-" gorm:"many2many:issue_parents;association_jointable_foreignkey:parent_id"` + Children []*Issue `json:"-" gorm:"many2many:issue_children;association_jointable_foreignkey:child_id"` + Related []*Issue `json:"-" gorm:"many2many:issue_related;association_jointable_foreignkey:related_id"` } func (i Issue) String() string { diff --git a/model/targets.go b/model/targets.go index 6abafccd0..fc57575c0 100644 --- a/model/targets.go +++ b/model/targets.go @@ -14,3 +14,8 @@ func ParseTargets(args []string) ([]multipmuri.Entity, error) { } return targets, nil } + +func ParseTarget(arg string) (multipmuri.Entity, error) { + defaultContext := multipmuri.NewGitHubService("") + return defaultContext.RelDecodeString(arg) +} diff --git a/sql/cmd_sql_info.go b/sql/cmd_sql_info.go index 204341925..2fdd04295 100644 --- a/sql/cmd_sql_info.go +++ b/sql/cmd_sql_info.go @@ -63,7 +63,7 @@ func runInfo(opts *infoOptions) error { log.Printf("failed to get count for %q: %v", tableName, err) continue } - fmt.Printf("stats: %-20s %3d\n", tableName, count) + fmt.Printf("stats: %-20s %5d\n", tableName, count) } return nil diff --git a/sql/helpers.go b/sql/helpers.go index 1dbb961c0..9d4fd5f79 100644 --- a/sql/helpers.go +++ b/sql/helpers.go @@ -2,6 +2,7 @@ package sql import ( "github.com/jinzhu/gorm" + "go.uber.org/zap" "moul.io/depviz/model" ) @@ -19,5 +20,6 @@ func LoadAllIssues(db *gorm.DB) (model.Issues, error) { break } } + zap.L().Debug("fetched issues", zap.Int("quantity", len(allIssues))) return allIssues, nil }