Skip to content
This repository has been archived by the owner on Jul 4, 2024. It is now read-only.

Improve API error responses #3882

Merged
merged 9 commits into from
May 28, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ func (r *repository) Delete(ctx context.Context, id, tenantID string) error {

if tenantID == "" {
conditions = append(conditions, repo.NewNullCondition(tenantIDColumn))
return r.deleterGlobal.DeleteOneGlobal(ctx, conditions)
if err := r.deleterGlobal.DeleteOneGlobal(ctx, conditions); apperrors.IsInternalServerError(err) {
return apperrors.NewTenantRequiredError()
}
return nil
}

return r.deleterWithEmbeddedTenant.DeleteOne(ctx, resource.FormationTemplate, tenantID, conditions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"testing"
"time"

"github.com/kyma-incubator/compass/components/director/pkg/persistence"
"github.com/stretchr/testify/require"

"github.com/kyma-incubator/compass/components/director/internal/model"
"github.com/kyma-incubator/compass/components/director/pkg/pagination"

Expand Down Expand Up @@ -172,24 +175,6 @@ func TestRepository_Exists(t *testing.T) {
}

func TestRepository_Delete(t *testing.T) {
suiteWithoutTenant := testdb.RepoDeleteTestSuite{
Name: "Delete Formation Template By ID when there is no tenant",
SQLQueryDetails: []testdb.SQLQueryDetails{
{
Query: regexp.QuoteMeta(`DELETE FROM public.formation_templates WHERE id = $1 AND tenant_id IS NULL`),
Args: []driver.Value{testFormationTemplateID},
ValidResult: sqlmock.NewResult(-1, 1),
InvalidResult: sqlmock.NewResult(-1, 2),
},
},
RepoConstructorFunc: formationtemplate.NewRepository,
ConverterMockProvider: func() testdb.Mock {
return &automock.EntityConverter{}
},
IsGlobal: true,
MethodArgs: []interface{}{testFormationTemplateID, ""},
}

suiteWithTenant := testdb.RepoDeleteTestSuite{
Name: "Delete Formation Template By ID when there is tenant",
SQLQueryDetails: []testdb.SQLQueryDetails{
Expand All @@ -208,7 +193,25 @@ func TestRepository_Delete(t *testing.T) {
}

suiteWithTenant.Run(t)
suiteWithoutTenant.Run(t)

t.Run("Return tenant is required error when deleting tenant scoped formation template with provided tenant header", func(t *testing.T) {
// GIVEN
db, mock := testdb.MockDatabase(t)
defer mock.AssertExpectations(t)
ctx := persistence.SaveToContext(ctx, db)

mock.ExpectExec(regexp.QuoteMeta(`DELETE FROM public.formation_templates WHERE id = $1 AND tenant_id IS NULL`)).
WithArgs(testFormationTemplateID).WillReturnResult(sqlmock.NewResult(-1, 0))

// WHEN
mockConverter := &automock.EntityConverter{}
r := formationtemplate.NewRepository(mockConverter)
err := r.Delete(ctx, testFormationTemplateID, "")

// THEN
require.Error(t, err)
require.Contains(t, err.Error(), "Tenant is required")
})
}

func TestRepository_List(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions components/director/pkg/apperrors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,11 @@ func IsNotFoundError(err error) bool {
return ErrorCode(err) == NotFound
}

// IsInternalServerError checks if the provided error is an internal server error
func IsInternalServerError(err error) bool {
return ErrorCode(err) == InternalError
}

// IsTenantRequired missing godoc
func IsTenantRequired(err error) bool {
return ErrorCode(err) == TenantRequired
Expand Down
2 changes: 1 addition & 1 deletion components/hydrator/internal/tenantmapping/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (h *Handler) getObjectContexts(ctx context.Context, reqData oathkeeper.ReqD

objectContext, err := provider.GetObjectContext(ctx, reqData, *details)
if err != nil {
return nil, errors.Wrap(err, "while getting objectContexts: ")
return nil, err
}

objectContexts = append(objectContexts, objectContext)
Expand Down