fix(endpoint-cache): remove additional logic in get operation

The callee will make decision based on value in Expires parameter
as per SDK enhancement proposal.

Author: trivikr

packages/endpoint-cache/README.md

@@ -13,5 +13,4 @@ You probably shouldn't, at least directly.
 
 - uses `mnemonist/lru-cache` for storing the cache.
 - the `set` operation stores milliseconds elapsed since the UNIX epoch in Expires param based on CachePeriodInMinutes provided in Endpoint.
-- the `get` operation returns all un-expired endpoints with their Expires values.
-- the `getEndpoint` operation returns a randomly selected un-expired endpoint.
+- the `get` operation returns all endpoints with their Expires values.

packages/endpoint-cache/src/EndpointCache.spec.ts

@@ -28,9 +28,6 @@ describe(EndpointCache.name, () => {
       Expires: now + CachePeriodInMinutes * 60 * 1000,
     }));
 
-  const getMaxCachePeriodInMins = (endpoints: Endpoint[]) =>
-    Math.max(...endpoints.map((endpoint) => endpoint.CachePeriodInMinutes));
-
   beforeEach(() => {
     ((LRUCache as unknown) as jest.Mock).mockReturnValueOnce({
       set,
@@ -60,72 +57,42 @@ describe(EndpointCache.name, () => {
       jest.spyOn(Date, "now").mockImplementation(() => now);
     });
 
-    const verifyHasAndGetCalls = () => {
-      expect(has).toHaveBeenCalledTimes(1);
-      expect(has).toHaveBeenCalledWith(key);
-      expect(get).toHaveBeenCalledTimes(1);
-      expect(get).toHaveBeenCalledWith(key);
-    };
-
-    it("returns undefined if cache doesn't have key", () => {
-      has.mockReturnValueOnce(false);
-      expect(endpointCache.get(key)).toBeUndefined();
-      expect(has).toHaveBeenCalledTimes(1);
-      expect(has).toHaveBeenCalledWith(key);
-      expect(peek).not.toHaveBeenCalled();
-      expect(get).not.toHaveBeenCalled();
-    });
-
-    it("returns undefined if cache has empty array", () => {
-      has.mockReturnValueOnce(true);
-      peek.mockReturnValueOnce([]);
-      expect(endpointCache.get(key)).toBeUndefined();
-      expect(has).toHaveBeenCalledTimes(1);
-      expect(has).toHaveBeenCalledWith(key);
-      expect(peek).toHaveBeenCalledTimes(1);
-      expect(peek).toHaveBeenCalledWith(key);
-      expect(get).not.toHaveBeenCalled();
-    });
-
-    it("returns undefined if cache returns undefined for key", () => {
-      get.mockReturnValueOnce(undefined);
-      expect(endpointCache.get(key)).toBeUndefined();
-      verifyHasAndGetCalls();
-      expect(set).not.toHaveBeenCalled();
-    });
-
-    it("returns undefined if endpoints have expired", () => {
-      const maxCachePeriod = getMaxCachePeriodInMins(mockEndpoints);
-      jest.spyOn(Date, "now").mockImplementation(() => now + (maxCachePeriod + 1) * 60 * 1000);
-      expect(endpointCache.get(key)).toBeUndefined();
-      verifyHasAndGetCalls();
-      expect(set).toHaveBeenCalledTimes(1);
-      expect(set).toHaveBeenCalledWith(key, []);
-    });
-
-    describe("getEndpoint", () => {
-      it("returns one of the un-expired endpoints", () => {
-        expect(mockEndpoints.map((endpoint) => endpoint.Address)).toContain(endpointCache.getEndpoint(key));
-        verifyHasAndGetCalls();
+    describe("returns undefined", () => {
+      it("if cache doesn't have key", () => {
+        has.mockReturnValueOnce(false);
+        expect(endpointCache.get(key)).toBeUndefined();
+        expect(has).toHaveBeenCalledWith(key);
+        expect(peek).not.toHaveBeenCalled();
+        expect(get).not.toHaveBeenCalled();
         expect(set).not.toHaveBeenCalled();
       });
 
-      it("returns un-expired endpoint", () => {
-        jest.spyOn(Date, "now").mockImplementation(() => now + 90 * 1000);
-        expect(endpointCache.getEndpoint(key)).toEqual(mockEndpoints[1].Address);
-        verifyHasAndGetCalls();
+      it("if cache has empty array", () => {
+        peek.mockReturnValueOnce([]);
+        expect(endpointCache.get(key)).toBeUndefined();
+        expect(has).toHaveBeenCalledWith(key);
+        expect(peek).toHaveBeenCalledWith(key);
+        expect(get).not.toHaveBeenCalled();
         expect(set).not.toHaveBeenCalled();
       });
 
-      [0, 1].forEach((index) => {
-        it(`returns un-expired endpoint at index ${index}`, () => {
-          jest.spyOn(Math, "floor").mockImplementation(() => index);
-          expect(mockEndpoints.map((endpoint) => endpoint.Address)).toContain(endpointCache.getEndpoint(key));
-          verifyHasAndGetCalls();
-          expect(set).not.toHaveBeenCalled();
-        });
+      it("if cache returns undefined for key", () => {
+        get.mockReturnValueOnce(undefined);
+        expect(endpointCache.get(key)).toBeUndefined();
+        expect(has).toHaveBeenCalledWith(key);
+        expect(peek).toHaveBeenCalledWith(key);
+        expect(get).toHaveBeenCalledWith(key);
+        expect(set).not.toHaveBeenCalled();
       });
     });
+
+    it("returns endpoints if available", () => {
+      expect(endpointCache.get(key)).toEqual(getEndpointsWithExpiry(mockEndpoints));
+      expect(has).toHaveBeenCalledWith(key);
+      expect(peek).toHaveBeenCalledWith(key);
+      expect(get).toHaveBeenCalledWith(key);
+      expect(set).not.toHaveBeenCalled();
+    });
   });
 
   describe("set", () => {

packages/endpoint-cache/src/EndpointCache.ts

@@ -14,22 +14,7 @@ export class EndpointCache {
   }
 
   /**
-   * Returns an un-expired endpoint for the given key.
-   *
-   * @param endpointsWithExpiry
-   * @returns
-   */
-  getEndpoint(key: string) {
-    const endpointsWithExpiry = this.get(key);
-    if (!endpointsWithExpiry || endpointsWithExpiry.length === 0) {
-      return undefined;
-    }
-    const endpoints = endpointsWithExpiry.map((endpoint) => endpoint.Address);
-    return endpoints[Math.floor(Math.random() * endpoints.length)];
-  }
-
-  /**
-   * Returns un-expired endpoints for the given key.
+   * Returns endpoints for the given key.
    *
    * @param key
    * @returns
@@ -39,15 +24,12 @@ export class EndpointCache {
       return;
     }
 
-    const value = this.cache.get(key);
-    if (!value) {
+    const endpointsWithExpiry = this.cache.get(key);
+    if (!endpointsWithExpiry) {
       return;
     }
 
-    const now = Date.now();
-    const endpointsWithExpiry = value.filter((endpoint) => now < endpoint.Expires);
     if (endpointsWithExpiry.length === 0) {
-      this.delete(key);
       return undefined;
     }