Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes 4925: fixup DBErrorToApi funcs #893

Merged
merged 2 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: e.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, &uuid)
}
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 @@ -775,10 +777,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 @@ -825,14 +824,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 @@ -945,7 +944,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 @@ -964,14 +963,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 @@ -1322,7 +1321,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 @@ -1354,7 +1353,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
59 changes: 30 additions & 29 deletions pkg/dao/snapshots.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,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 @@ -199,13 +193,7 @@ func (sDao *snapshotDaoImpl) fetch(ctx context.Context, uuid string) (models.Sna
Where("uuid = ?", UuidifyString(uuid)).
First(&snapshot)
if result.Error != nil {
if result.Error == gorm.ErrRecordNotFound {
return models.Snapshot{}, &ce.DaoError{
Message: "Could not find snapshot with UUID " + uuid,
NotFound: true,
}
}
return models.Snapshot{}, result.Error
return models.Snapshot{}, SnapshotsDBToApiError(result.Error, &uuid)
}
return snapshot, nil
}
Expand All @@ -219,17 +207,36 @@ func (sDao *snapshotDaoImpl) FetchModel(ctx context.Context, uuid string, includ
result = sDao.db.WithContext(ctx).Where("uuid = ?", uuid).First(&snap)
}
if result.Error != nil {
if errors.Is(result.Error, gorm.ErrRecordNotFound) {
return models.Snapshot{}, &ce.DaoError{
Message: "Could not find snapshot with UUID " + uuid,
NotFound: true,
}
}
return models.Snapshot{}, result.Error
return models.Snapshot{}, SnapshotsDBToApiError(result.Error, &uuid)
}
return snap, nil
}

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

daoError := ce.DaoError{}
if errors.Is(e, gorm.ErrRecordNotFound) {
msg := "Snapshot 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 Expand Up @@ -320,10 +327,7 @@ func (sDao *snapshotDaoImpl) SoftDelete(ctx context.Context, snapUUID string) er
var snap models.Snapshot
err := sDao.db.WithContext(ctx).Where("uuid = ?", snapUUID).First(&snap).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return &ce.DaoError{NotFound: true, Message: "Could not find snapshot with UUID " + snapUUID}
}
return err
return SnapshotsDBToApiError(err, &snapUUID)
}
err = sDao.db.WithContext(ctx).Delete(&snap).Error
if err != nil {
Expand Down Expand Up @@ -400,10 +404,7 @@ func (sDao *snapshotDaoImpl) ClearDeletedAt(ctx context.Context, snapUUID string
var snap models.Snapshot
err := sDao.db.WithContext(ctx).Unscoped().Where("uuid = ?", snapUUID).First(&snap).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return &ce.DaoError{NotFound: true, Message: "Could not find snapshot with UUID " + snapUUID}
}
return err
return SnapshotsDBToApiError(err, &snapUUID)
}
err = sDao.db.WithContext(ctx).Unscoped().Model(&snap).Update("deleted_at", nil).Error
if err != nil {
Expand Down
Loading
Loading