internal/datasource: factor out getters
Move the list of getters into the common dataSource implementation.
Also, have the constructor take the list getters initially. It
simplifies the state of the dataSource (no chance of someone adding a
getter after some modules have been fetched).
For golang/go#47780
Change-Id: I08b005da13b7bd1ad784ceaf30d722da634009b2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/344669
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/cmd/pkgsite/main.go b/cmd/pkgsite/main.go
index 1d82356..184523b 100644
--- a/cmd/pkgsite/main.go
+++ b/cmd/pkgsite/main.go
@@ -67,7 +67,8 @@
}
func newServer(ctx context.Context, paths []string, gopathMode bool) (*frontend.Server, error) {
- lds := datasource.NewLocal(source.NewClient(time.Second))
+ getters := buildGetters(ctx, paths, gopathMode)
+ lds := datasource.NewLocal(getters, source.NewClient(time.Second))
server, err := frontend.NewServer(frontend.ServerConfig{
DataSourceGetter: func(context.Context) internal.DataSource { return lds },
StaticPath: template.TrustedSourceFromFlag(flag.Lookup("static").Value),
@@ -75,12 +76,11 @@
if err != nil {
return nil, err
}
- addGetters(ctx, lds, paths, gopathMode)
return server, nil
}
-// load loads local modules from pathList.
-func addGetters(ctx context.Context, ds *datasource.LocalDataSource, paths []string, gopathMode bool) {
+func buildGetters(ctx context.Context, paths []string, gopathMode bool) []fetch.ModuleGetter {
+ var getters []fetch.ModuleGetter
loaded := len(paths)
for _, path := range paths {
var (
@@ -96,11 +96,12 @@
log.Error(ctx, err)
loaded--
} else {
- ds.AddModuleGetter(mg)
+ getters = append(getters, mg)
}
}
if loaded == 0 {
log.Fatalf(ctx, "failed to load module(s) at %v", paths)
}
+ return getters
}
diff --git a/internal/datasource/datasource.go b/internal/datasource/datasource.go
index ff6b4da..634ebd0 100644
--- a/internal/datasource/datasource.go
+++ b/internal/datasource/datasource.go
@@ -17,9 +17,9 @@
// dataSource implements the internal.DataSource interface, by trying a list of
// fetch.ModuleGetters to fetch modules and caching the results.
type dataSource struct {
+ getters []fetch.ModuleGetter
sourceClient *source.Client
-
- cache *lru.Cache
+ cache *lru.Cache
}
// cacheEntry holds a fetched module or an error, if the fetch failed.
@@ -30,13 +30,14 @@
const maxCachedModules = 100
-func newDataSource(sc *source.Client) *dataSource {
+func newDataSource(getters []fetch.ModuleGetter, sc *source.Client) *dataSource {
cache, err := lru.New(maxCachedModules)
if err != nil {
// Can only happen if size is bad.
panic(err)
}
return &dataSource{
+ getters: getters,
sourceClient: sc,
cache: cache,
}
diff --git a/internal/datasource/datasource_test.go b/internal/datasource/datasource_test.go
index feadae6..34213be 100644
--- a/internal/datasource/datasource_test.go
+++ b/internal/datasource/datasource_test.go
@@ -13,7 +13,7 @@
)
func TestCache(t *testing.T) {
- ds := newDataSource(nil)
+ ds := newDataSource(nil, nil)
m1 := &internal.Module{}
ds.cachePut("m1", fetch.LocalVersion, m1, nil)
ds.cachePut("m2", "v1.0.0", nil, derrors.NotFound)
diff --git a/internal/datasource/local.go b/internal/datasource/local.go
index 5c1eda8..923bb56 100644
--- a/internal/datasource/local.go
+++ b/internal/datasource/local.go
@@ -11,7 +11,6 @@
"os"
"path/filepath"
"strings"
- "sync"
"time"
"golang.org/x/pkgsite/internal"
@@ -25,30 +24,18 @@
// locally. It is not backed by a database or a proxy instance.
type LocalDataSource struct {
sourceClient *source.Client
-
- mu sync.Mutex
- getters []fetch.ModuleGetter
- ds *dataSource
+ ds *dataSource
}
// New creates and returns a new local datasource that bypasses license
// checks by default.
-func NewLocal(sc *source.Client) *LocalDataSource {
+func NewLocal(getters []fetch.ModuleGetter, sc *source.Client) *LocalDataSource {
return &LocalDataSource{
sourceClient: sc,
- ds: newDataSource(sc),
+ ds: newDataSource(getters, sc),
}
}
-// AddModuleGetter adds a module getter to the DataSource. To look up a module,
-// the getters are tried in the order they were added until the desired module
-// is found.
-func (ds *LocalDataSource) AddModuleGetter(g fetch.ModuleGetter) {
- ds.mu.Lock()
- defer ds.mu.Unlock()
- ds.getters = append(ds.getters, g)
-}
-
// getModule gets the module at the given path and version. It first checks the
// cache, and if it isn't there it then tries to fetch it.
func (ds *LocalDataSource) getModule(ctx context.Context, path, version string) (*internal.Module, error) {
@@ -69,7 +56,7 @@
defer func() {
log.Infof(ctx, "local DataSource: fetched %s@%s in %s with error %v", modulePath, version, time.Since(start), err)
}()
- for _, g := range ds.getters {
+ for _, g := range ds.ds.getters {
fr := fetch.FetchModule(ctx, modulePath, version, g, ds.sourceClient)
if fr.Error == nil {
adjust(fr.Module)
diff --git a/internal/datasource/local_test.go b/internal/datasource/local_test.go
index f1baab0..ffef9ab 100644
--- a/internal/datasource/local_test.go
+++ b/internal/datasource/local_test.go
@@ -69,8 +69,10 @@
},
}
- datasource = NewLocal(source.NewClientForTesting())
- var dirs []string
+ var (
+ dirs []string
+ getters []fetch.ModuleGetter
+ )
for _, module := range modules {
directory, err := testhelper.CreateTestDirectory(module)
if err != nil {
@@ -81,8 +83,10 @@
if err != nil {
log.Fatal(err)
}
- datasource.AddModuleGetter(mg)
+ getters = append(getters, mg)
}
+ datasource = NewLocal(getters, source.NewClientForTesting())
+
return func() {
for _, d := range dirs {
os.RemoveAll(d)
diff --git a/internal/datasource/proxy.go b/internal/datasource/proxy.go
index 451ab26..e4d13ca 100644
--- a/internal/datasource/proxy.go
+++ b/internal/datasource/proxy.go
@@ -35,10 +35,10 @@
}
func newProxyDataSource(proxyClient *proxy.Client, sourceClient *source.Client) *ProxyDataSource {
+ ds := newDataSource([]fetch.ModuleGetter{fetch.NewProxyModuleGetter(proxyClient)}, sourceClient)
return &ProxyDataSource{
- ds: newDataSource(sourceClient),
- proxyClient: proxyClient,
-
+ ds: ds,
+ proxyClient: proxyClient,
modulePathToVersions: make(map[string][]string),
packagePathToModules: make(map[string][]string),
bypassLicenseCheck: false,
@@ -83,7 +83,7 @@
if mod != nil || err != nil {
return mod, err
}
- res := fetch.FetchModule(ctx, modulePath, version, fetch.NewProxyModuleGetter(ds.proxyClient), ds.ds.sourceClient)
+ res := fetch.FetchModule(ctx, modulePath, version, ds.ds.getters[0], ds.ds.sourceClient)
defer res.Defer()
m := res.Module
if m != nil {
