Skip to content

Commit

Permalink
Add support for RDBMS schema management
Browse files Browse the repository at this point in the history
With `DB_ALLOW_DESTRUCTIVE_CHANGES=true` Corteza can change DB tables and columns when Compose Module configuration changes.
  • Loading branch information
skamensky authored and darh committed Nov 26, 2022
1 parent 19191a4 commit 9ac6387
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 54 deletions.
6 changes: 6 additions & 0 deletions server/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions server/app/options/db.cue
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 1 addition & 1 deletion server/compose/service/dal_interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 25 additions & 7 deletions server/compose/service/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1346,14 +1351,27 @@ 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
attrAux, err = moduleSystemFieldsToAttributes(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
}
Expand Down
59 changes: 40 additions & 19 deletions server/pkg/dal/diff.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -54,33 +59,48 @@ 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
}

// 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,
})
}

// Other stuff
// @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,
})
}
}
Expand All @@ -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
}
Expand Down
38 changes: 36 additions & 2 deletions server/pkg/dal/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ func TestDiff_same(t *testing.T) {
Attributes: AttributeSet{{
Ident: "F1",
Type: TypeText{},
Store: &CodecPlain{},
}},
}

Expand All @@ -23,64 +24,97 @@ 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) {
a := &Model{
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) {
a := &Model{
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 changes: 1 addition & 1 deletion server/pkg/dal/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 9ac6387

Please sign in to comment.