This differential introduces MMKV to Android app. For now only string getter and setter is implemented but future diffs will introduce new methods. This
differential introduces simplest API we can test.
Details
Details
- Reviewers
tomek bartek - Commits
- rCOMM1c15cd25b111: Introduce MMKV to Android app
- Below the CommMMKV.initialize() line add line that sets value under a string key.
- In CommNotificationsHandler.java at the beginning add line that tries to get string from the same key and logs this key in case it is present.
- Send notification and check it the same string is logged.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/android/app/src/main/java/app/comm/android/fbjni/CommMMKV.java | ||
|---|---|---|
| 20 ↗ | (On Diff #37011) | Same as here. |
| 27 ↗ | (On Diff #37011) | I have to admit that is a little bit of code duplication - we could export comm::crypto:Tools::generateRandomString to Java. It could add some additional boilerplate so I opted for faster option. Additionally Base 64 strings don't differ much from the output comm::crypto:Tools::generateRandomString - the differences are that instead of " " and "_" we use "+" and "=". |
| 40 ↗ | (On Diff #37011) | I tried to recreate dispatch_once functionality here being inspired by what we do with CommSecureStore. |
| 59 ↗ | (On Diff #37011) | Same as here[[ https://phab.comm.dev/D11045#inline-67015 | name ]]. |
| native/android/app/src/main/java/app/comm/android/fbjni/CommMMKV.java | ||
|---|---|---|
| 27–30 ↗ | (On Diff #37011) | On iOS, you generate random string of length 16, while here you base64-encode 16 bytes, which results in ~20-24 char long string. |
Comment Actions
Fix bug: replace "initializeMMKV" with "initialize" when calling Java method from C++ CommMMKVJavaClass.
| native/android/app/src/cpp/CommMMKV.cpp | ||
|---|---|---|
| 36 ↗ | (On Diff #37064) | Does this method return std::optional<std::string>? |
| 44–52 ↗ | (On Diff #37064) | It doesn't seem we're capturing anything |
| 69 ↗ | (On Diff #37064) | Can we have the new line at the end? |
| native/android/app/src/main/java/app/comm/android/fbjni/CommMMKV.java | ||
| 18–19 ↗ | (On Diff #37064) | It doesn't seem efficient to create a new instance of MMKV on every call of getMMKVInstance. Can we store it as a static variable? |
| 82 ↗ | (On Diff #37064) | newline |
| 27–30 ↗ | (On Diff #37011) | Doesn't that mean that the number of possible encryption keys is different on different platforms? |
| native/android/app/src/cpp/CommMMKV.cpp | ||
|---|---|---|
| 36 ↗ | (On Diff #37064) | Call to result->toStdString() returns std::string but it is a C++ feature that we can return type A from a function which signature says that the return type is T if T has constructor from A. |
| native/android/app/src/main/java/app/comm/android/fbjni/CommMMKV.java | ||
| 18–19 ↗ | (On Diff #37064) | It is not recreated - MMKV has a global dict mapping from ID to instance. See: https://github.com/Tencent/MMKV/blob/master/Core/MMKV_Android.cpp#L139 |