Skip to content

Conversation

@shammond42
Copy link

Summary

This PR fixes several bugs and improves the reliability of StorageManagerSharedPrefs. The changes address key collision issues, race conditions, and adds error handling to maintain consistency between in-memory and persistent storage.

Issues Fixed

1. Key Collision Bug (Critical)

Problem: init() loaded ALL keys from SharedPreferences, not just Aptabase events. Since SharedPreferences is a global store, this caused unrelated application data to be loaded into the events map.

Solution: Added filtering to only load keys with the aptabase_ prefix.

for (final key in keys) {
  if (key.startsWith(_keyPrefix)) {  // Now filters for Aptabase keys only
    final value = _prefs.getString(key);
    // ...
  }
}

2. Improve Encapsulation

Problem: The aptabase_ prefix was defined in aptabase_flutter.dart and used in storage_manager_shared_prefs.dart, coupling the main SDK to storage implementation details.

Solution: Moved all prefix handling into StorageManagerSharedPrefs. The class now adds/removes the prefix internally, and the main SDK passes unprefixed keys.

  • Added _keyPrefix constant in StorageManagerSharedPrefs
  • Updated init() to strip prefix when loading
  • Updated addEvent() and deleteEvents() to add prefix when persisting
  • Removed aptabase_ from key generation in aptabase_flutter.dart

Benefits: Different StorageManager implementations can now use different prefixes or none at all.

3. Race Condition Risk

Problem: In-memory state was updated before disk persistence completed. If persistence failed, in-memory and disk state would be inconsistent.

// OLD - unsafe
_events[key] = event;  // Memory updated first
await sharedPrefs.setString(key, event);  // If this fails, inconsistent!

Solution: Persist to disk first, only update memory on success.

// NEW - safe
final success = await _prefs.setString("$_keyPrefix$key", event);
if (success) {
  _events[key] = event;  // Only update memory if disk write succeeded
}

4. Add Error Handling

Problem: SharedPreferences operations could fail (disk full, permissions issues) with no recovery mechanism, leaving state inconsistent.

Solution: Added try-catch blocks with appropriate error handling:

  • addEvent(): Rethrows exceptions so callers know the operation failed. Events not persisted are not added to memory.
  • deleteEvents(): Silently catches errors since events were already sent successfully. Orphaned keys are cleaned up on next init().

5. Performance - Cache SharedPreferences

Problem: Every addEvent() and deleteEvents() call invoked SharedPreferences.getInstance().

Solution: Cache the instance during init() using late final:

late final SharedPreferences _prefs;

@override
Future<void> init() async {
  _prefs = await SharedPreferences.getInstance();
  // ...
}

Benefits:

  • Eliminates redundant getInstance() calls
  • Cleaner code without null assertions
  • Clear initialization contract

Breaking Changes

None. The API surface remains identical. Changes are internal implementation improvements.

The command dart pub get, made some updates to external package versions.
Without a consistent namespacing of the keys (such as in init) there could be collisions with SharedPreferences settings from other packages or the app itself. This commit moves the namspacing into storage_manager_shared_prefs.dart and applies it to every key reference.
1. Update the _events map first, so that any errors accessing SharedPreferences do not prevent memory from being upated.
2. Add some error handling around SharedPreferences calls to avoid interrupting the flow if it isn’t necessary.
Get the shared preferences instance once during init. This prevents an async await call with every event.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant