wip

Author: thomaswhyyou

packages/client/src/clients/guide/client.ts

@@ -7,7 +7,7 @@ import Knock from "../../knock";
 
 import {
   DEFAULT_GROUP_KEY,
-  SelectionResult,
+  // SelectionResult,
   byKey,
   checkStateIfThrottled,
   findDefaultGroup,
@@ -46,7 +46,9 @@ import {
   SelectFilterParams,
   SelectGuideOpts,
   SelectGuidesOpts,
-  SelectQueryParams,
+  SelectQueryLimit,
+  SelectionResult,
+  // SelectQueryParams,
   StepMessageState,
   StoreState,
   TargetParams,
@@ -151,7 +153,16 @@ const safeJsonParseDebugParams = (value: string): DebugState => {
   }
 };
 
-const select = (state: StoreState, filters: SelectFilterParams = {}) => {
+type SelectQueryMetadata = {
+  limit: SelectQueryLimit;
+  opts: SelectGuideOpts;
+};
+
+const select = (
+  state: StoreState,
+  filters: SelectFilterParams,
+  metadata: SelectQueryMetadata,
+) => {
   // A map of selected guides as values, with its order index as keys.
   const result = new SelectionResult();
 
@@ -180,7 +191,8 @@ const select = (state: StoreState, filters: SelectFilterParams = {}) => {
     result.set(index, guide);
   }
 
-  result.metadata = { guideGroup: defaultGroup, filters };
+  result.metadata = { guideGroup: defaultGroup, filters, ...metadata };
+
   return result;
 };
 
@@ -614,29 +626,35 @@ export class KnockGuideClient {
       `[Guide] .selectGuides (filters: ${formatFilters(filters)}; state: ${formatState(state)})`,
     );
 
+    // 1. First, call selectGuide() using the same filters to ensure we have a
+    // group stage open and respect throttling. This isn't the real query, but
+    // rather it's a shortcut ahead of handling the actual query result below.
     const selectedGuide = this.selectGuide(state, filters, {
       ...opts,
-      // Do not record this selectGuide() query, since this is a short to check
-      // throttling, and we should be recording the actual selectGuides query
-      // (as needed).
+      // Don't record this result, not the actual query result we need.
       recordSelectQuery: false,
     });
 
-    // Record AFTER the selectGuide() query to ensure we have a group stage open
-    // to be able to record.
+    // 2. Now make the actual select query with the provided filters and opts,
+    // and record the result (as needed). By default, we only record the result
+    // while in debugging.
     const { recordSelectQuery = !!state.debug?.debugging } = opts;
-    this.maybeRecordSelectQuery(
-      { ...filters, limit: "one" },
-      { ...opts, recordSelectQuery },
-    );
+    const metadata: SelectQueryMetadata = {
+      limit: "all",
+      opts: { ...opts, recordSelectQuery },
+    };
+    const result = select(state, filters, metadata);
+    this.maybeRecordSelectResult(result);
 
+    // 3. Stop if there is not at least one guide to return.
     if (!selectedGuide) {
       return [];
     }
 
     // There should be at least one guide to return here now.
-    const guides = [...select(state, filters).values()];
+    const guides = [...result.values()];
 
+    // 4. If throttled, filter out any throttled guides.
     if (!opts.includeThrottled && checkStateIfThrottled(state)) {
       const unthrottledGuides = guides.filter(
         (g) => g.bypass_global_group_limit,
@@ -702,15 +720,15 @@ export class KnockGuideClient {
       this.stage = this.openGroupStage(); // Assign here to make tsc happy
     }
 
-    // Must come AFTER we ensure a group stage exists, so we can record select
-    // queries.
+    // Must come AFTER we ensure a group stage exists above, so we can record
+    // select queries. By default, we only record the result while in debugging.
     const { recordSelectQuery = !!state.debug?.debugging } = opts;
-    this.maybeRecordSelectQuery(
-      { ...filters, limit: "one" },
-      { ...opts, recordSelectQuery },
-    );
-
-    const result = select(state, filters);
+    const metadata: SelectQueryMetadata = {
+      limit: "one",
+      opts: { ...opts, recordSelectQuery },
+    };
+    const result = select(state, filters, metadata);
+    this.maybeRecordSelectResult(result);
 
     if (result.size === 0) {
       this.knock.log("[Guide] Selection found zero result");
@@ -731,10 +749,12 @@ export class KnockGuideClient {
 
     // Check if inside the throttle window (i.e. throttled) and if so stop and
     // return undefined unless explicitly given the option to include throttled.
-    if (!opts.includeThrottled && checkStateIfThrottled(state)) {
-      this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`);
-      return undefined;
-    }
+    const throttled = !opts.includeThrottled && checkStateIfThrottled(state);
+
+    // if (!opts.includeThrottled && checkStateIfThrottled(state)) {
+    //   this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`);
+    //   return undefined;
+    // }
 
     switch (this.stage.status) {
       case "open": {
@@ -747,6 +767,11 @@ export class KnockGuideClient {
         this.knock.log(`[Guide] Patching the group stage: ${guide.key}`);
         this.stage.ordered[index] = guide.key;
 
+        if (throttled) {
+          this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`);
+          return undefined;
+        }
+
         const ret = this.stage.resolved === guide.key ? guide : undefined;
         this.knock.log(
           `[Guide] Returning \`${ret?.key}\` (stage: ${formatGroupStage(this.stage)})`,
@@ -755,6 +780,11 @@ export class KnockGuideClient {
       }
 
       case "closed": {
+        if (throttled) {
+          this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`);
+          return undefined;
+        }
+
         const ret = this.stage.resolved === guide.key ? guide : undefined;
         this.knock.log(
           `[Guide] Returning \`${ret?.key}\` (stage: ${formatGroupStage(this.stage)})`,
@@ -764,36 +794,66 @@ export class KnockGuideClient {
     }
   }
 
-  private maybeRecordSelectQuery(
-    params: SelectQueryParams,
-    opts: SelectGuideOpts,
-  ) {
+  private maybeRecordSelectResult(result: SelectionResult) {
+    if (!result.metadata) return;
+
+    const { opts, filters, limit } = result.metadata;
     if (!opts.recordSelectQuery) return;
+    if (!filters.key && !filters.type) return;
     if (!this.stage || this.stage.status === "closed") return;
-    if (!params.key && !params.type) return;
-
-    // Deep merge into the query logs:
-    const queriesByKey = this.stage.queries.key || {};
-    if (params.key) {
-      queriesByKey[params.key] = {
-        ...(queriesByKey[params.key] || {}),
-        ...{ [params.limit]: opts },
+
+    // Deep merge to accumulate the results.
+    const queriedByKey = this.stage.results.key || {};
+    if (filters.key) {
+      queriedByKey[filters.key] = {
+        ...(queriedByKey[filters.key] || {}),
+        ...{ [limit]: result },
       };
     }
-    const queriesByType = this.stage.queries.type || {};
-    if (params.type) {
-      queriesByType[params.type] = {
-        ...(queriesByType[params.type] || {}),
-        ...{ [params.limit]: opts },
+    const queriedByType = this.stage.results.type || {};
+    if (filters.type) {
+      queriedByType[filters.type] = {
+        ...(queriedByType[filters.type] || {}),
+        ...{ [limit]: result },
       };
     }
 
     this.stage = {
       ...this.stage,
-      queries: { key: queriesByKey, type: queriesByType },
+      results: { key: queriedByKey, type: queriedByType },
     };
   }
 
+  // private maybeRecordSelectQuery(
+  //   params: SelectQueryParams,
+  //   opts: SelectGuideOpts,
+  // ) {
+  //   if (!opts.recordSelectQuery) return;
+  //   if (!this.stage || this.stage.status === "closed") return;
+  //   if (!params.key && !params.type) return;
+  //
+  //   // Deep merge into the query logs:
+  //   const queriesByKey = this.stage.queries.key || {};
+  //   if (params.key) {
+  //     queriesByKey[params.key] = {
+  //       ...(queriesByKey[params.key] || {}),
+  //       ...{ [params.limit]: opts },
+  //     };
+  //   }
+  //   const queriesByType = this.stage.queries.type || {};
+  //   if (params.type) {
+  //     queriesByType[params.type] = {
+  //       ...(queriesByType[params.type] || {}),
+  //       ...{ [params.limit]: opts },
+  //     };
+  //   }
+  //
+  //   this.stage = {
+  //     ...this.stage,
+  //     queries: { key: queriesByKey, type: queriesByType },
+  //   };
+  // }
+
   getStage() {
     return this.stage;
   }
@@ -813,7 +873,7 @@ export class KnockGuideClient {
     this.stage = {
       status: "open",
       ordered: [],
-      queries: {},
+      results: {},
       timeoutId,
     };
 

packages/client/src/clients/guide/helpers.ts

@@ -3,28 +3,14 @@ import {
   GuideActivationUrlRuleData,
   GuideData,
   GuideGroupData,
-  KnockGuide,
+  // KnockGuide,
   KnockGuideActivationUrlPattern,
   SelectFilterParams,
+  // SelectGuideOpts,
+  // SelectQueryLimit,
   StoreState,
 } from "./types";
 
-type SelectionResultMetadata = {
-  guideGroup: GuideGroupData;
-  filters: SelectFilterParams;
-};
-
-// Extends the map class to allow having metadata on it, which is used to record
-// the guide group context for the selection result (though currently only a
-// default global group is supported).
-export class SelectionResult<K = number, V = KnockGuide> extends Map<K, V> {
-  metadata: SelectionResultMetadata | undefined;
-
-  constructor() {
-    super();
-  }
-}
-
 export const formatGroupStage = (stage: GroupStage) => {
   return `status=${stage.status}, resolved=${stage.resolved}`;
 };

packages/client/src/clients/guide/index.ts

@@ -14,4 +14,5 @@ export type {
   SelectGuidesOpts as KnockSelectGuidesOpts,
   StoreState as KnockGuideClientStoreState,
   GroupStage as KnockGuideClientGroupStage,
+  SelectionResult as KnockGuideSelectionResult,
 } from "./types";

packages/client/src/clients/guide/types.ts

@@ -1,5 +1,24 @@
 import { GenericData } from "@knocklabs/types";
 
+type SelectionResultMetadata = {
+  guideGroup: GuideGroupData;
+  // Additional info about the underlying select query behind the result.
+  filters: SelectFilterParams;
+  limit: SelectQueryLimit;
+  opts: SelectGuideOpts;
+};
+
+// Extends the map class to allow having metadata on it, which is used to record
+// the guide group context for the selection result (though currently only a
+// default global group is supported).
+export class SelectionResult<K = number, V = KnockGuide> extends Map<K, V> {
+  metadata: SelectionResultMetadata | undefined;
+
+  constructor() {
+    super();
+  }
+}
+
 //
 // Fetch guides API
 //
@@ -231,6 +250,7 @@ export type SelectFilterParams = {
 
 export type SelectGuideOpts = {
   includeThrottled?: boolean;
+  // XXX: record result
   recordSelectQuery?: boolean;
 };
 
@@ -248,24 +268,27 @@ export type ConstructorOpts = {
   throttleCheckInterval?: number;
 };
 
-export type SelectQueryParams = SelectFilterParams & {
-  limit: "one" | "all";
-};
+// i.e. useGuide vs useGuides
+export type SelectQueryLimit = "one" | "all";
+
+// export type SelectQueryParams = SelectFilterParams & {
+//   limit: "one" | "all";
+// };
 
-type SelectGuideOptsByLimit = {
-  one?: SelectGuideOpts;
-  all?: SelectGuideOpts;
+type SelectionResultByLimit = {
+  one?: SelectionResult;
+  all?: SelectionResult;
 };
 
-type RecordedSelectQueries = {
-  key?: Record<KnockGuide["key"], SelectGuideOptsByLimit>;
-  type?: Record<KnockGuide["key"], SelectGuideOptsByLimit>;
+type RecordedSelectionResults = {
+  key?: Record<KnockGuide["key"], SelectionResultByLimit>;
+  type?: Record<KnockGuide["type"], SelectionResultByLimit>;
 };
 
 export type GroupStage = {
   status: "open" | "closed" | "patch";
   ordered: Array<KnockGuide["key"]>;
   resolved?: KnockGuide["key"];
-  queries: RecordedSelectQueries;
   timeoutId: ReturnType<typeof setTimeout> | null;
+  results: RecordedSelectionResults;
 };