Skip to content

Commit

Permalink
chore: centralize default ttl and milliseconds conversion
Browse files Browse the repository at this point in the history
Previously the code had lots of ad hoc conversion:
- deciding to use the provided ttl vs default
- converting to milliseconds by multiplying by 1000

We centralize these calculations for maintainability and to reduce
errors.
  • Loading branch information
malandis committed Dec 6, 2024
1 parent a014cc6 commit fbf6d92
Show file tree
Hide file tree
Showing 13 changed files with 130 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
TopicLimits,
} from '@gomomento/sdk-core/dist/src/messages/cache-info';
import {RetryInterceptor} from './grpc/retry-interceptor';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export interface ControlClientProps {
configuration: Configuration;
Expand All @@ -32,7 +33,8 @@ export interface ControlClientProps {
export class CacheControlClient {
private readonly clientWrapper: GrpcClientWrapper<grpcControl.ScsControlClient>;
private readonly interceptors: Interceptor[];
private static readonly REQUEST_TIMEOUT_MS: number = 60 * 1000;
private static readonly REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(60);
private readonly logger: MomentoLogger;
private readonly cacheServiceErrorMapper: CacheServiceErrorMapper;

Expand Down
71 changes: 48 additions & 23 deletions packages/client-sdk-nodejs/src/internal/cache-data-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ import {common} from '@gomomento/generated-types/dist/common';
import {
GetBatchCallOptions,
GetCallOptions,
secondsToMilliseconds,
SetBatchCallOptions,
SetBatchItem,
SetCallOptions,
Expand Down Expand Up @@ -154,7 +155,8 @@ export class CacheDataClient implements IDataClient {
private readonly credentialProvider: CredentialProvider;
private readonly defaultTtlSeconds: number;
private readonly requestTimeoutMs: number;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number = 5 * 1000;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(5);
private readonly logger: MomentoLogger;
private readonly cacheServiceErrorMapper: CacheServiceErrorMapper;
private readonly interceptors: Interceptor[];
Expand Down Expand Up @@ -418,6 +420,29 @@ export class CacheDataClient implements IDataClient {
}
}

/**
* Returns the TTL in milliseconds for a collection.
* If the provided TTL is not set, it defaults to the instance's default TTL.
* @param ttl - The CollectionTttl object containing the TTL value.
* @returns The TTL in milliseconds.
*/
private collectionTtlOrDefaultMilliseconds(ttl: CollectionTtl): number {
return (
ttl.ttlMilliseconds() ?? secondsToMilliseconds(this.defaultTtlSeconds)
);
}

/**
* Returns the TTL in milliseconds.
* If the provided TTL is not set, it defaults to the instance's default TTL.
* @param ttl
* @returns The TTL in milliseconds.
*/
private ttlOrDefaultMilliseconds(ttl?: number): number {
const ttlSeconds = ttl ?? this.defaultTtlSeconds;
return secondsToMilliseconds(ttlSeconds);
}

public async set(
cacheName: string,
key: string | Uint8Array,
Expand Down Expand Up @@ -469,7 +494,7 @@ export class CacheDataClient implements IDataClient {
const request = new grpcCache._SetRequest({
cache_body: value,
cache_key: key,
ttl_milliseconds: ttl * 1000,
ttl_milliseconds: secondsToMilliseconds(ttl),
});
const metadata = this.createMetadata(cacheName);
return await new Promise((resolve, reject) => {
Expand Down Expand Up @@ -568,7 +593,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(setName),
this.convertArray(elements),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -997,7 +1022,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(key),
this.convert(value),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1098,7 +1123,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(key),
encodedValue,
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1179,7 +1204,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(key),
this.convert(value),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1262,7 +1287,7 @@ export class CacheDataClient implements IDataClient {
this.convert(key),
this.convert(value),
this.convert(equal),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1346,7 +1371,7 @@ export class CacheDataClient implements IDataClient {
this.convert(key),
this.convert(value),
this.convert(notEqual),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1430,7 +1455,7 @@ export class CacheDataClient implements IDataClient {
this.convert(key),
this.convert(value),
this.convert(notEqual),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1515,7 +1540,7 @@ export class CacheDataClient implements IDataClient {
this.convert(key),
this.convert(value),
this.convert(equal),
ttl ? ttl * 1000 : this.defaultTtlSeconds * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -1892,7 +1917,7 @@ export class CacheDataClient implements IDataClient {
const setRequest = new grpcCache._SetRequest({
cache_key: item[0],
cache_body: item[1],
ttl_milliseconds: item[2] * 1000,
ttl_milliseconds: secondsToMilliseconds(item[2]),
});
setRequests.push(setRequest);
}
Expand Down Expand Up @@ -1958,7 +1983,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(listName),
this.convertArray(values),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl(),
truncateFrontToSize
);
Expand Down Expand Up @@ -2027,7 +2052,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(listName),
this.convertArray(values),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl(),
truncateBackToSize
);
Expand Down Expand Up @@ -2171,7 +2196,7 @@ export class CacheDataClient implements IDataClient {
this.convert(listName),
startIndex,
endIndex,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -2406,7 +2431,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(listName),
this.convert(value),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl(),
truncateFrontToSize
);
Expand Down Expand Up @@ -2474,7 +2499,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(listName),
this.convert(value),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl(),
truncateBackToSize
);
Expand Down Expand Up @@ -2655,7 +2680,7 @@ export class CacheDataClient implements IDataClient {
this.convert(dictionaryName),
this.convert(field),
this.convert(value),
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -2725,7 +2750,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(dictionaryName),
dictionaryFieldValuePairs,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -3114,7 +3139,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(field),
amount,
(ttl || this.defaultTtlSeconds) * 1000
this.ttlOrDefaultMilliseconds(ttl)
);
});
}
Expand Down Expand Up @@ -3182,7 +3207,7 @@ export class CacheDataClient implements IDataClient {
this.convert(dictionaryName),
this.convert(field),
amount,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -3255,7 +3280,7 @@ export class CacheDataClient implements IDataClient {
this.convert(sortedSetName),
this.convert(value),
score,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -3327,7 +3352,7 @@ export class CacheDataClient implements IDataClient {
cacheName,
this.convert(sortedSetName),
sortedSetValueScorePairs,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down Expand Up @@ -3800,7 +3825,7 @@ export class CacheDataClient implements IDataClient {
this.convert(sortedSetName),
this.convert(value),
amount,
ttl.ttlMilliseconds() || this.defaultTtlSeconds * 1000,
this.collectionTtlOrDefaultMilliseconds(ttl),
ttl.refreshTtl()
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ import {
import {RetryInterceptor} from './grpc/retry-interceptor';
import {AuthClientConfigurations} from '../index';
import {AuthClientAllProps} from './auth-client-all-props';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export class InternalAuthClient implements IAuthClient {
private static readonly REQUEST_TIMEOUT_MS: number = 60 * 1000;
private static readonly REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(60);

private readonly cacheServiceErrorMapper: CacheServiceErrorMapper;
private readonly creds: CredentialProvider;
Expand Down
4 changes: 3 additions & 1 deletion packages/client-sdk-nodejs/src/internal/ping-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {GrpcClientWrapper} from './grpc/grpc-client-wrapper';
import {Configuration} from '../config/configuration';
import {CredentialProvider, MomentoLogger} from '../';
import {RetryInterceptor} from './grpc/retry-interceptor';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export interface PingClientProps {
configuration: Configuration;
Expand All @@ -18,7 +19,8 @@ export interface PingClientProps {
export class InternalNodeGrpcPingClient {
private readonly clientWrapper: GrpcClientWrapper<grpcPing.PingClient>;
private readonly interceptors: Interceptor[];
private static readonly REQUEST_TIMEOUT_MS: number = 60 * 1000;
private static readonly REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(60);
private readonly logger: MomentoLogger;

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/client-sdk-nodejs/src/internal/pubsub-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ import {TopicConfiguration} from '../config/topic-configuration';
import {TopicClientAllProps} from './topic-client-all-props';
import {grpcChannelOptionsFromGrpcConfig} from './grpc/grpc-channel-options';
import {RetryInterceptor} from './grpc/retry-interceptor';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export class PubsubClient extends AbstractPubsubClient<ServiceError> {
private readonly client: grpcPubsub.PubsubClient;
protected readonly credentialProvider: CredentialProvider;
private readonly unaryRequestTimeoutMs: number;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number = 5 * 1000;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(5);
private static readonly DEFAULT_MAX_SESSION_MEMORY_MB: number = 256;
private readonly unaryInterceptors: Interceptor[];
private readonly streamingInterceptors: Interceptor[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import {validateStoreName} from '@gomomento/sdk-core/dist/src/internal/utils';
import {CreateStore, DeleteStore} from '@gomomento/sdk-core';
import {RetryInterceptor} from './grpc/retry-interceptor';
import {StorageClientAllProps} from './storage-client-all-props';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export class StorageControlClient {
private readonly clientWrapper: grpcControl.ScsControlClient;
private readonly interceptors: Interceptor[];
private static readonly REQUEST_TIMEOUT_MS: number = 60 * 1000;
private static readonly REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(60);
private readonly logger: MomentoLogger;
private readonly cacheServiceErrorMapper: CacheServiceErrorMapper;

Expand Down
4 changes: 3 additions & 1 deletion packages/client-sdk-nodejs/src/internal/webhook-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import {
} from '@gomomento/sdk-core/dist/src/internal/utils';
import {RetryInterceptor} from './grpc/retry-interceptor';
import {TopicClientAllProps} from './topic-client-all-props';
import {secondsToMilliseconds} from '@gomomento/sdk-core/dist/src/utils';

export class WebhookClient implements IWebhookClient {
private readonly webhookClient: grpcWebhook.WebhookClient;
protected readonly credentialProvider: CredentialProvider;
private readonly logger: MomentoLogger;
private readonly cacheServiceErrorMapper: CacheServiceErrorMapper;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number = 5 * 1000;
private static readonly DEFAULT_REQUEST_TIMEOUT_MS: number =
secondsToMilliseconds(5);
private readonly unaryInterceptors: Interceptor[];

/**
Expand Down
Loading

0 comments on commit fbf6d92

Please sign in to comment.