From e149709f11c6f6d27417ec9bed40ba2be09256d7 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 24 Mar 2020 10:40:24 +0100 Subject: [PATCH] update merge strategy --- .../userDataSync/common/globalStateMerge.ts | 86 +++++++++------ .../userDataSync/common/globalStateSync.ts | 101 ++++++++++++------ .../userDataSync/common/userDataSync.ts | 3 +- 3 files changed, 125 insertions(+), 65 deletions(-) diff --git a/src/vs/platform/userDataSync/common/globalStateMerge.ts b/src/vs/platform/userDataSync/common/globalStateMerge.ts index f4520a76a38..f40cfb12729 100644 --- a/src/vs/platform/userDataSync/common/globalStateMerge.ts +++ b/src/vs/platform/userDataSync/common/globalStateMerge.ts @@ -4,63 +4,78 @@ *--------------------------------------------------------------------------------------------*/ import * as objects from 'vs/base/common/objects'; -import { IGlobalState } from 'vs/platform/userDataSync/common/userDataSync'; +import { IStorageValue } from 'vs/platform/userDataSync/common/userDataSync'; import { IStringDictionary } from 'vs/base/common/collections'; import { values } from 'vs/base/common/map'; +import { IStorageKey } from 'vs/platform/userDataSync/common/storageKeys'; -export function merge(localGloablState: IGlobalState, remoteGlobalState: IGlobalState | null, lastSyncGlobalState: IGlobalState | null): { local?: IGlobalState, remote?: IGlobalState } { - if (!remoteGlobalState) { - return { remote: localGloablState }; - } - - const { local: localArgv, remote: remoteArgv } = doMerge(localGloablState.argv, remoteGlobalState.argv, lastSyncGlobalState ? lastSyncGlobalState.argv : null); - const { local: localStorage, remote: remoteStorage } = doMerge(localGloablState.storage, remoteGlobalState.storage, lastSyncGlobalState ? lastSyncGlobalState.storage : null); - const local: IGlobalState | undefined = localArgv || localStorage ? { argv: localArgv || localGloablState.argv, storage: localStorage || localGloablState.storage } : undefined; - const remote: IGlobalState | undefined = remoteArgv || remoteStorage ? { argv: remoteArgv || remoteGlobalState.argv, storage: remoteStorage || remoteGlobalState.storage } : undefined; - - return { local, remote }; +export interface IMergeResult { + local: { added: IStringDictionary, removed: string[], updated: IStringDictionary }; + remote: IStringDictionary | null; } -function doMerge(local: IStringDictionary, remote: IStringDictionary, base: IStringDictionary | null): { local?: IStringDictionary, remote?: IStringDictionary } { - const localToRemote = compare(local, remote); +export function merge(localStorage: IStringDictionary, remoteStorage: IStringDictionary | null, baseStorage: IStringDictionary | null, storageKeys: ReadonlyArray): IMergeResult { + if (!remoteStorage) { + return { remote: localStorage, local: { added: {}, removed: [], updated: {} } }; + } + + const localToRemote = compare(localStorage, remoteStorage); if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) { // No changes found between local and remote. - return {}; + return { remote: null, local: { added: {}, removed: [], updated: {} } }; } - const baseToRemote = base ? compare(base, remote) : { added: Object.keys(remote).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; - if (baseToRemote.added.size === 0 && baseToRemote.removed.size === 0 && baseToRemote.updated.size === 0) { - // No changes found between base and remote. - return { remote: local }; + const baseToRemote = baseStorage ? compare(baseStorage, remoteStorage) : { added: Object.keys(remoteStorage).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + const baseToLocal = baseStorage ? compare(baseStorage, localStorage) : { added: Object.keys(localStorage).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + + const local: { added: IStringDictionary, removed: string[], updated: IStringDictionary } = { added: {}, removed: [], updated: {} }; + const remote: IStringDictionary = objects.deepClone(remoteStorage); + + // Added in remote + for (const key of values(baseToRemote.added)) { + local.added[key] = remoteStorage[key]; } - const baseToLocal = base ? compare(base, local) : { added: Object.keys(local).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; - if (baseToLocal.added.size === 0 && baseToLocal.removed.size === 0 && baseToLocal.updated.size === 0) { - // No changes found between base and local. - return { local: remote }; + // Updated in Remote + for (const key of values(baseToRemote.updated)) { + local.updated[key] = remoteStorage[key]; } - const merged = objects.deepClone(local); + // Removed in remote + for (const key of values(baseToRemote.removed)) { + local.removed.push(key); + } - // Added in remote - for (const key of values(baseToRemote.added)) { - merged[key] = remote[key]; + // Added in local + for (const key of values(baseToLocal.added)) { + if (!baseToRemote.added.has(key)) { + remote[key] = localStorage[key]; + } } // Updated in Remote for (const key of values(baseToRemote.updated)) { - merged[key] = remote[key]; + if (!baseToRemote.updated.has(key) || !baseToRemote.removed.has(key)) { + const remoteValue = remote[key]; + const localValue = localStorage[key]; + if (localValue.version >= remoteValue.version) { + remote[key] = remoteValue; + } + } } - // Removed in remote & local + // Removed in remote for (const key of values(baseToRemote.removed)) { - // Got removed in local - if (baseToLocal.removed.has(key)) { - delete merged[key]; + if (!baseToRemote.updated.has(key)) { + const remoteValue = remote[key]; + const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0]; + if (storageKey.version >= remoteValue.version) { + delete remote[key]; + } } } - return { local: merged, remote: merged }; + return { local, remote: areSame(remote, remoteStorage) ? null : remoteStorage }; } function compare(from: IStringDictionary, to: IStringDictionary): { added: Set, removed: Set, updated: Set } { @@ -84,3 +99,8 @@ function compare(from: IStringDictionary, to: IStringDictionary): { ad return { added, removed, updated }; } +function areSame(a: IStringDictionary, b: IStringDictionary): boolean { + const { added, removed, updated } = compare(a, b); + return added.size === 0 && removed.size === 0 && updated.size === 0; +} + diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index bd5354631e5..ec0669f1f5d 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -20,11 +20,12 @@ import { URI } from 'vs/base/common/uri'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; import { IStorageKeysSyncRegistryService } from 'vs/platform/userDataSync/common/storageKeys'; +const argvStoragePrefx = 'globalState.argv.'; const argvProperties: string[] = ['locale']; interface ISyncPreviewResult { - readonly local: IGlobalState | undefined; - readonly remote: IGlobalState | undefined; + readonly local: { added: IStringDictionary, removed: string[], updated: IStringDictionary }; + readonly remote: IStringDictionary | null; readonly localUserData: IGlobalState; readonly remoteUserData: IRemoteUserData; readonly lastSyncUserData: IRemoteUserData | null; @@ -69,8 +70,9 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs if (remoteUserData.syncData !== null) { const localUserData = await this.getLocalGlobalState(); - const local: IGlobalState = JSON.parse(remoteUserData.syncData.content); - await this.apply({ local, remote: undefined, remoteUserData, localUserData, lastSyncUserData }); + const localGlobalState: IGlobalState = JSON.parse(remoteUserData.syncData.content); + const { local, remote } = merge(localGlobalState.storage, null, null, this.storageKeysSyncRegistryService.storageKeys); + await this.apply({ local, remote, remoteUserData, localUserData, lastSyncUserData }); } // No remote exists to pull @@ -99,7 +101,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs const localUserData = await this.getLocalGlobalState(); const lastSyncUserData = await this.getLastSyncUserData(); const remoteUserData = await this.getRemoteUserData(lastSyncUserData); - await this.apply({ local: undefined, remote: localUserData, remoteUserData, localUserData, lastSyncUserData }, true); + await this.apply({ local: { added: {}, removed: [], updated: {} }, remote: localUserData.storage, remoteUserData, localUserData, lastSyncUserData }, true); this.logService.info(`${this.syncResourceLogLabel}: Finished pushing UI State.`); } finally { @@ -143,8 +145,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs async hasLocalData(): Promise { try { - const localGloablState = await this.getLocalGlobalState(); - if (localGloablState.argv['locale'] !== 'en') { + const { storage } = await this.getLocalGlobalState(); + if (Object.keys(storage).length > 1 || storage[`${argvStoragePrefx}.locale`]?.value !== 'en') { return true; } } catch (error) { @@ -171,20 +173,21 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs this.logService.trace(`${this.syncResourceLogLabel}: Remote ui state does not exist. Synchronizing ui state for the first time.`); } - const { local, remote } = merge(localGloablState, remoteGlobalState, lastSyncGlobalState); + const { local, remote } = merge(localGloablState.storage, remoteGlobalState.storage, lastSyncGlobalState, this.storageKeysSyncRegistryService.storageKeys); return { local, remote, remoteUserData, localUserData: localGloablState, lastSyncUserData }; } private async apply({ local, remote, remoteUserData, lastSyncUserData, localUserData }: ISyncPreviewResult, forcePush?: boolean): Promise { - const hasChanges = local || remote; + const hasLocalChanged = Object.keys(local.added).length > 0 || Object.keys(local.updated).length > 0 || local.removed.length > 0; + const hasRemoteChanged = remote !== null; - if (!hasChanges) { + if (!hasLocalChanged && !hasRemoteChanged) { this.logService.info(`${this.syncResourceLogLabel}: No changes found during synchronizing ui state.`); } - if (local) { + if (hasLocalChanged) { // update local this.logService.trace(`${this.syncResourceLogLabel}: Updating local ui state...`); await this.backupLocal(JSON.stringify(localUserData)); @@ -192,7 +195,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs this.logService.info(`${this.syncResourceLogLabel}: Updated local ui state`); } - if (remote) { + if (hasRemoteChanged) { // update remote this.logService.trace(`${this.syncResourceLogLabel}: Updating remote ui state...`); const content = JSON.stringify(remote); @@ -209,13 +212,12 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs } private async getLocalGlobalState(): Promise { - const argv: IStringDictionary = {}; const storage: IStringDictionary = {}; const argvContent: string = await this.getLocalArgvContent(); const argvValue: IStringDictionary = parse(argvContent); for (const argvProperty of argvProperties) { if (argvValue[argvProperty] !== undefined) { - argv[argvProperty] = argvValue[argvProperty]; + storage[`${argvStoragePrefx}${argvProperty}`] = { version: 1, value: argvValue[argvProperty] }; } } for (const { key, version } of this.storageKeysSyncRegistryService.storageKeys) { @@ -224,7 +226,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs storage[key] = { version, value }; } } - return { argv, storage }; + return { storage }; } private async getLocalArgvContent(): Promise { @@ -235,25 +237,51 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs return '{}'; } - private async writeLocalGlobalState(globalState: IGlobalState): Promise { - const argvContent = await this.getLocalArgvContent(); - let content = argvContent; - for (const argvProperty of Object.keys(globalState.argv)) { - content = edit(content, [argvProperty], globalState.argv[argvProperty], {}); - } - if (argvContent !== content) { - this.logService.trace(`${this.syncResourceLogLabel}: Updating locale...`); - await this.fileService.writeFile(this.environmentService.argvResource, VSBuffer.fromString(content)); - this.logService.info(`${this.syncResourceLogLabel}: Updated locale.`); - } + private async writeLocalGlobalState({ added, removed, updated }: { added: IStringDictionary, updated: IStringDictionary, removed: string[] }): Promise { + const argv: IStringDictionary = {}; const updatedStorage: IStringDictionary = {}; - for (const key of Object.keys(globalState.storage)) { - const { version, value } = globalState.storage[key]; + const handleUpdatedStorage = (storage: IStringDictionary): void => { + for (const key of Object.keys(storage)) { + if (key.startsWith(argvStoragePrefx)) { + argv[key.substring(argvStoragePrefx.length)] = storage[key].value; + continue; + } + const { version, value } = storage[key]; + const storageKey = this.storageKeysSyncRegistryService.storageKeys.filter(storageKey => storageKey.key === key)[0]; + if (!storageKey) { + this.logService.info(`${this.syncResourceLogLabel}: Skipped updating ${key} in storage. It is not registered to sync.`); + continue; + } + if (storageKey.version !== version) { + this.logService.info(`${this.syncResourceLogLabel}: Skipped updating ${key} in storage. Local version '${storageKey.version}' and remote version '${version} are not same.`); + continue; + } + if (String(value) !== String(this.storageService.get(key, StorageScope.GLOBAL))) { + updatedStorage[key] = value; + } + } + }; + handleUpdatedStorage(added); + handleUpdatedStorage(updated); + for (const key of removed) { + if (key.startsWith(argvStoragePrefx)) { + argv[key.substring(argvStoragePrefx.length)] = undefined; + continue; + } const storageKey = this.storageKeysSyncRegistryService.storageKeys.filter(storageKey => storageKey.key === key)[0]; - if (storageKey && storageKey.version === version && String(value) !== String(this.storageService.get(key, StorageScope.GLOBAL))) { - updatedStorage[key] = value; + if (!storageKey) { + this.logService.info(`${this.syncResourceLogLabel}: Skipped updating ${key} in storage. It is not registered to sync.`); + continue; + } + if (this.storageService.get(key, StorageScope.GLOBAL) !== undefined) { + updatedStorage[key] = undefined; } } + if (Object.keys(argv).length) { + this.logService.trace(`${this.syncResourceLogLabel}: Updating locale...`); + await this.updateArgv(argv); + this.logService.info(`${this.syncResourceLogLabel}: Updated locale`); + } const updatedStorageKeys: string[] = Object.keys(updatedStorage); if (updatedStorageKeys.length) { this.logService.trace(`${this.syncResourceLogLabel}: Updating global state...`); @@ -264,4 +292,17 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs } } + private async updateArgv(argv: IStringDictionary): Promise { + const argvContent = await this.getLocalArgvContent(); + let content = argvContent; + for (const argvProperty of Object.keys(argv)) { + content = edit(content, [argvProperty], argv[argvProperty], {}); + } + if (argvContent !== content) { + this.logService.trace(`${this.syncResourceLogLabel}: Updating locale...`); + await this.fileService.writeFile(this.environmentService.argvResource, VSBuffer.fromString(content)); + this.logService.info(`${this.syncResourceLogLabel}: Updated locale.`); + } + } + } diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index 23a5bd45146..d60ad78872e 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -233,11 +233,10 @@ export interface ISyncExtension { export interface IStorageValue { version: number; - value: any; + value: string; } export interface IGlobalState { - argv: IStringDictionary; storage: IStringDictionary; } -- GitLab