From 08f7e6a9fab68f572ff1cf21af6a6b0a4df5c390 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Thu, 20 Sep 2018 11:34:13 +0100
Subject: [PATCH] refactor(ui): prevent concurrent fetches

Refactor the timer so instead of calling fetch directly every  seconds we call it every 1 second, check if we could & should fetch (fetch isn't already/still in progress and  seconds passed since last fetch) and if needed call fetch.
This will also allow us to pause fetches when user is interacting with alerts so there's no whack-a-mole with buttons
---
 ui/src/Components/Fetcher/index.js      | 63 +++++++++++++++----------
 ui/src/Components/Fetcher/index.test.js | 54 +++++++++++++++++----
 2 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/ui/src/Components/Fetcher/index.js b/ui/src/Components/Fetcher/index.js
index e20f0332a..1361825a5 100644
--- a/ui/src/Components/Fetcher/index.js
+++ b/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() {
diff --git a/ui/src/Components/Fetcher/index.test.js b/ui/src/Components/Fetcher/index.test.js
index b10db3132..2f4679602 100644
--- a/ui/src/Components/Fetcher/index.test.js
+++ b/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);
   });