Skip to content

Commit

Permalink
Fixes 4925: fixup DBErrorToApi funcs
Browse files Browse the repository at this point in the history
  • Loading branch information
jlsherrill committed Nov 18, 2024
1 parent 398a3d2 commit 6741fd7
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 97 deletions.
10 changes: 3 additions & 7 deletions pkg/dao/admin_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ func (a adminTaskInfoDaoImpl) Fetch(ctx context.Context, id string) (api.AdminTa

taskInfoResponse := api.AdminTaskInfoResponse{}
if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return taskInfoResponse, &ce.DaoError{NotFound: true, Message: "Could not find task with UUID " + id}
} else {
return taskInfoResponse, DBErrorToApi(result.Error)
}
return taskInfoResponse, TasksDBToApiError(result.Error, &id)
}

if taskInfo.Typename == payloads.Snapshot {
Expand Down Expand Up @@ -95,13 +91,13 @@ func (a adminTaskInfoDaoImpl) List(

result := filteredDB.Model(&tasks).Count(&totalTasks)
if result.Error != nil {
return api.AdminTaskInfoCollectionResponse{}, totalTasks, DBErrorToApi(filteredDB.Error)
return api.AdminTaskInfoCollectionResponse{}, totalTasks, TasksDBToApiError(filteredDB.Error, nil)
}

result = filteredDB.Offset(pageData.Offset).Limit(pageData.Limit).Order(order).Find(&tasks)

if result.Error != nil {
return api.AdminTaskInfoCollectionResponse{}, totalTasks, DBErrorToApi(filteredDB.Error)
return api.AdminTaskInfoCollectionResponse{}, totalTasks, TasksDBToApiError(filteredDB.Error, nil)
}

taskResponses := convertAdminTaskInfoToResponses(tasks)
Expand Down
2 changes: 1 addition & 1 deletion pkg/dao/admin_tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (suite *AdminTaskSuite) TestFetchInvalidUUID() {
daoError, ok := err.(*ce.DaoError)
assert.True(t, ok)
assert.True(t, daoError.NotFound)
assert.Equal(t, "Could not find task with UUID "+invalidUUID, daoError.Message)
assert.Equal(t, "Task with UUID "+invalidUUID+" not found", daoError.Message)
}

// Occurs if repository/repository configuration associated with task is deleted
Expand Down
2 changes: 1 addition & 1 deletion pkg/dao/environments.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (r environmentDaoImpl) List(ctx context.Context, orgID string, repositoryCo
if err != nil {
return api.RepositoryEnvironmentCollectionResponse{},
totalEnvironments,
DBErrorToApi(err)
RepositoryDBErrorToApi(err, &repositoryConfigUUID)
}
return api.RepositoryEnvironmentCollectionResponse{},
totalEnvironments,
Expand Down
2 changes: 1 addition & 1 deletion pkg/dao/package_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (r packageGroupDaoImpl) List(ctx context.Context, orgID string, repositoryC
if err != nil {
return api.RepositoryPackageGroupCollectionResponse{},
totalPackageGroups,
DBErrorToApi(err)
RepositoryDBErrorToApi(err, &repositoryConfigUUID)
}
return api.RepositoryPackageGroupCollectionResponse{},
totalPackageGroups,
Expand Down
73 changes: 36 additions & 37 deletions pkg/dao/repository_configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func GetRepositoryConfigDao(db *gorm.DB, pulpClient pulp_client.PulpClient) Repo
}
}

func DBErrorToApi(e error) *ce.DaoError {
func RepositoryDBErrorToApi(e error, uuid *string) *ce.DaoError {
var dupKeyName string
if e == nil {
return nil
Expand Down Expand Up @@ -69,9 +69,21 @@ func DBErrorToApi(e error) *ce.DaoError {
return &ce.DaoError{BadValidation: dbError.Validation, Message: dbError.Message}
}

daoErr := ce.DaoError{
Message: "Database Error",
NotFound: ce.HttpCodeForDaoError(e) == 404, // Check if isNotFoundError
daoErr := ce.DaoError{}
if errors.Is(e, gorm.ErrRecordNotFound) {
msg := "Repository not found"
if uuid != nil {
msg = fmt.Sprintf("Repository with UUID %s not found", *uuid)
}
daoErr = ce.DaoError{
Message: msg,
NotFound: true,
}
} else {
daoErr = ce.DaoError{
Message: "Database Error",
NotFound: ce.HttpCodeForDaoError(e) == 404, // Check if isNotFoundError
}
}

daoErr.Wrap(e)
Expand All @@ -98,13 +110,13 @@ func (r repositoryConfigDaoImpl) Create(ctx context.Context, newRepoReq api.Repo

if newRepo.URL == "" || newRepo.Origin == config.OriginUpload {
if err := r.db.WithContext(ctx).Create(&newRepo).Error; err != nil {
return api.RepositoryResponse{}, DBErrorToApi(err)
return api.RepositoryResponse{}, RepositoryDBErrorToApi(err, nil)
}
} else if newRepo.URL != "" {
cleanedUrl := models.CleanupURL(newRepo.URL)
// Repo configs with the same URL share a repository object
if err := r.db.WithContext(ctx).Where("url = ?", cleanedUrl).FirstOrCreate(&newRepo).Error; err != nil {
return api.RepositoryResponse{}, DBErrorToApi(err)
return api.RepositoryResponse{}, RepositoryDBErrorToApi(err, nil)
}
}
if newRepoReq.OrgID != nil {
Expand All @@ -116,13 +128,13 @@ func (r repositoryConfigDaoImpl) Create(ctx context.Context, newRepoReq api.Repo
newRepoConfig.RepositoryUUID = newRepo.Base.UUID

if err := r.db.WithContext(ctx).Create(&newRepoConfig).Error; err != nil {
return api.RepositoryResponse{}, DBErrorToApi(err)
return api.RepositoryResponse{}, RepositoryDBErrorToApi(err, nil)
}

// reload the repoConfig to fetch repository info too
newRepoConfig, err := r.fetchRepoConfig(ctx, newRepoConfig.OrgID, newRepoConfig.UUID, false)
if err != nil {
return api.RepositoryResponse{}, DBErrorToApi(err)
return api.RepositoryResponse{}, RepositoryDBErrorToApi(err, nil)
}

var created api.RepositoryResponse
Expand Down Expand Up @@ -208,14 +220,14 @@ func (r repositoryConfigDaoImpl) bulkCreate(tx *gorm.DB, newRepositories []api.R
}

if err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errorList[i] = dbErr
tx.RollbackTo("beforecreate")
continue
}
newRepoConfigs[i].RepositoryUUID = newRepos[i].UUID
if err := tx.Create(&newRepoConfigs[i]).Error; err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errorList[i] = dbErr
tx.RollbackTo("beforecreate")
continue
Expand Down Expand Up @@ -556,10 +568,7 @@ func (r repositoryConfigDaoImpl) fetchRepoConfig(ctx context.Context, orgID stri
First(&found)

if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return found, &ce.DaoError{NotFound: true, Message: "Could not find repository with UUID " + uuid}
}
return found, DBErrorToApi(result.Error)
return found, RepositoryDBErrorToApi(result.Error, nil)
}
return found, nil
}
Expand All @@ -575,10 +584,7 @@ func (r repositoryConfigDaoImpl) FetchByRepoUuid(ctx context.Context, orgID stri
First(&repoConfig)

if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return repo, &ce.DaoError{NotFound: true, Message: "Could not find repository with UUID " + repoUuid}
}
return repo, DBErrorToApi(result.Error)
return repo, RepositoryDBErrorToApi(result.Error, &repoUuid)
}

ModelToApiFields(repoConfig, &repo)
Expand All @@ -594,11 +600,7 @@ func (r repositoryConfigDaoImpl) FetchWithoutOrgID(ctx context.Context, uuid str
First(&found)

if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return repo, &ce.DaoError{NotFound: true, Message: "Could not find repository with UUID " + uuid}
} else {
return repo, DBErrorToApi(result.Error)
}
return repo, RepositoryDBErrorToApi(result.Error, &uuid)
}
ModelToApiFields(found, &repo)
return repo, nil
Expand Down Expand Up @@ -633,15 +635,15 @@ func (r repositoryConfigDaoImpl) Update(ctx context.Context, orgID, uuid string,
cleanedUrl := models.CleanupURL(*repoParams.URL)
err = tx.FirstOrCreate(&repo, "url = ?", cleanedUrl).Error
if err != nil {
return DBErrorToApi(err)
return RepositoryDBErrorToApi(err, nil)
}
repoConfig.RepositoryUUID = repo.UUID
updatedUrl = repoConfig.Repository.URL != cleanedUrl
}

repoConfig.Repository = models.Repository{}
if err := tx.Model(&repoConfig).Omit("LastSnapshot").Updates(repoConfig.MapForUpdate()).Error; err != nil {
return DBErrorToApi(err)
return RepositoryDBErrorToApi(err, nil)
}

repositoryResponse := api.RepositoryResponse{}
Expand Down Expand Up @@ -669,7 +671,7 @@ func (r repositoryConfigDaoImpl) Update(ctx context.Context, orgID, uuid string,

repoConfig.Repository = models.Repository{}
if err := r.db.WithContext(ctx).Model(&repoConfig).Omit("LastSnapshot").Updates(repoConfig.MapForUpdate()).Error; err != nil {
return updatedUrl, DBErrorToApi(err)
return updatedUrl, RepositoryDBErrorToApi(err, nil)
}

return updatedUrl, nil
Expand Down Expand Up @@ -746,10 +748,7 @@ func (r repositoryConfigDaoImpl) Delete(ctx context.Context, orgID string, uuid

err := r.db.WithContext(ctx).Unscoped().Where("uuid = ? AND org_id = ?", UuidifyString(uuid), orgID).First(&repoConfig).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return &ce.DaoError{NotFound: true, Message: "Could not find repository with UUID " + uuid}
}
return DBErrorToApi(err)
return RepositoryDBErrorToApi(err, &uuid)
}

if err = r.db.WithContext(ctx).Unscoped().Delete(&repoConfig).Error; err != nil {
Expand Down Expand Up @@ -796,14 +795,14 @@ func (r repositoryConfigDaoImpl) bulkDelete(ctx context.Context, tx *gorm.DB, or
var repoConfig models.RepositoryConfiguration

if repoConfig, err = r.fetchRepoConfig(ctx, orgID, uuids[i], false); err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errors[i] = dbErr
tx.RollbackTo(save)
continue
}

if err = tx.Delete(&repoConfig).Error; err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errors[i] = dbErr
tx.RollbackTo(save)
continue
Expand Down Expand Up @@ -916,7 +915,7 @@ func (r repositoryConfigDaoImpl) bulkImport(tx *gorm.DB, reposToImport []api.Rep
Where("repositories.url = ? and repository_configurations.org_id = ?", cleanedUrl, newRepoConfigs[i].OrgID).
First(&existingRepo).Error
if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errorList[i] = dbErr
tx.RollbackTo("beforeimport")
continue
Expand All @@ -935,14 +934,14 @@ func (r repositoryConfigDaoImpl) bulkImport(tx *gorm.DB, reposToImport []api.Rep
} else {
// no existing repo, create (or find) repo and create repo config
if err = tx.Where("url = ?", cleanedUrl).FirstOrCreate(&newRepos[i]).Error; err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errorList[i] = dbErr
tx.RollbackTo("beforeimport")
continue
}
newRepoConfigs[i].RepositoryUUID = newRepos[i].UUID
if err = tx.Create(&newRepoConfigs[i]).Error; err != nil {
dbErr = DBErrorToApi(err)
dbErr = RepositoryDBErrorToApi(err, nil)
errorList[i] = dbErr
tx.RollbackTo("beforeimport")
continue
Expand Down Expand Up @@ -1293,7 +1292,7 @@ func (r repositoryConfigDaoImpl) validateName(ctx context.Context, orgId string,
}
if err := query.Find(&found).Error; err != nil {
response.Valid = false
return DBErrorToApi(err)
return RepositoryDBErrorToApi(err, nil)
}

if found.UUID != "" {
Expand Down Expand Up @@ -1325,7 +1324,7 @@ func (r repositoryConfigDaoImpl) validateUrl(ctx context.Context, orgId string,

if err := query.Find(&found).Error; err != nil {
response.URL.Valid = false
return DBErrorToApi(err)
return RepositoryDBErrorToApi(err, nil)
}

if found.UUID != "" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/dao/rpms.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (r *rpmDaoImpl) List(
if err != nil {
return api.RepositoryRpmCollectionResponse{},
totalRpms,
DBErrorToApi(err)
RepositoryDBErrorToApi(err, &repositoryConfigUUID)
}
return api.RepositoryRpmCollectionResponse{},
totalRpms,
Expand Down
34 changes: 27 additions & 7 deletions pkg/dao/snapshots.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dao

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
Expand Down Expand Up @@ -73,13 +74,7 @@ func (sDao *snapshotDaoImpl) List(
First(&repoConfig)

if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return api.SnapshotCollectionResponse{}, totalSnaps, &ce.DaoError{
Message: "Could not find repository with UUID " + repoConfigUUID,
NotFound: true,
}
}
return api.SnapshotCollectionResponse{}, totalSnaps, DBErrorToApi(result.Error)
return api.SnapshotCollectionResponse{}, totalSnaps, RepositoryDBErrorToApi(result.Error, &repoConfigUUID)
}
sortMap := map[string]string{
"created_at": "created_at",
Expand Down Expand Up @@ -208,6 +203,31 @@ func (sDao *snapshotDaoImpl) fetch(ctx context.Context, uuid string) (models.Sna
return snapshot, nil
}

func SnapshotsDBToApiError(e error, uuid *string) *ce.DaoError {
if e == nil {
return nil
}

daoError := ce.DaoError{}
if errors.Is(e, gorm.ErrRecordNotFound) {
msg := "Task not found"
if uuid != nil {
msg = fmt.Sprintf("Snapshot with UUID %s not found", *uuid)
}
daoError = ce.DaoError{
Message: msg,
NotFound: true,
}
} else {
daoError = ce.DaoError{
Message: e.Error(),
NotFound: ce.HttpCodeForDaoError(e) == 404, // Check if isNotFoundError
}
}
daoError.Wrap(e)
return &daoError
}

func (sDao *snapshotDaoImpl) GetRepositoryConfigurationFile(ctx context.Context, orgID, snapshotUUID string, isLatest bool) (string, error) {
var repoID string
snapshot, err := sDao.fetch(ctx, snapshotUUID)
Expand Down
2 changes: 1 addition & 1 deletion pkg/dao/snapshots_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ func (s *SnapshotsSuite) TestListNotFoundBadOrgId() {
assert.True(t, daoError.NotFound)
assert.Equal(t, int64(0), total)
assert.Equal(t, 0, len(collection.Data))
assert.ErrorContains(t, err, "Could not find repository with UUID "+rConfig.UUID)
assert.ErrorContains(t, err, "Repository with UUID "+rConfig.UUID+" not found")
}

func (s *SnapshotsSuite) TestFetchForRepoUUID() {
Expand Down
Loading

0 comments on commit 6741fd7

Please sign in to comment.