fix: cache logic for feature (#379)

* fix: cache logic for feature * test: add compute test
reearth · Dec 19, 2022 · 67bc52a · 67bc52a
1 parent ccae02a
commit 67bc52a
Show file tree

Hide file tree

Showing 2 changed files with 184 additions and 15 deletions.
diff --git a/src/core/mantle/atoms/compute.test.ts b/src/core/mantle/atoms/compute.test.ts
@@ -1,16 +1,22 @@
 import { renderHook, act, waitFor } from "@testing-library/react";
 import { useAtom } from "jotai";
 import { useMemo } from "react";
-import { test, expect, vi } from "vitest";
+import { test, expect, vi, beforeEach } from "vitest";
 
-import { fetchData } from "../data";
+import * as DataCache from "../data";
 import { EvalContext, evalLayer } from "../evaluator";
 import type { Data, DataRange, Feature, Layer, LayerSimple } from "../types";
 
 import { doubleKeyCacheAtom } from "./cache";
 import { computeAtom } from "./compute";
 
-const data: Data = { type: "geojson", url: "https://example.com/example.geojson" };
+type TestData = Data & { test_id: string };
+
+const data: TestData = {
+  test_id: "a",
+  type: "geojson",
+  url: "https://example.com/example.geojson",
+};
 const range: DataRange = { x: 0, y: 0, z: 0 };
 const layer: Layer = { id: "xxx", type: "simple", data };
 const features: Feature[] = [{ id: "a", geometry: { type: "Point", coordinates: [0, 0] } }];
@@ -22,7 +28,14 @@ const features2: Feature[] = [
   },
 ];
 
+beforeEach(() => {
+  vi.resetAllMocks();
+});
+
 test("computeAtom", async () => {
+  const fetchDataMock = vi.spyOn(DataCache, "fetchData");
+  fetchDataMock.mockReturnValue(Promise.resolve(features));
+
   const { result } = renderHook(() => {
     const atoms = useMemo(() => computeAtom(doubleKeyCacheAtom<string, string, Feature[]>()), []);
     const [result, set] = useAtom(atoms);
@@ -189,13 +202,173 @@ test("computeAtom", async () => {
   expect(result.current.result).toBeUndefined();
 });
 
+test("computeAtom with cache", async () => {
+  const fetchDataMock = vi.spyOn(DataCache, "fetchData");
+  fetchDataMock.mockImplementation(async data => {
+    return [
+      {
+        id: (data as any).test_id || "",
+        geometry: data.value?.geometry || { type: "Point", coordinates: [0, 0] },
+      },
+    ];
+  });
+  const internalFeatures: Feature[] = [
+    { id: features[0].id, geometry: { type: "Point", coordinates: [1, 1] } },
+  ];
+  const { result } = renderHook(() => {
+    const atoms = useMemo(() => computeAtom(doubleKeyCacheAtom<string, string, Feature[]>()), []);
+    const [result, set] = useAtom(atoms);
+    return { result, set };
+  });
+
+  expect(result.current.result).toBeUndefined();
+
+  // Set data.url
+  await act(async () => {
+    await waitFor(() =>
+      result.current.set({
+        type: "setLayer",
+        layer: {
+          id: "xxx",
+          type: "simple",
+          data,
+        },
+      }),
+    );
+  });
+
+  expect(result.current.result).toEqual({
+    id: "xxx",
+    layer: {
+      id: "xxx",
+      type: "simple",
+      data,
+    },
+    status: "ready",
+    features: features,
+    originalFeatures: [...features],
+  });
+
+  expect(fetchDataMock).toBeCalledTimes(1);
+
+  const sharedData = {
+    test_id: "a",
+    type: "geojson",
+    value: { type: "Feature", geometry: internalFeatures[0].geometry },
+  } as TestData;
+
+  // Set `data.value` and add marker property.
+  // It should replace existing cache with new data.
+  await act(async () => {
+    await waitFor(() =>
+      result.current.set({
+        type: "setLayer",
+        layer: {
+          id: "xxx",
+          type: "simple",
+          data: sharedData,
+          marker: {
+            imageColor: "black",
+          },
+        },
+      }),
+    );
+  });
+
+  expect(result.current.result).toEqual({
+    id: "xxx",
+    layer: {
+      id: "xxx",
+      type: "simple",
+      data: sharedData,
+      marker: {
+        imageColor: "black",
+      },
+    },
+    status: "ready",
+    features: internalFeatures,
+    originalFeatures: [...internalFeatures],
+  });
+
+  expect(fetchDataMock).toBeCalledTimes(2);
+
+  // Set same `data.value` and change marker imageColor.
+  // It should not replace existing cache when same data is specified.
+  await act(async () => {
+    await waitFor(() =>
+      result.current.set({
+        type: "setLayer",
+        layer: {
+          id: "xxx",
+          type: "simple",
+          data: sharedData,
+          marker: {
+            imageColor: "white",
+          },
+        },
+      }),
+    );
+  });
+
+  expect(result.current.result).toEqual({
+    id: "xxx",
+    layer: {
+      id: "xxx",
+      type: "simple",
+      data: sharedData,
+      marker: {
+        imageColor: "white",
+      },
+    },
+    status: "ready",
+    features: internalFeatures,
+    originalFeatures: [...internalFeatures],
+  });
+
+  // It should not be invoked when the data is same.
+  expect(fetchDataMock).toBeCalledTimes(2);
+
+  // Set `data.url`.
+  // It should replace existing cache with new data.
+  await act(async () => {
+    await waitFor(() =>
+      result.current.set({
+        type: "setLayer",
+        layer: {
+          id: "xxx",
+          type: "simple",
+          data: {
+            test_id: "a",
+            type: "geojson",
+            url: "https://example.com/example-1.geojson",
+          } as TestData,
+        },
+      }),
+    );
+  });
+
+  expect(result.current.result).toEqual({
+    id: "xxx",
+    layer: {
+      id: "xxx",
+      type: "simple",
+      data: {
+        test_id: "a",
+        type: "geojson",
+        url: "https://example.com/example-1.geojson",
+      },
+    },
+    status: "ready",
+    features: [{ id: "a", geometry: { type: "Point", coordinates: [0, 0] } }],
+    originalFeatures: [{ id: "a", geometry: { type: "Point", coordinates: [0, 0] } }],
+  });
+
+  expect(fetchDataMock).toBeCalledTimes(3);
+});
+
 vi.mock("../evaluator", (): { evalLayer: typeof evalLayer } => ({
   evalLayer: async (layer: LayerSimple, ctx: EvalContext) => {
     if (!layer.data) return { layer: {}, features: undefined };
     return { layer: {}, features: await ctx.getAllFeatures(layer.data) };
   },
 }));
-
-vi.mock("../data", (): { fetchData: typeof fetchData } => ({
-  fetchData: async () => features,
-}));
diff --git a/src/core/mantle/atoms/compute.ts b/src/core/mantle/atoms/compute.ts
@@ -87,12 +87,10 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {
     // but attempts to retrieve data from the network if the main feature is not yet in the cache.
     const getAllFeatures = async (data: Data) => {
       const getterAll = get(dataAtoms.getAll);
+      const allFeatures = getterAll(layerId, data);
 
       // Ignore cache if data is embedded
-      if (!data.value) {
-        const allFeatures = getterAll(layerId, data);
-        if (allFeatures) return allFeatures.flat();
-      }
+      if (allFeatures) return allFeatures.flat();
 
       await set(dataAtoms.fetch, { data, layerId });
       return getterAll(layerId, data)?.flat() ?? [];
@@ -103,12 +101,10 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {
     // If it is not stored in the cache, it attempts to retrieve the data from the network.
     const getFeatures = async (data: Data, range?: DataRange) => {
       const getter = get(dataAtoms.get);
+      const c = getter(layerId, data, range);
 
       // Ignore cache if data is embedded
-      if (!data.value) {
-        const c = getter(layerId, data, range);
-        if (c) return c;
-      }
+      if (c) return c;
 
       await set(dataAtoms.fetch, { data, range, layerId });
       return getter(layerId, data, range);