Skip to content

Private property should start with underscore“ #103

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

Merged
merged 1 commit into from
Mar 24, 2020
Merged

Private property should start with underscore“ #103

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
77 changes: 40 additions & 37 deletions src/build/buildController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,58 @@ import { BuildInput, BuildType } from './buildInput';
import { DocfxExecutionResult } from './buildResult';

export class BuildController {
private activeWorkSpaceFolder: vscode.WorkspaceFolder;
private currentBuildCorrelationId: string;

public instanceAvailable: boolean;
private _activeWorkSpaceFolder: vscode.WorkspaceFolder;
private _currentBuildCorrelationId: string;
private _instanceAvailable: boolean;

constructor(
private buildExecutor: BuildExecutor,
private opBuildAPIClient: OPBuildAPIClient,
private diagnosticController: DiagnosticController,
private eventStream: EventStream,
private _buildExecutor: BuildExecutor,
private _opBuildAPIClient: OPBuildAPIClient,
private _diagnosticController: DiagnosticController,
private _eventStream: EventStream,
) {
this.instanceAvailable = true;
this._instanceAvailable = true;
}

public get instanceAvailable(): boolean {
return this._instanceAvailable;
}

public async build(correlationId: string, uri: vscode.Uri, credential: Credential): Promise<void> {
let buildInput: BuildInput;
this.eventStream.post(new BuildTriggered(correlationId));
this._eventStream.post(new BuildTriggered(correlationId));
let start = Date.now();

try {
buildInput = await this.getBuildInput(uri, credential);
this.setAvailableFlag();
} catch (err) {
this.eventStream.post(new BuildFailed(correlationId, buildInput, getTotalTimeInSeconds(), err));
this._eventStream.post(new BuildFailed(correlationId, buildInput, getTotalTimeInSeconds(), err));
return;
}

try {
this.currentBuildCorrelationId = correlationId;
this.eventStream.post(new BuildStarted(this.activeWorkSpaceFolder.name));
let buildResult = await this.buildExecutor.RunBuild(correlationId, buildInput, credential.userInfo.userToken);
this._currentBuildCorrelationId = correlationId;
this._eventStream.post(new BuildStarted(this._activeWorkSpaceFolder.name));
let buildResult = await this._buildExecutor.RunBuild(correlationId, buildInput, credential.userInfo.userToken);
// TODO: For multiple docset repo, we still need to generate report if one docset build crashed
switch (buildResult.result) {
case DocfxExecutionResult.Succeeded:
visualizeBuildReport(buildInput.localRepositoryPath, this.diagnosticController, this.eventStream);
this.eventStream.post(new BuildSucceeded(correlationId, buildInput, getTotalTimeInSeconds(), buildResult));
visualizeBuildReport(buildInput.localRepositoryPath, this._diagnosticController, this._eventStream);
this._eventStream.post(new BuildSucceeded(correlationId, buildInput, getTotalTimeInSeconds(), buildResult));
break;
case DocfxExecutionResult.Canceled:
this.eventStream.post(new BuildCanceled(correlationId, buildInput, getTotalTimeInSeconds()));
this._eventStream.post(new BuildCanceled(correlationId, buildInput, getTotalTimeInSeconds()));
break;
case DocfxExecutionResult.Failed:
throw new DocsError('Running DocFX failed', ErrorCode.RunDocfxFailed);
}
}
catch (err) {
this.eventStream.post(new BuildFailed(correlationId, buildInput, getTotalTimeInSeconds(), err));
this._eventStream.post(new BuildFailed(correlationId, buildInput, getTotalTimeInSeconds(), err));
}
finally {
this.currentBuildCorrelationId = undefined;
this._currentBuildCorrelationId = undefined;
this.resetAvailableFlag();
}

Expand All @@ -74,35 +77,35 @@ export class BuildController {
}

public cancelBuild(): void {
if (!this.instanceAvailable) {
if (!this._instanceAvailable) {
try {
this.eventStream.post(new CancelBuildTriggered(this.currentBuildCorrelationId));
this.buildExecutor.cancelBuild();
this.eventStream.post(new CancelBuildSucceeded(this.currentBuildCorrelationId));
this._eventStream.post(new CancelBuildTriggered(this._currentBuildCorrelationId));
this._buildExecutor.cancelBuild();
this._eventStream.post(new CancelBuildSucceeded(this._currentBuildCorrelationId));
} catch (err) {
this.eventStream.post(new CancelBuildFailed(this.currentBuildCorrelationId, err));
this._eventStream.post(new CancelBuildFailed(this._currentBuildCorrelationId, err));
}
}
}

private setAvailableFlag() {
if (!this.instanceAvailable) {
if (!this._instanceAvailable) {
throw new DocsError('Last build has not finished.', ErrorCode.TriggerBuildWhenInstantNotAvailable);
}
this.instanceAvailable = false;
this.eventStream.post(new BuildInstantAllocated());
this._instanceAvailable = false;
this._eventStream.post(new BuildInstantAllocated());
}

private resetAvailableFlag() {
this.instanceAvailable = true;
this.eventStream.post(new BuildInstantReleased());
this._instanceAvailable = true;
this._eventStream.post(new BuildInstantReleased());
}

private async getBuildInput(uri: vscode.Uri, credential: Credential): Promise<BuildInput> {
if (uri) {
// Trigger build from the right click the workspace file.
this.activeWorkSpaceFolder = vscode.workspace.getWorkspaceFolder(uri);
} else if (!this.activeWorkSpaceFolder) {
this._activeWorkSpaceFolder = vscode.workspace.getWorkspaceFolder(uri);
} else if (!this._activeWorkSpaceFolder) {
// Trigger build from command palette or click the status bar
let workspaceFolders = vscode.workspace.workspaceFolders;
if (workspaceFolders) {
Expand All @@ -113,13 +116,13 @@ export class BuildController {
ErrorCode.TriggerBuildWithoutSpecificWorkspace
);
}
this.activeWorkSpaceFolder = workspaceFolders[0];
this._activeWorkSpaceFolder = workspaceFolders[0];
}
}

// Check the workspace is a valid Docs repository
await this.validateWorkSpaceFolder(this.activeWorkSpaceFolder);
let localRepositoryPath = this.activeWorkSpaceFolder.uri.fsPath;
await this.validateWorkSpaceFolder(this._activeWorkSpaceFolder);
let localRepositoryPath = this._activeWorkSpaceFolder.uri.fsPath;

// Check user sign in status
if (credential.signInStatus !== 'SignedIn') {
Expand Down Expand Up @@ -172,7 +175,7 @@ export class BuildController {
}

private async retrieveRepositoryInfo(localRepositoryPath: string, buildUserToken: string): Promise<string[]> {
this.eventStream.post(new BuildProgress('Retrieving repository information for current workspace folder...\n'));
this._eventStream.post(new BuildProgress('Retrieving repository information for current workspace folder...\n'));

let localRepositoryUrl: string;
try {
Expand All @@ -181,9 +184,9 @@ export class BuildController {
throw new Error(`Cannot get the repository information for current workspace folder(${err.message})`);
}

let originalRepositoryUrl = await this.opBuildAPIClient.getOriginalRepositoryUrl(localRepositoryUrl, buildUserToken, this.eventStream);
let originalRepositoryUrl = await this._opBuildAPIClient.getOriginalRepositoryUrl(localRepositoryUrl, buildUserToken, this._eventStream);

this.eventStream.post(new RepositoryInfoRetrieved(localRepositoryUrl, originalRepositoryUrl));
this._eventStream.post(new RepositoryInfoRetrieved(localRepositoryUrl, originalRepositoryUrl));
return [localRepositoryUrl, originalRepositoryUrl];
}
}
68 changes: 34 additions & 34 deletions src/build/buildExecutor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,44 +17,44 @@ import config from '../config';
import TelemetryReporter from '../telemetryReporter';

export class BuildExecutor {
private cwd: string;
private binary: string;
private runningChildProcess: ChildProcess;
private static skipRestore: boolean = false;
private _cwd: string;
private _binary: string;
private _runningChildProcess: ChildProcess;
private static SKIP_RESTORE: boolean = false;

constructor(
context: ExtensionContext,
private platformInfo: PlatformInformation,
private environmentController: EnvironmentController,
private eventStream: EventStream,
private telemetryReporter: TelemetryReporter
private _platformInfo: PlatformInformation,
private _environmentController: EnvironmentController,
private _eventStream: EventStream,
private _telemetryReporter: TelemetryReporter
) {
let runtimeDependencies = <Package[]>context.packageJson.runtimeDependencies;
let buildPackage = runtimeDependencies.find((pkg: Package) => pkg.name === 'docfx' && pkg.rid === this.platformInfo.rid);
let buildPackage = runtimeDependencies.find((pkg: Package) => pkg.name === 'docfx' && pkg.rid === this._platformInfo.rid);
let absolutePackage = AbsolutePathPackage.getAbsolutePathPackage(buildPackage, context.extensionPath);
this.cwd = absolutePackage.installPath.value;
this.binary = absolutePackage.binary;
this._cwd = absolutePackage.installPath.value;
this._binary = absolutePackage.binary;
}

public async RunBuild(correlationId: string, input: BuildInput, buildUserToken: string): Promise<BuildResult> {
let buildResult = <BuildResult>{
result: DocfxExecutionResult.Succeeded,
isRestoreSkipped: BuildExecutor.skipRestore
isRestoreSkipped: BuildExecutor.SKIP_RESTORE
};

let outputPath = path.join(input.localRepositoryPath, OUTPUT_FOLDER_NAME);
fs.emptyDirSync(outputPath);

let [envs, stdinInput] = this.getBuildParameters(correlationId, input, buildUserToken);

if (!BuildExecutor.skipRestore) {
if (!BuildExecutor.SKIP_RESTORE) {
let restoreStart = Date.now();
let result = await this.restore(correlationId, input.localRepositoryPath, outputPath, envs, stdinInput);
if (result !== 'Succeeded') {
buildResult.result = result;
return buildResult;
}
BuildExecutor.skipRestore = true;
BuildExecutor.SKIP_RESTORE = true;
buildResult.restoreTimeInSeconds = getDurationInSeconds(Date.now() - restoreStart);
}

Expand All @@ -65,12 +65,12 @@ export class BuildExecutor {
}

public async cancelBuild() {
if (this.runningChildProcess) {
this.runningChildProcess.kill('SIGKILL');
if (this.platformInfo.isWindows()) {
if (this._runningChildProcess) {
this._runningChildProcess.kill('SIGKILL');
if (this._platformInfo.isWindows()) {
// For Windows, grand child process will still keep running even parent process has been killed.
// So we need to kill them manually
await killProcessTree(this.runningChildProcess.pid);
await killProcessTree(this._runningChildProcess.pid);
}
}
}
Expand All @@ -79,15 +79,15 @@ export class BuildExecutor {
let envs: any = {
'DOCFX_CORRELATION_ID': correlationId,
'DOCFX_REPOSITORY_URL': input.originalRepositoryUrl,
'DOCS_ENVIRONMENT': this.environmentController.env
'DOCS_ENVIRONMENT': this._environmentController.env
};
if (this.telemetryReporter.getUserOptIn()) {
if (this._telemetryReporter.getUserOptIn()) {
// TODO: docfx need to support more common properties, e.g. if it is local build or server build
envs['APPINSIGHTS_INSTRUMENTATIONKEY'] = config.AIKey[this.environmentController.env];
envs['APPINSIGHTS_INSTRUMENTATIONKEY'] = config.AIKey[this._environmentController.env];
}

let secrets = <any>{
[`${extensionConfig.OPBuildAPIEndPoint[this.environmentController.env]}`]: {
[`${extensionConfig.OPBuildAPIEndPoint[this._environmentController.env]}`]: {
"headers": {
"X-OP-BuildUserToken": buildUserToken
}
Expand All @@ -113,11 +113,11 @@ export class BuildExecutor {
envs: any,
stdinInput: string): Promise<DocfxExecutionResult> {
return new Promise((resolve, reject) => {
this.eventStream.post(new DocfxRestoreStarted());
let command = `${this.binary} restore "${repositoryPath}" --legacy --output "${outputPath}" --stdin`;
this.runningChildProcess = executeDocfx(
this._eventStream.post(new DocfxRestoreStarted());
let command = `${this._binary} restore "${repositoryPath}" --legacy --output "${outputPath}" --stdin`;
this._runningChildProcess = executeDocfx(
command,
this.eventStream,
this._eventStream,
(code: number, signal: string) => {
let docfxExecutionResult: DocfxExecutionResult;
if (signal === 'SIGKILL') {
Expand All @@ -127,10 +127,10 @@ export class BuildExecutor {
} else {
docfxExecutionResult = DocfxExecutionResult.Failed;
}
this.eventStream.post(new DocfxRestoreCompleted(correlationId, docfxExecutionResult, code));
this._eventStream.post(new DocfxRestoreCompleted(correlationId, docfxExecutionResult, code));
resolve(docfxExecutionResult);
},
{ env: envs, cwd: this.cwd },
{ env: envs, cwd: this._cwd },
stdinInput
);
});
Expand All @@ -142,11 +142,11 @@ export class BuildExecutor {
envs: any,
stdinInput: string): Promise<DocfxExecutionResult> {
return new Promise((resolve, reject) => {
this.eventStream.post(new DocfxBuildStarted());
let command = `${this.binary} build "${repositoryPath}" --legacy --dry-run --output "${outputPath}" --stdin`;
this.runningChildProcess = executeDocfx(
this._eventStream.post(new DocfxBuildStarted());
let command = `${this._binary} build "${repositoryPath}" --legacy --dry-run --output "${outputPath}" --stdin`;
this._runningChildProcess = executeDocfx(
command,
this.eventStream,
this._eventStream,
(code: number, signal: string) => {
let docfxExecutionResult: DocfxExecutionResult;
if (signal === 'SIGKILL') {
Expand All @@ -156,10 +156,10 @@ export class BuildExecutor {
} else {
docfxExecutionResult = DocfxExecutionResult.Failed;
}
this.eventStream.post(new DocfxBuildCompleted(docfxExecutionResult, code));
this._eventStream.post(new DocfxBuildCompleted(docfxExecutionResult, code));
resolve(docfxExecutionResult);
},
{ env: envs, cwd: this.cwd },
{ env: envs, cwd: this._cwd },
stdinInput
);
});
Expand Down
4 changes: 2 additions & 2 deletions src/build/opBuildAPIClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const OP_BUILD_USER_TOKEN_HEADER_NAME = 'X-OP-BuildUserToken';
type RequestMethod = 'POST' | 'GET';

export class OPBuildAPIClient {
constructor(private environmentController: EnvironmentController) { }
constructor(private _environmentController: EnvironmentController) { }

public async getOriginalRepositoryUrl(gitRepoUrl: string, opBuildUserToken: string, eventStream: EventStream): Promise<string> {
const requestUrl = `${this.APIBaseUrl}/v2/Repositories/OriginalRepositoryUrl`
Expand All @@ -27,7 +27,7 @@ export class OPBuildAPIClient {
}

private get APIBaseUrl() {
return extensionConfig.OPBuildAPIEndPoint[this.environmentController.env];
return extensionConfig.OPBuildAPIEndPoint[this._environmentController.env];
}

private async sendRequest(
Expand Down
Loading