From b3057ea1add23f1bc9325acbaa3b1a85b3991b1c Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Fri, 9 Feb 2024 17:29:02 +0100 Subject: [PATCH] feat: preSetHook working --- inventory_test.go | 27 ++++++++++++-------- registry.go | 63 +++++++++++++++++++++++++++++++++++++---------- registry_test.go | 43 ++++++++++++++++++++++++++++++-- 3 files changed, 108 insertions(+), 25 deletions(-) diff --git a/inventory_test.go b/inventory_test.go index 9524629..c56978d 100644 --- a/inventory_test.go +++ b/inventory_test.go @@ -11,15 +11,18 @@ import ( ) var ( - personClassPath = "testdata/classes/person.yaml" - pizzaClassPath = "testdata/classes/pizza.yaml" - hansClassPath = "testdata/classes/people/hans.yaml" - johnClassPath = "testdata/classes/people/john.yaml" - peopleClassPath = "testdata/classes/people.yaml" - stripPrefix = data.NewPathFromOsPath("testdata/classes") + personClassPath = "testdata/classes/person.yaml" + pizzaClassPath = "testdata/classes/pizza.yaml" + commonFoodClassPath = "testdata/classes/food/common.yaml" + foodClassPath = "testdata/classes/food.yaml" + hansClassPath = "testdata/classes/people/hans.yaml" + johnClassPath = "testdata/classes/people/john.yaml" + janeClassPath = "testdata/classes/people/jane.yaml" + peopleClassPath = "testdata/classes/people.yaml" + stripPrefix = data.NewPathFromOsPath("testdata/classes") ) -func makeClasses(t *testing.T) (*skipper.Class, *skipper.Class, *skipper.Class, *skipper.Class) { +func makeClasses(t *testing.T) (*skipper.Class, *skipper.Class, *skipper.Class, *skipper.Class, *skipper.Class) { person, err := skipper.NewClass(personClassPath, codec.NewYamlCodec()) assert.NoError(t, err) assert.NotNil(t, person) @@ -36,11 +39,15 @@ func makeClasses(t *testing.T) (*skipper.Class, *skipper.Class, *skipper.Class, assert.NoError(t, err) assert.NotNil(t, person) - return person, pizza, hans, john + foodCommon, err := skipper.NewClass(commonFoodClassPath, codec.NewYamlCodec()) + assert.NoError(t, err) + assert.NotNil(t, foodCommon) + + return person, pizza, hans, john, foodCommon } func makeInventory(t *testing.T) *skipper.Inventory { - person, pizza, hans, john := makeClasses(t) + person, pizza, hans, john, _ := makeClasses(t) inventory, err := skipper.NewInventory() assert.NoError(t, err) @@ -72,7 +79,7 @@ func TestNewInventory(t *testing.T) { } func TestInventoryRegisterClass(t *testing.T) { - person, pizza, _, _ := makeClasses(t) + person, pizza, _, _, _ := makeClasses(t) inventory := makeInventory(t) // Test case: Class already exists in the default scope diff --git a/registry.go b/registry.go index 13f0f46..902aa20 100644 --- a/registry.go +++ b/registry.go @@ -74,7 +74,39 @@ func (reg *Registry) RegisterClass(classIdentifier data.Path, class *Class) erro return errs } - // All paths are unique, register everything. + err := class.SetPreSetHook(func(class Class, path data.Path, _ data.Value) error { + classIdentifierStr, err := reg.GetClassIdentifierByClassId(class.ID()) + if err != nil { + return fmt.Errorf("unable to locate identifier of the class, this should not happen") + } + + // strip the class name from the identifier and assemble registry-absolute path + // which would be created by this 'Set' call + classIdentifier := data.NewPath(classIdentifierStr) + registryPath := classIdentifier.StripSuffix(classIdentifier.LastSegment()).AppendPath(path) + + // if the path does already exist and is owned by a *different* class, then + // we need to prevent the Set call as it would introduce a duplicate path. + if classIdentifier, exists := reg.paths[registryPath.String()]; exists { + existingPathClass, _ := reg.classes[classIdentifier] + if existingPathClass.ID() != class.ID() { + return fmt.Errorf("path already owned by '%s': %w: %s", classIdentifier, ErrDuplicatePath, registryPath) + } + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to register pre-set hook in class: %w", err) + } + + err = class.SetPostSetHook(func(class Class, path data.Path, _ data.Value) error { + spew.Println("POST WRITE", path) + return nil + }) + if err != nil { + return fmt.Errorf("failed to register post-set hook in class: %w", err) + } + reg.classes[classIdentifier.String()] = class for _, classPath := range classPaths { reg.paths[classPath] = classIdentifier.String() @@ -83,27 +115,32 @@ func (reg *Registry) RegisterClass(classIdentifier data.Path, class *Class) erro return nil } -func (reg *Registry) GetClassByName(className string) (*Class, error) { - for _, class := range reg.classes { - if class.Name == className { - return class, nil - } - } - return nil, fmt.Errorf("%s: %w", className, ErrClassDoesNotExist) -} - -func (reg *Registry) GetClassByIdentifier(classIdentifier data.Path) (*Class, error) { - if classIdentifier.String() == "" { +func (reg *Registry) GetClassByIdentifier(classIdentifier string) (*Class, error) { + if classIdentifier == "" { return nil, ErrEmptyClassIdentifier } - class, exists := reg.classes[classIdentifier.String()] + class, exists := reg.classes[classIdentifier] if !exists { return nil, ErrNamespaceDoesNotExist } return class, nil } +func (reg *Registry) GetClassIdentifierByClassId(id string) (string, error) { + if id == "" { + return "", ErrEmptyClassId + } + + for identifier, class := range reg.classes { + if class.ID() == id { + return identifier, nil + } + } + + return "", fmt.Errorf("%w: %s", ErrClassDoesNotExist, id) +} + func (reg *Registry) ResolveClass(path data.Path) (*Class, error) { // If the path is 'foo.bar.baz' it could map to: // namespace 'foo' -> class 'bar' -> path 'baz' diff --git a/registry_test.go b/registry_test.go index fc239bc..72e3995 100644 --- a/registry_test.go +++ b/registry_test.go @@ -11,7 +11,7 @@ import ( ) func makeNewRegistry(t *testing.T) *skipper.Registry { - persons, pizza, hans, john := makeClasses(t) + persons, pizza, hans, john, foodCommon := makeClasses(t) registry := skipper.NewRegistry() // Test: register multiple classes in two namespaces @@ -23,6 +23,8 @@ func makeNewRegistry(t *testing.T) *skipper.Registry { assert.NoError(t, err) err = registry.RegisterClass(data.NewPathVar("people", john.Name), john) assert.NoError(t, err) + err = registry.RegisterClass(data.NewPathVar("food", foodCommon.Name), foodCommon) + assert.NoError(t, err) return registry } @@ -32,7 +34,7 @@ func TestNewRegistry(t *testing.T) { } func TestRegistryRegisterClass(t *testing.T) { - _, _, _, john := makeClasses(t) + _, _, _, john, _ := makeClasses(t) registry := makeNewRegistry(t) // Test: class already registered @@ -53,6 +55,43 @@ func TestRegistryRegisterClass(t *testing.T) { assert.NotNil(t, people) err = registry.RegisterClass(data.NewPath(people.Name), people) assert.ErrorIs(t, err, skipper.ErrDuplicatePath) + + // Test: class already has a pre-set hook + jane, err := skipper.NewClass(janeClassPath, codec.NewYamlCodec()) + assert.NoError(t, err) + assert.NotNil(t, jane) + jane.SetPreSetHook(func(class skipper.Class, path data.Path, value data.Value) error { return nil }) + err = registry.RegisterClass(data.NewPathVar("people", jane.Name), jane) + assert.ErrorIs(t, err, skipper.ErrCannotOverwriteHook) + + // Test: class already has a post-set hook + jane, err = skipper.NewClass(janeClassPath, codec.NewYamlCodec()) + assert.NoError(t, err) + assert.NotNil(t, jane) + jane.SetPostSetHook(func(class skipper.Class, path data.Path, value data.Value) error { return nil }) + err = registry.RegisterClass(data.NewPathVar("people", jane.Name), jane) + assert.ErrorIs(t, err, skipper.ErrCannotOverwriteHook) +} + +func TestRegistryPreSetHook(t *testing.T) { + registry := makeNewRegistry(t) + + // Register a 'food' class which can cause issues with the 'food.common' class + food, err := skipper.NewClass(foodClassPath, codec.NewYamlCodec()) + assert.NoError(t, err) + assert.NotNil(t, food) + err = registry.RegisterClass(data.NewPath(food.Name), food) + assert.NoError(t, err) + + // Test: Attempt to set existing path from different class + // The path 'food.common.should_be_tasty' is already registered by the 'food.common' class + // and must not be registered again by the 'food' class. + err = food.Set("food.common.should_be_tasty", false) + assert.ErrorIs(t, err, skipper.ErrDuplicatePath) + + // Test: Attempting to set a non-existing path must work + err = food.Set("food.common.eaten", false) + assert.NoError(t, err) } // func TestRegistryClassesInNamespace(t *testing.T) {