[Firebase Segmentation] Add custom installation id cache layer and tests for it.

[diwu-arete]

No description provided.

[vkryachko]

Actually, taking a step back: it looks like your persistence needs are pretty simple at the moment: e.g. it's one value per firebase App. That said, do you actually need sqlite? alternatively can you just use shared preferences for persistence?

Related: do you ever plan to support direct boot mode? in which case you might want to persist state in a direct-boot aware way.

[diwu-arete]

Actually, taking a step back: it looks like your persistence needs are pretty simple at the moment: e.g. it's one value per firebase App. That said, do you actually need sqlite? alternatively can you just use shared preferences for persistence?

Related: do you ever plan to support direct boot mode? in which case you might want to persist state in a direct-boot aware way.

As talked with Vlad offline, change to use shared preference.

[vkryachko]

Due to the way SharedPrefs are implemented, this class does not have to be a singleton, as multiple instances of sharedPrefs that point to the same file share the same thread-safe in-memory backing structure. As a side-effect of doing it your interface will become much simpler, as you won't have to pass FirebaseApp around to be able to read/write to the cache, e.g. something along these lines:

class Cache {
  // note: injecting direct deps only https://github.com/google/guice/wiki/InjectOnlyDirectDependencies
  Cache(String persistenceKey, SharedPreferences prefs) {}
  Value read();
  void write(Value value);
}

When your SDK initializes it can then construct this cache as new Cache(app.getPersistenceKey(), context.getSharedPreferences()); Note that Cache is completely decoupled from FirebaseApp, so there is not need to initialize FirebaseApp during tests, which leads to faster more isolated tests. wdyt?

[diwu-arete]

1. For passing in a String argument and SharedPreference argument from the caller, I don't like the idea, because I don't want the caller to pass in a random string or a random shared pref name.

2. For singleton or non-singleton, I don't have a strong opinion. But the singleton mode can centralize management of all FirebaseApps in one shared pref file, which allows to clearAll() for example, and it doesn't hurt the test too much.

[vkryachko]

I don't feel strongly re 1.

2. you can centralize all FirebaseApps in one shared pref file even if you don't use a singleton, as calling getSharedPreferences() with the same name+mode returns the same instance, clearAll can also be implemented as a static method that takes a Context. It's ultimately up to your team to decide the best approach here, but I feel like singleton does not provide many benefits due to the nature of how sharedPrefs work. If you do decide to use a singleton, pls make getInstance() thread-safe.

[diwu-arete]

Changing to thread-safe singleton.

If we find singleton pattern doesn't work well later, we can always change it.

[vkryachko]

I don't feel strongly re 1.

2. you can centralize all FirebaseApps in one shared pref file even if you don't use a singleton, as calling getSharedPreferences() with the same name+mode returns the same instance, clearAll can also be implemented as a static method that takes a Context. It's ultimately up to your team to decide the best approach here, but I feel like singleton does not provide many benefits due to the nature of how sharedPrefs work. If you do decide to use a singleton, pls make getInstance() thread-safe.

[vkryachko]

Lgmt