Repository: lf-edge/ekuiper
Pull request #2821 (2024-04-28 07:18:01)

refactor: provide schema when instantiating


Patch:
diff --git a/internal/converter/converter.go b/internal/converter/converter.go
index 02eccb4d..b7eff524 100644
--- a/internal/converter/converter.go
+++ b/internal/converter/converter.go
@@ -28,13 +28,16 @@ import (
 )
 
 func init() {
-	modules.RegisterConverter(message.FormatJson, func(_ string, _ string, _ string) (message.Converter, error) {
-		return json.GetConverter()
+	modules.RegisterConverter(message.FormatJson, func(_ string, _ string, _ string, schema map[string]*ast.JsonStreamField) (message.Converter, error) {
+		if schema == nil {
+			return json.GetConverter()
+		}
+		return json.NewFastJsonConverter(schema), nil
 	})
-	modules.RegisterConverter(message.FormatBinary, func(_ string, _ string, _ string) (message.Converter, error) {
+	modules.RegisterConverter(message.FormatBinary, func(_ string, _ string, _ string, _ map[string]*ast.JsonStreamField) (message.Converter, error) {
 		return binary.GetConverter()
 	})
-	modules.RegisterConverter(message.FormatDelimited, func(_ string, _ string, delimiter string) (message.Converter, error) {
+	modules.RegisterConverter(message.FormatDelimited, func(_ string, _ string, delimiter string, _ map[string]*ast.JsonStreamField) (message.Converter, error) {
 		return delimited.NewConverter(delimiter)
 	})
 }
@@ -50,16 +53,6 @@ func GetOrCreateConverter(options *ast.Options) (c message.Converter, err error)
 	if t == "" {
 		t = message.FormatJson
 	}
-	if t == message.FormatJson {
-		// it's unit test
-		if options.RuleID == "" || options.StreamName == "" {
-			return json.GetConverter()
-		}
-		if options.IsWildCard {
-			return json.NewFastJsonConverter(nil), nil
-		}
-		return json.NewFastJsonConverter(options.Schema), nil
-	}
 
 	schemaFile := ""
 	schemaName := options.SCHEMAID
@@ -70,8 +63,12 @@ func GetOrCreateConverter(options *ast.Options) (c message.Converter, err error)
 			schemaName = r[1]
 		}
 	}
+	schema := options.Schema
+	if options.IsWildCard {
+		schema = nil
+	}
 	if c, ok := modules.Converters[t]; ok {
-		return c(schemaFile, schemaName, options.DELIMITER)
+		return c(schemaFile, schemaName, options.DELIMITER, schema)
 	}
 	return nil, fmt.Errorf("format type %s not supported", t)
 }
diff --git a/internal/converter/ext_converter.go b/internal/converter/ext_converter.go
index 28564c27..eba3ab06 100644
--- a/internal/converter/ext_converter.go
+++ b/internal/converter/ext_converter.go
@@ -20,12 +20,13 @@ import (
 	"github.com/lf-edge/ekuiper/v2/internal/converter/protobuf"
 	"github.com/lf-edge/ekuiper/v2/internal/pkg/def"
 	"github.com/lf-edge/ekuiper/v2/internal/schema"
+	"github.com/lf-edge/ekuiper/v2/pkg/ast"
 	"github.com/lf-edge/ekuiper/v2/pkg/message"
 	"github.com/lf-edge/ekuiper/v2/pkg/modules"
 )
 
 func init() {
-	modules.RegisterConverter(message.FormatProtobuf, func(schemaFileName string, schemaMessageName string, _ string) (message.Converter, error) {
+	modules.RegisterConverter(message.FormatProtobuf, func(schemaFileName string, schemaMessageName string, _ string, _ map[string]*ast.JsonStreamField) (message.Converter, error) {
 		ffs, err := schema.GetSchemaFile(def.PROTOBUF, schemaFileName)
 		if err != nil {
 			return nil, err
diff --git a/internal/converter/ext_mock.go b/internal/converter/ext_mock.go
index dd6fcdc5..993dbae1 100644
--- a/internal/converter/ext_mock.go
+++ b/internal/converter/ext_mock.go
@@ -21,13 +21,14 @@ import (
 	"time"
 
 	"github.com/lf-edge/ekuiper/contract/v2/api"
+	"github.com/lf-edge/ekuiper/v2/pkg/ast"
 	"github.com/lf-edge/ekuiper/v2/pkg/message"
 	"github.com/lf-edge/ekuiper/v2/pkg/modules"
 	"github.com/lf-edge/ekuiper/v2/pkg/timex"
 )
 
 func init() {
-	modules.RegisterConverter("mock", func(_ string, _ string, _ string) (message.Converter, error) {
+	modules.RegisterConverter("mock", func(_ string, _ string, _ string, _ map[string]*ast.JsonStreamField) (message.Converter, error) {
 		return &MockConverter{}, nil
 	})
 }
diff --git a/internal/io/file/sink.go b/internal/io/file/sink.go
index 61cefe9f..2d4cb207 100644
--- a/internal/io/file/sink.go
+++ b/internal/io/file/sink.go
@@ -27,29 +27,33 @@ import (
 	"github.com/lf-edge/ekuiper/v2/pkg/cast"
 	"github.com/lf-edge/ekuiper/v2/pkg/message"
 	"github.com/lf-edge/ekuiper/v2/pkg/model"
+	"github.com/lf-edge/ekuiper/v2/pkg/modules"
 	"github.com/lf-edge/ekuiper/v2/pkg/timex"
 )
 
 type sinkConf struct {
-	RollingInterval    int64    `json:"rollingInterval"`
-	RollingCount       int      `json:"rollingCount"`
-	RollingNamePattern string   `json:"rollingNamePattern"` // where to add the timestamp to the file name
-	CheckInterval      int64    `json:"checkInterval"`
-	Path               string   `json:"path"` // support dynamic property, when rolling, make sure the path is updated
-	FileType           FileType `json:"fileType"`
-	HasHeader          bool     `json:"hasHeader"`
-	Delimiter          string   `json:"delimiter"`
-	Format             string   `json:"format"` // only use for validation; transformation is done in sink_node
-	Compression        string   `json:"compression"`
-	Encryption         string   `json:"encryption"`
-	Fields             []string `json:"fields"` // only use for extracting header for csv; transformation is done in sink_node
+	RollingInterval    int64          `json:"rollingInterval"`
+	RollingCount       int            `json:"rollingCount"`
+	RollingNamePattern string         `json:"rollingNamePattern"` // where to add the timestamp to the file name
+	RollingHook        string         `json:"rollingHook"`
+	RollingHookProps   map[string]any `json:"rollingHookProps"`
+	CheckInterval      int64          `json:"checkInterval"`
+	Path               string         `json:"path"` // support dynamic property, when rolling, make sure the path is updated
+	FileType           FileType       `json:"fileType"`
+	HasHeader          bool           `json:"hasHeader"`
+	Delimiter          string         `json:"delimiter"`
+	Format             string         `json:"format"` // only use for validation; transformation is done in sink_node
+	Compression        string         `json:"compression"`
+	Encryption         string         `json:"encryption"`
+	Fields             []string       `json:"fields"` // only use for extracting header for csv; transformation is done in sink_node
 }
 
 type fileSink struct {
 	c *sinkConf
 
-	mux sync.Mutex
-	fws map[string]*fileWriter
+	mux      sync.Mutex
+	fws      map[string]*fileWriter
+	rollHook modules.RollHook
 }
 
 func (m *fileSink) Provision(ctx api.StreamContext, props map[string]interface{}) error {
@@ -97,7 +101,17 @@ func (m *fileSink) Provision(ctx api.StreamContext, props map[string]interface{}
 	if _, ok := compressionTypes[c.Compression]; !ok && c.Compression != "" {
 		return fmt.Errorf("compression must be one of gzip, zstd")
 	}
-
+	if c.RollingHook != "" {
+		h, ok := modules.GetFileRollHook(c.RollingHook)
+		if !ok {
+			return fmt.Errorf("rolling hook %s is not registered", c.RollingHook)
+		}
+		err := h.Provision(ctx, c.RollingHookProps)
+		if err != nil {
+			return err
+		}
+		m.rollHook = h
+	}
 	m.c = c
 	m.fws = make(map[string]*fileWriter)
 	return nil
@@ -116,15 +130,11 @@ func (m *fileSink) Connect(ctx api.StreamContext) error {
 					m.mux.Lock()
 					for k, v := range m.fws {
 						if now.Sub(v.Start) > time.Duration(m.c.RollingInterval)*time.Millisecond {
-							ctx.GetLogger().Debugf("rolling file %s", k)
-							err := v.Close(ctx)
-							// TODO how to inform this error to the rule
+							err := m.roll(ctx, k, v)
+							// TODO how to deal with this error
 							if err != nil {
 								ctx.GetLogger().Errorf("file sink fails to close file %s with error %s.", k, err)
 							}
-							delete(m.fws, k)
-							// The file will be created when the next item comes
-							v.Written = false
 						}
 					}
 					m.mux.Unlock()
@@ -166,16 +176,9 @@ func (m *fileSink) Collect(ctx api.StreamContext, item []byte) error {
 	if m.c.RollingCount > 0 {
 		fw.Count++
 		if fw.Count >= m.c.RollingCount {
-			e = fw.Close(ctx)
-			if e != nil {
-				return e
-			}
-			delete(m.fws, fn)
-			fw.Count = 0
-			fw.Written = false
+			return m.roll(ctx, fn, fw)
 		}
 	}
-
 	return nil
 }
 
@@ -183,14 +186,40 @@ func (m *fileSink) Close(ctx api.StreamContext) error {
 	ctx.GetLogger().Infof("Closing file sink")
 	var errs []error
 	for k, v := range m.fws {
-		if e := v.Close(ctx); e != nil {
+		e := m.roll(ctx, k, v)
+		if e != nil {
 			ctx.GetLogger().Errorf("failed to close file %s: %v", k, e)
 			errs = append(errs, e)
 		}
 	}
+	if m.rollHook != nil {
+		e := m.rollHook.Close(ctx)
+		if e != nil {
+			errs = append(errs, e)
+		}
+	}
 	return errors.Join(errs...)
 }
 
+func (m *fileSink) roll(ctx api.StreamContext, k string, v *fileWriter) error {
+	ctx.GetLogger().Debugf("rolling file %s", k)
+	err := v.Close(ctx)
+	if err != nil {
+		return err
+	} else {
+		if m.rollHook != nil {
+			err = m.rollHook.RollDone(ctx, v.File.Name())
+			if err != nil {
+				return err
+			}
+		}
+	}
+	delete(m.fws, k)
+	// The file will be created when the next item comes
+	v.Written = false
+	return nil
+}
+
 // GetFws returns the file writer for the given file name, if the file writer does not exist, it will create one
 // The item is used to get the csv header if needed
 func (m *fileSink) GetFws(ctx api.StreamContext, fn string, item interface{}) (*fileWriter, error) {
diff --git a/pkg/message/artifacts.go b/pkg/message/artifacts.go
index bb9a127f..43c0c261 100644
--- a/pkg/message/artifacts.go
+++ b/pkg/message/artifacts.go
@@ -38,7 +38,7 @@ type Converter interface {
 
 // ConverterProvider The format, schema information are passed in by stream options
 // The columns information is defined in the source side, like file source
-type ConverterProvider func(schemaFileName string, SchemaMessageName string, delimiter string) (Converter, error)
+type ConverterProvider func(schemaFileName string, SchemaMessageName string, delimiter string, logicalSchema map[string]*ast.JsonStreamField) (Converter, error)
 
 type SchemaResetAbleConverter interface {
 	ResetSchema(schema map[string]*ast.JsonStreamField)
diff --git a/pkg/modules/file_roll_hook.go b/pkg/modules/file_roll_hook.go
new file mode 100644
index 00000000..1546a71f
--- /dev/null
+++ b/pkg/modules/file_roll_hook.go
@@ -0,0 +1,38 @@
+// Copyright 2024 EMQ Technologies Co., Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package modules
+
+import "github.com/lf-edge/ekuiper/contract/v2/api"
+
+type RollHook interface {
+	Provision(ctx api.StreamContext, props map[string]any) error
+	RollDone(ctx api.StreamContext, filePath string) error
+	api.Closable
+}
+
+type RollHookProvider func() RollHook
+
+var fileRollHooks = map[string]RollHookProvider{}
+
+func RegisterFileRollHook(name string, provider RollHookProvider) {
+	fileRollHooks[name] = provider
+}
+
+func GetFileRollHook(name string) (RollHook, bool) {
+	if p, ok := fileRollHooks[name]; ok {
+		return p(), ok
+	}
+	return nil, false
+}


Test patch:
diff --git a/internal/io/file/sink_rolling_hook_test.go b/internal/io/file/sink_rolling_hook_test.go
new file mode 100644
index 00000000..4b5889e0
--- /dev/null
+++ b/internal/io/file/sink_rolling_hook_test.go
@@ -0,0 +1,238 @@
+// Copyright 2024 EMQ Technologies Co., Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package file
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/lf-edge/ekuiper/contract/v2/api"
+	"github.com/lf-edge/ekuiper/v2/internal/conf"
+	"github.com/lf-edge/ekuiper/v2/internal/topo/topotest/mockclock"
+	"github.com/lf-edge/ekuiper/v2/pkg/message"
+	mockContext "github.com/lf-edge/ekuiper/v2/pkg/mock/context"
+	"github.com/lf-edge/ekuiper/v2/pkg/modules"
+)
+
+func TestRegister(t *testing.T) {
+	modules.RegisterFileRollHook("mock", func() modules.RollHook {
+		return &MockRollingHook{}
+	})
+	_, ok := modules.GetFileRollHook("mock")
+	assert.True(t, ok)
+	_, ok = modules.GetFileRollHook("none")
+	assert.False(t, ok)
+}
+
+func TestProvision(t *testing.T) {
+	modules.RegisterFileRollHook("mock", func() modules.RollHook {
+		return &MockRollingHook{}
+	})
+	tests := []struct {
+		name string
+		c    *sinkConf
+		err  string
+		p    map[string]interface{}
+	}{
+		{
+			name: "new props",
+			c: &sinkConf{
+				CheckInterval:      500,
+				Path:               "test",
+				FileType:           CSV_TYPE,
+				Format:             message.FormatDelimited,
+				Delimiter:          ",",
+				RollingCount:       1000000,
+				RollingNamePattern: "none",
+				RollingHook:        "mock",
+				RollingHookProps: map[string]any{
+					"custom": "test",
+				},
+			},
+			p: map[string]interface{}{
+				"checkInterval":      500,
+				"path":               "test",
+				"fileType":           "csv",
+				"format":             message.FormatDelimited,
+				"rollingNamePattern": "none",
+				"rollingHook":        "mock",
+				"rollingHookProps": map[string]any{
+					"custom": "test",
+				},
+			},
+		},
+		{ // invalid rolling hook
+			name: "invalid rolling hook",
+			p: map[string]interface{}{
+				"rollingInterval": 500,
+				"rollingCount":    0,
+				"rollingHook":     "invalid",
+			},
+			err: "rolling hook invalid is not registered",
+		},
+		{
+			name: "invalid rolling props",
+			err:  "missing props custom",
+			p: map[string]interface{}{
+				"rollingInterval": 500,
+				"rollingCount":    0,
+				"rollingHook":     "mock",
+				"rollingHookProps": map[string]any{
+					"delay": 25,
+				},
+			},
+		},
+	}
+	ctx := mockContext.NewMockContext("test1", "test")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := &fileSink{}
+			err := m.Provision(ctx, tt.p)
+			if tt.err != "" {
+				assert.Error(t, err)
+				assert.EqualError(t, err, tt.err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, m.c, tt.c)
+			}
+		})
+	}
+}
+
+func TestCollectRolling(t *testing.T) {
+	// Remove existing files
+	err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if filepath.Ext(path) == ".log" {
+			fmt.Println("Deleting file:", path)
+			return os.Remove(path)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf.IsTesting = true
+	tests := []struct {
+		name     string
+		fname    string
+		contents [2][]byte
+	}{
+		{
+			name:  "lines",
+			fname: "test_lines.log",
+			contents: [2][]byte{
+				[]byte("{\"key\":\"value0\",\"ts\":460}\n{\"key\":\"value1\",\"ts\":910}\n{\"key\":\"value2\",\"ts\":1360}"),
+				[]byte("{\"key\":\"value3\",\"ts\":1810}\n{\"key\":\"value4\",\"ts\":2260}"),
+			},
+		},
+	}
+
+	// Create a stream context for testing
+	ctx := mockContext.NewMockContext("rule", "testRolling")
+	hook := &MockRollingHook{}
+	modules.RegisterFileRollHook("mock", func() modules.RollHook {
+		return hook
+	})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a file sink with the temporary file path
+			sink := &fileSink{}
+			err := sink.Provision(ctx, map[string]interface{}{
+				"path":               tt.fname,
+				"fileType":           LINES_TYPE,
+				"rollingInterval":    1000,
+				"checkInterval":      500,
+				"rollingCount":       0,
+				"rollingNamePattern": "suffix",
+				"rollingHook":        "mock",
+				"rollingHookProps": map[string]any{
+					"custom": 123,
+				},
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+			mockclock.ResetClock(10)
+			err = sink.Connect(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			c := mockclock.GetMockClock()
+
+			for i := 0; i < 5; i++ {
+				c.Add(450 * time.Millisecond)
+				m := map[string]interface{}{"key": "value" + strconv.Itoa(i), "ts": c.Now().UnixMilli()}
+				b, err := json.Marshal(m)
+				assert.NoError(t, err)
+				if err := sink.Collect(ctx, b); err != nil {
+					t.Errorf("unexpected error: %s", err)
+				}
+			}
+			c.After(2000 * time.Millisecond)
+			if err = sink.Close(ctx); err != nil {
+				t.Errorf("unexpected close error: %s", err)
+			}
+			// Should write to 2 files
+			for i := 0; i < 2; i++ {
+				// Read the contents of the temporary file and check if they match the collected items
+				var fn string
+				fn = fmt.Sprintf("test_%s-%d.log", LINES_TYPE, 460+1350*i)
+
+				var contents []byte
+				contents, err := os.ReadFile(fn)
+				if err != nil {
+					t.Fatal(err)
+				}
+				assert.Equal(t, contents, tt.contents[i])
+			}
+			exp := []string{"test_lines-460.log", "test_lines-1810.log"}
+			assert.Equal(t, hook.result, exp)
+		})
+	}
+}
+
+type MockRollingHook struct {
+	result []string
+}
+
+func (m *MockRollingHook) Provision(ctx api.StreamContext, props map[string]any) error {
+	if _, ok := props["custom"]; !ok {
+		return fmt.Errorf("missing props custom")
+	}
+	return nil
+}
+
+func (m *MockRollingHook) RollDone(ctx api.StreamContext, filePath string) error {
+	ctx.GetLogger().Infof("roll done for %s", filePath)
+	m.result = append(m.result, filePath)
+	return nil
+}
+
+func (m *MockRollingHook) Close(ctx api.StreamContext) error {
+	ctx.GetLogger().Infof("roll hook close")
+	return nil
+}
+
+var _ modules.RollHook = &MockRollingHook{}