dmitri.shuralyov.com/website/gido

don't show same issue or CL multiple times

Previously, when using an import path pattern to query issues or CLs,
issues or CLs that had more than one import paths associated with
them had a chance of being displayed multiple times (if more than
import path matched the import path pattern).

Fix this by pre-computing lists of all issues and CLs along with
their import path associations, then use those lists when handling
import path pattern queries. This approach makes it easier to avoid
duplicate issues/CLs.

Fixes https://dmitri.shuralyov.com/website/gido/...$issues/4.
dmitshur committed 5 years ago commit 5bb3cef2f158d3631e57543ba937054986990cb0
Showing partial commit. Full Commit
Collapse all
changes.go
@@ -138,46 +138,66 @@ func (h *handler) serveChangesPattern(w http.ResponseWriter, req *http.Request,
		pkgs = h.s.CmdPackages
	default:
		pkgs = expandPattern(h.s.AllPackages, pattern)
	}

	ics := make(map[string]*Directory) // Import path -> Directory.
	h.s.IssuesAndChangesMu.RLock()
	for _, p := range pkgs {
		if ic, ok := h.s.IssuesAndChanges[p]; ok {
			ics[p] = ic
		}
	}
	h.s.IssuesAndChangesMu.RUnlock()
	var cs []change.Change
	var openChanges, closedChanges, openIssues int
	for p, ic := range ics {
		switch {
		case filter == change.FilterOpen:
			for _, c := range ic.OpenChanges {
				c.Title = ImportPathToFullPrefix(p) + c.Title
				cs = append(cs, c)
	match := matchPaths(pattern)
	h.s.IssuesAndChangesMu.RLock()
	switch {
	case filter == change.FilterOpen:
		for _, c := range h.s.OpenChanges {
			if !match(c.Paths) {
				continue
			}
		case filter == change.FilterClosedMerged:
			for _, c := range ic.ClosedChanges {
				c.Title = ImportPathToFullPrefix(p) + c.Title
				cs = append(cs, c)
			cs = append(cs, c.Change)
		}
		openChanges = len(cs)
		for _, c := range h.s.ClosedChanges {
			if !match(c.Paths) {
				continue
			}
		case filter == change.FilterAll:
			for _, c := range ic.OpenChanges {
				c.Title = ImportPathToFullPrefix(p) + c.Title
				cs = append(cs, c)
			closedChanges++
		}
	case filter == change.FilterClosedMerged:
		for _, c := range h.s.ClosedChanges {
			if !match(c.Paths) {
				continue
			}
			for _, c := range ic.ClosedChanges {
				c.Title = ImportPathToFullPrefix(p) + c.Title
				cs = append(cs, c)
			cs = append(cs, c.Change)
		}
		closedChanges = len(cs)
		for _, c := range h.s.OpenChanges {
			if !match(c.Paths) {
				continue
			}
			openChanges++
		}
		openChanges += len(ic.OpenChanges)
		closedChanges += len(ic.ClosedChanges)
		openIssues += len(ic.OpenIssues)
	case filter == change.FilterAll:
		for _, c := range h.s.OpenChanges {
			if !match(c.Paths) {
				continue
			}
			cs = append(cs, c.Change)
		}
		openChanges = len(cs)
		for _, c := range h.s.ClosedChanges {
			if !match(c.Paths) {
				continue
			}
			cs = append(cs, c.Change)
		}
		closedChanges = len(cs) - openChanges
	}
	for _, i := range h.s.OpenIssues {
		if !match(i.Paths) {
			continue
		}
		openIssues++
	}
	h.s.IssuesAndChangesMu.RUnlock()
	sort.Slice(cs, func(i, j int) bool { return cs[i].CreatedAt.After(cs[j].CreatedAt) })

	w.Header().Set("Content-Type", "text/html; charset=utf-8")
	err = h.executeTemplate(w, req, "Header", map[string]interface{}{
		"PageName":      pattern,
issues.go
@@ -139,46 +139,66 @@ func (h *handler) serveIssuesPattern(w http.ResponseWriter, req *http.Request, p
		pkgs = h.s.CmdPackages
	default:
		pkgs = expandPattern(h.s.AllPackages, pattern)
	}

	ics := make(map[string]*Directory) // Import path -> Directory.
	h.s.IssuesAndChangesMu.RLock()
	for _, p := range pkgs {
		if ic, ok := h.s.IssuesAndChanges[p]; ok {
			ics[p] = ic
		}
	}
	h.s.IssuesAndChangesMu.RUnlock()
	var is []issues.Issue
	var openIssues, closedIssues, openChanges int
	for p, ic := range ics {
		switch {
		case filter == issues.StateFilter(issues.OpenState):
			for _, i := range ic.OpenIssues {
				i.Title = ImportPathToFullPrefix(p) + i.Title
				is = append(is, i)
	match := matchPaths(pattern)
	h.s.IssuesAndChangesMu.RLock()
	switch {
	case filter == issues.StateFilter(issues.OpenState):
		for _, i := range h.s.OpenIssues {
			if !match(i.Paths) {
				continue
			}
		case filter == issues.StateFilter(issues.ClosedState):
			for _, i := range ic.ClosedIssues {
				i.Title = ImportPathToFullPrefix(p) + i.Title
				is = append(is, i)
			is = append(is, i.Issue)
		}
		openIssues = len(is)
		for _, i := range h.s.ClosedIssues {
			if !match(i.Paths) {
				continue
			}
		case filter == issues.AllStates:
			for _, i := range ic.OpenIssues {
				i.Title = ImportPathToFullPrefix(p) + i.Title
				is = append(is, i)
			closedIssues++
		}
	case filter == issues.StateFilter(issues.ClosedState):
		for _, i := range h.s.ClosedIssues {
			if !match(i.Paths) {
				continue
			}
			for _, i := range ic.ClosedIssues {
				i.Title = ImportPathToFullPrefix(p) + i.Title
				is = append(is, i)
			is = append(is, i.Issue)
		}
		closedIssues = len(is)
		for _, i := range h.s.OpenIssues {
			if !match(i.Paths) {
				continue
			}
			openIssues++
		}
		openIssues += len(ic.OpenIssues)
		closedIssues += len(ic.ClosedIssues)
		openChanges += len(ic.OpenChanges)
	case filter == issues.AllStates:
		for _, i := range h.s.OpenIssues {
			if !match(i.Paths) {
				continue
			}
			is = append(is, i.Issue)
		}
		openIssues = len(is)
		for _, i := range h.s.ClosedIssues {
			if !match(i.Paths) {
				continue
			}
			is = append(is, i.Issue)
		}
		closedIssues = len(is) - openIssues
	}
	for _, c := range h.s.OpenChanges {
		if !match(c.Paths) {
			continue
		}
		openChanges++
	}
	h.s.IssuesAndChangesMu.RUnlock()
	sort.Slice(is, func(i, j int) bool { return is[i].CreatedAt.After(is[j].CreatedAt) })

	w.Header().Set("Content-Type", "text/html; charset=utf-8")
	err = h.executeTemplate(w, req, "Header", map[string]interface{}{
		"PageName":      pattern,
main.go
@@ -56,11 +56,11 @@ func main() {
		log.Fatalln(err)
	}
}

func run(ctx context.Context, router Router, analyticsFile string) error {
	log.Println("Starting.")
	log.Println("Starting...")

	if err := mime.AddExtensionType(".woff2", "font/woff2"); err != nil {
		return err
	}

packages.go
@@ -145,10 +145,51 @@ func expandPattern(allPackages []string, pattern string) []string {
		matched = append(matched, pkg)
	}
	return matched
}

// matchPaths(pattern)(paths) reports whether any of the import paths paths
// match the import path pattern pattern.
// It uses the same rules for pattern matching as matchPattern.
func matchPaths(pattern string) func(paths []string) bool {
	switch pattern {
	case "all", "...":
		// "all" expands to all packages found in all the GOPATH trees.
		return func([]string) bool { return true }
	case "std":
		// "std" is like all but expands to just the packages in the standard Go library.
		return func(paths []string) bool {
			for _, p := range paths {
				if isStandard(p) {
					return true
				}
			}
			return false
		}
	case "cmd":
		// "cmd" expands to the Go repository's commands and their internal libraries.
		return func(paths []string) bool {
			for _, p := range paths {
				if p == "cmd" || strings.HasPrefix(p, "cmd/") {
					return true
				}
			}
			return false
		}
	default:
		match := matchPattern(pattern)
		return func(paths []string) bool {
			for _, p := range paths {
				if match(p) {
					return true
				}
			}
			return false
		}
	}
}

// matchPattern(pattern)(name) reports whether name matches pattern.
// Pattern is a limited glob pattern in which '...' means 'any string',
// foo/... matches foo too, and there is no other special syntax.
func matchPattern(pattern string) func(name string) bool {
	re := regexp.QuoteMeta(pattern)
service.go
@@ -16,15 +16,17 @@ import (
	"golang.org/x/build/maintner"
	"golang.org/x/build/maintner/godata"
)

type service struct {
	IssuesAndChangesMu sync.RWMutex
	// IssuesAndChanges contains issues and changes for all packages. Map key is import path.
	// An additional entry with key otherPackages is for issues and changes that don't fit
	// into any existing Go package.
	IssuesAndChangesMu sync.RWMutex
	IssuesAndChanges   map[string]*Directory
	IssuesAndChanges           map[string]*Directory
	OpenIssues, ClosedIssues   []IssueAndPaths  // All issues with at least 1 import path.
	OpenChanges, ClosedChanges []ChangeAndPaths // All changes with at least 1 import path.

	AllPackages []string // All packages. Sorted by import path, standard library first.
	StdPackages []string // Packages in the standard Go library.
	CmdPackages []string // Go repository's commands and their internal libraries.
}
@@ -32,10 +34,19 @@ type service struct {
type Directory struct {
	OpenIssues, ClosedIssues   []issues.Issue
	OpenChanges, ClosedChanges []change.Change
}

type IssueAndPaths struct {
	issues.Issue
	Paths []string // One or more import paths that are associated with the issue.
}
type ChangeAndPaths struct {
	change.Change
	Paths []string // One or more import paths that are associated with the change.
}

func newService(ctx context.Context) *service {
	issuesAndChanges := emptyDirectories()

	// Initialize lists of packages.
	var all, std, cmd []string
@@ -106,13 +117,15 @@ func (s *service) poll(ctx context.Context) {
	if err != nil {
		log.Fatalln("poll: initial initCorpus failed:", err)
	}

	for {
		issuesAndChanges := issuesAndChanges(repo, corpus.Gerrit())
		issuesAndChanges, oi, ci, oc, cc := issuesAndChanges(repo, corpus.Gerrit())
		s.IssuesAndChangesMu.Lock()
		s.IssuesAndChanges = issuesAndChanges
		s.OpenIssues, s.ClosedIssues = oi, ci
		s.OpenChanges, s.ClosedChanges = oc, cc
		s.IssuesAndChangesMu.Unlock()
		for {
			updateError := corpus.Update(ctx)
			if updateError == maintner.ErrSplit {
				log.Println("corpus.Update: Corpus out of sync. Re-fetching corpus.")
@@ -143,12 +156,16 @@ func initCorpus(ctx context.Context) (*maintner.Corpus, *maintner.GitHubRepo, er
		return nil, nil, fmt.Errorf("go.googlesource.com/go Gerrit project not found")
	}
	return corpus, repo, nil
}

func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) map[string]*Directory {
	issuesAndChanges := emptyDirectories()
func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) (
	issuesAndChanges map[string]*Directory,
	openIssues, closedIssues []IssueAndPaths,
	openChanges, closedChanges []ChangeAndPaths,
) {
	issuesAndChanges = emptyDirectories()

	// Collect issues.
	err := repo.ForeachIssue(func(i *maintner.GitHubIssue) error {
		if i.NotExist || i.PullRequest {
			return nil
@@ -182,25 +199,33 @@ func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) map[st
				CreatedAt: i.Created,
			},
			Replies: replies,
		}

		var added bool
		var paths []string
		for _, p := range pkgs {
			ic := issuesAndChanges[p]
			if ic == nil {
				continue
			}
			paths = append(paths, p)
			switch issue.State {
			case issues.OpenState:
				ic.OpenIssues = append(ic.OpenIssues, issue)
			case issues.ClosedState:
				ic.ClosedIssues = append(ic.ClosedIssues, issue)
			}
			added = true
		}
		if !added {
		if len(paths) > 0 {
			issue.Title = ImportPathsToFullPrefix(paths) + title
			switch issue.State {
			case issues.OpenState:
				openIssues = append(openIssues, IssueAndPaths{issue, paths})
			case issues.ClosedState:
				closedIssues = append(closedIssues, IssueAndPaths{issue, paths})
			}
		} else {
			ic := issuesAndChanges[otherPackages]
			issue.Title = i.Title
			switch issue.State {
			case issues.OpenState:
				ic.OpenIssues = append(ic.OpenIssues, issue)
@@ -248,25 +273,33 @@ func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) map[st
				Author:    gitUser(cl.Commit.Author),
				CreatedAt: cl.Created,
				Replies:   len(cl.Messages),
			}

			var added bool
			var paths []string
			for _, p := range pkgs {
				ic := issuesAndChanges[p]
				if ic == nil {
					continue
				}
				paths = append(paths, p)
				switch c.State {
				case change.OpenState:
					ic.OpenChanges = append(ic.OpenChanges, c)
				case change.ClosedState, change.MergedState:
					ic.ClosedChanges = append(ic.ClosedChanges, c)
				}
				added = true
			}
			if !added {
			if len(paths) > 0 {
				c.Title = ImportPathsToFullPrefix(paths) + title
				switch c.State {
				case change.OpenState:
					openChanges = append(openChanges, ChangeAndPaths{c, paths})
				case change.ClosedState, change.MergedState:
					closedChanges = append(closedChanges, ChangeAndPaths{c, paths})
				}
			} else {
				ic := issuesAndChanges[root]
				if ic == nil {
					ic = issuesAndChanges[otherPackages]
				}
				c.Title = prefixedTitle
@@ -292,11 +325,11 @@ func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) map[st
		sort.Slice(p.ClosedIssues, func(i, j int) bool { return p.ClosedIssues[i].ID > p.ClosedIssues[j].ID })
		sort.Slice(p.OpenChanges, func(i, j int) bool { return p.OpenChanges[i].ID > p.OpenChanges[j].ID })
		sort.Slice(p.ClosedChanges, func(i, j int) bool { return p.ClosedChanges[i].ID > p.ClosedChanges[j].ID })
	}

	return issuesAndChanges
	return issuesAndChanges, openIssues, closedIssues, openChanges, closedChanges
}

// gerritProjects maps each supported Gerrit "server/project" to
// the import path that corresponds to the root of that project.
var gerritProjects = map[string]string{
@@ -332,11 +365,12 @@ var gerritProjects = map[string]string{
	"go.googlesource.com/xerrors":    "golang.org/x/xerrors",
}

const otherPackages = "other"

// ImportPathToFullPrefix returns the an issue title prefix (including ": ") for the given import path.
// ImportPathToFullPrefix returns the an issue title prefix (including ": ")
// for the given import path.
// If path equals to otherPackages, an empty prefix is returned.
func ImportPathToFullPrefix(path string) string {
	switch {
	default:
		// Use all other import paths directly as prefix.
@@ -348,10 +382,31 @@ func ImportPathToFullPrefix(path string) string {
		// Empty prefix for otherPackages.
		return ""
	}
}

// ImportPathsToFullPrefix returns the an issue title prefix (including ": ")
// for the given import paths.
func ImportPathsToFullPrefix(paths []string) string {
	if len(paths) == 1 {
		return ImportPathToFullPrefix(paths[0])
	}
	var b strings.Builder
	for i, p := range paths {
		if i != 0 {
			b.WriteString(", ")
		}
		if strings.HasPrefix(p, "golang.org/x/") || p == "golang.org/x" {
			// Map "golang.org/x/..." to "x/...".
			p = p[len("golang.org/"):]
		}
		b.WriteString(p)
	}
	b.WriteString(": ")
	return b.String()
}

// ghState converts a GitHub issue state into a issues.State.
func ghState(issue *maintner.GitHubIssue) issues.State {
	switch issue.Closed {
	case false:
		return issues.OpenState