mirror of
https://github.com/laurent22/joplin.git
synced 2024-12-24 10:27:10 +02:00
All: Fixed issue where synchroniser would try to update a shared folder that is not longer accessible
This commit is contained in:
parent
98fba37cd3
commit
667d6420f2
@ -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));
|
||||
|
@ -94,8 +94,10 @@ class Logger {
|
||||
};
|
||||
}
|
||||
|
||||
setLevel(level: LogLevel) {
|
||||
public setLevel(level: LogLevel) {
|
||||
const previous = this.level_;
|
||||
this.level_ = level;
|
||||
return previous;
|
||||
}
|
||||
|
||||
level() {
|
||||
|
@ -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);
|
||||
|
||||
|
@ -282,7 +282,7 @@ export default class Folder extends BaseItem {
|
||||
return this.db().selectAll(sql, [folderId]);
|
||||
}
|
||||
|
||||
private static async rootSharedFolders(): Promise<FolderEntity[]> {
|
||||
public static async rootSharedFolders(): Promise<FolderEntity[]> {
|
||||
return this.db().selectAll('SELECT id, share_id FROM folders WHERE parent_id = "" AND share_id != ""');
|
||||
}
|
||||
|
||||
|
@ -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<string, any>, _body: any): Promise<any> => {
|
||||
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);
|
||||
});
|
||||
|
||||
|
||||
});
|
||||
|
@ -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<void> {
|
||||
//
|
||||
// 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<void> {
|
||||
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<StateShare> {
|
||||
const note = await Note.load(noteId);
|
||||
if (!note) throw new Error(`No such note: ${noteId}`);
|
||||
|
Loading…
Reference in New Issue
Block a user