Fix record-value-set encoding & cleanup before automation

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)

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)})
+}

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)
+			}
+		})
+	}
+}