ammarahm-ed / react-native-mmkv-storage

An ultra fast (0.0002s read/write), small & encrypted mobile key-value storage framework for React Native written in C++ using JSI
https://rnmmkv.now.sh
MIT License
1.58k stars 110 forks source link

Interesting - lots of potential #16

Closed bneigher closed 4 years ago

bneigher commented 4 years ago

Interesting project, have you considered writing an adapter to work with https://github.com/rt2zz/redux-persist? If that was in place, I bet this would take off

ammarahm-ed commented 4 years ago

Thats a great idea. I will look into it and see how it is going to work with that. Actually i don't use refux so often so it never crossed my mind. Currently I am working on Native Admob Ads for react-native which is almost done, so after that I will take a look.

ammarahm-ed commented 4 years ago

Looks pretty simple. I will write one in a few days so this could work with redux persist also.

bneigher commented 4 years ago

Awesome, I'll be ur beta tester :)

ammarahm-ed commented 4 years ago

I will put this together soon.

ammarahm-ed commented 4 years ago

So I have implemented the required. methods for this to work with redux-persist. You can test this from this branch: https://github.com/ammarahm-ed/react-native-mmkv-storage/tree/patch-persist-storage

You can implement this as follows:

import MMKVStorage from 'react-native-mmkv-storage';

const storage = await new MMKVStorage.Loader().initialize();

const persistConfig = {
  //...
  storage
}

And if you are using encryption etc. or initializing with an ID, it would work similarly. Just pass the created storage object to persistConfig and it will work.

mralj commented 4 years ago

Hi,

I tried proposed solution, I have manually changed api.js and index.d.ts to reflect the changes, after that I did rm -rf ios/Podfile.lock and pod install.

Unfortunately, I am getting this error:

Screenshot 2020-04-18 at 22 10 08

Am I doing anything wrong ?

Additionally, isn't this supposed to be async ? const storage = await new MMKVStorage.Loader().initialize();

If I haven't made any mistakes, maybe that's the problem.


Additionally I made quick wrapper around mmk to expose getItem and setItem

import MMKVStorage from "react-native-mmkv-storage";

class MMKVPersistor {
  private storage: MMKVStorage.API;

  constructor() {
    // @ts-ignore
    this.storage = new MMKVStorage.Loader().initialize();
  }

  public getItem(key: string) {
    return this.storage.getStringAsync(key);
  }

  public setItem(key: string, item: any) {
    return this.storage.setStringAsync(key, item);
  }

  public removeItem(key: string) {
    return new Promise(resolve => {
      this.storage.removeItem(key);
      resolve();
    });
  }
}

But this doesn't work either:

Screenshot 2020-04-18 at 22 28 19

Again, I think async is issue here :/ But ofc. I. may be wrong, and messed smth. other up 🤞

ammarahm-ed commented 4 years ago

Ahh sorry! Yes it is async. I didn't mention it in the example above. I have made the edit. Are you doing this from the above branch I mentioned? If so then try again with await or .then() and it will work. @mralj

And also the removeItem function is async already.

Also the way you have made the wrapper will not work, because the MMKVStorage.API class is never exposed directly but only when you initialize the library it is returned by the promise and then you can use it.

I have implemented this here, after some testing it will be merged soon so you can use this branch to test: https://github.com/ammarahm-ed/react-native-mmkv-storage/tree/patch-persist-storage

mralj commented 4 years ago

Ahh sorry! Yes it is async. I didn't mention it in the example above. I have made the edit. Are you doing this from the above branch I mentioned? If so then try again with await or .then() and it will work. @mralj

Unfortunately I cannot get it to work this way, because ATM react-native does not support top-level await :/

Also the way you have made the wrapper will not work, because the MMKVStorage.API class is never exposed directly but only when you initialize the library it is returned by the promise and then you can use it.

I thought so, I hoped that due to your example there was some synchronous undocumented initialized.

I have played with our code bit, and changed couple of things:

In file persist.config we have:

export const storage = new MMKVPersistor();

function createPersistSliceConfig(config: IPersistSliceConfigParams): IPersistSliceConfig {
  const persistDefaultData: IPersistSliceConfig = {
    storage: storage,
    blacklist: [],
    debug: false,
    key: "",
    stateReconciler: autoMergeLevel2,
    timeout: 0,
    version: 0
  };

  return {
    ...persistDefaultData,
    ...config
  };
}

...

Then in index.ts:

export let store: Store<IReduxState, AnyAction>;

// We are using Wix RNN
Navigation.events().registerAppLaunchedListener(async () => {
  // firstly initialize MMKV 
  await storage.init();

  // then initialize Redux store
  store = createReduxStore();
  persistReduxStore(store);

});

And finally here is my wrapper:

import MMKVStorage from "react-native-mmkv-storage";

class MMKVPersistor {
  private storage: MMKVStorage.API | undefined;

  public async init() {
    this.storage = await new MMKVStorage.Loader().withInstanceID("test").initialize();
  }

  public getItem(key: string) {
    return this.storage?.getStringAsync(key);
  }

  public setItem(key: string, item: any) {
    return this.storage?.setStringAsync(key, item);
  }

  public removeItem(key: string) {
    return this.storage?.removeItem(key);
  }
}

export default MMKVPersistor;

Unfortunately redux store does not get persisted :/ But it's highly likely I messed something up.

Happy to take another crack at this if/when it gets merged :)

EDIT:

Here is log from Flipper:

        23:49:14.401    MobileApp   [I] <MMKV.cpp:296::loadFromFile> loaded [mmkvIdStore] with 1 values
    23:49:14.401    MobileApp   [E] <libMMKV.mm:137::+[MMKV mmkvWithID:cryptKey:relativePath:mode:]> MMKV not initialized properly, must call +initializeMMKV: in main thread before calling any other MMKV methods
    23:49:14.401    MobileApp   [D] <libMMKV.mm:523::+[MMKV mmapKeyWithMMapID:relativePath:]> mmapKey: test
    23:49:14.402    MobileApp   [I] <MemoryFile_OSX.cpp:36::tryResetFileProtection> protection on [/Users/mralj/Library/Developer/CoreSimulator/Devices/3064CBFB-8364-47D2-927C-9CD2E7A99FD0/data/Containers/Data/Application/D85368F7-EA57-4A5F-A7EE-9A27BFE5AEB9/Documents/mmkv/test] is (null)
    23:49:14.402    MobileApp   [I] <MemoryFile_OSX.cpp:36::tryResetFileProtection> protection on [/Users/mralj/Library/Developer/CoreSimulator/Devices/3064CBFB-8364-47D2-927C-9CD2E7A99FD0/data/Containers/Data/Application/D85368F7-EA57-4A5F-A7EE-9A27BFE5AEB9/Documents/mmkv/test.crc] is (null)
    23:49:14.403    MobileApp   [I] <MMKV.cpp:263::loadFromFile> loading [test] with 8 actual size, file size 4096, InterProcess 0, meta info version:3
    23:49:14.403    MobileApp   [I] <MMKV.cpp:268::loadFromFile> loading [test] with crc 342000313 sequence 1 version 3
    23:49:14.403    MobileApp   [I] <MMKV.cpp:296::loadFromFile> loaded [test] with 1 values
ammarahm-ed commented 4 years ago

Thanks for the detailed information. I will thoroughly look into this and make a proper solution tomorrow so that it actually works with redux-persist. From above it appears that the MMKV is not getting initialized for some reason. You can test the same on android maybe and see if you face a similar problem? .

Also tell me which RN version are you on

mralj commented 4 years ago

I am on RN 0.62.2

I have tested:

  1. iOS simulator
  2. Android emulator
  3. Android real device

On all 3 same thing is happening :/

ammarahm-ed commented 4 years ago

@mralj I got it to work now. I will post a solution soon here.

ammarahm-ed commented 4 years ago

Ok so here is how i got redux-persist to load properly.

In store.js I have set the storage to null. And I have created a createPersistStore function which will initialize the store when storage is loaded.

store.js

import { createStore, applyMiddleware } from 'redux';
import { createLogger } from 'redux-logger';
import { persistStore, persistReducer } from 'redux-persist';

import rootReducer from '../reducers/index';

// Do not assign any value to the following at start:
var persistedReducer;
var store;
let persistor;

const persistConfig = {

  // Root?
  key: 'root',
  // Storage is set to null initially
  storage:null,

  whitelist: [
    'authReducer',
  ],
  // Blacklist (Don't Save Specific Reducers)
  blacklist: [
    'counterReducer',
  ],
};

// When the storage is loaded, we will call this function to initialize the `persist-storage`.

export function createPersistStore(storage) {
//set the storage we created
  persistConfig.storage = storage;  

// create persistReducer
  persistedReducer = persistReducer(persistConfig, rootReducer)

// create the store
  store = createStore(
    persistedReducer,
    applyMiddleware(
      createLogger(),
    ),
  );

// create the persist store
  persistor = persistStore(store);

}

export {
  store,
  persistor,
};

And I have created a StorageProvider Component which basically loads the App after persist-store is created.

storage.js:

// Import MMKV at the top of file
import MMKVStorage from 'react-native-mmkv-storage';

import React from 'react';
import App from '../../App';
import {Provider} from 'react-redux';
import {PersistGate} from 'redux-persist/integration/react';
import {store, persistor, createPersistStore} from '../store/store';
import { useState, useEffect } from 'react';

export const StorageProvider = props => {
 // prevent the app from loading before store is created
  const [loading, setLoading] = useState(true);

  useEffect(() => {

// Initialize the MMKV instance

    new MMKVStorage.Loader().initialize().then(instance => {
     // call this function from storage.js to load the persist-store
      createPersistStore(instance);
   // Load the App.
      setLoading(false);
    });
  });

  return loading ? null : (
    <Provider store={store}>
      <PersistGate loading={null} persistor={persistor}>
        <App/>
      </PersistGate>
    </Provider>
  );
};

And in index.js:

import {AppRegistry} from 'react-native';
import {name as appName} from './app.json';
import {StorageProvider} from './redux/storage';

AppRegistry.registerComponent(appName, () => StorageProvider);

Working Demo: https://github.com/ammarahm-ed/redux-persist-demo

mralj commented 4 years ago

Awesome, thank you very much for this :)

Unfortunately, I still could not get it working for us, but we have everything set up pretty differently.

I tried to align your approach and ours, but was still getting same error, so it must issue on our side.

Unfortunately we are way behind on our release schedule date, and, since AsyncStorage is woking without any issues for us, I really cannot spare any more time on this. Sorry :(

I am really thankful for taking your time for creating this and making it works with redux-persist :)

ammarahm-ed commented 4 years ago

If you have time, you can create a small example app and send me a link to it so I will look into it and fix the problem you are facing or give me an idea of what it is so I can do some research. I have some spare time so I can look into it. @mralj

mralj commented 4 years ago

Thank you very much for your offer! I'll try my best to squeeze the this in and get to the bottom of the issue, or, as you suggested create sampler app, but I cannot promise anything, as I already said, we are waaay behind our deadlines 😞

Also, must initialization be async ? ie. would it be possible to create both initialize which would behave in synchronous way, and initializeAsync which would be same as current initialize ?

ammarahm-ed commented 4 years ago

@mralj initialization can be synchronous but it will be callback based then. Since if we leave it in the air after making the call, it is highly likely that the storage will not be created until then.

Can you handle the logic in a callback function or is that not possible too?

ammarahm-ed commented 4 years ago

export const storage = new MMKVPersistor(); // HERE

function createPersistSliceConfig(config: IPersistSliceConfigParams): IPersistSliceConfig {
  const persistDefaultData: IPersistSliceConfig = {
    storage: storage, /// HERE
    blacklist: [],
    debug: false,
    key: "",
    stateReconciler: autoMergeLevel2,
    timeout: 0,
    version: 0
  };

  return {
    ...persistDefaultData,
    ...config
  };
}

So you are creating the config and loading the storage when it is not initialized. So even when in index.ts you have created the storage. The value in your IPersistSliceConfig is not updated automatically. Hence you should try to update it after creating the storage. Since createPersistSliceConfig is a function, you should pass the storage instance as an argument after creation and then load the store and persistor. I think it will work.


export let store: Store<IReduxState, AnyAction>;

// We are using Wix RNN
Navigation.events().registerAppLaunchedListener(async () => {
  // firstly initialize MMKV 

  let storage = await storage.init();
   createPersistSliceConfig(storage);  // something like this.
  // then initialize Redux store
  store = createReduxStore();
  persistReduxStore(store);

});
mralj commented 4 years ago

Hi, sorry for not replying sooner. I have already tried injecting storage to createPersistSliceConfig(actually I have smth like generateCeatePersistSliceConfig which takes as an argument storage and returns createPersistSliceConfig so storage is partially applied).

But that did not help neither. I don't think there is any problem with react-native-mmkv-storage combined redux-persist, it's probably something in our setup, which is bit complicated so it would take some time to figure out what's actually causing the issue.

My guess is that some of other files is trying to access the data in storage before it's initialization, but I cannot figure out which 🤷‍♂️ (and it doesn't help that we have multiple entry points to the app, using sagas, higher order reducers etc.)

ammarahm-ed commented 4 years ago

@mralj well I am glad to tell you that I have decided to make the initialization synchronous after thinking over it. I hope it will be implemented fully in a day or two. So then you use it directly without any issues. I hope.

mralj commented 4 years ago

That sounds awesome, I will create reminder to check it again in couple of days!

Once again, thank you very much :)

ammarahm-ed commented 4 years ago

@mralj and also you should create MMKV with an instance ID for redux-persist specifically so it is used only by redux-persist and for the rest of the app, you should create another instance of mmkv with and another ID to keep things separate. It will also be more performant and wont cause issues as you stated above.

ammarahm-ed commented 4 years ago

@mralj new version released with synchronous API and redux-persist support built-in.

mralj commented 4 years ago

Great, thanks for letting me know, when I find time, I will check it out, and report back 😁

bneigher commented 4 years ago

@ammarahm-ed awesome, I just swapped FilesystemStorage for this in my app, and it appears to be working.

Reading your (very well done) API docs, I do have a question about encryption <> redux-persist use case. Do we need to manually call encrypt()?

If so, where would you advise this being done in a redux-persist world? I believe redux-persist persists the storage on a timeout, which would be very much so internal to the lib, so I doubt it can be done there... Would it makes sense to do it whenever the app moves to the background?

LMK and I'll report back with how the swap performs over time in my app

bneigher commented 4 years ago

Ah wait - making an archive on iOS is failing: No member named 'armv8_crc32' in namespace 'mmkv'

MMKVCore/MMKV_OSX.cpp

void MMKV::minimalInit(MMKVPath_t defaultRootDir) {
    ThreadLock::ThreadOnce(&once_control, initialize);

    // crc32 instruction requires A10 chip, aka iPhone 7 or iPad 6th generation
    int device = 0, version = 0;
    GetAppleMachineInfo(device, version);
#    ifdef __aarch64__
    if ((device == iPhone && version >= 9) || (device == iPad && version >= 7)) {
        CRC32 = mmkv::armv8_crc32;
    }
#    endif
    MMKVInfo("Apple Device:%d, version:%d", device, version);

    g_rootDir = defaultRootDir;
    mkPath(g_rootDir);

    MMKVInfo("default root dir: " MMKV_PATH_FORMAT, g_rootDir.c_str());
}

Seems to be a new issue with the 1.1.x version of mmkv. https://github.com/Tencent/MMKV/issues/444

TLDR Temp fix: Delete the line CRC32 = mmkv::armv8_crc32; and the build will work

ammarahm-ed commented 4 years ago

@bneigher No you do not need to call it manually. Basically the encrypt and decrypt functions are supposed to only encrypt an MMKV Instance that you have created without encryption or with encryption respectively. Calling it during an app session will have no effect on your current encrypted or simple instance of MMKV.

For encryption to work with redux-persist, you do not need to do anything extra. Just follow the API docs for normal encryption.

const MMKV = new MMKV.Loader().withEncryption().initialize();

// And then load it into redux-persist like you do normally.

I do not think it is possible to lock or unlock an MMKV instance like by calling encrypt or decrypt. Once you kill your app, the instance is removed from memory. And when you load your app, a copy of instance is kept in memory I guess and everything is written to encrypted storage & the memory so your encrypted MMKV file is always encrypted. I hope that helps.

ammarahm-ed commented 4 years ago

@bneigher your second issue, well I dont have a actual Apple device to test this on. But hope that it will be fixed soon in the next version of MMKV since we do not have our own pod, i dont know how to fix this for other developers. Maybe they will have to do the above step manually until there is a proper fix available.

The issue is closed, It was fixed I think. We are using MMKV v1.1.1 which should not have this problem. Can you check which version of MMKV is installed in your Podfile.lock?

bneigher commented 4 years ago

@ammarahm-ed oh I see, I misread that in the docs. Okay I think I understand. Cool that with an unencrypted state, changing the instantiation to include .withEncryption simply worked without having to purge / wipe the state. Feels great that this simply works.

As for the armv8_crc32, yes, my install is using 1.1.1 Podfile

- react-native-mmkv-storage (0.3.2):
    - MMKV (= 1.1.1)
    - React

It should break for anyone trying to build (archive) their ios app. But its not an issue with your repo, simply removing the problematic line as suggested by the workaround in the issue works for me, so if anyone else runs into that hopefully they see this

ammarahm-ed commented 4 years ago

@bneigher The encrypt and decrypt functions fit a in a use case where the user has to control to keep the data encrypted or decrypted. So even when in your initialization:

const MMKV = new MMKVStorage.Loader().initialize()

You are not encrypting it, later in the lifecycle of App, lets say user has some important data stored and they want to keep it encrypted. Then

MMKV.encrypt()

this will encrypted the existing instance of storage and you will not need to change your initialisation function either. The library will handle all the encryption and decryption for you without an issue even when in your intialization you do not have withEncryption it will work.

bneigher commented 4 years ago

@ammarahm-ed okay. But when initializing like:

const MMKV = new MMKVStorage.Loader().withEncryption.initialize()

Is this ONLY decrypting the storage contents first upon boot, and then writing back to storage unencrypted? The typical operator understanding of a user of redux-persist is essentially I should just provider a storage provider as a parameter and then the data is stored and is read automatically with the internal access methods..

Once you kill your app, the instance is removed from memory. I guess I'm curious when the redux state will be encrypted now.

I think the traditional use case here is that if somehow malware is able to find where on the device the redux persisted storage is located, that even if found the information is unusable. And whenever redux-persist decides to set the persist:@key it does so by encrypting the contents after it has successfully been written to storage. AFAIK since I haven't dove too deep into it yet, and based on what I think is going on, is my pathology of how this library operates incorrect?

Apologize for being Naive if I've overlooked the behavior from your docs

ammarahm-ed commented 4 years ago

@bneigher it will decrypt the storage a keep a copy in memory as I believe, your actual storage file is always encrypted I believe. So if you kill your app, of course the storage will already be encrypted. Its like it writes to the encrypted storage with the password you initialize it with and encrypts it again while you read from a fresh copy in memory which is the reason it is fast. So its like it keeps a Cache in memory. there is a method I think, clearMemoryCache which cleans up the storage even from Cache.

bneigher commented 4 years ago

got it. well, very happy about this.. thanks for the work @ammarahm-ed and @mralj

ammarahm-ed commented 4 years ago

@bneigher you are welcome, tell me if you find any issue or have another great idea like this one!

mralj commented 4 years ago

I can confirm now it works like a charm :)

I have tested on Android, and iOS simulator.

Unfortunately could not test on device, nor do Archive option due to error others pointed out :/

Hopefully authors of MMKV will fix this issue in one of their next releases (but as I understand it form this author could not reproduce the issue).

I have 0 knowledge of bridging native RN modules, but could it be some strange RN & MMKV combo ?

ammarahm-ed commented 4 years ago

@mralj use the above provided work-around if you are unable to build it or archive it for a real device. It will work like that.

And I don't know about that. maybe it is the reason.

kyle-ssg commented 4 years ago

@ammarahm-ed, is there any reason why you could see this as being the case? I'm not sure if it's due to the popularity of this library but everyone on the issue seems to be using react native.

The workaround wouldn't be suitable for me as my RN apps build in CI.

ammarahm-ed commented 4 years ago

This library does not touch any internal working of the mmkv library. I still have no idea why this happens only on react-native. Have you tried to archive a fresh RN project with MMKV added as a 'pod' and see if you still see the same issue. The issue created above was closed because the OP found out that it was not related to MMKV lib, their builds were also on CI. @kyle-ssg

kyle-ssg commented 4 years ago

Thanks for the reply @ammarahm-ed, I can confirm on a fresh RN project with only react-native-mmkv-storage installed the same issue occurs.

ammarahm-ed commented 4 years ago

I see, maybe try to add pod 'MMKV' to your main Podfile and remove this library. Then try to archive it. See if the issue still occurs. @kyle-ssg

kyle-ssg commented 4 years ago

Hey @ammarahm-ed, I can confirm this still caused the compilation error. However, there's a patch to MMKV which fixes this issue mentioned here https://github.com/Tencent/MMKV/issues/444. MMKV will be publishing an update, it would be good to update the podspec to point to the new version once it is published.

I guess this should be resolved with https://github.com/ammarahm-ed/react-native-mmkv-storage/issues/24 ?

ammarahm-ed commented 4 years ago

Thats good news, I will watch the MMKV project and push an update as soon as a new version is released.

kyle-ssg commented 4 years ago

Thats good news, I will watch the MMKV project and push an update as soon as a new version is released.

@ammarahm-ed linked 1.2.0 tag in https://github.com/ammarahm-ed/react-native-mmkv-storage/issues/24. I feel like it may be worth closing this issue since there isn't really a specific issue / feature request it's directed towards (I'm guilty of contributing to this :D).

ammarahm-ed commented 4 years ago

That's true, I will close this now.