fix(endpoint-cache): revert removal of additional logic in get operation

TimeStream Service Developer Guide refers to CachePeriodInMinutes as Time to Live (TTL) value. It also specifies calls to be made for the duration of TTL. Refs: https://a.co/2e8J7xT
aws · May 6, 2021 · d102d16 · d102d16
1 parent 521b4d1
commit d102d16
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 32 deletions.
diff --git a/packages/endpoint-cache/README.md b/packages/endpoint-cache/README.md
@@ -13,4 +13,5 @@ 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 endpoints with their Expires values.
+- the `get` operation returns all un-expired endpoints with their Expires values.
+- the `getEndpoint` operation returns a randomly selected un-expired endpoint.
diff --git a/packages/endpoint-cache/src/EndpointCache.spec.ts b/packages/endpoint-cache/src/EndpointCache.spec.ts
@@ -28,6 +28,9 @@ 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,
@@ -57,41 +60,71 @@ describe(EndpointCache.name, () => {
       jest.spyOn(Date, "now").mockImplementation(() => now);
     });
 
-    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();
-      });
+    const verifyHasAndGetCalls = () => {
+      expect(has).toHaveBeenCalledTimes(1);
+      expect(has).toHaveBeenCalledWith(key);
+      expect(get).toHaveBeenCalledTimes(1);
+      expect(get).toHaveBeenCalledWith(key);
+    };
 
-      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();
+    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();
         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);
+      it("returns un-expired endpoint", () => {
+        jest.spyOn(Date, "now").mockImplementation(() => now + 90 * 1000);
+        expect(endpointCache.getEndpoint(key)).toEqual(mockEndpoints[1].Address);
+        verifyHasAndGetCalls();
         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();
+      [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();
+        });
+      });
     });
   });
 

diff --git a/packages/endpoint-cache/src/EndpointCache.ts b/packages/endpoint-cache/src/EndpointCache.ts
@@ -14,7 +14,22 @@ export class EndpointCache {
   }
 
   /**
-   * Returns endpoints for the given key.
+   * 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.
    *
    * @param key
    * @returns
@@ -24,12 +39,15 @@ export class EndpointCache {
       return;
     }
 
-    const endpointsWithExpiry = this.cache.get(key);
-    if (!endpointsWithExpiry) {
+    const value = this.cache.get(key);
+    if (!value) {
       return;
     }
 
+    const now = Date.now();
+    const endpointsWithExpiry = value.filter((endpoint) => now < endpoint.Expires);
     if (endpointsWithExpiry.length === 0) {
+      this.delete(key);
       return undefined;
     }