fix: Updated WrappedExpiringStorage to use timer.unref

* fix: updated WrappedExpiringStorage tests and timer.unref calls

* fix: removed finalizable configs and inheritors that only used timer

* fix: updated test function to test setSafeInterval and timer.unref
This commit is contained in:
zg009 2023-03-13 02:30:42 -05:00 committed by GitHub
parent 63fd062f16
commit b6faed0db3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 12 additions and 68 deletions

View File

@ -24,17 +24,6 @@
"relativePath": "/forgot-password/",
"source": { "@id": "urn:solid-server:default:KeyValueStorage" }
}
},
{
"comment": "Makes sure the expiring storage cleanup timer is stopped when the application needs to stop.",
"@id": "urn:solid-server:default:Finalizer",
"@type": "ParallelHandler",
"handlers": [
{
"@type": "FinalizableHandler",
"finalizable": { "@id": "urn:solid-server:default:ExpiringForgotPasswordStorage" }
}
]
}
]
}

View File

@ -17,17 +17,6 @@
"relativePath": "/idp/tokens/",
"source": { "@id": "urn:solid-server:default:KeyValueStorage" }
}
},
{
"comment": "Makes sure the expiring storage cleanup timer is stopped when the application needs to stop.",
"@id": "urn:solid-server:default:Finalizer",
"@type": "ParallelHandler",
"handlers": [
{
"@type": "FinalizableHandler",
"finalizable": { "@id": "urn:solid-server:default:ExpiringTokenStorage" }
}
]
}
]
}

View File

@ -10,17 +10,6 @@
"@type": "WrappedExpiringStorage",
"source": { "@type": "MemoryMapStorage" }
}
},
{
"comment": "Makes sure the expiring storage cleanup timer is stopped when the application needs to stop.",
"@id": "urn:solid-server:default:Finalizer",
"@type": "ParallelHandler",
"handlers": [
{
"@type": "FinalizableHandler",
"finalizable": { "@id": "urn:solid-server:default:ExpiringAclCache" }
}
]
}
]
}

View File

@ -1,4 +1,3 @@
import type { Finalizable } from '../../init/final/Finalizable';
import { getLoggerFor } from '../../logging/LogUtil';
import { InternalServerError } from '../../util/errors/InternalServerError';
import { setSafeInterval } from '../../util/TimerUtil';
@ -13,7 +12,7 @@ export type Expires<T> = { expires?: string; payload: T };
* Will delete expired entries when trying to get their value.
* Has a timer that will delete all expired data every hour (default value).
*/
export class WrappedExpiringStorage<TKey, TValue> implements ExpiringStorage<TKey, TValue>, Finalizable {
export class WrappedExpiringStorage<TKey, TValue> implements ExpiringStorage<TKey, TValue> {
protected readonly logger = getLoggerFor(this);
private readonly source: KeyValueStorage<TKey, Expires<TValue>>;
private readonly timer: NodeJS.Timeout;
@ -28,6 +27,7 @@ export class WrappedExpiringStorage<TKey, TValue> implements ExpiringStorage<TKe
'Failed to remove expired entries',
this.removeExpiredEntries.bind(this),
timeout * 60 * 1000);
this.timer.unref();
}
public async get(key: TKey): Promise<TValue | undefined> {
@ -121,11 +121,4 @@ export class WrappedExpiringStorage<TKey, TValue> implements ExpiringStorage<TKe
}
return result;
}
/**
* Stops the continuous cleanup timer.
*/
public async finalize(): Promise<void> {
clearInterval(this.timer);
}
}

View File

@ -121,7 +121,12 @@ describe('A WrappedExpiringStorage', (): void => {
// Disable interval function and simply check it was called with the correct parameters
// Otherwise it gets quite difficult to verify the async interval function gets executed
const mockInterval = jest.spyOn(global, 'setInterval');
mockInterval.mockImplementation(jest.fn());
// We only need to call the timer.unref() once when the object is created
const mockTimer = { unref: jest.fn() };
const mockFn = jest.fn().mockReturnValueOnce(mockTimer);
mockInterval.mockImplementationOnce(mockFn);
// Timeout of 1 minute
storage = new WrappedExpiringStorage(source, 1);
const data = [
@ -141,33 +146,12 @@ describe('A WrappedExpiringStorage', (): void => {
// Await the function that should have been executed by the interval
await (mockInterval.mock.calls[0][0] as () => Promise<void>)();
// Make sure timer.unref() is called on initialization
expect(mockTimer.unref).toHaveBeenCalledTimes(1);
// Make sure setSafeInterval has been called once as well
expect(mockFn).toHaveBeenCalledTimes(1);
expect(source.delete).toHaveBeenCalledTimes(1);
expect(source.delete).toHaveBeenLastCalledWith('key2');
mockInterval.mockRestore();
});
it('can stop the timer.', async(): Promise<void> => {
const mockInterval = jest.spyOn(global, 'setInterval');
const mockClear = jest.spyOn(global, 'clearInterval');
// Timeout of 1 minute
storage = new WrappedExpiringStorage(source, 1);
const data = [
[ 'key1', createExpires('data1', tomorrow) ],
[ 'key2', createExpires('data2', yesterday) ],
[ 'key3', createExpires('data3') ],
];
source.entries.mockImplementationOnce(function* (): any {
yield* data;
});
await expect(storage.finalize()).resolves.toBeUndefined();
// Make sure clearInterval was called with the interval timer
expect(mockClear.mock.calls).toHaveLength(1);
expect(mockClear.mock.calls[0]).toHaveLength(1);
expect(mockClear.mock.calls[0][0]).toBe(mockInterval.mock.results[0].value);
mockInterval.mockRestore();
mockClear.mockRestore();
});
});