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.

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

Add a reason attribute to filePUT and fileGET #54244

Merged
merged 2 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { EncodingMode, ConfirmResult, EditorInput, IFileEditorInput, ITextEditor
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
import { BinaryEditorModel } from 'vs/workbench/common/editor/binaryEditorModel';
import { FileOperationError, FileOperationResult } from 'vs/platform/files/common/files';
import { ITextFileService, AutoSaveMode, ModelState, TextFileModelChangeEvent } from 'vs/workbench/services/textfile/common/textfiles';
import { ITextFileService, AutoSaveMode, ModelState, TextFileModelChangeEvent, LoadReason } from 'vs/workbench/services/textfile/common/textfiles';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IReference } from 'vs/base/common/lifecycle';
Expand Down Expand Up @@ -260,7 +260,8 @@ export class FileEditorInput extends EditorInput implements IFileEditorInput {
return this.textFileService.models.loadOrCreate(this.resource, {
encoding: this.preferredEncoding,
reload: { async: true }, // trigger a reload of the model if it exists already but do not wait to show the model
allowBinary: this.forceOpenAsText
allowBinary: this.forceOpenAsText,
reason: LoadReason.EDITOR
}).then(model => {

// This is a bit ugly, because we first resolve the model and then resolve a model reference. the reason being that binary
Expand Down
36 changes: 25 additions & 11 deletions src/vs/workbench/services/textfile/common/textFileEditorModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import { guessMimeTypes } from 'vs/base/common/mime';
import { toErrorMessage } from 'vs/base/common/errorMessage';
import URI from 'vs/base/common/uri';
import * as diagnostics from 'vs/base/common/diagnostics';
import * as types from 'vs/base/common/types';
import { isUndefinedOrNull } from 'vs/base/common/types';
import { IMode } from 'vs/editor/common/modes';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { ITextFileService, IAutoSaveConfiguration, ModelState, ITextFileEditorModel, ISaveOptions, ISaveErrorHandler, ISaveParticipant, StateChange, SaveReason, IRawTextContent, ILoadOptions } from 'vs/workbench/services/textfile/common/textfiles';
import { ITextFileService, IAutoSaveConfiguration, ModelState, ITextFileEditorModel, ISaveOptions, ISaveErrorHandler, ISaveParticipant, StateChange, SaveReason, IRawTextContent, ILoadOptions, LoadReason } from 'vs/workbench/services/textfile/common/textfiles';
import { EncodingMode } from 'vs/workbench/common/editor';
import { BaseTextEditorModel } from 'vs/workbench/common/editor/textEditorModel';
import { IBackupFileService } from 'vs/workbench/services/backup/common/backup';
Expand Down Expand Up @@ -275,7 +275,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
isReadonly: false
};

return this.loadWithContent(content, backup);
return this.loadWithContent(content, options, backup);
}

// Otherwise load from file
Expand Down Expand Up @@ -313,7 +313,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil

// Guard against the model having changed in the meantime
if (currentVersionId === this.versionId) {
return this.loadWithContent(content);
return this.loadWithContent(content, options);
}

return this;
Expand Down Expand Up @@ -346,7 +346,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
});
}

private loadWithContent(content: IRawTextContent, backup?: URI): TPromise<TextFileEditorModel> {
private loadWithContent(content: IRawTextContent, options?: ILoadOptions, backup?: URI): TPromise<TextFileEditorModel> {
return this.doLoadWithContent(content, backup).then(model => {

// Telemetry: We log the fileGet telemetry event after the model has been loaded to ensure a good mimetype
Expand All @@ -360,10 +360,16 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
"fileGet" : {
"mimeType" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"ext": { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"path": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
"path": { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"reason": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
Copy link
Contributor

@ramya-rao-a ramya-rao-a Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason here is a number and so comes under measurements not properties in the eventual payload of the telemetry event. So add the "isMeasurement": true in the GDPR annotation here.

Same for the filePUT event

}
*/
this.telemetryService.publicLog('fileGet', { mimeType: guessMimeTypes(this.resource.fsPath).join(', '), ext: path.extname(this.resource.fsPath), path: this.hashService.createSHA1(this.resource.fsPath) });
this.telemetryService.publicLog('fileGet', {
mimeType: guessMimeTypes(this.resource.fsPath).join(', '),
ext: path.extname(this.resource.fsPath),
path: this.hashService.createSHA1(this.resource.fsPath),
reason: options && options.reason ? options.reason : LoadReason.OTHER
});
}

return model;
Expand Down Expand Up @@ -592,7 +598,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
}

private doSave(versionId: number, options: ISaveOptions): TPromise<void> {
if (types.isUndefinedOrNull(options.reason)) {
if (isUndefinedOrNull(options.reason)) {
options.reason = SaveReason.EXPLICIT;
}

Expand Down Expand Up @@ -718,10 +724,15 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
/* __GDPR__
"filePUT" : {
"mimeType" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"ext": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
"ext": { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"reason": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
}
*/
this.telemetryService.publicLog('filePUT', { mimeType: guessMimeTypes(this.resource.fsPath).join(', '), ext: path.extname(this.lastResolvedDiskStat.resource.fsPath) });
this.telemetryService.publicLog('filePUT', {
mimeType: guessMimeTypes(this.resource.fsPath).join(', '),
ext: path.extname(this.resource.fsPath),
reason: options.reason
});
}

// Update dirty state unless model has changed meanwhile
Expand Down Expand Up @@ -761,6 +772,9 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
}

private isSettingsFile(): boolean {
if (path.extname(this.resource.fsPath) !== '.json') {
return false;
}

// Check for global settings file
if (path.isEqual(this.resource.fsPath, this.environmentService.appSettingsPath, !isLinux)) {
Expand Down Expand Up @@ -940,7 +954,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
}

isResolved(): boolean {
return !types.isUndefinedOrNull(this.lastResolvedDiskStat);
return !isUndefinedOrNull(this.lastResolvedDiskStat);
}

isReadonly(): boolean {
Expand Down
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 URI from 'vs/base/common/uri';
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
import { dispose, IDisposable, Disposable } from 'vs/base/common/lifecycle';
import { ITextFileEditorModel, ITextFileEditorModelManager, TextFileModelChangeEvent, StateChange, IModelLoadOrCreateOptions, ILoadOptions } from 'vs/workbench/services/textfile/common/textfiles';
import { ITextFileEditorModel, ITextFileEditorModelManager, TextFileModelChangeEvent, StateChange, IModelLoadOrCreateOptions } from 'vs/workbench/services/textfile/common/textfiles';
import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ResourceMap } from 'vs/base/common/map';
Expand Down Expand Up @@ -132,11 +132,6 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
return pendingLoad;
}

let modelLoadOptions: ILoadOptions;
if (options && options.allowBinary) {
modelLoadOptions = { allowBinary: true };
}

let modelPromise: TPromise<ITextFileEditorModel>;

// Model exists
Expand All @@ -152,7 +147,7 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE

// sync reload: do not return until model reloaded
else {
modelPromise = model.load(modelLoadOptions);
modelPromise = model.load(options);
}
} else {
modelPromise = TPromise.as(model);
Expand All @@ -162,7 +157,7 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
// Model does not exist
else {
model = this.instantiationService.createInstance(TextFileEditorModel, resource, options ? options.encoding : void 0);
modelPromise = model.load(modelLoadOptions);
modelPromise = model.load(options);

// Install state change listener
this.mapResourceToStateChangeListener.set(resource, model.onDidStateChange(state => {
Expand Down
15 changes: 15 additions & 0 deletions src/vs/workbench/services/textfile/common/textfiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ export enum SaveReason {
WINDOW_CHANGE = 4
}

export enum LoadReason {
EDITOR = 1,
REFERENCE = 2,
OTHER = 3
}

export const ITextFileService = createDecorator<ITextFileService>(TEXT_FILE_SERVICE_ID);

export interface IRawTextContent extends IBaseStat {
Expand All @@ -140,6 +146,10 @@ export interface IRawTextContent extends IBaseStat {

export interface IModelLoadOrCreateOptions {

/**
* Context why the model is being loaded or created.
*/
reason?: LoadReason;

/**
* The encoding to use when resolving the model text content.
Expand Down Expand Up @@ -210,6 +220,11 @@ export interface ILoadOptions {
* Allow to load a model even if we think it is a binary file.
*/
allowBinary?: boolean;

/**
* Context why the model is being loaded.
*/
reason?: LoadReason;
}

export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ITextModel } from 'vs/editor/common/model';
import { IDisposable, toDisposable, IReference, ReferenceCollection, ImmortalReference } from 'vs/base/common/lifecycle';
import { IModelService } from 'vs/editor/common/services/modelService';
import { ResourceEditorModel } from 'vs/workbench/common/editor/resourceEditorModel';
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
import { ITextFileService, LoadReason } from 'vs/workbench/services/textfile/common/textfiles';
import * as network from 'vs/base/common/network';
import { ITextModelService, ITextModelContentProvider, ITextEditorModel } from 'vs/editor/common/services/resolverService';
import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
Expand All @@ -34,7 +34,7 @@ class ResourceModelCollection extends ReferenceCollection<TPromise<ITextEditorMo
createReferencedObject(key: string): TPromise<ITextEditorModel> {
const resource = URI.parse(key);
if (this.fileService.canHandleResource(resource)) {
return this.textFileService.models.loadOrCreate(resource);
return this.textFileService.models.loadOrCreate(resource, { reason: LoadReason.REFERENCE });
}

return this.resolveTextModelContent(key).then(() => this.instantiationService.createInstance(ResourceEditorModel, resource));
Expand Down