-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor gera.Map + fix KV override with empty value issue + unit tests #603
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
58e075e
[common] Upgrade hierarchical KV store to Go generics + unit tests
teo 7b06f13
[core] Use gera.Map[string, string] instead of gera.StringMap
teo d2cf281
[core] Remove gera.StringMap
teo 6b6c60d
[build] Add gera to test dirs in Makefile
teo c734fe6
[common] Improve hierarchical KV store test cases
teo 4a0cf62
[core] Use custom unmarshaler in gera.Map[string, string] instances
teo eccc6fe
[common] Fix override with empty value issue
teo e7191db
[core] Add comments on gera and defaults/vars/userVars mechanism
teo 67a8d37
[core] Test kvStoreUnmarshalYAMLWithTags and YAML→workflow unmarshal
teo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
/* | ||
* === This file is part of ALICE O² === | ||
* | ||
* Copyright 2020 CERN and copyright holders of ALICE O². | ||
* Copyright 2020-2024 CERN and copyright holders of ALICE O². | ||
* Author: Teo Mrnjavac <[email protected]> | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
|
@@ -26,82 +26,137 @@ | |
// | ||
// A gera.Map uses a map[string]interface{} as backing store, and it can wrap other gera.Map instances. | ||
// Values in child maps override any value provided by a gera.Map that's wrapped in the hierarchy. | ||
// | ||
// The name is reminiscent of hiera, as in hierarchical, but it was deemed desirable to avoid future confusion with the | ||
// Hiera KV store used by Puppet, a different product altogether, so instead of the ancient Greek root, "gera" comes | ||
// from the Italian root instead where "hi" becomes a soft "g". | ||
package gera | ||
|
||
import ( | ||
"sync" | ||
|
||
"dario.cat/mergo" | ||
) | ||
|
||
type Map interface { | ||
Wrap(m Map) Map | ||
type Map[K comparable, V any] interface { | ||
Wrap(m Map[K, V]) Map[K, V] | ||
IsHierarchyRoot() bool | ||
HierarchyContains(m Map) bool | ||
Unwrap() Map | ||
HierarchyContains(m Map[K, V]) bool | ||
Unwrap() Map[K, V] | ||
|
||
Has(key string) bool | ||
Has(key K) bool | ||
Len() int | ||
|
||
Get(key string) (interface{}, bool) | ||
Set(key string, value interface{}) bool | ||
Get(key K) (V, bool) | ||
Set(key K, value V) bool | ||
Del(key K) bool | ||
|
||
Flattened() (map[string]interface{}, error) | ||
WrappedAndFlattened(m Map) (map[string]interface{}, error) | ||
Flattened() (map[K]V, error) | ||
FlattenedParent() (map[K]V, error) | ||
WrappedAndFlattened(m Map[K, V]) (map[K]V, error) | ||
|
||
Raw() map[string]interface{} | ||
Raw() map[K]V | ||
Copy() Map[K, V] | ||
RawCopy() map[K]V | ||
} | ||
|
||
func MakeMap() Map { | ||
return &WrapMap{ | ||
theMap: make(map[string]interface{}), | ||
func MakeMap[K comparable, V any]() *WrapMap[K, V] { | ||
return &WrapMap[K, V]{ | ||
theMap: make(map[K]V), | ||
parent: nil, | ||
} | ||
} | ||
|
||
func MakeMapWithMap(fromMap map[string]interface{}) Map { | ||
myMap := &WrapMap{ | ||
func MakeMapWithMap[K comparable, V any](fromMap map[K]V) *WrapMap[K, V] { | ||
myMap := &WrapMap[K, V]{ | ||
theMap: fromMap, | ||
parent: nil, | ||
} | ||
return myMap | ||
} | ||
|
||
func MakeMapWithMapCopy(fromMap map[string]interface{}) Map { | ||
newBackingMap := make(map[string]interface{}) | ||
func FlattenStack[K comparable, V any](maps ...Map[K, V]) (flattened map[K]V, err error) { | ||
flattenedMap := MakeMap[K, V]() | ||
for _, oneMap := range maps { | ||
var localFlattened map[K]V | ||
localFlattened, err = oneMap.Flattened() | ||
if err != nil { | ||
return | ||
} | ||
flattenedMap = MakeMapWithMap(localFlattened).Wrap(flattenedMap).(*WrapMap[K, V]) | ||
} | ||
|
||
flattened, err = flattenedMap.Flattened() | ||
return | ||
} | ||
|
||
func MakeMapWithMapCopy[K comparable, V any](fromMap map[K]V) *WrapMap[K, V] { | ||
newBackingMap := make(map[K]V) | ||
for k, v := range fromMap { | ||
newBackingMap[k] = v | ||
} | ||
|
||
return MakeMapWithMap(newBackingMap) | ||
} | ||
|
||
type WrapMap struct { | ||
theMap map[string]interface{} | ||
parent Map | ||
type WrapMap[K comparable, V any] struct { | ||
theMap map[K]V | ||
parent Map[K, V] | ||
|
||
unmarshalYAML func(w Map[K, V], unmarshal func(interface{}) error) error | ||
marshalYAML func(w Map[K, V]) (interface{}, error) | ||
|
||
mu sync.RWMutex | ||
} | ||
|
||
func (w *WrapMap[K, V]) WithUnmarshalYAML(unmarshalYAML func(w Map[K, V], unmarshal func(interface{}) error) error) *WrapMap[K, V] { | ||
w.unmarshalYAML = unmarshalYAML | ||
return w | ||
} | ||
|
||
func (w *WrapMap[K, V]) WithMarshalYAML(marshalYAML func(w Map[K, V]) (interface{}, error)) *WrapMap[K, V] { | ||
w.marshalYAML = marshalYAML | ||
return w | ||
} | ||
|
||
func (w *WrapMap) UnmarshalYAML(unmarshal func(interface{}) error) error { | ||
m := make(map[string]interface{}) | ||
func (w *WrapMap[K, V]) UnmarshalYAML(unmarshal func(interface{}) error) error { | ||
if w.unmarshalYAML != nil { | ||
return w.unmarshalYAML(w, unmarshal) | ||
} | ||
|
||
m := make(map[K]V) | ||
err := unmarshal(&m) | ||
if err == nil { | ||
*w = WrapMap{ | ||
*w = WrapMap[K, V]{ | ||
theMap: m, | ||
parent: nil, | ||
} | ||
} | ||
return err | ||
} | ||
|
||
func (w *WrapMap) IsHierarchyRoot() bool { | ||
func (w *WrapMap[K, V]) MarshalYAML() (interface{}, error) { | ||
if w.marshalYAML != nil { | ||
return w.marshalYAML(w) | ||
} | ||
|
||
return w.theMap, nil | ||
} | ||
|
||
func (w *WrapMap[K, V]) IsHierarchyRoot() bool { | ||
if w == nil || w.parent != nil { | ||
return false | ||
} | ||
return true | ||
} | ||
|
||
func (w *WrapMap) HierarchyContains(m Map) bool { | ||
func (w *WrapMap[K, V]) HierarchyContains(m Map[K, V]) bool { | ||
if w == nil || w.parent == nil { | ||
return false | ||
} | ||
if w == m { | ||
return true | ||
} | ||
if w.parent == m { | ||
return true | ||
} | ||
|
@@ -110,7 +165,7 @@ func (w *WrapMap) HierarchyContains(m Map) bool { | |
|
||
// Wraps this map around the gera.Map m, which becomes the new parent. | ||
// Returns a pointer to the composite map (i.e. to itself in its new state). | ||
func (w *WrapMap) Wrap(m Map) Map { | ||
func (w *WrapMap[K, V]) Wrap(m Map[K, V]) Map[K, V] { | ||
if w == nil { | ||
return nil | ||
} | ||
|
@@ -120,7 +175,7 @@ func (w *WrapMap) Wrap(m Map) Map { | |
|
||
// Unwraps this map from its parent. | ||
// Returns a pointer to the former parent which was just unwrapped. | ||
func (w *WrapMap) Unwrap() Map { | ||
func (w *WrapMap[K, V]) Unwrap() Map[K, V] { | ||
if w == nil { | ||
return nil | ||
} | ||
|
@@ -129,33 +184,55 @@ func (w *WrapMap) Unwrap() Map { | |
return p | ||
} | ||
|
||
func (w *WrapMap) Get(key string) (value interface{}, ok bool) { | ||
func (w *WrapMap[K, V]) Get(key K) (value V, ok bool) { | ||
if w == nil || w.theMap == nil { | ||
return nil, false | ||
return value, false | ||
} | ||
|
||
w.mu.RLock() | ||
defer w.mu.RUnlock() | ||
|
||
if val, ok := w.theMap[key]; ok { | ||
return val, true | ||
} | ||
if w.parent != nil { | ||
return w.parent.Get(key) | ||
} | ||
return nil, false | ||
return value, false | ||
} | ||
|
||
func (w *WrapMap) Set(key string, value interface{}) (ok bool) { | ||
func (w *WrapMap[K, V]) Set(key K, value V) (ok bool) { | ||
if w == nil || w.theMap == nil { | ||
return false | ||
} | ||
|
||
w.mu.Lock() | ||
defer w.mu.Unlock() | ||
|
||
w.theMap[key] = value | ||
return true | ||
} | ||
|
||
func (w *WrapMap) Has(key string) bool { | ||
func (w *WrapMap[K, V]) Del(key K) (ok bool) { | ||
if w == nil || w.theMap == nil { | ||
return false | ||
} | ||
|
||
w.mu.Lock() | ||
defer w.mu.Unlock() | ||
|
||
if _, exists := w.theMap[key]; exists { | ||
delete(w.theMap, key) | ||
} | ||
return true | ||
} | ||
|
||
func (w *WrapMap[K, V]) Has(key K) bool { | ||
_, ok := w.Get(key) | ||
return ok | ||
} | ||
|
||
func (w *WrapMap) Len() int { | ||
func (w *WrapMap[K, V]) Len() int { | ||
if w == nil || w.theMap == nil { | ||
return 0 | ||
} | ||
|
@@ -166,53 +243,98 @@ func (w *WrapMap) Len() int { | |
return len(flattened) | ||
} | ||
|
||
func (w *WrapMap) Flattened() (map[string]interface{}, error) { | ||
func (w *WrapMap[K, V]) Flattened() (map[K]V, error) { | ||
if w == nil { | ||
return nil, nil | ||
} | ||
|
||
out := make(map[string]interface{}) | ||
w.mu.RLock() | ||
defer w.mu.RUnlock() | ||
|
||
thisMapCopy := make(map[K]V) | ||
for k, v := range w.theMap { | ||
out[k] = v | ||
thisMapCopy[k] = v | ||
} | ||
if w.parent == nil { | ||
return out, nil | ||
return thisMapCopy, nil | ||
} | ||
|
||
flattenedParent, err := w.parent.Flattened() | ||
if err != nil { | ||
return out, err | ||
return thisMapCopy, err | ||
} | ||
|
||
err = mergo.Merge(&flattenedParent, thisMapCopy, mergo.WithOverride) | ||
return flattenedParent, err | ||
} | ||
|
||
func (w *WrapMap[K, V]) FlattenedParent() (map[K]V, error) { | ||
if w == nil { | ||
return nil, nil | ||
} | ||
|
||
if w.parent == nil { | ||
return make(map[K]V), nil | ||
} | ||
|
||
err = mergo.Merge(&out, flattenedParent) | ||
return out, err | ||
return w.parent.Flattened() | ||
} | ||
|
||
func (w *WrapMap) WrappedAndFlattened(m Map) (map[string]interface{}, error) { | ||
func (w *WrapMap[K, V]) WrappedAndFlattened(m Map[K, V]) (map[K]V, error) { | ||
if w == nil { | ||
return nil, nil | ||
} | ||
|
||
out := make(map[string]interface{}) | ||
w.mu.RLock() | ||
|
||
thisMapCopy := make(map[K]V) | ||
for k, v := range w.theMap { | ||
out[k] = v | ||
thisMapCopy[k] = v | ||
} | ||
|
||
w.mu.RUnlock() | ||
|
||
if m == nil { | ||
return out, nil | ||
return thisMapCopy, nil | ||
} | ||
|
||
flattenedM, err := m.Flattened() | ||
if err != nil { | ||
return out, err | ||
return thisMapCopy, err | ||
} | ||
|
||
err = mergo.Merge(&out, flattenedM) | ||
return out, err | ||
err = mergo.Merge(&flattenedM, thisMapCopy, mergo.WithOverride) | ||
return flattenedM, err | ||
} | ||
|
||
func (w *WrapMap) Raw() map[string]interface{} { | ||
func (w *WrapMap[K, V]) Raw() map[K]V { // allows unmutexed access to map, can be unsafe! | ||
if w == nil { | ||
return nil | ||
} | ||
return w.theMap | ||
} | ||
|
||
func (w *WrapMap[K, V]) Copy() Map[K, V] { | ||
if w == nil { | ||
return nil | ||
} | ||
|
||
w.mu.RLock() | ||
defer w.mu.RUnlock() | ||
|
||
newMap := &WrapMap[K, V]{ | ||
theMap: make(map[K]V, len(w.theMap)), | ||
parent: w.parent, | ||
} | ||
for k, v := range w.theMap { | ||
newMap.theMap[k] = v | ||
} | ||
return newMap | ||
} | ||
|
||
func (w *WrapMap[K, V]) RawCopy() map[K]V { // always safe | ||
if w == nil { | ||
return nil | ||
} | ||
return w.Copy().Raw() | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason why use type arguments? I see only Map[string,string] used everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be safe to make the key a string forever, not so sure about the value. Basically
Map
already existed and I didn't want to preclude using non-string values in the future, if we so wish, mostly for the purposes of the template system, as well as usinggera
in contexts other than the template system. Since Go got generics a few years ago, the untemplatedgera.Map
became old style go, relying oninterface{}
, so the paths forward were either (1) to remove it and only usegera.StringMap
, or (2) to updategera.Map
for modern Go. I choose 2.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, thank you