chore: move ping functionality into cache client (#516)

* chore: move ping functionality into cache client

* fix: fix test typo

* fix: simplify calculation of ping client endpoint

packages/client-sdk-web/src/cache-client.ts

@@ -1,15 +1,17 @@
 import {ControlClient} from './internal/control-client';
 import {DataClient} from './internal/data-client';
+import {PingClient} from './internal/ping-client';
 import {
   CredentialProvider,
   NoopMomentoLoggerFactory,
 } from '@gomomento/sdk-core';
 import {
   AbstractCacheClient,
   IControlClient,
-  IDataClient,
   ICacheClient,
-} from '@gomomento/sdk-core/dist/src/internal/clients/index';
+  IDataClient,
+  IPingClient,
+} from '@gomomento/sdk-core/dist/src/internal/clients';
 
 export interface CacheClientProps {
   credentialProvider: CredentialProvider;
@@ -19,7 +21,8 @@ export class CacheClient extends AbstractCacheClient implements ICacheClient {
   constructor(props: CacheClientProps) {
     const controlClient: IControlClient = createControlClient(props);
     const dataClient: IDataClient = createDataClient(props);
-    super(controlClient, [dataClient]);
+    const pingClient: IPingClient = createPingClient(props);
+    super(controlClient, [dataClient], pingClient);
   }
 }
 
@@ -51,3 +54,17 @@ function createDataClient(props: CacheClientProps): IDataClient {
     defaultTtlSeconds: 60,
   });
 }
+
+function createPingClient(props: CacheClientProps): IPingClient {
+  return new PingClient({
+    // TODO
+    // TODO
+    // TODO these shouldn't be hard-coded
+    // TODO
+    // TODO
+    endpoint: props.credentialProvider.getCacheEndpoint(),
+    configuration: {
+      getLoggerFactory: () => new NoopMomentoLoggerFactory(),
+    },
+  });
+}

packages/client-sdk-web/src/internal/ping-client.ts

@@ -5,16 +5,18 @@ import {Request, UnaryInterceptor, UnaryResponse} from 'grpc-web';
 import {_PingRequest} from '@gomomento/generated-types-webtext/dist/cacheping_pb';
 import {Configuration} from '../config/configuration';
 import {MomentoLogger} from '@gomomento/sdk-core';
+import {IPingClient} from '@gomomento/sdk-core/dist/src/internal/clients';
 
 export interface PingClientProps {
   endpoint: string;
   configuration: Configuration;
 }
 
-export class InternalGrpcWebPingClient<
+export class PingClient<
   REQ extends Request<REQ, RESP>,
   RESP extends UnaryResponse<REQ, RESP>
-> {
+> implements IPingClient
+{
   private readonly clientWrapper: ping.PingClient;
   private readonly unaryInterceptors: UnaryInterceptor<REQ, RESP>[];
   private readonly logger: MomentoLogger;
@@ -31,7 +33,7 @@ export class InternalGrpcWebPingClient<
       ).createHeadersInterceptor(),
     ];
     this.clientWrapper = new ping.PingClient(
-      `https://cache.${props.endpoint}:443`,
+      `https://${props.endpoint}:443`,
       null,
       {
         unaryInterceptors: this.unaryInterceptors,

packages/client-sdk-web/test/integration/ping.test.ts

@@ -1,14 +1,38 @@
-import {PingClient} from '../../src/ping-client';
-import {NoopMomentoLoggerFactory} from '@gomomento/sdk-core';
+import {
+  CredentialProvider,
+  MomentoLoggerFactory,
+  NoopMomentoLoggerFactory,
+} from '@gomomento/sdk-core';
+import {CacheClient} from '../../src';
+import {PingClient} from '../../src/internal/ping-client';
+import {expectWithMessage} from '@gomomento/common-integration-tests';
 
 describe('ping service', () => {
   it('ping should work', async () => {
-    const client = new PingClient({
-      endpoint: 'cell-alpha-dev.preprod.a.momentohq.com',
+    const cacheClient = new CacheClient({
+      credentialProvider: CredentialProvider.fromEnvironmentVariable({
+        environmentVariableName: 'TEST_AUTH_TOKEN',
+      }),
+    });
+    await cacheClient.ping();
+  });
+  it('should fail on bad URL', async () => {
+    const pingClient = new PingClient({
+      endpoint: 'bad.url',
       configuration: {
-        getLoggerFactory: () => new NoopMomentoLoggerFactory(),
+        getLoggerFactory(): MomentoLoggerFactory {
+          return new NoopMomentoLoggerFactory();
+        },
       },
     });
-    await client.ping();
+    try {
+      await pingClient.ping();
+      // we shouldn't get to the assertion below
+      expect(true).toBeFalse();
+    } catch (error) {
+      expectWithMessage(() => {
+        expect((error as Error).name).toEqual('RpcError');
+      }, `expected RpcError but got ${(error as Error).toString()}`);
+    }
   });
 });

packages/core/src/internal/clients/cache/AbstractCacheClient.ts

@@ -65,26 +65,39 @@ import {
 } from './ICacheClient';
 import {IControlClient} from './IControlClient';
 import {IDataClient} from './IDataClient';
+import {IPingClient} from './IPingClient';
 
 export abstract class AbstractCacheClient implements ICacheClient {
   // making these protected until we fully abstract away the nodejs client
   protected readonly controlClient: IControlClient;
   protected readonly dataClients: IDataClient[];
+  // TODO: Make pingClient required if and when the nodejs side starts adding
+  //  one as well
+  protected readonly pingClient?: IPingClient;
   protected nextDataClientIndex: number;
 
   protected constructor(
     controlClient: IControlClient,
-    dataClients: IDataClient[]
+    dataClients: IDataClient[],
+    pingClient?: IPingClient
   ) {
     this.controlClient = controlClient;
     this.dataClients = dataClients;
+    this.pingClient = pingClient;
 
     // We round-robin the requests through all of our clients.  Since javascript
     // is single-threaded, we don't have to worry about thread safety on this
     // index variable.
     this.nextDataClientIndex = 0;
   }
 
+  /**
+   * Ping the service to verify it is up and running
+   */
+  public async ping(): Promise<void> {
+    return await this.pingClient?.ping();
+  }
+
   /**
    * Creates a cache if it does not exist.
    *

packages/core/src/internal/clients/cache/ICacheClient.ts

@@ -1,4 +1,5 @@
 import {IControlClient} from './IControlClient';
+import {IPingClient} from './IPingClient';
 import {
   CacheDelete,
   CacheGet,
@@ -67,7 +68,7 @@ export type SortedSetFetchByRankOptions = SortedSetFetchByRankCallOptions;
 export type SortedSetFetchByScoreOptions = SortedSetFetchByScoreCallOptions;
 export type SortedSetIncrementOptions = CollectionCallOptions;
 
-export interface ICacheClient extends IControlClient {
+export interface ICacheClient extends IControlClient, IPingClient {
   get(cacheName: string, key: string | Uint8Array): Promise<CacheGet.Response>;
   set(
     cacheName: string,

packages/core/src/internal/clients/cache/index.ts

@@ -2,3 +2,4 @@ export * from './AbstractCacheClient';
 export * from './ICacheClient';
 export * from './IControlClient';
 export * from './IDataClient';
+export * from './IPingClient';

packages/core/src/internal/clients/index.ts

@@ -1,4 +1,3 @@
-export * from './ping';
 export * from './cache';
 export * from './auth';
 export * from './pubsub';