Skip to content

Commit 9d4a450

Browse files
FabianGosebrinkFabianGosebrink
authored
Merge pull request #1237 from damienbod/fabiangosebrink/fixing-storage-issue
WIP - Fix browser storage issues
2 parents 64bc9c1 + adc3e1f commit 9d4a450

File tree

4 files changed

+54
-41
lines changed

4 files changed

+54
-41
lines changed

‎projects/angular-auth-oidc-client/src/lib/storage/browser-storage.service.spec.ts‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@ describe('Browser Service', () => {
3131
describe('read', () => {
3232
it('returns null if there is no storage', () => {
3333
spyOn(service as any, 'hasStorage').and.returnValue(false);
34-
expect(service.read('anything', 'configId')).toBeFalse();
34+
35+
expect(service.read('anything', 'configId')).toBeNull();
3536
});
3637

37-
it('returns false if getStorage returns null', () => {
38+
it('returns null if getStorage returns null', () => {
3839
spyOn(service as any, 'hasStorage').and.returnValue(true);
3940
spyOn(service as any, 'getStorage').and.returnValue(null);
40-
expect(service.read('anything', 'configId')).toBeFalsy();
41+
expect(service.read('anything', 'configId')).toBeNull();
4142
});
4243

4344
it('returns null if getItem returns null', () => {
@@ -49,7 +50,10 @@ describe('Browser Service', () => {
4950
return returnValue;
5051
},
5152
});
52-
expect(service.read('anything', 'configId')).toBeFalsy();
53+
54+
const result = service.read('anything', 'configId');
55+
56+
expect(result).toBeNull();
5357
});
5458

5559
it('returns the item if getItem returns an item', () => {
@@ -61,52 +65,54 @@ describe('Browser Service', () => {
6165
return returnValue;
6266
},
6367
});
64-
expect(service.read('anything', 'configId')).toEqual(JSON.parse(returnValue));
68+
69+
const result = service.read('anything', 'configId');
70+
71+
expect(result).toEqual(JSON.parse(returnValue));
6572
});
6673
});
6774

6875
describe('write', () => {
69-
it('returns null if there is no storage', () => {
76+
it('returns false if there is no storage', () => {
7077
spyOn(service as any, 'hasStorage').and.returnValue(false);
71-
expect(service.write('anyKey', 'anyvalue', 'configId')).toBeFalse();
78+
79+
expect(service.write('configId', 'anyvalue')).toBeFalse();
7280
});
7381

74-
it('returns null if getStorage returns null', () => {
82+
it('returns false if getStorage returns null', () => {
7583
spyOn(service as any, 'hasStorage').and.returnValue(true);
7684
spyOn(service as any, 'getStorage').and.returnValue(null);
77-
expect(service.write('anyKey', 'anyvalue', 'configId')).toBeFalse();
85+
86+
expect(service.write('configId', 'anyvalue')).toBeFalse();
7887
});
7988

80-
it('returns undefined if setItem gets called correctly', () => {
89+
it('writes object correctly with configId', () => {
8190
spyOn(service as any, 'hasStorage').and.returnValue(true);
82-
8391
const serviceObject = {
8492
write: (a, b) => {},
8593
};
94+
const writeSpy = spyOn(serviceObject, 'write').and.callThrough();
95+
spyOn(service as any, 'getStorage').and.returnValue(serviceObject);
8696

87-
const setItemSpy = spyOn(serviceObject, 'write').and.callThrough();
97+
const result = service.write({ anyKey: 'anyvalue' }, 'configId');
8898

89-
spyOn(service as any, 'getStorage').and.returnValue(serviceObject);
90-
const result = service.write('anyKey', 'anyvalue', 'configId');
9199
expect(result).toBe(true);
92-
expect(setItemSpy).toHaveBeenCalledWith('anyKey', JSON.stringify('anyvalue'));
100+
expect(writeSpy).toHaveBeenCalledWith('configId', JSON.stringify({ anyKey: 'anyvalue' }));
93101
});
94102

95103
it('writes null if item is falsy', () => {
96104
spyOn(service as any, 'hasStorage').and.returnValue(true);
97-
98105
const serviceObject = {
99106
write: (a, b) => {},
100107
};
101-
102-
const setItemSpy = spyOn(serviceObject, 'write').and.callThrough();
103-
108+
const writeSpy = spyOn(serviceObject, 'write').and.callThrough();
104109
const somethingFalsy = '';
105-
106110
spyOn(service as any, 'getStorage').and.returnValue(serviceObject);
107-
const result = service.write('anyKey', somethingFalsy, 'configId');
111+
112+
const result = service.write(somethingFalsy, 'configId');
113+
108114
expect(result).toBe(true);
109-
expect(setItemSpy).toHaveBeenCalledWith('anyKey', JSON.stringify(null));
115+
expect(writeSpy).toHaveBeenCalledWith('configId', JSON.stringify(null));
110116
});
111117
});
112118

@@ -119,20 +125,19 @@ describe('Browser Service', () => {
119125
it('returns false if getStorage returns null', () => {
120126
spyOn(service as any, 'hasStorage').and.returnValue(true);
121127
spyOn(service as any, 'getStorage').and.returnValue(null);
122-
expect(service.remove('anything', 'configId')).toBeFalsy();
128+
expect(service.remove('anything', 'configId')).toBeFalse();
123129
});
124130

125131
it('returns true if removeItem is called', () => {
126132
spyOn(service as any, 'hasStorage').and.returnValue(true);
127-
128133
const serviceObject = {
129134
remove: (a) => {},
130135
};
131-
132136
const setItemSpy = spyOn(serviceObject, 'remove').and.callThrough();
133-
134137
spyOn(service as any, 'getStorage').and.returnValue(serviceObject);
138+
135139
const result = service.remove('anyKey', 'configId');
140+
136141
expect(result).toBe(true);
137142
expect(setItemSpy).toHaveBeenCalledWith('anyKey');
138143
});
@@ -148,7 +153,7 @@ describe('Browser Service', () => {
148153
spyOn(service as any, 'hasStorage').and.returnValue(true);
149154
spyOn(service as any, 'getStorage').and.returnValue(null);
150155

151-
expect(service.clear('configId')).toBeFalsy();
156+
expect(service.clear('configId')).toBeFalse();
152157
});
153158

154159
it('returns true if clear is called', () => {

‎projects/angular-auth-oidc-client/src/lib/storage/browser-storage.service.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,43 @@ export class BrowserStorageService {
1111
if (!this.hasStorage()) {
1212
this.loggerService.logDebug(configId, `Wanted to read '${key}' but Storage was undefined`);
1313

14-
return false;
14+
return null;
15+
}
16+
17+
const storage = this.getStorage(configId);
18+
19+
if (!storage) {
20+
this.loggerService.logDebug(configId, `Wanted to read config for '${configId}' but Storage was falsy`);
21+
22+
return null;
1523
}
1624

17-
const item = this.getStorage(configId)?.read(key);
25+
const storedConfig = storage.read(configId);
1826

19-
if (!item) {
27+
if (!storedConfig) {
2028
return null;
2129
}
2230

23-
return JSON.parse(item);
31+
return JSON.parse(storedConfig);
2432
}
2533

26-
write(key: string, value: any, configId: string): boolean {
34+
write(value: any, configId: string): boolean {
2735
if (!this.hasStorage()) {
28-
this.loggerService.logDebug(configId, `Wanted to write '${key}/${value}' but Storage was falsy`);
36+
this.loggerService.logDebug(configId, `Wanted to write '${value}' but Storage was falsy`);
2937

3038
return false;
3139
}
3240

3341
const storage = this.getStorage(configId);
3442
if (!storage) {
35-
this.loggerService.logDebug(configId, `Wanted to write '${key}/${value}' but Storage was falsy`);
43+
this.loggerService.logDebug(configId, `Wanted to write '${value}' but Storage was falsy`);
3644

3745
return false;
3846
}
3947

4048
value = value || null;
4149

42-
storage.write(`${key}`, JSON.stringify(value));
50+
storage.write(configId, JSON.stringify(value));
4351

4452
return true;
4553
}
@@ -58,7 +66,7 @@ export class BrowserStorageService {
5866
return false;
5967
}
6068

61-
storage.remove(`${key}`);
69+
storage.remove(key);
6270

6371
return true;
6472
}

‎projects/angular-auth-oidc-client/src/lib/storage/storage-persistence.service.spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('Storage Persistence Service', () => {
4444
service.write('authNonce', 'anyValue', 'configId');
4545

4646
expect(readSpy).toHaveBeenCalledWith('authNonce', 'configId');
47-
expect(writeSpy).toHaveBeenCalledWith('authNonce', { authNonce: 'anyValue' }, 'configId');
47+
expect(writeSpy).toHaveBeenCalledWith({ authNonce: 'anyValue' }, 'configId');
4848
});
4949
});
5050

@@ -56,7 +56,7 @@ describe('Storage Persistence Service', () => {
5656
service.remove('authNonce', 'configId');
5757

5858
expect(readSpy).toHaveBeenCalledWith('authNonce', 'configId');
59-
expect(writeSpy).toHaveBeenCalledWith('authNonce', {}, 'configId');
59+
expect(writeSpy).toHaveBeenCalledWith({}, 'configId');
6060
});
6161

6262
it('does not crash when read with configId returns null', () => {
@@ -66,7 +66,7 @@ describe('Storage Persistence Service', () => {
6666
service.remove('authNonce', 'configId');
6767

6868
expect(readSpy).toHaveBeenCalledWith('authNonce', 'configId');
69-
expect(writeSpy).toHaveBeenCalledWith('authNonce', {}, 'configId');
69+
expect(writeSpy).toHaveBeenCalledWith({}, 'configId');
7070
});
7171
});
7272

‎projects/angular-auth-oidc-client/src/lib/storage/storage-persistence.service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ export class StoragePersistenceService {
3333
const storedConfig = this.browserStorageService.read(key, configId) || {};
3434

3535
storedConfig[key] = value;
36-
this.browserStorageService.write(key, storedConfig, configId);
36+
this.browserStorageService.write(storedConfig, configId);
3737
}
3838

3939
remove(key: StorageKeys, configId: string): void {
4040
const storedConfig = this.browserStorageService.read(key, configId) || {};
4141

4242
delete storedConfig[key];
4343

44-
this.browserStorageService.write(key, storedConfig, configId);
44+
this.browserStorageService.write(storedConfig, configId);
4545
}
4646

4747
clear(configId: string): void {

0 commit comments

Comments
 (0)