Skip to content

Commit 8368384

Browse files
julieqiujulieqiu
committed
internal/postgres: change GetVersionMap logic
The logic for GetVersionMap is changed so that an explicit module path is now required. All logic is also now the same for all requestedVersions (as opposed to a special case for latest). This function is only used by the frontend to check if a version exists as part of a frontend fetch, and this behavior makes more sense for that use case. Updates golang/go#36811 Updates golang/go#37002 Updates golang/go#37106 Change-Id: I415a2730daa6edc023f80c0c615521047311f35b Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/744833 Reviewed-by: Jonathan Amsterdam <[email protected]>
1 parent fc6bef5 commit 8368384

File tree

3 files changed

+44
-195
lines changed

3 files changed

+44
-195
lines changed

‎internal/postgres/version_map.go

Lines changed: 22 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package postgres
77
import (
88
"context"
99
"database/sql"
10+
"fmt"
1011

1112
"golang.org/x/pkgsite/internal"
1213
"golang.org/x/pkgsite/internal/derrors"
@@ -59,105 +60,30 @@ func (db *DB) UpsertVersionMap(ctx context.Context, vm *internal.VersionMap) (er
5960
return err
6061
}
6162

62-
// GetVersionMap fetches a version_map entry corresponding to the given path,
63+
// GetVersionMap fetches a version_map entry corresponding to the given
6364
// modulePath and requestedVersion.
64-
func (db *DB) GetVersionMap(ctx context.Context, path, modulePath, requestedVersion string) (_ *internal.VersionMap, err error) {
65-
defer derrors.Wrap(&err, "DB.GetVersionMap(ctx, tx, %q, %q, %q)", path, modulePath, requestedVersion)
66-
67-
var (
68-
query string
69-
args []interface{}
70-
)
71-
72-
if requestedVersion == internal.LatestVersion && modulePath == internal.UnknownModulePath {
73-
// Return the version_map for the latest resolved_version at
74-
// the longest module path.
75-
//
76-
// In order to determine if a path exists in our database, and
77-
// the module path corresponding to it, we use
78-
// packages.tsv_parent_directories when module_path is not
79-
// specified.
80-
query = `
81-
SELECT
82-
vm.module_path,
83-
vm.requested_version,
84-
vm.resolved_version,
85-
vm.status,
86-
vm.error
87-
FROM
88-
packages p
89-
INNER JOIN
90-
version_map vm
91-
ON
92-
p.module_path = vm.module_path
93-
AND p.version = vm.resolved_version
94-
WHERE
95-
p.tsv_parent_directories @@ $1::tsquery
96-
ORDER BY
97-
module_path DESC,
98-
sort_version DESC
99-
LIMIT 1;`
100-
args = []interface{}{path}
101-
} else if requestedVersion != internal.LatestVersion && modulePath == internal.UnknownModulePath {
102-
// Return the version_map for the specified requested version at the
103-
// longest module path.
104-
query = `
105-
SELECT
106-
vm.module_path,
107-
vm.requested_version,
108-
vm.resolved_version,
109-
vm.status,
110-
vm.error
111-
FROM
112-
packages p
113-
INNER JOIN
114-
version_map vm
115-
ON
116-
p.module_path = vm.module_path
117-
AND p.version = vm.resolved_version
118-
WHERE
119-
p.tsv_parent_directories @@ $1::tsquery
120-
AND vm.requested_version = $2
121-
ORDER BY
122-
module_path DESC;`
123-
args = []interface{}{path, requestedVersion}
124-
} else if requestedVersion == internal.LatestVersion && modulePath != internal.UnknownModulePath {
125-
// Return the version map for the latest resolved_version and
126-
// specified module_path.
127-
query = `
128-
SELECT
129-
module_path,
130-
requested_version,
131-
resolved_version,
132-
status,
133-
error
134-
FROM
135-
version_map vm
136-
WHERE
137-
module_path = $1
138-
ORDER BY
139-
sort_version DESC
140-
LIMIT 1;`
141-
args = []interface{}{modulePath}
142-
} else {
143-
// Return the version_map for the specified requested version and module path.
144-
query = `
145-
SELECT
146-
module_path,
147-
requested_version,
148-
resolved_version,
149-
status,
150-
error
151-
FROM
152-
version_map
153-
WHERE
154-
module_path=$1
155-
AND requested_version=$2;`
156-
args = []interface{}{modulePath, requestedVersion}
65+
func (db *DB) GetVersionMap(ctx context.Context, modulePath, requestedVersion string) (_ *internal.VersionMap, err error) {
66+
defer derrors.Wrap(&err, "DB.GetVersionMap(ctx, tx, %q, %q)", modulePath, requestedVersion)
67+
if modulePath == internal.UnknownModulePath {
68+
return nil, fmt.Errorf("modulePath must be specified: %w", derrors.InvalidArgument)
15769
}
70+
71+
query := `
72+
SELECT
73+
module_path,
74+
requested_version,
75+
resolved_version,
76+
status,
77+
error
78+
FROM
79+
version_map
80+
WHERE
81+
module_path=$1
82+
AND requested_version=$2;`
15883
var vm internal.VersionMap
159-
switch db.db.QueryRow(ctx, query, args...).Scan(
160-
&vm.ModulePath, &vm.RequestedVersion, &vm.ResolvedVersion, &vm.Status, &vm.Error) {
84+
err = db.db.QueryRow(ctx, query, modulePath, requestedVersion).Scan(
85+
&vm.ModulePath, &vm.RequestedVersion, &vm.ResolvedVersion, &vm.Status, &vm.Error)
86+
switch err {
16187
case nil:
16288
return &vm, nil
16389
case sql.ErrNoRows:

‎internal/postgres/version_map_test.go

Lines changed: 12 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ import (
1010

1111
"github.com/google/go-cmp/cmp"
1212
"golang.org/x/pkgsite/internal"
13+
"golang.org/x/pkgsite/internal/experiment"
1314
"golang.org/x/pkgsite/internal/testing/sample"
1415
)
1516

1617
func TestReadAndWriteVersionMap(t *testing.T) {
1718
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
1819
defer cancel()
20+
ctx = experiment.NewContext(ctx, experiment.NewSet(map[string]bool{
21+
internal.ExperimentInsertDirectories: true,
22+
}))
1923
defer ResetTestDB(testDB, t)
2024

2125
m := sample.Module("golang.org/x/tools", sample.VersionString, "go/packages")
@@ -30,97 +34,16 @@ func TestReadAndWriteVersionMap(t *testing.T) {
3034
ResolvedVersion: "v1.0.0",
3135
Status: 200,
3236
}
33-
err = testDB.UpsertVersionMap(ctx, vm)
34-
if err != nil {
37+
if err = testDB.UpsertVersionMap(ctx, vm); err != nil {
3538
t.Fatal(err)
3639
}
3740

38-
for _, test := range []struct {
39-
name, path, modulePath, version string
40-
}{
41-
{
42-
name: "package path - latest version, unknown module",
43-
path: "golang.org/x/tools/go/packages",
44-
modulePath: internal.UnknownModulePath,
45-
version: internal.LatestVersion,
46-
},
47-
{
48-
name: "package path - latest version, known module",
49-
path: "golang.org/x/tools/go/packages",
50-
modulePath: "golang.org/x/tools",
51-
version: internal.LatestVersion,
52-
},
53-
{
54-
name: "package path - specified version, unknown module",
55-
path: "golang.org/x/tools/go/packages",
56-
modulePath: internal.UnknownModulePath,
57-
version: "master",
58-
},
59-
{
60-
name: "package path - specified version, known module",
61-
path: "golang.org/x/tools/go/packages",
62-
modulePath: "golang.org/x/tools",
63-
version: "master",
64-
},
65-
{
66-
name: "directory path - latest version, unknown module",
67-
path: "golang.org/x/tools/go",
68-
modulePath: internal.UnknownModulePath,
69-
version: internal.LatestVersion,
70-
},
71-
{
72-
name: "directory path - latest version, known module",
73-
path: "golang.org/x/tools/go",
74-
modulePath: "golang.org/x/tools",
75-
version: internal.LatestVersion,
76-
},
77-
{
78-
name: "directory path - specified version, unknown module",
79-
path: "golang.org/x/tools/go",
80-
modulePath: internal.UnknownModulePath,
81-
version: "master",
82-
},
83-
{
84-
name: "directory path - specified version, known module",
85-
path: "golang.org/x/tools/go",
86-
modulePath: "golang.org/x/tools",
87-
version: "master",
88-
},
89-
{
90-
name: "module path - latest version, unknown module",
91-
path: "golang.org/x/tools",
92-
modulePath: internal.UnknownModulePath,
93-
version: internal.LatestVersion,
94-
},
95-
{
96-
name: "module path - latest version, known module",
97-
path: "golang.org/x/tools",
98-
modulePath: "golang.org/x/tools",
99-
version: internal.LatestVersion,
100-
},
101-
{
102-
name: "module path - specified version, unknown module",
103-
path: "golang.org/x/tools",
104-
modulePath: internal.UnknownModulePath,
105-
version: "master",
106-
},
107-
{
108-
name: "module path - specified version, known module",
109-
path: "golang.org/x/tools",
110-
modulePath: "golang.org/x/tools",
111-
version: "master",
112-
},
113-
} {
114-
t.Run(test.name, func(t *testing.T) {
115-
got, err := testDB.GetVersionMap(ctx, test.path, test.modulePath, test.version)
116-
if err != nil {
117-
t.Fatal(err)
118-
}
119-
if diff := cmp.Diff(vm, got); diff != "" {
120-
t.Errorf("t.Errorf(ctx, %q, %q, %q) mismatch (-want +got):\n%s",
121-
test.path, test.modulePath, test.version, diff)
122-
}
123-
})
41+
got, err := testDB.GetVersionMap(ctx, vm.ModulePath, vm.RequestedVersion)
42+
if err != nil {
43+
t.Fatal(err)
44+
}
45+
if diff := cmp.Diff(vm, got); diff != "" {
46+
t.Fatalf("t.Errorf(ctx, %q, %q) mismatch (-want +got):\n%s", vm.ModulePath, vm.RequestedVersion, diff)
12447
}
12548
}
12649
func TestUpsertVersionMap(t *testing.T) {
@@ -133,7 +56,7 @@ func TestUpsertVersionMap(t *testing.T) {
13356
if err != nil {
13457
t.Fatal(err)
13558
}
136-
got, err := testDB.GetVersionMap(ctx, vm.ModulePath, vm.ModulePath, vm.RequestedVersion)
59+
got, err := testDB.GetVersionMap(ctx, vm.ModulePath, vm.RequestedVersion)
13760
if err != nil {
13861
t.Fatal(err)
13962
}

‎internal/worker/fetch_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@ func TestFetchAndUpdateState_NotFound(t *testing.T) {
8080
if vs.Status != want {
8181
t.Fatalf("testDB.GetModuleVersionState(ctx, %q, %q): status = %v, want = %d", modulePath, version, vs.Status, want)
8282
}
83-
vm, err := testDB.GetVersionMap(ctx, modulePath, modulePath, version)
83+
vm, err := testDB.GetVersionMap(ctx, modulePath, version)
8484
if err != nil {
8585
t.Fatal(err)
8686
}
8787
if vm.Status != want {
88-
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q, %q): status = %d, want = %d", modulePath, modulePath, version, vm.Status, want)
88+
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q): status = %d, want = %d", modulePath, version, vm.Status, want)
8989
}
9090
}
9191

@@ -203,12 +203,12 @@ func checkModuleNotFound(t *testing.T, ctx context.Context, modulePath, version
203203
if vs.Status != wantCode {
204204
t.Fatalf("testDB.GetModuleVersionState(ctx, %q, %q): status=%v, want %d", modulePath, version, vs.Status, wantCode)
205205
}
206-
vm, err := testDB.GetVersionMap(ctx, modulePath, modulePath, version)
206+
vm, err := testDB.GetVersionMap(ctx, modulePath, version)
207207
if err != nil {
208208
t.Fatal(err)
209209
}
210210
if vm.Status != wantCode {
211-
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q, %q): status=%v, want %d", modulePath, modulePath, version, vm.Status, wantCode)
211+
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q): status=%v, want %d", modulePath, version, vm.Status, wantCode)
212212
}
213213
}
214214

@@ -236,12 +236,12 @@ func TestFetchAndUpdateState_BadRequestedVersion(t *testing.T) {
236236
if !errors.Is(err, derrors.NotFound) {
237237
t.Fatal(err)
238238
}
239-
vm, err := testDB.GetVersionMap(ctx, modulePath, modulePath, version)
239+
vm, err := testDB.GetVersionMap(ctx, modulePath, version)
240240
if err != nil {
241241
t.Fatal(err)
242242
}
243243
if vm.Status != http.StatusNotFound {
244-
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q, %q): status=%v, want %d", modulePath, modulePath, version, vm.Status, http.StatusNotFound)
244+
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q): status=%v, want %d", modulePath, version, vm.Status, http.StatusNotFound)
245245
}
246246
}
247247

@@ -276,12 +276,12 @@ func TestFetchAndUpdateState_Incomplete(t *testing.T) {
276276
if vs.Status != want {
277277
t.Fatalf("testDB.GetModuleVersionState(ctx, %q, %q): status=%v, want %d", modulePath, version, vs.Status, want)
278278
}
279-
vm, err := testDB.GetVersionMap(ctx, modulePath, modulePath, version)
279+
vm, err := testDB.GetVersionMap(ctx, modulePath, version)
280280
if err != nil {
281281
t.Fatal(err)
282282
}
283283
if vm.Status != want {
284-
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q, %q): status=%v, want %d", modulePath, modulePath, version, vm.Status, want)
284+
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q): status=%v, want %d", modulePath, version, vm.Status, want)
285285
}
286286
gotStates, err := testDB.GetPackageVersionStatesForModule(ctx, modulePath, version)
287287
if err != nil {
@@ -357,12 +357,12 @@ func TestFetchAndUpdateState_Mismatch(t *testing.T) {
357357
t.Errorf("testDB.GetModuleVersionState(ctx, %q, %q): goModPath=%q, want %q", modulePath, version, vs.GoModPath, goModPath)
358358
}
359359

360-
vm, err := testDB.GetVersionMap(ctx, modulePath, modulePath, version)
360+
vm, err := testDB.GetVersionMap(ctx, modulePath, version)
361361
if err != nil {
362362
t.Fatal(err)
363363
}
364364
if vm.Status != wantCode {
365-
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q, %q): status=%v, want %d", modulePath, modulePath, version, vm.Status, wantCode)
365+
t.Fatalf("testDB.GetVersionMap(ctx, %q, %q): status=%v, want %d", modulePath, version, vm.Status, wantCode)
366366
}
367367
}
368368

0 commit comments

Comments
 (0)