fix(store): pointer-based stores, concurrency and robustness fixes

august.go

@@ -23,7 +23,7 @@ type August struct {
 	mu             sync.RWMutex
 	storeRegistry  map[string]reflect.Type // A map registrying the store types
 	config         AugustConfig            // August configuration
-	storage        map[string]AugustStore  // A map of all the stores
+	storage        map[string]*AugustStore // A map of all the stores
 	eventFunc      AugustEventFunc         // A function to call when an event happens
 	systemModCache []string                // Every time we modify a file, we add info about it so that FSNotify doesn't trigger on it
 }
@@ -62,7 +62,7 @@ func Init() *August {
 		Format:     "json",
 		FSNotify:   true,
 	}
-	storage := make(map[string]AugustStore)
+	storage := make(map[string]*AugustStore)
 
 	a := &August{
 		storeRegistry:  stores,
@@ -81,18 +81,45 @@ func (a *August) Verbose() {
 }
 
 // Set a config option.
-func (a *August) Config(k AugustConfigOption, v interface{}) {
+func (a *August) Config(k AugustConfigOption, v interface{}) error {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	log.Printf("Setting config: %s to %v", k, v)
 
-	if k == Config_Verbose && v.(bool) {
-		// set verbose mode if we configure that
-		a.Verbose()
+	switch k {
+	case Config_StorageDir:
+		val, ok := v.(string)
+		if !ok {
+			return fmt.Errorf("config option %s requires a string value", k)
+		}
+		a.config.StorageDir = val
+	case Config_Verbose:
+		val, ok := v.(bool)
+		if !ok {
+			return fmt.Errorf("config option %s requires a bool value", k)
+		}
+		a.config.Verbose = val
+		if val {
+			a.Verbose()
+		}
+	case Config_Format:
+		val, ok := v.(string)
+		if !ok {
+			return fmt.Errorf("config option %s requires a string value", k)
+		}
+		a.config.Format = val
+	case Config_FSNotify:
+		val, ok := v.(bool)
+		if !ok {
+			return fmt.Errorf("config option %s requires a bool value", k)
+		}
+		a.config.FSNotify = val
+	default:
+		return fmt.Errorf("unknown config option: %s", k)
 	}
 
-	reflect.ValueOf(&a.config).Elem().FieldByName(k.String()).Set(reflect.ValueOf(v))
 	log.Printf("Config: %+v", a.config)
+	return nil
 }
 
 func (a *August) SetEventFunc(f AugustEventFunc) {
@@ -129,13 +156,12 @@ func (a *August) Unmarshal(input []byte, output interface{}) error {
 
 // Get a store by name.
 func (a *August) GetStore(name string) (*AugustStore, error) {
-	a.mu.Lock()
-	defer a.mu.Unlock()
+	a.mu.RLock()
+	defer a.mu.RUnlock()
 	if store, ok := a.storage[name]; ok {
-		return &store, nil
-	} else {
-		return &AugustStore{}, fmt.Errorf("data store %s not found", name)
+		return store, nil
 	}
+	return nil, fmt.Errorf("data store %s not found", name)
 }
 
 // Register a store.
@@ -145,7 +171,7 @@ func (a *August) Register(name string, store interface{}) {
 	log.Printf("Registering store: %s of type %T", name, store)
 
 	a.storeRegistry[name] = reflect.TypeOf(store)
-	a.storage[name] = AugustStore{
+	a.storage[name] = &AugustStore{
 		name:   name,
 		parent: a,
 		data:   make(map[string]AugustStoreDataset),
@@ -154,29 +180,38 @@ func (a *August) Register(name string, store interface{}) {
 
 // Populate registry is used during initial startup to load any existing data.
 func (a *August) populateRegistry(name string) error {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	if _, ok := a.storeRegistry[name]; !ok {
+	a.mu.RLock()
+	_, ok := a.storeRegistry[name]
+	store := a.storage[name]
+	ext := "." + a.config.Format
+	storageDir := a.config.StorageDir
+	a.mu.RUnlock()
+
+	if !ok {
 		return fmt.Errorf("store %s does not exists", name)
 	}
 
 	// check the directory for files and load them
-	dir, err := os.ReadDir(a.config.StorageDir + "/" + name)
+	dir, err := os.ReadDir(storageDir + "/" + name)
 	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
 		return err
 	}
 
-	store := a.storage[name]
-
 	for _, file := range dir {
-		// skip invalid files
-		if file.IsDir() || file.Type().IsRegular() && file.Name()[len(file.Name())-len(a.config.Format):] != a.config.Format {
+		// skip directories and files that don't have the expected extension
+		if file.IsDir() {
+			continue
+		}
+		fname := file.Name()
+		if !strings.HasSuffix(fname, ext) {
 			continue
 		}
 
-		id := file.Name()[:len(file.Name())-len(a.config.Format)-1]
-		log.Printf("Loading file: %s for registry %s as ID %s", file.Name(), name, id)
-		// read the file
+		id := fname[:len(fname)-len(ext)]
+		log.Printf("Loading file: %s for registry %s as ID %s", fname, name, id)
 		store.loadFromFile(id)
 	}
 	return nil
@@ -311,6 +346,8 @@ func (a *August) Run() error {
 // detected, and returns true + deletes the entry if it does.
 func (a *August) handleModCacheSkip(method, name, id string) bool {
 	cacheName := fmt.Sprintf("%s::%s::%s", method, name, id)
+	a.mu.Lock()
+	defer a.mu.Unlock()
 	for i, v := range a.systemModCache {
 		if v == cacheName {
 			log.Printf("[FS Notify] Found %s, skipping FS modify actions", cacheName)

augustStore.go

@@ -136,6 +136,8 @@ func (as *AugustStore) GetIds() []string {
 
 // GetAll returns all values in the store.
 func (as *AugustStore) GetAll() (map[string]interface{}, error) {
+	as.mu.RLock()
+	defer as.mu.RUnlock()
 
 	if len((*as).data) == 0 {
 		return nil, fmt.Errorf("no data found for store: %s", (*as).name)
@@ -144,8 +146,6 @@ func (as *AugustStore) GetAll() (map[string]interface{}, error) {
 	newSet := make(map[string]interface{})
 
 	for id, val := range (*as).data {
-		as.mu.RLock()
-		defer as.mu.RUnlock()
 		newSet[id] = val.data
 	}
 
@@ -248,7 +248,11 @@ func (as *AugustStore) ValidateId(id string) error {
 func (as *AugustStore) event(name string, id string) {
 	cacheName := fmt.Sprintf("%s::%s::%s", name, (*as).name, id)
 	log.Printf("[EVENT FIRED] %s", cacheName)
+	as.parent.mu.Lock()
 	as.parent.systemModCache = append(as.parent.systemModCache, cacheName)
+	as.parent.mu.Unlock()
+	// eventFunc is invoked outside the lock to prevent deadlocks if the
+	// callback re-enters August methods that acquire the same mutex.
 	(*as).parent.eventFunc(name, (*as).name, id)
 }
 

go.mod

@@ -3,11 +3,9 @@ module github.com/solafide-dev/august
 go 1.20
 
 require (
+	github.com/fsnotify/fsnotify v1.6.0
 	github.com/google/uuid v1.3.1
 	gopkg.in/yaml.v3 v3.0.1
 )
 
-require (
-	github.com/fsnotify/fsnotify v1.6.0 // indirect
-	golang.org/x/sys v0.0.0-20220908164124-27713097b956 // indirect
-)
+require golang.org/x/sys v0.0.0-20220908164124-27713097b956 // indirect