From c516ab405bcf135237fe48d5d6f708a9bf5ddd7a Mon Sep 17 00:00:00 2001 From: mbalint Date: Mon, 29 Mar 2021 10:35:39 +0200 Subject: [PATCH] Mobile: Resolves #720: Add "Sync only on Wi-Fi" option (#4729) --- .eslintignore | 3 + .gitignore | 3 + packages/app-cli/tests/registry.ts | 84 +++++++++++++++++++ .../android/app/src/main/AndroidManifest.xml | 1 + .../components/side-menu-content.js | 10 +++ packages/app-mobile/package.json | 1 + packages/app-mobile/root.tsx | 33 +++++++- packages/lib/models/Setting.ts | 9 ++ packages/lib/registry.ts | 17 +++- 9 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 packages/app-cli/tests/registry.ts diff --git a/.eslintignore b/.eslintignore index 3037c45d34..29b102735a 100644 --- a/.eslintignore +++ b/.eslintignore @@ -121,6 +121,9 @@ packages/app-cli/tests/models_Note.js.map packages/app-cli/tests/models_Setting.d.ts packages/app-cli/tests/models_Setting.js packages/app-cli/tests/models_Setting.js.map +packages/app-cli/tests/registry.d.ts +packages/app-cli/tests/registry.js +packages/app-cli/tests/registry.js.map packages/app-cli/tests/services/plugins/RepositoryApi.d.ts packages/app-cli/tests/services/plugins/RepositoryApi.js packages/app-cli/tests/services/plugins/RepositoryApi.js.map diff --git a/.gitignore b/.gitignore index e476bb5729..1d8db95803 100644 --- a/.gitignore +++ b/.gitignore @@ -108,6 +108,9 @@ packages/app-cli/tests/models_Note.js.map packages/app-cli/tests/models_Setting.d.ts packages/app-cli/tests/models_Setting.js packages/app-cli/tests/models_Setting.js.map +packages/app-cli/tests/registry.d.ts +packages/app-cli/tests/registry.js +packages/app-cli/tests/registry.js.map packages/app-cli/tests/services/plugins/RepositoryApi.d.ts packages/app-cli/tests/services/plugins/RepositoryApi.js packages/app-cli/tests/services/plugins/RepositoryApi.js.map diff --git a/packages/app-cli/tests/registry.ts b/packages/app-cli/tests/registry.ts new file mode 100644 index 0000000000..7f1e6ca226 --- /dev/null +++ b/packages/app-cli/tests/registry.ts @@ -0,0 +1,84 @@ +import Setting from '@joplin/lib/models/Setting'; +import { reg } from '@joplin/lib/registry'; + +const sync = { + start: jest.fn().mockReturnValue({}), +}; + +describe('Registry', function() { + let originalSyncTarget: typeof reg.syncTarget; + + beforeAll(() => { + Setting.setConstant('env', 'prod'); + originalSyncTarget = reg.syncTarget; + reg.syncTarget = () => ({ + isAuthenticated: () => true, + synchronizer: () => sync, + }); + }); + + afterAll(() => { + Setting.setConstant('env', 'dev'); + reg.syncTarget = originalSyncTarget; + }); + + beforeEach(() => { + jest.useFakeTimers(); + Setting.setValue('sync.interval', 300); + }); + + afterEach(() => { + Setting.setValue('sync.interval', 0); + reg.setupRecurrentSync(); + }); + + describe('when on mobile data', () => { + + beforeEach(() => { + Setting.setValue('sync.mobileWifiOnly', true); + reg.setIsOnMobileData(true); + }); + + it('should not sync automatically', () => { + reg.setupRecurrentSync(); + jest.runOnlyPendingTimers(); + + expect(sync.start).toHaveBeenCalledTimes(0); + }); + + it('should sync if do wifi check is false', done => { + void reg.scheduleSync(1, null, false) + .then(() =>{ + expect(sync.start).toHaveBeenCalled(); + done(); + }); + + jest.runOnlyPendingTimers(); + }); + + it('should sync if "sync only over wifi" is disabled in settings', () => { + Setting.setValue('sync.mobileWifiOnly', false); + reg.setupRecurrentSync(); + jest.runOnlyPendingTimers(); + + expect(sync.start).toHaveBeenCalled(); + }); + + }); + + describe('when not on mobile data', () => { + + beforeEach(() => { + Setting.setValue('sync.mobileWifiOnly', true); + reg.setIsOnMobileData(false); + }); + + it('should sync automatically', () => { + reg.setupRecurrentSync(); + jest.runOnlyPendingTimers(); + + expect(sync.start).toHaveBeenCalled(); + }); + + }); +}); diff --git a/packages/app-mobile/android/app/src/main/AndroidManifest.xml b/packages/app-mobile/android/app/src/main/AndroidManifest.xml index 7e958555fb..13db0c8dc6 100644 --- a/packages/app-mobile/android/app/src/main/AndroidManifest.xml +++ b/packages/app-mobile/android/app/src/main/AndroidManifest.xml @@ -6,6 +6,7 @@ + diff --git a/packages/app-mobile/components/side-menu-content.js b/packages/app-mobile/components/side-menu-content.js index 7dc364803e..0a463cfd7b 100644 --- a/packages/app-mobile/components/side-menu-content.js +++ b/packages/app-mobile/components/side-menu-content.js @@ -345,6 +345,14 @@ class SideMenuContentComponent extends Component { ); } + if (this.props.syncOnlyOverWifi && this.props.isOnMobileData) { + items.push( + + { _('Mobile data - auto-sync disabled') } + + ); + } + return {items}; } @@ -404,6 +412,8 @@ const SideMenuContent = connect(state => { collapsedFolderIds: state.collapsedFolderIds, decryptionWorker: state.decryptionWorker, resourceFetcher: state.resourceFetcher, + isOnMobileData: state.isOnMobileData, + syncOnlyOverWifi: state.settings['sync.mobileWifiOnly'], }; })(SideMenuContentComponent); diff --git a/packages/app-mobile/package.json b/packages/app-mobile/package.json index 8a2c602dde..3c8063da25 100644 --- a/packages/app-mobile/package.json +++ b/packages/app-mobile/package.json @@ -18,6 +18,7 @@ "@react-native-community/clipboard": "^1.5.0", "@react-native-community/datetimepicker": "^3.0.3", "@react-native-community/geolocation": "^2.0.2", + "@react-native-community/netinfo": "^6.0.0", "@react-native-community/push-notification-ios": "^1.6.0", "@react-native-community/slider": "^3.0.3", "buffer": "^5.0.8", diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx index 6a8af7ec87..48b4f42b66 100644 --- a/packages/app-mobile/root.tsx +++ b/packages/app-mobile/root.tsx @@ -29,6 +29,7 @@ import SyncTargetOneDrive from '@joplin/lib/SyncTargetOneDrive'; const { AppState, Keyboard, NativeModules, BackHandler, Animated, View, StatusBar } = require('react-native'); +import NetInfo from '@react-native-community/netinfo'; const DropdownAlert = require('react-native-dropdownalert').default; const AlarmServiceDriver = require('./services/AlarmServiceDriver').default; const SafeAreaView = require('./components/SafeAreaView'); @@ -118,7 +119,7 @@ const generalMiddleware = (store: any) => (next: any) => async (action: any) => if (action.type == 'NAV_GO') Keyboard.dismiss(); if (['NOTE_UPDATE_ONE', 'NOTE_DELETE', 'FOLDER_UPDATE_ONE', 'FOLDER_DELETE'].indexOf(action.type) >= 0) { - if (!await reg.syncTarget().syncStarted()) void reg.scheduleSync(5 * 1000, { syncSteps: ['update_remote', 'delete_remote'] }); + if (!await reg.syncTarget().syncStarted()) void reg.scheduleSync(5 * 1000, { syncSteps: ['update_remote', 'delete_remote'] }, true); SearchEngine.instance().scheduleSyncTables(); } @@ -151,7 +152,7 @@ const generalMiddleware = (store: any) => (next: any) => async (action: any) => // Schedule a sync operation so that items that need to be encrypted // are sent to sync target. - void reg.scheduleSync(); + void reg.scheduleSync(null, null, true); } if (action.type == 'NAV_GO' && action.routeName == 'Notes') { @@ -194,6 +195,7 @@ const appDefaultState = Object.assign({}, defaultState, { route: DEFAULT_ROUTE, noteSelectionEnabled: false, noteSideMenuOptions: null, + isOnMobileData: false, }); const appReducer = (state = appDefaultState, action: any) => { @@ -359,6 +361,12 @@ const appReducer = (state = appDefaultState, action: any) => { newState.noteSideMenuOptions = action.options; break; + case 'MOBILE_DATA_WARNING_UPDATE': + + newState = Object.assign({}, state); + newState.isOnMobileData = action.isOnMobileData; + break; + } } catch (error) { error.message = `In reducer: ${error.message} Action: ${JSON.stringify(action)}`; @@ -583,7 +591,9 @@ async function initialize(dispatch: Function) { // When the app starts we want the full sync to // start almost immediately to get the latest data. - void reg.scheduleSync(1000).then(() => { + // doWifiConnectionCheck set to true so initial sync + // doesn't happen on mobile data + void reg.scheduleSync(1000, null, true).then(() => { // Wait for the first sync before updating the notifications, since synchronisation // might change the notifications. void AlarmService.updateAllNotifications(); @@ -646,6 +656,22 @@ class AppComponent extends React.Component { state: 'initializing', }); + try { + // This will be called right after adding the event listener + // so there's no need to check netinfo on startup + this.unsubscribeNetInfoHandler_ = NetInfo.addEventListener(({ type, details }) => { + const isMobile = details.isConnectionExpensive || type === 'cellular'; + reg.setIsOnMobileData(isMobile); + this.props.dispatch({ + type: 'MOBILE_DATA_WARNING_UPDATE', + isOnMobileData: isMobile, + }); + }); + } catch (error) { + reg.logger().warn('Something went wrong while checking network info'); + reg.logger().info(error); + } + await initialize(this.props.dispatch); this.props.dispatch({ @@ -679,6 +705,7 @@ class AppComponent extends React.Component { componentWillUnmount() { AppState.removeEventListener('change', this.onAppStateChange_); + if (this.unsubscribeNetInfoHandler_) this.unsubscribeNetInfoHandler_(); } componentDidUpdate(prevProps: any) { diff --git a/packages/lib/models/Setting.ts b/packages/lib/models/Setting.ts index 6ca2e4c402..64efd3bde7 100644 --- a/packages/lib/models/Setting.ts +++ b/packages/lib/models/Setting.ts @@ -949,6 +949,15 @@ class Setting extends BaseModel { }, storage: SettingStorage.File, }, + 'sync.mobileWifiOnly': { + value: false, + type: SettingItemType.Bool, + section: 'sync', + public: true, + label: () => _('Synchronise only over WiFi connection'), + storage: SettingStorage.File, + appTypes: ['mobile'], + }, noteVisiblePanes: { value: ['editor', 'viewer'], type: SettingItemType.Array, storage: SettingStorage.File, public: false, appTypes: ['desktop'] }, tagHeaderIsExpanded: { value: true, type: SettingItemType.Bool, public: false, appTypes: ['desktop'] }, folderHeaderIsExpanded: { value: true, type: SettingItemType.Bool, public: false, appTypes: ['desktop'] }, diff --git a/packages/lib/registry.ts b/packages/lib/registry.ts index f689ebf316..690f7c3ebd 100644 --- a/packages/lib/registry.ts +++ b/packages/lib/registry.ts @@ -15,6 +15,7 @@ class Registry { private scheduleSyncId_: any; private recurrentSyncId_: any; private db_: any; + private isOnMobileData_ = false; logger() { if (!this.logger_) { @@ -38,6 +39,12 @@ class Registry { this.showErrorMessageBoxHandler_(message); } + // If isOnMobileData is true, the doWifiConnectionCheck is not set + // and the sync.mobileWifiOnly setting is true it will cancel the sync. + setIsOnMobileData(isOnMobileData: boolean) { + this.isOnMobileData_ = isOnMobileData; + } + resetSyncTarget(syncTargetId: number = null) { if (syncTargetId === null) syncTargetId = Setting.value('sync.target'); delete this.syncTargets_[syncTargetId]; @@ -74,7 +81,7 @@ class Registry { } }; - scheduleSync = async (delay: number = null, syncOptions: any = null) => { + scheduleSync = async (delay: number = null, syncOptions: any = null, doWifiConnectionCheck: boolean = false) => { this.schedSyncCalls_.push(true); try { @@ -104,6 +111,12 @@ class Registry { this.scheduleSyncId_ = null; this.logger().info('Preparing scheduled sync'); + if (doWifiConnectionCheck && Setting.value('sync.mobileWifiOnly') && this.isOnMobileData_) { + this.logger().info('Sync cancelled because we\'re on mobile data'); + promiseResolve(); + return; + } + const syncTargetId = Setting.value('sync.target'); if (!(await this.syncTarget(syncTargetId).isAuthenticated())) { @@ -191,7 +204,7 @@ class Registry { this.recurrentSyncId_ = shim.setInterval(() => { this.logger().info('Running background sync on timer...'); - void this.scheduleSync(0); + void this.scheduleSync(0, null, true); }, 1000 * Setting.value('sync.interval')); } } finally {