Skip to content

Commit

Permalink
make ResourceMap always strict, #48258
Browse files Browse the repository at this point in the history
  • Loading branch information
jrieken committed Apr 20, 2018
1 parent b0ede6c commit 9f752b9
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 59 deletions.
16 changes: 5 additions & 11 deletions src/vs/base/common/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,12 @@ export class TernarySearchTree<E> {

export class ResourceMap<T> {

protected map: Map<string, T>;
protected readonly map: Map<string, T>;
protected readonly ignoreCase?: boolean;

constructor(private ignoreCase?: boolean) {
constructor() {
this.map = new Map<string, T>();
this.ignoreCase = false; // in the future this should be an uri-comparator
}

public set(resource: URI, value: T): void {
Expand Down Expand Up @@ -414,18 +416,10 @@ export class ResourceMap<T> {

return key;
}
}

export class StrictResourceMap<T> extends ResourceMap<T> {

constructor() {
super();
}

public keys(): URI[] {
return keys(this.map).map(key => URI.parse(key));
return keys(this.map).map(URI.parse);
}

}

// We should fold BoundedMap and LinkedMap. See https://github.com/Microsoft/vscode/issues/28496
Expand Down
38 changes: 19 additions & 19 deletions src/vs/base/test/common/map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -544,32 +544,32 @@ suite('Map', () => {
assert.equal(map.get(uncFile), 'true');
});

test('ResourceMap - files (ignorecase)', function () {
const map = new ResourceMap<any>(true);
// test('ResourceMap - files (ignorecase)', function () {
// const map = new ResourceMap<any>(true);

const fileA = URI.parse('file://some/filea');
const fileB = URI.parse('some://some/other/fileb');
const fileAUpper = URI.parse('file://SOME/FILEA');
// const fileA = URI.parse('file://some/filea');
// const fileB = URI.parse('some://some/other/fileb');
// const fileAUpper = URI.parse('file://SOME/FILEA');

map.set(fileA, 'true');
assert.equal(map.get(fileA), 'true');
// map.set(fileA, 'true');
// assert.equal(map.get(fileA), 'true');

assert.equal(map.get(fileAUpper), 'true');
// assert.equal(map.get(fileAUpper), 'true');

assert.ok(!map.get(fileB));
// assert.ok(!map.get(fileB));

map.set(fileAUpper, 'false');
assert.equal(map.get(fileAUpper), 'false');
// map.set(fileAUpper, 'false');
// assert.equal(map.get(fileAUpper), 'false');

assert.equal(map.get(fileA), 'false');
// assert.equal(map.get(fileA), 'false');

const windowsFile = URI.file('c:\\test with %25\\c#code');
const uncFile = URI.file('\\\\shäres\\path\\c#\\plugin.json');
// const windowsFile = URI.file('c:\\test with %25\\c#code');
// const uncFile = URI.file('\\\\shäres\\path\\c#\\plugin.json');

map.set(windowsFile, 'true');
map.set(uncFile, 'true');
// map.set(windowsFile, 'true');
// map.set(uncFile, 'true');

assert.equal(map.get(windowsFile), 'true');
assert.equal(map.get(uncFile), 'true');
});
// assert.equal(map.get(windowsFile), 'true');
// assert.equal(map.get(uncFile), 'true');
// });
});
4 changes: 2 additions & 2 deletions src/vs/platform/configuration/common/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { IConfigurationRegistry, Extensions, OVERRIDE_PROPERTY_PATTERN } from 'vs/platform/configuration/common/configurationRegistry';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';

export const IConfigurationService = createDecorator<IConfigurationService>('configurationService');

Expand Down Expand Up @@ -47,7 +47,7 @@ export interface IConfigurationChangeEvent {

// Following data is used for Extension host configuration event
changedConfiguration: IConfigurationModel;
changedConfigurationByResource: StrictResourceMap<IConfigurationModel>;
changedConfigurationByResource: ResourceMap<IConfigurationModel>;
}

export interface IConfigurationService {
Expand Down
14 changes: 7 additions & 7 deletions src/vs/platform/configuration/common/configurationModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
'use strict';

import * as json from 'vs/base/common/json';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';
import * as arrays from 'vs/base/common/arrays';
import * as types from 'vs/base/common/types';
import * as objects from 'vs/base/common/objects';
Expand Down Expand Up @@ -278,15 +278,15 @@ export class ConfigurationModelParser {
export class Configuration {

private _workspaceConsolidatedConfiguration: ConfigurationModel = null;
private _foldersConsolidatedConfigurations: StrictResourceMap<ConfigurationModel> = new StrictResourceMap<ConfigurationModel>();
private _foldersConsolidatedConfigurations: ResourceMap<ConfigurationModel> = new ResourceMap<ConfigurationModel>();

constructor(
private _defaultConfiguration: ConfigurationModel,
private _userConfiguration: ConfigurationModel,
private _workspaceConfiguration: ConfigurationModel = new ConfigurationModel(),
private _folderConfigurations: StrictResourceMap<ConfigurationModel> = new StrictResourceMap<ConfigurationModel>(),
private _folderConfigurations: ResourceMap<ConfigurationModel> = new ResourceMap<ConfigurationModel>(),
private _memoryConfiguration: ConfigurationModel = new ConfigurationModel(),
private _memoryConfigurationByResource: StrictResourceMap<ConfigurationModel> = new StrictResourceMap<ConfigurationModel>(),
private _memoryConfigurationByResource: ResourceMap<ConfigurationModel> = new ResourceMap<ConfigurationModel>(),
private _freeze: boolean = true) {
}

Expand Down Expand Up @@ -394,7 +394,7 @@ export class Configuration {
return this._workspaceConfiguration;
}

protected get folders(): StrictResourceMap<ConfigurationModel> {
protected get folders(): ResourceMap<ConfigurationModel> {
return this._folderConfigurations;
}

Expand Down Expand Up @@ -534,15 +534,15 @@ export class ConfigurationChangeEvent extends AbstractConfigurationChangeEvent i

constructor(
private _changedConfiguration: ConfigurationModel = new ConfigurationModel(),
private _changedConfigurationByResource: StrictResourceMap<ConfigurationModel> = new StrictResourceMap<ConfigurationModel>()) {
private _changedConfigurationByResource: ResourceMap<ConfigurationModel> = new ResourceMap<ConfigurationModel>()) {
super();
}

get changedConfiguration(): IConfigurationModel {
return this._changedConfiguration;
}

get changedConfigurationByResource(): StrictResourceMap<IConfigurationModel> {
get changedConfigurationByResource(): ResourceMap<IConfigurationModel> {
return this._changedConfigurationByResource;
}

Expand Down
10 changes: 5 additions & 5 deletions src/vs/workbench/api/node/extHostConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ConfigurationTarget as ExtHostConfigurationTarget } from './extHostType
import { IConfigurationData, ConfigurationTarget, IConfigurationModel } from 'vs/platform/configuration/common/configuration';
import { Configuration, ConfigurationChangeEvent, ConfigurationModel } from 'vs/platform/configuration/common/configurationModels';
import { WorkspaceConfigurationChangeEvent } from 'vs/workbench/services/configuration/common/configurationModels';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';
import { ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry';
import { isObject } from 'vs/base/common/types';

Expand Down Expand Up @@ -209,7 +209,7 @@ export class ExtHostConfiguration implements ExtHostConfigurationShape {

private _toConfigurationChangeEvent(data: IWorkspaceConfigurationChangeEventData): vscode.ConfigurationChangeEvent {
const changedConfiguration = new ConfigurationModel(data.changedConfiguration.contents, data.changedConfiguration.keys, data.changedConfiguration.overrides);
const changedConfigurationByResource: StrictResourceMap<ConfigurationModel> = new StrictResourceMap<ConfigurationModel>();
const changedConfigurationByResource: ResourceMap<ConfigurationModel> = new ResourceMap<ConfigurationModel>();
for (const key of Object.keys(data.changedConfigurationByResource)) {
const resource = URI.parse(key);
const model = data.changedConfigurationByResource[key];
Expand All @@ -225,11 +225,11 @@ export class ExtHostConfiguration implements ExtHostConfigurationShape {
const defaultConfiguration = ExtHostConfiguration.parseConfigurationModel(data.defaults);
const userConfiguration = ExtHostConfiguration.parseConfigurationModel(data.user);
const workspaceConfiguration = ExtHostConfiguration.parseConfigurationModel(data.workspace);
const folders: StrictResourceMap<ConfigurationModel> = Object.keys(data.folders).reduce((result, key) => {
const folders: ResourceMap<ConfigurationModel> = Object.keys(data.folders).reduce((result, key) => {
result.set(URI.parse(key), ExtHostConfiguration.parseConfigurationModel(data.folders[key]));
return result;
}, new StrictResourceMap<ConfigurationModel>());
return new Configuration(defaultConfiguration, userConfiguration, workspaceConfiguration, folders, new ConfigurationModel(), new StrictResourceMap<ConfigurationModel>(), false);
}, new ResourceMap<ConfigurationModel>());
return new Configuration(defaultConfiguration, userConfiguration, workspaceConfiguration, folders, new ConfigurationModel(), new ResourceMap<ConfigurationModel>(), false);
}

private static parseConfigurationModel(model: IConfigurationModel): ConfigurationModel {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { IConfigurationRegistry, IConfigurationPropertySchema, Extensions, ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry';
import { IStoredWorkspaceFolder } from 'vs/platform/workspaces/common/workspaces';
import { Workspace } from 'vs/platform/workspace/common/workspace';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';
import URI from 'vs/base/common/uri';

export class WorkspaceConfigurationModelParser extends ConfigurationModelParser {
Expand Down Expand Up @@ -125,9 +125,9 @@ export class Configuration extends BaseConfiguration {
defaults: ConfigurationModel,
user: ConfigurationModel,
workspaceConfiguration: ConfigurationModel,
folders: StrictResourceMap<ConfigurationModel>,
folders: ResourceMap<ConfigurationModel>,
memoryConfiguration: ConfigurationModel,
memoryConfigurationByResource: StrictResourceMap<ConfigurationModel>,
memoryConfigurationByResource: ResourceMap<ConfigurationModel>,
private readonly _workspace: Workspace) {
super(defaults, user, workspaceConfiguration, folders, memoryConfiguration, memoryConfigurationByResource);
}
Expand Down Expand Up @@ -235,8 +235,8 @@ export class AllKeysConfigurationChangeEvent extends AbstractConfigurationChange
return this._changedConfiguration;
}

get changedConfigurationByResource(): StrictResourceMap<IConfigurationModel> {
return new StrictResourceMap();
get changedConfigurationByResource(): ResourceMap<IConfigurationModel> {
return new ResourceMap();
}

get affectedKeys(): string[] {
Expand All @@ -256,7 +256,7 @@ export class WorkspaceConfigurationChangeEvent implements IConfigurationChangeEv
return this.configurationChangeEvent.changedConfiguration;
}

get changedConfigurationByResource(): StrictResourceMap<IConfigurationModel> {
get changedConfigurationByResource(): ResourceMap<IConfigurationModel> {
return this.configurationChangeEvent.changedConfigurationByResource;
}

Expand Down Expand Up @@ -286,4 +286,4 @@ export class WorkspaceConfigurationChangeEvent implements IConfigurationChangeEv

return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { TPromise } from 'vs/base/common/winjs.base';
import { dirname, basename } from 'path';
import * as assert from 'vs/base/common/assert';
import { Event, Emitter } from 'vs/base/common/event';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';
import { equals, deepClone } from 'vs/base/common/objects';
import { Disposable } from 'vs/base/common/lifecycle';
import { Queue } from 'vs/base/common/async';
Expand Down Expand Up @@ -51,7 +51,7 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat
private defaultConfiguration: DefaultConfigurationModel;
private userConfiguration: UserConfiguration;
private workspaceConfiguration: WorkspaceConfiguration;
private cachedFolderConfigs: StrictResourceMap<FolderConfiguration>;
private cachedFolderConfigs: ResourceMap<FolderConfiguration>;

private workspaceEditingQueue: Queue<void>;

Expand Down Expand Up @@ -453,18 +453,18 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat

private loadConfiguration(): TPromise<void> {
// reset caches
this.cachedFolderConfigs = new StrictResourceMap<FolderConfiguration>();
this.cachedFolderConfigs = new ResourceMap<FolderConfiguration>();

const folders = this.workspace.folders;
return this.loadFolderConfigurations(folders)
.then((folderConfigurations) => {

let workspaceConfiguration = this.getWorkspaceConfigurationModel(folderConfigurations);
const folderConfigurationModels = new StrictResourceMap<ConfigurationModel>();
const folderConfigurationModels = new ResourceMap<ConfigurationModel>();
folderConfigurations.forEach((folderConfiguration, index) => folderConfigurationModels.set(folders[index].uri, folderConfiguration));

const currentConfiguration = this._configuration;
this._configuration = new Configuration(this.defaultConfiguration, this.userConfiguration.configurationModel, workspaceConfiguration, folderConfigurationModels, new ConfigurationModel(), new StrictResourceMap<ConfigurationModel>(), this.getWorkbenchState() !== WorkbenchState.EMPTY ? this.workspace : null); //TODO: Sandy Avoid passing null
this._configuration = new Configuration(this.defaultConfiguration, this.userConfiguration.configurationModel, workspaceConfiguration, folderConfigurationModels, new ConfigurationModel(), new ResourceMap<ConfigurationModel>(), this.getWorkbenchState() !== WorkbenchState.EMPTY ? this.workspace : null); //TODO: Sandy Avoid passing null

if (currentConfiguration) {
const changedKeys = this._configuration.compare(currentConfiguration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import URI from 'vs/base/common/uri';
import { ConfigurationChangeEvent, ConfigurationModel } from 'vs/platform/configuration/common/configurationModels';
import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry';
import { StrictResourceMap } from 'vs/base/common/map';
import { ResourceMap } from 'vs/base/common/map';

suite('FolderSettingsModelParser', () => {

Expand Down Expand Up @@ -194,7 +194,7 @@ suite('AllKeysConfigurationChangeEvent', () => {

test('changeEvent affects keys for any resource', () => {
const configuraiton = new Configuration(new ConfigurationModel({}, ['window.title', 'window.zoomLevel', 'window.restoreFullscreen', 'workbench.editor.enablePreview', 'window.restoreWindows']),
new ConfigurationModel(), new ConfigurationModel(), new StrictResourceMap(), new ConfigurationModel(), new StrictResourceMap(), null);
new ConfigurationModel(), new ConfigurationModel(), new ResourceMap(), new ConfigurationModel(), new ResourceMap(), null);
let testObject = new AllKeysConfigurationChangeEvent(configuraiton, ConfigurationTarget.USER, null);

assert.deepEqual(testObject.affectedKeys, ['window.title', 'window.zoomLevel', 'window.restoreFullscreen', 'workbench.editor.enablePreview', 'window.restoreWindows']);
Expand Down Expand Up @@ -234,4 +234,4 @@ suite('AllKeysConfigurationChangeEvent', () => {
assert.ok(!testObject.affectsConfiguration('files'));
assert.ok(!testObject.affectsConfiguration('files', URI.file('file1')));
});
});
});

0 comments on commit 9f752b9

Please sign in to comment.