internal: remove methods from DataSource
At the moment there are several methods not implemented by the
proxydatasource. These are removed from internal.DataSource.
Fixes b/150138536
Change-Id: Id1eef4b2497bd46c8e3cecf8a083ef81bfbe2f47
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/768688
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/datasource.go b/internal/datasource.go
index e844143..99e2a48 100644
--- a/internal/datasource.go
+++ b/internal/datasource.go
@@ -15,17 +15,9 @@
// See the internal/postgres package for further documentation of these
// methods, particularly as they pertain to the main postgres implementation.
- // IsExcluded reports whether the path is excluded from processinng.
- IsExcluded(ctx context.Context, path string) (bool, error)
- // Search searches the database with a query.
- Search(ctx context.Context, query string, limit, offset int) ([]*SearchResult, error)
-
// GetDirectoryNew returns information about a directory, which may also be a module and/or package.
// The module and version must both be known.
GetDirectoryNew(ctx context.Context, dirPath, modulePath, version string) (_ *VersionedDirectory, err error)
- // GetImportedBy returns a slice of import paths corresponding to packages
- // that import the given package path (at any version).
- GetImportedBy(ctx context.Context, pkgPath, version string, limit int) ([]string, error)
// GetImports returns a slice of import paths imported by the package
// specified by path and version.
GetImports(ctx context.Context, pkgPath, modulePath, version string) ([]string, error)
@@ -47,10 +39,6 @@
// GetTaggedVersionsForModule returns LegacyModuleInfo for all known tagged
// versions for any module containing a package with the given import path.
GetTaggedVersionsForPackageSeries(ctx context.Context, pkgPath string) ([]*LegacyModuleInfo, error)
- // GetVersionMap returns the VersionMap corresponding to the provided modulePath and requestedVersion.
- GetVersionMap(ctx context.Context, modulePath, requestedVersion string) (*VersionMap, error)
- // GetStdlibPathsWithSuffix returns standard library paths with the given suffix.
- GetStdlibPathsWithSuffix(ctx context.Context, suffix string) ([]string, error)
// TODO(b/155474770): Deprecate these methods.
//
diff --git a/internal/frontend/details.go b/internal/frontend/details.go
index 4c1acba..149a6a7 100644
--- a/internal/frontend/details.go
+++ b/internal/frontend/details.go
@@ -16,6 +16,7 @@
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/experiment"
+ "golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
)
@@ -138,7 +139,11 @@
},
}
}
- excluded, err := ds.IsExcluded(ctx, path)
+ db, ok := ds.(*postgres.DB)
+ if !ok {
+ return nil
+ }
+ excluded, err := db.IsExcluded(ctx, path)
if err != nil {
return err
}
diff --git a/internal/frontend/details_test.go b/internal/frontend/details_test.go
index b5e8131..8b54e11 100644
--- a/internal/frontend/details_test.go
+++ b/internal/frontend/details_test.go
@@ -8,7 +8,6 @@
"context"
"net/http"
"net/url"
- "strings"
"testing"
"golang.org/x/pkgsite/internal"
@@ -108,7 +107,6 @@
want int
}{
{"import/path", "v1.2.3", http.StatusOK},
- {"bad/path", "v1.2.3", http.StatusNotFound},
{"import/path", "v1.2.bad", http.StatusBadRequest},
}
@@ -131,7 +129,3 @@
type fakeDataSource struct {
internal.DataSource
}
-
-func (fakeDataSource) IsExcluded(_ context.Context, path string) (bool, error) {
- return strings.HasPrefix(path, "bad"), nil
-}
diff --git a/internal/frontend/fetch.go b/internal/frontend/fetch.go
index b0b7ed8..2d212e6 100644
--- a/internal/frontend/fetch.go
+++ b/internal/frontend/fetch.go
@@ -47,6 +47,11 @@
// TODO(golang/go#37002): This should be a POST request, since it is causing a change in state.
// update middleware.AcceptMethods so that this can be a POST instead of a GET.
func (s *Server) fetchHandler(w http.ResponseWriter, r *http.Request) {
+ if _, ok := s.ds.(*postgres.DB); !ok {
+ // There's no reason for the proxydatasource to need this codepath.
+ http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
+ return
+ }
if !isActiveFrontendFetch(r.Context()) {
// If the experiment flag is not on, treat this as a request for the
// "fetch" package, which does not exist.
@@ -93,7 +98,8 @@
}
// Generate all possible module paths for the fullPath.
- modulePaths, err := modulePathsToFetch(parentCtx, s.ds, fullPath, modulePath)
+ db := s.ds.(*postgres.DB)
+ modulePaths, err := modulePathsToFetch(parentCtx, db, fullPath, modulePath)
if err != nil {
return derrors.ToHTTPStatus(err), err.Error()
}
@@ -165,7 +171,8 @@
// Before enqueuing the module version to be fetched, check if we have
// already attempted to fetch it in the past. If so, just return the result
// from that fetch process.
- fr = checkForPath(ctx, s.ds, fullPath, modulePath, requestedVersion)
+ db := s.ds.(*postgres.DB)
+ fr = checkForPath(ctx, db, fullPath, modulePath, requestedVersion)
if fr.status == http.StatusOK {
return fr
}
@@ -184,11 +191,11 @@
}
// After the fetch request is enqueued, poll the database until it has been
// inserted or the request times out.
- return pollForPath(ctx, s.ds, pollEvery, fullPath, modulePath, requestedVersion)
+ return pollForPath(ctx, db, pollEvery, fullPath, modulePath, requestedVersion)
}
// pollForPath polls the database until a row for fullPath is found.
-func pollForPath(ctx context.Context, ds internal.DataSource, pollEvery time.Duration,
+func pollForPath(ctx context.Context, db *postgres.DB, pollEvery time.Duration,
fullPath, modulePath, requestedVersion string) *fetchResult {
fr := &fetchResult{modulePath: modulePath}
defer derrors.Wrap(&fr.err, "pollForRedirectURL(%q, %q, %q)", modulePath, fullPath, requestedVersion)
@@ -204,7 +211,7 @@
case <-ticker.C:
ctx2, cancel := context.WithTimeout(ctx, pollEvery)
defer cancel()
- fr = checkForPath(ctx2, ds, fullPath, modulePath, requestedVersion)
+ fr = checkForPath(ctx2, db, fullPath, modulePath, requestedVersion)
if fr.status != http.StatusProcessing {
return fr
}
@@ -218,7 +225,7 @@
// process that was initiated is not yet complete. If the row exists version_map
// but not paths, it means that a module was found at the requestedVersion, but
// not the fullPath, so errPathDoesNotExistInModule is returned.
-func checkForPath(ctx context.Context, ds internal.DataSource, fullPath, modulePath, requestedVersion string) (fr *fetchResult) {
+func checkForPath(ctx context.Context, db *postgres.DB, fullPath, modulePath, requestedVersion string) (fr *fetchResult) {
defer func() {
// Based on
// https://github.com/lib/pq/issues/577#issuecomment-298341053, it seems
@@ -235,7 +242,7 @@
// Check the version_map table to see if a row exists for modulePath and
// requestedVersion.
- vm, err := ds.GetVersionMap(ctx, modulePath, requestedVersion)
+ vm, err := db.GetVersionMap(ctx, modulePath, requestedVersion)
if err != nil {
// If an error is returned, there are two possibilities:
// (1) A row for this modulePath and version does not exist.
@@ -296,7 +303,7 @@
// was 200 or 290). Now check the paths table to see if the fullPath exists.
// vm.status for the module version was either a 200 or 290. Now determine if
// the fullPath exists in that module.
- if _, _, _, err := ds.GetPathInfo(ctx, fullPath, modulePath, vm.ResolvedVersion); err != nil {
+ if _, _, _, err := db.GetPathInfo(ctx, fullPath, modulePath, vm.ResolvedVersion); err != nil {
if errors.Is(err, derrors.NotFound) {
// The module version exists, but the fullPath does not exist in
// that module version.
diff --git a/internal/frontend/imports.go b/internal/frontend/imports.go
index a04b36c..3e8ecd5 100644
--- a/internal/frontend/imports.go
+++ b/internal/frontend/imports.go
@@ -9,6 +9,7 @@
"strings"
"golang.org/x/pkgsite/internal"
+ "golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
)
@@ -72,10 +73,10 @@
const importedByLimit = 20001
-// fetchImportedByDetails fetches importers for the package version specified by
+// etchImportedByDetails fetches importers for the package version specified by
// path and version from the database and returns a ImportedByDetails.
-func fetchImportedByDetails(ctx context.Context, ds internal.DataSource, pkgPath, modulePath string) (*ImportedByDetails, error) {
- importedBy, err := ds.GetImportedBy(ctx, pkgPath, modulePath, importedByLimit)
+func fetchImportedByDetails(ctx context.Context, db *postgres.DB, pkgPath, modulePath string) (*ImportedByDetails, error) {
+ importedBy, err := db.GetImportedBy(ctx, pkgPath, modulePath, importedByLimit)
if err != nil {
return nil, err
}
diff --git a/internal/frontend/package.go b/internal/frontend/package.go
index 75813ec..5335ccc 100644
--- a/internal/frontend/package.go
+++ b/internal/frontend/package.go
@@ -14,6 +14,7 @@
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/log"
+ "golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
)
@@ -209,7 +210,11 @@
if !stdlib.Contains(shortcut) {
return "", nil
}
- matches, err := s.ds.GetStdlibPathsWithSuffix(ctx, shortcut)
+ db, ok := s.ds.(*postgres.DB)
+ if !ok {
+ return "", &serverError{status: http.StatusFailedDependency}
+ }
+ matches, err := db.GetStdlibPathsWithSuffix(ctx, shortcut)
if err != nil {
return "", err
}
diff --git a/internal/frontend/search.go b/internal/frontend/search.go
index 2f44305..69fa748 100644
--- a/internal/frontend/search.go
+++ b/internal/frontend/search.go
@@ -16,6 +16,7 @@
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/log"
+ "golang.org/x/pkgsite/internal/postgres"
)
const defaultSearchLimit = 10
@@ -43,9 +44,8 @@
// fetchSearchPage fetches data matching the search query from the database and
// returns a SearchPage.
-func fetchSearchPage(ctx context.Context, ds internal.DataSource, query string, pageParams paginationParams) (*SearchPage, error) {
-
- dbresults, err := ds.Search(ctx, query, pageParams.limit, pageParams.offset())
+func fetchSearchPage(ctx context.Context, db *postgres.DB, query string, pageParams paginationParams) (*SearchPage, error) {
+ dbresults, err := db.Search(ctx, query, pageParams.limit, pageParams.offset())
if err != nil {
return nil, err
}
@@ -103,6 +103,12 @@
// /search?q=<query>. If <query> is an exact match for a package path, the user
// will be redirected to the details page.
func (s *Server) serveSearch(w http.ResponseWriter, r *http.Request) error {
+ db, ok := s.ds.(*postgres.DB)
+ if !ok {
+ // The proxydatasource does not support the imported by page.
+ return &serverError{status: http.StatusFailedDependency}
+ }
+
ctx := r.Context()
query := searchQuery(r)
if query == "" {
@@ -114,8 +120,7 @@
http.Redirect(w, r, path, http.StatusFound)
return nil
}
-
- page, err := fetchSearchPage(ctx, s.ds, query, newPaginationParams(r, defaultSearchLimit))
+ page, err := fetchSearchPage(ctx, db, query, newPaginationParams(r, defaultSearchLimit))
if err != nil {
return fmt.Errorf("fetchSearchPage(ctx, db, %q): %v", query, err)
}
diff --git a/internal/frontend/tabs.go b/internal/frontend/tabs.go
index 30faa36..73ae83b 100644
--- a/internal/frontend/tabs.go
+++ b/internal/frontend/tabs.go
@@ -13,6 +13,7 @@
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/licenses"
+ "golang.org/x/pkgsite/internal/postgres"
)
// TabSettings defines tab-specific metadata.
@@ -152,7 +153,12 @@
case "imports":
return fetchImportsDetails(ctx, ds, pkg.Path, pkg.ModulePath, pkg.Version)
case "importedby":
- return fetchImportedByDetails(ctx, ds, pkg.Path, pkg.ModulePath)
+ db, ok := ds.(*postgres.DB)
+ if !ok {
+ // The proxydatasource does not support the imported by page.
+ return nil, &serverError{status: http.StatusFailedDependency}
+ }
+ return fetchImportedByDetails(ctx, db, pkg.Path, pkg.ModulePath)
case "licenses":
return fetchPackageLicensesDetails(ctx, ds, pkg.Path, pkg.ModulePath, pkg.Version)
case "overview":
@@ -175,7 +181,12 @@
case "imports":
return fetchImportsDetails(ctx, ds, vdir.Path, vdir.ModulePath, vdir.Version)
case "importedby":
- return fetchImportedByDetails(ctx, ds, vdir.Path, vdir.ModulePath)
+ db, ok := ds.(*postgres.DB)
+ if !ok {
+ // The proxydatasource does not support the imported by page.
+ return nil, &serverError{status: http.StatusFailedDependency}
+ }
+ return fetchImportedByDetails(ctx, db, vdir.Path, vdir.ModulePath)
case "licenses":
return fetchPackageLicensesDetails(ctx, ds, vdir.Path, vdir.ModulePath, vdir.Version)
case "overview":
diff --git a/internal/proxydatasource/datasource.go b/internal/proxydatasource/datasource.go
index 670e5ea..2f4eb74 100644
--- a/internal/proxydatasource/datasource.go
+++ b/internal/proxydatasource/datasource.go
@@ -104,11 +104,6 @@
}, nil
}
-// GetImportedBy is unimplemented.
-func (ds *DataSource) GetImportedBy(ctx context.Context, path, version string, limit int) (_ []string, err error) {
- return nil, nil
-}
-
// GetImports returns package imports as extracted from the module zip.
func (ds *DataSource) GetImports(ctx context.Context, pkgPath, modulePath, version string) (_ []string, err error) {
defer derrors.Wrap(&err, "GetImports(%q, %q, %q)", pkgPath, modulePath, version)
@@ -232,11 +227,6 @@
return &m.LegacyModuleInfo, nil
}
-// Search is unimplemented.
-func (ds *DataSource) Search(ctx context.Context, query string, limit, offset int) ([]*internal.SearchResult, error) {
- return []*internal.SearchResult{}, nil
-}
-
// getModule retrieves a version from the cache, or failing that queries and
// processes the version from the proxy.
func (ds *DataSource) getModule(ctx context.Context, modulePath, version string) (_ *internal.Module, err error) {
@@ -420,11 +410,6 @@
return nil, fmt.Errorf("package missing from module %s: %w", m.ModulePath, derrors.NotFound)
}
-// IsExcluded is unimplemented.
-func (*DataSource) IsExcluded(context.Context, string) (bool, error) {
- return false, nil
-}
-
// GetExperiments is unimplemented.
func (*DataSource) GetExperiments(ctx context.Context) ([]*internal.Experiment, error) {
return nil, nil
@@ -455,14 +440,3 @@
}
return m.ModulePath, m.Version, isPackage, nil
}
-
-// GetVersionMap is unimplemented. The proxydatasource does not have any need
-// for this method.
-func (ds *DataSource) GetVersionMap(ctx context.Context, modulePath, version string) (*internal.VersionMap, error) {
- return nil, nil
-}
-
-// GetStdlibPathsWithSuffix is unimplemented.
-func (ds *DataSource) GetStdlibPathsWithSuffix(ctx context.Context, suffix string) ([]string, error) {
- return nil, nil
-}
