Merge pull request #45 from prymitive/fetcher

refactor(ui): prevent concurrent fetches

ui/src/Components/Fetcher/index.js

@@ -1,10 +1,12 @@
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 
-import { toJS } from "mobx";
+import { observable, action } from "mobx";
 import { observer } from "mobx-react";
 
-import { AlertStore } from "Stores/AlertStore";
+import moment from "moment";
+
+import { AlertStore, AlertStoreStatuses } from "Stores/AlertStore";
 import { Settings } from "Stores/Settings";
 
 const Fetcher = observer(
@@ -14,40 +16,53 @@ const Fetcher = observer(
       settingsStore: PropTypes.instanceOf(Settings).isRequired
     };
 
-    timer = null;
+    lastTick = observable(
+      {
+        time: moment(0),
+        update() {
+          this.time = moment();
+        }
+      },
+      {
+        update: action
+      }
+    );
 
-    interval = null;
-
-    setTimer() {
+    fetchIfIdle = () => {
       const { alertStore, settingsStore } = this.props;
 
-      const newInterval = toJS(settingsStore.fetchConfig.config.interval);
+      const nextTick = moment(this.lastTick.time).add(
+        settingsStore.fetchConfig.config.interval,
+        "seconds"
+      );
 
-      if (this.interval !== newInterval) {
-        if (this.timer !== null) clearInterval(this.timer);
+      const pastDeadline = moment().isSameOrAfter(nextTick);
 
-        this.interval = newInterval;
-        this.timer = setInterval(
-          () => alertStore.fetchWithThrottle(),
-          this.interval * 1000
-        );
+      const status = alertStore.status.value.toString();
+      const updateInProgress =
+        status === AlertStoreStatuses.Fetching.toString() ||
+        status === AlertStoreStatuses.Processing.toString();
+
+      if (pastDeadline && !updateInProgress) {
+        this.lastTick.update();
+        alertStore.fetchWithThrottle();
       }
+    };
+
+    timerTick = () => {
+      this.fetchIfIdle();
+    };
+
+    componentDidMount() {
+      this.fetchIfIdle();
+      this.timer = setInterval(this.timerTick, 1000);
     }
 
     componentDidUpdate() {
       const { alertStore } = this.props;
 
+      this.lastTick.update();
       alertStore.fetchWithThrottle();
-
-      this.setTimer();
-    }
-
-    componentDidMount() {
-      const { alertStore } = this.props;
-
-      alertStore.fetchWithThrottle();
-
-      this.setTimer();
     }
 
     componentWillUnmount() {

ui/src/Components/Fetcher/index.test.js

@@ -2,6 +2,8 @@ import React from "react";
 
 import { mount } from "enzyme";
 
+import { advanceTo, advanceBy, clear } from "jest-date-mock";
+
 import { EmptyAPIResponse } from "__mocks__/Fetch";
 
 import { AlertStore } from "Stores/AlertStore";
@@ -9,24 +11,30 @@ import { Settings } from "Stores/Settings";
 
 import { Fetcher } from ".";
 
+let alertStore;
+let settingsStore;
+let fetchSpy;
+
 beforeAll(() => {
   jest.useFakeTimers();
 });
 
-let alertStore;
-let settingsStore;
-
 beforeEach(() => {
-  fetch.mockResponse(JSON.stringify(EmptyAPIResponse()));
+  advanceTo(new Date(2000, 1, 1, 0, 0, 0));
 
   alertStore = new AlertStore(["label=value"]);
+  fetchSpy = jest
+    .spyOn(alertStore, "fetchWithThrottle")
+    .mockImplementation(() => {});
+
   settingsStore = new Settings();
+  settingsStore.fetchConfig.config.interval = 30;
 });
 
 afterEach(() => {
   jest.clearAllTimers();
-
-  global.fetch.mockRestore();
+  jest.clearAllMocks();
+  clear();
 });
 
 const MockEmptyAPIResponseWithoutFilters = () => {
@@ -57,6 +65,30 @@ describe("<Fetcher />", () => {
     expect(tree.html()).toBe(FetcherSpan("label=value", 60));
   });
 
+  it("changing interval changes how often fetch is called", () => {
+    settingsStore.fetchConfig.config.interval = 1;
+    MountedFetcher();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    settingsStore.fetchConfig.config.interval = 600;
+
+    advanceBy(3 * 1000);
+    jest.runOnlyPendingTimers();
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+
+    advanceBy(32 * 1000);
+    jest.runOnlyPendingTimers();
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+
+    advanceBy(62 * 1000);
+    jest.runOnlyPendingTimers();
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+
+    advanceBy(602 * 1000);
+    jest.runOnlyPendingTimers();
+    expect(fetchSpy).toHaveBeenCalledTimes(3);
+  });
+
   it("re-renders on filters change", () => {
     MockEmptyAPIResponseWithoutFilters();
     const tree = MountedFetcher();
@@ -86,13 +118,19 @@ describe("<Fetcher />", () => {
     expect(fetchSpy).toHaveBeenCalledTimes(2);
   });
 
-  it("keeps calling alertStore.fetchWithThrottle after running pending timers", () => {
-    const fetchSpy = jest.spyOn(alertStore, "fetchWithThrottle");
+  it("keeps calling alertStore.fetchWithThrottle every minute", () => {
     MountedFetcher();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    advanceBy(62 * 1000);
     jest.runOnlyPendingTimers();
     expect(fetchSpy).toHaveBeenCalledTimes(2);
+
+    advanceBy(62 * 1000);
     jest.runOnlyPendingTimers();
     expect(fetchSpy).toHaveBeenCalledTimes(3);
+
+    advanceBy(62 * 1000);
     jest.runOnlyPendingTimers();
     expect(fetchSpy).toHaveBeenCalledTimes(4);
   });