From 7c669459b405d4b20e7f7b993529b9494ba0cc6b Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Wed, 26 Jan 2022 17:44:14 +0000
Subject: [PATCH] Disable API caching by default and deep clone data fetch
 config to avoid direct mutation side effects

---
 packages/frontend-core/src/api/index.js       | 25 ++++++++++++++++++-
 packages/frontend-core/src/fetch/DataFetch.js |  7 ++++--
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/packages/frontend-core/src/api/index.js b/packages/frontend-core/src/api/index.js
index 2c1df6ffaf..0d9ab4449f 100644
--- a/packages/frontend-core/src/api/index.js
+++ b/packages/frontend-core/src/api/index.js
@@ -23,7 +23,25 @@ import { buildUserEndpoints } from "./user"
 import { buildViewEndpoints } from "./views"
 
 const defaultAPIClientConfig = {
+  /**
+   * Certain definitions can't change at runtime for client apps, such as the
+   * schema of tables. The endpoints that are cacheable can be cached by passing
+   * in this flag. It's disabled by default to avoid bugs with stale data.
+   */
+  enableCaching: false,
+
+  /**
+   * A function can be passed in to attach headers to all outgoing requests.
+   * This function is passed in the headers object, which should be directly
+   * mutated. No return value is required.
+   */
   attachHeaders: null,
+
+  /**
+   * A function can be passed in which will be invoked any time an API error
+   * occurs. An error is defined as a status code >= 400. This function is
+   * invoked before the actual JS error is thrown up the stack.
+   */
   onError: null,
 }
 
@@ -161,8 +179,13 @@ export const createAPIClient = config => {
       if (!external) {
         url = `/${url}`.replace("//", "/")
       }
+
+      // Cache the request if possible and desired
+      const cacheRequest = cache && config?.enableCaching
+      const handler = cacheRequest ? makeCachedApiCall : makeApiCall
+
       const enrichedParams = { ...params, method, url }
-      return await (cache ? makeCachedApiCall : makeApiCall)(enrichedParams)
+      return await handler(enrichedParams)
     } catch (error) {
       if (config?.onError) {
         config.onError(error)
diff --git a/packages/frontend-core/src/fetch/DataFetch.js b/packages/frontend-core/src/fetch/DataFetch.js
index 72e1d84f00..3455ee132b 100644
--- a/packages/frontend-core/src/fetch/DataFetch.js
+++ b/packages/frontend-core/src/fetch/DataFetch.js
@@ -1,4 +1,5 @@
 import { writable, derived, get } from "svelte/store"
+import { cloneDeep } from "lodash/fp"
 import {
   buildLuceneQuery,
   luceneLimit,
@@ -303,10 +304,12 @@ export default class DataFetch {
       return
     }
 
-    // Assign new options and reload data
+    // Assign new options and reload data.
+    // Clone the new options to ensure that some external source doesn't end up
+    // mutating the real values in the config.
     this.options = {
       ...this.options,
-      ...newOptions,
+      ...cloneDeep(newOptions),
     }
     await this.getInitialData()
   }