Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

Commit 303a556

Browse files
mkevinosullivanmkevinosullivan
committed
Updates from PR feedback (round 2)
1 parent 4a0647e commit 303a556

File tree

10 files changed

+107
-55
lines changed

10 files changed

+107
-55
lines changed

‎src/auth/session/session_storage.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ interface SessionStorage {
3737
*
3838
* @param shop shop of the session(s) to return
3939
*/
40-
findSessionsByShop?(
41-
shop: string,
42-
): Promise<SessionInterface[] | {[key: string]: unknown}[]>;
40+
findSessionsByShop?(shop: string): Promise<SessionInterface[]>;
4341
}
4442

4543
export {SessionStorage};

‎src/auth/session/storage/__tests__/battery-of-tests.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export function batteryOfTests(storageFactory: () => Promise<SessionStorage>) {
124124
if (shop1Sessions) {
125125
expect(shop1Sessions.length).toBe(2);
126126
expect(
127-
sessionArraysEqual(shop1Sessions as SessionInterface[], [
127+
sessionArraysEqual(shop1Sessions, [
128128
sessions[0] as SessionInterface,
129129
sessions[2] as SessionInterface,
130130
]),
@@ -155,9 +155,7 @@ export function batteryOfTests(storageFactory: () => Promise<SessionStorage>) {
155155
if (shop1Sessions) {
156156
expect(shop1Sessions.length).toBe(2);
157157
const idsToDelete = shop1Sessions.map((session) => session.id);
158-
await expect(
159-
storage.deleteSessions(idsToDelete as string[]),
160-
).resolves.toBe(true);
158+
await expect(storage.deleteSessions(idsToDelete)).resolves.toBe(true);
161159
shop1Sessions = await storage.findSessionsByShop(
162160
'delete-shop1-sessions',
163161
);

‎src/auth/session/storage/__tests__/custom.test.ts

+84-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {CustomSessionStorage} from '../custom';
33
import {SessionStorageError} from '../../../../error';
44

55
describe('custom session storage', () => {
6-
test('can perform actions', async () => {
6+
test('can perform core actions', async () => {
77
const sessionId = 'test_session';
88
let session: Session | undefined = new Session(
99
sessionId,
@@ -56,6 +56,89 @@ describe('custom session storage', () => {
5656
expect(deleteCalled).toBe(true);
5757
});
5858

59+
test('can perform optional actions', async () => {
60+
const prefix = 'custom_sessions';
61+
let sessions = [
62+
new Session(`${prefix}_1`, 'shop1-sessions', 'state', true),
63+
new Session(`${prefix}_2`, 'shop2-sessions', 'state', true),
64+
new Session(`${prefix}_3`, 'shop1-sessions', 'state', true),
65+
new Session(`${prefix}_4`, 'shop3-sessions', 'state', true),
66+
];
67+
68+
let deleteSessionsCalled = false;
69+
let findSessionsByShopCalled = false;
70+
const storage = new CustomSessionStorage(
71+
() => {
72+
return Promise.resolve(true);
73+
},
74+
() => {
75+
return Promise.resolve(sessions[0]);
76+
},
77+
() => {
78+
return Promise.resolve(true);
79+
},
80+
() => {
81+
deleteSessionsCalled = true;
82+
sessions = [sessions[1], sessions[3]];
83+
return Promise.resolve(true);
84+
},
85+
() => {
86+
findSessionsByShopCalled = true;
87+
if (deleteSessionsCalled) {
88+
return Promise.resolve([]);
89+
} else {
90+
return Promise.resolve([sessions[0], sessions[2]]);
91+
}
92+
},
93+
);
94+
95+
await expect(
96+
storage.findSessionsByShop('shop1_sessinons'),
97+
).resolves.toEqual([sessions[0], sessions[2]]);
98+
expect(findSessionsByShopCalled).toBe(true);
99+
100+
await expect(
101+
storage.deleteSessions([`${prefix}_1`, `${prefix}_3`]),
102+
).resolves.toBe(true);
103+
expect(deleteSessionsCalled).toBe(true);
104+
expect(sessions.length).toBe(2);
105+
await expect(
106+
storage.findSessionsByShop('shop1_sessinons'),
107+
).resolves.toEqual([]);
108+
});
109+
110+
test('missing optional actions generate warnings', async () => {
111+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
112+
const prefix = 'custom_sessions';
113+
const session = new Session(`${prefix}_1`, 'shop1-sessions', 'state', true);
114+
115+
const storage = new CustomSessionStorage(
116+
() => {
117+
return Promise.resolve(true);
118+
},
119+
() => {
120+
return Promise.resolve(session);
121+
},
122+
() => {
123+
return Promise.resolve(true);
124+
},
125+
);
126+
127+
await expect(
128+
storage.findSessionsByShop('shop1_sessinons'),
129+
).resolves.toEqual([]);
130+
expect(warnSpy).toHaveBeenCalledWith(
131+
expect.stringContaining('findSessionsByShopCallback not defined.'),
132+
);
133+
134+
await expect(
135+
storage.deleteSessions([`${prefix}_1`, `${prefix}_3`]),
136+
).resolves.toBe(false);
137+
expect(warnSpy).toHaveBeenCalledWith(
138+
expect.stringContaining('deleteSessionsCallback not defined.'),
139+
);
140+
});
141+
59142
test('failures and exceptions are raised', () => {
60143
const sessionId = 'test_session';
61144
const session = new Session(sessionId, 'shop-url', 'state', true);

‎src/auth/session/storage/custom.ts

+13-26
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,26 @@ import * as ShopifyErrors from '../../../error';
55

66
export class CustomSessionStorage implements SessionStorage {
77
constructor(
8-
readonly storeSessionCallback: (
9-
session: SessionInterface,
10-
) => Promise<boolean>,
11-
readonly loadSessionCallback: (
8+
readonly storeCallback: (session: SessionInterface) => Promise<boolean>,
9+
readonly loadCallback: (
1210
id: string,
1311
) => Promise<SessionInterface | {[key: string]: unknown} | undefined>,
14-
readonly deleteSessionCallback: (id: string) => Promise<boolean>,
12+
readonly deleteCallback: (id: string) => Promise<boolean>,
1513
readonly deleteSessionsCallback?: (ids: string[]) => Promise<boolean>,
1614
readonly findSessionsByShopCallback?: (
1715
shop: string,
18-
) => Promise<SessionInterface[] | {[key: string]: unknown}[]>,
16+
) => Promise<SessionInterface[]>,
1917
) {
20-
this.storeSessionCallback = storeSessionCallback;
21-
this.loadSessionCallback = loadSessionCallback;
22-
this.deleteSessionCallback = deleteSessionCallback;
18+
this.storeCallback = storeCallback;
19+
this.loadCallback = loadCallback;
20+
this.deleteCallback = deleteCallback;
2321
this.deleteSessionsCallback = deleteSessionsCallback;
2422
this.findSessionsByShopCallback = findSessionsByShopCallback;
2523
}
2624

2725
public async storeSession(session: SessionInterface): Promise<boolean> {
2826
try {
29-
return await this.storeSessionCallback(session);
27+
return await this.storeCallback(session);
3028
} catch (error) {
3129
throw new ShopifyErrors.SessionStorageError(
3230
`CustomSessionStorage failed to store a session. Error Details: ${error}`,
@@ -37,7 +35,7 @@ export class CustomSessionStorage implements SessionStorage {
3735
public async loadSession(id: string): Promise<SessionInterface | undefined> {
3836
let result: SessionInterface | {[key: string]: unknown} | undefined;
3937
try {
40-
result = await this.loadSessionCallback(id);
38+
result = await this.loadCallback(id);
4139
} catch (error) {
4240
throw new ShopifyErrors.SessionStorageError(
4341
`CustomSessionStorage failed to load a session. Error Details: ${error}`,
@@ -78,7 +76,7 @@ export class CustomSessionStorage implements SessionStorage {
7876

7977
public async deleteSession(id: string): Promise<boolean> {
8078
try {
81-
return await this.deleteSessionCallback(id);
79+
return await this.deleteCallback(id);
8280
} catch (error) {
8381
throw new ShopifyErrors.SessionStorageError(
8482
`CustomSessionStorage failed to delete a session. Error Details: ${error}`,
@@ -103,28 +101,17 @@ export class CustomSessionStorage implements SessionStorage {
103101
return false;
104102
}
105103

106-
public async findSessionsByShop(
107-
shop: string,
108-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
109-
const sessions: SessionInterface[] = [];
104+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
105+
let sessions: SessionInterface[] = [];
110106

111107
if (this.findSessionsByShopCallback) {
112-
let results: SessionInterface[] | {[key: string]: unknown}[] = [];
113-
114108
try {
115-
results = await this.findSessionsByShopCallback(shop);
109+
sessions = await this.findSessionsByShopCallback(shop);
116110
} catch (error) {
117111
throw new ShopifyErrors.SessionStorageError(
118112
`CustomSessionStorage failed to find sessions by shop. Error Details: ${error}`,
119113
);
120114
}
121-
122-
if (results && results instanceof Array) {
123-
// loop through array and convert to SessionInterface
124-
results.forEach((element) => {
125-
sessions.push(element as SessionInterface);
126-
});
127-
}
128115
} else {
129116
console.warn(
130117
`CustomSessionStorage failed to find sessions by shop. Error Details: findSessionsByShopCallback not defined.`,

‎src/auth/session/storage/memory.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ export class MemorySessionStorage implements SessionStorage {
2525
return true;
2626
}
2727

28-
public async findSessionsByShop(
29-
shop: string,
30-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
28+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
3129
const results = Object.values(this.sessions).filter(
3230
(session) => session.shop === shop,
3331
);

‎src/auth/session/storage/mongodb.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,15 @@ export class MongoDBSessionStorage implements SessionStorage {
8080
return true;
8181
}
8282

83-
public async findSessionsByShop(
84-
shop: string,
85-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
83+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
8684
await this.ready;
8785

8886
const rawResults = await this.collection.find({shop}).toArray();
8987
if (!rawResults || rawResults?.length === 0) return [];
9088

91-
const results = rawResults.map((rawResult: any) =>
89+
return rawResults.map((rawResult: any) =>
9290
sessionFromEntries(Object.entries(rawResult)),
9391
);
94-
95-
return results ? results : [];
9692
}
9793

9894
public async disconnect(): Promise<void> {

‎src/auth/session/storage/mysql.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ export class MySQLSessionStorage implements SessionStorage {
9292
return true;
9393
}
9494

95-
public async findSessionsByShop(
96-
shop: string,
97-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
95+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
9896
await this.ready;
9997
const query = `
10098
SELECT * FROM ${this.options.sessionTableName}

‎src/auth/session/storage/postgresql.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,7 @@ export class PostgreSQLSessionStorage implements SessionStorage {
9898
return true;
9999
}
100100

101-
public async findSessionsByShop(
102-
shop: string,
103-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
101+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
104102
await this.ready;
105103
const query = `
106104
SELECT * FROM ${this.options.sessionTableName}

‎src/auth/session/storage/redis.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,7 @@ export class RedisSessionStorage implements SessionStorage {
7777
return true;
7878
}
7979

80-
public async findSessionsByShop(
81-
shop: string,
82-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
80+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
8381
await this.ready;
8482

8583
const keys = await this.client.keys('*');

‎src/auth/session/storage/sqlite.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ export class SQLiteSessionStorage implements SessionStorage {
7676
return true;
7777
}
7878

79-
public async findSessionsByShop(
80-
shop: string,
81-
): Promise<SessionInterface[] | {[key: string]: unknown}[]> {
79+
public async findSessionsByShop(shop: string): Promise<SessionInterface[]> {
8280
await this.ready;
8381
const query = `
8482
SELECT * FROM ${this.options.sessionTableName}

0 commit comments

Comments
 (0)