Refactor storage api (#932)

* remove Key() from BrowserStorage

* browser storage foreach

* use browser storage ForEach in CleanupExpiredPersistedStates

* add Contains to browser storage

pkg/app/state.go

@@ -407,22 +407,14 @@ func (m *stateManager) Cleanup() {
 // remove any persisted states that have expired. This method ensures that the
 // local storage is kept clean by eliminating outdated or irrelevant state data.
 func (m *stateManager) CleanupExpiredPersistedStates(ctx Context) {
-	object := Window().Get("Object")
-	if !object.Truthy() {
-		return
-	}
-
-	keys := object.Call("keys", Window().Get("localStorage"))
-	for i, l := 0, keys.Get("length").Int(); i < l; i++ {
-		key := keys.Index(i).String()
-
+	ctx.LocalStorage().ForEach(func(key string) {
 		var state storableState
 		ctx.LocalStorage().Get(key, &state)
-
-		if (len(state.Value) != 0 || len(state.EncryptedValue) != 0) && expiredTime(state.ExpiresAt) {
+		if (len(state.Value) != 0 || len(state.EncryptedValue) != 0) &&
+			expiredTime(state.ExpiresAt) {
 			ctx.LocalStorage().Del(key)
 		}
-	}
+	})
 }
 
 func storeValue(recv, v any) error {

pkg/app/storage.go

@@ -7,27 +7,45 @@ import (
 	"github.com/maxence-charriere/go-app/v9/pkg/errors"
 )
 
-// BrowserStorage is the interface that describes a web browser storage.
+// BrowserStorage defines an interface for interacting with web browser storage
+// mechanisms (such as localStorage in the Web API). It provides methods to set,
+// get, delete, and iterate over stored items, among other functionalities.
 type BrowserStorage interface {
-	// Set sets the value to the given key. The value must be json convertible.
+	// Set stores a value associated with a given key. The value must be capable
+	// of being converted to JSON format. If the value cannot be converted to
+	// JSON or if there's an issue with storage, an error is returned.
 	Set(k string, v any) error
 
-	// Get gets the item associated to the given key and store it in the given
-	// value.
-	// It returns an error if v is not a pointer.
+	// Get retrieves the value associated with a given key and stores it into
+	// the provided variable v. The variable v must be a pointer to a type that
+	// is compatible with the stored value. If v is not a pointer, an error is
+	// returned, indicating incorrect usage.
+	//
+	// If the key does not exist, the operation performs no action on v, leaving
+	// it unchanged. Use Contains to check whether a key exists.
 	Get(k string, v any) error
 
-	// Del deletes the item associated with the given key.
+	// Del removes the item associated with the specified key from the storage.
+	// If the key does not exist, the operation is a no-op.
 	Del(k string)
 
-	// Len returns the number of items stored.
+	// Len returns the total number of items currently stored. This count
+	// includes all keys, regardless of their value.
 	Len() int
 
-	// Key returns the key of the item associated to the given index.
-	Key(i int) (string, error)
-
-	// Clear deletes all items.
+	// Clear removes all items from the storage, effectively resetting it.
 	Clear()
+
+	// ForEach iterates over each item in the storage, executing the provided
+	// function f for each key. The exact order of iteration is not guaranteed
+	// and may vary across different implementations.
+	ForEach(f func(k string))
+
+	// Contains checks if the storage contains an item associated with the given
+	// key. It returns true if the item exists, false otherwise. This method
+	// provides a way to check for the existence of a key without retrieving the
+	// associated value.
+	Contains(k string) bool
 }
 
 type memoryStorage struct {
@@ -86,21 +104,15 @@ func (s *memoryStorage) Len() int {
 	return l
 }
 
-func (s *memoryStorage) Key(i int) (string, error) {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-
-	j := 0
+func (s *memoryStorage) ForEach(f func(key string)) {
 	for k := range s.data {
-		if i == j {
-			return k, nil
-		}
-		j++
+		f(k)
 	}
+}
 
-	return "", errors.New("index out of range").
-		WithTag("index", i).
-		WithTag("len", s.Len())
+func (s *memoryStorage) Contains(k string) bool {
+	_, ok := s.data[k]
+	return ok
 }
 
 type jsStorage struct {
@@ -140,7 +152,7 @@ func (s *jsStorage) Get(k string, v any) error {
 	defer s.mutex.RUnlock()
 
 	item := Window().Get(s.name).Call("getItem", k)
-	if !item.Truthy() {
+	if item.IsNull() {
 		return nil
 	}
 
@@ -150,34 +162,44 @@ func (s *jsStorage) Get(k string, v any) error {
 func (s *jsStorage) Del(k string) {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
-
 	Window().Get(s.name).Call("removeItem", k)
 }
 
 func (s *jsStorage) Clear() {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
-
 	Window().Get(s.name).Call("clear")
 }
 
 func (s *jsStorage) Len() int {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
-
 	return s.len()
 }
 
 func (s *jsStorage) len() int {
 	return Window().Get(s.name).Get("length").Int()
 }
 
-func (s *jsStorage) Key(i int) (string, error) {
-	if l := s.len(); i < 0 || i >= l {
-		return "", errors.New("index out of range").
-			WithTag("index", i).
-			WithTag("len", l)
+func (s *jsStorage) ForEach(f func(key string)) {
+	s.mutex.Lock()
+	length := s.len()
+	keys := make(map[string]struct{}, length)
+	for i := 0; i < length; i++ {
+		key := Window().Get(s.name).Call("key", i)
+		if key.Truthy() {
+			keys[key.String()] = struct{}{}
+		}
+	}
+	s.mutex.Unlock()
+
+	for key := range keys {
+		f(key)
 	}
+}
 
-	return Window().Get(s.name).Call("key", i).String(), nil
+func (s *jsStorage) Contains(k string) bool {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	return !Window().Get(s.name).Call("getItem", k).IsNull()
 }

pkg/app/storage_test.go

@@ -56,16 +56,17 @@ func testBrowserStorage(t *testing.T, s BrowserStorage) {
 			function: testBrowserStorageGetError,
 		},
 		{
-			scenario: "get key at given index",
-			function: testBrowserStorageKey,
+			scenario: "len returns the storage length",
+			function: testBrowserStorageLen,
 		},
 		{
-			scenario: "get key at given index returns an error",
-			function: testBrowserStorageKeyError,
+			scenario: "foreach iterates over each storage keys",
+			function: testBrowserStorageForEach,
 		},
+
 		{
-			scenario: "len returns the storage length",
-			function: testBrowserStorageLen,
+			scenario: "contains reports an existing key",
+			function: testBrowserStorageContains,
 		},
 	}
 
@@ -163,28 +164,47 @@ func testBrowserStorageFull(t *testing.T, s BrowserStorage) {
 	t.Log(err)
 }
 
-func testBrowserStorageKey(t *testing.T, s BrowserStorage) {
+func testBrowserStorageLen(t *testing.T, s BrowserStorage) {
 	s.Clear()
 
-	err := s.Set("hello", 42)
-	require.NoError(t, err)
+	s.Set("hello", 42)
+	s.Set("world", 42)
+	s.Set("bye", 42)
 
-	v, err := s.Key(0)
-	require.NoError(t, err)
-	require.Equal(t, "hello", v)
+	require.Equal(t, 3, s.Len())
 }
 
-func testBrowserStorageKeyError(t *testing.T, s BrowserStorage) {
-	_, err := s.Key(42)
-	require.Error(t, err)
+func testBrowserStorageForEach(t *testing.T, s BrowserStorage) {
+	s.Clear()
+
+	keys := []string{
+		"starwars",
+		"startrek",
+		"alien",
+		"marvel",
+		"dune",
+		"lords of the rings",
+	}
+	for _, k := range keys {
+		s.Set(k, 3000)
+	}
+	require.Equal(t, s.Len(), len(keys))
+
+	keyMap := make(map[string]struct{})
+	s.ForEach(func(key string) {
+		keyMap[key] = struct{}{}
+	})
+	require.Len(t, keyMap, len(keys))
+
+	for _, k := range keys {
+		require.Contains(t, keyMap, k)
+	}
 }
 
-func testBrowserStorageLen(t *testing.T, s BrowserStorage) {
+func testBrowserStorageContains(t *testing.T, s BrowserStorage) {
 	s.Clear()
 
-	s.Set("hello", 42)
-	s.Set("world", 42)
-	s.Set("bye", 42)
-
-	require.Equal(t, 3, s.Len())
+	require.False(t, s.Contains("lightsaber"))
+	s.Set("lightsaber", true)
+	require.True(t, s.Contains("lightsaber"))
 }