3
0

Fix create and update with unique constraints

When creating or updating resource that did not match unique constraint
filters, check wrongly reported not-unique error when matching (and
valid) resource was found in the store.

This fix adds explicit check if resource to be stored does not match
constraint filters and skips the rest of the constraint checking
procedure.
This commit is contained in:
Denis Arh
2021-10-12 19:49:45 +02:00
parent 1fa84826c3
commit 59ffe768a8
42 changed files with 506 additions and 37 deletions
+19 -3
View File
@@ -729,20 +729,36 @@ func (s *Store) check{{ export $.Types.Singular }}Constraints(ctx context.Contex
return nil
}
var checks = make([]func () error, 0)
{{- range $.Lookups }}
{{ if .UniqueConstraintCheck }}
{
checks = append(checks, func () error {
// Skip lookup by {{ .Suffix }} if {{ export $.Types.Singular }} does not match filters
{{- range $field, $value := .Filter }}
if res.{{ $field }} != {{ $value }} {
return nil
}
{{ end }}
ex, err := s.{{ toggleExport .Export "Lookup" $.Types.Singular "By" .Suffix }}(ctx{{ template "extraArgsCall" $ }}{{- range .RDBMSColumns }}, res.{{ .Field }} {{- end }})
if err == nil && ex != nil {{- range $.RDBMS.Columns.PrimaryKeyFields }} && ex.{{ .Field }} != res.{{ .Field }} {{ end }} {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
return nil
})
{{ end }}
{{ end }}
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -220,5 +220,13 @@ func (s *Store) checkActionlogConstraints(ctx context.Context, res *actionlog.Ac
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -567,5 +567,13 @@ func (s *Store) checkApigwFilterConstraints(ctx context.Context, res *types.Apig
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -564,5 +564,13 @@ func (s *Store) checkApigwRouteConstraints(ctx context.Context, res *types.Apigw
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -602,5 +602,13 @@ func (s *Store) checkApplicationConstraints(ctx context.Context, res *types.Appl
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -351,5 +351,13 @@ func (s *Store) checkAttachmentConstraints(ctx context.Context, res *types.Attac
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+16 -1
View File
@@ -642,13 +642,28 @@ func (s *Store) checkAuthClientConstraints(ctx context.Context, res *types.AuthC
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Handle if AuthClient does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupAuthClientByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+13 -1
View File
@@ -325,13 +325,25 @@ func (s *Store) checkAuthConfirmedClientConstraints(ctx context.Context, res *ty
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by UserIDClientID if AuthConfirmedClient does not match filters
ex, err := s.LookupAuthConfirmedClientByUserIDClientID(ctx, res.UserID, res.ClientID)
if err == nil && ex != nil && ex.UserID != res.UserID && ex.ClientID != res.ClientID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+25 -5
View File
@@ -297,31 +297,51 @@ func (s *Store) checkAuthOa2tokenConstraints(ctx context.Context, res *types.Aut
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Code if AuthOa2token does not match filters
ex, err := s.LookupAuthOa2tokenByCode(ctx, res.Code)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by Access if AuthOa2token does not match filters
ex, err := s.LookupAuthOa2tokenByAccess(ctx, res.Access)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by Refresh if AuthOa2token does not match filters
ex, err := s.LookupAuthOa2tokenByRefresh(ctx, res.Refresh)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+13 -1
View File
@@ -332,13 +332,25 @@ func (s *Store) checkAuthSessionConstraints(ctx context.Context, res *types.Auth
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by ID if AuthSession does not match filters
ex, err := s.LookupAuthSessionByID(ctx, res.ID)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -624,5 +624,13 @@ func (s *Store) checkAutomationSessionConstraints(ctx context.Context, res *type
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -604,5 +604,13 @@ func (s *Store) checkAutomationTriggerConstraints(ctx context.Context, res *type
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+16 -1
View File
@@ -643,13 +643,28 @@ func (s *Store) checkAutomationWorkflowConstraints(ctx context.Context, res *typ
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Handle if AutomationWorkflow does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupAutomationWorkflowByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -354,5 +354,13 @@ func (s *Store) checkComposeAttachmentConstraints(ctx context.Context, res *type
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -610,5 +610,13 @@ func (s *Store) checkComposeChartConstraints(ctx context.Context, res *types.Cha
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+16 -1
View File
@@ -364,13 +364,28 @@ func (s *Store) checkComposeModuleFieldConstraints(ctx context.Context, res *typ
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by ModuleIDName if ComposeModuleField does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupComposeModuleFieldByModuleIDName(ctx, res.ModuleID, res.Name)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+16 -1
View File
@@ -624,13 +624,28 @@ func (s *Store) checkComposeModuleConstraints(ctx context.Context, res *types.Mo
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by NamespaceIDHandle if ComposeModule does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupComposeModuleByNamespaceIDHandle(ctx, res.NamespaceID, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+16 -1
View File
@@ -611,13 +611,28 @@ func (s *Store) checkComposeNamespaceConstraints(ctx context.Context, res *types
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Slug if ComposeNamespace does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupComposeNamespaceBySlug(ctx, res.Slug)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -631,5 +631,13 @@ func (s *Store) checkComposePageConstraints(ctx context.Context, res *types.Page
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -324,5 +324,13 @@ func (s *Store) checkComposeRecordValueConstraints(ctx context.Context, _mod *ty
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -489,5 +489,13 @@ func (s *Store) checkComposeRecordConstraints(ctx context.Context, _mod *types.M
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -344,5 +344,13 @@ func (s *Store) checkCredentialsConstraints(ctx context.Context, res *types.Cred
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -595,5 +595,13 @@ func (s *Store) checkFederationExposedModuleConstraints(ctx context.Context, res
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -612,5 +612,13 @@ func (s *Store) checkFederationModuleMappingConstraints(ctx context.Context, res
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -378,5 +378,13 @@ func (s *Store) checkFederationNodeConstraints(ctx context.Context, res *types.N
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -588,5 +588,13 @@ func (s *Store) checkFederationNodesSyncConstraints(ctx context.Context, res *ty
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -592,5 +592,13 @@ func (s *Store) checkFederationSharedModuleConstraints(ctx context.Context, res
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+31 -7
View File
@@ -383,40 +383,64 @@ func (s *Store) checkFlagConstraints(ctx context.Context, res *types.Flag) error
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by KindResourceIDName if Flag does not match filters
ex, err := s.LookupFlagByKindResourceIDName(ctx, res.Kind, res.ResourceID, res.Name)
if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.OwnedBy != res.OwnedBy && ex.Name != res.Name {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by KindResourceID if Flag does not match filters
ex, err := s.LookupFlagByKindResourceID(ctx, res.Kind, res.ResourceID)
if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.OwnedBy != res.OwnedBy && ex.Name != res.Name {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by KindResourceIDOwnedBy if Flag does not match filters
ex, err := s.LookupFlagByKindResourceIDOwnedBy(ctx, res.Kind, res.ResourceID, res.OwnedBy)
if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.OwnedBy != res.OwnedBy && ex.Name != res.Name {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by KindResourceIDOwnedByName if Flag does not match filters
ex, err := s.LookupFlagByKindResourceIDOwnedByName(ctx, res.Kind, res.ResourceID, res.OwnedBy, res.Name)
if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.OwnedBy != res.OwnedBy && ex.Name != res.Name {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+13 -1
View File
@@ -333,13 +333,25 @@ func (s *Store) checkLabelConstraints(ctx context.Context, res *types.Label) err
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by KindResourceIDName if Label does not match filters
ex, err := s.LookupLabelByKindResourceIDName(ctx, res.Kind, res.ResourceID, res.Name)
if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.Name != res.Name {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -519,5 +519,13 @@ func (s *Store) checkMessagebusQueueMessageConstraints(ctx context.Context, res
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -578,5 +578,13 @@ func (s *Store) checkMessagebusQueueSettingConstraints(ctx context.Context, res
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -315,5 +315,13 @@ func (s *Store) checkRbacRuleConstraints(ctx context.Context, res *rbac.Rule) er
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -613,5 +613,13 @@ func (s *Store) checkReminderConstraints(ctx context.Context, res *types.Reminde
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+16 -1
View File
@@ -621,13 +621,28 @@ func (s *Store) checkReportConstraints(ctx context.Context, res *types.Report) e
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Handle if Report does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupReportByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -614,5 +614,13 @@ func (s *Store) checkResourceTranslationConstraints(ctx context.Context, res *ty
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+8
View File
@@ -310,5 +310,13 @@ func (s *Store) checkRoleMemberConstraints(ctx context.Context, res *types.RoleM
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+20 -1
View File
@@ -631,13 +631,32 @@ func (s *Store) checkRoleConstraints(ctx context.Context, res *types.Role) error
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Handle if Role does not match filters
if res.ArchivedAt != nil {
return nil
}
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupRoleByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+8
View File
@@ -337,5 +337,13 @@ func (s *Store) checkSettingConstraints(ctx context.Context, res *types.SettingV
return nil
}
var checks = make([]func() error, 0)
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
}
+16 -1
View File
@@ -627,13 +627,28 @@ func (s *Store) checkTemplateConstraints(ctx context.Context, res *types.Templat
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Handle if Template does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupTemplateByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+34 -5
View File
@@ -664,31 +664,60 @@ func (s *Store) checkUserConstraints(ctx context.Context, res *types.User) error
return nil
}
{
var checks = make([]func() error, 0)
checks = append(checks, func() error {
// Skip lookup by Email if User does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupUserByEmail(ctx, res.Email)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by Handle if User does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupUserByHandle(ctx, res.Handle)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
}
{
return nil
})
checks = append(checks, func() error {
// Skip lookup by Username if User does not match filters
if res.DeletedAt != nil {
return nil
}
ex, err := s.LookupUserByUsername(ctx, res.Username)
if err == nil && ex != nil && ex.ID != res.ID {
return store.ErrNotUnique.Stack(1)
} else if !errors.IsNotFound(err) {
return err
}
return nil
})
for _, check := range checks {
if err := check(); err != nil {
return err
}
}
return nil
+13 -3
View File
@@ -2,6 +2,10 @@ package tests
import (
"context"
"strings"
"testing"
"time"
"github.com/cortezaproject/corteza-server/pkg/filter"
"github.com/cortezaproject/corteza-server/pkg/id"
"github.com/cortezaproject/corteza-server/pkg/rand"
@@ -9,9 +13,6 @@ import (
"github.com/cortezaproject/corteza-server/system/types"
_ "github.com/joho/godotenv/autoload"
"github.com/stretchr/testify/require"
"strings"
"testing"
"time"
)
func testRoles(t *testing.T, s store.Roles) {
@@ -73,6 +74,15 @@ func testRoles(t *testing.T, s store.Roles) {
req.NoError(s.UpdateRole(ctx, role))
})
t.Run("create and update deleted with existing handle", func(t *testing.T) {
req, role := truncAndCreate(t)
deletedRole := makeNew("copy")
deletedRole.DeletedAt = now()
deletedRole.Handle = role.Handle
req.NoError(store.CreateRole(ctx, s, deletedRole))
req.NoError(store.UpdateRole(ctx, s, deletedRole))
})
t.Run("lookup by handle", func(t *testing.T) {
req, role := truncAndCreate(t)
fetched, err := s.LookupRoleByHandle(ctx, role.Handle)
+13 -3
View File
@@ -3,6 +3,10 @@ package tests
import (
"context"
"fmt"
"strings"
"testing"
"time"
"github.com/cortezaproject/corteza-server/pkg/filter"
"github.com/cortezaproject/corteza-server/pkg/id"
"github.com/cortezaproject/corteza-server/pkg/rand"
@@ -10,9 +14,6 @@ import (
"github.com/cortezaproject/corteza-server/system/types"
_ "github.com/joho/godotenv/autoload"
"github.com/stretchr/testify/require"
"strings"
"testing"
"time"
)
func testUsers(t *testing.T, s store.Users) {
@@ -144,6 +145,15 @@ func testUsers(t *testing.T, s store.Users) {
req.Equal(user.ID, fetched.ID)
})
t.Run("create and update deleted with existing email", func(t *testing.T) {
req, user := truncAndCreate(t)
deletedUser := makeNew("copy")
deletedUser.DeletedAt = now()
deletedUser.Email = user.Email
req.NoError(store.CreateUser(ctx, s, deletedUser))
req.NoError(store.UpdateUser(ctx, s, deletedUser))
})
t.Run("search", func(t *testing.T) {
t.Run("by ID", func(t *testing.T) {
req, prefill := truncAndFill(t, 5)