DeforaNetworks/corteza
3
0
Fork 0
Code Activity

Add support for RDBMS schema management

Browse Source 
With `DB_ALLOW_DESTRUCTIVE_CHANGES=true` Corteza can change DB tables and columns when Compose Module configuration changes.
This commit is contained in:
skamensky
2022-11-26 11:40:23 +02:00
committed by Denis Arh
parent 19191a4312
commit 9ac63876be
12 changed files with 195 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
server/.env.example
View File

@@ -26,6 +26,12 @@
# Default: sqlite3://file::memory:?cache=shared&mode=memory
# DB_DSN=sqlite3://file::memory:?cache=shared&mode=memory
###############################################################################
# Allow for irreversible changes to the database schema such as dropping columns and tables.
# Type: bool
# Default: <no value>
# DB_ALLOW_DESTRUCTIVE_CHANGES=<no value>
###############################################################################
###############################################################################
# HTTP Client

5
server/app/options/db.cue
View File

@@ -11,6 +11,11 @@ DB: schema.#optionsGroup & {
defaultValue: "sqlite3://file::memory:?cache=shared&mode=memory"
description: "Database connection string."
}
allow_destructive_schema_changes: {
type: "bool"
defaultGoExpr: "false"
description: "Allow for irreversible changes to the database schema such as dropping columns and tables."
}
}
title: "Connection to data store backend"
}

2
server/compose/service/dal_interfaces.go
View File

@@ -12,7 +12,7 @@ type (
SearchModels(ctx context.Context) (out dal.ModelSet, err error)
ReplaceModel(ctx context.Context, model *dal.Model) (err error)
RemoveModel(ctx context.Context, connectionID, ID uint64) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, old, new *dal.Attribute, trans ...dal.TransformationFunction) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, diff *dal.ModelDiff, hasRecords bool, trans ...dal.TransformationFunction) (err error)
GetConnectionByID(uint64) *dal.ConnectionWrap

32
server/compose/service/module.go
View File

@@ -79,7 +79,7 @@ type (
ReplaceModel(context.Context, *dal.Model) error
RemoveModel(ctx context.Context, connectionID, ID uint64) error
ReplaceModelAttribute(ctx context.Context, model *dal.Model, old, new *dal.Attribute, trans ...dal.TransformationFunction) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, diff *dal.ModelDiff, hasRecords bool, trans ...dal.TransformationFunction) (err error)
SearchModelIssues(ID uint64) []error
}
)
@@ -516,6 +516,8 @@ func (svc module) updater(ctx context.Context, namespaceID, moduleID uint64, act
err error
defConn *dal.ConnectionWrap
hasRecords bool
)
err = store.Tx(ctx, svc.store, func(ctx context.Context, s store.Storer) (err error) {
@@ -577,8 +579,7 @@ func (svc module) updater(ctx context.Context, namespaceID, moduleID uint64, act
if changes&moduleFieldsChanged > 0 {
var (
hasRecords bool
set types.RecordSet
set types.RecordSet
recFilter = types.RecordFilter{
Paging: filter.Paging{Limit: 1},
@@ -631,7 +632,7 @@ func (svc module) updater(ctx context.Context, namespaceID, moduleID uint64, act
if err = DalModelReplace(ctx, svc.dal, ns, old, m); err != nil {
return err
}
if err = dalAttributeReplace(ctx, svc.dal, ns, old, m); err != nil {
if err = dalAttributeReplace(ctx, svc.dal, ns, old, m, hasRecords); err != nil {
return err
}
} else {
@@ -1168,7 +1169,7 @@ func DalModelReplace(ctx context.Context, dmm dalModelManager, ns *types.Namespa
return
}
func dalAttributeReplace(ctx context.Context, dmm dalModelManager, ns *types.Namespace, old, new *types.Module) (err error) {
func dalAttributeReplace(ctx context.Context, dmm dalModelManager, ns *types.Namespace, old, new *types.Module, hasRecords bool) (err error) {
oldModel, err := modulesToModelSet(dmm, ns, old)
if err != nil {
return
@@ -1179,8 +1180,10 @@ func dalAttributeReplace(ctx context.Context, dmm dalModelManager, ns *types.Nam
}
diff := oldModel[0].Diff(newModel[0])
// TODO handle the fact that diff is a list of changes so the same field could be present more than once.
for _, d := range diff {
if err = dmm.ReplaceModelAttribute(ctx, oldModel[0], d.Original, d.Asserted); err != nil {
if err = dmm.ReplaceModelAttribute(ctx, oldModel[0], d, hasRecords); err != nil {
return
}
}
@@ -1319,6 +1322,8 @@ func ModuleToModel(ns *types.Namespace, mod *types.Module, inhIdent string) (mod
SensitivityLevelID: mod.Config.Privacy.SensitivityLevelID,
}
userDefinedFieldIdents := make(map[string]bool)
if model.Ident = mod.Config.DAL.Ident; model.Ident == "" {
// try with explicitly set ident on module's DAL config
// and fallback connection's default if it is empty
@@ -1346,6 +1351,11 @@ func ModuleToModel(ns *types.Namespace, mod *types.Module, inhIdent string) (mod
if err != nil {
return
}
for _, attr := range attrAux {
userDefinedFieldIdents[attr.Ident] = true
}
model.Attributes = append(model.Attributes, attrAux...)
// Convert system fields to attribute
@@ -1353,7 +1363,15 @@ func ModuleToModel(ns *types.Namespace, mod *types.Module, inhIdent string) (mod
if err != nil {
return
}
model.Attributes = append(model.Attributes, attrAux...)
for _, attr := range attrAux {
ok, _ := userDefinedFieldIdents[attr.Ident]
if !ok {
// make sure we're backward compatible:
// if, by some weird case, someone managed to get a system field name into
// the store, we'll turn a blind eye. We need to make sure not to include the field twice in this situation.
model.Attributes = append(model.Attributes, attr)
}
}
return
}

59
server/pkg/dal/diff.go
View File

@@ -1,24 +1,29 @@
package dal
type (
modelDiffType string
modelDiffType string
ModelModification string
// ModelDiff defines one identified missmatch between two models
ModelDiff struct {
Type modelDiffType
Type modelDiffType
Modification ModelModification
// Original will be nil when a new attribute is being added
Original *Attribute
// Asserted will be nil wen an existing attribute is being removed
Asserted *Attribute
// Inserted will be nil wen an existing attribute is being removed
Inserted *Attribute
}
ModelDiffSet []*ModelDiff
)
const (
AttributeMissing modelDiffType = "attributeMissing"
AttributeTypeMissmatch modelDiffType = "typeMissmatch"
AttributeSensitivityMissmatch modelDiffType = "sensitivityMissmatch"
AttributeMissing modelDiffType = "attributeMissing"
AttributeTypeMissmatch modelDiffType = "typeMissmatch"
AttributeSensitivityMismatch modelDiffType = "sensitivityMismatch"
AttributeCodecMismatch modelDiffType = "codecMismatch"
AttributeDeleted ModelModification = "deleted"
AttributeAdded ModelModification = "added"
AttributeChanged ModelModification = "changed"
)
// Diff calculates the diff between models a and b where a is used as base
@@ -54,13 +59,18 @@ func (a *Model) Diff(b *Model) (out ModelDiffSet) {
// Deleted and update ones
for _, _attrA := range a.Attributes {
attrA := _attrA
// store is an interface to something that could be a pointer.
// we need to copy it to make sure we don't get a nil pointer
// make sure not to modify this since it would modify the original
attrA.Store = _attrA.Store
// Missmatches
attrBAux, ok := bIndex[attrA.Ident]
if !ok {
out = append(out, &ModelDiff{
Type: AttributeMissing,
Original: attrA,
Type: AttributeMissing,
Modification: AttributeDeleted,
Original: attrA,
})
continue
}
@@ -68,9 +78,10 @@ func (a *Model) Diff(b *Model) (out ModelDiffSet) {
// Typecheck
if attrA.Type.Type() != attrBAux.attr.Type.Type() {
out = append(out, &ModelDiff{
Type: AttributeTypeMissmatch,
Original: attrA,
Asserted: attrBAux.attr,
Type: AttributeTypeMissmatch,
Modification: AttributeChanged,
Original: attrA,
Inserted: attrBAux.attr,
})
}
@@ -78,9 +89,18 @@ func (a *Model) Diff(b *Model) (out ModelDiffSet) {
// @todo improve; for now it'll do
if attrA.SensitivityLevelID != attrBAux.attr.SensitivityLevelID {
out = append(out, &ModelDiff{
Type: AttributeSensitivityMissmatch,
Original: attrA,
Asserted: attrBAux.attr,
Type: AttributeSensitivityMismatch,
Modification: AttributeChanged,
Original: attrA,
Inserted: attrBAux.attr,
})
}
if attrA.Store.Type() != attrBAux.attr.Store.Type() {
out = append(out, &ModelDiff{
Type: AttributeCodecMismatch,
Modification: AttributeChanged,
Original: attrA,
Inserted: attrBAux.attr,
})
}
}
@@ -93,9 +113,10 @@ func (a *Model) Diff(b *Model) (out ModelDiffSet) {
_, ok := aIndex[attrB.Ident]
if !ok {
out = append(out, &ModelDiff{
Type: AttributeMissing,
Original: nil,
Asserted: attrB,
Type: AttributeMissing,
Modification: AttributeAdded,
Original: nil,
Inserted: attrB,
})
continue
}

38
server/pkg/dal/diff_test.go
View File

@@ -11,6 +11,7 @@ func TestDiff_same(t *testing.T) {
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
@@ -23,18 +24,21 @@ func TestDiff_wrongAttrType(t *testing.T) {
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
b := &Model{
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeBlob{},
Store: &CodecPlain{},
}},
}
dd := a.Diff(b)
require.Len(t, dd, 1)
require.Equal(t, AttributeTypeMissmatch, dd[0].Type)
require.Equal(t, AttributeChanged, dd[0].Modification)
}
func TestDiff_removedAttr(t *testing.T) {
@@ -42,23 +46,27 @@ func TestDiff_removedAttr(t *testing.T) {
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}, {
Ident: "F2",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
b := &Model{
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
dd := a.Diff(b)
require.Len(t, dd, 1)
require.Equal(t, AttributeMissing, dd[0].Type)
require.Equal(t, AttributeDeleted, dd[0].Modification)
require.NotNil(t, dd[0].Original)
require.Nil(t, dd[0].Asserted)
require.Nil(t, dd[0].Inserted)
}
func TestDiff_addedAttr(t *testing.T) {
@@ -66,21 +74,47 @@ func TestDiff_addedAttr(t *testing.T) {
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
b := &Model{
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}, {
Ident: "F2",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
dd := a.Diff(b)
require.Len(t, dd, 1)
require.Equal(t, AttributeMissing, dd[0].Type)
require.Equal(t, AttributeAdded, dd[0].Modification)
require.Nil(t, dd[0].Original)
require.NotNil(t, dd[0].Asserted)
require.NotNil(t, dd[0].Inserted)
}
func TestDiff_changedCodec(t *testing.T) {
a := &Model{
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}
b := &Model{
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecRecordValueSetJSON{},
}},
}
dd := a.Diff(b)
require.Len(t, dd, 1)
require.Equal(t, AttributeCodecMismatch, dd[0].Type)
require.Equal(t, AttributeChanged, dd[0].Modification)
}

2
server/pkg/dal/driver.go
View File

@@ -85,7 +85,7 @@ type (
// Specific operations require data transformations (type change).
// Some basic ops. should be implemented on DB driver level, but greater controll can be
// achieved via the trans functions.
UpdateModelAttribute(ctx context.Context, sch *Model, old, new *Attribute, trans ...TransformationFunction) error
UpdateModelAttribute(ctx context.Context, sch *Model, diff *ModelDiff, hasRecords bool, trans ...TransformationFunction) error
}
ConnectionCloser interface {

28
server/pkg/dal/service.go
View File

@@ -44,7 +44,7 @@ type (
SearchModels(ctx context.Context) (out ModelSet, err error)
ReplaceModel(ctx context.Context, model *Model) (err error)
RemoveModel(ctx context.Context, connectionID, ID uint64) (err error)
ReplaceModelAttribute(ctx context.Context, model *Model, old, new *Attribute, trans ...TransformationFunction) (err error)
ReplaceModelAttribute(ctx context.Context, model *Model, dif *ModelDiff, hasRecords bool, trans ...TransformationFunction) (err error)
FindModelByResourceID(connectionID uint64, resourceID uint64) *Model
FindModelByResourceIdent(connectionID uint64, resourceType, resourceIdent string) *Model
FindModelByIdent(connectionID uint64, ident string) *Model
@@ -833,7 +833,7 @@ func (svc *service) removeModelFromRegistry(model *Model) {
// ReplaceModelAttribute adds new or updates an existing attribute for the given model
//
// We rely on the user to provide stable and valid attribute definitions.
func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, old, new *Attribute, trans ...TransformationFunction) (err error) {
func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, diff *ModelDiff, hasRecords bool, trans ...TransformationFunction) (err error) {
svc.logger.Debug("updating model attribute", zap.Uint64("model", model.ResourceID))
var (
@@ -860,12 +860,12 @@ func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, old
}
// In case we're deleting it we can ignore this check
if new != nil {
if !svc.sensitivityLevels.includes(new.SensitivityLevelID) {
issues.addModelIssue(model.ResourceID, errAttributeUpdateMissingSensitivityLevel(model.ConnectionID, model.ResourceID, new.SensitivityLevelID))
if diff.Inserted != nil {
if !svc.sensitivityLevels.includes(diff.Inserted.SensitivityLevelID) {
issues.addModelIssue(model.ResourceID, errAttributeUpdateMissingSensitivityLevel(model.ConnectionID, model.ResourceID, diff.Inserted.SensitivityLevelID))
} else {
if !svc.sensitivityLevels.isSubset(new.SensitivityLevelID, model.SensitivityLevelID) {
issues.addModelIssue(model.ResourceID, errAttributeUpdateGreaterSensitivityLevel(model.ConnectionID, model.ResourceID, new.SensitivityLevelID, model.SensitivityLevelID))
if !svc.sensitivityLevels.isSubset(diff.Inserted.SensitivityLevelID, model.SensitivityLevelID) {
issues.addModelIssue(model.ResourceID, errAttributeUpdateGreaterSensitivityLevel(model.ConnectionID, model.ResourceID, diff.Inserted.SensitivityLevelID, model.SensitivityLevelID))
}
}
}
@@ -881,7 +881,7 @@ func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, old
if !modelIssues && !connectionIssues {
svc.logger.Debug("updating model attribute", zap.Uint64("connection", model.ConnectionID), zap.Uint64("model", model.ResourceID))
err = conn.connection.UpdateModelAttribute(ctx, model, old, new, trans...)
err = conn.connection.UpdateModelAttribute(ctx, model, diff, hasRecords, trans...)
if err != nil {
issues.addModelIssue(model.ResourceID, err)
}
@@ -895,15 +895,15 @@ func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, old
}
// Update registry
if old == nil {
if diff.Original == nil {
// adding
model.Attributes = append(model.Attributes, new)
} else if new == nil {
model.Attributes = append(model.Attributes, diff.Original)
} else if diff.Original == nil {
// removing
model = svc.FindModelByResourceID(model.ConnectionID, model.ResourceID)
nSet := make(AttributeSet, 0, len(model.Attributes))
for _, attribute := range model.Attributes {
if attribute.Ident != old.Ident {
if attribute.Ident != diff.Original.Ident {
nSet = append(nSet, attribute)
}
}
@@ -912,8 +912,8 @@ func (svc *service) ReplaceModelAttribute(ctx context.Context, model *Model, old
// updating
model = svc.FindModelByResourceID(model.ConnectionID, model.ResourceID)
for i, attribute := range model.Attributes {
if attribute.Ident == old.Ident {
model.Attributes[i] = new
if attribute.Ident == diff.Original.Ident {
model.Attributes[i] = diff.Inserted
break
}
}

6
server/pkg/options/options.gen.go generated
View File

@@ -14,7 +14,8 @@ import (
type (
DBOpt struct {
DSN string `env:"DB_DSN"`
DSN string `env:"DB_DSN"`
AllowDestructiveSchemaChanges bool `env:"DB_ALLOW_DESTRUCTIVE_SCHEMA_CHANGES"`
}
HTTPClientOpt struct {
@@ -266,7 +267,8 @@ type (
// This function is auto-generated
func DB() (o *DBOpt) {
o = &DBOpt{
DSN: "sqlite3://file::memory:?cache=shared&mode=memory",
DSN: "sqlite3://file::memory:?cache=shared&mode=memory",
AllowDestructiveSchemaChanges: false,
}
// Custom defaults

66
server/store/adapters/rdbms/dal/connection.go
View File

@@ -3,6 +3,7 @@ package dal
import (
"context"
"fmt"
"github.com/cortezaproject/corteza/server/pkg/options"
"sync"
"github.com/cortezaproject/corteza/server/pkg/errors"
@@ -161,7 +162,7 @@ func (c *connection) CreateModel(ctx context.Context, mm ...*dal.Model) (err err
c.mux.Lock()
defer c.mux.Unlock()
for _, m := range mm {
err = ddl.UpdateModel(ctx, c.dataDefiner, m)
_, err = c.dataDefiner.TableLookup(ctx, m.Ident)
if errors.IsNotFound(err) {
if err = ddl.CreateModel(ctx, c.dataDefiner, m); err != nil {
return
@@ -169,6 +170,9 @@ func (c *connection) CreateModel(ctx context.Context, mm ...*dal.Model) (err err
} else if err != nil {
return
}
if err = ddl.EnsureIndexes(ctx, c.dataDefiner, m.Indexes...); err != nil {
return
}
// cache the model
c.models[cacheKey(m)] = Model(m, c.db, c.dialect)
@@ -220,12 +224,62 @@ func (c *connection) UpdateModel(ctx context.Context, old *dal.Model, new *dal.M
}
// UpdateModelAttribute alters column on a db table and runs data transformations
func (c *connection) UpdateModelAttribute(ctx context.Context, sch *dal.Model, old, new *dal.Attribute, trans ...dal.TransformationFunction) error {
// not raising not-supported error
// because we do not want to break
// DAL service model adding procedure
func (c *connection) UpdateModelAttribute(ctx context.Context, sch *dal.Model, diff *dal.ModelDiff, hasRecords bool, trans ...dal.TransformationFunction) error {
// @todo apply transformations
// @todo implement model column altering
var (
sampleAttribute *dal.Attribute
)
// this is mainly for messages code-paths where we don't care which attribute provides the information
if diff.Original != nil {
sampleAttribute = diff.Original
} else {
sampleAttribute = diff.Inserted
}
if diff.Type == dal.AttributeCodecMismatch {
return fmt.Errorf("cannot alter storage codec of attribute %s from %v to %v. ", sampleAttribute.Ident, diff.Original.Store.Type(), diff.Inserted.Store.Type())
}
// we're guaranteed by the check above that both codecs are the same
if sampleAttribute.Store.Type() != (&dal.CodecPlain{}).Type() {
// no need to alter column since this is not a normal column. It's a value column.
// Don't raise not-supported error in order to keep feature parity with previous implementation.
// i.e. we don't want to break DAL service model adding procedure
return nil
}
if !options.DB().AllowDestructiveSchemaChanges {
return fmt.Errorf("cannot modify %s. Changing physical schemas is not yet supported", sampleAttribute.Ident)
}
// @todo don't use a string literal. Receive the name from somewhere else
if sch.Ident == "compose_record" {
return fmt.Errorf(`issue adding %s. Cannot modify the schema of the generic "compose_record" table. Try setting your table name to a non-default value`, sampleAttribute.Ident)
}
switch diff.Modification {
case dal.AttributeChanged:
if diff.Modification == dal.AttributeChanged {
// @todo implement model column altering
return fmt.Errorf("cannot alter %s, physical column modification is not yet supported", sampleAttribute.Ident)
}
case dal.AttributeAdded:
if !diff.Inserted.Type.IsNullable() && hasRecords {
return fmt.Errorf("cannot add non-nullable attribute %s since there are records in the table", diff.Inserted.Ident)
}
col, err := c.dataDefiner.ConvertAttribute(diff.Inserted)
if err != nil {
return err
}
err = c.dataDefiner.ColumnAdd(ctx, sch.Ident, col)
if err != nil {
return err
}
case dal.AttributeDeleted:
err := c.dataDefiner.ColumnDrop(ctx, sch.Ident, diff.Original.StoreIdent())
if err != nil {
return err
}
}
return nil
}

3
server/tests/dal/dal_utils_test.go
View File

@@ -39,7 +39,8 @@ type (
SearchModels(ctx context.Context) (out dal.ModelSet, err error)
ReplaceModel(ctx context.Context, model *dal.Model) (err error)
RemoveModel(ctx context.Context, connectionID, ID uint64) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, old, new *dal.Attribute, trans ...dal.TransformationFunction) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, diff *dal.ModelDiff, hasRecords bool, trans ...dal.TransformationFunction) (err error)
FindModelByResourceID(connectionID uint64, resourceID uint64) *dal.Model
FindModelByResourceIdent(connectionID uint64, resourceType, resourceIdent string) *dal.Model
FindModelByIdent(connectionID uint64, ident string) *dal.Model

2
server/tests/workflows/main_test.go
View File

@@ -39,7 +39,7 @@ type (
SearchModels(ctx context.Context) (out dal.ModelSet, err error)
RemoveModel(ctx context.Context, connectionID, ID uint64) (err error)
ReplaceModel(ctx context.Context, model *dal.Model) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, old, new *dal.Attribute, trans ...dal.TransformationFunction) (err error)
ReplaceModelAttribute(ctx context.Context, model *dal.Model, diff *dal.ModelDiff, hasRecords bool, trans ...dal.TransformationFunction) (err error)
SearchModelIssues(resourceID uint64) (out []error)
Create(ctx context.Context, m dal.ModelRef, operations dal.OperationSet, vv ...dal.ValueGetter) error