internal/postgres: use series path in sort by latest
Using module path when sorting by the latest version can give the
wrong answer for some paths. Consider M/v9 and M/v10. The latter is
obviously later, but sorting by module path descending before version
returns M/v9. But sometimes we do want to sort by the module path
first, as when we prefer an older version in a nested module to a
newer version outside. (See
TestGetUnitMeta/prefer_pubsublite_nested_module.)
If we sort by series path instead of module path handles both cases.
When looking for the latest major version of a given module, we select
for the series path, so sorting by series path is a no-op. But the
series path correctly distinguishes nested modules from their parent
modules.
Fixes golang/go#43902
Change-Id: Icadab4ce2326f91ab3d3c81f46d17e482a2b9654
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/286452
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go
index 45241b5..72ef3c9 100644
--- a/internal/postgres/unit.go
+++ b/internal/postgres/unit.go
@@ -143,7 +143,7 @@
WHEN m.version_type = 'prerelease' THEN 4
ELSE 5
END`,
- "m.module_path DESC",
+ "m.series_path DESC",
"m.sort_version DESC",
).PlaceholderFormat(squirrel.Dollar)
}
@@ -157,8 +157,8 @@
WHEN m.version_type = 'prerelease' THEN 4
ELSE 5
END,
- m.sort_version DESC,
- m.module_path DESC`
+ m.series_path DESC,
+ m.sort_version DESC`
// GetUnit returns a unit from the database, along with all of the data
// associated with that unit.
diff --git a/internal/postgres/version_test.go b/internal/postgres/version_test.go
index b3adc90..8233f9d 100644
--- a/internal/postgres/version_test.go
+++ b/internal/postgres/version_test.go
@@ -240,6 +240,8 @@
sample.Module("a.com/M/v2", "v2.0.5", "all", "most"),
sample.Module("a.com/M/v3", "v3.0.1", "all", "some"),
sample.Module("a.com/M/D", "v1.3.0", "other"),
+ sample.Module("b.com/M/v9", "v9.0.0", ""),
+ sample.Module("b.com/M/v10", "v10.0.0", ""),
} {
if err := testDB.InsertModule(ctx, m); err != nil {
t.Fatal(err)
@@ -247,12 +249,13 @@
}
for _, test := range []struct {
- unit string
- want internal.LatestInfo
+ unit, module string
+ want internal.LatestInfo
}{
{
// A unit that is the module.
- "a.com/M",
+ "a.com/M", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.2.0",
MinorModulePath: "a.com/M",
@@ -263,7 +266,8 @@
},
{
// A unit that exists in all versions of the module.
- "a.com/M/all",
+ "a.com/M/all", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.2.0",
MinorModulePath: "a.com/M",
@@ -274,7 +278,8 @@
},
{
// A unit that exists in most versions, but not the latest major.
- "a.com/M/most",
+ "a.com/M/most", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.2.0",
MinorModulePath: "a.com/M",
@@ -285,7 +290,8 @@
},
{
// A unit that does not exist at the latest minor version, but does at the latest major.
- "a.com/M/some",
+ "a.com/M/some", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.1.1",
MinorModulePath: "a.com/M",
@@ -296,7 +302,8 @@
},
{
// A unit that does not exist at the latest minor or major versions.
- "a.com/M/one",
+ "a.com/M/one", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.1.1",
MinorModulePath: "a.com/M",
@@ -307,7 +314,8 @@
},
{
// A unit whose latest minor version is in a different module.
- "a.com/M/D/other",
+ "a.com/M/D/other", "a.com/M",
+
internal.LatestInfo{
MinorVersion: "v1.3.0",
MinorModulePath: "a.com/M/D",
@@ -316,9 +324,20 @@
MajorUnitPath: "a.com/M/v3",
},
},
+ {
+ // A module with v9 and v10 versions.
+ "b.com/M/v9", "b.com/M/v9",
+ internal.LatestInfo{
+ MinorVersion: "v9.0.0",
+ MinorModulePath: "b.com/M/v9",
+ UnitExistsAtMinor: true,
+ MajorModulePath: "b.com/M/v10",
+ MajorUnitPath: "b.com/M/v10",
+ },
+ },
} {
t.Run(test.unit, func(t *testing.T) {
- got, err := testDB.GetLatestInfo(ctx, test.unit, "a.com/M")
+ got, err := testDB.GetLatestInfo(ctx, test.unit, test.module)
if err != nil {
t.Fatal(err)
}
