fix: /app-config returns valid devflags

Suppose we have the following scenario: DEFAULT_DEVFLAGS = { a: {} } DB overrides = { a: { b: true } } When the server starts, DEVFLAGS and /app-config have the same data: DEVFLAGS = { a: { b: true } } /app-config returns { a: { b : true } } While the server is still running, if the DB override are changed: DB overrides = { a: { c: true } } The expected data is: DEVFLAGS = { a: { b: true } } /app-config returns { a: { c: true } } However, the bug is that /app-config will merge based on the current DEVFLAGS: /app-config returns { a: { b: true, c: true } } This commit ensures overrides are merged from DEFAULT_DEVFLAGS. Change-Id: I88a3c8db646b37eb3be7355c3d3a8993bea5ad59 GitOrigin-RevId: 72c263a3aec874d54f6617a1735125e3fa644c9f
plasmicapp · Dec 12, 2024 · fbcf211 · fbcf211
1 parent 112b4c3
commit fbcf211
Show file tree

Hide file tree

Showing 9 changed files with 184 additions and 27 deletions.
diff --git a/platform/wab/src/wab/client/app-ctx.ts b/platform/wab/src/wab/client/app-ctx.ts
@@ -20,6 +20,7 @@ import { ensure, swallowAsync } from "@/wab/shared/common";
 import { isAdminTeamEmail } from "@/wab/shared/devflag-utils";
 import {
   applyDevFlagOverrides,
+  applyDevFlagOverridesToTarget,
   applyPlasmicUserDevFlagOverrides,
   DEVFLAGS,
   DevFlagsType,
@@ -475,10 +476,10 @@ export async function loadAppCtx(
     // Next apply client-specified overrides, via url params and
     // localStorage
     const clientConfigOverrides = getClientDevFlagOverrides();
-    applyDevFlagOverrides(appConfigOverrides, clientConfigOverrides);
+    applyDevFlagOverridesToTarget(appConfigOverrides, clientConfigOverrides);
 
     // Apply to DEVFLAGS
-    applyDevFlagOverrides(DEVFLAGS, appConfigOverrides);
+    applyDevFlagOverrides(appConfigOverrides);
 
     const starters = loadStarters(baseApi, user, dbConfigOverrides);
 

diff --git a/platform/wab/src/wab/client/client-dev-flags.ts b/platform/wab/src/wab/client/client-dev-flags.ts
@@ -2,7 +2,11 @@ import { isTopFrame } from "@/wab/client/cli-routes";
 import { getPlasmicStudioArgs } from "@/wab/client/frame-ctx/plasmic-studio-args";
 import { assert, withoutNils } from "@/wab/shared/common";
 import { dbg } from "@/wab/shared/dbg";
-import { applyDevFlagOverrides, DEVFLAGS, DevFlagsType } from "@/wab/shared/devflags";
+import {
+  applyDevFlagOverrides,
+  DEVFLAGS,
+  DevFlagsType,
+} from "@/wab/shared/devflags";
 import { isPlainObject } from "lodash";
 
 export function getClientDevFlagOverrides(): DevFlagsType {
@@ -123,7 +127,7 @@ export function initClientFlags(flags: DevFlagsType): DevFlagsType {
     flags[key] = overrides[key] ?? value;
   });
 
-  applyDevFlagOverrides(DEVFLAGS, flags);
+  applyDevFlagOverrides(flags);
 
   return flags;
 }
diff --git a/platform/wab/src/wab/client/components/Shell.tsx b/platform/wab/src/wab/client/components/Shell.tsx
@@ -86,7 +86,7 @@ export function main() {
     };
   }
 
-  applyDevFlagOverrides(DEVFLAGS, initClientFlags(DEVFLAGS));
+  applyDevFlagOverrides(initClientFlags(DEVFLAGS));
 
   const production = DeploymentFlags.DEPLOYENV === "production";
 

diff --git a/platform/wab/src/wab/server/db/DbMgr.ts b/platform/wab/src/wab/server/db/DbMgr.ts
@@ -22,6 +22,7 @@ import {
   unbundleProjectFromBundle,
   unbundleProjectFromData,
 } from "@/wab/server/db/DbBundleLoader";
+import { getDevFlagsMergedWithOverrides } from "@/wab/server/db/appconfig";
 import {
   AppAccessRegistry,
   AppAuthConfig,
@@ -1137,11 +1138,7 @@ export class DbMgr implements MigrationDbMgr {
   ) {
     const userId = this.checkNormalUser();
     const user = await this.getUserById(userId);
-    const devflags = mergeSane(
-      {},
-      DEVFLAGS,
-      JSON.parse((await this.tryGetDevFlagOverrides())?.data ?? "{}")
-    ) as typeof DEVFLAGS;
+    const devflags = await getDevFlagsMergedWithOverrides(this);
     const team = this.teams().create({
       ...this.stampNew(true),
       name,
@@ -3368,11 +3365,7 @@ export class DbMgr implements MigrationDbMgr {
       "publish",
       true
     );
-    const devflags = mergeSane(
-      {},
-      DEVFLAGS,
-      JSON.parse((await this.tryGetDevFlagOverrides())?.data ?? "{}")
-    ) as typeof DEVFLAGS;
+    const devflags = await getDevFlagsMergedWithOverrides(this);
     if (!devflags.serverPublishProjectIds.includes(projectId)) {
       throw new UnauthorizedError("Access denied");
     }

diff --git a/platform/wab/src/wab/server/db/appconfig.ts b/platform/wab/src/wab/server/db/appconfig.ts
@@ -1,11 +1,18 @@
-import { mergeSane } from "@/wab/shared/common";
-import { DEVFLAGS } from "@/wab/shared/devflags";
 import { DbMgr } from "@/wab/server/db/DbMgr";
+import {
+  applyDevFlagOverridesToDefaults,
+  DevFlagsType,
+} from "@/wab/shared/devflags";
 
+/**
+ * WARNING: This function may return devflags inconsistent with the DEVFLAGS.
+ * DEVFLAGS is only computed once, when the server starts.
+ * If an override is submitted after the server starts, this function will
+ * include the new overrides, while DEVFLAGS will stay the same.
+ */
 export async function getDevFlagsMergedWithOverrides(
   mgr: DbMgr
-): Promise<typeof DEVFLAGS> {
+): Promise<DevFlagsType> {
   const overrides = await mgr.tryGetDevFlagOverrides();
-  const merged = mergeSane({}, DEVFLAGS, JSON.parse(overrides?.data ?? "{}"));
-  return merged;
+  return applyDevFlagOverridesToDefaults(JSON.parse(overrides?.data ?? "{}"));
 }
diff --git a/platform/wab/src/wab/server/workers/worker-utils.ts b/platform/wab/src/wab/server/workers/worker-utils.ts
@@ -1,9 +1,9 @@
-import { applyDevFlagOverrides, DEVFLAGS } from "@/wab/shared/devflags";
 import { MigrationDbMgr } from "@/wab/server/db/BundleMigrator";
+import { applyDevFlagOverrides } from "@/wab/shared/devflags";
 
 export async function ensureDevFlags(dbMgr: MigrationDbMgr) {
   const devflags = await dbMgr.tryGetDevFlagOverrides();
   if (devflags) {
-    applyDevFlagOverrides(DEVFLAGS, JSON.parse(devflags.data));
+    applyDevFlagOverrides(JSON.parse(devflags.data));
   }
 }
diff --git a/platform/wab/src/wab/shared/ApiSchema.ts b/platform/wab/src/wab/shared/ApiSchema.ts
@@ -12,7 +12,7 @@ import {
 } from "@/wab/shared/data-sources-meta/data-sources";
 import { WebhookHeader } from "@/wab/shared/db-json-blobs";
 import {
-  DEVFLAGS,
+  DevFlagsType,
   InsertableTemplateComponentResolution,
   InsertableTemplateTokenResolution,
 } from "@/wab/shared/devflags";
@@ -897,7 +897,7 @@ export interface GetDevFlagOverridesVersionsResponse {
 export interface SetDevFlagOverridesResponse {}
 
 export interface AppConfigResponse {
-  config: typeof DEVFLAGS;
+  config: DevFlagsType;
 }
 
 export interface GetClipResponse {

diff --git a/platform/wab/src/wab/shared/devflags.spec.ts b/platform/wab/src/wab/shared/devflags.spec.ts
@@ -0,0 +1,135 @@
+import {
+  applyDevFlagOverrides,
+  applyDevFlagOverridesToDefaults,
+  applyDevFlagOverridesToTarget,
+  DEVFLAGS,
+  DevFlagsType,
+} from "@/wab/shared/devflags";
+
+describe("devflags", () => {
+  describe("applyDevFlagOverridesToTarget", () => {
+    it("merges objects properly", () => {
+      const devflags = {
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            visibility: false,
+            typography: false,
+          },
+        },
+      } as DevFlagsType;
+
+      applyDevFlagOverridesToTarget(devflags, {
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            visibility: true,
+          },
+        },
+      });
+      expect(
+        devflags.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(true);
+      expect(
+        devflags.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(false);
+
+      applyDevFlagOverridesToTarget(devflags, {
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            typography: true,
+          },
+        },
+      });
+      expect(
+        devflags.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(true);
+      expect(
+        devflags.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(true);
+    });
+  });
+
+  describe("applyDevFlagOverridesToDefaults", () => {
+    it("merges from default devflags", () => {
+      const devflags1 = applyDevFlagOverridesToDefaults({
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            visibility: true,
+          },
+        },
+      });
+      expect(
+        devflags1.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(true);
+      expect(
+        devflags1.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(false);
+
+      const devflags2 = applyDevFlagOverridesToDefaults({
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            typography: true,
+          },
+        },
+      });
+      expect(
+        devflags2.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(false);
+      expect(
+        devflags2.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(true);
+    });
+  });
+
+  describe("applyDevFlagOverrides", () => {
+    it("merges from default devflags", () => {
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(false);
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(false);
+
+      applyDevFlagOverrides({
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            visibility: true,
+          },
+        },
+      });
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(true);
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(false);
+
+      applyDevFlagOverrides({
+        defaultContentCreatorConfig: {
+          styleSectionVisibilities: {
+            typography: true,
+          },
+        },
+      });
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.visibility
+      ).toBe(false);
+      expect(
+        DEVFLAGS.defaultContentCreatorConfig.styleSectionVisibilities
+          ?.typography
+      ).toBe(true);
+    });
+  });
+});
diff --git a/platform/wab/src/wab/shared/devflags.ts b/platform/wab/src/wab/shared/devflags.ts
@@ -684,7 +684,7 @@ Object.assign(DEFAULT_DEVFLAGS, DEFAULT_DEVFLAG_OVERRIDES);
 export type DevFlagsType = typeof DEFAULT_DEVFLAGS;
 export const DEVFLAGS = cloneDeep(DEFAULT_DEVFLAGS);
 
-export function normalizeDevFlags(flags: DevFlagsType) {
+function normalizeDevFlags(flags: DevFlagsType) {
   if (flags.variants) {
     flags.unconditionalEdits = true;
     flags.ephemeralRecording = true;
@@ -698,10 +698,27 @@ export function normalizeDevFlags(flags: DevFlagsType) {
   }
 }
 
-export function applyDevFlagOverrides(
+/** Applies overrides to DEVFLAGS. */
+export function applyDevFlagOverrides(overrides: Partial<DevFlagsType>): void {
+  // Apply overrides to default devflags to avoid mergeSane persisting keys that
+  // were present on an old override but removed in a new override (see tests).
+  Object.assign(DEVFLAGS, applyDevFlagOverridesToDefaults(overrides));
+}
+
+/** Applies overrides to a copy of the default devflags and returns it. */
+export function applyDevFlagOverridesToDefaults(
+  overrides: Partial<DevFlagsType>
+): DevFlagsType {
+  const devflags = cloneDeep(DEFAULT_DEVFLAGS);
+  applyDevFlagOverridesToTarget(devflags, overrides);
+  return devflags;
+}
+
+/** Applies overrides to a target. */
+export function applyDevFlagOverridesToTarget(
   target: DevFlagsType,
   overrides: Partial<DevFlagsType>
-) {
+): void {
   mergeSane(target, overrides);
   normalizeDevFlags(target);
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -86,7 +86,7 @@ export function main() { @@
         };
       }
-      applyDevFlagOverrides(DEVFLAGS, initClientFlags(DEVFLAGS));
+      applyDevFlagOverrides(initClientFlags(DEVFLAGS));
       const production = DeploymentFlags.DEPLOYENV === "production";
@@ Expand Down @@