Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
  • Loading branch information
sandy081 authored May 30, 2022
1 parent 06fbe24 commit fb3b47f
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 9 deletions.
8 changes: 4 additions & 4 deletions src/vs/platform/userDataSync/common/abstractSynchronizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { FormattingOptions } from 'vs/base/common/jsonFormatter';
import { Disposable } from 'vs/base/common/lifecycle';
import { IExtUri } from 'vs/base/common/resources';
import { uppercaseFirstLetter } from 'vs/base/common/strings';
import { isString } from 'vs/base/common/types';
import { isString, isUndefined } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { IHeaders } from 'vs/base/parts/request/common/request';
import { localize } from 'vs/nls';
Expand Down Expand Up @@ -798,10 +798,10 @@ export abstract class AbstractJsonFileSynchroniser extends AbstractFileSynchroni
super(file, resource, fileService, environmentService, storageService, userDataSyncStoreService, userDataSyncBackupStoreService, userDataSyncEnablementService, telemetryService, logService, configurationService, uriIdentityService);
}

protected hasErrors(content: string): boolean {
protected hasErrors(content: string, isArray: boolean): boolean {
const parseErrors: ParseError[] = [];
parse(content, parseErrors, { allowEmptyContent: true, allowTrailingComma: true });
return parseErrors.length > 0;
const result = parse(content, parseErrors, { allowEmptyContent: true, allowTrailingComma: true });
return parseErrors.length > 0 || (!isUndefined(result) && isArray !== Array.isArray(result));
}

private _formattingOptions: Promise<FormattingOptions> | undefined = undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/userDataSync/common/keybindingsSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
if (remoteContent) {
let localContent: string = fileContent ? fileContent.value.toString() : '[]';
localContent = localContent || '[]';
if (this.hasErrors(localContent)) {
if (this.hasErrors(localContent, true)) {
throw new UserDataSyncError(localize('errorInvalidSettings', "Unable to sync keybindings because the content in the file is not valid. Please open the file and correct it."), UserDataSyncErrorCode.LocalInvalidContent, this.resource);
}

Expand Down Expand Up @@ -222,7 +222,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
if (content !== null) {
content = content.trim();
content = content || '[]';
if (this.hasErrors(content)) {
if (this.hasErrors(content, true)) {
throw new UserDataSyncError(localize('errorInvalidSettings', "Unable to sync keybindings because the content in the file is not valid. Please open the file and correct it."), UserDataSyncErrorCode.LocalInvalidContent, this.resource);
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/vs/platform/userDataSync/common/settingsSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
// First time syncing to remote
else if (fileContent) {
this.logService.trace(`${this.syncResourceLogLabel}: Remote settings does not exist. Synchronizing settings for the first time.`);
mergedContent = fileContent.value.toString();
mergedContent = fileContent.value.toString().trim() || '{}';
this.validateContent(mergedContent);
hasRemoteChanged = true;
}

Expand Down Expand Up @@ -340,7 +341,7 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
}

private validateContent(content: string): void {
if (this.hasErrors(content)) {
if (this.hasErrors(content, false)) {
throw new UserDataSyncError(localize('errorInvalidSettings', "Unable to sync settings as there are errors/warning in settings file."), UserDataSyncErrorCode.LocalInvalidContent, this.resource);
}
}
Expand Down
13 changes: 12 additions & 1 deletion src/vs/platform/userDataSync/test/common/keybindingsSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
import { IFileService } from 'vs/platform/files/common/files';
import { ILogService } from 'vs/platform/log/common/log';
import { getKeybindingsContentFromSyncContent, KeybindingsSynchroniser } from 'vs/platform/userDataSync/common/keybindingsSync';
import { IUserDataSyncStoreService, SyncResource } from 'vs/platform/userDataSync/common/userDataSync';
import { IUserDataSyncStoreService, SyncResource, UserDataSyncError, UserDataSyncErrorCode } from 'vs/platform/userDataSync/common/userDataSync';
import { UserDataSyncClient, UserDataSyncTestServer } from 'vs/platform/userDataSync/test/common/userDataSyncClient';

suite('KeybindingsSync', () => {
Expand Down Expand Up @@ -202,4 +202,15 @@ suite('KeybindingsSync', () => {
assert.deepStrictEqual(server.requests, []);
});

test('sync throws invalid content error - content is an object', async () => {
await client.instantiationService.get(IFileService).writeFile(client.instantiationService.get(IEnvironmentService).keybindingsResource, VSBuffer.fromString('{}'));
try {
await testObject.sync(await client.manifest());
assert.fail('should fail with invalid content error');
} catch (e) {
assert.ok(e instanceof UserDataSyncError);
assert.deepStrictEqual((<UserDataSyncError>e).code, UserDataSyncErrorCode.LocalInvalidContent);
}
});

});
11 changes: 11 additions & 0 deletions src/vs/platform/userDataSync/test/common/settingsSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,17 @@ suite('SettingsSync - Auto', () => {
}
});

test('sync throws invalid content error - content is an array', async () => {
await updateSettings('[]', client);
try {
await testObject.sync(await client.manifest());
assert.fail('should fail with invalid content error');
} catch (e) {
assert.ok(e instanceof UserDataSyncError);
assert.deepStrictEqual((<UserDataSyncError>e).code, UserDataSyncErrorCode.LocalInvalidContent);
}
});

test('sync when there are conflicts', async () => {
const client2 = disposableStore.add(new UserDataSyncClient(server));
await client2.setUp(true);
Expand Down

0 comments on commit fb3b47f

Please sign in to comment.