Quellcode durchsuchen

feat(frontend): Cache request data (#4816)

sp.wack vor 1 Jahr
Ursprung
Commit
c3991c870d

+ 74 - 0
frontend/__tests__/utils/cache.test.ts

@@ -0,0 +1,74 @@
+import { afterEach } from "node:test";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { cache } from "#/utils/cache";
+
+describe("Cache", () => {
+  const testKey = "key";
+  const testData = { message: "Hello, world!" };
+  const testTTL = 1000; // 1 second
+
+  beforeEach(() => {
+    localStorage.clear();
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it("sets data in localStorage with expiration", () => {
+    cache.set(testKey, testData, testTTL);
+    const cachedEntry = JSON.parse(
+      localStorage.getItem(`app_cache_${testKey}`) || "",
+    );
+
+    expect(cachedEntry.data).toEqual(testData);
+    expect(cachedEntry.expiration).toBeGreaterThan(Date.now());
+  });
+
+  it("gets data from localStorage if not expired", () => {
+    cache.set(testKey, testData, testTTL);
+
+    expect(cache.get(testKey)).toEqual(testData);
+  });
+
+  it("should expire after 5 minutes by default", () => {
+    cache.set(testKey, testData);
+    expect(cache.get(testKey)).not.toBeNull();
+
+    vi.advanceTimersByTime(5 * 60 * 1000 + 1);
+
+    expect(cache.get(testKey)).toBeNull();
+    expect(localStorage.getItem(`app_cache_${testKey}`)).toBeNull();
+  });
+
+  it("returns null if cached data is expired", () => {
+    cache.set(testKey, testData, testTTL);
+
+    vi.advanceTimersByTime(testTTL + 1);
+    expect(cache.get(testKey)).toBeNull();
+    expect(localStorage.getItem(`app_cache_${testKey}`)).toBeNull();
+  });
+
+  it("deletes data from localStorage", () => {
+    cache.set(testKey, testData, testTTL);
+    cache.delete(testKey);
+
+    expect(localStorage.getItem(`app_cache_${testKey}`)).toBeNull();
+  });
+
+  it("clears all data with the app prefix from localStorage", () => {
+    cache.set(testKey, testData, testTTL);
+    cache.set("anotherKey", { data: "More data" }, testTTL);
+    cache.clearAll();
+
+    expect(localStorage.length).toBe(0);
+  });
+
+  it("does not retrieve non-prefixed data from localStorage when clearing", () => {
+    localStorage.setItem("nonPrefixedKey", "should remain");
+    cache.set(testKey, testData, testTTL);
+    cache.clearAll();
+    expect(localStorage.getItem("nonPrefixedKey")).toBe("should remain");
+  });
+});

+ 0 - 27
frontend/src/api/github.ts

@@ -139,33 +139,6 @@ export const retrieveGitHubUser = async (
   return error;
 };
 
-/**
- * Given a GitHub token and a repository name, creates a repository for the authenticated user
- * @param token The GitHub token
- * @param repositoryName Name of the repository to create
- * @param description Description of the repository
- * @param isPrivate Boolean indicating if the repository should be private
- * @returns The created repository or an error response
- */
-export const createGitHubRepository = async (
-  token: string,
-  repositoryName: string,
-  description?: string,
-  isPrivate = true,
-): Promise<GitHubRepository | GitHubErrorReponse> => {
-  const response = await fetch("https://api.github.com/user/repos", {
-    method: "POST",
-    headers: generateGitHubAPIHeaders(token),
-    body: JSON.stringify({
-      name: repositoryName,
-      description,
-      private: isPrivate,
-    }),
-  });
-
-  return response.json();
-};
-
 export const retrieveLatestGitHubCommit = async (
   token: string,
   repository: string,

+ 29 - 4
frontend/src/api/open-hands.ts

@@ -1,4 +1,5 @@
 import { request } from "#/services/api";
+import { cache } from "#/utils/cache";
 import {
   SaveFileSuccessResponse,
   FileUploadSuccessResponse,
@@ -15,7 +16,13 @@ class OpenHands {
    * @returns List of models available
    */
   static async getModels(): Promise<string[]> {
-    return request("/api/options/models");
+    const cachedData = cache.get<string[]>("models");
+    if (cachedData) return cachedData;
+
+    const data = await request("/api/options/models");
+    cache.set("models", data);
+
+    return data;
   }
 
   /**
@@ -23,7 +30,13 @@ class OpenHands {
    * @returns List of agents available
    */
   static async getAgents(): Promise<string[]> {
-    return request(`/api/options/agents`);
+    const cachedData = cache.get<string[]>("agents");
+    if (cachedData) return cachedData;
+
+    const data = await request(`/api/options/agents`);
+    cache.set("agents", data);
+
+    return data;
   }
 
   /**
@@ -31,11 +44,23 @@ class OpenHands {
    * @returns List of security analyzers available
    */
   static async getSecurityAnalyzers(): Promise<string[]> {
-    return request(`/api/options/security-analyzers`);
+    const cachedData = cache.get<string[]>("agents");
+    if (cachedData) return cachedData;
+
+    const data = await request(`/api/options/security-analyzers`);
+    cache.set("security-analyzers", data);
+
+    return data;
   }
 
   static async getConfig(): Promise<GetConfigResponse> {
-    return request("/config.json");
+    const cachedData = cache.get<GetConfigResponse>("config");
+    if (cachedData) return cachedData;
+
+    const data = await request("/config.json");
+    cache.set("config", data);
+
+    return data;
   }
 
   /**

+ 1 - 7
frontend/src/routes/_oh._index/route.tsx

@@ -25,9 +25,6 @@ import { generateGitHubAuthUrl } from "#/utils/generate-github-auth-url";
 import { GitHubRepositoriesSuggestionBox } from "#/components/github-repositories-suggestion-box";
 import { convertZipToBase64 } from "#/utils/convert-zip-to-base64";
 
-let cachedRepositories: ReturnType<
-  typeof retrieveAllGitHubUserRepositories
-> | null = null;
 export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
   let isSaas = false;
   let githubClientId: string | null = null;
@@ -48,12 +45,9 @@ export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
   let repositories: ReturnType<
     typeof retrieveAllGitHubUserRepositories
   > | null = null;
-  if (cachedRepositories) {
-    repositories = cachedRepositories;
-  } else if (ghToken) {
+  if (ghToken) {
     const data = retrieveAllGitHubUserRepositories(ghToken);
     repositories = data;
-    cachedRepositories = data;
   }
 
   let githubAuthUrl: string | null = null;

+ 9 - 13
frontend/src/routes/_oh.app.tsx

@@ -69,8 +69,6 @@ const isAgentStateChange = (
   data.extras instanceof Object &&
   "agent_state" in data.extras;
 
-let lastCommitCached: GitHubCommit | null = null;
-let repoForLastCommit: string | null = null;
 export const clientLoader = async () => {
   const ghToken = localStorage.getItem("ghToken");
 
@@ -84,16 +82,14 @@ export const clientLoader = async () => {
 
   if (repo) localStorage.setItem("repo", repo);
 
-  if (!lastCommitCached || repoForLastCommit !== repo) {
-    if (ghToken && repo) {
-      const data = await retrieveLatestGitHubCommit(ghToken, repo);
-      if (isGitHubErrorReponse(data)) {
-        // TODO: Handle error
-        console.error("Failed to retrieve latest commit", data);
-      } else {
-        [lastCommitCached] = data;
-        repoForLastCommit = repo;
-      }
+  let lastCommit: GitHubCommit | null = null;
+  if (ghToken && repo) {
+    const data = await retrieveLatestGitHubCommit(ghToken, repo);
+    if (isGitHubErrorReponse(data)) {
+      // TODO: Handle error
+      console.error("Failed to retrieve latest commit", data);
+    } else {
+      [lastCommit] = data;
     }
   }
 
@@ -103,7 +99,7 @@ export const clientLoader = async () => {
     ghToken,
     repo,
     q,
-    lastCommit: lastCommitCached,
+    lastCommit,
   });
 };
 

+ 15 - 26
frontend/src/routes/_oh.tsx

@@ -34,9 +34,6 @@ import { AnalyticsConsentFormModal } from "#/components/analytics-consent-form-m
 import { setCurrentAgentState } from "#/state/agentSlice";
 import AgentState from "#/types/AgentState";
 
-// Cache for clientLoader results
-let clientLoaderCache: any = null; // eslint-disable-line @typescript-eslint/no-explicit-any
-
 export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
   try {
     const config = await OpenHands.getConfig();
@@ -61,28 +58,22 @@ export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
   let isAuthed = false;
   let githubAuthUrl: string | null = null;
   let user: GitHubUser | GitHubErrorReponse | null = null;
-  if (!clientLoaderCache || clientLoaderCache.ghToken !== ghToken) {
-    try {
-      isAuthed = await userIsAuthenticated();
-      if (!isAuthed && window.__GITHUB_CLIENT_ID__) {
-        const requestUrl = new URL(request.url);
-        githubAuthUrl = generateGitHubAuthUrl(
-          window.__GITHUB_CLIENT_ID__,
-          requestUrl,
-        );
-      }
-    } catch (error) {
-      isAuthed = false;
-      githubAuthUrl = null;
+  try {
+    isAuthed = await userIsAuthenticated();
+    if (!isAuthed && window.__GITHUB_CLIENT_ID__) {
+      const requestUrl = new URL(request.url);
+      githubAuthUrl = generateGitHubAuthUrl(
+        window.__GITHUB_CLIENT_ID__,
+        requestUrl,
+      );
     }
-
-    if (ghToken) user = await retrieveGitHubUser(ghToken);
-  } else {
-    isAuthed = clientLoaderCache.isAuthed;
-    githubAuthUrl = clientLoaderCache.githubAuthUrl;
-    user = clientLoaderCache.user;
+  } catch (error) {
+    isAuthed = false;
+    githubAuthUrl = null;
   }
 
+  if (ghToken) user = await retrieveGitHubUser(ghToken);
+
   const settings = getSettings();
   await i18n.changeLanguage(settings.LANGUAGE);
 
@@ -93,7 +84,7 @@ export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
   }
 
   // Store the results in cache
-  clientLoaderCache = {
+  return defer({
     token,
     ghToken,
     isAuthed,
@@ -102,9 +93,7 @@ export const clientLoader = async ({ request }: ClientLoaderFunctionArgs) => {
     settingsIsUpdated,
     settings,
     analyticsConsent,
-  };
-
-  return defer(clientLoaderCache);
+  });
 };
 
 export function ErrorBoundary() {

+ 2 - 0
frontend/src/routes/logout.ts

@@ -1,10 +1,12 @@
 import { json } from "@remix-run/react";
 import posthog from "posthog-js";
+import { cache } from "#/utils/cache";
 
 export const clientAction = () => {
   const ghToken = localStorage.getItem("ghToken");
   if (ghToken) localStorage.removeItem("ghToken");
 
+  cache.clearAll();
   posthog.reset();
 
   return json({ success: true });

+ 70 - 0
frontend/src/utils/cache.ts

@@ -0,0 +1,70 @@
+type CacheKey = string;
+type CacheEntry<T> = {
+  data: T;
+  expiration: number;
+};
+
+class Cache {
+  private prefix = "app_cache_";
+
+  private defaultTTL = 5 * 60 * 1000; // 5 minutes
+
+  /**
+   * Generate a unique key with prefix for local storage
+   * @param key The key to be stored in local storage
+   * @returns The unique key with prefix
+   */
+  private getKey(key: CacheKey): string {
+    return `${this.prefix}${key}`;
+  }
+
+  /**
+   * Retrieve the cached data from local storage
+   * @param key The key to be retrieved from local storage
+   * @returns The data stored in local storage
+   */
+  public get<T>(key: CacheKey): T | null {
+    const cachedEntry = localStorage.getItem(this.getKey(key));
+    if (cachedEntry) {
+      const { data, expiration } = JSON.parse(cachedEntry) as CacheEntry<T>;
+      if (Date.now() < expiration) return data;
+      this.delete(key); // Remove expired cache
+    }
+
+    return null;
+  }
+
+  /**
+   * Store the data in local storage with expiration
+   * @param key The key to be stored in local storage
+   * @param data The data to be stored in local storage
+   * @param ttl The time to live for the data in milliseconds
+   * @returns void
+   */
+  public set<T>(key: CacheKey, data: T, ttl = this.defaultTTL): void {
+    const expiration = Date.now() + ttl;
+    const entry: CacheEntry<T> = { data, expiration };
+    localStorage.setItem(this.getKey(key), JSON.stringify(entry));
+  }
+
+  /**
+   * Remove the data from local storage
+   * @param key The key to be removed from local storage
+   * @returns void
+   */
+  public delete(key: CacheKey): void {
+    localStorage.removeItem(this.getKey(key));
+  }
+
+  /**
+   * Clear all data with the app prefix from local storage
+   * @returns void
+   */
+  public clearAll(): void {
+    Object.keys(localStorage).forEach((key) => {
+      if (key.startsWith(this.prefix)) localStorage.removeItem(key);
+    });
+  }
+}
+
+export const cache = new Cache();

+ 10 - 2
frontend/src/utils/user-is-authenticated.ts

@@ -1,12 +1,20 @@
 import OpenHands from "#/api/open-hands";
+import { cache } from "./cache";
 
 export const userIsAuthenticated = async () => {
   if (window.__APP_MODE__ === "oss") return true;
 
+  const cachedData = cache.get<boolean>("user_is_authenticated");
+  if (cachedData) return cachedData;
+
+  let authenticated = false;
   try {
     await OpenHands.authenticate();
-    return true;
+    authenticated = true;
   } catch (error) {
-    return false;
+    authenticated = false;
   }
+
+  cache.set("user_is_authenticated", authenticated, 3 * 60 * 1000); // cache for 3 minutes
+  return authenticated;
 };