Skip to content

Commit

Permalink
User data provider should not iterate over raw changes (fix #126660) (#…
Browse files Browse the repository at this point in the history 
…139793)
  • Loading branch information
@bpasero
bpasero authored Jan 3, 2022
1 parent be6b343 commit 5d6872f
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 25 deletions.
1 change: 1 addition & 0 deletions src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ class SharedProcessMain extends Disposable {
// processes, we want a single process handling these operations.
this._register(new DiskFileSystemProviderClient(mainProcessService.getChannel(LOCAL_FILE_SYSTEM_CHANNEL_NAME), { pathCaseSensitive: isLinux })),
Schemas.userData,
fileService,
logService
));
fileService.registerProvider(Schemas.userData, userDataFileSystemProvider);
Expand Down
8 changes: 8 additions & 0 deletions src/vs/platform/files/common/files.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,14 @@ export class FileChangesEvent {
*/
get rawAdded(): TernarySearchTree<URI, IFileChange> | undefined { return this.added; }

/**
* @deprecated use the `contains` or `affects` method to efficiently find
* out if the event relates to a given resource. these methods ensure:
* - that there is no expensive lookup needed (by using a `TernarySearchTree`)
* - correctly handles `FileChangeType.DELETED` events
*/
get rawUpdated(): TernarySearchTree<URI, IFileChange> | undefined { return this.updated; }

/**
* @deprecated use the `contains` or `affects` method to efficiently find
* out if the event relates to a given resource. these methods ensure:
Expand Down
31 changes: 20 additions & 11 deletions src/vs/platform/userData/common/fileUserDataProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { Event, Emitter } from 'vs/base/common/event';
import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { IFileSystemProviderWithFileReadWriteCapability, IFileChange, IWatchOptions, IStat, FileOverwriteOptions, FileType, FileWriteOptions, FileDeleteOptions, FileSystemProviderCapabilities, IFileSystemProviderWithFileReadStreamCapability, FileReadStreamOptions, IFileSystemProviderWithFileAtomicReadCapability } from 'vs/platform/files/common/files';
import { IFileSystemProviderWithFileReadWriteCapability, IFileChange, IWatchOptions, IStat, FileOverwriteOptions, FileType, FileWriteOptions, FileDeleteOptions, FileSystemProviderCapabilities, IFileSystemProviderWithFileReadStreamCapability, FileReadStreamOptions, IFileSystemProviderWithFileAtomicReadCapability, IFileService, FileChangesEvent, FileChangeType } from 'vs/platform/files/common/files';
import { URI } from 'vs/base/common/uri';
import { CancellationToken } from 'vs/base/common/cancellation';
import { newWriteableStream, ReadableStreamEvents } from 'vs/base/common/stream';
Expand Down Expand Up @@ -34,10 +34,11 @@ export class FileUserDataProvider extends Disposable implements
private readonly fileSystemScheme: string,
private readonly fileSystemProvider: IFileSystemProviderWithFileReadWriteCapability & (IFileSystemProviderWithFileReadStreamCapability | IFileSystemProviderWithFileAtomicReadCapability),
private readonly userDataScheme: string,
private readonly logService: ILogService,
fileService: IFileService,
private readonly logService: ILogService
) {
super();
this._register(this.fileSystemProvider.onDidChangeFile(e => this.handleFileChanges(e)));
this._register(fileService.onDidFilesChange(e => this.handleFileChanges(e)));
}

watch(resource: URI, opts: IWatchOptions): IDisposable {
Expand Down Expand Up @@ -91,15 +92,23 @@ export class FileUserDataProvider extends Disposable implements
return this.fileSystemProvider.delete(this.toFileSystemResource(resource), opts);
}

private handleFileChanges(changes: readonly IFileChange[]): void {
private handleFileChanges(e: FileChangesEvent): void {
const userDataChanges: IFileChange[] = [];
for (const change of changes) {
const userDataResource = this.toUserDataResource(change.resource);
if (this.watchResources.findSubstr(userDataResource)) {
userDataChanges.push({
resource: userDataResource,
type: change.type
});
for (const changes of [{ raw: e.rawAdded, type: FileChangeType.ADDED }, { raw: e.rawUpdated, type: FileChangeType.UPDATED }, { raw: e.rawDeleted, type: FileChangeType.DELETED }]) {
if (changes.raw) {
for (const [resource] of changes.raw) {
if (resource.scheme !== this.fileSystemScheme) {
continue; // only interested in file schemes
}

const userDataResource = this.toUserDataResource(resource);
if (this.watchResources.findSubstr(userDataResource)) {
userDataChanges.push({
resource: userDataResource,
type: changes.type
});
}
}
}
}
if (userDataChanges.length) {
Expand Down
20 changes: 17 additions & 3 deletions src/vs/platform/userData/test/browser/fileUserDataProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { IFileService, FileChangeType, IFileChange, IFileSystemProviderWithFileReadWriteCapability, IStat, FileType, FileSystemProviderCapabilities } from 'vs/platform/files/common/files';
import { IFileService, FileChangeType, IFileChange, IFileSystemProviderWithFileReadWriteCapability, IStat, FileType, FileSystemProviderCapabilities, FileChangesEvent } from 'vs/platform/files/common/files';
import { FileService } from 'vs/platform/files/common/fileService';
import { NullLogService } from 'vs/platform/log/common/log';
import { Schemas } from 'vs/base/common/network';
Expand All @@ -18,6 +18,7 @@ import { InMemoryFileSystemProvider } from 'vs/platform/files/common/inMemoryFil
import { AbstractNativeEnvironmentService } from 'vs/platform/environment/common/environmentService';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import product from 'vs/platform/product/common/product';
import { isLinux } from 'vs/base/common/platform';

const ROOT = URI.file('tests').with({ scheme: 'vscode-tests' });

Expand Down Expand Up @@ -51,7 +52,7 @@ suite('FileUserDataProvider', () => {

environmentService = new TestEnvironmentService(userDataHomeOnDisk);

fileUserDataProvider = new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, logService);
fileUserDataProvider = new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new FileService(logService), logService);
disposables.add(fileUserDataProvider);
disposables.add(testObject.registerProvider(Schemas.userData, fileUserDataProvider));
});
Expand Down Expand Up @@ -299,6 +300,18 @@ class TestFileSystemProvider implements IFileSystemProviderWithFileReadWriteCapa

}

class TestFileService extends FileService {

private readonly _onDidFilesChange2 = this._register(new Emitter<FileChangesEvent>());
override readonly onDidFilesChange = this._onDidFilesChange2.event;

constructor(fileEventEmitter: Emitter<readonly IFileChange[]>) {
super(new NullLogService());

fileEventEmitter.event(changes => this._onDidFilesChange2.fire(new FileChangesEvent(changes, !isLinux)));
}
}

suite('FileUserDataProvider - Watching', () => {

let testObject: FileUserDataProvider;
Expand All @@ -310,7 +323,8 @@ suite('FileUserDataProvider - Watching', () => {
disposables.add(fileEventEmitter);

setup(() => {
testObject = disposables.add(new FileUserDataProvider(rootFileResource.scheme, new TestFileSystemProvider(fileEventEmitter.event), Schemas.userData, new NullLogService()));
const logService = new NullLogService();
testObject = disposables.add(new FileUserDataProvider(rootFileResource.scheme, new TestFileSystemProvider(fileEventEmitter.event), Schemas.userData, new TestFileService(fileEventEmitter), logService));
});

teardown(() => disposables.clear());
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/electron-sandbox/desktop.main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export class DesktopMain extends Disposable {
fileService.registerProvider(Schemas.file, diskFileSystemProvider);

// User Data Provider
fileService.registerProvider(Schemas.userData, this._register(new FileUserDataProvider(Schemas.file, diskFileSystemProvider, Schemas.userData, logService)));
fileService.registerProvider(Schemas.userData, this._register(new FileUserDataProvider(Schemas.file, diskFileSystemProvider, Schemas.userData, fileService, logService)));

// URI Identity
const uriIdentityService = new UriIdentityService(fileService);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/configuration/test/browser/configurationEditingService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ suite('ConfigurationEditingService', () => {
environmentService = TestEnvironmentService;
instantiationService.stub(IEnvironmentService, environmentService);
const remoteAgentService = disposables.add(instantiationService.createInstance(RemoteAgentService, null));
disposables.add(fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, logService))));
disposables.add(fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, logService))));
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
workspaceService = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));
Expand Down
14 changes: 7 additions & 7 deletions src/vs/workbench/services/configuration/test/browser/configurationService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ suite('WorkspaceContextService - Folder', () => {
await fileService.createFolder(folder);

const environmentService = TestEnvironmentService;
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
testObject = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, new RemoteAgentService(null, environmentService, TestProductService, new RemoteAuthorityResolverService(undefined, undefined), new SignService(undefined), new NullLogService()), new UriIdentityService(fileService), new NullLogService()));
await (<WorkspaceService>testObject).initialize(convertToWorkspacePayload(folder));
});
Expand Down Expand Up @@ -142,7 +142,7 @@ suite('WorkspaceContextService - Workspace', () => {
const environmentService = TestEnvironmentService;
const remoteAgentService = disposables.add(instantiationService.createInstance(RemoteAgentService, null));
instantiationService.stub(IRemoteAgentService, remoteAgentService);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
testObject = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));

instantiationService.stub(IWorkspaceContextService, testObject);
Expand Down Expand Up @@ -200,7 +200,7 @@ suite('WorkspaceContextService - Workspace Editing', () => {
const environmentService = TestEnvironmentService;
const remoteAgentService = instantiationService.createInstance(RemoteAgentService, null);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
testObject = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));

instantiationService.stub(IFileService, fileService);
Expand Down Expand Up @@ -443,7 +443,7 @@ suite('WorkspaceService - Initialization', () => {
environmentService = TestEnvironmentService;
const remoteAgentService = instantiationService.createInstance(RemoteAgentService, null);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
testObject = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IWorkspaceContextService, testObject);
Expand Down Expand Up @@ -692,7 +692,7 @@ suite('WorkspaceConfigurationService - Folder', () => {
environmentService = TestEnvironmentService;
const remoteAgentService = instantiationService.createInstance(RemoteAgentService, null);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
workspaceService = testObject = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IWorkspaceContextService, testObject);
Expand Down Expand Up @@ -1363,7 +1363,7 @@ suite('WorkspaceConfigurationService-Multiroot', () => {
environmentService = TestEnvironmentService;
const remoteAgentService = instantiationService.createInstance(RemoteAgentService, null);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
const workspaceService = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache() }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));

instantiationService.stub(IFileService, fileService);
Expand Down Expand Up @@ -2023,7 +2023,7 @@ suite('WorkspaceConfigurationService - Remote Folder', () => {
environmentService = TestEnvironmentService;
const remoteEnvironmentPromise = new Promise<Partial<IRemoteAgentEnvironment>>(c => resolveRemoteEnvironment = () => c({ settingsPath: remoteSettingsResource }));
const remoteAgentService = instantiationService.stub(IRemoteAgentService, <Partial<IRemoteAgentService>>{ getEnvironment: () => remoteEnvironmentPromise });
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
const configurationCache: IConfigurationCache = { read: () => Promise.resolve(''), write: () => Promise.resolve(), remove: () => Promise.resolve(), needsCaching: () => false };
testObject = disposables.add(new WorkspaceService({ configurationCache, remoteAuthority }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));
instantiationService.stub(IWorkspaceContextService, testObject);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/keybinding/test/browser/keybindingEditing.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ suite('KeybindingsEditing', () => {
instantiationService.stub(IThemeService, new TestThemeService());
instantiationService.stub(ILanguageConfigurationService, new TestLanguageConfigurationService());
instantiationService.stub(IModelService, disposables.add(instantiationService.createInstance(ModelService)));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, new NullLogService())));
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(ROOT.scheme, fileSystemProvider, Schemas.userData, fileService, new NullLogService())));
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IUriIdentityService, new UriIdentityService(fileService));
instantiationService.stub(IWorkingCopyFileService, disposables.add(instantiationService.createInstance(WorkingCopyFileService)));
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyBackupService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class NodeTestWorkingCopyBackupService extends NativeWorkingCopyBackupSer

this.diskFileSystemProvider = new DiskFileSystemProvider(logService);
fileService.registerProvider(Schemas.file, this.diskFileSystemProvider);
fileService.registerProvider(Schemas.userData, new FileUserDataProvider(Schemas.file, this.diskFileSystemProvider, Schemas.userData, logService));
fileService.registerProvider(Schemas.userData, new FileUserDataProvider(Schemas.file, this.diskFileSystemProvider, Schemas.userData, fileService, logService));

this.fileService = fileService;
this.backupResourceJoiners = [];
Expand Down

0 comments on commit 5d6872f

Please sign in to comment.