Skip to content

Commit

Permalink
fix race condition with repo add files
Browse files Browse the repository at this point in the history 
Do all relevant database reading/modifying inside `maybeRunTaskInBackground`
  • Loading branch information
@neolynx
neolynx committed Oct 21, 2024
1 parent a771707 commit a01e915
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 51 deletions.
54 changes: 24 additions & 30 deletions api/repos.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,14 @@ func apiReposPackagesAddDelete(c *gin.Context, taskNamePrefix string, cb func(li
return
}

err = collection.LoadComplete(repo)
if err != nil {
AbortWithJSONError(c, 500, err)
return
}

resources := []string{string(repo.Key())}

maybeRunTaskInBackground(c, taskNamePrefix+repo.Name, resources, func(out aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
err = collection.LoadComplete(repo)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}

out.Printf("Loading packages...\n")
list, err := deb.NewPackageListFromRefList(repo.RefList(), collectionFactory.PackageCollection(), nil)
if err != nil {
Expand Down Expand Up @@ -360,12 +360,6 @@ func apiReposPackageFromDir(c *gin.Context) {
return
}

err = collection.LoadComplete(repo)
if err != nil {
AbortWithJSONError(c, 500, err)
return
}

var taskName string
var sources []string
if fileParam == "" {
Expand All @@ -379,6 +373,11 @@ func apiReposPackageFromDir(c *gin.Context) {
resources := []string{string(repo.Key())}
resources = append(resources, sources...)
maybeRunTaskInBackground(c, taskName, resources, func(out aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
err = collection.LoadComplete(repo)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}

verifier := context.GetVerifier()

var (
Expand Down Expand Up @@ -480,17 +479,7 @@ func apiReposCopyPackage(c *gin.Context) {
return
}

err = collectionFactory.LocalRepoCollection().LoadComplete(dstRepo)
if err != nil {
AbortWithJSONError(c, http.StatusBadRequest, fmt.Errorf("dest repo error: %s", err))
return
}

var (
srcRefList *deb.PackageRefList
srcRepo *deb.LocalRepo
)

var srcRepo *deb.LocalRepo
srcRepo, err = collectionFactory.LocalRepoCollection().ByName(srcRepoName)
if err != nil {
AbortWithJSONError(c, http.StatusBadRequest, fmt.Errorf("src repo error: %s", err))
Expand All @@ -502,17 +491,22 @@ func apiReposCopyPackage(c *gin.Context) {
return
}

err = collectionFactory.LocalRepoCollection().LoadComplete(srcRepo)
if err != nil {
AbortWithJSONError(c, http.StatusBadRequest, fmt.Errorf("src repo error: %s", err))
return
}

srcRefList = srcRepo.RefList()
taskName := fmt.Sprintf("Copy packages from repo %s to repo %s", srcRepoName, dstRepoName)
resources := []string{string(dstRepo.Key()), string(srcRepo.Key())}

maybeRunTaskInBackground(c, taskName, resources, func(_ aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
err = collectionFactory.LocalRepoCollection().LoadComplete(dstRepo)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusBadRequest, Value: nil}, fmt.Errorf("dest repo error: %s", err)
}

err = collectionFactory.LocalRepoCollection().LoadComplete(srcRepo)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusBadRequest, Value: nil}, fmt.Errorf("src repo error: %s", err)
}

srcRefList := srcRepo.RefList()

reporter := &aptly.RecordingResultReporter{
Warnings: []string{},
AddedLines: []string{},
Expand Down
41 changes: 20 additions & 21 deletions api/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,17 @@ func apiSnapshotsCreate(c *gin.Context) {
return
}

err = snapshotCollection.LoadComplete(sources[i])
if err != nil {
AbortWithJSONError(c, 500, err)
return
}

resources = append(resources, string(sources[i].ResourceKey()))
}

maybeRunTaskInBackground(c, "Create snapshot "+b.Name, resources, func(_ aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
for i := range sources {

Check failure on line 142 in api/snapshot.go

View workflow job for this annotation

GitHub Actions / lint 

File is not `gofmt`-ed with `-s` (gofmt)
err = snapshotCollection.LoadComplete(sources[i])
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}
}

list := deb.NewPackageList()

// verify package refs and build package list
Expand Down Expand Up @@ -468,17 +469,16 @@ func apiSnapshotsMerge(c *gin.Context) {
return
}

err = snapshotCollection.LoadComplete(sources[i])
if err != nil {
AbortWithJSONError(c, http.StatusInternalServerError, err)
return
}
resources[i] = string(sources[i].ResourceKey())
}

maybeRunTaskInBackground(c, "Merge snapshot "+name, resources, func(_ aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
result := sources[0].RefList()
for i := 1; i < len(sources); i++ {
err = snapshotCollection.LoadComplete(sources[i])
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}
result = result.Merge(sources[i].RefList(), overrideMatching, false)
}

Expand Down Expand Up @@ -566,27 +566,26 @@ func apiSnapshotsPull(c *gin.Context) {
AbortWithJSONError(c, http.StatusNotFound, err)
return
}
err = collectionFactory.SnapshotCollection().LoadComplete(toSnapshot)
if err != nil {
AbortWithJSONError(c, http.StatusInternalServerError, err)
return
}

// Load <Source> snapshot
sourceSnapshot, err := collectionFactory.SnapshotCollection().ByName(body.Source)
if err != nil {
AbortWithJSONError(c, http.StatusNotFound, err)
return
}
err = collectionFactory.SnapshotCollection().LoadComplete(sourceSnapshot)
if err != nil {
AbortWithJSONError(c, http.StatusInternalServerError, err)
return
}

resources := []string{string(sourceSnapshot.ResourceKey()), string(toSnapshot.ResourceKey())}
taskName := fmt.Sprintf("Pull snapshot %s into %s and save as %s", body.Source, name, body.Destination)
maybeRunTaskInBackground(c, taskName, resources, func(_ aptly.Progress, _ *task.Detail) (*task.ProcessReturnValue, error) {
err = collectionFactory.SnapshotCollection().LoadComplete(toSnapshot)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}
err = collectionFactory.SnapshotCollection().LoadComplete(sourceSnapshot)
if err != nil {
return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
}

// convert snapshots to package list
toPackageList, err := deb.NewPackageListFromRefList(toSnapshot.RefList(), collectionFactory.PackageCollection(), context.Progress())
if err != nil {
Expand Down

0 comments on commit a01e915

Please sign in to comment.