Skip to content

Commit e6ef6f8

Browse files
sandy081sandy081
authored
fix handling not included properties (microsoft#264310)
1 parent bc727a2 commit e6ef6f8

File tree

2 files changed

+277
-9
lines changed

2 files changed

+277
-9
lines changed

‎src/vs/platform/configuration/common/configurationModels.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { IExtUri } from '../../../base/common/resources.js';
1414
import * as types from '../../../base/common/types.js';
1515
import { URI, UriComponents } from '../../../base/common/uri.js';
1616
import { addToValueTree, ConfigurationTarget, getConfigurationValue, IConfigurationChange, IConfigurationChangeEvent, IConfigurationCompareResult, IConfigurationData, IConfigurationModel, IConfigurationOverrides, IConfigurationUpdateOverrides, IConfigurationValue, IInspectValue, IOverrides, removeFromValueTree, toValuesTree } from './configuration.js';
17-
import { ConfigurationScope, Extensions, IConfigurationPropertySchema, IConfigurationRegistry, overrideIdentifiersFromKey, OVERRIDE_PROPERTY_REGEX } from './configurationRegistry.js';
17+
import { ConfigurationScope, Extensions, IConfigurationPropertySchema, IConfigurationRegistry, overrideIdentifiersFromKey, OVERRIDE_PROPERTY_REGEX, IRegisteredConfigurationPropertySchema } from './configurationRegistry.js';
1818
import { FileOperation, IFileService } from '../../files/common/files.js';
1919
import { ILogService } from '../../log/common/log.js';
2020
import { Registry } from '../../registry/common/platform.js';
@@ -405,25 +405,27 @@ export class ConfigurationModelParser {
405405
}
406406

407407
protected doParseRaw(raw: any, options?: ConfigurationParseOptions): IConfigurationModel & { restricted?: string[]; hasExcludedProperties?: boolean } {
408-
const configurationProperties = Registry.as<IConfigurationRegistry>(Extensions.Configuration).getConfigurationProperties();
409-
const filtered = this.filter(raw, configurationProperties, true, options);
408+
const registry = Registry.as<IConfigurationRegistry>(Extensions.Configuration);
409+
const configurationProperties = registry.getConfigurationProperties();
410+
const excludedConfigurationProperties = registry.getExcludedConfigurationProperties();
411+
const filtered = this.filter(raw, configurationProperties, excludedConfigurationProperties, true, options);
410412
raw = filtered.raw;
411413
const contents = toValuesTree(raw, message => this.logService.error(`Conflict in settings file ${this._name}: ${message}`));
412414
const keys = Object.keys(raw);
413415
const overrides = this.toOverrides(raw, message => this.logService.error(`Conflict in settings file ${this._name}: ${message}`));
414416
return { contents, keys, overrides, restricted: filtered.restricted, hasExcludedProperties: filtered.hasExcludedProperties };
415417
}
416418

417-
private filter(properties: any, configurationProperties: { [qualifiedKey: string]: IConfigurationPropertySchema | undefined }, filterOverriddenProperties: boolean, options?: ConfigurationParseOptions): { raw: {}; restricted: string[]; hasExcludedProperties: boolean } {
419+
private filter(properties: any, configurationProperties: IStringDictionary<IRegisteredConfigurationPropertySchema>, excludedConfigurationProperties: IStringDictionary<IRegisteredConfigurationPropertySchema>, filterOverriddenProperties: boolean, options?: ConfigurationParseOptions): { raw: {}; restricted: string[]; hasExcludedProperties: boolean } {
418420
let hasExcludedProperties = false;
419-
if (!options?.scopes && !options?.skipRestricted && !options?.exclude?.length) {
421+
if (!options?.scopes && !options?.skipRestricted && !options?.skipUnregistered && !options?.exclude?.length) {
420422
return { raw: properties, restricted: [], hasExcludedProperties };
421423
}
422424
const raw: any = {};
423425
const restricted: string[] = [];
424426
for (const key in properties) {
425427
if (OVERRIDE_PROPERTY_REGEX.test(key) && filterOverriddenProperties) {
426-
const result = this.filter(properties[key], configurationProperties, false, options);
428+
const result = this.filter(properties[key], configurationProperties, excludedConfigurationProperties, false, options);
427429
raw[key] = result.raw;
428430
hasExcludedProperties = hasExcludedProperties || result.hasExcludedProperties;
429431
restricted.push(...result.restricted);
@@ -432,7 +434,7 @@ export class ConfigurationModelParser {
432434
if (propertySchema?.restricted) {
433435
restricted.push(key);
434436
}
435-
if (this.shouldInclude(key, propertySchema, options)) {
437+
if (this.shouldInclude(key, propertySchema, excludedConfigurationProperties, options)) {
436438
raw[key] = properties[key];
437439
} else {
438440
hasExcludedProperties = true;
@@ -442,7 +444,7 @@ export class ConfigurationModelParser {
442444
return { raw, restricted, hasExcludedProperties };
443445
}
444446

445-
private shouldInclude(key: string, propertySchema: IConfigurationPropertySchema | undefined, options: ConfigurationParseOptions): boolean {
447+
private shouldInclude(key: string, propertySchema: IConfigurationPropertySchema | undefined, excludedConfigurationProperties: IStringDictionary<IRegisteredConfigurationPropertySchema>, options: ConfigurationParseOptions): boolean {
446448
if (options.exclude?.includes(key)) {
447449
return false;
448450
}
@@ -459,7 +461,8 @@ export class ConfigurationModelParser {
459461
return false;
460462
}
461463

462-
const scope = propertySchema ? typeof propertySchema.scope !== 'undefined' ? propertySchema.scope : ConfigurationScope.WINDOW : undefined;
464+
const schema = propertySchema ?? excludedConfigurationProperties[key];
465+
const scope = schema ? typeof schema.scope !== 'undefined' ? schema.scope : ConfigurationScope.WINDOW : undefined;
463466
if (scope === undefined || options.scopes === undefined) {
464467
return true;
465468
}

‎src/vs/platform/configuration/test/common/configurationModels.test.ts‎

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,271 @@ suite('ConfigurationModelParser', () => {
101101

102102
});
103103

104+
suite('ConfigurationModelParser - Excluded Properties', () => {
105+
106+
ensureNoDisposablesAreLeakedInTestSuite();
107+
108+
const configurationRegistry = Registry.as<IConfigurationRegistry>(Extensions.Configuration);
109+
110+
let testConfigurationNodes: any[] = [];
111+
112+
setup(() => reset());
113+
teardown(() => reset());
114+
115+
function reset() {
116+
if (testConfigurationNodes.length > 0) {
117+
configurationRegistry.deregisterConfigurations(testConfigurationNodes);
118+
testConfigurationNodes = [];
119+
}
120+
}
121+
122+
function registerTestConfiguration() {
123+
const node = {
124+
'id': 'ExcludedPropertiesTest',
125+
'type': 'object',
126+
'properties': {
127+
'regularProperty': {
128+
'type': 'string' as const,
129+
'default': 'regular',
130+
'restricted': false
131+
},
132+
'restrictedProperty': {
133+
'type': 'string' as const,
134+
'default': 'restricted',
135+
'restricted': true
136+
},
137+
'excludedProperty': {
138+
'type': 'string' as const,
139+
'default': 'excluded',
140+
'restricted': true,
141+
'included': false
142+
},
143+
'excludedNonRestrictedProperty': {
144+
'type': 'string' as const,
145+
'default': 'excludedNonRestricted',
146+
'restricted': false,
147+
'included': false
148+
}
149+
}
150+
};
151+
152+
configurationRegistry.registerConfiguration(node);
153+
testConfigurationNodes.push(node);
154+
return node;
155+
}
156+
157+
test('should handle excluded restricted properties correctly', () => {
158+
registerTestConfiguration();
159+
160+
const testObject = new ConfigurationModelParser('test', new NullLogService());
161+
const testData = {
162+
'regularProperty': 'regularValue',
163+
'restrictedProperty': 'restrictedValue',
164+
'excludedProperty': 'excludedValue',
165+
'excludedNonRestrictedProperty': 'excludedNonRestrictedValue'
166+
};
167+
168+
testObject.parse(JSON.stringify(testData), { skipRestricted: true });
169+
170+
assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue');
171+
assert.strictEqual(testObject.configurationModel.getValue('restrictedProperty'), undefined);
172+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue');
173+
assert.strictEqual(testObject.configurationModel.getValue('excludedNonRestrictedProperty'), 'excludedNonRestrictedValue');
174+
assert.ok(testObject.restrictedConfigurations.includes('restrictedProperty'));
175+
assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty'));
176+
});
177+
178+
test('should find excluded properties when checking for restricted settings', () => {
179+
registerTestConfiguration();
180+
181+
const testObject = new ConfigurationModelParser('test', new NullLogService());
182+
const testData = {
183+
'excludedProperty': 'excludedValue'
184+
};
185+
186+
testObject.parse(JSON.stringify(testData));
187+
188+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue');
189+
assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty'));
190+
});
191+
192+
test('should handle override properties with excluded configurations', () => {
193+
registerTestConfiguration();
194+
195+
const testObject = new ConfigurationModelParser('test', new NullLogService());
196+
const testData = {
197+
'[typescript]': {
198+
'regularProperty': 'overrideRegular',
199+
'restrictedProperty': 'overrideRestricted',
200+
'excludedProperty': 'overrideExcluded'
201+
}
202+
};
203+
204+
testObject.parse(JSON.stringify(testData), { skipRestricted: true });
205+
206+
const overrideConfig = testObject.configurationModel.override('typescript');
207+
assert.strictEqual(overrideConfig.getValue('regularProperty'), 'overrideRegular');
208+
assert.strictEqual(overrideConfig.getValue('restrictedProperty'), undefined);
209+
assert.strictEqual(overrideConfig.getValue('excludedProperty'), 'overrideExcluded');
210+
});
211+
212+
test('should handle scope filtering with excluded properties', () => {
213+
const node = {
214+
'id': 'ScopeExcludedTest',
215+
'type': 'object',
216+
'properties': {
217+
'windowProperty': {
218+
'type': 'string' as const,
219+
'default': 'window',
220+
'scope': ConfigurationScope.WINDOW
221+
},
222+
'applicationProperty': {
223+
'type': 'string' as const,
224+
'default': 'application',
225+
'scope': ConfigurationScope.APPLICATION
226+
},
227+
'excludedApplicationProperty': {
228+
'type': 'string' as const,
229+
'default': 'excludedApplication',
230+
'scope': ConfigurationScope.APPLICATION,
231+
'included': false
232+
}
233+
}
234+
};
235+
236+
configurationRegistry.registerConfiguration(node);
237+
testConfigurationNodes.push(node);
238+
239+
const testObject = new ConfigurationModelParser('test', new NullLogService());
240+
const testData = {
241+
'windowProperty': 'windowValue',
242+
'applicationProperty': 'applicationValue',
243+
'excludedApplicationProperty': 'excludedApplicationValue'
244+
};
245+
246+
testObject.parse(JSON.stringify(testData), { scopes: [ConfigurationScope.WINDOW] });
247+
248+
assert.strictEqual(testObject.configurationModel.getValue('windowProperty'), 'windowValue');
249+
assert.strictEqual(testObject.configurationModel.getValue('applicationProperty'), undefined);
250+
assert.strictEqual(testObject.configurationModel.getValue('excludedApplicationProperty'), undefined);
251+
});
252+
253+
test('filter should handle include/exclude options with excluded properties', () => {
254+
registerTestConfiguration();
255+
256+
const testObject = new ConfigurationModelParser('test', new NullLogService());
257+
const testData = {
258+
'regularProperty': 'regularValue',
259+
'excludedProperty': 'excludedValue'
260+
};
261+
262+
testObject.parse(JSON.stringify(testData), { include: ['excludedProperty'] });
263+
264+
assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue');
265+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue');
266+
});
267+
268+
test('should handle exclude options with excluded properties', () => {
269+
registerTestConfiguration();
270+
271+
const testObject = new ConfigurationModelParser('test', new NullLogService());
272+
const testData = {
273+
'regularProperty': 'regularValue',
274+
'excludedProperty': 'excludedValue'
275+
};
276+
277+
testObject.parse(JSON.stringify(testData), { exclude: ['regularProperty'] });
278+
279+
assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), undefined);
280+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue');
281+
});
282+
283+
test('should report hasExcludedProperties correctly when excluded properties are filtered', () => {
284+
registerTestConfiguration();
285+
286+
const testObject = new ConfigurationModelParser('test', new NullLogService());
287+
const testData = {
288+
'regularProperty': 'regularValue',
289+
'restrictedProperty': 'restrictedValue',
290+
'excludedProperty': 'excludedValue'
291+
};
292+
293+
testObject.parse(JSON.stringify(testData), { skipRestricted: true });
294+
295+
const model = testObject.configurationModel;
296+
297+
assert.notStrictEqual(model.raw, undefined, 'Raw should be set when properties are excluded');
298+
});
299+
300+
test('skipUnregistered should exclude unregistered properties', () => {
301+
registerTestConfiguration();
302+
303+
const testObject = new ConfigurationModelParser('test', new NullLogService());
304+
305+
testObject.parse(JSON.stringify({
306+
'unregisteredProperty': 'value3'
307+
}), { skipUnregistered: true });
308+
309+
assert.strictEqual(testObject.configurationModel.getValue('unregisteredProperty'), undefined);
310+
});
311+
312+
test('shouldInclude method works correctly with excluded properties for skipUnregistered', () => {
313+
registerTestConfiguration();
314+
315+
const testObject = new ConfigurationModelParser('test', new NullLogService());
316+
317+
testObject.parse(JSON.stringify({
318+
'regularProperty': 'value1',
319+
'excludedProperty': 'value2',
320+
'unregisteredProperty': 'value3'
321+
}), { skipUnregistered: true });
322+
323+
assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'value1');
324+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), undefined);
325+
assert.strictEqual(testObject.configurationModel.getValue('unregisteredProperty'), undefined);
326+
});
327+
328+
test('excluded properties are found during property schema lookup', () => {
329+
registerTestConfiguration();
330+
331+
const registry = Registry.as<IConfigurationRegistry>(Extensions.Configuration);
332+
333+
const excludedProperties = registry.getExcludedConfigurationProperties();
334+
assert.ok(excludedProperties['excludedProperty'], 'Excluded property should be in excluded properties map');
335+
assert.ok(excludedProperties['excludedNonRestrictedProperty'], 'Excluded non-restricted property should be in excluded properties map');
336+
337+
const regularProperties = registry.getConfigurationProperties();
338+
assert.strictEqual(regularProperties['excludedProperty'], undefined, 'Excluded property should not be in regular properties map');
339+
assert.strictEqual(regularProperties['excludedNonRestrictedProperty'], undefined, 'Excluded non-restricted property should not be in regular properties map');
340+
341+
assert.ok(regularProperties['regularProperty'], 'Regular property should be in regular properties map');
342+
assert.ok(regularProperties['restrictedProperty'], 'Restricted property should be in regular properties map');
343+
});
344+
345+
test('should correctly use shouldInclude with excluded properties for scope and unregistered filtering', () => {
346+
registerTestConfiguration();
347+
348+
const testObject = new ConfigurationModelParser('test', new NullLogService());
349+
const testData = {
350+
'regularProperty': 'regularValue',
351+
'restrictedProperty': 'restrictedValue',
352+
'excludedProperty': 'excludedValue',
353+
'excludedNonRestrictedProperty': 'excludedNonRestrictedValue',
354+
'unknownProperty': 'unknownValue'
355+
};
356+
357+
testObject.parse(JSON.stringify(testData), { skipRestricted: true });
358+
359+
assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue');
360+
assert.strictEqual(testObject.configurationModel.getValue('restrictedProperty'), undefined);
361+
assert.ok(testObject.restrictedConfigurations.includes('restrictedProperty'));
362+
assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue');
363+
assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty'));
364+
assert.strictEqual(testObject.configurationModel.getValue('excludedNonRestrictedProperty'), 'excludedNonRestrictedValue');
365+
assert.strictEqual(testObject.configurationModel.getValue('unknownProperty'), 'unknownValue');
366+
});
367+
});
368+
104369
suite('ConfigurationModel', () => {
105370

106371
ensureNoDisposablesAreLeakedInTestSuite();

0 commit comments

Comments
 (0)