diff --git a/compose/service/record.go b/compose/service/record.go index 1979f39f0..e8db45398 100644 --- a/compose/service/record.go +++ b/compose/service/record.go @@ -478,6 +478,14 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) { return rve } + // Before we pass values to record-before-update handling events + // values needs do be cleaned up + // + // Value merge inside procUpdate sets delete flag we need + // when changes are applied but we do not want deleted values + // to be sent to handler + upd.Values = upd.Values.GetClean() + // Scripts can (besides simple error value) return complex record value error set // that is passed back to the UI or any other API consumer // @@ -502,6 +510,8 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) { return } + // Final value cleanup + // These (clean) values are returned (and sent to after-update handler) upd.Values = upd.Values.GetClean() // At this point we can return the value @@ -518,7 +528,7 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) { // This logic is kept in a utility function - it's used in the beginning // of the update procedure and after results are back from the automation scripts // -// Both these points introduce external data that need to be checked fully in the same maner +// Both these points introduce external data that need to be checked fully in the same manner func (svc record) procUpdate(invokerID uint64, m *types.Module, upd *types.Record, old *types.Record) *types.RecordValueErrorSet { // Mark all values as updated (new) upd.Values.SetUpdatedFlag(true) diff --git a/compose/types/record.go b/compose/types/record.go index f9d6d4bda..97c9852ad 100644 --- a/compose/types/record.go +++ b/compose/types/record.go @@ -1,6 +1,7 @@ package types import ( + "encoding/json" "time" "github.com/cortezaproject/corteza-server/pkg/permissions" @@ -59,3 +60,19 @@ loop: func (r Record) PermissionResource() permissions.Resource { return ModulePermissionResource.AppendID(r.ModuleID) } + +// UnmarshalJSON for custom record deserialization +// +// Due to https://github.com/golang/go/issues/21092, we should manually reset the given record value set. +// If this is skipped there is a chance of data corruption; ie. wrong value is removed/edited +func (r *Record) UnmarshalJSON(data []byte) error { + // Reset value set + r.Values = nil + + // Deserialize to r (*Record) via auxRecord auxiliary record type alias + // + // This prevents inf. loop where json.Unmarshal directly on Record type + // calls this function + type auxRecord Record + return json.Unmarshal(data, &struct{ *auxRecord }{auxRecord: (*auxRecord)(r)}) +} diff --git a/compose/types/record_test.go b/compose/types/record_test.go new file mode 100644 index 000000000..97e8abbde --- /dev/null +++ b/compose/types/record_test.go @@ -0,0 +1,70 @@ +package types + +import ( + "encoding/json" + "github.com/davecgh/go-spew/spew" + "reflect" + "testing" + "time" +) + +func TestRecordUnmarshal(t *testing.T) { + tests := []struct { + name string + + // clean + preloaded *Record + + // update with + scripted *Record + + // final version + final *Record + }{ + { + "first step", + &Record{ + ID: 42, + Values: RecordValueSet{ + &RecordValue{Name: "foo", Value: "foo"}, + &RecordValue{Name: "bar", Value: "foo", Updated: true, DeletedAt: &time.Time{}}, + &RecordValue{Name: "baz", Value: "1"}, + }, + }, + &Record{ + ID: 82, + Values: RecordValueSet{ + &RecordValue{Name: "foo", Value: "foo"}, + &RecordValue{Name: "baz", Value: "1"}, + &RecordValue{Name: "baz", Value: "2"}, + &RecordValue{Name: "baz", Value: "3"}, + }, + }, + &Record{ + ID: 82, + Values: RecordValueSet{ + &RecordValue{Name: "foo", Value: "foo"}, + &RecordValue{Name: "baz", Value: "1"}, + &RecordValue{Name: "baz", Value: "2"}, + &RecordValue{Name: "baz", Value: "3"}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if j, err := json.Marshal(tt.scripted); err != nil { + t.Errorf("failed to marshal record: %v", err) + } else if err = json.Unmarshal(j, tt.preloaded); err != nil { + t.Errorf("failed to unmarshal record: %v", err) + } else { + spew.Dump(string(j)) + } + + if !reflect.DeepEqual(tt.preloaded, tt.final) { + t.Errorf("preloaded:\n%v\n\nfinal\n%v", tt.preloaded.Values, tt.final.Values) + } + }) + } +}