diff options
Diffstat (limited to 'internal/postgres')
| -rw-r--r-- | internal/postgres/details_test.go | 12 | ||||
| -rw-r--r-- | internal/postgres/insert_module.go | 120 | ||||
| -rw-r--r-- | internal/postgres/insert_module_test.go | 2 | ||||
| -rw-r--r-- | internal/postgres/search.go | 10 |
4 files changed, 70 insertions, 74 deletions
diff --git a/internal/postgres/details_test.go b/internal/postgres/details_test.go index 6addc34e..b75ee92e 100644 --- a/internal/postgres/details_test.go +++ b/internal/postgres/details_test.go @@ -72,7 +72,7 @@ func TestPostgres_GetVersionInfo_Latest(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { for _, v := range tc.modules { - if err := testDB.saveModule(ctx, v); err != nil { + if err := saveModule(ctx, testDB.db, v); err != nil { t.Error(err) } } @@ -172,7 +172,7 @@ func TestPostgres_GetImportsAndImportedBy(t *testing.T) { defer cancel() for _, v := range testModules { - if err := testDB.saveModule(ctx, v); err != nil { + if err := saveModule(ctx, testDB.db, v); err != nil { t.Error(err) } } @@ -283,7 +283,7 @@ func TestPostgres_GetTaggedAndPseudoVersions(t *testing.T) { // TODO: move this handling into SimpleVersion once ParseVersionType is // factored out of fetch.go m.VersionType = version.TypePseudo - if err := testDB.saveModule(ctx, m); err != nil { + if err := saveModule(ctx, testDB.db, m); err != nil { t.Fatal(err) } @@ -299,7 +299,7 @@ func TestPostgres_GetTaggedAndPseudoVersions(t *testing.T) { } for _, m := range tc.modules { - if err := testDB.saveModule(ctx, m); err != nil { + if err := saveModule(ctx, testDB.db, m); err != nil { t.Fatal(err) } } @@ -360,7 +360,7 @@ func TestGetPackagesInVersion(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - if err := testDB.saveModule(ctx, tc.module); err != nil { + if err := saveModule(ctx, testDB.db, tc.module); err != nil { t.Error(err) } @@ -409,7 +409,7 @@ func TestGetPackageLicenses(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - if err := testDB.saveModule(ctx, testModule); err != nil { + if err := saveModule(ctx, testDB.db, testModule); err != nil { t.Fatal(err) } diff --git a/internal/postgres/insert_module.go b/internal/postgres/insert_module.go index 019d4c81..8c9768ca 100644 --- a/internal/postgres/insert_module.go +++ b/internal/postgres/insert_module.go @@ -42,42 +42,44 @@ func (db *DB) InsertModule(ctx context.Context, m *internal.Module) (err error) } removeNonDistributableData(m) - if err := db.saveModule(ctx, m); err != nil { - return err - } + return db.db.Transact(ctx, func(tx *database.DB) error { + if err := saveModule(ctx, tx, m); err != nil { + return err + } - // If there is a more recent version of this module that has an alternative - // module path, then do not insert its packages into search_documents. This - // happens when a module that initially does not have a go.mod file is - // forked or fetched via some non-canonical path (such as an alternative - // capitalization), and then in a later version acquires a go.mod file. - // - // To take an actual example: github.com/sirupsen/logrus@v1.1.0 has a go.mod - // file that establishes that path as canonical. But v1.0.6 does not have a - // go.mod file. So the miscapitalized path github.com/Sirupsen/logrus at - // v1.1.0 is marked as an alternative path (code 491) by - // internal/fetch.FetchModule and is not inserted into the DB, but at - // v1.0.6 it is considered valid, and we end up here. We still insert - // github.com/Sirupsen/logrus@v1.0.6 in the versions table and friends so - // that users who import it can find information about it, but we don't want - // it showing up in search results. - // - // Note that we end up here only if we first saw the alternative version - // (github.com/Sirupsen/logrus@v1.1.0 in the example) and then see the valid - // one. The "if code == 491" section of internal/worker.fetchAndUpdateState - // handles the case where we fetch the versions in the other order. - row := db.db.QueryRow(ctx, ` + // If there is a more recent version of this module that has an alternative + // module path, then do not insert its packages into search_documents. This + // happens when a module that initially does not have a go.mod file is + // forked or fetched via some non-canonical path (such as an alternative + // capitalization), and then in a later version acquires a go.mod file. + // + // To take an actual example: github.com/sirupsen/logrus@v1.1.0 has a go.mod + // file that establishes that path as canonical. But v1.0.6 does not have a + // go.mod file. So the miscapitalized path github.com/Sirupsen/logrus at + // v1.1.0 is marked as an alternative path (code 491) by + // internal/fetch.FetchModule and is not inserted into the DB, but at + // v1.0.6 it is considered valid, and we end up here. We still insert + // github.com/Sirupsen/logrus@v1.0.6 in the versions table and friends so + // that users who import it can find information about it, but we don't want + // it showing up in search results. + // + // Note that we end up here only if we first saw the alternative version + // (github.com/Sirupsen/logrus@v1.1.0 in the example) and then see the valid + // one. The "if code == 491" section of internal/worker.fetchAndUpdateState + // handles the case where we fetch the versions in the other order. + row := tx.QueryRow(ctx, ` SELECT 1 FROM module_version_states WHERE module_path = $1 AND sort_version > $2 and status = 491`, - m.ModulePath, version.ForSorting(m.Version)) - var x int - if err := row.Scan(&x); err != sql.ErrNoRows { - log.Infof(ctx, "%s@%s: not inserting into search documents", m.ModulePath, m.Version) - return err - } + m.ModulePath, version.ForSorting(m.Version)) + var x int + if err := row.Scan(&x); err != sql.ErrNoRows { + log.Infof(ctx, "%s@%s: not inserting into search documents", m.ModulePath, m.Version) + return err + } - // Insert the module's packages into search_documents. - return db.UpsertSearchDocuments(ctx, m) + // Insert the module's packages into search_documents. + return UpsertSearchDocuments(ctx, tx, m) + }) } // saveModule inserts a Module into the database along with its packages, @@ -87,31 +89,29 @@ func (db *DB) InsertModule(ctx context.Context, m *internal.Module) (err error) // // A derrors.InvalidArgument error will be returned if the given module and // licenses are invalid. -func (db *DB) saveModule(ctx context.Context, m *internal.Module) (err error) { - derrors.Wrap(&err, "DB.saveModule(ctx, Version(%q, %q))", m.ModulePath, m.Version) - return db.db.Transact(ctx, func(tx *database.DB) error { - // If the version exists, delete it to force an overwrite. This allows us - // to selectively repopulate data after a code change. - if err := db.DeleteModule(ctx, tx, m.ModulePath, m.Version); err != nil { - return fmt.Errorf("error deleting existing versions: %v", err) - } - moduleID, err := insertModule(ctx, tx, m) - if err != nil { - return err - } - if err := insertLicenses(ctx, tx, m, moduleID); err != nil { - return err - } - if err := insertPackages(ctx, tx, m); err != nil { +func saveModule(ctx context.Context, tx *database.DB, m *internal.Module) (err error) { + derrors.Wrap(&err, "saveModule(ctx, tx, Module(%q, %q))", m.ModulePath, m.Version) + // If the version exists, delete it to force an overwrite. This allows us + // to selectively repopulate data after a code change. + if err := DeleteModule(ctx, tx, m.ModulePath, m.Version); err != nil { + return fmt.Errorf("error deleting existing versions: %v", err) + } + moduleID, err := insertModule(ctx, tx, m) + if err != nil { + return err + } + if err := insertLicenses(ctx, tx, m, moduleID); err != nil { + return err + } + if err := insertPackages(ctx, tx, m); err != nil { + return err + } + if experiment.IsActive(ctx, internal.ExperimentInsertDirectories) { + if err := insertDirectories(ctx, tx, m, moduleID); err != nil { return err } - if experiment.IsActive(ctx, internal.ExperimentInsertDirectories) { - if err := insertDirectories(ctx, tx, m, moduleID); err != nil { - return err - } - } - return nil - }) + } + return nil } func insertModule(ctx context.Context, db *database.DB, m *internal.Module) (_ int, err error) { @@ -545,16 +545,12 @@ func removeNonDistributableData(m *internal.Module) { } // DeleteModule deletes a Version from the database. -func (db *DB) DeleteModule(ctx context.Context, ddb *database.DB, modulePath, version string) (err error) { - defer derrors.Wrap(&err, "DB.DeleteModule(ctx, ddb, %q, %q)", modulePath, version) - - if ddb == nil { - ddb = db.db - } +func DeleteModule(ctx context.Context, db *database.DB, modulePath, version string) (err error) { + defer derrors.Wrap(&err, "DeleteModule(ctx, db, %q, %q)", modulePath, version) // We only need to delete from the modules table. Thanks to ON DELETE // CASCADE constraints, that will trigger deletions from all other tables. const stmt = `DELETE FROM modules WHERE module_path=$1 AND version=$2` - _, err = ddb.Exec(ctx, stmt, modulePath, version) + _, err = db.Exec(ctx, stmt, modulePath, version) return err } diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go index 780099e7..4b81d14e 100644 --- a/internal/postgres/insert_module_test.go +++ b/internal/postgres/insert_module_test.go @@ -271,7 +271,7 @@ func TestPostgres_DeleteVersion(t *testing.T) { if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); err != nil { t.Fatal(err) } - if err := testDB.DeleteModule(ctx, nil, v.ModulePath, v.Version); err != nil { + if err := DeleteModule(ctx, testDB.db, v.ModulePath, v.Version); err != nil { t.Fatal(err) } if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); !errors.Is(err, derrors.NotFound) { diff --git a/internal/postgres/search.go b/internal/postgres/search.go index e41a69ee..2530ae01 100644 --- a/internal/postgres/search.go +++ b/internal/postgres/search.go @@ -544,14 +544,14 @@ var upsertSearchStatement = fmt.Sprintf(` ;`, hllRegisterCount) // UpsertSearchDocuments adds search information for mod ot the search_documents table. -func (db *DB) UpsertSearchDocuments(ctx context.Context, mod *internal.Module) (err error) { +func UpsertSearchDocuments(ctx context.Context, db *database.DB, mod *internal.Module) (err error) { defer derrors.Wrap(&err, "UpsertSearchDocuments(ctx, %q)", mod.ModulePath) for _, pkg := range mod.Packages { if isInternalPackage(pkg.Path) { continue } - err := db.UpsertSearchDocument(ctx, upsertSearchDocumentArgs{ + err := UpsertSearchDocument(ctx, db, upsertSearchDocumentArgs{ PackagePath: pkg.Path, ModulePath: mod.ModulePath, Synopsis: pkg.Synopsis, @@ -578,8 +578,8 @@ type upsertSearchDocumentArgs struct { // // The given module should have already been validated via a call to // validateModule. -func (db *DB) UpsertSearchDocument(ctx context.Context, args upsertSearchDocumentArgs) (err error) { - defer derrors.Wrap(&err, "UpsertSearchDocument(ctx, %q, %q)", args.PackagePath, args.ModulePath) +func UpsertSearchDocument(ctx context.Context, db *database.DB, args upsertSearchDocumentArgs) (err error) { + defer derrors.Wrap(&err, "UpsertSearchDocument(ctx, db, %q, %q)", args.PackagePath, args.ModulePath) // Only summarize the README if the package and module have the same path. if args.PackagePath != args.ModulePath { @@ -588,7 +588,7 @@ func (db *DB) UpsertSearchDocument(ctx context.Context, args upsertSearchDocumen } pathTokens := strings.Join(GeneratePathTokens(args.PackagePath), " ") sectionB, sectionC, sectionD := SearchDocumentSections(args.Synopsis, args.ReadmeFilePath, args.ReadmeContents) - _, err = db.db.Exec(ctx, upsertSearchStatement, args.PackagePath, pathTokens, sectionB, sectionC, sectionD) + _, err = db.Exec(ctx, upsertSearchStatement, args.PackagePath, pathTokens, sectionB, sectionC, sectionD) return err } |
