From 88a75784cebb220f5c30b053e0ec0797c61cbe9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toma=C5=BE=20Jerman?= <tomaz.jerman121@gmail.com>
Date: Thu, 14 Jul 2022 15:01:26 +0200
Subject: [PATCH] Tweak rv sanitizer->value expression interaction

When record value expression returns null the value gets omitted.
Tweak value sanitizer logic for more consistency.
---
 compose/service/values/expr.go      | 37 +++++++++++++++-----------
 compose/service/values/expr_test.go | 16 ++++++++++-
 compose/service/values/sanitizer.go | 41 ++++++++---------------------
 3 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/compose/service/values/expr.go b/compose/service/values/expr.go
index d218c5ab6..5abc58ae1 100644
--- a/compose/service/values/expr.go
+++ b/compose/service/values/expr.go
@@ -3,8 +3,10 @@ package values
 import (
 	"context"
 	"fmt"
+
 	"github.com/cortezaproject/corteza-server/compose/types"
 	"github.com/cortezaproject/corteza-server/pkg/expr"
+	"github.com/modern-go/reflect2"
 )
 
 func makeInvalidExprErr(field *types.ModuleField, expr string, err error) types.RecordValueError {
@@ -78,24 +80,27 @@ func Expression(ctx context.Context, m *types.Module, r *types.Record, old *type
 		}
 
 		var strings []string
-		if values, isSlice := tmp.([]interface{}); isSlice {
-			if !f.Multi {
-				rve.Push(makeValueExprIncompErr(f))
-				continue
-			}
+		// When the expression results in null (nil) delete that record value.
+		// The below Values.Replace already does it so we simply ignore strings
+		if !reflect2.IsNil(tmp) {
+			if values, isSlice := tmp.([]interface{}); isSlice {
+				if !f.Multi {
+					rve.Push(makeValueExprIncompErr(f))
+					continue
+				}
 
-			strings = make([]string, len(values))
-			for i, value := range values {
-				strings[i] = sanitize(f, value)
-			}
-		} else {
-			if f.Multi {
-				rve.Push(makeValueExprIncompErr(f))
-				continue
-			}
-
-			strings = []string{sanitize(f, tmp)}
+				strings = make([]string, len(values))
+				for i, value := range values {
+					strings[i] = sanitize(f, value)
+				}
+			} else {
+				if f.Multi {
+					rve.Push(makeValueExprIncompErr(f))
+					continue
+				}
 
+				strings = []string{sanitize(f, tmp)}
+			}
 		}
 
 		r.Values = r.Values.Replace(f.Name, strings...)
diff --git a/compose/service/values/expr_test.go b/compose/service/values/expr_test.go
index 5388426f7..1760b5e99 100644
--- a/compose/service/values/expr_test.go
+++ b/compose/service/values/expr_test.go
@@ -2,9 +2,10 @@ package values
 
 import (
 	"context"
+	"testing"
+
 	"github.com/cortezaproject/corteza-server/compose/types"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 func TestExpressions(t *testing.T) {
@@ -162,4 +163,17 @@ func TestExpressions(t *testing.T) {
 		req.Equal("b", new.Values.Get("f1", 1).Value)
 		req.Equal("c", new.Values.Get("f1", 2).Value)
 	})
+
+	t.Run("omit record value when expr returns null (nil)", func(t *testing.T) {
+		var (
+			req = require.New(t)
+			m   = makeModule("f1", "String", `null`)
+			r   = &types.Record{}
+			rve = &types.RecordValueErrorSet{}
+		)
+
+		Expression(ctx, m, r, nil, rve)
+		req.True(rve.IsValid())
+		req.Empty(r.Values)
+	})
 }
diff --git a/compose/service/values/sanitizer.go b/compose/service/values/sanitizer.go
index 5af1769e5..850c5b02b 100644
--- a/compose/service/values/sanitizer.go
+++ b/compose/service/values/sanitizer.go
@@ -9,6 +9,7 @@ import (
 	"github.com/cortezaproject/corteza-server/pkg/expr"
 	"github.com/cortezaproject/corteza-server/pkg/logger"
 	"github.com/cortezaproject/corteza-server/pkg/xss"
+	"github.com/spf13/cast"
 	"go.uber.org/zap"
 
 	"github.com/cortezaproject/corteza-server/compose/types"
@@ -117,34 +118,7 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco
 			}
 		}
 
-		// Per field type validators
-		switch kind {
-		case "bool":
-			v.Value = sBool(v.Value)
-
-		case "datetime":
-			v.Value = sDatetime(v.Value, f.Options.Bool("onlyDate"), f.Options.Bool("onlyTime"))
-
-		case "number":
-			v.Value = sNumber(v.Value, f.Options.Precision())
-
-		case "string":
-			v.Value = sString(v.Value)
-
-			// Uncomment when they become relevant for sanitization
-			//case "email":
-			//	v = s.sEmail(v, f, m)
-			//case "file":
-			//	v = s.sFile(v, f, m)
-			//case "record":
-			//	v = s.sRecord(v, f, m)
-			//case "select":
-			//	v = s.sSelect(v, f, m)
-			//case "url":
-			//	v = s.sUrl(v, f, m)
-			//case "user":
-			//	v = s.sUser(v, f, m)
-		}
+		v.Value = sanitize(f, v.Value)
 	}
 
 	return
@@ -284,8 +258,13 @@ func sNumber(num interface{}, p uint) string {
 	return str
 }
 
-func sString(str string) string {
-	return xss.RichText(str)
+func sString(str interface{}) string {
+	base, err := cast.ToStringE(str)
+	if err != nil {
+		return ""
+	}
+
+	return xss.RichText(base)
 }
 
 // sanitize casts value to field kind format
@@ -297,6 +276,8 @@ func sanitize(f *types.ModuleField, v interface{}) string {
 		v = sDatetime(v, f.Options.Bool("onlyDate"), f.Options.Bool("onlyTime"))
 	case "number":
 		v = sNumber(v, f.Options.Precision())
+	case "string":
+		v = sString(v)
 	}
 
 	return fmt.Sprintf("%v", v)