From edb3b685b5f9905311f1255f449e88c44cdaa229 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 15:26:04 +0100 Subject: [PATCH 1/9] Remove node-fetch mock from backend-core and replace with nock. --- packages/backend-core/package.json | 2 +- packages/backend-core/src/environment.ts | 27 +++ .../processors/posthog/PosthogProcessor.ts | 2 +- .../posthog/tests/PosthogProcessor.spec.ts | 29 ++- packages/backend-core/src/features/index.ts | 70 +++++- .../src/features/tests/features.spec.ts | 69 ++++-- .../passport/sso/tests/oidc.spec.ts | 16 +- .../middleware/passport/sso/tests/sso.spec.ts | 20 +- .../src/plugin/tests/validation.spec.ts | 226 ++++++++++++++---- packages/backend-core/src/redis/redis.ts | 4 + .../src/redis/tests/redis.spec.ts | 7 +- .../tests/core/utilities/mocks/fetch.ts | 17 -- .../tests/core/utilities/mocks/index.ts | 2 - .../tests/core/utilities/mocks/posthog.ts | 7 - packages/backend-core/tests/jestSetup.ts | 13 +- packages/server/src/startup/index.ts | 4 + packages/worker/src/index.ts | 2 + yarn.lock | 17 +- 18 files changed, 386 insertions(+), 148 deletions(-) delete mode 100644 packages/backend-core/tests/core/utilities/mocks/fetch.ts delete mode 100644 packages/backend-core/tests/core/utilities/mocks/posthog.ts diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index bf5215a724..b68cba5fd9 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -45,7 +45,7 @@ "passport-oauth2-refresh": "^2.1.0", "pino": "8.11.0", "pino-http": "8.3.3", - "posthog-node": "1.3.0", + "posthog-node": "4.0.1", "pouchdb": "7.3.0", "pouchdb-find": "7.2.2", "redlock": "4.2.0", diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 64e3187956..5091a4971a 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -1,5 +1,6 @@ import { existsSync, readFileSync } from "fs" import { ServiceType } from "@budibase/types" +import { cloneDeep } from "lodash" function isTest() { return isJest() @@ -208,6 +209,32 @@ const environment = { OPENAI_API_KEY: process.env.OPENAI_API_KEY, } +export function setEnv(newEnvVars: Partial): () => void { + const oldEnv = cloneDeep(environment) + + let key: keyof typeof newEnvVars + for (key in newEnvVars) { + environment._set(key, newEnvVars[key]) + } + + return () => { + for (const [key, value] of Object.entries(oldEnv)) { + environment._set(key, value) + } + } +} + +export function withEnv(envVars: Partial, f: () => T) { + const cleanup = setEnv(envVars) + const result = f() + if (result instanceof Promise) { + return result.finally(cleanup) + } else { + cleanup() + return result + } +} + type EnvironmentKey = keyof typeof environment export const SECRETS: EnvironmentKey[] = [ "API_ENCRYPTION_KEY", diff --git a/packages/backend-core/src/events/processors/posthog/PosthogProcessor.ts b/packages/backend-core/src/events/processors/posthog/PosthogProcessor.ts index d37b85a9b8..12d2bb7e2c 100644 --- a/packages/backend-core/src/events/processors/posthog/PosthogProcessor.ts +++ b/packages/backend-core/src/events/processors/posthog/PosthogProcessor.ts @@ -1,4 +1,4 @@ -import PostHog from "posthog-node" +import { PostHog } from "posthog-node" import { Event, Identity, Group, BaseEvent } from "@budibase/types" import { EventProcessor } from "../types" import env from "../../../environment" diff --git a/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts b/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts index d9a5504073..1d9e341ffe 100644 --- a/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts +++ b/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts @@ -1,9 +1,7 @@ import { testEnv } from "../../../../../tests/extra" import PosthogProcessor from "../PosthogProcessor" import { Event, IdentityType, Hosting } from "@budibase/types" - -const tk = require("timekeeper") - +import tk from "timekeeper" import * as cache from "../../../../cache/generic" import { CacheKey } from "../../../../cache/generic" import * as context from "../../../../context" @@ -32,27 +30,30 @@ describe("PosthogProcessor", () => { describe("processEvent", () => { it("processes event", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} await processor.processEvent(Event.APP_CREATED, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalledTimes(1) + expect(spy).toHaveBeenCalledTimes(1) }) it("honours exclusions", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} await processor.processEvent(Event.AUTH_SSO_UPDATED, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalledTimes(0) + expect(spy).toHaveBeenCalledTimes(0) }) it("removes audited information", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = { @@ -63,7 +64,7 @@ describe("PosthogProcessor", () => { } await processor.processEvent(Event.USER_CREATED, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalled() + expect(spy).toHaveBeenCalled() // @ts-ignore const call = processor.posthog.capture.mock.calls[0][0] expect(call.properties.audited).toBeUndefined() @@ -73,6 +74,8 @@ describe("PosthogProcessor", () => { describe("rate limiting", () => { it("sends daily event once in same day", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") + const identity = newIdentity() const properties = {} @@ -82,11 +85,12 @@ describe("PosthogProcessor", () => { tk.freeze(new Date(2022, 0, 1, 15, 0)) await processor.processEvent(Event.SERVED_BUILDER, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalledTimes(1) + expect(spy).toHaveBeenCalledTimes(1) }) it("sends daily event once per unique day", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} @@ -102,11 +106,13 @@ describe("PosthogProcessor", () => { tk.freeze(new Date(2022, 0, 3, 6, 0)) await processor.processEvent(Event.SERVED_BUILDER, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalledTimes(3) + expect(spy).toHaveBeenCalledTimes(3) }) it("sends event again after cache expires", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") + const identity = newIdentity() const properties = {} @@ -120,11 +126,12 @@ describe("PosthogProcessor", () => { tk.freeze(new Date(2022, 0, 1, 14, 0)) await processor.processEvent(Event.SERVED_BUILDER, identity, properties) - expect(processor.posthog.capture).toHaveBeenCalledTimes(2) + expect(spy).toHaveBeenCalledTimes(2) }) it("sends per app events once per day per app", async () => { const processor = new PosthogProcessor("test") + const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} @@ -160,10 +167,10 @@ describe("PosthogProcessor", () => { } await runAppEvents("app_1") - expect(processor.posthog.capture).toHaveBeenCalledTimes(4) + expect(spy).toHaveBeenCalledTimes(4) await runAppEvents("app_2") - expect(processor.posthog.capture).toHaveBeenCalledTimes(8) + expect(spy).toHaveBeenCalledTimes(8) }) }) }) diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index d7f7c76436..7a77eb8b56 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,13 +1,40 @@ import env from "../environment" import * as context from "../context" import { cloneDeep } from "lodash" +import { PostHog } from "posthog-node" +import { IdentityType } from "@budibase/types" -class Flag { - static withDefault(value: T) { - return new Flag(value) +let posthog: PostHog | undefined +export function init() { + if (env.POSTHOG_TOKEN) { + posthog = new PostHog(env.POSTHOG_TOKEN, { + host: "https://us.i.posthog.com", + }) + } +} + +abstract class Flag { + static boolean(defaultValue: boolean): Flag { + return new BooleanFlag(defaultValue) } - private constructor(public defaultValue: T) {} + protected constructor(public defaultValue: T) {} + + abstract parse(value: any): T +} + +class BooleanFlag extends Flag { + parse(value: any) { + if (typeof value === "string") { + return ["true", "t", "1"].includes(value.toLowerCase()) + } + + if (typeof value === "boolean") { + return value + } + + throw new Error(`could not parse value "${value}" as boolean`) + } } // This is the primary source of truth for feature flags. If you want to add a @@ -15,10 +42,10 @@ class Flag { // All of the machinery in this file is to make sure that flags have their // default values set correctly and their types flow through the system. const FLAGS = { - LICENSING: Flag.withDefault(false), - GOOGLE_SHEETS: Flag.withDefault(false), - USER_GROUPS: Flag.withDefault(false), - ONBOARDING_TOUR: Flag.withDefault(false), + LICENSING: Flag.boolean(false), + GOOGLE_SHEETS: Flag.boolean(false), + USER_GROUPS: Flag.boolean(false), + ONBOARDING_TOUR: Flag.boolean(false), } const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { @@ -53,9 +80,10 @@ function isFlagName(name: string): name is keyof Flags { * they will be accessed through this function as well. */ export async function fetch(): Promise { - const currentTenantId = context.getTenantId() const flags = defaultFlags() + const currentTenantId = context.getTenantId() + const split = (env.TENANT_FEATURE_FLAGS || "") .split(",") .map(x => x.split(":")) @@ -79,11 +107,33 @@ export async function fetch(): Promise { throw new Error(`Feature: ${feature} is not a boolean`) } - // @ts-ignore flags[feature] = value } } + const identity = context.getIdentity() + if (posthog && identity?.type === IdentityType.USER) { + const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) + for (const [name, value] of Object.entries(posthogFlags)) { + const key = name as keyof typeof FLAGS + const flag = FLAGS[key] + if (!flag) { + // We don't want an unexpected PostHog flag to break the app, so we + // just log it and continue. + console.warn(`Unexpected posthog flag "${name}": ${value}`) + continue + } + + try { + flags[key] = flag.parse(value) + } catch (err) { + // We don't want an invalid PostHog flag to break the app, so we just + // log it and continue. + console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + } + } + } + return flags } diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 83a89940b8..9022d568ee 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -1,16 +1,8 @@ -import { defaultFlags, fetch, get, Flags } from "../" +import { IdentityContext, IdentityType } from "@budibase/types" +import { defaultFlags, fetch, get, Flags, init } from "../" import { context } from "../.." -import env from "../../environment" - -async function withFlags(flags: string, f: () => T): Promise { - const oldFlags = env.TENANT_FEATURE_FLAGS - env._set("TENANT_FEATURE_FLAGS", flags) - try { - return await f() - } finally { - env._set("TENANT_FEATURE_FLAGS", oldFlags) - } -} +import { setEnv, withEnv } from "../../environment" +import nock from "nock" describe("feature flags", () => { interface TestCase { @@ -48,8 +40,8 @@ describe("feature flags", () => { ])( 'should find flags $expected for $tenant with string "$flags"', ({ tenant, flags, expected }) => - context.doInTenant(tenant, () => - withFlags(flags, async () => { + context.doInTenant(tenant, async () => + withEnv({ TENANT_FEATURE_FLAGS: flags }, async () => { const flags = await fetch() expect(flags).toMatchObject(expected) @@ -75,12 +67,51 @@ describe("feature flags", () => { }, ])( "should fail with message \"$expected\" for $tenant with string '$flags'", - async ({ tenant, flags, expected }) => { + ({ tenant, flags, expected }) => context.doInTenant(tenant, () => - withFlags(flags, async () => { - await expect(fetch()).rejects.toThrow(expected) - }) + withEnv({ TENANT_FEATURE_FLAGS: flags }, () => + expect(fetch()).rejects.toThrow(expected) + ) ) - } ) + + // describe("posthog", () => { + // const identity: IdentityContext = { + // _id: "us_1234", + // tenantId: "budibase", + // type: IdentityType.USER, + // email: "test@example.com", + // firstName: "Test", + // lastName: "User", + // } + + // let cleanup: () => void + + // beforeAll(() => { + // cleanup = setEnv({ POSTHOG_TOKEN: "test" }) + // init() + // }) + + // afterAll(() => { + // cleanup() + // }) + + // beforeEach(() => { + // nock.cleanAll() + // }) + + // it("should be able to read flags from posthog", () => + // context.doInIdentityContext(identity, async () => { + // nock("https://app.posthog.com") + // .get("/api/feature_flags/tenant/budibase") + // .reply(200, { + // flags: { + // "budibase:onboardingTour": true, + // }, + // }) + + // const flags = await fetch() + // expect(flags.ONBOARDING_TOUR).toBe(true) + // })) + // }) }) diff --git a/packages/backend-core/src/middleware/passport/sso/tests/oidc.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/oidc.spec.ts index a705739bd6..594e197204 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/oidc.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/oidc.spec.ts @@ -1,4 +1,4 @@ -import { generator, mocks, structures } from "../../../../../tests" +import { generator, structures } from "../../../../../tests" import { JwtClaims, OIDCInnerConfig, @@ -7,6 +7,7 @@ import { } from "@budibase/types" import * as _sso from "../sso" import * as oidc from "../oidc" +import nock from "nock" jest.mock("@techpass/passport-openidconnect") const mockStrategy = require("@techpass/passport-openidconnect").Strategy @@ -22,16 +23,9 @@ describe("oidc", () => { const oidcConfig: OIDCInnerConfig = structures.sso.oidcConfig() const wellKnownConfig = structures.sso.oidcWellKnownConfig() - function mockRetrieveWellKnownConfig() { - // mock the request to retrieve the oidc configuration - mocks.fetch.mockReturnValue({ - ok: true, - json: () => wellKnownConfig, - }) - } - beforeEach(() => { - mockRetrieveWellKnownConfig() + nock.cleanAll() + nock(oidcConfig.configUrl).get("/").reply(200, wellKnownConfig) }) describe("strategyFactory", () => { @@ -42,8 +36,6 @@ describe("oidc", () => { ) await oidc.strategyFactory(strategyConfiguration, mockSaveUser) - expect(mocks.fetch).toHaveBeenCalledWith(oidcConfig.configUrl) - const expectedOptions = { issuer: wellKnownConfig.issuer, authorizationURL: wellKnownConfig.authorization_endpoint, diff --git a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts index ea9584c284..9fa82b6594 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts @@ -5,6 +5,7 @@ import { SSOAuthDetails, User } from "@budibase/types" import { HTTPError } from "../../../../errors" import * as sso from "../sso" import * as context from "../../../../context" +import nock from "nock" const mockDone = jest.fn() const mockSaveUser = jest.fn() @@ -23,6 +24,7 @@ describe("sso", () => { beforeEach(() => { jest.clearAllMocks() testEnv.singleTenant() + nock.cleanAll() }) describe("validation", () => { @@ -51,15 +53,6 @@ describe("sso", () => { }) }) - function mockGetProfilePicture() { - mocks.fetch.mockReturnValueOnce( - Promise.resolve({ - status: 200, - headers: { get: () => "image/" }, - }) - ) - } - describe("when the user doesn't exist", () => { let user: User let details: SSOAuthDetails @@ -68,7 +61,10 @@ describe("sso", () => { users.getById.mockImplementationOnce(() => { throw new HTTPError("", 404) }) - mockGetProfilePicture() + + nock("http://example.com").get("/").reply(200, undefined, { + "Content-Type": "image/png", + }) user = structures.users.user() delete user._rev @@ -131,7 +127,9 @@ describe("sso", () => { existingUser = structures.users.user() existingUser._id = structures.uuid() details = structures.sso.authDetails(existingUser) - mockGetProfilePicture() + nock("http://example.com").get("/").reply(200, undefined, { + "Content-Type": "image/png", + }) }) describe("exists by email", () => { diff --git a/packages/backend-core/src/plugin/tests/validation.spec.ts b/packages/backend-core/src/plugin/tests/validation.spec.ts index 0fea009645..6f1a3e300b 100644 --- a/packages/backend-core/src/plugin/tests/validation.spec.ts +++ b/packages/backend-core/src/plugin/tests/validation.spec.ts @@ -1,12 +1,129 @@ import { validate } from "../utils" import fetch from "node-fetch" import { PluginType } from "@budibase/types" +import nock from "nock" -const repoUrl = - "https://raw.githubusercontent.com/Budibase/budibase-skeleton/master" -const automationLink = `${repoUrl}/automation/schema.json.hbs` -const componentLink = `${repoUrl}/component/schema.json.hbs` -const datasourceLink = `${repoUrl}/datasource/schema.json.hbs` +const automationLink = `http://example.com/automation/schema.json` +const componentLink = `http://example.com/component/schema.json` +const datasourceLink = `http://example.com/datasource/schema.json` + +function mockDatasourceSchema() { + nock("http://example.com") + .get("/datasource/schema.json") + .reply(200, { + type: "datasource", + metadata: {}, + schema: { + docs: "https://docs.budibase.com", + friendlyName: "Basic HTTP", + type: "API", + description: "Performs a basic HTTP calls to a URL", + datasource: { + url: { + type: "string", + required: true, + }, + cookie: { + type: "string", + required: false, + }, + }, + query: { + create: { + type: "json", + }, + read: { + type: "fields", + fields: { + queryString: { + display: "Query string", + type: "string", + required: false, + }, + }, + }, + update: { + type: "json", + }, + delete: { + type: "fields", + fields: { + id: { + type: "string", + required: true, + }, + }, + }, + }, + }, + }) +} + +function mockAutomationSchema() { + nock("http://example.com") + .get("/automation/schema.json") + .reply(200, { + type: "automation", + metadata: {}, + schema: { + name: "{{ name }}", + tagline: "{{ description }}", + icon: "Actions", + description: "{{ description }}", + type: "action", + stepId: "{{ name }}", + inputs: { + text: "", + }, + schema: { + inputs: { + properties: { + text: { + type: "string", + title: "Log", + }, + }, + required: ["text"], + }, + outputs: { + properties: { + success: { + type: "boolean", + description: "Whether the action was successful", + }, + message: { + type: "string", + description: "What was output", + }, + }, + required: ["success", "message"], + }, + }, + }, + }) +} + +function mockComponentSchema() { + nock("http://example.com") + .get("/component/schema.json") + .reply(200, { + type: "component", + metadata: {}, + schema: { + name: "{{ name }}", + friendlyName: "{{ name }}", + description: "{{ description }}", + icon: "Text", + settings: [ + { + type: "text", + key: "text", + label: "Text", + }, + ], + }, + }) +} async function getSchema(link: string) { const response = await fetch(link) @@ -31,53 +148,62 @@ async function runTest(opts: { link?: string; schema?: any }) { return error } -describe("it should be able to validate an automation schema", () => { - it("should return automation skeleton schema is valid", async () => { - const error = await runTest({ link: automationLink }) - expect(error).toBeUndefined() +describe("plugin validation", () => { + beforeEach(() => { + nock.cleanAll() + mockAutomationSchema() + mockComponentSchema() + mockDatasourceSchema() }) - it("should fail given invalid automation schema", async () => { - const error = await runTest({ - schema: { - type: PluginType.AUTOMATION, - schema: {}, - }, + describe("it should be able to validate an automation schema", () => { + it("should return automation skeleton schema is valid", async () => { + const error = await runTest({ link: automationLink }) + expect(error).toBeUndefined() + }) + + it("should fail given invalid automation schema", async () => { + const error = await runTest({ + schema: { + type: PluginType.AUTOMATION, + schema: {}, + }, + }) + expect(error).toBeDefined() + }) + }) + + describe("it should be able to validate a component schema", () => { + it("should return component skeleton schema is valid", async () => { + const error = await runTest({ link: componentLink }) + expect(error).toBeUndefined() + }) + + it("should fail given invalid component schema", async () => { + const error = await runTest({ + schema: { + type: PluginType.COMPONENT, + schema: {}, + }, + }) + expect(error).toBeDefined() + }) + }) + + describe("it should be able to validate a datasource schema", () => { + it("should return datasource skeleton schema is valid", async () => { + const error = await runTest({ link: datasourceLink }) + expect(error).toBeUndefined() + }) + + it("should fail given invalid datasource schema", async () => { + const error = await runTest({ + schema: { + type: PluginType.DATASOURCE, + schema: {}, + }, + }) + expect(error).toBeDefined() }) - expect(error).toBeDefined() - }) -}) - -describe("it should be able to validate a component schema", () => { - it("should return component skeleton schema is valid", async () => { - const error = await runTest({ link: componentLink }) - expect(error).toBeUndefined() - }) - - it("should fail given invalid component schema", async () => { - const error = await runTest({ - schema: { - type: PluginType.COMPONENT, - schema: {}, - }, - }) - expect(error).toBeDefined() - }) -}) - -describe("it should be able to validate a datasource schema", () => { - it("should return datasource skeleton schema is valid", async () => { - const error = await runTest({ link: datasourceLink }) - expect(error).toBeUndefined() - }) - - it("should fail given invalid datasource schema", async () => { - const error = await runTest({ - schema: { - type: PluginType.DATASOURCE, - schema: {}, - }, - }) - expect(error).toBeDefined() }) }) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 79f75421d3..1271086fc5 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -111,6 +111,10 @@ function init(selectDb = DEFAULT_SELECT_DB) { CLIENTS[selectDb] = client } +export function closeAll() { + Object.values(CLIENTS).forEach(client => client.disconnect()) +} + function waitForConnection(selectDb: number = DEFAULT_SELECT_DB) { return new Promise(resolve => { if (pickClient(selectDb) == null) { diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index 9160c6a6dd..376afbfab7 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -1,6 +1,6 @@ import { GenericContainer, StartedTestContainer } from "testcontainers" import { generator, structures } from "../../../tests" -import RedisWrapper from "../redis" +import RedisWrapper, { closeAll } from "../redis" import { env } from "../.." import { randomUUID } from "crypto" @@ -23,7 +23,10 @@ describe("redis", () => { env._set("REDIS_PASSWORD", 0) }) - afterAll(() => container?.stop()) + afterAll(() => { + container?.stop() + closeAll() + }) beforeEach(async () => { redis = new RedisWrapper(structures.db.id()) diff --git a/packages/backend-core/tests/core/utilities/mocks/fetch.ts b/packages/backend-core/tests/core/utilities/mocks/fetch.ts deleted file mode 100644 index f7447d2c47..0000000000 --- a/packages/backend-core/tests/core/utilities/mocks/fetch.ts +++ /dev/null @@ -1,17 +0,0 @@ -const mockFetch = jest.fn((url: any, opts: any) => { - const fetch = jest.requireActual("node-fetch") - const env = jest.requireActual("../../../../src/environment").default - if (url.includes(env.COUCH_DB_URL) || url.includes("raw.github")) { - return fetch(url, opts) - } - return undefined -}) - -const enable = () => { - jest.mock("node-fetch", () => mockFetch) -} - -export default { - ...mockFetch, - enable, -} diff --git a/packages/backend-core/tests/core/utilities/mocks/index.ts b/packages/backend-core/tests/core/utilities/mocks/index.ts index 8705e563cb..ef7304d64e 100644 --- a/packages/backend-core/tests/core/utilities/mocks/index.ts +++ b/packages/backend-core/tests/core/utilities/mocks/index.ts @@ -5,7 +5,5 @@ export const accounts = jest.mocked(_accounts) export * as date from "./date" export * as licenses from "./licenses" -export { default as fetch } from "./fetch" export * from "./alerts" import "./events" -import "./posthog" diff --git a/packages/backend-core/tests/core/utilities/mocks/posthog.ts b/packages/backend-core/tests/core/utilities/mocks/posthog.ts deleted file mode 100644 index e9cc653ccc..0000000000 --- a/packages/backend-core/tests/core/utilities/mocks/posthog.ts +++ /dev/null @@ -1,7 +0,0 @@ -jest.mock("posthog-node", () => { - return jest.fn().mockImplementation(() => { - return { - capture: jest.fn(), - } - }) -}) diff --git a/packages/backend-core/tests/jestSetup.ts b/packages/backend-core/tests/jestSetup.ts index e5d144290b..e7f2a6cc98 100644 --- a/packages/backend-core/tests/jestSetup.ts +++ b/packages/backend-core/tests/jestSetup.ts @@ -2,14 +2,21 @@ import "./core/logging" import env from "../src/environment" import { cleanup } from "../src/timers" import { mocks, testContainerUtils } from "./core/utilities" - -// must explicitly enable fetch mock -mocks.fetch.enable() +import nock from "nock" // mock all dates to 2020-01-01T00:00:00.000Z // use tk.reset() to use real dates in individual tests import tk from "timekeeper" +nock.disableNetConnect() +nock.enableNetConnect(host => { + return ( + host.includes("localhost") || + host.includes("127.0.0.1") || + host.includes("::1") + ) +}) + tk.freeze(mocks.date.MOCK_DATE) if (!process.env.DEBUG) { diff --git a/packages/server/src/startup/index.ts b/packages/server/src/startup/index.ts index 5bb1f9aa0f..53c4f884cc 100644 --- a/packages/server/src/startup/index.ts +++ b/packages/server/src/startup/index.ts @@ -9,6 +9,7 @@ import { users, cache, env as coreEnv, + features, } from "@budibase/backend-core" import { watch } from "../watch" import * as automations from "../automations" @@ -96,6 +97,9 @@ export async function startup( console.log("Initialising events") eventInit() + console.log("Initialising feature flags") + features.init() + if (app && server) { console.log("Initialising websockets") initialiseWebsockets(app, server) diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 85e5d6ad2e..d59d8d96ef 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -18,6 +18,7 @@ import { timers, redis, cache, + features, } from "@budibase/backend-core" db.init() @@ -99,6 +100,7 @@ export default server.listen(parseInt(env.PORT || "4002"), async () => { // configure events to use the pro audit log write // can't integrate directly into backend-core due to cyclic issues await events.processors.init(proSdk.auditLogs.write) + features.init() }) process.on("uncaughtException", err => { diff --git a/yarn.lock b/yarn.lock index 0195f19a2a..123eec3dd9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2073,7 +2073,7 @@ passport-oauth2-refresh "^2.1.0" pino "8.11.0" pino-http "8.3.3" - posthog-node "1.3.0" + posthog-node "4.0.1" pouchdb "7.3.0" pouchdb-find "7.2.2" redlock "4.2.0" @@ -7343,7 +7343,7 @@ axios-retry@^3.1.9: "@babel/runtime" "^7.15.4" is-retry-allowed "^2.2.0" -axios@0.24.0, axios@1.1.3, axios@1.6.3, axios@^0.21.1, axios@^1.0.0, axios@^1.1.3, axios@^1.4.0, axios@^1.5.0: +axios@0.24.0, axios@1.1.3, axios@1.6.3, axios@^0.21.1, axios@^1.0.0, axios@^1.1.3, axios@^1.4.0, axios@^1.5.0, axios@^1.6.2: version "1.6.3" resolved "https://registry.yarnpkg.com/axios/-/axios-1.6.3.tgz#7f50f23b3aa246eff43c54834272346c396613f4" integrity sha512-fWyNdeawGam70jXSVlKl+SUNVcL6j6W79CuSIPfi6HnDUmSCH6gyUys/HrqHeA/wU0Az41rRgean494d0Jb+ww== @@ -18110,6 +18110,14 @@ posthog-node@1.3.0: remove-trailing-slash "^0.1.1" uuid "^8.3.2" +posthog-node@4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/posthog-node/-/posthog-node-4.0.1.tgz#eb8b6cdf68c3fdd0dc2b75e8aab2e0ec3727fb2a" + integrity sha512-rtqm2h22QxLGBrW2bLYzbRhliIrqgZ0k+gF0LkQ1SNdeD06YE5eilV0MxZppFSxC8TfH0+B0cWCuebEnreIDgQ== + dependencies: + axios "^1.6.2" + rusha "^0.8.14" + pouch-stream@^0.4.0: version "0.4.1" resolved "https://registry.yarnpkg.com/pouch-stream/-/pouch-stream-0.4.1.tgz#0c6d8475c9307677627991a2f079b301c3b89bdd" @@ -19574,6 +19582,11 @@ run-parallel@^1.1.9: dependencies: queue-microtask "^1.2.2" +rusha@^0.8.14: + version "0.8.14" + resolved "https://registry.yarnpkg.com/rusha/-/rusha-0.8.14.tgz#a977d0de9428406138b7bb90d3de5dcd024e2f68" + integrity sha512-cLgakCUf6PedEu15t8kbsjnwIFFR2D4RfL+W3iWFJ4iac7z4B0ZI8fxy4R3J956kAI68HclCFGL8MPoUVC3qVA== + rxjs@^6.6.6: version "6.6.7" resolved "https://registry.yarnpkg.com/rxjs/-/rxjs-6.6.7.tgz#90ac018acabf491bf65044235d5863c4dab804c9" From a9b4d0017f7ab418646c57a3039cad835ba1668d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 16:59:33 +0100 Subject: [PATCH 2/9] add tests for posthog feature flags --- packages/backend-core/src/features/index.ts | 51 ++++++- .../src/features/tests/features.spec.ts | 130 ++++++++++++------ 2 files changed, 135 insertions(+), 46 deletions(-) diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index 7a77eb8b56..4a29dc6daf 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,14 +1,15 @@ import env from "../environment" import * as context from "../context" import { cloneDeep } from "lodash" -import { PostHog } from "posthog-node" +import { PostHog, PostHogOptions } from "posthog-node" import { IdentityType } from "@budibase/types" let posthog: PostHog | undefined -export function init() { +export function init(opts?: PostHogOptions) { if (env.POSTHOG_TOKEN) { posthog = new PostHog(env.POSTHOG_TOKEN, { host: "https://us.i.posthog.com", + ...opts, }) } } @@ -18,6 +19,14 @@ abstract class Flag { return new BooleanFlag(defaultValue) } + static string(defaultValue: string): Flag { + return new StringFlag(defaultValue) + } + + static number(defaultValue: number): Flag { + return new NumberFlag(defaultValue) + } + protected constructor(public defaultValue: T) {} abstract parse(value: any): T @@ -37,6 +46,32 @@ class BooleanFlag extends Flag { } } +class StringFlag extends Flag { + parse(value: any) { + if (typeof value === "string") { + return value + } + throw new Error(`could not parse value "${value}" as string`) + } +} + +class NumberFlag extends Flag { + parse(value: any) { + if (typeof value === "number") { + return value + } + + if (typeof value === "string") { + const parsed = parseFloat(value) + if (!isNaN(parsed)) { + return parsed + } + } + + throw new Error(`could not parse value "${value}" as number`) + } +} + // This is the primary source of truth for feature flags. If you want to add a // new flag, add it here and use the `fetch` and `get` functions to access it. // All of the machinery in this file is to make sure that flags have their @@ -46,6 +81,10 @@ const FLAGS = { GOOGLE_SHEETS: Flag.boolean(false), USER_GROUPS: Flag.boolean(false), ONBOARDING_TOUR: Flag.boolean(false), + + _TEST_BOOLEAN: Flag.boolean(false), + _TEST_STRING: Flag.string("default value"), + _TEST_NUMBER: Flag.number(0), } const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { @@ -107,6 +146,7 @@ export async function fetch(): Promise { throw new Error(`Feature: ${feature} is not a boolean`) } + // @ts-ignore flags[feature] = value } } @@ -114,7 +154,7 @@ export async function fetch(): Promise { const identity = context.getIdentity() if (posthog && identity?.type === IdentityType.USER) { const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) - for (const [name, value] of Object.entries(posthogFlags)) { + for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { const key = name as keyof typeof FLAGS const flag = FLAGS[key] if (!flag) { @@ -124,8 +164,11 @@ export async function fetch(): Promise { continue } + const payload = posthogFlags.featureFlagPayloads?.[name] + try { - flags[key] = flag.parse(value) + // @ts-ignore + flags[key] = flag.parse(payload || value) } catch (err) { // We don't want an invalid PostHog flag to break the app, so we just // log it and continue. diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 9022d568ee..de40e24b3c 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -2,6 +2,7 @@ import { IdentityContext, IdentityType } from "@budibase/types" import { defaultFlags, fetch, get, Flags, init } from "../" import { context } from "../.." import { setEnv, withEnv } from "../../environment" +import nodeFetch from "node-fetch" import nock from "nock" describe("feature flags", () => { @@ -14,23 +15,23 @@ describe("feature flags", () => { it.each([ { tenant: "tenant1", - flags: "tenant1:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: true }, + flags: "tenant1:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant1:!ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: false }, + flags: "tenant1:!_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: false }, }, { tenant: "tenant1", - flags: "*:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: true }, + flags: "*:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant2:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: false }, + flags: "tenant2:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: false }, }, { tenant: "tenant1", @@ -62,7 +63,7 @@ describe("feature flags", () => { it.each([ { tenant: "tenant1", - flags: "tenant1:ONBOARDING_TOUR,tenant1:FOO", + flags: "tenant1:_TEST_BOOLEAN,tenant1:FOO", expected: "Feature: FOO is not an allowed option", }, ])( @@ -75,43 +76,88 @@ describe("feature flags", () => { ) ) - // describe("posthog", () => { - // const identity: IdentityContext = { - // _id: "us_1234", - // tenantId: "budibase", - // type: IdentityType.USER, - // email: "test@example.com", - // firstName: "Test", - // lastName: "User", - // } + describe("posthog", () => { + const identity: IdentityContext = { + _id: "us_1234", + tenantId: "budibase", + type: IdentityType.USER, + email: "test@example.com", + firstName: "Test", + lastName: "User", + } - // let cleanup: () => void + let cleanup: () => void - // beforeAll(() => { - // cleanup = setEnv({ POSTHOG_TOKEN: "test" }) - // init() - // }) + beforeAll(() => { + cleanup = setEnv({ POSTHOG_TOKEN: "test" }) + }) - // afterAll(() => { - // cleanup() - // }) + afterAll(() => { + cleanup() + }) - // beforeEach(() => { - // nock.cleanAll() - // }) + beforeEach(() => { + nock.cleanAll() - // it("should be able to read flags from posthog", () => - // context.doInIdentityContext(identity, async () => { - // nock("https://app.posthog.com") - // .get("/api/feature_flags/tenant/budibase") - // .reply(200, { - // flags: { - // "budibase:onboardingTour": true, - // }, - // }) + // We need to pass in node-fetch here otherwise nock won't get used + // because posthog-node uses axios under the hood. + init({ fetch: nodeFetch }) + }) - // const flags = await fetch() - // expect(flags.ONBOARDING_TOUR).toBe(true) - // })) - // }) + function mockFlags(flags: { + featureFlags?: Record + featureFlagPayloads?: Record + }) { + nock("https://us.i.posthog.com") + .post("/decide/?v=3", body => { + return body.token === "test" && body.distinct_id === "us_1234" + }) + .reply(200, flags) + } + + it("should be able to read flags from posthog", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: true, + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(true) + }) + }) + + it("should be able to read flags from posthog with payloads", async () => { + mockFlags({ + featureFlags: { + _TEST_STRING: true, + }, + featureFlagPayloads: { + _TEST_STRING: "test payload", + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_STRING).toBe("test payload") + }) + }) + + it("should be able to read flags from posthog with numbers", async () => { + mockFlags({ + featureFlags: { + _TEST_NUMBER: true, + }, + featureFlagPayloads: { + _TEST_NUMBER: 123, + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_NUMBER).toBe(123) + }) + }) + }) }) From d7e07bb44aaca857383e913e156a3254948c5978 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 17:33:47 +0100 Subject: [PATCH 3/9] Fix worker tests. --- .../src/features/tests/features.spec.ts | 14 ++++++- .../middleware/passport/sso/tests/sso.spec.ts | 2 +- .../src/api/routes/global/tests/auth.spec.ts | 29 +++++++------ .../api/routes/global/tests/realEmail.spec.ts | 42 +++++++++++++------ packages/worker/src/tests/api/configs.ts | 2 +- packages/worker/src/tests/jestSetup.ts | 16 +++++-- 6 files changed, 70 insertions(+), 35 deletions(-) diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index de40e24b3c..25e9765287 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -71,7 +71,7 @@ describe("feature flags", () => { ({ tenant, flags, expected }) => context.doInTenant(tenant, () => withEnv({ TENANT_FEATURE_FLAGS: flags }, () => - expect(fetch()).rejects.toThrow(expected) + expect(fetch).rejects.toThrow(expected) ) ) ) @@ -159,5 +159,17 @@ describe("feature flags", () => { expect(flags._TEST_NUMBER).toBe(123) }) }) + + it("should not fail when a flag is not known", async () => { + mockFlags({ + featureFlags: { + _SOME_RANDOM_FLAG: true, + }, + }) + + await context.doInIdentityContext(identity, async () => { + await expect(fetch()).resolves.not.toThrow() + }) + }) }) }) diff --git a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts index 9fa82b6594..62b71055d9 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts @@ -1,4 +1,4 @@ -import { structures, mocks } from "../../../../../tests" +import { structures } from "../../../../../tests" import { testEnv } from "../../../../../tests/extra" import { SSOAuthDetails, User } from "@budibase/types" diff --git a/packages/worker/src/api/routes/global/tests/auth.spec.ts b/packages/worker/src/api/routes/global/tests/auth.spec.ts index 3f2b3045de..9a004497c7 100644 --- a/packages/worker/src/api/routes/global/tests/auth.spec.ts +++ b/packages/worker/src/api/routes/global/tests/auth.spec.ts @@ -292,9 +292,9 @@ describe("/api/global/auth", () => { it("redirects to auth provider", async () => { nock("http://someconfigurl").get("/").times(1).reply(200, { issuer: "test", - authorization_endpoint: "http://localhost/auth", - token_endpoint: "http://localhost/token", - userinfo_endpoint: "http://localhost/userinfo", + authorization_endpoint: "http://example.com/auth", + token_endpoint: "http://example.com/token", + userinfo_endpoint: "http://example.com/userinfo", }) const configId = await generateOidcConfig() @@ -305,7 +305,7 @@ describe("/api/global/auth", () => { const location: string = res.get("location") expect( location.startsWith( - `http://localhost/auth?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A10000%2Fapi%2Fglobal%2Fauth%2F${config.tenantId}%2Foidc%2Fcallback&scope=openid%20profile%20email%20offline_access` + `http://example.com/auth?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A10000%2Fapi%2Fglobal%2Fauth%2F${config.tenantId}%2Foidc%2Fcallback&scope=openid%20profile%20email%20offline_access` ) ).toBe(true) }) @@ -313,11 +313,13 @@ describe("/api/global/auth", () => { describe("GET /api/global/auth/:tenantId/oidc/callback", () => { it("logs in", async () => { + const email = `${generator.guid()}@example.com` + nock("http://someconfigurl").get("/").times(2).reply(200, { issuer: "test", - authorization_endpoint: "http://localhost/auth", - token_endpoint: "http://localhost/token", - userinfo_endpoint: "http://localhost/userinfo", + authorization_endpoint: "http://example.com/auth", + token_endpoint: "http://example.com/token", + userinfo_endpoint: "http://example.com/userinfo", }) const token = jwt.sign( @@ -326,20 +328,20 @@ describe("/api/global/auth", () => { sub: "sub", aud: "clientId", exp: Math.floor(Date.now() / 1000) + 60 * 60, - email: "oauth@example.com", + email, }, "secret" ) - nock("http://localhost").post("/token").reply(200, { + nock("http://example.com").post("/token").reply(200, { access_token: "access", refresh_token: "refresh", id_token: token, }) - nock("http://localhost").get("/userinfo?schema=openid").reply(200, { + nock("http://example.com").get("/userinfo?schema=openid").reply(200, { sub: "sub", - email: "oauth@example.com", + email, }) const configId = await generateOidcConfig() @@ -351,10 +353,7 @@ describe("/api/global/auth", () => { ) } - expect(events.auth.login).toHaveBeenCalledWith( - "oidc", - "oauth@example.com" - ) + expect(events.auth.login).toHaveBeenCalledWith("oidc", email) expect(events.auth.login).toHaveBeenCalledTimes(1) expect(res.status).toBe(302) const location: string = res.get("location") diff --git a/packages/worker/src/api/routes/global/tests/realEmail.spec.ts b/packages/worker/src/api/routes/global/tests/realEmail.spec.ts index bea627d5b6..a18d8ee247 100644 --- a/packages/worker/src/api/routes/global/tests/realEmail.spec.ts +++ b/packages/worker/src/api/routes/global/tests/realEmail.spec.ts @@ -12,6 +12,33 @@ const nodemailer = require("nodemailer") // for the real email tests give them a long time to try complete/fail jest.setTimeout(30000) +function cancelableTimeout(timeout: number): [Promise, () => void] { + let timeoutId: NodeJS.Timeout + return [ + new Promise((resolve, reject) => { + timeoutId = setTimeout(() => { + reject({ + status: 301, + errno: "ETIME", + }) + }, timeout) + }), + () => { + clearTimeout(timeoutId) + }, + ] +} + +async function withTimeout( + timeout: number, + promise: Promise +): Promise { + const [timeoutPromise, cancel] = cancelableTimeout(timeout) + const result = (await Promise.race([promise, timeoutPromise])) as T + cancel() + return result +} + describe("/api/global/email", () => { const config = new TestConfiguration() @@ -30,19 +57,8 @@ describe("/api/global/email", () => { ) { let response, text try { - const timeout = () => - new Promise((resolve, reject) => - setTimeout( - () => - reject({ - status: 301, - errno: "ETIME", - }), - 20000 - ) - ) - await Promise.race([config.saveEtherealSmtpConfig(), timeout()]) - await Promise.race([config.saveSettingsConfig(), timeout()]) + await withTimeout(20000, config.saveEtherealSmtpConfig()) + await withTimeout(20000, config.saveSettingsConfig()) let res if (attachments) { res = await config.api.emails diff --git a/packages/worker/src/tests/api/configs.ts b/packages/worker/src/tests/api/configs.ts index 74cef2bf8b..86fe0830ca 100644 --- a/packages/worker/src/tests/api/configs.ts +++ b/packages/worker/src/tests/api/configs.ts @@ -40,7 +40,7 @@ export class ConfigAPI extends TestAPI { const sessionContent = JSON.parse( Buffer.from(koaSession, "base64").toString("utf-8") ) - const handle = sessionContent["openidconnect:localhost"].state.handle + const handle = sessionContent["openidconnect:example.com"].state.handle return this.request .get(`/api/global/auth/${this.config.getTenantId()}/oidc/callback`) .query({ code: "test", state: handle }) diff --git a/packages/worker/src/tests/jestSetup.ts b/packages/worker/src/tests/jestSetup.ts index 0f933bd562..6a98031d34 100644 --- a/packages/worker/src/tests/jestSetup.ts +++ b/packages/worker/src/tests/jestSetup.ts @@ -1,13 +1,21 @@ import { mocks, testContainerUtils } from "@budibase/backend-core/tests" import env from "../environment" import { env as coreEnv, timers } from "@budibase/backend-core" - -// must explicitly enable fetch mock -mocks.fetch.enable() +import nock from "nock" // mock all dates to 2020-01-01T00:00:00.000Z // use tk.reset() to use real dates in individual tests -const tk = require("timekeeper") +import tk from "timekeeper" + +nock.disableNetConnect() +nock.enableNetConnect(host => { + return ( + host.includes("localhost") || + host.includes("127.0.0.1") || + host.includes("::1") || + host.includes("ethereal.email") // used in realEmail.spec.ts + ) +}) tk.freeze(mocks.date.MOCK_DATE) From 88031094968764be853f5d13617d9a5cd3966faf Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 17:50:39 +0100 Subject: [PATCH 4/9] Add DataDog tracing to feature flags. --- packages/backend-core/src/features/index.ts | 116 ++++++++++-------- .../src/features/tests/features.spec.ts | 38 +++++- 2 files changed, 105 insertions(+), 49 deletions(-) diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index 4a29dc6daf..2ffa35974f 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -3,6 +3,7 @@ import * as context from "../context" import { cloneDeep } from "lodash" import { PostHog, PostHogOptions } from "posthog-node" import { IdentityType } from "@budibase/types" +import tracer from "dd-trace" let posthog: PostHog | undefined export function init(opts?: PostHogOptions) { @@ -119,65 +120,84 @@ function isFlagName(name: string): name is keyof Flags { * they will be accessed through this function as well. */ export async function fetch(): Promise { - const flags = defaultFlags() + return await tracer.trace("features.fetch", async span => { + const tags: Record = {} - const currentTenantId = context.getTenantId() + const flags = defaultFlags() - const split = (env.TENANT_FEATURE_FLAGS || "") - .split(",") - .map(x => x.split(":")) - for (const [tenantId, ...features] of split) { - if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { - continue - } + const currentTenantId = context.getTenantId() + const specificallySetFalse = new Set() - for (let feature of features) { - let value = true - if (feature.startsWith("!")) { - feature = feature.slice(1) - value = false - } - - if (!isFlagName(feature)) { - throw new Error(`Feature: ${feature} is not an allowed option`) - } - - if (typeof flags[feature] !== "boolean") { - throw new Error(`Feature: ${feature} is not a boolean`) - } - - // @ts-ignore - flags[feature] = value - } - } - - const identity = context.getIdentity() - if (posthog && identity?.type === IdentityType.USER) { - const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) - for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { - const key = name as keyof typeof FLAGS - const flag = FLAGS[key] - if (!flag) { - // We don't want an unexpected PostHog flag to break the app, so we - // just log it and continue. - console.warn(`Unexpected posthog flag "${name}": ${value}`) + const split = (env.TENANT_FEATURE_FLAGS || "") + .split(",") + .map(x => x.split(":")) + for (const [tenantId, ...features] of split) { + if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { continue } - const payload = posthogFlags.featureFlagPayloads?.[name] + for (let feature of features) { + let value = true + if (feature.startsWith("!")) { + feature = feature.slice(1) + value = false + specificallySetFalse.add(feature) + } + + if (!isFlagName(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } + + if (typeof flags[feature] !== "boolean") { + throw new Error(`Feature: ${feature} is not a boolean`) + } - try { // @ts-ignore - flags[key] = flag.parse(payload || value) - } catch (err) { - // We don't want an invalid PostHog flag to break the app, so we just - // log it and continue. - console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + flags[feature] = value + tags[`flags.${feature}.source`] = "environment" } } - } - return flags + const identity = context.getIdentity() + if (posthog && identity?.type === IdentityType.USER) { + const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) + for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { + const key = name as keyof typeof FLAGS + const flag = FLAGS[key] + if (!flag) { + // We don't want an unexpected PostHog flag to break the app, so we + // just log it and continue. + console.warn(`Unexpected posthog flag "${name}": ${value}`) + continue + } + + if (flags[key] === true || specificallySetFalse.has(key)) { + // If the flag is already set to through environment variables, we + // don't want to override it back to false here. + continue + } + + const payload = posthogFlags.featureFlagPayloads?.[name] + + try { + // @ts-ignore + flags[key] = flag.parse(payload || value) + tags[`flags.${key}.source`] = "posthog" + } catch (err) { + // We don't want an invalid PostHog flag to break the app, so we just + // log it and continue. + console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + } + } + } + + for (const [key, value] of Object.entries(flags)) { + tags[`flags.${key}.value`] = value + } + span?.addTags(tags) + + return flags + }) } // Gets a single feature flag value. This is a convenience function for diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 25e9765287..1b85a74024 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -79,7 +79,7 @@ describe("feature flags", () => { describe("posthog", () => { const identity: IdentityContext = { _id: "us_1234", - tenantId: "budibase", + tenantId: "tenant1", type: IdentityType.USER, email: "test@example.com", firstName: "Test", @@ -171,5 +171,41 @@ describe("feature flags", () => { await expect(fetch()).resolves.not.toThrow() }) }) + + it("should not override flags set in the environment", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: false, + }, + }) + + await withEnv( + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:_TEST_BOOLEAN` }, + async () => { + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(true) + }) + } + ) + }) + + it("should not override flags set in the environment with a ! prefix", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: true, + }, + }) + + await withEnv( + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:!_TEST_BOOLEAN` }, + async () => { + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(false) + }) + } + ) + }) }) }) From 3d590e879e7abcc8c6eaf72ad044db00260ffe8b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 9 Aug 2024 10:30:27 +0100 Subject: [PATCH 5/9] Rely on withEnv and setEnv functions in respective environment.ts files. --- packages/backend-core/src/index.ts | 2 +- .../{apikeys.spec.js => apikeys.spec.ts} | 15 ++--- .../src/api/routes/tests/application.spec.ts | 9 ++- .../src/api/routes/tests/attachment.spec.ts | 5 +- .../server/src/api/routes/tests/row.spec.ts | 7 ++- .../src/api/routes/tests/search.spec.ts | 8 ++- .../tests/{static.spec.js => static.spec.ts} | 25 ++++---- .../src/api/routes/tests/templates.spec.ts | 3 +- .../src/api/routes/tests/viewV2.spec.ts | 14 +++-- .../src/api/routes/tests/webhook.spec.ts | 3 +- .../tests/20240604153647_initial_sqs.spec.ts | 10 +++- .../src/automations/tests/openai.spec.ts | 5 +- packages/server/src/environment.ts | 27 +++++++++ .../integrations/tests/googlesheets.spec.ts | 3 +- .../sdk/app/rows/search/tests/search.spec.ts | 11 ++-- .../server/src/startup/tests/startup.spec.ts | 12 +++- .../src/tests/utilities/TestConfiguration.ts | 59 ------------------- 17 files changed, 109 insertions(+), 109 deletions(-) rename packages/server/src/api/routes/tests/{apikeys.spec.js => apikeys.spec.ts} (75%) rename packages/server/src/api/routes/tests/{static.spec.js => static.spec.ts} (88%) diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index a14a344655..dbdce51c50 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -27,7 +27,7 @@ export * as locks from "./redis/redlockImpl" export * as utils from "./utils" export * as errors from "./errors" export * as timers from "./timers" -export { default as env } from "./environment" +export { default as env, withEnv, setEnv } from "./environment" export * as blacklist from "./blacklist" export * as docUpdates from "./docUpdates" export * from "./utils/Duration" diff --git a/packages/server/src/api/routes/tests/apikeys.spec.js b/packages/server/src/api/routes/tests/apikeys.spec.ts similarity index 75% rename from packages/server/src/api/routes/tests/apikeys.spec.js rename to packages/server/src/api/routes/tests/apikeys.spec.ts index 678da38f28..30f87fcfec 100644 --- a/packages/server/src/api/routes/tests/apikeys.spec.js +++ b/packages/server/src/api/routes/tests/apikeys.spec.ts @@ -1,11 +1,12 @@ -const setup = require("./utilities") -const { checkBuilderEndpoint } = require("./utilities/TestFunctions") +import { withEnv } from "../../../environment" +import { getRequest, getConfig, afterAll as _afterAll } from "./utilities" +import { checkBuilderEndpoint } from "./utilities/TestFunctions" describe("/api/keys", () => { - let request = setup.getRequest() - let config = setup.getConfig() + let request = getRequest() + let config = getConfig() - afterAll(setup.afterAll) + afterAll(_afterAll) beforeAll(async () => { await config.init() @@ -13,7 +14,7 @@ describe("/api/keys", () => { describe("fetch", () => { it("should allow fetching", async () => { - await config.withEnv({ SELF_HOSTED: "true" }, async () => { + await withEnv({ SELF_HOSTED: "true" }, async () => { const res = await request .get(`/api/keys`) .set(config.defaultHeaders()) @@ -34,7 +35,7 @@ describe("/api/keys", () => { describe("update", () => { it("should allow updating a value", async () => { - await config.withEnv({ SELF_HOSTED: "true" }, async () => { + await withEnv({ SELF_HOSTED: "true" }, async () => { const res = await request .put(`/api/keys/TEST`) .send({ diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 6ae598ee84..a8e060a2b5 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -14,7 +14,12 @@ jest.mock("../../../utilities/redis", () => ({ import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" import { AppStatus } from "../../../db/utils" -import { events, utils, context } from "@budibase/backend-core" +import { + events, + utils, + context, + withEnv as withCoreEnv, +} from "@budibase/backend-core" import env from "../../../environment" import { type App } from "@budibase/types" import tk from "timekeeper" @@ -353,7 +358,7 @@ describe("/applications", () => { .delete(`/api/global/roles/${prodAppId}`) .reply(200, {}) - await config.withCoreEnv({ SQS_SEARCH_ENABLE: "true" }, async () => { + await withCoreEnv({ SQS_SEARCH_ENABLE: "true" }, async () => { await config.api.application.delete(app.appId) }) }) diff --git a/packages/server/src/api/routes/tests/attachment.spec.ts b/packages/server/src/api/routes/tests/attachment.spec.ts index 7ff4e67bcc..53e0a1b15d 100644 --- a/packages/server/src/api/routes/tests/attachment.spec.ts +++ b/packages/server/src/api/routes/tests/attachment.spec.ts @@ -1,3 +1,4 @@ +import { withEnv } from "../../../environment" import * as setup from "./utilities" import { APIError } from "@budibase/types" @@ -28,7 +29,7 @@ describe("/api/applications/:appId/sync", () => { }) it("should reject an upload with a malicious file extension", async () => { - await config.withEnv({ SELF_HOSTED: undefined }, async () => { + await withEnv({ SELF_HOSTED: undefined }, async () => { let resp = (await config.api.attachment.process( "ohno.exe", Buffer.from([0]), @@ -39,7 +40,7 @@ describe("/api/applications/:appId/sync", () => { }) it("should reject an upload with a malicious uppercase file extension", async () => { - await config.withEnv({ SELF_HOSTED: undefined }, async () => { + await withEnv({ SELF_HOSTED: undefined }, async () => { let resp = (await config.api.attachment.process( "OHNO.EXE", Buffer.from([0]), diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index cf94eb9f13..9351976d66 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -40,6 +40,7 @@ import _, { merge } from "lodash" import * as uuid from "uuid" import { Knex } from "knex" import { InternalTables } from "../../../db/utils" +import { withEnv } from "../../../environment" const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString() tk.freeze(timestamp) @@ -1688,7 +1689,7 @@ describe.each([ } const row = await config.api.row.save(testTable._id!, draftRow) - await config.withEnv({ SELF_HOSTED: "true" }, async () => { + await withEnv({ SELF_HOSTED: "true" }, async () => { return context.doInAppContext(config.getAppId(), async () => { const enriched: Row[] = await outputProcessing(table, [row]) const [targetRow] = enriched @@ -2456,7 +2457,7 @@ describe.each([ describe("Formula JS protection", () => { it("should time out JS execution if a single cell takes too long", async () => { - await config.withEnv({ JS_PER_INVOCATION_TIMEOUT_MS: 40 }, async () => { + await withEnv({ JS_PER_INVOCATION_TIMEOUT_MS: 40 }, async () => { const js = Buffer.from( ` let i = 0; @@ -2494,7 +2495,7 @@ describe.each([ }) it("should time out JS execution if a multiple cells take too long", async () => { - await config.withEnv( + await withEnv( { JS_PER_INVOCATION_TIMEOUT_MS: 40, JS_PER_REQUEST_TIMEOUT_MS: 80, diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d8c2a4a257..5c0a04d64f 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -11,6 +11,8 @@ import { MIN_VALID_DATE, SQLITE_DESIGN_DOC_ID, utils, + withEnv as withCoreEnv, + setEnv as setCoreEnv, } from "@budibase/backend-core" import * as setup from "./utilities" @@ -64,9 +66,9 @@ describe.each([ let rows: Row[] beforeAll(async () => { - await config.withCoreEnv({ SQS_SEARCH_ENABLE: "true" }, () => config.init()) + await withCoreEnv({ SQS_SEARCH_ENABLE: "true" }, () => config.init()) if (isSqs) { - envCleanup = config.setCoreEnv({ + envCleanup = setCoreEnv({ SQS_SEARCH_ENABLE: "true", SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], }) @@ -2668,7 +2670,7 @@ describe.each([ }) it("can still page when the hard limit is hit", async () => { - await config.withCoreEnv( + await withCoreEnv( { SQL_MAX_ROWS: "6", }, diff --git a/packages/server/src/api/routes/tests/static.spec.js b/packages/server/src/api/routes/tests/static.spec.ts similarity index 88% rename from packages/server/src/api/routes/tests/static.spec.js rename to packages/server/src/api/routes/tests/static.spec.ts index 1358b5418a..62b72b2b8f 100644 --- a/packages/server/src/api/routes/tests/static.spec.js +++ b/packages/server/src/api/routes/tests/static.spec.ts @@ -8,23 +8,24 @@ jest.mock("aws-sdk", () => ({ })), })) -const setup = require("./utilities") -const { constants } = require("@budibase/backend-core") +import { Datasource, SourceName } from "@budibase/types" +import { setEnv } from "../../../environment" +import { getRequest, getConfig, afterAll as _afterAll } from "./utilities" +import { constants } from "@budibase/backend-core" describe("/static", () => { - let request = setup.getRequest() - let config = setup.getConfig() - let app - let cleanupEnv + let request = getRequest() + let config = getConfig() + let cleanupEnv: () => void afterAll(() => { - setup.afterAll() + _afterAll() cleanupEnv() }) beforeAll(async () => { - cleanupEnv = config.setEnv({ SELF_HOSTED: "true" }) - app = await config.init() + cleanupEnv = setEnv({ SELF_HOSTED: "true" }) + await config.init() }) describe("/app", () => { @@ -49,7 +50,7 @@ describe("/static", () => { delete headers[constants.Header.APP_ID] const res = await request - .get(`/app${config.prodApp.url}`) + .get(`/app${config.getProdApp().url}`) .set(headers) .expect(200) @@ -68,14 +69,14 @@ describe("/static", () => { describe("/attachments", () => { describe("generateSignedUrls", () => { - let datasource + let datasource: Datasource beforeEach(async () => { datasource = await config.createDatasource({ datasource: { type: "datasource", name: "Test", - source: "S3", + source: SourceName.S3, config: {}, }, }) diff --git a/packages/server/src/api/routes/tests/templates.spec.ts b/packages/server/src/api/routes/tests/templates.spec.ts index 81c615487d..98d375dec6 100644 --- a/packages/server/src/api/routes/tests/templates.spec.ts +++ b/packages/server/src/api/routes/tests/templates.spec.ts @@ -2,6 +2,7 @@ import * as setup from "./utilities" import path from "path" import nock from "nock" import { generator } from "@budibase/backend-core/tests" +import { withEnv as withCoreEnv } from "@budibase/backend-core" interface App { background: string @@ -89,7 +90,7 @@ describe("/templates", () => { SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], } - await config.withCoreEnv(env, async () => { + await withCoreEnv(env, async () => { const name = generator.guid().replaceAll("-", "") const url = `/${name}` diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index ea6aedbe3c..e18cffeaa8 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -24,7 +24,12 @@ import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" import merge from "lodash/merge" import { quotas } from "@budibase/pro" -import { db, roles } from "@budibase/backend-core" +import { + db, + roles, + withEnv as withCoreEnv, + setEnv as setCoreEnv, +} from "@budibase/backend-core" describe.each([ ["lucene", undefined], @@ -89,12 +94,11 @@ describe.each([ } beforeAll(async () => { - await config.withCoreEnv( - { SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, - () => config.init() + await withCoreEnv({ SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, () => + config.init() ) if (isSqs) { - envCleanup = config.setCoreEnv({ + envCleanup = setCoreEnv({ SQS_SEARCH_ENABLE: "true", SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], }) diff --git a/packages/server/src/api/routes/tests/webhook.spec.ts b/packages/server/src/api/routes/tests/webhook.spec.ts index b741ce0820..9c40a14f98 100644 --- a/packages/server/src/api/routes/tests/webhook.spec.ts +++ b/packages/server/src/api/routes/tests/webhook.spec.ts @@ -2,6 +2,7 @@ import { Webhook } from "@budibase/types" import * as setup from "./utilities" import { checkBuilderEndpoint } from "./utilities/TestFunctions" import { mocks } from "@budibase/backend-core/tests" +import { setEnv } from "../../../environment" const { basicWebhook, basicAutomation, collectAutomation } = setup.structures @@ -17,7 +18,7 @@ describe("/webhooks", () => { }) const setupTest = async () => { - cleanupEnv = config.setEnv({ SELF_HOSTED: "true" }) + cleanupEnv = setEnv({ SELF_HOSTED: "true" }) await config.init() const autoConfig = basicAutomation() autoConfig.definition.trigger.schema = { diff --git a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts index f337958a5b..5f036bb87d 100644 --- a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts +++ b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts @@ -1,6 +1,10 @@ import * as setup from "../../../api/routes/tests/utilities" import { basicTable } from "../../../tests/utilities/structures" -import { db as dbCore, SQLITE_DESIGN_DOC_ID } from "@budibase/backend-core" +import { + db as dbCore, + SQLITE_DESIGN_DOC_ID, + withEnv as withCoreEnv, +} from "@budibase/backend-core" import { LinkDocument, DocumentType, @@ -69,11 +73,11 @@ function oldLinkDocument(): Omit { type SQSEnvVar = "SQS_MIGRATION_ENABLE" | "SQS_SEARCH_ENABLE" async function sqsDisabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withCoreEnv({ [envVar]: "", SQS_SEARCH_ENABLE_TENANTS: [] }, cb) + await withCoreEnv({ [envVar]: "", SQS_SEARCH_ENABLE_TENANTS: [] }, cb) } async function sqsEnabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withCoreEnv( + await withCoreEnv( { [envVar]: "1", SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()] }, cb ) diff --git a/packages/server/src/automations/tests/openai.spec.ts b/packages/server/src/automations/tests/openai.spec.ts index 6bebe16a49..41456f116d 100644 --- a/packages/server/src/automations/tests/openai.spec.ts +++ b/packages/server/src/automations/tests/openai.spec.ts @@ -1,5 +1,6 @@ import { getConfig, runStep, afterAll as _afterAll } from "./utilities" import { OpenAI } from "openai" +import { withEnv as withCoreEnv, setEnv as setCoreEnv } from "@budibase/backend-core" jest.mock("openai", () => ({ OpenAI: jest.fn().mockImplementation(() => ({ @@ -32,7 +33,7 @@ describe("test the openai action", () => { }) beforeEach(() => { - resetEnv = config.setCoreEnv({ OPENAI_API_KEY: "abc123" }) + resetEnv = setCoreEnv({ OPENAI_API_KEY: "abc123" }) }) afterEach(() => { @@ -42,7 +43,7 @@ describe("test the openai action", () => { afterAll(_afterAll) it("should present the correct error message when the OPENAI_API_KEY variable isn't set", async () => { - await config.withCoreEnv({ OPENAI_API_KEY: "" }, async () => { + await withCoreEnv({ OPENAI_API_KEY: "" }, async () => { let res = await runStep("OPENAI", { prompt: OPENAI_PROMPT }) expect(res.response).toEqual( "OpenAI API Key not configured - please add the OPENAI_API_KEY environment variable." diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index e1525ebd1b..05ebbe3151 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -1,5 +1,6 @@ import { env as coreEnv } from "@budibase/backend-core" import { ServiceType } from "@budibase/types" +import cloneDeep from "lodash/cloneDeep" coreEnv._set("SERVICE_TYPE", ServiceType.APPS) import { join } from "path" @@ -133,6 +134,32 @@ const environment = { }, } +export function setEnv(newEnvVars: Partial): () => void { + const oldEnv = cloneDeep(environment) + + let key: keyof typeof newEnvVars + for (key in newEnvVars) { + environment._set(key, newEnvVars[key]) + } + + return () => { + for (const [key, value] of Object.entries(oldEnv)) { + environment._set(key, value) + } + } +} + +export function withEnv(envVars: Partial, f: () => T) { + const cleanup = setEnv(envVars) + const result = f() + if (result instanceof Promise) { + return result.finally(cleanup) + } else { + cleanup() + return result + } +} + function cleanVariables() { // clean up any environment variable edge cases for (let [key, value] of Object.entries(environment)) { diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 9b1ea815f5..e9d5290b2c 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -1,3 +1,4 @@ +import { setEnv as setCoreEnv } from "@budibase/backend-core" import type { GoogleSpreadsheetWorksheet } from "google-spreadsheet" import nock from "nock" @@ -40,7 +41,7 @@ describe("Google Sheets Integration", () => { let cleanupEnv: () => void beforeAll(() => { - cleanupEnv = config.setCoreEnv({ + cleanupEnv = setCoreEnv({ GOOGLE_CLIENT_ID: "test", GOOGLE_CLIENT_SECRET: "test", }) diff --git a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts index 47124d6667..252b9a6556 100644 --- a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts @@ -3,6 +3,10 @@ import { Datasource, FieldType, Row, Table } from "@budibase/types" import TestConfiguration from "../../../../../tests/utilities/TestConfiguration" import { search } from "../../../../../sdk/app/rows/search" import { generator } from "@budibase/backend-core/tests" +import { + withEnv as withCoreEnv, + setEnv as setCoreEnv, +} from "@budibase/backend-core" import { DatabaseName, getDatasource, @@ -31,13 +35,12 @@ describe.each([ let rows: Row[] beforeAll(async () => { - await config.withCoreEnv( - { SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, - () => config.init() + await withCoreEnv({ SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, () => + config.init() ) if (isSqs) { - envCleanup = config.setCoreEnv({ + envCleanup = setCoreEnv({ SQS_SEARCH_ENABLE: "true", SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], }) diff --git a/packages/server/src/startup/tests/startup.spec.ts b/packages/server/src/startup/tests/startup.spec.ts index fd2e0df61a..7209033db8 100644 --- a/packages/server/src/startup/tests/startup.spec.ts +++ b/packages/server/src/startup/tests/startup.spec.ts @@ -1,6 +1,12 @@ +import { withEnv } from "../../environment" import TestConfiguration from "../../tests/utilities/TestConfiguration" import { startup } from "../index" -import { users, utils, tenancy } from "@budibase/backend-core" +import { + users, + utils, + tenancy, + withEnv as withCoreEnv, +} from "@budibase/backend-core" import nock from "nock" describe("check BB_ADMIN environment variables", () => { @@ -23,13 +29,13 @@ describe("check BB_ADMIN environment variables", () => { const EMAIL = "budibase@budibase.com", PASSWORD = "budibase" await tenancy.doInTenant(tenancy.DEFAULT_TENANT_ID, async () => { - await config.withEnv( + await withEnv( { MULTI_TENANCY: "0", SELF_HOSTED: "1", }, () => - config.withCoreEnv( + withCoreEnv( { BB_ADMIN_USER_EMAIL: EMAIL, BB_ADMIN_USER_PASSWORD: PASSWORD, diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 3d53149385..443d3c6bd9 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -247,65 +247,6 @@ export default class TestConfiguration { } } - async withEnv(newEnvVars: Partial, f: () => Promise) { - let cleanup = this.setEnv(newEnvVars) - try { - return await f() - } finally { - cleanup() - } - } - - /* - * Sets the environment variables to the given values and returns a function - * that can be called to reset the environment variables to their original values. - */ - setEnv(newEnvVars: Partial): () => void { - const oldEnv = cloneDeep(env) - - let key: keyof typeof newEnvVars - for (key in newEnvVars) { - env._set(key, newEnvVars[key]) - } - - return () => { - for (const [key, value] of Object.entries(oldEnv)) { - env._set(key, value) - } - } - } - - async withCoreEnv( - newEnvVars: Partial, - f: () => Promise - ) { - let cleanup = this.setCoreEnv(newEnvVars) - try { - return await f() - } finally { - cleanup() - } - } - - /* - * Sets the environment variables to the given values and returns a function - * that can be called to reset the environment variables to their original values. - */ - setCoreEnv(newEnvVars: Partial): () => void { - const oldEnv = cloneDeep(coreEnv) - - let key: keyof typeof newEnvVars - for (key in newEnvVars) { - coreEnv._set(key, newEnvVars[key]) - } - - return () => { - for (const [key, value] of Object.entries(oldEnv)) { - coreEnv._set(key, value) - } - } - } - async withUser(user: User, f: () => Promise) { const oldUser = this.user this.user = user From 4887ca261ebd2c2d87e0117349d3ba1ba66dfe2f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 9 Aug 2024 11:27:43 +0100 Subject: [PATCH 6/9] Improve testing of feature flags by not polluting production flags with test ones. --- packages/backend-core/src/environment.ts | 1 + packages/backend-core/src/features/index.ts | 265 ++++++++---------- .../src/features/tests/features.spec.ts | 87 +++--- .../worker/src/api/controllers/global/self.ts | 2 +- 4 files changed, 173 insertions(+), 182 deletions(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 5091a4971a..e064398153 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -145,6 +145,7 @@ const environment = { COOKIE_DOMAIN: process.env.COOKIE_DOMAIN, PLATFORM_URL: process.env.PLATFORM_URL || "", POSTHOG_TOKEN: process.env.POSTHOG_TOKEN, + POSTHOG_API_HOST: process.env.POSTHOG_API_HOST || "https://us.i.posthog.com", ENABLE_ANALYTICS: process.env.ENABLE_ANALYTICS, TENANT_FEATURE_FLAGS: process.env.TENANT_FEATURE_FLAGS, CLOUDFRONT_CDN: process.env.CLOUDFRONT_CDN, diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index 2ffa35974f..3f0a6cf6b0 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,6 +1,5 @@ import env from "../environment" import * as context from "../context" -import { cloneDeep } from "lodash" import { PostHog, PostHogOptions } from "posthog-node" import { IdentityType } from "@budibase/types" import tracer from "dd-trace" @@ -9,13 +8,13 @@ let posthog: PostHog | undefined export function init(opts?: PostHogOptions) { if (env.POSTHOG_TOKEN) { posthog = new PostHog(env.POSTHOG_TOKEN, { - host: "https://us.i.posthog.com", + host: env.POSTHOG_API_HOST, ...opts, }) } } -abstract class Flag { +export abstract class Flag { static boolean(defaultValue: boolean): Flag { return new BooleanFlag(defaultValue) } @@ -33,6 +32,16 @@ abstract class Flag { abstract parse(value: any): T } +type UnwrapFlag = F extends Flag ? U : never + +export type FlagValues = { + [K in keyof T]: UnwrapFlag +} + +type KeysOfType = { + [K in keyof T]: T[K] extends Flag ? K : never +}[keyof T] + class BooleanFlag extends Flag { parse(value: any) { if (typeof value === "string") { @@ -73,149 +82,123 @@ class NumberFlag extends Flag { } } +export class FlagSet, T extends { [key: string]: V }> { + private readonly flags: T + + constructor(flags: T) { + this.flags = flags + } + + defaults(): FlagValues { + return Object.keys(this.flags).reduce((acc, key) => { + const typedKey = key as keyof T + acc[typedKey] = this.flags[key].defaultValue + return acc + }, {} as FlagValues) + } + + isFlagName(name: string | number | symbol): name is keyof T { + return this.flags[name as keyof T] !== undefined + } + + async get(key: K): Promise[K]> { + const flags = await this.fetch() + return flags[key] + } + + async isEnabled>(key: K): Promise { + const flags = await this.fetch() + return flags[key] + } + + async fetch(): Promise> { + return await tracer.trace("features.fetch", async span => { + const tags: Record = {} + + const flags = this.defaults() + + const currentTenantId = context.getTenantId() + const specificallySetFalse = new Set() + + const split = (env.TENANT_FEATURE_FLAGS || "") + .split(",") + .map(x => x.split(":")) + for (const [tenantId, ...features] of split) { + if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { + continue + } + + for (let feature of features) { + let value = true + if (feature.startsWith("!")) { + feature = feature.slice(1) + value = false + specificallySetFalse.add(feature) + } + + if (!this.isFlagName(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } + + if (typeof flags[feature] !== "boolean") { + throw new Error(`Feature: ${feature} is not a boolean`) + } + + // @ts-ignore + flags[feature] = value + tags[`flags.${feature}.source`] = "environment" + } + } + + const identity = context.getIdentity() + if (posthog && identity?.type === IdentityType.USER) { + const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) + for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { + const flag = this.flags[name] + if (!flag) { + // We don't want an unexpected PostHog flag to break the app, so we + // just log it and continue. + console.warn(`Unexpected posthog flag "${name}": ${value}`) + continue + } + + if (flags[name] === true || specificallySetFalse.has(name)) { + // If the flag is already set to through environment variables, we + // don't want to override it back to false here. + continue + } + + const payload = posthogFlags.featureFlagPayloads?.[name] + + try { + // @ts-ignore + flags[name] = flag.parse(payload || value) + tags[`flags.${name}.source`] = "posthog" + } catch (err) { + // We don't want an invalid PostHog flag to break the app, so we just + // log it and continue. + console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + } + } + } + + for (const [key, value] of Object.entries(flags)) { + tags[`flags.${key}.value`] = value + } + span?.addTags(tags) + + return flags + }) + } +} + // This is the primary source of truth for feature flags. If you want to add a // new flag, add it here and use the `fetch` and `get` functions to access it. // All of the machinery in this file is to make sure that flags have their // default values set correctly and their types flow through the system. -const FLAGS = { +export const flags = new FlagSet({ LICENSING: Flag.boolean(false), GOOGLE_SHEETS: Flag.boolean(false), USER_GROUPS: Flag.boolean(false), ONBOARDING_TOUR: Flag.boolean(false), - - _TEST_BOOLEAN: Flag.boolean(false), - _TEST_STRING: Flag.string("default value"), - _TEST_NUMBER: Flag.number(0), -} - -const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { - const typedKey = key as keyof typeof FLAGS - // @ts-ignore - acc[typedKey] = FLAGS[typedKey].defaultValue - return acc -}, {} as Flags) - -type UnwrapFlag = F extends Flag ? U : never -export type Flags = { - [K in keyof typeof FLAGS]: UnwrapFlag<(typeof FLAGS)[K]> -} - -// Exported for use in tests, should not be used outside of this file. -export function defaultFlags(): Flags { - return cloneDeep(DEFAULTS) -} - -function isFlagName(name: string): name is keyof Flags { - return FLAGS[name as keyof typeof FLAGS] !== undefined -} - -/** - * Reads the TENANT_FEATURE_FLAGS environment variable and returns a Flags object - * populated with the flags for the current tenant, filling in the default values - * if the flag is not set. - * - * Check the tests for examples of how TENANT_FEATURE_FLAGS should be formatted. - * - * In future we plan to add more ways of setting feature flags, e.g. PostHog, and - * they will be accessed through this function as well. - */ -export async function fetch(): Promise { - return await tracer.trace("features.fetch", async span => { - const tags: Record = {} - - const flags = defaultFlags() - - const currentTenantId = context.getTenantId() - const specificallySetFalse = new Set() - - const split = (env.TENANT_FEATURE_FLAGS || "") - .split(",") - .map(x => x.split(":")) - for (const [tenantId, ...features] of split) { - if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { - continue - } - - for (let feature of features) { - let value = true - if (feature.startsWith("!")) { - feature = feature.slice(1) - value = false - specificallySetFalse.add(feature) - } - - if (!isFlagName(feature)) { - throw new Error(`Feature: ${feature} is not an allowed option`) - } - - if (typeof flags[feature] !== "boolean") { - throw new Error(`Feature: ${feature} is not a boolean`) - } - - // @ts-ignore - flags[feature] = value - tags[`flags.${feature}.source`] = "environment" - } - } - - const identity = context.getIdentity() - if (posthog && identity?.type === IdentityType.USER) { - const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) - for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { - const key = name as keyof typeof FLAGS - const flag = FLAGS[key] - if (!flag) { - // We don't want an unexpected PostHog flag to break the app, so we - // just log it and continue. - console.warn(`Unexpected posthog flag "${name}": ${value}`) - continue - } - - if (flags[key] === true || specificallySetFalse.has(key)) { - // If the flag is already set to through environment variables, we - // don't want to override it back to false here. - continue - } - - const payload = posthogFlags.featureFlagPayloads?.[name] - - try { - // @ts-ignore - flags[key] = flag.parse(payload || value) - tags[`flags.${key}.source`] = "posthog" - } catch (err) { - // We don't want an invalid PostHog flag to break the app, so we just - // log it and continue. - console.warn(`Error parsing posthog flag "${name}": ${value}`, err) - } - } - } - - for (const [key, value] of Object.entries(flags)) { - tags[`flags.${key}.value`] = value - } - span?.addTags(tags) - - return flags - }) -} - -// Gets a single feature flag value. This is a convenience function for -// `fetch().then(flags => flags[name])`. -export async function get(name: K): Promise { - const flags = await fetch() - return flags[name] -} - -type BooleanFlags = { - [K in keyof typeof FLAGS]: (typeof FLAGS)[K] extends Flag ? K : never -}[keyof typeof FLAGS] - -// Convenience function for boolean flag values. This makes callsites more -// readable for boolean flags. -export async function isEnabled( - name: K -): Promise { - const flags = await fetch() - return flags[name] -} +}) diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 1b85a74024..95819831b2 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -1,53 +1,60 @@ import { IdentityContext, IdentityType } from "@budibase/types" -import { defaultFlags, fetch, get, Flags, init } from "../" +import { Flag, FlagSet, FlagValues, init } from "../" import { context } from "../.." import { setEnv, withEnv } from "../../environment" import nodeFetch from "node-fetch" import nock from "nock" +const schema = { + TEST_BOOLEAN: Flag.boolean(false), + TEST_STRING: Flag.string("default value"), + TEST_NUMBER: Flag.number(0), +} +const flags = new FlagSet(schema) + describe("feature flags", () => { interface TestCase { tenant: string flags: string - expected: Partial + expected: Partial> } it.each([ { tenant: "tenant1", - flags: "tenant1:_TEST_BOOLEAN", - expected: { _TEST_BOOLEAN: true }, + flags: "tenant1:TEST_BOOLEAN", + expected: { TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant1:!_TEST_BOOLEAN", - expected: { _TEST_BOOLEAN: false }, + flags: "tenant1:!TEST_BOOLEAN", + expected: { TEST_BOOLEAN: false }, }, { tenant: "tenant1", - flags: "*:_TEST_BOOLEAN", - expected: { _TEST_BOOLEAN: true }, + flags: "*:TEST_BOOLEAN", + expected: { TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant2:_TEST_BOOLEAN", - expected: { _TEST_BOOLEAN: false }, + flags: "tenant2:TEST_BOOLEAN", + expected: { TEST_BOOLEAN: false }, }, { tenant: "tenant1", flags: "", - expected: defaultFlags(), + expected: flags.defaults(), }, ])( 'should find flags $expected for $tenant with string "$flags"', - ({ tenant, flags, expected }) => + ({ tenant, flags: envFlags, expected }) => context.doInTenant(tenant, async () => - withEnv({ TENANT_FEATURE_FLAGS: flags }, async () => { - const flags = await fetch() - expect(flags).toMatchObject(expected) + withEnv({ TENANT_FEATURE_FLAGS: envFlags }, async () => { + const values = await flags.fetch() + expect(values).toMatchObject(expected) for (const [key, expectedValue] of Object.entries(expected)) { - const value = await get(key as keyof Flags) + const value = await flags.get(key as keyof typeof schema) expect(value).toBe(expectedValue) } }) @@ -63,15 +70,15 @@ describe("feature flags", () => { it.each([ { tenant: "tenant1", - flags: "tenant1:_TEST_BOOLEAN,tenant1:FOO", + flags: "tenant1:TEST_BOOLEAN,tenant1:FOO", expected: "Feature: FOO is not an allowed option", }, ])( "should fail with message \"$expected\" for $tenant with string '$flags'", - ({ tenant, flags, expected }) => + ({ tenant, flags: envFlags, expected }) => context.doInTenant(tenant, () => - withEnv({ TENANT_FEATURE_FLAGS: flags }, () => - expect(fetch).rejects.toThrow(expected) + withEnv({ TENANT_FEATURE_FLAGS: envFlags }, () => + expect(flags.fetch()).rejects.toThrow(expected) ) ) ) @@ -118,45 +125,45 @@ describe("feature flags", () => { it("should be able to read flags from posthog", async () => { mockFlags({ featureFlags: { - _TEST_BOOLEAN: true, + TEST_BOOLEAN: true, }, }) await context.doInIdentityContext(identity, async () => { - const flags = await fetch() - expect(flags._TEST_BOOLEAN).toBe(true) + const values = await flags.fetch() + expect(values.TEST_BOOLEAN).toBe(true) }) }) it("should be able to read flags from posthog with payloads", async () => { mockFlags({ featureFlags: { - _TEST_STRING: true, + TEST_STRING: true, }, featureFlagPayloads: { - _TEST_STRING: "test payload", + TEST_STRING: "test payload", }, }) await context.doInIdentityContext(identity, async () => { - const flags = await fetch() - expect(flags._TEST_STRING).toBe("test payload") + const values = await flags.fetch() + expect(values.TEST_STRING).toBe("test payload") }) }) it("should be able to read flags from posthog with numbers", async () => { mockFlags({ featureFlags: { - _TEST_NUMBER: true, + TEST_NUMBER: true, }, featureFlagPayloads: { - _TEST_NUMBER: 123, + TEST_NUMBER: 123, }, }) await context.doInIdentityContext(identity, async () => { - const flags = await fetch() - expect(flags._TEST_NUMBER).toBe(123) + const values = await flags.fetch() + expect(values.TEST_NUMBER).toBe(123) }) }) @@ -168,23 +175,23 @@ describe("feature flags", () => { }) await context.doInIdentityContext(identity, async () => { - await expect(fetch()).resolves.not.toThrow() + await expect(flags.fetch()).resolves.not.toThrow() }) }) it("should not override flags set in the environment", async () => { mockFlags({ featureFlags: { - _TEST_BOOLEAN: false, + TEST_BOOLEAN: false, }, }) await withEnv( - { TENANT_FEATURE_FLAGS: `${identity.tenantId}:_TEST_BOOLEAN` }, + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:TEST_BOOLEAN` }, async () => { await context.doInIdentityContext(identity, async () => { - const flags = await fetch() - expect(flags._TEST_BOOLEAN).toBe(true) + const values = await flags.fetch() + expect(values.TEST_BOOLEAN).toBe(true) }) } ) @@ -193,16 +200,16 @@ describe("feature flags", () => { it("should not override flags set in the environment with a ! prefix", async () => { mockFlags({ featureFlags: { - _TEST_BOOLEAN: true, + TEST_BOOLEAN: true, }, }) await withEnv( - { TENANT_FEATURE_FLAGS: `${identity.tenantId}:!_TEST_BOOLEAN` }, + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:!TEST_BOOLEAN` }, async () => { await context.doInIdentityContext(identity, async () => { - const flags = await fetch() - expect(flags._TEST_BOOLEAN).toBe(false) + const values = await flags.fetch() + expect(values.TEST_BOOLEAN).toBe(false) }) } ) diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index ec154adf7f..14a2ce8c73 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -104,7 +104,7 @@ export async function getSelf(ctx: any) { ctx.body = await groups.enrichUserRolesFromGroups(user) // add the feature flags for this tenant - const flags = await features.fetch() + const flags = await features.flags.fetch() ctx.body.flags = flags addSessionAttributesToUser(ctx) From 7505d60888946b5bbaa36020e83016bcb806cffa Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 9 Aug 2024 11:31:05 +0100 Subject: [PATCH 7/9] Extract out processor and spy variables to reduce repetition. --- .../posthog/tests/PosthogProcessor.spec.ts | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts b/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts index 1d9e341ffe..1486753e4f 100644 --- a/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts +++ b/packages/backend-core/src/events/processors/posthog/tests/PosthogProcessor.spec.ts @@ -16,6 +16,9 @@ const newIdentity = () => { } describe("PosthogProcessor", () => { + let processor: PosthogProcessor + let spy: jest.SpyInstance + beforeAll(() => { testEnv.singleTenant() }) @@ -25,25 +28,21 @@ describe("PosthogProcessor", () => { await cache.bustCache( `${CacheKey.EVENTS_RATE_LIMIT}:${Event.SERVED_BUILDER}` ) + + processor = new PosthogProcessor("test") + spy = jest.spyOn(processor.posthog, "capture") }) describe("processEvent", () => { it("processes event", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") - const identity = newIdentity() const properties = {} await processor.processEvent(Event.APP_CREATED, identity, properties) - expect(spy).toHaveBeenCalledTimes(1) }) it("honours exclusions", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") - const identity = newIdentity() const properties = {} @@ -52,9 +51,6 @@ describe("PosthogProcessor", () => { }) it("removes audited information", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") - const identity = newIdentity() const properties = { email: "test", @@ -65,6 +61,7 @@ describe("PosthogProcessor", () => { await processor.processEvent(Event.USER_CREATED, identity, properties) expect(spy).toHaveBeenCalled() + // @ts-ignore const call = processor.posthog.capture.mock.calls[0][0] expect(call.properties.audited).toBeUndefined() @@ -73,9 +70,6 @@ describe("PosthogProcessor", () => { describe("rate limiting", () => { it("sends daily event once in same day", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") - const identity = newIdentity() const properties = {} @@ -89,8 +83,6 @@ describe("PosthogProcessor", () => { }) it("sends daily event once per unique day", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} @@ -110,9 +102,6 @@ describe("PosthogProcessor", () => { }) it("sends event again after cache expires", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") - const identity = newIdentity() const properties = {} @@ -130,8 +119,6 @@ describe("PosthogProcessor", () => { }) it("sends per app events once per day per app", async () => { - const processor = new PosthogProcessor("test") - const spy = jest.spyOn(processor.posthog, "capture") const identity = newIdentity() const properties = {} From d716d4b4d0d6b434104ddeae407010681b735a3a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 9 Aug 2024 11:45:55 +0100 Subject: [PATCH 8/9] Fix test failures and lint. --- packages/pro | 2 +- packages/server/src/tests/utilities/TestConfiguration.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/pro b/packages/pro index 62ef0e2d6e..7fe713e51a 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 62ef0e2d6e83522b6732fb3c61338de303f06ff0 +Subproject commit 7fe713e51afea77c8024579582a4e1a4ec1b55b3 diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 443d3c6bd9..d5ed2ce906 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -70,7 +70,6 @@ import { } from "@budibase/types" import API from "./api" -import { cloneDeep } from "lodash" import jwt, { Secret } from "jsonwebtoken" import { Server } from "http" From 083b595d50caf1b90d9a8f72e28485fa66a43088 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 9 Aug 2024 11:58:57 +0100 Subject: [PATCH 9/9] Fix formatting. --- packages/server/src/automations/tests/openai.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/automations/tests/openai.spec.ts b/packages/server/src/automations/tests/openai.spec.ts index 41456f116d..3a5a57475a 100644 --- a/packages/server/src/automations/tests/openai.spec.ts +++ b/packages/server/src/automations/tests/openai.spec.ts @@ -1,6 +1,9 @@ import { getConfig, runStep, afterAll as _afterAll } from "./utilities" import { OpenAI } from "openai" -import { withEnv as withCoreEnv, setEnv as setCoreEnv } from "@budibase/backend-core" +import { + withEnv as withCoreEnv, + setEnv as setCoreEnv, +} from "@budibase/backend-core" jest.mock("openai", () => ({ OpenAI: jest.fn().mockImplementation(() => ({