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>

internal/datasource.go

@@ -15,17 +15,9 @@ type DataSource interface {
 	// 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 @@ type DataSource interface {
 	// 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.
 	//

internal/frontend/details.go

@@ -16,6 +16,7 @@ import (
 	"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 @@ func checkPathAndVersion(ctx context.Context, ds internal.DataSource, path, vers
 			},
 		}
 	}
-	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
 	}

internal/frontend/details_test.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"net/http"
 	"net/url"
-	"strings"
 	"testing"
 
 	"golang.org/x/pkgsite/internal"
@@ -108,7 +107,6 @@ func TestCheckPathAndVersion(t *testing.T) {
 		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 @@ func TestCheckPathAndVersion(t *testing.T) {
 type fakeDataSource struct {
 	internal.DataSource
 }
-
-func (fakeDataSource) IsExcluded(_ context.Context, path string) (bool, error) {
-	return strings.HasPrefix(path, "bad"), nil
-}

internal/frontend/fetch.go

@@ -47,6 +47,11 @@ var (
 // 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 @@ func (s *Server) fetchAndPoll(parentCtx context.Context, modulePath, fullPath, r
 	}
 
 	// 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 @@ func (s *Server) fetchModule(ctx context.Context, fullPath, modulePath, requeste
 	// 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 @@ func (s *Server) fetchModule(ctx context.Context, fullPath, modulePath, requeste
 	}
 	// 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 @@ func pollForPath(ctx context.Context, ds internal.DataSource, pollEvery time.Dur
 		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 @@ func pollForPath(ctx context.Context, ds internal.DataSource, pollEvery time.Dur
 // 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 @@ func checkForPath(ctx context.Context, ds internal.DataSource, fullPath, moduleP
 
 	// 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 @@ func checkForPath(ctx context.Context, ds internal.DataSource, fullPath, moduleP
 	// 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.

internal/frontend/imports.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"golang.org/x/pkgsite/internal"
+	"golang.org/x/pkgsite/internal/postgres"
 	"golang.org/x/pkgsite/internal/stdlib"
 )
 
@@ -72,10 +73,10 @@ type ImportedByDetails struct {
 
 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
 	}

internal/frontend/package.go

@@ -14,6 +14,7 @@ import (
 	"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 @@ func (s *Server) stdlibPathForShortcut(ctx context.Context, shortcut string) (pa
 	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
 	}

internal/frontend/search.go

@@ -16,6 +16,7 @@ import (
 	"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 @@ type SearchResult struct {
 
 // 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 @@ func approximateNumber(estimate int, sigma float64) int {
 // /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 @@ func (s *Server) serveSearch(w http.ResponseWriter, r *http.Request) error {
 		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)
 	}

internal/frontend/tabs.go

@@ -13,6 +13,7 @@ import (
 
 	"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 @@ func fetchDetailsForPackage(ctx context.Context, r *http.Request, tab string, ds
 	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 @@ func fetchDetailsForVersionedDirectory(ctx context.Context, r *http.Request, tab
 	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":

internal/proxydatasource/datasource.go

@@ -104,11 +104,6 @@ func (ds *DataSource) GetDirectoryNew(ctx context.Context, dirPath, modulePath,
 	}, 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 @@ func (ds *DataSource) GetModuleInfo(ctx context.Context, modulePath, version str
 	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 @@ func packageFromVersion(pkgPath string, m *internal.Module) (_ *internal.LegacyV
 	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 @@ func (ds *DataSource) GetPathInfo(ctx context.Context, path, inModulePath, inVer
 	}
 	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
-}