artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 17:51:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

codeintel: Fix missing transaction in UpdateUploadsReferenceCounts function (#40378)

Browse Source
This commit is contained in:
Cesar Jimenez 2022-08-15 17:05:09 -04:00 committed by GitHub
parent ceedc2d688
commit a67dba61a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 5 additions and 178 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
internal/codeintel/uploads/internal/store/store.go
View File

@ -4,8 +4,6 @@ import (
"context"
"time"
"github.com/keegancsmith/sqlf"
"github.com/opentracing/opentracing-go/log"
logger "github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/internal/codeintel/uploads/shared"
@ -14,7 +12,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
"github.com/sourcegraph/sourcegraph/internal/observation"
"github.com/sourcegraph/sourcegraph/lib/codeintel/precise"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
// Store provides the interface for uploads storage.
@ -23,9 +20,6 @@ type Store interface {
Transact(ctx context.Context) (Store, error)
Done(err error) error
// Not in use yet.
List(ctx context.Context, opts ListOpts) (uploads []shared.Upload, err error)
// Commits
GetCommitsVisibleToUpload(ctx context.Context, uploadID, limit int, token *string) (_ []string, nextToken *string, err error)
GetOldestCommitDate(ctx context.Context, repositoryID int) (time.Time, bool, error)
@ -110,29 +104,3 @@ func (s *store) transact(ctx context.Context) (*store, error) {
func (s *store) Done(err error) error {
return s.db.Done(err)
}
// ListOpts specifies options for listing uploads.
type ListOpts struct {
Limit int
}
// List returns a list of uploads.
func (s *store) List(ctx context.Context, opts ListOpts) (uploads []shared.Upload, err error) {
ctx, _, endObservation := s.operations.list.With(ctx, &err, observation.Args{})
defer func() {
endObservation(1, observation.Args{LogFields: []log.Field{
log.Int("numUploads", len(uploads)),
}})
}()
// This is only a stub and will be replaced or significantly modified
// in https://github.com/sourcegraph/sourcegraph/issues/33375
_, _ = scanUploads(s.db.Query(ctx, sqlf.Sprintf(listQuery, opts.Limit)))
return nil, errors.Newf("unimplemented: uploads.store.List")
}
const listQuery = `
-- source: internal/codeintel/uploads/internal/store/store.go:List
SELECT id FROM TODO
LIMIT %s
`

16
internal/codeintel/uploads/internal/store/store_uploads.go
View File

@ -377,20 +377,20 @@ func (s *store) HardDeleteUploadsByIDs(ctx context.Context, ids ...int) (err err
idQueries = append(idQueries, sqlf.Sprintf("%s", id))
}
tx, err := s.db.Transact(ctx)
tx, err := s.transact(ctx)
if err != nil {
return err
}
defer func() { err = tx.Done(err) }()
defer func() { err = tx.db.Done(err) }()
// Before deleting the record, ensure that we decrease the number of existant references
// to all of this upload's dependencies. This also selects a new upload to canonically provide
// the same package as the deleted upload, if such an upload exists.
if _, err := s.UpdateUploadsReferenceCounts(ctx, ids, shared.DependencyReferenceCountUpdateTypeRemove); err != nil {
if _, err := tx.UpdateUploadsReferenceCounts(ctx, ids, shared.DependencyReferenceCountUpdateTypeRemove); err != nil {
return err
}
if err := tx.Exec(ctx, sqlf.Sprintf(hardDeleteUploadsByIDsQuery, sqlf.Join(idQueries, ", "))); err != nil {
if err := tx.db.Exec(ctx, sqlf.Sprintf(hardDeleteUploadsByIDsQuery, sqlf.Join(idQueries, ", "))); err != nil {
return err
}
@ -577,12 +577,6 @@ func (s *store) UpdateUploadsReferenceCounts(ctx context.Context, ids []int, dep
return 0, nil
}
tx, err := s.db.Transact(ctx)
if err != nil {
return 0, err
}
defer func() { err = tx.Done(err) }()
idArray := pq.Array(ids)
excludeCondition := sqlf.Sprintf("TRUE")
@ -590,7 +584,7 @@ func (s *store) UpdateUploadsReferenceCounts(ctx context.Context, ids []int, dep
excludeCondition = sqlf.Sprintf("NOT (u.id = ANY (%s))", idArray)
}
result, err := tx.ExecResult(ctx, sqlf.Sprintf(
result, err := s.db.ExecResult(ctx, sqlf.Sprintf(
updateUploadsReferenceCountsQuery,
idArray,
idArray,

123
internal/codeintel/uploads/mocks_test.go
View File

@ -96,9 +96,6 @@ type MockStore struct {
// HasRepositoryFunc is an instance of a mock function object
// controlling the behavior of the method HasRepository.
HasRepositoryFunc *StoreHasRepositoryFunc
// ListFunc is an instance of a mock function object controlling the
// behavior of the method List.
ListFunc *StoreListFunc
// RepoNameFunc is an instance of a mock function object controlling the
// behavior of the method RepoName.
RepoNameFunc *StoreRepoNameFunc
@ -256,11 +253,6 @@ func NewMockStore() *MockStore {
return
},
},
ListFunc: &StoreListFunc{
defaultHook: func(context.Context, store.ListOpts) (r0 []shared.Upload, r1 error) {
return
},
},
RepoNameFunc: &StoreRepoNameFunc{
defaultHook: func(context.Context, int) (r0 string, r1 error) {
return
@ -443,11 +435,6 @@ func NewStrictMockStore() *MockStore {
panic("unexpected invocation of MockStore.HasRepository")
},
},
ListFunc: &StoreListFunc{
defaultHook: func(context.Context, store.ListOpts) ([]shared.Upload, error) {
panic("unexpected invocation of MockStore.List")
},
},
RepoNameFunc: &StoreRepoNameFunc{
defaultHook: func(context.Context, int) (string, error) {
panic("unexpected invocation of MockStore.RepoName")
@ -588,9 +575,6 @@ func NewMockStoreFrom(i store.Store) *MockStore {
HasRepositoryFunc: &StoreHasRepositoryFunc{
defaultHook: i.HasRepository,
},
ListFunc: &StoreListFunc{
defaultHook: i.List,
},
RepoNameFunc: &StoreRepoNameFunc{
defaultHook: i.RepoName,
},
@ -3024,113 +3008,6 @@ func (c StoreHasRepositoryFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// StoreListFunc describes the behavior when the List method of the parent
// MockStore instance is invoked.
type StoreListFunc struct {
defaultHook func(context.Context, store.ListOpts) ([]shared.Upload, error)
hooks []func(context.Context, store.ListOpts) ([]shared.Upload, error)
history []StoreListFuncCall
mutex sync.Mutex
}
// List delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockStore) List(v0 context.Context, v1 store.ListOpts) ([]shared.Upload, error) {
r0, r1 := m.ListFunc.nextHook()(v0, v1)
m.ListFunc.appendCall(StoreListFuncCall{v0, v1, r0, r1})
return r0, r1
}
// SetDefaultHook sets function that is called when the List method of the
// parent MockStore instance is invoked and the hook queue is empty.
func (f *StoreListFunc) SetDefaultHook(hook func(context.Context, store.ListOpts) ([]shared.Upload, error)) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// List method of the parent MockStore instance invokes the hook at the
// front of the queue and discards it. After the queue is empty, the default
// hook function is invoked for any future action.
func (f *StoreListFunc) PushHook(hook func(context.Context, store.ListOpts) ([]shared.Upload, error)) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
}
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *StoreListFunc) SetDefaultReturn(r0 []shared.Upload, r1 error) {
f.SetDefaultHook(func(context.Context, store.ListOpts) ([]shared.Upload, error) {
return r0, r1
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *StoreListFunc) PushReturn(r0 []shared.Upload, r1 error) {
f.PushHook(func(context.Context, store.ListOpts) ([]shared.Upload, error) {
return r0, r1
})
}
func (f *StoreListFunc) nextHook() func(context.Context, store.ListOpts) ([]shared.Upload, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
if len(f.hooks) == 0 {
return f.defaultHook
}
hook := f.hooks[0]
f.hooks = f.hooks[1:]
return hook
}
func (f *StoreListFunc) appendCall(r0 StoreListFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of StoreListFuncCall objects describing the
// invocations of this function.
func (f *StoreListFunc) History() []StoreListFuncCall {
f.mutex.Lock()
history := make([]StoreListFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// StoreListFuncCall is an object that describes an invocation of method
// List on an instance of MockStore.
type StoreListFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
// Arg1 is the value of the 2nd argument passed to this method
// invocation.
Arg1 store.ListOpts
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 []shared.Upload
// Result1 is the value of the 2nd result returned from this method
// invocation.
Result1 error
}
// Args returns an interface slice containing the arguments of this
// invocation.
func (c StoreListFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c StoreListFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// StoreRepoNameFunc describes the behavior when the RepoName method of the
// parent MockStore instance is invoked.
type StoreRepoNameFunc struct {

12
internal/codeintel/uploads/service.go
View File

@ -28,7 +28,6 @@ var _ service = (*Service)(nil)
type service interface {
// Not in use yet.
List(ctx context.Context, opts ListOpts) (uploads []Upload, err error)
Get(ctx context.Context, id int) (upload Upload, ok bool, err error)
GetBatch(ctx context.Context, ids ...int) (uploads []Upload, err error)
Enqueue(ctx context.Context, state UploadState, reader io.Reader) (err error)
@ -99,17 +98,6 @@ func newService(store store.Store, lsifstore lsifstore.LsifStore, gsc shared.Git
type Upload = shared.Upload
type ListOpts struct {
Limit int
}
func (s *Service) List(ctx context.Context, opts ListOpts) (uploads []Upload, err error) {
ctx, _, endObservation := s.operations.list.With(ctx, &err, observation.Args{})
defer endObservation(1, observation.Args{})
return s.store.List(ctx, store.ListOpts(opts))
}
func (s *Service) Get(ctx context.Context, id int) (upload Upload, ok bool, err error) {
ctx, _, endObservation := s.operations.get.With(ctx, &err, observation.Args{})
defer endObservation(1, observation.Args{})