internal/postgres: fix GetLatestMajorPathForV1Path for gopkg.in
GetLatestMajorPathForV1Path was not handling the special
case of gopkg.in modules correctly.
Also, split the logic for extracting the major version of a module
path to a separate function.
Change-Id: Ie7758330a14ebaab6360fafcc4bf2d3ae46f6479
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/307470
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/postgres/path.go b/internal/postgres/path.go
index 61df19d..239a0be 100644
--- a/internal/postgres/path.go
+++ b/internal/postgres/path.go
@@ -13,6 +13,7 @@
"strings"
"github.com/lib/pq"
+ "golang.org/x/mod/module"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/database"
"golang.org/x/pkgsite/internal/derrors"
@@ -35,7 +36,7 @@
ORDER BY p.path DESC
LIMIT 1
);`
- paths := map[string]string{}
+ paths := map[string]string{} // from unit path to series path
err = db.db.RunQuery(ctx, q, func(rows *sql.Rows) error {
var p, sp string
if err := rows.Scan(&p, &sp); err != nil {
@@ -56,14 +57,10 @@
// Trim the series path and suffix from the unit path.
// Keep only the N following vN.
suffix := internal.Suffix(v1path, sp)
- v := strings.TrimSuffix(strings.TrimPrefix(
- strings.TrimSuffix(strings.TrimPrefix(p, sp), suffix), "/v"), "/")
- var i int
- if v != "" {
- i, err = strconv.Atoi(v)
- if err != nil {
- return "", 0, fmt.Errorf("strconv.Atoi(%q): %v", v, err)
- }
+ modPath := strings.TrimSuffix(p, "/"+suffix)
+ i, err := modulePathMajorVersion(modPath)
+ if err != nil {
+ return "", 0, err
}
if maj <= i {
maj = i
@@ -77,6 +74,23 @@
return majPath, maj, nil
}
+// modulePathMajorVersion returns the numeric major version for the given module path.
+// If the path has no "vN" suffix, it returns 1.
+func modulePathMajorVersion(modulePath string) (int, error) {
+ _, m, ok := module.SplitPathVersion(modulePath)
+ if !ok {
+ return 0, fmt.Errorf("bad module path %q", modulePath)
+ }
+ if m == "" {
+ return 1, nil
+ }
+ // First two characters are either ".v" or "/v".
+ if len(m) < 3 {
+ return 0, fmt.Errorf("bad version %q from module.SplitPathVersion(%q)", m, modulePath)
+ }
+ return strconv.Atoi(m[2:])
+}
+
// upsertPath adds path into the paths table if it does not exist, and returns
// its ID either way.
// It assumes it is running inside a transaction.
diff --git a/internal/postgres/path_test.go b/internal/postgres/path_test.go
index 9600f29..61f0cfd 100644
--- a/internal/postgres/path_test.go
+++ b/internal/postgres/path_test.go
@@ -44,6 +44,12 @@
[]string{"m.com/v4@v4.0.0"},
"m.com/v4", 4,
},
+ {
+ "gopkg.in",
+ "gopkg.in/yaml",
+ []string{"gopkg.in/yaml.v1@v1.0.0", "gopkg.in/yaml.v2@v2.0.0"},
+ "gopkg.in/yaml.v2", 2,
+ },
} {
t.Run(test.name, func(t *testing.T) {
testDB, release := acquire(t)
@@ -78,6 +84,26 @@
}
}
+func TestModulePathMajorVersion(t *testing.T) {
+ for _, test := range []struct {
+ in string
+ want int
+ }{
+ {"m.com", 1},
+ {"m.com/v123", 123},
+ {"gopkg.in/m.v1", 1},
+ {"gopkg.in/m.v35", 35},
+ } {
+ got, err := modulePathMajorVersion(test.in)
+ if err != nil {
+ t.Fatalf("%s: %v", test.in, err)
+ }
+ if got != test.want {
+ t.Errorf("%s: got %d, want %d", test.in, got, test.want)
+ }
+ }
+}
+
func TestUpsertPathConcurrently(t *testing.T) {
// Verify that we get no constraint violations or other errors when
// the same path is upserted multiple times concurrently.
