Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preferences: fix PreferenceProvider.merge method #12275

Merged
merged 1 commit into from
Mar 9, 2023
Merged

preferences: fix PreferenceProvider.merge method #12275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 66 additions & 35 deletions examples/api-tests/src/launch-preferences.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ describe('Launch Preferences', function () {
},
expectation: {
'version': '0.2.0',
'configurations': [validConfiguration, bogusConfiguration],
'compounds': [bogusCompound, bogusCompound2]
'configurations': [validConfiguration2, validConfiguration, bogusConfiguration],
'compounds': [validCompound, bogusCompound, bogusCompound2]
}
});

Expand All @@ -306,15 +306,15 @@ describe('Launch Preferences', function () {
expectation: {
'version': '0.2.0',
'configurations': [validConfiguration2],
'compounds': [bogusCompound, bogusCompound2]
'compounds': [validCompound, bogusCompound, bogusCompound2]
},
inspectExpectation: {
preferenceName: 'launch',
defaultValue: defaultLaunch,
workspaceValue: {
'version': '0.2.0',
'configurations': [validConfiguration2],
'compounds': [bogusCompound, bogusCompound2]
'compounds': [validCompound, bogusCompound, bogusCompound2]
}
}
});
Expand Down Expand Up @@ -461,6 +461,53 @@ describe('Launch Preferences', function () {
]);
}


function mergeLaunchConfigurations(config1, config2) {
if (config1 === undefined && config2 === undefined) {
return undefined;
}
if (config2 === undefined) {
return config1;
}

let result;
// skip invalid configs
if (typeof config1 === 'object' && !Array.isArray(config1)) {
result = { ...config1 };
}
if (typeof config2 === 'object' && !Array.isArray(config2)) {
result = { ...(result ?? {}), ...config2 }
}
//merge configurations and compounds arrays
const mergedConfigurations = mergeArrays(config1?.configurations, config2?.configurations);
if (mergedConfigurations) {
result.configurations = mergedConfigurations
}
const mergedCompounds = mergeArrays(config1?.compounds, config2?.compounds);
if (mergedCompounds) {
result.compounds = mergedCompounds;
}
return result;
}

function mergeArrays(array1, array2) {
if (array1 === undefined && array2 === undefined) {
return undefined;
}
if (!Array.isArray(array1) && !Array.isArray(array2)) {
return undefined;
}
let result = [];
if (Array.isArray(array1)) {
result = [...array1];
}
if (Array.isArray(array2)) {
result = [...result, ...array2];
}
return result;
}


const originalShouldOverwrite = fileResourceResolver['shouldOverwrite'];

before(async () => {
Expand Down Expand Up @@ -560,8 +607,8 @@ describe('Launch Preferences', function () {
preferenceName: 'launch',
defaultValue: defaultLaunch
};
const workspaceValue = launch || settingsLaunch;
if (workspaceValue !== undefined) {
const workspaceValue = mergeLaunchConfigurations(settingsLaunch, launch);
if (workspaceValue !== undefined && JSON.stringify(workspaceValue) !== '{}') {
Object.assign(expected, { workspaceValue });
}
}
Expand All @@ -582,8 +629,8 @@ describe('Launch Preferences', function () {
workspaceFolderValue: inspectExpectation.workspaceValue
});
} else {
const value = launch || settingsLaunch;
if (value !== undefined) {
const value = mergeLaunchConfigurations(settingsLaunch, launch);
if (value !== undefined && JSON.stringify(value) !== '{}') {
Object.assign(expected, {
workspaceValue: value,
workspaceFolderValue: value
Expand All @@ -599,7 +646,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -608,7 +655,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -626,7 +673,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -636,7 +683,11 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue && inspect.workspaceValue.configurations;
assert.deepStrictEqual(actual, [validConfiguration, validConfiguration2]);
let expect = [validConfiguration, validConfiguration2];
if (Array.isArray(settingsLaunch?.configurations)) {
expect = [...(settingsLaunch.configurations), ...expect]
}
assert.deepStrictEqual(actual, expect);
});
}

Expand All @@ -650,11 +701,8 @@ describe('Launch Preferences', function () {
if (Array.isArray(expected)) {
expected = { ...expected };
}
if (expected && !expected.configurations && settingsLaunch && settingsLaunch.configurations !== undefined) {
expected.configurations = settingsLaunch.configurations;
}
}
expected = expected || settingsLaunch;
expected = mergeLaunchConfigurations(settingsLaunch, expected);
assert.deepStrictEqual(actual && actual.workspaceValue, expected);
});

Expand All @@ -665,31 +713,14 @@ describe('Launch Preferences', function () {
const actual = preferences.inspect('launch');
const actualWorkspaceValue = actual && actual.workspaceValue;

let expected = undefined;
let expected = { ...launch };
if (launch) {
expected = { ...launch };
delete expected['configurations'];
}
if (settings) {
let _settingsLaunch = undefined;
if (typeof settingsLaunch === 'object' && !Array.isArray(settings['launch']) && settings['launch'] !== null) {
_settingsLaunch = settingsLaunch;
} else {
_settingsLaunch = expectation;
}
if (expected) {
if (_settingsLaunch.configurations !== undefined) {
expected.configurations = _settingsLaunch.configurations;
}
} else {
expected = _settingsLaunch;
}
}

expected = mergeLaunchConfigurations(settingsLaunch, expected);
assert.deepStrictEqual(actualWorkspaceValue, expected);
});
}

});

}
Expand Down
36 changes: 36 additions & 0 deletions packages/core/src/browser/preferences/preference-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// *****************************************************************************
// Copyright (C) 2023 EclipseSource and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// http://www.eclipse.org/legal/epl-2.0.
//
// This Source Code may also be made available under the following Secondary
// Licenses when the conditions for such availability set forth in the Eclipse
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
// with the GNU Classpath Exception which is available at
// https://www.gnu.org/software/classpath/license.html.
//
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
// *****************************************************************************

import { PreferenceProvider } from './preference-provider';
const { expect } = require('chai');

describe('PreferenceProvider', () => {
it('should preserve extra source fields on merge', () => {
const result = PreferenceProvider.merge({ 'configurations': [], 'compounds': [] }, { 'configurations': [] });
expect(result).deep.equals({ 'configurations': [], 'compounds': [] });
});
it('should preserve extra target fields on merge', () => {
const result = PreferenceProvider.merge({ 'configurations': [] }, { 'configurations': [], 'compounds': [] });
expect(result).deep.equals({ 'configurations': [], 'compounds': [] });
});
it('should merge array values', () => {
const result = PreferenceProvider.merge(
{ 'configurations': [{ 'name': 'test1', 'request': 'launch' }], 'compounds': [] },
{ 'configurations': [{ 'name': 'test2' }] }
);
expect(result).deep.equals({ 'configurations': [{ 'name': 'test1', 'request': 'launch' }, { 'name': 'test2' }], 'compounds': [] });
});
});
3 changes: 3 additions & 0 deletions packages/core/src/browser/preferences/preference-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ export abstract class PreferenceProvider implements Disposable {
if (JSONExt.isObject(source[key]) && JSONExt.isObject(value)) {
this.merge(source[key], value);
continue;
} else if (JSONExt.isArray(source[key]) && JSONExt.isArray(value)) {
source[key] = [...JSONExt.deepCopy(source[key] as any), ...JSONExt.deepCopy(value)];
continue;
}
}
source[key] = JSONExt.deepCopy(value);
Expand Down