Skip to content

Commit

Permalink
perf: Base storage only touches and get tags if tag list is not empty
Browse files Browse the repository at this point in the history
Additionally fixed all eslint issues and removed any type.

BREAKING CHANGE: fixed typings for get/set and managers. Throw errors if executor returns undefined. Executor should always return value or null - for emptiness

fix: Remove undefined as get return type.

Also removed `E extends Executor<R>` type parameter.

`Record.value` is always defined.

WriteOptions now has type parameter used in getTags signature.

Throw an error if executor returns undefined.
  • Loading branch information
alexkvak authored and drwatsno committed May 8, 2020
1 parent 490acec commit 22b8d3a
Show file tree
Hide file tree
Showing 33 changed files with 217 additions and 195 deletions.
12 changes: 9 additions & 3 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
{
"parser": "@typescript-eslint/parser",
"plugins": [
"@typescript-eslint"
],
"plugins": ["@typescript-eslint"],
"extends": [
"prettier",
"eslint:recommended",
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended"
],
"overrides": [
{
"files": ["src/**/__mocks__/*.ts", "*.spec.ts"],
"rules": {
"@typescript-eslint/no-explicit-any": "off"
}
}
]
}
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
"clean": "rm -rf dist",
"build": "tsc",
"watch": "tsc -w",
"format": "prettier --write tests/**/*.ts src/**/*.ts",
"lint": "prettier -c tests/**/*.ts src/**/*.ts && eslint src/**/*.ts tests/**/*.ts",
"format": "prettier --write tests/**/*.ts src/*.ts src/**/*.ts",
"lint": "prettier -c tests/**/*.ts src/*.ts src/**/*.ts && eslint src tests --ext .ts --max-warnings 0",
"check": "npm run lint && npm run test:unit",
"test": "npm run test:unit",
"test:unit": "jest --coverage --verbose --passWithNoTests",
Expand Down
51 changes: 28 additions & 23 deletions src/Cache.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import defaultsDeep from "lodash/defaultsDeep";
import { ConnectionStatus } from "./ConnectionStatus";
import { Executor, ValueOfExecutor } from "./Executor";
import { ReadWriteOptions, Storage, WriteOptions } from "./storage/Storage";
import { StorageAdapter } from "./StorageAdapter";
import { BaseStorage } from "./storage/BaseStorage";
import { Executor, runExecutor } from "./Executor";
import { Logger } from "./Logger";
import { Manager } from "./Manager";
import { ManagerOptions } from "./managers/BaseManager";
import RefreshAheadManager from "./managers/RefreshAheadManager";
import { BaseManager, ManagerOptions } from "./managers/BaseManager";
import {RecordValue} from "./storage/Record";
import { BaseStorage } from "./storage/BaseStorage";
import { Record } from "./storage/Record";
import { ReadWriteOptions, Storage, WriteOptions } from "./storage/Storage";
import { StorageAdapter } from "./StorageAdapter";

export interface CacheWithCustomStorageOptions {
storage: Storage;
Expand All @@ -17,20 +17,25 @@ export interface CacheWithBaseStorageOptions {
adapter: StorageAdapter;
tagsAdapter?: StorageAdapter;
}
export interface ManagerConstructor<T extends BaseManager = any> {

export interface ManagerConstructor<T extends Manager> {
new (options: ManagerOptions): T;
getName(): string;
}

export type CacheOptions = (CacheWithCustomStorageOptions | CacheWithBaseStorageOptions) & {
logger: Logger;
expiresIn?: number;
prefix?: string;
hashKeys?: boolean;
};
export const isCustomStorageOptions = (options: any): options is CacheWithCustomStorageOptions =>
Boolean(options.storage);
export const isBaseStorageOptions = (options: any): options is CacheWithBaseStorageOptions =>
Boolean(options.adapter) && !Boolean(options.storage);

export const isCustomStorageOptions = (options: object): options is CacheWithCustomStorageOptions =>
Object.prototype.hasOwnProperty.call(options, "storage");

export const isBaseStorageOptions = (options: object): options is CacheWithBaseStorageOptions =>
Object.prototype.hasOwnProperty.call(options, "adapter");

export interface ManagerSelectorOptions {
manager?: string;
}
Expand Down Expand Up @@ -97,19 +102,19 @@ class Cache {
* Get delegates call to default or provided manager. The only thing it does by itself is checking
* the connection status of storage. If storage is disconnected calls executor directly and returns result.
*/
public async get<E extends Executor>(
public async get<R>(
key: string,
executor: E,
options: ReadWriteOptions & ManagerSelectorOptions = {}
): Promise<ValueOfExecutor<E>> {
executor: Executor<R>,
options: ReadWriteOptions<R> & ManagerSelectorOptions = {}
): Promise<R> {
const connectionStatus = this.storage.getConnectionStatus();

if (connectionStatus !== ConnectionStatus.CONNECTED) {
this.logger.error(
`Storage connection status is "${connectionStatus}", cache is unavailable!. Running executor.`
);

return executor();
return runExecutor(executor);
}

const { manager: managerName = RefreshAheadManager.getName() } = options;
Expand All @@ -122,11 +127,11 @@ class Cache {
/**
* Just like "get" this method delegates call to default or provided manager
*/
public async set(
public async set<R>(
key: string,
value: RecordValue,
options: WriteOptions & ManagerSelectorOptions = {}
): Promise<any> {
value: R,
options: WriteOptions<R> & ManagerSelectorOptions = {}
): Promise<Record<R>> {
const { manager: managerName = RefreshAheadManager.getName() } = options;
const computedOptions = defaultsDeep({}, options, { expiresIn: this.expiresIn });
const manager = this.getManager(managerName);
Expand All @@ -145,15 +150,15 @@ class Cache {
* cache.touch(['news']);
* ```
*/
public async touch(tags: string[]): Promise<any> {
public async touch(tags: string[]): Promise<void> {
return this.storage.touch(tags);
}

/**
* Register a new cache manager
*/
public registerManager(
managerClass: ManagerConstructor,
public registerManager<T extends Manager>(
managerClass: ManagerConstructor<T>,
name?: string | null,
options: Partial<ManagerOptions> = {}
): void {
Expand Down
22 changes: 16 additions & 6 deletions src/Executor.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import { executorReturnsUndefinedError } from "./errors/errors";
import { ReadWriteOptions } from "./storage/Storage";
import { Record } from "./storage/Record";

export interface ExecutorContext {
export interface ExecutorContext<R> {
key: string;
executor: Executor;
options: ReadWriteOptions;
record?: Record;
executor: Executor<R>;
options: ReadWriteOptions<R>;
record?: Record<R>;
}

export type ValueOfExecutor<V extends Executor> = ReturnType<V> extends Promise<infer RT> ? RT : V;
export type Executor = (...args: any[]) => Promise<any> | any;
export async function runExecutor<R>(executor: Executor<R>): Promise<R> {
const result = await executor();

if (result === undefined) {
throw executorReturnsUndefinedError();
}

return result;
}

export type Executor<R> = (...args: unknown[]) => Promise<R> | R;
2 changes: 1 addition & 1 deletion src/LockedKeyRetrieveStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ExecutorContext } from "./Executor";
*/
export interface LockedKeyRetrieveStrategy {
getName(): string;
get<R = unknown>(context: ExecutorContext): Promise<R>;
get<R>(context: ExecutorContext<R>): Promise<R>;
}

export enum LockedKeyRetrieveStrategyTypes {
Expand Down
8 changes: 4 additions & 4 deletions src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* errors and trace/debug information
*/
export interface Logger {
info(...args: any[]): void;
trace(...args: any[]): void;
warn(...args: any[]): void;
error(...args: any[]): void;
info(...args: unknown[]): void;
trace(...args: unknown[]): void;
warn(...args: unknown[]): void;
error(...args: unknown[]): void;
}
8 changes: 4 additions & 4 deletions src/Manager.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { Executor, ValueOfExecutor } from "./Executor";
import { Executor } from "./Executor";
import { WriteOptions, ReadWriteOptions } from "./storage/Storage";
import {RecordValue} from "./storage/Record";
import { Record } from "./storage/Record";

/**
* Manager is the basic interface for all caching classes. Manager must implement
* two simple methods - get, and set. Cache class will delegate it's get and set calls to manager
* which must decide what record should be threaten as invalid, when and how to update record
*/
export interface Manager {
get<E extends Executor>(key: string, executor: E, options: ReadWriteOptions): Promise<ValueOfExecutor<E>>;
set(key: string, value: RecordValue, options?: WriteOptions): Promise<any>;
get<R>(key: string, executor: Executor<R>, options: ReadWriteOptions<R>): Promise<R>;
set<R>(key: string, value: R, options?: WriteOptions<R>): Promise<Record<R>>;
}
2 changes: 1 addition & 1 deletion src/adapters/RedisStorageAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export class RedisStorageAdapter implements StorageAdapter {
/**
* Implementing this method in the redis adapter will trigger a callback as soon as the redis send event to the 'ready' event.
*/
public onConnect(callback: (...args: any[]) => void): void {
public onConnect(callback: (...args: unknown[]) => void): void {
this.redisInstance.on("ready", callback);
}

Expand Down
7 changes: 3 additions & 4 deletions src/adapters/TestStorageAdapter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { ConnectionStatus } from "../ConnectionStatus";
import { StorageAdapterOptions, StorageAdapter } from "../StorageAdapter";
import { StorageAdapter, StorageAdapterOptions } from "../StorageAdapter";

class TestStorageAdapter implements StorageAdapter {
options: StorageAdapterOptions;
Expand Down Expand Up @@ -61,9 +62,7 @@ class TestStorageAdapter implements StorageAdapter {
async releaseLock(key: string): Promise<boolean> {
this.checkConnection();

const isDeleted = await this.del(`${key}_lock`);

return isDeleted;
return this.del(`${key}_lock`);
}

async isLockExists(key: string): Promise<boolean> {
Expand Down
5 changes: 0 additions & 5 deletions src/deserialize.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import deserialize from "./deserialize";

describe("deserizalize", () => {
it("returns undefined as is", () => {
expect(deserialize(undefined)).toEqual(undefined);
});

it("parses valid json string or null", () => {
expect(deserialize(null)).toEqual(null);
expect(deserialize("null")).toEqual(null);
expect(deserialize('"test"')).toEqual("test");
expect(deserialize("1")).toEqual(1);
Expand Down
6 changes: 1 addition & 5 deletions src/deserialize.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import * as errors from "./errors/errors";

export default function(value: any): any {
if (value === undefined) {
return value;
}

export default function<R>(value: string): R {
try {
return JSON.parse(value);
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions src/errors/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export const ERRORS = {
RequestMaximumTimeoutExceededError: "RequestMaximumTimeoutExceededError",
WaitForResultError: "WaitForResultError",
OperationTimeoutError: "OperationTimeoutError",
ExecutorReturnsUndefinedError: "ExecutorReturnsUndefinedError",
};
6 changes: 6 additions & 0 deletions src/errors/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,9 @@ export function operationTimeoutError(timeout: number): Error {
export function isOperationTimeoutError(error: unknown): boolean {
return error instanceof Error && error.name === ERRORS.OperationTimeoutError;
}

export function executorReturnsUndefinedError(): Error {
const text = "Executor should not return undefined. Correct value for emptiness is null.";

return customError(ERRORS.ExecutorReturnsUndefinedError, text);
}
3 changes: 1 addition & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { StorageAdapter, StorageAdapterOptions } from "./StorageAdapter";
import { Tag, Tags } from "./storage/Storage";
import {Record, RecordValue} from "./storage/Record";
import { Record } from "./storage/Record";
import Cache, { CacheOptions } from "./Cache";
import RedisStorageAdapter from "./adapters/RedisStorageAdapter";
import MemcachedStorageAdapter from "./adapters/MemcachedStorageAdapter";
Expand All @@ -14,7 +14,6 @@ export {
StorageAdapterOptions,
Record,
Tag,
RecordValue,
Tags,
RedisStorageAdapter,
MemcachedStorageAdapter,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LockedKeyRetrieveStrategy } from "../LockedKeyRetrieveStrategy";
import { ExecutorContext } from "../Executor";
import { ExecutorContext, runExecutor } from "../Executor";

/**
* This locked key retrieve strategy is default and suitable for most cases. It just
Expand All @@ -10,7 +10,7 @@ export class RunExecutorLockedKeyRetrieveStrategy implements LockedKeyRetrieveSt
return "runExecutor";
}

async get(context: ExecutorContext): Promise<any> {
return context.executor();
async get<R>(context: ExecutorContext<R>): Promise<R> {
return runExecutor(context.executor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ describe("WaitForResultLockedKeyRetrieveStrategy", () => {
expect(instance.getName()).toEqual(expect.any(String));
});

it("get deserializes value", async () => {
keyLockCheckFnMock.mockReturnValue(false);
getRecordMock.mockReturnValue({ value: `{"a":1}` });

expect(await instance.get({ key: "test" })).toEqual({ a: 1 });
getRecordMock.mockReset();
});

it("get throws if null in cache", async () => {
getRecordMock.mockReturnValue(null);
await expect(instance.get({ key: "test" })).rejects.toThrow("Error while waiting for result in cache");
Expand All @@ -53,12 +61,4 @@ describe("WaitForResultLockedKeyRetrieveStrategy", () => {
await expect(instance.get({ key: "test" })).rejects.toThrow("Exceeded maximum timeout of 100");
keyLockCheckFnMock.mockReset();
});

it("get returns undefined if record value is undefined", async () => {
keyLockCheckFnMock.mockReturnValue(false);
getRecordMock.mockReturnValue({ value: undefined });

expect(await instance.get({ key: "test" })).toEqual(undefined);
getRecordMock.mockReset();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const DEFAULT_MAXIMUM_TIMEOUT = 3000;
export const DEFAULT_REQUEST_TIMEOUT = 250;

export type KeyLockCheckFn = (key: string) => boolean | Promise<boolean>;
export type GetRecordFn = (key: string) => Promise<any>;
export type GetRecordFn = <R>(key: string) => Promise<Record<R> | null>;
export type WaitForResultLockedKeyRetrieveStrategyOptions = {
maximumTimeout?: number;
requestTimeout?: number;
Expand Down Expand Up @@ -44,21 +44,21 @@ export class WaitForResultLockedKeyRetrieveStrategy implements LockedKeyRetrieve
return "waitForResult";
}

public async get(context: ExecutorContext): Promise<any> {
public async get<R>(context: ExecutorContext<R>): Promise<R> {
const startTime = Date.now();
const retryRequest = async (): Promise<any> => {
const retryRequest = async (): Promise<R> => {
if (Date.now() < startTime + this.maximumTimeout) {
const isLocked = await this.keyIsLocked(context.key);

if (!isLocked) {
const rec: Record | null = await this.getRecord(context.key);
const rec = await this.getRecord<string>(context.key);

switch (rec) {
case null:
case undefined:
throw errors.waitForResultError();
default:
return deserialize(rec.value);
return deserialize<R>(rec.value);
}
}

Expand Down

0 comments on commit 22b8d3a

Please sign in to comment.