Skip to content

Commit

Permalink
feat: customizable cache implementation (#2527)
Browse files Browse the repository at this point in the history
* customizable cache implementation

* remove console

* jsdoc

* lint

* review remarks

* public api check

* Cache methods return promise

* rename interface

* lint

* new destination interface

* update jsdoc + other fixes

* review remarks

* review remarks

* remove generic because we initialize with the DefaultCacheType so varialbe would be CacheType<any> and then you can leave it out.

* add chagelog

Co-authored-by: d068544 <[email protected]>
  • Loading branch information
deekshas8 and FrankEssenberger authored Jun 15, 2022
1 parent ffeeb94 commit 0909460
Show file tree
Hide file tree
Showing 14 changed files with 347 additions and 153 deletions.
5 changes: 5 additions & 0 deletions .changeset/long-tables-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-cloud-sdk/connectivity': minor
---

[New Functionality] Add interface `DestinationCacheInterface` and method `setDestinationCache` to support implementation of custom destination cache.
5 changes: 4 additions & 1 deletion packages/connectivity/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export {
parseDestination,
toDestinationNameUrl,
sanitizeDestination,
DestinationCacheInterface,
CacheEntry,
CachingOptions,
getDestination,
useOrFetchDestination,
Expand Down Expand Up @@ -35,7 +37,8 @@ export {
buildHeadersForDestination,
getClientCredentialsToken,
getUserToken,
registerDestination
registerDestination,
setDestinationCache
} from './scp-cf';

export type {
Expand Down
26 changes: 14 additions & 12 deletions packages/connectivity/src/scp-cf/cache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,26 @@ const cacheOne = new Cache<Destination>({ hours: 0, minutes: 5, seconds: 0 });
const cacheTwo = new Cache<ClientCredentialsResponse>();

describe('Cache', () => {
afterEach(() => {
afterEach(async () => {
cacheOne.clear();
cacheTwo.clear();
destinationCache.clear();
await destinationCache.clear();
clientCredentialsTokenCache.clear();
});

it('non-existing item in cache should return undefined', () => {
const actual = cacheOne.get('notExistingDest');
expect(actual).toBeUndefined();
it('non-existing item in cache should return undefined', async () => {
expect(cacheOne.get('notExistingDest')).toBeUndefined();
});

it('item should be retrieved correctly', () => {
cacheOne.set('one', destinationOne);
cacheOne.set('one', { entry: destinationOne });
const actual = cacheOne.get('one');
expect(actual).toEqual(destinationOne);
});

it('retrieving expired item should return undefined', () => {
jest.useFakeTimers('modern');
cacheOne.set('one', destinationOne);
cacheOne.set('one', { entry: destinationOne });

const minutesToExpire = 6;
// Shift time to expire the set item
Expand All @@ -56,7 +55,7 @@ describe('Cache', () => {
});

it('clear() should remove all entries in cache', () => {
cacheOne.set('one', destinationOne);
cacheOne.set('one', { entry: destinationOne });
cacheOne.clear();
expect(cacheOne.hasKey('one')).toBeFalsy();
});
Expand All @@ -69,7 +68,7 @@ describe('Cache', () => {
jti: '',
scope: ''
};
cacheTwo.set('someToken', dummyToken);
cacheTwo.set('someToken', { entry: dummyToken });
expect(cacheTwo.get('someToken')).toEqual(dummyToken);
});

Expand All @@ -81,14 +80,17 @@ describe('Cache', () => {
jti: '',
scope: ''
};
cacheTwo.set('expiredToken', dummyToken, 10);
cacheTwo.set('validToken', dummyToken, Date.now() + 5000);
cacheTwo.set('expiredToken', { entry: dummyToken, expires: 10 });
cacheTwo.set('validToken', {
entry: dummyToken,
expires: Date.now() + 5000
});
expect(cacheTwo.get('expiredToken')).toBeUndefined();
expect(cacheTwo.get('validToken')).toBe(dummyToken);
});

it('should not hit cache for undefined key', () => {
cacheOne.set(undefined, {} as Destination);
cacheOne.set(undefined, { entry: {} as Destination });
const actual = cacheOne.get(undefined);
expect(actual).toBeUndefined();
});
Expand Down
24 changes: 13 additions & 11 deletions packages/connectivity/src/scp-cf/cache.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
interface CacheInterface<T> {
hasKey(key: string): boolean;
get(key: string): T | undefined;
set(key: string, item: T, expirationTime?: number): void;
get(key: string | undefined): T | undefined;
set(key: string | undefined, item: CacheEntry<T>): void;
clear(): void;
}

interface DateInputObject {
/**
* @internal
*/
export interface DateInputObject {
hours?: number;
minutes?: number;
seconds?: number;
milliseconds?: number;
}

/**
* @internal
* Respresentation of a cached object.
*/
export interface CacheEntry<T> {
expires?: number;
Expand Down Expand Up @@ -81,15 +85,13 @@ export class Cache<T> implements CacheInterface<T> {
/**
* Setter of entries in cache.
* @param key - The entry's key.
* @param entry - The entry to cache.
* @param expirationTime - The time expressed in UTC in which the given entry expires.
* @param item - The entry to cache.
*/
set(key: string | undefined, entry: T, expirationTime?: number): void {
set(key: string | undefined, item: CacheEntry<T>): void {
if (key) {
const expires = expirationTime
? expirationTime
: inferExpirationTime(this.defaultValidityTime);
this.cache[key] = { entry, expires };
const expires =
item.expires ?? inferExpirationTime(this.defaultValidityTime);
this.cache[key] = { entry: item.entry, expires };
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ const ClientCredentialsTokenCache = (
clientId: string,
token: ClientCredentialsResponse
): void => {
cache.set(
getCacheKey(url, clientId),
token,
token.expires_in ? Date.now() + token.expires_in * 1000 : undefined
);
cache.set(getCacheKey(url, clientId), {
entry: token,
expires: token.expires_in
? Date.now() + token.expires_in * 1000
: undefined
});
},
clear: (): void => {
cache.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('destination loading precedence', () => {

it('retrieves service binding destinations third', async () => {
delete process.env['destinations'];
registerDestinationCache.clear();
await registerDestinationCache.clear();
const actual = await getDestination({
destinationName
});
Expand All @@ -92,7 +92,7 @@ describe('destination loading precedence', () => {

it('retrieves destinations from destination-service last', async () => {
delete process.env['destinations'];
registerDestinationCache.clear();
await registerDestinationCache.clear();
mockServiceBindings();

await expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function getDestination(
): Promise<Destination | null> {
return (
searchEnvVariablesForDestination(options) ||
searchRegisteredDestination(options) ||
(await searchRegisteredDestination(options)) ||
searchServiceBindingForDestination(options.destinationName) ||
getDestinationFromDestinationService(options)
);
Expand Down
Loading

0 comments on commit 0909460

Please sign in to comment.