Skip to content

Added Logger and RequestContextClasses #5

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

Merged
merged 2 commits into from
Apr 5, 2017
Merged

Conversation

rohitnarula7176
Copy link
Contributor

#4

@msftclas
Copy link

msftclas commented Apr 4, 2017

@rohitnarula7176,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@@ -2,14 +2,14 @@ namespace MSAL {
export class AccessTokenKey {
authority: string;
clientId: string;
user: User;
homeObjectId: string;
Copy link

Choose a reason for hiding this comment

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

rename this to userIdentifier. this applies everywhere.

this.nonce = Utils.Guid();
}
this.correlationId = Utils.Guid();
this.state = Utils.Guid();
Copy link

Choose a reason for hiding this comment

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

rename Utils.Guid to Utils.CreateNewGuid()

str.push('scope=' + encodeURIComponent(this.parseScope(scopes)));
str.push('client_id=' + encodeURIComponent(this.clientId));
str.push('redirect_uri=' + encodeURIComponent(this.redirectUri));
str.push('state=' + encodeURIComponent(this.state));
str.push('nonce=' + encodeURIComponent(this.nonce));
if (this.extraQueryParameters) {
if (this.extraQueryParameters)
Copy link

Choose a reason for hiding this comment

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

always use curly braces for readability and avoiding bugs. applies everywhere.

static get error(): string { return "msal.error"; }
static get loginRequest(): string { return "msal.login.request"; }
static get loginError(): string { return "msal.login.error"; }
static get renewStatus(): string { return "msal.token.renew.status"; }
Copy link

Choose a reason for hiding this comment

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

❤️ this change.

lib/Logger.ts Outdated
VERBOSE

export interface ILoggerCallback {
(level: LogLevel, message: string,containsPii:boolean): boolean;
Copy link

Choose a reason for hiding this comment

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

it should be void;

export class Logger {// Singleton Class
private static _instance: Logger;
private _correlationId: string;
get correlationId(): string { return this._correlationId; }
Copy link

Choose a reason for hiding this comment

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

we should have set property on the correlationID.

lib/Logger.ts Outdated
set level(logLevel: LogLevel) {
if (LogLevel[logLevel])
this._level = logLevel;
else throw new Error("Provide a valid value for level. Possibles range for logLevel is 0-3");
Copy link

Choose a reason for hiding this comment

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

this should be truly enum and not a number.

if (RequestContext._instance) {
return RequestContext._instance;
}
this._logger = new Logger(correlationId);
Copy link

Choose a reason for hiding this comment

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

add line break.


private _localCallback: ILoggerCallback;
get localCallback(): ILoggerCallback { return this._localCallback; }
set localCallback(localCallback: ILoggerCallback) {
Copy link

Choose a reason for hiding this comment

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

set should fail is callback is already set.

@@ -1,45 +1,63 @@
///<reference path='Storage.ts'/>

interface Window {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to new file


namespace MSAL {

interface User {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let this be in its own file.

@rohitnarula7176 rohitnarula7176 merged commit 71f65a3 into dev Apr 5, 2017
@rohitnarula7176 rohitnarula7176 deleted the rohitn/loggerAPI branch April 5, 2017 01:45
@sameerag sameerag mentioned this pull request Jun 26, 2020
1 task
sangonzal added a commit that referenced this pull request Jun 30, 2020
tnorling added a commit that referenced this pull request Sep 30, 2020
Broker #5: Move initialization out of the constructor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants