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 1 year ago commit 5bb3cef2f158d3631e57543ba937054986990cb0
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