From 54c685e57c1ec84498b403cc739a8d665ef60ff6 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Tue, 27 Feb 2024 00:24:10 +0100 Subject: [PATCH] feat: minor cleanup; Can now register multiple hooks with the Class. --- class.go | 92 +++++++++++++++++++++++------------------------- registry.go | 13 ++----- registry_test.go | 16 --------- 3 files changed, 48 insertions(+), 73 deletions(-) diff --git a/class.go b/class.go index 76ae4ae..13db877 100644 --- a/class.go +++ b/class.go @@ -11,7 +11,6 @@ import ( var ( ErrEmptyFilePath = fmt.Errorf("file path cannot be empty") - ErrCannotOverwriteHook = fmt.Errorf("cannot overwrite existing hook") ErrEmptyClassId = fmt.Errorf("empty class id") ErrEmptyClassIdentifier = fmt.Errorf("class identifier cannot be empty") ErrInvalidClassIdentifier = fmt.Errorf("invalid class identifier") @@ -59,8 +58,6 @@ type DataContainer interface { DataSetterGetter DataWalker AbsolutePathMaker - AllPaths() []data.Path - LeafPaths() []data.Path } // Class defines the main file-data abstraction used by skipper. @@ -77,8 +74,8 @@ type Class struct { // The Class itself exposes all the functionality of the container anyway. container DataContainer // The class allows hooks to be registered to monitor each call to Set - preSetHook SetHookFunc - postSetHook SetHookFunc + preSetHooks []SetHookFunc + postSetHooks []SetHookFunc } // NewClass attempts to create a new class given a filesystem path and a codec. @@ -118,12 +115,12 @@ func NewClass(filePath string, codec Codec, identifier ClassIdentifier) (*Class, } return &Class{ - container: container, - Identifier: identifier, - FilePath: filePath, - Name: className, - preSetHook: nil, - postSetHook: nil, + container: container, + Identifier: identifier, + FilePath: filePath, + Name: className, + preSetHooks: nil, + postSetHooks: nil, }, nil } @@ -133,10 +130,12 @@ func (c Class) Get(path string) (data.Value, error) { return c.container.GetPath(data.NewPath(path)) } +// GetPath is the same as [Class#Get], but it accepts a [data.Path] func (c Class) GetPath(path data.Path) (data.Value, error) { return c.container.GetPath(path) } +// HasPath returns true if the given path exists within the Class. func (c Class) HasPath(path data.Path) bool { _, err := c.container.GetPath(path) if err != nil { @@ -160,11 +159,9 @@ func (c *Class) Set(path string, value interface{}) error { return err } - if c.preSetHook != nil { - err = c.preSetHook(*c, absPath, data.NewValue(value)) - if err != nil { - return err - } + err = c.callPreSetHooks(absPath, data.NewValue(value)) + if err != nil { + return err } err = c.container.SetPath(absPath, value) @@ -172,44 +169,53 @@ func (c *Class) Set(path string, value interface{}) error { return err } - if c.postSetHook != nil { - return c.postSetHook(*c, absPath, data.NewValue(value)) + err = c.callPostSetHooks(absPath, data.NewValue(value)) + if err != nil { + return err } return nil } +// SetPath the same as [Class#Set], but it accepts a [data.Path] instead. func (c *Class) SetPath(path data.Path, value interface{}) error { return c.container.SetPath(path, value) } -// SetPreSetHook sets the preSetHook of the class -// The function can only be called ONCE, after that it will always error. -// This is done to prevent circumventing an existing hook. -// TODO: Allow registration of multiple hooks -func (c *Class) SetPreSetHook(setHookFunc SetHookFunc) error { - if c.preSetHook != nil { - return ErrCannotOverwriteHook - } - c.preSetHook = setHookFunc - return nil +// RegisterPreSetHook registers a new pre-Set-hook. +// Hooks are called in the order they were registered. +func (c *Class) RegisterPreSetHook(preSetHookFunc SetHookFunc) { + c.preSetHooks = append(c.preSetHooks, preSetHookFunc) +} + +// RegisterPostSetHook registers a new post-Set-hook. +// Hooks are called in the order they were registered. +func (c *Class) RegisterPostSetHook(postSetHookFunc SetHookFunc) { + c.postSetHooks = append(c.postSetHooks, postSetHookFunc) } -// SetPostSetHook sets the postSetHook of the class -// The function can only be called ONCE, after that it will always error. -// This is done to prevent circumventing an existing hook. -// TODO: Allow registration of multiple hooks -func (c *Class) SetPostSetHook(setHookFunc SetHookFunc) error { - if c.postSetHook != nil { - return ErrCannotOverwriteHook +// callPreSetHooks will call all registered preSetHooks in the order they were registered. +// If one hook returns an error, the execution is halted and the error is returned immediately. +func (c *Class) callPreSetHooks(path data.Path, value data.Value) error { + for _, hook := range c.preSetHooks { + err := hook(*c, path, value) + if err != nil { + return err + } } - c.postSetHook = setHookFunc return nil } -// AllPaths returns every single path of the underlying container -func (c *Class) AllPaths() []data.Path { - return c.container.AllPaths() +// callPostSetHooks will call all registered postSetHooks in the order they were registered. +// If one hook returns an error, the execution is halted and the error is returned immediately. +func (c *Class) callPostSetHooks(path data.Path, value data.Value) error { + for _, hook := range c.postSetHooks { + err := hook(*c, path, value) + if err != nil { + return err + } + } + return nil } // Walk allows traversing the underlying container data. @@ -268,11 +274,3 @@ func ClassLoader(basePath string, files []string, codec Codec) ([]*Class, error) return classes, nil } - -func CreateClassIdentifier(filePaths []string, classFilePath string) data.Path { - commonPathPrefix := CommonPathPrefix(filePaths) - strippedPath := strings.Replace(classFilePath, commonPathPrefix, "", 1) - classIdentifier := data.NewPathFromOsPath(strippedPath) - - return classIdentifier -} diff --git a/registry.go b/registry.go index e5c665c..a04c6df 100644 --- a/registry.go +++ b/registry.go @@ -90,16 +90,9 @@ func (reg *Registry) RegisterClass(class *Class) error { return errs } - // inject hooks to monitor writes to the class which - // ensure that the registry stays valid - err := class.SetPreSetHook(reg.classPreSetHook()) - if err != nil { - return fmt.Errorf("failed to register pre-set hook in class: %w", err) - } - err = class.SetPostSetHook(reg.classPostSetHook()) - if err != nil { - return fmt.Errorf("failed to register post-set hook in class: %w", err) - } + // inject hooks to monitor writes to the class which ensure that the registry can stay valid + class.RegisterPreSetHook(reg.classPreSetHook()) + class.RegisterPostSetHook(reg.classPostSetHook()) // register class and all its paths for _, classPath := range classPaths { diff --git a/registry_test.go b/registry_test.go index 5e60c9e..59d3ab7 100644 --- a/registry_test.go +++ b/registry_test.go @@ -113,22 +113,6 @@ func TestRegistryRegisterClass(t *testing.T) { // Test: register a class which introduces a duplicate path err = registry.RegisterClass(peopleClass) assert.ErrorIs(t, err, ErrDuplicatePath) - - // Test: class already has a pre-set hook - jane, err := NewClass(janeClassPath, codec.NewYamlCodec(), data.NewPath("people.jane")) - assert.NoError(t, err) - assert.NotNil(t, jane) - jane.SetPreSetHook(func(class Class, path data.Path, value data.Value) error { return nil }) - err = registry.RegisterClass(jane) - assert.ErrorIs(t, err, ErrCannotOverwriteHook) - - // Test: class already has a post-set hook - jane, err = NewClass(janeClassPath, codec.NewYamlCodec(), data.NewPath("people.jane")) - assert.NoError(t, err) - assert.NotNil(t, jane) - jane.SetPostSetHook(func(class Class, path data.Path, value data.Value) error { return nil }) - err = registry.RegisterClass(jane) - assert.ErrorIs(t, err, ErrCannotOverwriteHook) } func TestRegistryPreSetHook(t *testing.T) {