doc: factor out redirect logic from newPackage to gosrc.MaybeRedirect
This is a refactor to bring the redirect check into a small dedicated
function within gosrc. It can be tested more directly/easily.
Change-Id: Ib9c907bae00384c1f7fef74146b19b715976c546
Reviewed-on: https://go-review.googlesource.com/134475
Reviewed-by: Tuo Shan <shantuo@google.com>
diff --git a/doc/builder.go b/doc/builder.go
index e372d3d..0dac830 100644
--- a/doc/builder.go
+++ b/doc/builder.go
@@ -577,28 +577,11 @@
return pkg, nil
}
- switch {
- case bpkg.ImportComment != "":
- // Redirect to import path comment, if not already there.
- if dir.ImportPath != bpkg.ImportComment {
- return nil, gosrc.NotFoundError{
- Message: "not at canonical import path",
- Redirect: bpkg.ImportComment,
- }
- }
-
- // Redirect to GitHub's reported canonical casing when there's no import path comment,
- // and the import path differs from resolved GitHub path only in case.
- case bpkg.ImportComment == "" && dir.ResolvedGitHubPath != "" &&
- strings.EqualFold(dir.ImportPath, dir.ResolvedGitHubPath):
-
- // Redirect to resolved GitHub path, if not already there.
- if dir.ImportPath != dir.ResolvedGitHubPath {
- return nil, gosrc.NotFoundError{
- Message: "not at canonical import path",
- Redirect: dir.ResolvedGitHubPath,
- }
- }
+ // Use information we have by now (import comment and resolved GitHub path)
+ // to redirect to a canonical import path, when it's possible to do so reliably.
+ err = gosrc.MaybeRedirect(dir.ImportPath, bpkg.ImportComment, dir.ResolvedGitHubPath)
+ if err != nil {
+ return nil, err
}
// Parse the Go files
diff --git a/doc/builder_test.go b/doc/builder_test.go
index 763f9e4..7160cda 100644
--- a/doc/builder_test.go
+++ b/doc/builder_test.go
@@ -9,8 +9,6 @@
import (
"go/ast"
"testing"
-
- "github.com/golang/gddo/gosrc"
)
var badSynopsis = []string{
@@ -114,108 +112,3 @@
}
}
}
-
-// TestNewPackageRedirect tests that newPackage redirects
-// and does not redirect as expected, in various situations.
-// See https://github.com/golang/gddo/issues/507
-// and https://github.com/golang/gddo/issues/579.
-func TestNewPackageRedirect(t *testing.T) {
- // robpike.io/ivy package.
- // Vanity import path, hosted on GitHub, with import path comment.
- ivy := gosrc.Directory{
- Files: []*gosrc.File{
- {Name: "main.go", Data: []byte("package main // import \"robpike.io/ivy\"\n")},
- },
- ResolvedGitHubPath: "github.com/robpike/ivy",
- }
-
- // go4.org/sort package.
- // Vanity import path, hosted on GitHub, without import path comment.
- go4sort := gosrc.Directory{
- Files: []*gosrc.File{
- {Name: "main.go", Data: []byte("package sort\n")},
- },
- ResolvedGitHubPath: "github.com/go4org/go4/sort",
- }
-
- // github.com/teamwork/validate package.
- // Hosted on GitHub, with import path comment that doesn't match canonical GitHub case.
- // See issue https://github.com/golang/gddo/issues/507.
- gtv := gosrc.Directory{
- Files: []*gosrc.File{
- {Name: "main.go", Data: []byte("package validate // import \"github.com/teamwork/validate\"\n")},
- },
- ResolvedGitHubPath: "github.com/Teamwork/validate", // Note that this differs from import path comment.
- }
-
- tests := []struct {
- name string
- repo gosrc.Directory
- requestPath string
- wantRedirect string // Empty string means no redirect.
- }{
- // ivy.
- {
- repo: ivy, name: "ivy repo: access canonical path -> no redirect",
- requestPath: "robpike.io/ivy",
- },
- {
- repo: ivy, name: "ivy repo: access GitHub path -> redirect to import comment",
- requestPath: "github.com/robpike/ivy",
- wantRedirect: "robpike.io/ivy",
- },
- {
- repo: ivy, name: "ivy repo: access GitHub path with weird casing -> redirect to import comment",
- requestPath: "github.com/RoBpIkE/iVy",
- wantRedirect: "robpike.io/ivy",
- },
-
- // go4sort.
- {
- repo: go4sort, name: "go4sort repo: access canonical path -> no redirect",
- requestPath: "go4.org/sort",
- },
- {
- repo: go4sort, name: "go4sort repo: access GitHub path -> no redirect",
- requestPath: "github.com/go4org/go4/sort",
- },
- {
- repo: go4sort, name: "go4sort repo: access GitHub path with weird casing -> redirect to resolved GitHub case",
- requestPath: "github.com/gO4oRg/Go4/sort",
- wantRedirect: "github.com/go4org/go4/sort",
- },
-
- // gtv.
- {
- repo: gtv, name: "gtv repo: access canonical path -> no redirect",
- requestPath: "github.com/teamwork/validate",
- },
- {
- repo: gtv, name: "gtv repo: access canonical GitHub path -> redirect to import comment",
- requestPath: "github.com/Teamwork/validate",
- wantRedirect: "github.com/teamwork/validate",
- },
- {
- repo: gtv, name: "gtv repo: access GitHub path with weird casing -> redirect to import comment",
- requestPath: "github.com/tEaMwOrK/VaLiDaTe",
- wantRedirect: "github.com/teamwork/validate",
- },
- }
- for _, tt := range tests {
- dir := tt.repo
- dir.ImportPath = tt.requestPath
-
- var want error
- if tt.wantRedirect != "" {
- want = gosrc.NotFoundError{
- Message: "not at canonical import path",
- Redirect: tt.wantRedirect,
- }
- }
-
- _, got := newPackage(&dir)
- if got != want {
- t.Errorf("%s: got error %v, want %v", tt.name, got, want)
- }
- }
-}
diff --git a/gosrc/gosrc.go b/gosrc/gosrc.go
index 776bd45..d1a0be8 100644
--- a/gosrc/gosrc.go
+++ b/gosrc/gosrc.go
@@ -541,3 +541,35 @@
}
return nil, NotFoundError{Message: "path does not match registered service"}
}
+
+// MaybeRedirect uses the provided import path, import comment, and resolved GitHub path
+// to make a decision of whether to redirect to another, more canonical import path.
+// It returns nil error to indicate no redirect, or a NotFoundError error to redirect.
+func MaybeRedirect(importPath, importComment, resolvedGitHubPath string) error {
+ switch {
+ case importComment != "":
+ // Redirect to import comment, if not already there.
+ if importPath != importComment {
+ return NotFoundError{
+ Message: "not at canonical import path",
+ Redirect: importComment,
+ }
+ }
+
+ // Redirect to GitHub's reported canonical casing when there's no import comment,
+ // and the import path differs from resolved GitHub path only in case.
+ case importComment == "" && resolvedGitHubPath != "" &&
+ strings.EqualFold(importPath, resolvedGitHubPath):
+
+ // Redirect to resolved GitHub path, if not already there.
+ if importPath != resolvedGitHubPath {
+ return NotFoundError{
+ Message: "not at canonical import path",
+ Redirect: resolvedGitHubPath,
+ }
+ }
+ }
+
+ // No redirect.
+ return nil
+}
diff --git a/gosrc/gosrc_test.go b/gosrc/gosrc_test.go
index 723c73c..3349926 100644
--- a/gosrc/gosrc_test.go
+++ b/gosrc/gosrc_test.go
@@ -319,3 +319,103 @@
}
}
}
+
+// TestMaybeRedirect tests that MaybeRedirect redirects
+// and does not redirect as expected, in various situations.
+// See https://github.com/golang/gddo/issues/507
+// and https://github.com/golang/gddo/issues/579.
+func TestMaybeRedirect(t *testing.T) {
+ type repo struct {
+ ImportComment string
+ ResolvedGitHubPath string
+ }
+
+ // robpike.io/ivy package.
+ // Vanity import path, hosted on GitHub, with import comment.
+ ivy := repo{
+ ImportComment: "robpike.io/ivy",
+ ResolvedGitHubPath: "github.com/robpike/ivy",
+ }
+
+ // go4.org/sort package.
+ // Vanity import path, hosted on GitHub, without import comment.
+ go4sort := repo{
+ ResolvedGitHubPath: "github.com/go4org/go4/sort",
+ }
+
+ // github.com/teamwork/validate package.
+ // Hosted on GitHub, with import comment that doesn't match canonical GitHub case.
+ // See issue https://github.com/golang/gddo/issues/507.
+ gtv := repo{
+ ImportComment: "github.com/teamwork/validate",
+ ResolvedGitHubPath: "github.com/Teamwork/validate", // Note that this differs from import comment.
+ }
+
+ tests := []struct {
+ name string
+ repo repo
+ requestPath string
+ wantRedirect string // Empty string means no redirect.
+ }{
+ // ivy.
+ {
+ repo: ivy, name: "ivy repo: access canonical path -> no redirect",
+ requestPath: "robpike.io/ivy",
+ },
+ {
+ repo: ivy, name: "ivy repo: access GitHub path -> redirect to import comment",
+ requestPath: "github.com/robpike/ivy",
+ wantRedirect: "robpike.io/ivy",
+ },
+ {
+ repo: ivy, name: "ivy repo: access GitHub path with weird casing -> redirect to import comment",
+ requestPath: "github.com/RoBpIkE/iVy",
+ wantRedirect: "robpike.io/ivy",
+ },
+
+ // go4sort.
+ {
+ repo: go4sort, name: "go4sort repo: access canonical path -> no redirect",
+ requestPath: "go4.org/sort",
+ },
+ {
+ repo: go4sort, name: "go4sort repo: access GitHub path -> no redirect",
+ requestPath: "github.com/go4org/go4/sort",
+ },
+ {
+ repo: go4sort, name: "go4sort repo: access GitHub path with weird casing -> redirect to resolved GitHub case",
+ requestPath: "github.com/gO4oRg/Go4/sort",
+ wantRedirect: "github.com/go4org/go4/sort",
+ },
+
+ // gtv.
+ {
+ repo: gtv, name: "gtv repo: access canonical path -> no redirect",
+ requestPath: "github.com/teamwork/validate",
+ },
+ {
+ repo: gtv, name: "gtv repo: access canonical GitHub path -> redirect to import comment",
+ requestPath: "github.com/Teamwork/validate",
+ wantRedirect: "github.com/teamwork/validate",
+ },
+ {
+ repo: gtv, name: "gtv repo: access GitHub path with weird casing -> redirect to import comment",
+ requestPath: "github.com/tEaMwOrK/VaLiDaTe",
+ wantRedirect: "github.com/teamwork/validate",
+ },
+ }
+ for _, tt := range tests {
+ var want error
+ if tt.wantRedirect != "" {
+ want = NotFoundError{
+ Message: "not at canonical import path",
+ Redirect: tt.wantRedirect,
+ }
+ }
+
+ got := MaybeRedirect(tt.requestPath, tt.repo.ImportComment, tt.repo.ResolvedGitHubPath)
+ if got != want {
+ t.Errorf("%s: got error %v, want %v", tt.name, got, want)
+ }
+ }
+}
