From a853dacbdb4706040cc2f63107caaad0e4b6aba4 Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Tue, 17 Mar 2020 17:05:48 +0800 Subject: [PATCH 1/6] refreshable file datasource --- ext/datasource/datasource.go | 36 +++ ext/datasource/datasource_test.go | 64 +++++ ext/datasource/mock.go | 17 ++ .../refreshable_file/file_datasource.go | 119 ++++++++ .../refreshable_file/file_datasource_test.go | 261 ++++++++++++++++++ go.mod | 1 + go.sum | 1 + 7 files changed, 499 insertions(+) create mode 100644 ext/datasource/datasource_test.go create mode 100644 ext/datasource/mock.go create mode 100644 ext/datasource/refreshable_file/file_datasource.go create mode 100644 ext/datasource/refreshable_file/file_datasource_test.go diff --git a/ext/datasource/datasource.go b/ext/datasource/datasource.go index 79c3bcb44..cf32d52c3 100644 --- a/ext/datasource/datasource.go +++ b/ext/datasource/datasource.go @@ -22,3 +22,39 @@ type DataSource interface { // Close the data source. io.Closer } + +type Base struct { + handlers []PropertyHandler +} + +func (b *Base) Handlers() []PropertyHandler { + return b.handlers +} + +// return idx if existed, else return -1 +func (b *Base) indexOfHandler(h PropertyHandler) int { + for idx, handler := range b.handlers { + if handler == h { + return idx + } + } + return -1 +} + +func (b *Base) AddPropertyHandler(h PropertyHandler) { + if h == nil || b.indexOfHandler(h) >= 0 { + return + } + b.handlers = append(b.handlers, h) +} + +func (b *Base) RemovePropertyHandler(h PropertyHandler) { + if h == nil { + return + } + idx := b.indexOfHandler(h) + if idx < 0 { + return + } + b.handlers = append(b.handlers[:idx], b.handlers[idx+1:]...) +} diff --git a/ext/datasource/datasource_test.go b/ext/datasource/datasource_test.go new file mode 100644 index 000000000..2d80c0942 --- /dev/null +++ b/ext/datasource/datasource_test.go @@ -0,0 +1,64 @@ +package datasource + +import ( + "github.com/stretchr/testify/assert" + "reflect" + "testing" +) + +func TestBase_AddPropertyHandler(t *testing.T) { + t.Run("AddPropertyHandler_nil", func(t *testing.T) { + b := &Base{ + handlers: make([]PropertyHandler, 0), + } + b.AddPropertyHandler(nil) + assert.True(t, len(b.handlers) == 0, "Fail to execute the case TestBase_AddPropertyHandler.") + }) + + t.Run("AddPropertyHandler_Normal", func(t *testing.T) { + b := &Base{ + handlers: make([]PropertyHandler, 0), + } + h := &DefaultPropertyHandler{} + b.AddPropertyHandler(h) + assert.True(t, len(b.handlers) == 1 && reflect.DeepEqual(b.handlers[0], h), "Fail to execute the case TestBase_AddPropertyHandler.") + }) +} + +func TestBase_RemovePropertyHandler(t *testing.T) { + t.Run("TestBase_RemovePropertyHandler_nil", func(t *testing.T) { + b := &Base{ + handlers: make([]PropertyHandler, 0), + } + h1 := &DefaultPropertyHandler{} + b.handlers = append(b.handlers, h1) + b.RemovePropertyHandler(nil) + assert.True(t, len(b.handlers) == 1, "The case TestBase_RemovePropertyHandler execute failed.") + }) + + t.Run("TestBase_RemovePropertyHandler", func(t *testing.T) { + b := &Base{ + handlers: make([]PropertyHandler, 0), + } + h1 := &DefaultPropertyHandler{} + b.handlers = append(b.handlers, h1) + b.RemovePropertyHandler(h1) + assert.True(t, len(b.handlers) == 0, "The case TestBase_RemovePropertyHandler execute failed.") + }) +} + +func TestBase_indexOfHandler(t *testing.T) { + t.Run("TestBase_indexOfHandler", func(t *testing.T) { + b := &Base{ + handlers: make([]PropertyHandler, 0), + } + h1 := &DefaultPropertyHandler{} + b.handlers = append(b.handlers, h1) + h2 := &DefaultPropertyHandler{} + b.handlers = append(b.handlers, h2) + h3 := &DefaultPropertyHandler{} + b.handlers = append(b.handlers, h3) + + assert.True(t, b.indexOfHandler(h2) == 1, "Fail to execute the case TestBase_indexOfHandler.") + }) +} diff --git a/ext/datasource/mock.go b/ext/datasource/mock.go new file mode 100644 index 000000000..4c5eddf50 --- /dev/null +++ b/ext/datasource/mock.go @@ -0,0 +1,17 @@ +package datasource + +import "github.com/stretchr/testify/mock" + +type MockPropertyHandler struct { + mock.Mock +} + +func (m *MockPropertyHandler) isPropertyConsistent(src interface{}) bool { + args := m.Called(src) + return args.Bool(0) +} + +func (m *MockPropertyHandler) Handle(src []byte) error { + args := m.Called(src) + return args.Error(0) +} diff --git a/ext/datasource/refreshable_file/file_datasource.go b/ext/datasource/refreshable_file/file_datasource.go new file mode 100644 index 000000000..428444279 --- /dev/null +++ b/ext/datasource/refreshable_file/file_datasource.go @@ -0,0 +1,119 @@ +package refreshable_file + +import ( + "github.com/alibaba/sentinel-golang/ext/datasource" + "github.com/alibaba/sentinel-golang/logging" + "github.com/alibaba/sentinel-golang/util" + "github.com/fsnotify/fsnotify" + "github.com/pkg/errors" + "io/ioutil" + "os" +) + +var ( + logger = logging.GetDefaultLogger() +) + +type RefreshableFileDataSource struct { + datasource.Base + sourceFilePath string + isInitialized util.AtomicBool + closeChan chan struct{} + watcher *fsnotify.Watcher +} + +func NewFileDataSource(sourceFilePath string, handlers ...datasource.PropertyHandler) (*RefreshableFileDataSource, error) { + var ds = &RefreshableFileDataSource{ + sourceFilePath: sourceFilePath, + closeChan: make(chan struct{}), + } + for _, h := range handlers { + ds.AddPropertyHandler(h) + } + return ds, ds.Initialize() +} + +func (s *RefreshableFileDataSource) ReadSource() ([]byte, error) { + f, err := os.Open(s.sourceFilePath) + if err != nil { + return nil, errors.Errorf("Fail to open the property file, err: %+v.", errors.WithStack(err)) + } + defer f.Close() + + src, err := ioutil.ReadAll(f) + if err != nil { + return nil, errors.Errorf("Fail to read file, err: %+v.", errors.WithStack(err)) + } + return src, nil +} + +func (s *RefreshableFileDataSource) Initialize() error { + if !s.isInitialized.CompareAndSet(false, true) { + return nil + } + + err := s.doReadAndUpdate() + if err != nil { + logger.Errorf("Fail to execute doReadAndUpdate, err: %+v", err) + } + + w, err := fsnotify.NewWatcher() + if err != nil { + return errors.Errorf("Fail to new a watcher instance of fsnotify, err: %+v", err) + } + err = w.Add(s.sourceFilePath) + if err != nil { + return errors.Errorf("Fail add a watcher on file(%s), err: %+v", s.sourceFilePath, err) + } + s.watcher = w + + go util.RunWithRecover(func() { + defer s.watcher.Close() + for { + select { + case ev := <-s.watcher.Events: + if ev.Op&fsnotify.Write == fsnotify.Write { + err := s.doReadAndUpdate() + if err != nil { + logger.Errorf("Fail to execute doReadAndUpdate, err: %+v", err) + } + } + + if ev.Op&fsnotify.Remove == fsnotify.Remove || ev.Op&fsnotify.Rename == fsnotify.Rename { + logger.Errorf("The file source(%s) was removed or renamed.", s.sourceFilePath) + for _, h := range s.Handlers() { + err := h.Handle(nil) + if err != nil { + logger.Errorf("RefreshableFileDataSource fail to publish property, err: %+v.", errors.WithStack(err)) + } + } + } + case err := <-s.watcher.Errors: + logger.Errorf("Watch err on file(%s), err: %+v", s.sourceFilePath, err) + case <-s.closeChan: + return + } + } + }, logger) + return nil +} + +func (s *RefreshableFileDataSource) doReadAndUpdate() error { + src, err := s.ReadSource() + if err != nil { + return errors.Errorf("Fail to read source, err: %+v", err) + } + for _, h := range s.Handlers() { + err := h.Handle(src) + if err != nil { + return errors.Errorf("RefreshableFileDataSource fail to publish property, err: %+v.", err) + } + } + return nil +} + +func (s *RefreshableFileDataSource) Close() error { + s.closeChan <- struct{}{} + logger.Info("The RefreshableFileDataSource had been closed.") + return nil +} diff --git a/ext/datasource/refreshable_file/file_datasource_test.go b/ext/datasource/refreshable_file/file_datasource_test.go new file mode 100644 index 000000000..50a4e2b4b --- /dev/null +++ b/ext/datasource/refreshable_file/file_datasource_test.go @@ -0,0 +1,261 @@ +package refreshable_file + +import ( + "github.com/alibaba/sentinel-golang/ext/datasource" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + tmock "github.com/stretchr/testify/mock" + "io/ioutil" + "os" + "reflect" + "strings" + "testing" + "time" +) + +const ( + TestFlowRules = `[ + { + "id": 0, + "resource": "abc0", + "limitApp": "default", + "grade": 1, + "strategy": 0, + "controlBehavior": 0, + "refResource": "refDefault", + "warmUpPeriodSec": 10, + "maxQueueingTimeMs": 1000, + "clusterMode": false, + "clusterConfig": { + "thresholdType": 0 + } + }, + { + "id": 1, + "resource": "abc1", + "limitApp": "default", + "grade": 1, + "strategy": 0, + "controlBehavior": 0, + "refResource": "refDefault", + "warmUpPeriodSec": 10, + "maxQueueingTimeMs": 1000, + "clusterMode": false, + "clusterConfig": { + "thresholdType": 0 + } + }, + { + "id": 2, + "resource": "abc2", + "limitApp": "default", + "grade": 1, + "strategy": 0, + "controlBehavior": 0, + "refResource": "refDefault", + "warmUpPeriodSec": 10, + "maxQueueingTimeMs": 1000, + "clusterMode": false, + "clusterConfig": { + "thresholdType": 0 + } + } +]` + TestSystemRules = `[ + { + "id": 0, + "metricType": 0, + "adaptiveStrategy": 0 + }, + { + "id": 1, + "metricType": 0, + "adaptiveStrategy": 0 + }, + { + "id": 2, + "metricType": 0, + "adaptiveStrategy": 0 + } +]` + TestFlowRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRule.json" + TestSystemRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRule.json" +) + +func prepareFlowRulesTestFile() error { + content := []byte(TestFlowRules) + return ioutil.WriteFile(TestFlowRulesFile, content, os.ModePerm) +} + +func deleteFlowRulesTestFile() error { + return os.Remove(TestFlowRulesFile) +} + +func prepareSystemRulesTestFile() error { + content := []byte(TestSystemRules) + return ioutil.WriteFile(TestSystemRulesFile, content, os.ModePerm) +} + +func deleteSystemRulesTestFile() error { + return os.Remove(TestSystemRulesFile) +} + +func TestRefreshableFileDataSource_ReadSource(t *testing.T) { + t.Run("RefreshableFileDataSource_ReadSource_Nil", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + s := &RefreshableFileDataSource{ + sourceFilePath: TestSystemRulesFile + "NotExisted", + } + got, err := s.ReadSource() + assert.True(t, got == nil && err != nil && strings.Contains(err.Error(), "Fail to open the property file")) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) + + t.Run("RefreshableFileDataSource_ReadSource_Normal", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + s := &RefreshableFileDataSource{ + sourceFilePath: TestSystemRulesFile, + } + got, err := s.ReadSource() + if err != nil { + t.Errorf("Fail to execute ReadSource, err: %+v", err) + } + assert.True(t, reflect.DeepEqual(got, []byte(TestSystemRules))) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) +} + +func TestRefreshableFileDataSource_doReadAndUpdate(t *testing.T) { + t.Run("TestRefreshableFileDataSource_doReadAndUpdate_normal", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + s := &RefreshableFileDataSource{ + sourceFilePath: TestSystemRulesFile, + closeChan: make(chan struct{}), + } + mh1 := &datasource.MockPropertyHandler{} + mh1.On("Handle", tmock.Anything).Return(nil) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + s.AddPropertyHandler(mh1) + + err = s.doReadAndUpdate() + assert.True(t, err == nil, "Fail to doReadAndUpdate.") + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) + + t.Run("TestRefreshableFileDataSource_doReadAndUpdate_Handler_err", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + s := &RefreshableFileDataSource{ + sourceFilePath: TestSystemRulesFile, + closeChan: make(chan struct{}), + } + mh1 := &datasource.MockPropertyHandler{} + hErr := errors.New("Handle error") + mh1.On("Handle", tmock.Anything).Return(hErr) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + s.AddPropertyHandler(mh1) + + err = s.doReadAndUpdate() + assert.True(t, err != nil && strings.Contains(err.Error(), hErr.Error()), "Fail to doReadAndUpdate.") + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) +} + +func TestRefreshableFileDataSource_Close(t *testing.T) { + t.Run("TestRefreshableFileDataSource_Close", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + s := &RefreshableFileDataSource{ + sourceFilePath: TestSystemRulesFile, + closeChan: make(chan struct{}), + } + mh1 := &datasource.MockPropertyHandler{} + mh1.On("Handle", tmock.Anything).Return(nil) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + s.AddPropertyHandler(mh1) + + err = s.Initialize() + if err != nil { + t.Errorf("Fail to Initialize datasource, err: %+v", err) + } + + time.Sleep(1 * time.Second) + s.Close() + time.Sleep(1 * time.Second) + e := s.watcher.Add(TestSystemRulesFile) + assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) +} + +func TestNewFileDataSource_ALL_For_SystemRule(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + mh1 := &datasource.MockPropertyHandler{} + mh1.On("Handle", tmock.Anything).Return(nil) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + + ds, _ := NewFileDataSource(TestSystemRulesFile, mh1) + mh1.AssertNumberOfCalls(t, "Handle", 1) + + f, err := os.OpenFile(ds.sourceFilePath, os.O_RDWR|os.O_APPEND|os.O_SYNC, os.ModePerm) + if err != nil { + t.Errorf("Fail to open the property file, err: %+v.", err) + } + defer f.Close() + + f.WriteString("\n" + TestSystemRules) + f.Sync() + time.Sleep(3 * time.Second) + mh1.AssertNumberOfCalls(t, "Handle", 2) + + ds.Close() + e := ds.watcher.Add(TestSystemRulesFile) + assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } +} diff --git a/go.mod b/go.mod index 16f4f05d6..fe20eb305 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.13 require ( github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d // indirect github.com/apache/dubbo-go v0.1.2-0.20200224151332-dd1a3c24d656 + github.com/fsnotify/fsnotify v1.4.7 github.com/gin-gonic/gin v1.5.0 github.com/go-ole/go-ole v1.2.4 // indirect github.com/pkg/errors v0.8.1 diff --git a/go.sum b/go.sum index c996c6b4f..dc5f5f24f 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,7 @@ github.com/envoyproxy/protoc-gen-validate v0.0.14/go.mod h1:iSmxcyjqTsJpI2R4NaDN github.com/fastly/go-utils v0.0.0-20180712184237-d95a45783239/go.mod h1:Gdwt2ce0yfBxPvZrHkprdPPTTS3N5rwmLE8T22KBXlw= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/structs v0.0.0-20180123065059-ebf56d35bba7/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= +github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE= From 3c167e312b33eb852701f6bd5f0d1294f0da6964 Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Tue, 17 Mar 2020 17:26:12 +0800 Subject: [PATCH 2/6] refine code --- ext/datasource/refreshable_file/file_datasource.go | 2 +- ext/datasource/refreshable_file/file_datasource_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/datasource/refreshable_file/file_datasource.go b/ext/datasource/refreshable_file/file_datasource.go index 428444279..b52de5e66 100644 --- a/ext/datasource/refreshable_file/file_datasource.go +++ b/ext/datasource/refreshable_file/file_datasource.go @@ -114,6 +114,6 @@ func (s *RefreshableFileDataSource) doReadAndUpdate() error { func (s *RefreshableFileDataSource) Close() error { s.closeChan <- struct{}{} - logger.Info("The RefreshableFileDataSource had been closed.") + logger.Infof("The RefreshableFileDataSource for [%s] had been closed.", s.sourceFilePath) return nil } diff --git a/ext/datasource/refreshable_file/file_datasource_test.go b/ext/datasource/refreshable_file/file_datasource_test.go index 50a4e2b4b..e07d3c24a 100644 --- a/ext/datasource/refreshable_file/file_datasource_test.go +++ b/ext/datasource/refreshable_file/file_datasource_test.go @@ -78,8 +78,8 @@ const ( "adaptiveStrategy": 0 } ]` - TestFlowRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRule.json" - TestSystemRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRule.json" + TestFlowRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRules.json" + TestSystemRulesFile = "../../../tests/testdata/extension/refreshable_file/SystemRules.json" ) func prepareFlowRulesTestFile() error { From c2f98310a67d36a228c352fc7cb44ca992a6703f Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Tue, 17 Mar 2020 17:39:17 +0800 Subject: [PATCH 3/6] remove unused code --- .../refreshable_file/file_datasource_test.go | 61 ++----------------- 1 file changed, 4 insertions(+), 57 deletions(-) diff --git a/ext/datasource/refreshable_file/file_datasource_test.go b/ext/datasource/refreshable_file/file_datasource_test.go index e07d3c24a..b6e78cf4a 100644 --- a/ext/datasource/refreshable_file/file_datasource_test.go +++ b/ext/datasource/refreshable_file/file_datasource_test.go @@ -14,53 +14,6 @@ import ( ) const ( - TestFlowRules = `[ - { - "id": 0, - "resource": "abc0", - "limitApp": "default", - "grade": 1, - "strategy": 0, - "controlBehavior": 0, - "refResource": "refDefault", - "warmUpPeriodSec": 10, - "maxQueueingTimeMs": 1000, - "clusterMode": false, - "clusterConfig": { - "thresholdType": 0 - } - }, - { - "id": 1, - "resource": "abc1", - "limitApp": "default", - "grade": 1, - "strategy": 0, - "controlBehavior": 0, - "refResource": "refDefault", - "warmUpPeriodSec": 10, - "maxQueueingTimeMs": 1000, - "clusterMode": false, - "clusterConfig": { - "thresholdType": 0 - } - }, - { - "id": 2, - "resource": "abc2", - "limitApp": "default", - "grade": 1, - "strategy": 0, - "controlBehavior": 0, - "refResource": "refDefault", - "warmUpPeriodSec": 10, - "maxQueueingTimeMs": 1000, - "clusterMode": false, - "clusterConfig": { - "thresholdType": 0 - } - } -]` TestSystemRules = `[ { "id": 0, @@ -78,18 +31,12 @@ const ( "adaptiveStrategy": 0 } ]` - TestFlowRulesFile = "../../../tests/testdata/extension/refreshable_file/FlowRules.json" - TestSystemRulesFile = "../../../tests/testdata/extension/refreshable_file/SystemRules.json" ) -func prepareFlowRulesTestFile() error { - content := []byte(TestFlowRules) - return ioutil.WriteFile(TestFlowRulesFile, content, os.ModePerm) -} - -func deleteFlowRulesTestFile() error { - return os.Remove(TestFlowRulesFile) -} +var ( + TestSystemRulesDir = "./" + TestSystemRulesFile = TestSystemRulesDir + "SystemRules.json" +) func prepareSystemRulesTestFile() error { content := []byte(TestSystemRules) From c2f8bffaa87f680259adb2bc939fcb2b5b754d80 Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Tue, 17 Mar 2020 18:06:02 +0800 Subject: [PATCH 4/6] enhance ut --- ext/datasource/refreshable_file/file_datasource_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/datasource/refreshable_file/file_datasource_test.go b/ext/datasource/refreshable_file/file_datasource_test.go index b6e78cf4a..f5c4b4c82 100644 --- a/ext/datasource/refreshable_file/file_datasource_test.go +++ b/ext/datasource/refreshable_file/file_datasource_test.go @@ -198,6 +198,7 @@ func TestNewFileDataSource_ALL_For_SystemRule(t *testing.T) { mh1.AssertNumberOfCalls(t, "Handle", 2) ds.Close() + time.Sleep(1 * time.Second) e := ds.watcher.Add(TestSystemRulesFile) assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) From 5bffd8fa948037b28d0a89e77de7cf44e368074a Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Wed, 18 Mar 2020 20:26:50 +0800 Subject: [PATCH 5/6] polish code and add remove event ut --- .../refreshable_file.go} | 18 +-- .../refreshable_file_test.go} | 104 ++++++++++++------ ext/datasource/property_test.go | 2 +- 3 files changed, 81 insertions(+), 43 deletions(-) rename ext/datasource/{refreshable_file/file_datasource.go => file/refreshable_file.go} (83%) rename ext/datasource/{refreshable_file/file_datasource_test.go => file/refreshable_file_test.go} (68%) diff --git a/ext/datasource/refreshable_file/file_datasource.go b/ext/datasource/file/refreshable_file.go similarity index 83% rename from ext/datasource/refreshable_file/file_datasource.go rename to ext/datasource/file/refreshable_file.go index b52de5e66..64a0194c4 100644 --- a/ext/datasource/refreshable_file/file_datasource.go +++ b/ext/datasource/file/refreshable_file.go @@ -1,4 +1,4 @@ -package refreshable_file +package file import ( "github.com/alibaba/sentinel-golang/ext/datasource" @@ -22,7 +22,7 @@ type RefreshableFileDataSource struct { watcher *fsnotify.Watcher } -func NewFileDataSource(sourceFilePath string, handlers ...datasource.PropertyHandler) (*RefreshableFileDataSource, error) { +func NewFileDataSource(sourceFilePath string, handlers ...datasource.PropertyHandler) *RefreshableFileDataSource { var ds = &RefreshableFileDataSource{ sourceFilePath: sourceFilePath, closeChan: make(chan struct{}), @@ -30,19 +30,19 @@ func NewFileDataSource(sourceFilePath string, handlers ...datasource.PropertyHan for _, h := range handlers { ds.AddPropertyHandler(h) } - return ds, ds.Initialize() + return ds } func (s *RefreshableFileDataSource) ReadSource() ([]byte, error) { f, err := os.Open(s.sourceFilePath) if err != nil { - return nil, errors.Errorf("Fail to open the property file, err: %+v.", errors.WithStack(err)) + return nil, errors.Errorf("RefreshableFileDataSource fail to open the property file, err: %+v.", err) } defer f.Close() src, err := ioutil.ReadAll(f) if err != nil { - return nil, errors.Errorf("Fail to read file, err: %+v.", errors.WithStack(err)) + return nil, errors.Errorf("RefreshableFileDataSource fail to read file, err: %+v.", err) } return src, nil } @@ -63,7 +63,7 @@ func (s *RefreshableFileDataSource) Initialize() error { } err = w.Add(s.sourceFilePath) if err != nil { - return errors.Errorf("Fail add a watcher on file(%s), err: %+v", s.sourceFilePath, err) + return errors.Errorf("Fail add a watcher on file[%s], err: %+v", s.sourceFilePath, err) } s.watcher = w @@ -80,16 +80,16 @@ func (s *RefreshableFileDataSource) Initialize() error { } if ev.Op&fsnotify.Remove == fsnotify.Remove || ev.Op&fsnotify.Rename == fsnotify.Rename { - logger.Errorf("The file source(%s) was removed or renamed.", s.sourceFilePath) + logger.Errorf("The file source[%s] was removed or renamed.", s.sourceFilePath) for _, h := range s.Handlers() { err := h.Handle(nil) if err != nil { - logger.Errorf("RefreshableFileDataSource fail to publish property, err: %+v.", errors.WithStack(err)) + logger.Errorf("RefreshableFileDataSource fail to publish property, err: %+v.", err) } } } case err := <-s.watcher.Errors: - logger.Errorf("Watch err on file(%s), err: %+v", s.sourceFilePath, err) + logger.Errorf("Watch err on file[%s], err: %+v", s.sourceFilePath, err) case <-s.closeChan: return } diff --git a/ext/datasource/refreshable_file/file_datasource_test.go b/ext/datasource/file/refreshable_file_test.go similarity index 68% rename from ext/datasource/refreshable_file/file_datasource_test.go rename to ext/datasource/file/refreshable_file_test.go index f5c4b4c82..2627a7a1c 100644 --- a/ext/datasource/refreshable_file/file_datasource_test.go +++ b/ext/datasource/file/refreshable_file_test.go @@ -1,4 +1,4 @@ -package refreshable_file +package file import ( "github.com/alibaba/sentinel-golang/ext/datasource" @@ -174,36 +174,74 @@ func TestRefreshableFileDataSource_Close(t *testing.T) { } func TestNewFileDataSource_ALL_For_SystemRule(t *testing.T) { - err := prepareSystemRulesTestFile() - if err != nil { - t.Errorf("Fail to prepare test file, err: %+v", err) - } - - mh1 := &datasource.MockPropertyHandler{} - mh1.On("Handle", tmock.Anything).Return(nil) - mh1.On("isPropertyConsistent", tmock.Anything).Return(false) - - ds, _ := NewFileDataSource(TestSystemRulesFile, mh1) - mh1.AssertNumberOfCalls(t, "Handle", 1) - - f, err := os.OpenFile(ds.sourceFilePath, os.O_RDWR|os.O_APPEND|os.O_SYNC, os.ModePerm) - if err != nil { - t.Errorf("Fail to open the property file, err: %+v.", err) - } - defer f.Close() - - f.WriteString("\n" + TestSystemRules) - f.Sync() - time.Sleep(3 * time.Second) - mh1.AssertNumberOfCalls(t, "Handle", 2) - - ds.Close() - time.Sleep(1 * time.Second) - e := ds.watcher.Add(TestSystemRulesFile) - assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) - - err = deleteSystemRulesTestFile() - if err != nil { - t.Errorf("Fail to delete test file, err: %+v", err) - } + t.Run("TestNewFileDataSource_ALL_For_SystemRule_Write_Event", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + mh1 := &datasource.MockPropertyHandler{} + mh1.On("Handle", tmock.Anything).Return(nil) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + + ds := NewFileDataSource(TestSystemRulesFile, mh1) + err = ds.Initialize() + if err != nil { + t.Errorf("Fail to initialize the file data source, err: %+v", err) + } + mh1.AssertNumberOfCalls(t, "Handle", 1) + + f, err := os.OpenFile(ds.sourceFilePath, os.O_RDWR|os.O_APPEND|os.O_SYNC, os.ModePerm) + if err != nil { + t.Errorf("Fail to open the property file, err: %+v.", err) + } + defer f.Close() + + f.WriteString("\n" + TestSystemRules) + f.Sync() + time.Sleep(3 * time.Second) + mh1.AssertNumberOfCalls(t, "Handle", 2) + + ds.Close() + time.Sleep(1 * time.Second) + e := ds.watcher.Add(TestSystemRulesFile) + assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + }) + + t.Run("TestNewFileDataSource_ALL_For_SystemRule_Remove_Event", func(t *testing.T) { + err := prepareSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to prepare test file, err: %+v", err) + } + + mh1 := &datasource.MockPropertyHandler{} + mh1.On("Handle", tmock.Anything).Return(nil) + mh1.On("isPropertyConsistent", tmock.Anything).Return(false) + + ds := NewFileDataSource(TestSystemRulesFile, mh1) + err = ds.Initialize() + if err != nil { + t.Errorf("Fail to initialize the file data source, err: %+v", err) + } + mh1.AssertNumberOfCalls(t, "Handle", 1) + + err = deleteSystemRulesTestFile() + if err != nil { + t.Errorf("Fail to delete test file, err: %+v", err) + } + + time.Sleep(3 * time.Second) + mh1.AssertNumberOfCalls(t, "Handle", 2) + + ds.Close() + time.Sleep(1 * time.Second) + e := ds.watcher.Add(TestSystemRulesFile) + assert.True(t, e != nil && strings.Contains(e.Error(), "closed")) + }) + } diff --git a/ext/datasource/property_test.go b/ext/datasource/property_test.go index 89ab519f8..9bafc348e 100644 --- a/ext/datasource/property_test.go +++ b/ext/datasource/property_test.go @@ -2,8 +2,8 @@ package datasource import ( "encoding/json" - "errors" "github.com/alibaba/sentinel-golang/core/system" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "io/ioutil" "testing" From f4dfab2e33c37b7f56027c4ab1b40e5d87d6633a Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Wed, 18 Mar 2020 20:34:46 +0800 Subject: [PATCH 6/6] fix ut --- ext/datasource/file/refreshable_file_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/datasource/file/refreshable_file_test.go b/ext/datasource/file/refreshable_file_test.go index 2627a7a1c..0b76cbe20 100644 --- a/ext/datasource/file/refreshable_file_test.go +++ b/ext/datasource/file/refreshable_file_test.go @@ -58,7 +58,7 @@ func TestRefreshableFileDataSource_ReadSource(t *testing.T) { sourceFilePath: TestSystemRulesFile + "NotExisted", } got, err := s.ReadSource() - assert.True(t, got == nil && err != nil && strings.Contains(err.Error(), "Fail to open the property file")) + assert.True(t, got == nil && err != nil && strings.Contains(err.Error(), "RefreshableFileDataSource fail to open the property file")) err = deleteSystemRulesTestFile() if err != nil {