impr: lazy load chartData on results (@fehmer) (#6428)

Optimize results endpoint by removing heavy or unused data. 

We load the whole result chart data for up to 1000 results each time,
but it is very unlikely the user will view the charts for all old
results. By removing the size in my tests went down from 1152kb to
276kb.

---------

Co-authored-by: Miodec <jack@monkeytype.com>

backend/__tests__/api/controllers/result.spec.ts

@@ -333,6 +333,87 @@ describe("result controller test", () => {
       ).toBeRateLimited({ max: 30, windowMs: 24 * 60 * 60 * 1000 });
     });
   });
+  describe("getResultById", () => {
+    const getResultMock = vi.spyOn(ResultDal, "getResult");
+
+    afterEach(() => {
+      getResultMock.mockReset();
+    });
+
+    it("should get result", async () => {
+      //GIVEN
+      const result = givenDbResult(uid);
+      getResultMock.mockResolvedValue(result);
+
+      //WHEN
+      const { body } = await mockApp
+        .get(`/results/id/${result._id}`)
+        .set("Authorization", `Bearer ${uid}`)
+        .send()
+        .expect(200);
+
+      //THEN
+      expect(body.message).toEqual("Result retrieved");
+      expect(body.data).toEqual({ ...result, _id: result._id.toHexString() });
+    });
+    it("should get last result with ape key", async () => {
+      //GIVEN
+      await acceptApeKeys(true);
+      const apeKey = await mockAuthenticateWithApeKey(uid, await configuration);
+      const result = givenDbResult(uid);
+      getResultMock.mockResolvedValue(result);
+
+      //WHEN
+      await mockApp
+        .get(`/results/id/${result._id}`)
+        .set("Authorization", `ApeKey ${apeKey}`)
+        .send()
+        .expect(200);
+    });
+    it("should get last result with legacy values", async () => {
+      //GIVEN
+      const result = givenDbResult(uid, {
+        charStats: undefined,
+        incorrectChars: 5,
+        correctChars: 12,
+      });
+      getResultMock.mockResolvedValue(result);
+
+      //WHEN
+      const { body } = await mockApp
+        .get(`/results/id/${result._id}`)
+        .set("Authorization", `Bearer ${uid}`)
+        .send()
+        .expect(200);
+
+      //THEN
+      expect(body.message).toEqual("Result retrieved");
+      expect(body.data).toMatchObject({
+        _id: result._id.toHexString(),
+        charStats: [12, 5, 0, 0],
+      });
+      expect(body.data).not.toHaveProperty("correctChars");
+      expect(body.data).not.toHaveProperty("incorrectChars");
+    });
+    it("should rate limit get  result with ape key", async () => {
+      //GIVEN
+      const result = givenDbResult(uid, {
+        charStats: undefined,
+        incorrectChars: 5,
+        correctChars: 12,
+      });
+      getResultMock.mockResolvedValue(result);
+      await acceptApeKeys(true);
+      const apeKey = await mockAuthenticateWithApeKey(uid, await configuration);
+
+      //WHEN
+      await expect(
+        mockApp
+          .get(`/results/id/${result._id}`)
+          .set("Authorization", `ApeKey ${apeKey}`)
+      ).toBeRateLimited({ max: 60, windowMs: 60 * 60 * 1000 });
+    });
+  });
   describe("getLastResult", () => {
     const getLastResultMock = vi.spyOn(ResultDal, "getLastResult");
 

backend/src/api/controllers/result.ts

@@ -37,6 +37,8 @@ import {
   AddResultRequest,
   AddResultResponse,
   GetLastResultResponse,
+  GetResultByIdPath,
+  GetResultByIdResponse,
   GetResultsQuery,
   GetResultsResponse,
   UpdateResultTagsRequest,
@@ -131,12 +133,22 @@ export async function getResults(
   return new MonkeyResponse("Results retrieved", results.map(convertResult));
 }
 
+export async function getResultById(
+  req: MonkeyRequest<undefined, undefined, GetResultByIdPath>
+): Promise<GetResultByIdResponse> {
+  const { uid } = req.ctx.decodedToken;
+  const { resultId } = req.params;
+
+  const result = await ResultDAL.getResult(uid, resultId);
+  return new MonkeyResponse("Result retrieved", convertResult(result));
+}
+
 export async function getLastResult(
   req: MonkeyRequest
 ): Promise<GetLastResultResponse> {
   const { uid } = req.ctx.decodedToken;
-  const results = await ResultDAL.getLastResult(uid);
-  return new MonkeyResponse("Result retrieved", convertResult(results));
+  const result = await ResultDAL.getLastResult(uid);
+  return new MonkeyResponse("Result retrieved", convertResult(result));
 }
 
 export async function deleteAll(req: MonkeyRequest): Promise<MonkeyResponse> {

backend/src/api/routes/results.ts

@@ -8,6 +8,9 @@ export default s.router(resultsContract, {
   get: {
     handler: async (r) => callController(ResultController.getResults)(r),
   },
+  getById: {
+    handler: async (r) => callController(ResultController.getResultById)(r),
+  },
   add: {
     handler: async (r) => callController(ResultController.addResult)(r),
   },

backend/src/dal/result.ts

@@ -100,13 +100,23 @@ export async function getResults(
 ): Promise<DBResult[]> {
   const { onOrAfterTimestamp, offset, limit } = opts ?? {};
   let query = getResultCollection()
-    .find({
-      uid,
-      ...(!_.isNil(onOrAfterTimestamp) &&
-        !_.isNaN(onOrAfterTimestamp) && {
-          timestamp: { $gte: onOrAfterTimestamp },
-        }),
-    })
+    .find(
+      {
+        uid,
+        ...(!_.isNil(onOrAfterTimestamp) &&
+          !_.isNaN(onOrAfterTimestamp) && {
+            timestamp: { $gte: onOrAfterTimestamp },
+          }),
+      },
+      {
+        projection: {
+          chartData: 0,
+          keySpacingStats: 0,
+          keyDurationStats: 0,
+          name: 0,
+        },
+      }
+    )
     .sort({ timestamp: -1 });
 
   if (limit !== undefined) {

frontend/src/styles/account.scss

@@ -330,12 +330,19 @@
       margin: 0 0.1rem;
     }
     .miniResultChartButton {
-      opacity: 0.25;
       transition: 0.25s;
       cursor: pointer;
+      color: var(--text-color);
       &:hover {
         opacity: 1;
       }
+      &.loading {
+        pointer-events: none;
+      }
+      &.disabled .fas {
+        opacity: 0.5;
+        color: var(--sub-color);
+      }
     }
   }
 

frontend/src/ts/pages/account.ts

@@ -38,6 +38,7 @@ import { ResultFiltersGroupItem } from "@monkeytype/contracts/schemas/users";
 import { findLineByLeastSquares } from "../utils/numbers";
 import defaultResultFilters from "../constants/default-result-filters";
 import { SnapshotResult } from "../constants/default-snapshot";
+import Ape from "../ape";
 
 let filterDebug = false;
 //toggle filterdebug
@@ -105,12 +106,10 @@ function loadMoreLines(lineIndex?: number): void {
         )}" data-balloon-pos="up"><i class="fas fa-gamepad"></i></span>`;
     }
 
-    if (result.chartData === undefined) {
-      icons += `<span class="miniResultChartButton" aria-label="No chart data found" data-balloon-pos="up"><i class="fas fa-chart-line"></i></span>`;
-    } else if (result.chartData === "toolong") {
-      icons += `<span class="miniResultChartButton" aria-label="Chart history is not available for long tests" data-balloon-pos="up"><i class="fas fa-chart-line"></i></span>`;
+    if (result.chartData === "toolong" || result.testDuration > 122) {
+      icons += `<span class="miniResultChartButton disabled" aria-label="Graph history is not available for long tests" data-balloon-pos="up"><i class="fas fa-fw fa-chart-line"></i></span>`;
     } else {
-      icons += `<span class="miniResultChartButton" aria-label="View graph" data-balloon-pos="up" filteredResultsId="${i}" style="opacity: 1"><i class="fas fa-chart-line"></i></span>`;
+      icons += `<span class="miniResultChartButton" aria-label="View graph" data-balloon-pos="up" filteredResultsId="${i}"><i class="fas fa-fw fa-chart-line"></i></span>`;
     }
 
     let tagNames = "no tags";
@@ -1152,13 +1151,64 @@ $(".pageAccount #accountHistoryChart").on("click", () => {
   $(`#result-${index}`).addClass("active");
 });
 
-$(".pageAccount").on("click", ".miniResultChartButton", (event) => {
-  console.log("updating");
-  const filteredId = $(event.currentTarget).attr("filteredResultsId");
+$(".pageAccount").on("click", ".miniResultChartButton", async (event) => {
+  const target = $(event.currentTarget);
+  if (target.hasClass("loading")) return;
+  if (target.hasClass("disabled")) return;
+
+  const filteredId = target.attr("filteredResultsId");
   if (filteredId === undefined) return;
-  MiniResultChartModal.show(
-    filteredResults[parseInt(filteredId)]?.chartData as ChartData
-  );
+
+  const result = filteredResults[parseInt(filteredId)];
+  if (result === undefined) return;
+
+  let chartData = result.chartData as ChartData;
+
+  if (chartData === undefined) {
+    //need to load full result
+    target.addClass("loading");
+    target.attr("aria-label", null);
+    target.html('<i class="fas fa-fw fa-spin fa-circle-notch"></i>');
+    Loader.show();
+
+    const response = await Ape.results.getById({
+      params: { resultId: result._id },
+    });
+    Loader.hide();
+
+    target.html('<i class="fas fa-fw fa-chart-line"></i>');
+    target.removeClass("loading");
+
+    if (response.status !== 200) {
+      Notifications.add("Error fetching result: " + response.body.message, -1);
+      return;
+    }
+
+    chartData = response.body.data.chartData as ChartData;
+
+    //update local cache
+    result.chartData = chartData;
+    const dbResult = DB.getSnapshot()?.results?.find(
+      (it) => it._id === result._id
+    );
+    if (dbResult !== undefined) {
+      dbResult["chartData"] = result.chartData;
+    }
+
+    if (response.body.data.chartData === "toolong") {
+      target.attr(
+        "aria-label",
+        "Graph history is not available for long tests"
+      );
+      target.attr("data-baloon-pos", "up");
+      target.addClass("disabled");
+
+      Notifications.add("Graph history is not available for long tests", 0);
+      return;
+    }
+  }
+  target.attr("aria-label", "View graph");
+  MiniResultChartModal.show(chartData);
 });
 
 $(".pageAccount .group.history").on("click", ".history-wpm-header", () => {

packages/contracts/src/rate-limit/index.ts

@@ -138,6 +138,18 @@ export const limits = {
     max: 30,
   },
 
+  // Result by id
+  resultByIdGet: {
+    window: "hour",
+    max: 300,
+  },
+
+  // Result by id
+  resultByIdGetApe: {
+    window: "hour",
+    max: 60,
+  },
+
   resultsAdd: {
     window: "hour",
     max: 300,

packages/contracts/src/results.ts

@@ -9,6 +9,7 @@ import {
 import {
   CompletedEventSchema,
   PostResultResponseSchema,
+  ResultMinifiedSchema,
   ResultSchema,
 } from "./schemas/results";
 import { IdSchema } from "./schemas/util";
@@ -38,9 +39,20 @@ export const GetResultsQuerySchema = z.object({
 });
 export type GetResultsQuery = z.infer<typeof GetResultsQuerySchema>;
 
-export const GetResultsResponseSchema = responseWithData(z.array(ResultSchema));
+export const GetResultsResponseSchema = responseWithData(
+  z.array(ResultMinifiedSchema)
+);
 export type GetResultsResponse = z.infer<typeof GetResultsResponseSchema>;
 
+export const GetResultByIdPathSchema = z.object({
+  resultId: IdSchema,
+});
+export type GetResultByIdPath = z.infer<typeof GetResultByIdPathSchema>;
+
+export const GetResultByIdResponseSchema = responseWithData(ResultSchema);
+
+export type GetResultByIdResponse = z.infer<typeof GetResultByIdResponseSchema>;
+
 export const AddResultRequestSchema = z.object({
   result: CompletedEventSchema,
 });
@@ -92,6 +104,25 @@ export const resultsContract = c.router(
         },
       }),
     },
+    getById: {
+      summary: "get result by id",
+      description: "Get result by id",
+      method: "GET",
+      path: "/id/:resultId",
+      pathParams: GetResultByIdPathSchema,
+      responses: {
+        200: GetResultByIdResponseSchema,
+      },
+      metadata: meta({
+        authenticationOptions: {
+          acceptApeKeys: true,
+        },
+        rateLimit: {
+          normal: "resultByIdGet",
+          apeKey: "resultByIdGetApe",
+        },
+      }),
+    },
     add: {
       summary: "add result",
       description: "Add a test result for the current user",

packages/contracts/src/schemas/results.ts

@@ -95,6 +95,14 @@ export type Result<M extends Mode> = Omit<
   mode2: Mode2<M>;
 };
 
+export const ResultMinifiedSchema = ResultSchema.omit({
+  name: true,
+  keySpacingStats: true,
+  keyDurationStats: true,
+  chartData: true,
+});
+export type ResultMinified = z.infer<typeof ResultMinifiedSchema>;
+
 export const CompletedEventSchema = ResultBaseSchema.required({
   restartCount: true,
   incompleteTestSeconds: true,