Merge branch '2023.9.x-fix-dedup' into 2023.9.x

2023-12-21 10:09:45 +01:00
parent 321e0ff908 52ea952116
commit 0a2dcd6b98
13 changed files with 178 additions and 67 deletions
--- a/client/web/compose/src/components/Admin/Module/UniqueValues.vue
+++ b/client/web/compose/src/components/Admin/Module/UniqueValues.vue
@@ -1,16 +1,27 @@
 <template>
  <div>
-    <c-hint
-      :tooltip="$t('tooltip.performance')"
-      icon-class="text-warning"
-      class="float-right"
-    />
+    <b-form-group label-class="d-flex align-items-center text-primary p-0">
+      <template #label>
+        {{ $t('enabled') }}
+        <c-hint
+          :tooltip="$t('tooltip.performance')"
+          icon-class="text-warning"
+        />
+      </template>
+
+      <c-input-checkbox
+        v-model="isEnabled"
+        switch
+        :labels="checkboxLabel"
+        data-test-id="checkbox-enabled"
+      />
+    </b-form-group>

    <div
      v-for="(rule, index) in rules"
      :key="index"
    >
-      <hr v-if="index">
+      <hr>

      <h5 class="d-flex align-items-center">
        {{ $t('uniqueValueConstraint', { index: index + 1 }) }}
@@ -127,9 +138,8 @@

    <div class="d-flex justify-content-end">
      <b-button
-        size="lg"
        variant="outline-light"
-        class="d-flex align-items-center border-0 text-primary mt-3"
+        class="d-flex align-items-center border-0 text-primary"
        @click="addNewConstraint"
      >
        <font-awesome-icon
@@ -172,11 +182,22 @@ export default {
      get () {
        return this.module.config.recordDeDup.rules
      },
+
      set (value) {
        this.module.config.recordDeDup.rules = value
      },
    },

+    isEnabled: {
+      get () {
+        return this.module.config.recordDeDup.enabled
+      },
+
+      set (value) {
+        this.module.config.recordDeDup.enabled = value
+      },
+    },
+
    modifierOptions () {
      return [
        { value: 'ignore-case', text: this.$t('ignoreCase') },
--- a/lib/js/src/compose/types/module.ts
+++ b/lib/js/src/compose/types/module.ts
@@ -58,6 +58,7 @@ interface Config {
  };

  recordDeDup: {
+    enabled: boolean;
    rules: RecordDeDupRule[];
  };
 }
@@ -174,12 +175,8 @@ export class Module {
    },

    recordDeDup: {
-      rules: [
-        {
-          strict: true,
-          constraints: []
-        }
-      ],
+      enabled: false,
+      rules: [],
    },
  }

--- a/locale/en/corteza-server/compose/module.yaml
+++ b/locale/en/corteza-server/compose/module.yaml
@@ -16,3 +16,4 @@ errors:
  notAllowedToUpdate: not allowed to update this module
  notFound: module does not exist
  staleData: stale data
+  dedupConfigurationInvalidMissingConstraint: missing constraints on deduplication configuration
--- a/locale/en/corteza-webapp-compose/module.yaml
+++ b/locale/en/corteza-webapp-compose/module.yaml
@@ -190,6 +190,7 @@ edit:

    uniqueValues:
      title: Unique values
+      enabled: Enable duplication detection
      preventRecordsSave: Prevent record saving if duplicate values are found
      warningLabel: Warning or error message toast when constraint matches
      valueModifiers: Value modifiers
@@ -209,7 +210,7 @@ edit:
      addNewConstraint: Add new constraint
      uniqueValueConstraint: "Unique value constraint #{{index}}"
      tooltip:
-        performance: Using unique value constraints will impact performance
+        performance: Using duplication detection will impact performance

 federated: Federated
 forModule:
--- a/server/compose/service/module.go
+++ b/server/compose/service/module.go
@@ -710,6 +710,10 @@ func (svc module) handleUpdate(ctx context.Context, upd *types.Module) moduleUpd
 			return moduleUnchanged, err
 		}

+		if err = validateModuleDedupRules(ctx, upd); err != nil {
+			return moduleUnchanged, ModuleErrDedupConfigurationInvalidMissingConstraint()
+		}
+
 		if !svc.ac.CanUpdateModule(ctx, res) {
 			return moduleUnchanged, ModuleErrNotAllowedToUpdate()
 		}
@@ -1095,6 +1099,10 @@ func loadModuleLabels(ctx context.Context, s store.Labels, set ...*types.Module)
 	return nil
 }

+func validateModuleDedupRules(ctx context.Context, m *types.Module) error {
+	return m.Config.RecordDeDup.Rules.Validate()
+}
+
 // DalModelReload reloads all defined compose modules into the DAL
 func DalModelReload(ctx context.Context, s store.Storer, am schemaAltManager, dmm dalModelManager) (err error) {
 	// Get all available namespaces
--- a/server/compose/service/module_actions.gen.go
+++ b/server/compose/service/module_actions.gen.go
@@ -55,7 +55,6 @@ var (
 // setModule updates moduleActionProps's module
 //
 // This function is auto-generated.
-//
 func (p *moduleActionProps) setModule(module *types.Module) *moduleActionProps {
 	p.module = module
 	return p
@@ -64,7 +63,6 @@ func (p *moduleActionProps) setModule(module *types.Module) *moduleActionProps {
 // setChanged updates moduleActionProps's changed
 //
 // This function is auto-generated.
-//
 func (p *moduleActionProps) setChanged(changed *types.Module) *moduleActionProps {
 	p.changed = changed
 	return p
@@ -73,7 +71,6 @@ func (p *moduleActionProps) setChanged(changed *types.Module) *moduleActionProps
 // setFilter updates moduleActionProps's filter
 //
 // This function is auto-generated.
-//
 func (p *moduleActionProps) setFilter(filter *types.ModuleFilter) *moduleActionProps {
 	p.filter = filter
 	return p
@@ -82,7 +79,6 @@ func (p *moduleActionProps) setFilter(filter *types.ModuleFilter) *moduleActionP
 // setNamespace updates moduleActionProps's namespace
 //
 // This function is auto-generated.
-//
 func (p *moduleActionProps) setNamespace(namespace *types.Namespace) *moduleActionProps {
 	p.namespace = namespace
 	return p
@@ -91,7 +87,6 @@ func (p *moduleActionProps) setNamespace(namespace *types.Namespace) *moduleActi
 // Serialize converts moduleActionProps to actionlog.Meta
 //
 // This function is auto-generated.
-//
 func (p moduleActionProps) Serialize() actionlog.Meta {
 	var (
 		m = make(actionlog.Meta)
@@ -132,7 +127,6 @@ func (p moduleActionProps) Serialize() actionlog.Meta {
 // tr translates string and replaces meta value placeholder with values
 //
 // This function is auto-generated.
-//
 func (p moduleActionProps) Format(in string, err error) string {
 	var (
 		pairs = []string{"{{err}}"}
@@ -243,7 +237,6 @@ func (p moduleActionProps) Format(in string, err error) string {
 // String returns loggable description as string
 //
 // This function is auto-generated.
-//
 func (a *moduleAction) String() string {
 	var props = &moduleActionProps{}

@@ -271,7 +264,6 @@ func (e *moduleAction) ToAction() *actionlog.Action {
 // ModuleActionSearch returns "compose:module.search" action
 //
 // This function is auto-generated.
-//
 func ModuleActionSearch(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -291,7 +283,6 @@ func ModuleActionSearch(props ...*moduleActionProps) *moduleAction {
 // ModuleActionLookup returns "compose:module.lookup" action
 //
 // This function is auto-generated.
-//
 func ModuleActionLookup(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -311,7 +302,6 @@ func ModuleActionLookup(props ...*moduleActionProps) *moduleAction {
 // ModuleActionCreate returns "compose:module.create" action
 //
 // This function is auto-generated.
-//
 func ModuleActionCreate(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -331,7 +321,6 @@ func ModuleActionCreate(props ...*moduleActionProps) *moduleAction {
 // ModuleActionUpdate returns "compose:module.update" action
 //
 // This function is auto-generated.
-//
 func ModuleActionUpdate(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -351,7 +340,6 @@ func ModuleActionUpdate(props ...*moduleActionProps) *moduleAction {
 // ModuleActionDelete returns "compose:module.delete" action
 //
 // This function is auto-generated.
-//
 func ModuleActionDelete(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -371,7 +359,6 @@ func ModuleActionDelete(props ...*moduleActionProps) *moduleAction {
 // ModuleActionUndelete returns "compose:module.undelete" action
 //
 // This function is auto-generated.
-//
 func ModuleActionUndelete(props ...*moduleActionProps) *moduleAction {
 	a := &moduleAction{
 		timestamp: time.Now(),
@@ -394,9 +381,7 @@ func ModuleActionUndelete(props ...*moduleActionProps) *moduleAction {

 // ModuleErrGeneric returns "compose:module.generic" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrGeneric(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -430,9 +415,7 @@ func ModuleErrGeneric(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotFound returns "compose:module.notFound" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotFound(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -464,9 +447,7 @@ func ModuleErrNotFound(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNamespaceNotFound returns "compose:module.namespaceNotFound" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNamespaceNotFound(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -498,9 +479,7 @@ func ModuleErrNamespaceNotFound(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrInvalidID returns "compose:module.invalidID" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrInvalidID(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -532,9 +511,7 @@ func ModuleErrInvalidID(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrInvalidHandle returns "compose:module.invalidHandle" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrInvalidHandle(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -566,9 +543,7 @@ func ModuleErrInvalidHandle(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrHandleNotUnique returns "compose:module.handleNotUnique" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrHandleNotUnique(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -602,9 +577,7 @@ func ModuleErrHandleNotUnique(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNameNotUnique returns "compose:module.nameNotUnique" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNameNotUnique(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -638,9 +611,7 @@ func ModuleErrNameNotUnique(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrFieldNameReserved returns "compose:module.fieldNameReserved" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrFieldNameReserved(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -672,9 +643,7 @@ func ModuleErrFieldNameReserved(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrStaleData returns "compose:module.staleData" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrStaleData(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -706,9 +675,7 @@ func ModuleErrStaleData(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrInvalidNamespaceID returns "compose:module.invalidNamespaceID" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrInvalidNamespaceID(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -740,9 +707,7 @@ func ModuleErrInvalidNamespaceID(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToRead returns "compose:module.notAllowedToRead" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToRead(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -776,9 +741,7 @@ func ModuleErrNotAllowedToRead(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToSearch returns "compose:module.notAllowedToSearch" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToSearch(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -812,9 +775,7 @@ func ModuleErrNotAllowedToSearch(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToReadNamespace returns "compose:module.notAllowedToReadNamespace" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToReadNamespace(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -848,9 +809,7 @@ func ModuleErrNotAllowedToReadNamespace(mm ...*moduleActionProps) *errors.Error

 // ModuleErrNotAllowedToListModules returns "compose:module.notAllowedToListModules" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToListModules(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -884,9 +843,7 @@ func ModuleErrNotAllowedToListModules(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToCreate returns "compose:module.notAllowedToCreate" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToCreate(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -920,9 +877,7 @@ func ModuleErrNotAllowedToCreate(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToUpdate returns "compose:module.notAllowedToUpdate" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToUpdate(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -956,9 +911,7 @@ func ModuleErrNotAllowedToUpdate(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToDelete returns "compose:module.notAllowedToDelete" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToDelete(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -992,9 +945,7 @@ func ModuleErrNotAllowedToDelete(mm ...*moduleActionProps) *errors.Error {

 // ModuleErrNotAllowedToUndelete returns "compose:module.notAllowedToUndelete" as *errors.Error
 //
-//
 // This function is auto-generated.
-//
 func ModuleErrNotAllowedToUndelete(mm ...*moduleActionProps) *errors.Error {
 	var p = &moduleActionProps{}
 	if len(mm) > 0 {
@@ -1026,6 +977,40 @@ func ModuleErrNotAllowedToUndelete(mm ...*moduleActionProps) *errors.Error {
 	return e
 }

+// ModuleErrDedupConfigurationInvalidMissingConstraint returns "compose:module.dedupConfigurationInvalidMissingConstraint" as *errors.Error
+//
+// This function is auto-generated.
+func ModuleErrDedupConfigurationInvalidMissingConstraint(mm ...*moduleActionProps) *errors.Error {
+	var p = &moduleActionProps{}
+	if len(mm) > 0 {
+		p = mm[0]
+	}
+
+	var e = errors.New(
+		errors.KindInternal,
+
+		p.Format("invalid deduplication configuration - constraint not defined", nil),
+
+		errors.Meta("type", "dedupConfigurationInvalidMissingConstraint"),
+		errors.Meta("resource", "compose:module"),
+
+		// action log entry; no formatting, it will be applied inside recordAction fn.
+		errors.Meta(moduleLogMetaKey{}, "missing constraint on deduplication configuration for module"),
+		errors.Meta(modulePropsMetaKey{}, p),
+
+		// translation namespace & key
+		errors.Meta(locale.ErrorMetaNamespace{}, "compose"),
+		errors.Meta(locale.ErrorMetaKey{}, "module.errors.dedupConfigurationInvalidMissingConstraint"),
+
+		errors.StackSkip(1),
+	)
+
+	if len(mm) > 0 {
+	}
+
+	return e
+}
+
 // *********************************************************************************************************************
 // *********************************************************************************************************************

@@ -1034,7 +1019,6 @@ func ModuleErrNotAllowedToUndelete(mm ...*moduleActionProps) *errors.Error {
 // It will wrap unrecognized/internal errors with generic errors.
 //
 // This function is auto-generated.
-//
 func (svc module) recordAction(ctx context.Context, props *moduleActionProps, actionFn func(...*moduleActionProps) *moduleAction, err error) error {
 	if svc.actionlog == nil || actionFn == nil {
 		// action log disabled or no action fn passed, return error as-is
--- a/server/compose/service/module_actions.yaml
+++ b/server/compose/service/module_actions.yaml
@@ -116,3 +116,7 @@ errors:
  - error: notAllowedToUndelete
    message: "not allowed to undelete this module"
    log: "could not undelete {{module}}; insufficient permissions"
+
+  - error: dedupConfigurationInvalidMissingConstraint
+    message: "invalid deduplication configuration - constraint not defined"
+    log: "missing constraint on deduplication configuration for module"
--- a/server/compose/service/record.go
+++ b/server/compose/service/record.go
@@ -2025,6 +2025,10 @@ func (svc record) DupDetection(ctx context.Context, m *types.Module, rec *types.
 		config = m.Config.RecordDeDup
 	)

+	if !config.Enabled {
+		return
+	}
+
 	if len(config.Rules) > 0 {
 		records, _, err = svc.Find(ctx, types.RecordFilter{
 			ModuleID:    m.ID,
--- a/server/compose/types/module.go
+++ b/server/compose/types/module.go
@@ -90,6 +90,8 @@ type (
 	}

 	ModuleConfigRecordDeDup struct {
+		Enabled bool `json:"enabled"`
+
 		// strictly restrict record saving
 		// 		otherwise show a warning with list of duplicated records
 		Strict bool `json:"-"`
--- a/server/compose/types/record_detect_duplicates.go
+++ b/server/compose/types/record_detect_duplicates.go
@@ -3,10 +3,11 @@ package types
 import (
 	"context"
 	"fmt"
+	"strings"
+
 	"github.com/cortezaproject/corteza/server/pkg/locale"
 	"github.com/cortezaproject/corteza/server/pkg/str"
 	"github.com/spf13/cast"
-	"strings"
 )

 type (
@@ -181,6 +182,24 @@ func (rule DeDupRule) checkCaseSensitiveDuplication(ctx context.Context, ls loca
 	return
 }

+func (dr DeDupRuleSet) Validate() (err error) {
+	return dr.Walk(func(rule *DeDupRule) (err error) {
+		if !rule.HasAttributes() {
+			err = fmt.Errorf("deduplication not valid (no constraints)")
+			return
+		}
+
+		for _, a := range rule.Attributes() {
+			if a == "" {
+				err = fmt.Errorf("deduplication not valid (invalid field)")
+				return
+			}
+		}
+
+		return
+	})
+}
+
 func (c DeDupRuleConstraint) HasMultiValue() bool {
 	switch c.MultiValue {
 	case oneOf, equal:
--- a/server/compose/types/record_detect_duplicates_test.go
+++ b/server/compose/types/record_detect_duplicates_test.go
@@ -2,10 +2,11 @@ package types

 import (
 	"context"
+	"testing"
+
 	"github.com/cortezaproject/corteza/server/pkg/locale"
 	"github.com/spf13/cast"
 	"github.com/stretchr/testify/require"
-	"testing"
 )

 func TestDeDupRule_checkCaseSensitiveDuplication(t *testing.T) {
@@ -124,3 +125,41 @@ func Test_matchValue(t *testing.T) {
 		})
 	}
 }
+
+func Test_rulesetValidation(t *testing.T) {
+	var (
+		req = require.New(t)
+
+		tests = []struct {
+			name    string
+			ruleset DeDupRuleSet
+		}{
+			{
+				name: "no constraint",
+				ruleset: DeDupRuleSet{&DeDupRule{
+					Name:          "",
+					Strict:        true,
+					ConstraintSet: []*DeDupRuleConstraint{},
+				}},
+			},
+			{
+				name: "invalid constraint",
+				ruleset: DeDupRuleSet{&DeDupRule{
+					Name:   "",
+					Strict: true,
+					ConstraintSet: []*DeDupRuleConstraint{
+						{
+							Attribute: "",
+						},
+					},
+				}},
+			},
+		}
+	)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req.Error(tt.ruleset.Validate())
+		})
+	}
+}
--- a/server/pkg/provision/dedup.go
+++ b/server/pkg/provision/dedup.go
@@ -0,0 +1,30 @@
+package provision
+
+import (
+	"context"
+
+	"github.com/cortezaproject/corteza/server/compose/types"
+	"github.com/cortezaproject/corteza/server/store"
+	"go.uber.org/zap"
+)
+
+func invalidateDedupRules(ctx context.Context, log *zap.Logger, s store.Storer) (err error) {
+	var ll types.ModuleSet
+
+	if ll, _, err = s.SearchComposeModules(ctx, types.ModuleFilter{}); err != nil {
+		return
+	}
+
+	ll, _ = ll.Filter(func(m *types.Module) (bool, error) {
+		return m.Config.RecordDeDup.Rules.Validate() != nil, nil
+	})
+
+	ll.Walk(func(m *types.Module) error {
+		m.Config.RecordDeDup.Enabled = false
+		return nil
+	})
+
+	err = s.UpdateComposeModule(ctx, ll...)
+
+	return
+}
--- a/server/pkg/provision/provision.go
+++ b/server/pkg/provision/provision.go
@@ -40,6 +40,7 @@ func Run(ctx context.Context, log *zap.Logger, s store.Storer, provisionOpt opti
 		func() error { return oidcAutoDiscovery(ctx, log.Named("auth.oidc-auto-discovery"), s, authOpt) },
 		func() error { return defaultAuthClient(ctx, log.Named("auth.clients"), s, authOpt) },
 		func() error { return addAuthSuperUsers(ctx, log.Named("auth.super-users"), s, authOpt) },
+		func() error { return invalidateDedupRules(ctx, log.Named("compose.deduplication"), s) },
 	}

 	for _, fn := range ffn {