Skip to content

Commit d67c00d

Browse files
sam-gcsam-gc
committed
Feedback, tests
1 parent d17197e commit d67c00d

File tree

3 files changed

+104
-49
lines changed

3 files changed

+104
-49
lines changed

‎packages-exp/auth-exp/src/core/strategies/abstract_popup_redirect_action.test.ts renamed to ‎packages-exp/auth-exp/src/core/strategies/abstract_popup_redirect_operation.test.ts

Lines changed: 88 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { AuthEventManager } from '../auth/auth_event_manager';
3535
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
3636
import { UserCredentialImpl } from '../user/user_credential_impl';
3737
import { _getInstance } from '../util/instantiator';
38-
import { AbstractPopupRedirectAction } from './abstract_popup_redirect_action';
38+
import { AbstractPopupRedirectOperation } from './abstract_popup_redirect_operation';
3939
import * as idp from './idp';
4040

4141
use(sinonChai);
@@ -48,13 +48,13 @@ const ERROR = AUTH_ERROR_FACTORY.create(AuthErrorCode.INTERNAL_ERROR, {
4848
/**
4949
* A real class is needed to instantiate the action
5050
*/
51-
class WrapperAction extends AbstractPopupRedirectAction {
51+
class WrapperOperation extends AbstractPopupRedirectOperation {
5252
eventId = '100';
5353
onExecution = sinon.stub().returns(Promise.resolve());
5454
cleanUp = sinon.stub();
5555
}
5656

57-
describe('src/core/strategies/abstract_popup_redirect_action', () => {
57+
describe('src/core/strategies/abstract_popup_redirect_operation', () => {
5858
let auth: Auth;
5959
let resolver: PopupRedirectResolver;
6060
let eventManager: EventManager;
@@ -72,92 +72,147 @@ describe('src/core/strategies/abstract_popup_redirect_action', () => {
7272
});
7373

7474
context('#execute', () => {
75-
let action: WrapperAction;
75+
let operation: WrapperOperation;
7676

7777
beforeEach(() => {
78-
action = new WrapperAction(auth, AuthEventType.LINK_VIA_POPUP, resolver);
78+
operation = new WrapperOperation(auth, AuthEventType.LINK_VIA_POPUP, resolver);
7979
idpStubs._signIn.returns(Promise.resolve(new UserCredentialImpl(testUser(auth, 'uid'), null, OperationType.SIGN_IN)));
8080
});
8181

8282
/** Finishes out the promise */
8383
function finishPromise(outcome: AuthEvent|FirebaseError): void {
8484
delay((): void => {
8585
if (outcome instanceof FirebaseError) {
86-
action.onError(outcome);
86+
operation.onError(outcome);
8787
} else {
8888
// eslint-disable-next-line @typescript-eslint/no-floating-promises
89-
action.onAuthEvent(outcome);
89+
operation.onAuthEvent(outcome);
9090
}
9191
});
9292
}
9393

9494
it('initializes the resolver', async () => {
9595
sinon.spy(resolver, '_initialize');
96-
const promise = action.execute();
96+
const promise = operation.execute();
9797
finishPromise(authEvent());
9898
await promise;
9999
expect(resolver._initialize).to.have.been.calledWith(auth);
100100
});
101101

102102
it('calls subclass onExecution', async () => {
103103
finishPromise(authEvent());
104-
await action.execute();
105-
expect(action.onExecution).to.have.been.called;
104+
await operation.execute();
105+
expect(operation.onExecution).to.have.been.called;
106106
});
107107

108108
it('registers and unregisters itself with the event manager', async () => {
109109
sinon.spy(eventManager, 'registerConsumer');
110110
sinon.spy(eventManager, 'unregisterConsumer');
111111
finishPromise(authEvent());
112-
await action.execute();
113-
expect(eventManager.registerConsumer).to.have.been.calledWith(action);
114-
expect(eventManager.unregisterConsumer).to.have.been.calledWith(action);
112+
await operation.execute();
113+
expect(eventManager.registerConsumer).to.have.been.calledWith(operation);
114+
expect(eventManager.unregisterConsumer).to.have.been.calledWith(operation);
115115
});
116116

117117
it('unregisters itself in case of error', async () => {
118118
sinon.spy(eventManager, 'unregisterConsumer');
119119
finishPromise(ERROR);
120-
try { await action.execute(); } catch {}
121-
expect(eventManager.unregisterConsumer).to.have.been.calledWith(action);
120+
try { await operation.execute(); } catch {}
121+
expect(eventManager.unregisterConsumer).to.have.been.calledWith(operation);
122122
});
123123

124124
it('emits the user credential returned from idp task', async () => {
125125
finishPromise(authEvent());
126-
const cred = await action.execute();
126+
const cred = await operation.execute();
127127
expect(cred.user.uid).to.eq('uid');
128128
expect(cred.credential).to.be.null;
129129
expect(cred.operationType).to.eq(OperationType.SIGN_IN);
130130
});
131131

132-
it('bubbles up any error', async (done) => {
132+
it('bubbles up any error', done => {
133133
finishPromise(ERROR);
134-
try {
135-
await action.execute();
136-
} catch (e) {
134+
operation.execute().catch(e => {
137135
expect(e).to.eq(ERROR);
138136
done();
139-
}
137+
});
138+
});
139+
140+
it('calls cleanUp on error', async () => {
141+
finishPromise(ERROR);
142+
try {
143+
await operation.execute();
144+
} catch {}
145+
expect(operation.cleanUp).to.have.been.called;
146+
});
147+
148+
it('calls cleanUp on success', async () => {
149+
finishPromise(authEvent());
150+
await operation.execute();
151+
expect(operation.cleanUp).to.have.been.called;
140152
});
141153

142154
context('idp tasks', () => {
143-
function updateFilter(type: AuthEventType) {
144-
(action as unknown as Record<string, unknown>).filter = type;
155+
function updateFilter(type: AuthEventType): void {
156+
(operation as unknown as Record<string, unknown>).filter = type;
145157
}
146158

147-
const expectedIdpTaskParams: idp.IdpTaskParams = {
148-
auth,
149-
requestUri: BASE_AUTH_EVENT.urlResponse!,
150-
sessionId: BASE_AUTH_EVENT.sessionId!,
151-
tenantId: BASE_AUTH_EVENT.tenantId || undefined,
152-
postBody: BASE_AUTH_EVENT.postBody || undefined,
153-
};
159+
function expectedIdpTaskParams(): idp.IdpTaskParams {
160+
return {
161+
auth,
162+
requestUri: BASE_AUTH_EVENT.urlResponse!,
163+
sessionId: BASE_AUTH_EVENT.sessionId!,
164+
tenantId: BASE_AUTH_EVENT.tenantId || undefined,
165+
postBody: BASE_AUTH_EVENT.postBody || undefined,
166+
user: undefined,
167+
};
168+
}
154169

155-
it('routes signInWithPopup', async () => {
170+
it('routes SIGN_IN_VIA_POPUP', async () => {
156171
const type = AuthEventType.SIGN_IN_VIA_POPUP;
157172
updateFilter(type);
158173
finishPromise(authEvent({type}));
159-
await action.execute();
160-
expect(idp._signIn).to.have.been.calledWith(expectedIdpTaskParams);
174+
await operation.execute();
175+
expect(idp._signIn).to.have.been.calledWith(expectedIdpTaskParams());
176+
});
177+
178+
it('routes SIGN_IN_VIA_REDIRECT', async () => {
179+
const type = AuthEventType.SIGN_IN_VIA_REDIRECT;
180+
updateFilter(type);
181+
finishPromise(authEvent({type}));
182+
await operation.execute();
183+
expect(idp._signIn).to.have.been.calledWith(expectedIdpTaskParams());
184+
});
185+
186+
it('routes LINK_VIA_POPUP', async () => {
187+
const type = AuthEventType.LINK_VIA_POPUP;
188+
updateFilter(type);
189+
finishPromise(authEvent({type}));
190+
await operation.execute();
191+
expect(idp._link).to.have.been.calledWith(expectedIdpTaskParams());
192+
});
193+
194+
it('routes LINK_VIA_REDIRECT', async () => {
195+
const type = AuthEventType.LINK_VIA_REDIRECT;
196+
updateFilter(type);
197+
finishPromise(authEvent({type}));
198+
await operation.execute();
199+
expect(idp._link).to.have.been.calledWith(expectedIdpTaskParams());
200+
});
201+
202+
it('routes REAUTH_VIA_POPUP', async () => {
203+
const type = AuthEventType.REAUTH_VIA_POPUP;
204+
updateFilter(type);
205+
finishPromise(authEvent({type}));
206+
await operation.execute();
207+
expect(idp._reauth).to.have.been.calledWith(expectedIdpTaskParams());
208+
});
209+
210+
it('routes REAUTH_VIA_REDIRECT', async () => {
211+
const type = AuthEventType.REAUTH_VIA_REDIRECT;
212+
updateFilter(type);
213+
finishPromise(authEvent({type}));
214+
await operation.execute();
215+
expect(idp._reauth).to.have.been.calledWith(expectedIdpTaskParams());
161216
});
162217
});
163218
});

‎packages-exp/auth-exp/src/core/strategies/abstract_popup_redirect_action.ts renamed to ‎packages-exp/auth-exp/src/core/strategies/abstract_popup_redirect_operation.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ interface PendingPromise {
3535
* Popup event manager. Handles the popup's entire lifecycle; listens to auth
3636
* events
3737
*/
38-
export abstract class AbstractPopupRedirectAction implements AuthEventConsumer {
38+
export abstract class AbstractPopupRedirectOperation implements AuthEventConsumer {
3939
private pendingPromise: PendingPromise | null = null;
4040
private eventManager: EventManager | null = null;
4141

4242
abstract eventId: string|null;
4343

4444
constructor(
4545
protected readonly auth: Auth,
46-
readonly filter: AuthEventType|'redirect',
46+
readonly filter: AuthEventType,
4747
protected readonly resolver: PopupRedirectResolver,
4848
protected user?: User
4949
) {
@@ -92,9 +92,9 @@ export abstract class AbstractPopupRedirectAction implements AuthEventConsumer {
9292
this.reject(error);
9393
}
9494

95-
// isMatchingEvent(eventId: string|null): boolean {
96-
// return !!eventId && this.eventId === eventId;
97-
// }
95+
isMatchingEvent(eventId: string|null): boolean {
96+
return !!eventId && this.eventId === eventId;
97+
}
9898

9999
private getIdpTask(type: AuthEventType): IdpTask {
100100
switch(type) {
@@ -124,7 +124,7 @@ export abstract class AbstractPopupRedirectAction implements AuthEventConsumer {
124124
this.unregisterAndCleanUp();
125125
}
126126

127-
private unregisterAndCleanUp() {
127+
private unregisterAndCleanUp(): void {
128128
if (this.eventManager) {
129129
this.eventManager.unregisterConsumer(this);
130130
}

‎packages-exp/auth-exp/src/core/strategies/popup.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { Delay } from '../util/delay';
2525
import { _generateEventId } from '../util/event_id';
2626
import { _getInstance } from '../util/instantiator';
2727
import { AuthPopup } from '../util/popup';
28-
import { AbstractPopupRedirectAction } from './abstract_popup_redirect_action';
28+
import { AbstractPopupRedirectOperation } from './abstract_popup_redirect_operation';
2929

3030
// The event timeout is the same on mobile and desktop, no need for Delay.
3131
export const _AUTH_EVENT_TIMEOUT = 2020;
@@ -39,7 +39,7 @@ export async function signInWithPopup(
3939
const auth = authExtern as Auth;
4040
const resolver: PopupRedirectResolver = _getInstance(resolverExtern);
4141

42-
const action = new PopupAction(
42+
const action = new PopupOperation(
4343
auth,
4444
AuthEventType.SIGN_IN_VIA_POPUP,
4545
provider,
@@ -56,7 +56,7 @@ export async function reauthenticateWithPopup(
5656
const user = userExtern as User;
5757
const resolver: PopupRedirectResolver = _getInstance(resolverExtern);
5858

59-
const action = new PopupAction(
59+
const action = new PopupOperation(
6060
user.auth,
6161
AuthEventType.REAUTH_VIA_POPUP,
6262
provider,
@@ -74,7 +74,7 @@ export async function linkWithPopup(
7474
const user = userExtern as User;
7575
const resolver: PopupRedirectResolver = _getInstance(resolverExtern);
7676

77-
const action = new PopupAction(
77+
const action = new PopupOperation(
7878
user.auth,
7979
AuthEventType.LINK_VIA_POPUP,
8080
provider,
@@ -88,10 +88,10 @@ export async function linkWithPopup(
8888
* Popup event manager. Handles the popup's entire lifecycle; listens to auth
8989
* events
9090
*/
91-
class PopupAction extends AbstractPopupRedirectAction {
91+
class PopupOperation extends AbstractPopupRedirectOperation {
9292
// Only one popup is ever shown at once. The lifecycle of the current popup
9393
// can be managed / cancelled by the constructor.
94-
private static currentPopupAction: PopupAction | null = null;
94+
private static currentPopupAction: PopupOperation | null = null;
9595
private authWindow: AuthPopup | null = null;
9696
private pollId: number | null = null;
9797

@@ -103,11 +103,11 @@ class PopupAction extends AbstractPopupRedirectAction {
103103
user?: User
104104
) {
105105
super(auth, filter, resolver, user);
106-
if (PopupAction.currentPopupAction) {
107-
PopupAction.currentPopupAction.cancel();
106+
if (PopupOperation.currentPopupAction) {
107+
PopupOperation.currentPopupAction.cancel();
108108
}
109109

110-
PopupAction.currentPopupAction = this;
110+
PopupOperation.currentPopupAction = this;
111111
}
112112

113113
async onExecution(): Promise<void> {
@@ -147,7 +147,7 @@ class PopupAction extends AbstractPopupRedirectAction {
147147

148148
this.authWindow = null;
149149
this.pollId = null;
150-
PopupAction.currentPopupAction = null;
150+
PopupOperation.currentPopupAction = null;
151151
}
152152

153153
private pollUserCancellation(appName: string): void {

0 commit comments

Comments
 (0)