diff --git a/packages/app-desktop/gui/MainScreen/commands/leaveSharedFolder.ts b/packages/app-desktop/gui/MainScreen/commands/leaveSharedFolder.ts index 1ea03df0d..e0b23a9f7 100644 --- a/packages/app-desktop/gui/MainScreen/commands/leaveSharedFolder.ts +++ b/packages/app-desktop/gui/MainScreen/commands/leaveSharedFolder.ts @@ -1,7 +1,6 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; import { _ } from '@joplin/lib/locale'; import ShareService from '@joplin/lib/services/share/ShareService'; -import Setting from '@joplin/lib/models/Setting'; import Logger from '@joplin/lib/Logger'; const logger = Logger.create('leaveSharedFolder'); @@ -26,10 +25,7 @@ export const runtime = (): CommandRuntime => { const share = shares.find(s => s.folder_id === folderId); if (!share) throw new Error(_('Could not verify the share status of this notebook - aborting. Please try again when you are connected to the internet.')); - const userId = Setting.value('sync.userId'); - if (share.user.id === userId) throw new Error('Cannot leave own notebook'); - - await ShareService.instance().leaveSharedFolder(folderId); + await ShareService.instance().leaveSharedFolder(folderId, share.user.id); } catch (error) { logger.error(error); alert(_('Error: %s', error.message)); diff --git a/packages/app-desktop/package-lock.json b/packages/app-desktop/package-lock.json index 4b37b05ef..8d06f1fa3 100644 --- a/packages/app-desktop/package-lock.json +++ b/packages/app-desktop/package-lock.json @@ -1,12 +1,12 @@ { "name": "@joplin/app-desktop", - "version": "2.6.9", + "version": "2.6.10", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@joplin/app-desktop", - "version": "2.6.9", + "version": "2.6.10", "license": "MIT", "dependencies": { "@electron/remote": "^2.0.1", diff --git a/packages/app-desktop/package.json b/packages/app-desktop/package.json index 41e046f7e..b9e17ba4e 100644 --- a/packages/app-desktop/package.json +++ b/packages/app-desktop/package.json @@ -1,6 +1,6 @@ { "name": "@joplin/app-desktop", - "version": "2.6.9", + "version": "2.6.10", "description": "Joplin for Desktop", "main": "main.js", "private": true, diff --git a/packages/app-mobile/android/app/build.gradle b/packages/app-mobile/android/app/build.gradle index 57978c0e4..aa58d89cc 100644 --- a/packages/app-mobile/android/app/build.gradle +++ b/packages/app-mobile/android/app/build.gradle @@ -146,8 +146,8 @@ android { applicationId "net.cozic.joplin" minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion - versionCode 2097664 - versionName "2.6.8" + versionCode 2097665 + versionName "2.6.9" ndk { abiFilters "armeabi-v7a", "x86", "arm64-v8a", "x86_64" } diff --git a/packages/lib/Logger.ts b/packages/lib/Logger.ts index b0f481125..b8226b0b3 100644 --- a/packages/lib/Logger.ts +++ b/packages/lib/Logger.ts @@ -94,8 +94,10 @@ class Logger { }; } - setLevel(level: LogLevel) { + public setLevel(level: LogLevel) { + const previous = this.level_; this.level_ = level; + return previous; } level() { diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index 12ee9170a..f04d0e7b7 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -424,6 +424,7 @@ export default class Synchronizer { // Before synchronising make sure all share_id properties are set // correctly so as to share/unshare the right items. await Folder.updateAllShareIds(this.resourceService()); + if (this.shareService_) await this.shareService_.checkShareConsistency(); const itemUploader = new ItemUploader(this.api(), this.apiCall); diff --git a/packages/lib/models/Folder.ts b/packages/lib/models/Folder.ts index b61601e46..ab0744d8f 100644 --- a/packages/lib/models/Folder.ts +++ b/packages/lib/models/Folder.ts @@ -282,7 +282,7 @@ export default class Folder extends BaseItem { return this.db().selectAll(sql, [folderId]); } - private static async rootSharedFolders(): Promise { + public static async rootSharedFolders(): Promise { return this.db().selectAll('SELECT id, share_id FROM folders WHERE parent_id = "" AND share_id != ""'); } diff --git a/packages/lib/services/share/ShareService.test.ts b/packages/lib/services/share/ShareService.test.ts index 7e565a515..0d55e8c23 100644 --- a/packages/lib/services/share/ShareService.test.ts +++ b/packages/lib/services/share/ShareService.test.ts @@ -1,7 +1,7 @@ import Note from '../../models/Note'; import { encryptionService, msleep, setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils'; import ShareService from './ShareService'; -import reducer from '../../reducer'; +import reducer, { defaultState } from '../../reducer'; import { createStore } from 'redux'; import { FolderEntity, NoteEntity } from '../database/types'; import Folder from '../../models/Folder'; @@ -10,10 +10,15 @@ import { generateKeyPair } from '../e2ee/ppk'; import MasterKey from '../../models/MasterKey'; import { MasterKeyEntity } from '../e2ee/types'; import { updateMasterPassword } from '../e2ee/utils'; +import Logger, { LogLevel } from '../../Logger'; + +const testReducer = (state: any = defaultState, action: any) => { + return reducer(state, action); +}; function mockService(api: any) { const service = new ShareService(); - const store = createStore(reducer as any); + const store = createStore(testReducer as any); service.initialize(store, encryptionService(), api); return service; } @@ -149,4 +154,26 @@ describe('ShareService', function() { expect(content.ppkId).toBe(recipientPpk.id); }); + it('should leave folders that are no longer with the user', async () => { + // `checkShareConsistency` will emit a warning so we need to silent it + // in tests. + const previousLogLevel = Logger.globalLogger.setLevel(LogLevel.Error); + + const service = testShareFolderService({ + 'GET api/shares': async (_query: Record, _body: any): Promise => { + return { + items: [], + has_more: false, + }; + }, + }); + + const folder = await Folder.save({ share_id: 'nolongershared' }); + await service.checkShareConsistency(); + expect(await Folder.load(folder.id)).toBeFalsy(); + + Logger.globalLogger.setLevel(previousLogLevel); + }); + + }); diff --git a/packages/lib/services/share/ShareService.ts b/packages/lib/services/share/ShareService.ts index 791e92b99..c886a9fa8 100644 --- a/packages/lib/services/share/ShareService.ts +++ b/packages/lib/services/share/ShareService.ts @@ -181,10 +181,53 @@ export default class ShareService { // // We don't delete the children here because that would delete them for the // other share participants too. - public async leaveSharedFolder(folderId: string): Promise { + // + // If `folderShareUserId` is provided, the function will check that the user + // does not own the share. It would be an error to leave such a folder + // (instead "unshareFolder" should be called). + public async leaveSharedFolder(folderId: string, folderShareUserId: string = null): Promise { + if (folderShareUserId !== null) { + const userId = Setting.value('sync.userId'); + if (folderShareUserId === userId) throw new Error('Cannot leave own notebook'); + } + await Folder.delete(folderId, { deleteChildren: false }); } + // Finds any folder that is associated with a share, but the user no longer + // has access to the share, and remove these folders. This check is + // necessary otherwise sync will try to update items that are not longer + // accessible and will throw the error "Could not find share with ID: xxxx") + public async checkShareConsistency() { + const rootSharedFolders = await Folder.rootSharedFolders(); + let hasRefreshedShares = false; + let shares = this.shares; + + for (const folder of rootSharedFolders) { + let share = shares.find(s => s.id === folder.share_id); + + if (!share && !hasRefreshedShares) { + shares = await this.refreshShares(); + share = shares.find(s => s.id === folder.share_id); + hasRefreshedShares = true; + } + + if (!share) { + // This folder is a associated with a share, but the user no + // longer has access to this share. It can happen for two + // reasons: + // + // - It no longer exists + // - Or the user rejected that share from a different device, + // and the folder was not deleted as it should have been. + // + // In that case we need to leave the notebook. + logger.warn(`Found a folder that was associated with a share, but the user not longer has access to the share - leaving the folder. Folder: ${folder.title} (${folder.id}). Share: ${folder.share_id}`); + await this.leaveSharedFolder(folder.id); + } + } + } + public async shareNote(noteId: string): Promise { const note = await Note.load(noteId); if (!note) throw new Error(`No such note: ${noteId}`); diff --git a/readme/changelog_android.md b/readme/changelog_android.md index 8e7e1825b..428caf935 100644 --- a/readme/changelog_android.md +++ b/readme/changelog_android.md @@ -1,5 +1,9 @@ # Joplin Android app changelog +## [android-v2.6.9](https://github.com/laurent22/joplin/releases/tag/android-v2.6.9) - 2021-12-20T14:58:42Z + +- Fixed: Fixed issue where synchroniser would try to update a shared folder that is not longer accessible (667d642) + ## [android-v2.6.8](https://github.com/laurent22/joplin/releases/tag/android-v2.6.8) - 2021-12-17T10:15:00Z - Improved: Update Mermaid: 8.12.1 -> 8.13.5 (#5831 by Helmut K. C. Tessarek)