Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App config in DynamoDB for Segment app #1698

Merged
merged 8 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sour-hotels-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"segment": patch
---

Store app config in DynamoDB instead of Saleor app metadata.
8 changes: 4 additions & 4 deletions apps/segment/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ export const env = createEnv({
PORT: z.coerce.number().optional().default(3000),
SECRET_KEY: z.string(),
VERCEL_URL: z.string().optional(),
DYNAMODB_MAIN_TABLE_NAME: z.string().optional(),
AWS_REGION: z.string().optional(),
AWS_ACCESS_KEY_ID: z.string().optional(),
AWS_SECRET_ACCESS_KEY: z.string().optional(),
DYNAMODB_MAIN_TABLE_NAME: z.string(),
AWS_REGION: z.string(),
AWS_ACCESS_KEY_ID: z.string(),
AWS_SECRET_ACCESS_KEY: z.string(),
},
shared: {
NODE_ENV: z.enum(["development", "production", "test"]).optional().default("development"),
Expand Down
43 changes: 21 additions & 22 deletions apps/segment/src/lib/dyanmodb-apl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,18 @@ describe("DynamoAPL", () => {
});

it("should return ready:true when APL related env variables are set", async () => {
const repository = new InMemoryAPLRepository();
const apl = new DynamoAPL({ repository });

const result = await apl.isReady();

expect(result).toStrictEqual({ ready: true });
});

it("should return ready:false when APL related env variables are not set", async () => {
vi.spyOn(await import("@/env"), "env", "get").mockReturnValue({
DYNAMODB_MAIN_TABLE_NAME: "table_name",
// @ts-expect-error - testing missing env variables
DYNAMODB_MAIN_TABLE_NAME: undefined,
AWS_REGION: "region",
AWS_ACCESS_KEY_ID: "access_key_id",
AWS_SECRET_ACCESS_KEY: "secret_access_key",
Expand All @@ -173,33 +183,30 @@ describe("DynamoAPL", () => {
NODE_ENV: "test",
ENV: "local",
});

const repository = new InMemoryAPLRepository();
const apl = new DynamoAPL({ repository });

const result = await apl.isReady();

expect(result).toStrictEqual({ ready: true });
expect(result).toStrictEqual({
ready: false,
error: new DynamoAPL.MissingEnvVariablesError("Missing DynamoDB env variables"),
});
});

it("should return ready:false when APL related env variables are not set", async () => {
it("should return configured:true when APL related env variables are set", async () => {
const repository = new InMemoryAPLRepository();
const apl = new DynamoAPL({ repository });

const result = await apl.isReady();
const result = await apl.isConfigured();

expect(result).toStrictEqual({
ready: false,
error: new DynamoAPL.MissingEnvVariablesError("Missing DynamoDB env variables"),
});
expect(result).toStrictEqual({ configured: true });
});

it("should return configured:true when APL related env variables are set", async () => {
it("should return configured:false when APL related env variables are not set", async () => {
vi.spyOn(await import("@/env"), "env", "get").mockReturnValue({
DYNAMODB_MAIN_TABLE_NAME: "table_name",
AWS_REGION: "region",
AWS_ACCESS_KEY_ID: "access_key_id",
AWS_SECRET_ACCESS_KEY: "secret_access_key",
// @ts-expect-error - testing missing env variables
DYNAMODB_MAIN_TABLE_NAME: undefined,
APL: "dynamodb",
APP_LOG_LEVEL: "info",
MANIFEST_APP_ID: "",
Expand All @@ -209,15 +216,7 @@ describe("DynamoAPL", () => {
NODE_ENV: "test",
ENV: "local",
});
const repository = new InMemoryAPLRepository();
const apl = new DynamoAPL({ repository });

const result = await apl.isConfigured();

expect(result).toStrictEqual({ configured: true });
});

it("should return configured:false when APL related env variables are not set", async () => {
const repository = new InMemoryAPLRepository();
const apl = new DynamoAPL({ repository });

Expand Down

This file was deleted.

48 changes: 18 additions & 30 deletions apps/segment/src/modules/configuration/app-config.ts
Original file line number Diff line number Diff line change
@@ -1,46 +1,34 @@
import { z } from "zod";
import { err, ok, Result } from "neverthrow";

import { BaseError } from "@/errors";

import { RootConfig } from "./schemas/root-config.schema";

export class AppConfig {
private rootData: RootConfig.Shape = null;
static SetSegmentKeyError = BaseError.subclass("SetSegmentKeyError");

static JSONParseError = BaseError.subclass("JSONParseError");
constructor(private rootData: RootConfig.Shape) {}

constructor(initialData?: RootConfig.Shape) {
if (initialData) {
this.rootData = RootConfig.Schema.parse(initialData);
}
}
setSegmentWriteKey(
key: string,
): Result<AppConfig, InstanceType<typeof AppConfig.SetSegmentKeyError>> {
const parsedKey = RootConfig.Schema.shape.segmentWriteKey.safeParse(key);

static parse(serializedSchema: string) {
try {
const parsedJSON = JSON.parse(serializedSchema);

return new AppConfig(parsedJSON as RootConfig.Shape);
} catch (e) {
throw new AppConfig.JSONParseError("Error parsing JSON with app config", { cause: e });
if (!parsedKey.success) {
return err(
new AppConfig.SetSegmentKeyError("Invalid segment write key", {
cause: parsedKey.error,
}),
);
}
}

serialize() {
return JSON.stringify(this.rootData);
}

setSegmentWriteKey(key: string) {
const parsedKey = z.string().min(1).parse(key);
this.rootData.segmentWriteKey = parsedKey.data;

if (this.rootData) {
this.rootData.segmentWriteKey = parsedKey;
} else {
this.rootData = {
segmentWriteKey: parsedKey,
};
}
return ok(this);
}

return this;
getSegmentWriteKey() {
return this.rootData.segmentWriteKey;
}

getConfig() {
Expand Down
84 changes: 56 additions & 28 deletions apps/segment/src/modules/configuration/configuration.router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ import { z } from "zod";

import { createLogger } from "@/logger";

import { DynamoConfigRepositoryFactory } from "../db/dynamo-config-factory";
import { protectedClientProcedure } from "../trpc/protected-client-procedure";
import { router } from "../trpc/trpc-server";
import { WebhooksActivityClient } from "../webhooks/webhook-activity/webhook-activity-client";
import { WebhookActivityService } from "../webhooks/webhook-activity/webhook-activity-service";
import { AppConfigMetadataManager } from "./app-config-metadata-manager";
import { AppConfig } from "./app-config";
import { DynamoAppConfigManager } from "./dynamo-app-config-manager";

const logger = createLogger("configurationRouter");

const configRepository = DynamoConfigRepositoryFactory.create();
const configManager = DynamoAppConfigManager.create(configRepository);

export const configurationRouter = router({
getWebhookConfig: protectedClientProcedure.query(async ({ ctx }) => {
const webhookActivityClient = new WebhooksActivityClient(ctx.apiClient);
Expand All @@ -30,45 +35,68 @@ export const configurationRouter = router({
return { areWebhooksActive: isActiveResult.value.some(Boolean) };
}),
getConfig: protectedClientProcedure.query(async ({ ctx }) => {
const manager = AppConfigMetadataManager.createFromAuthData({
appId: ctx.appId,
const config = await configManager.get({
saleorApiUrl: ctx.saleorApiUrl,
token: ctx.appToken,
appId: ctx.appId,
});

const config = await manager.get();

logger.debug("Successfully fetched config");

return config.getConfig();
if (config) {
return config.getConfig();
}

return null;
}),
setConfig: protectedClientProcedure.input(z.string().min(1)).mutation(async ({ input, ctx }) => {
const manager = AppConfigMetadataManager.createFromAuthData({
appId: ctx.appId,
saleorApiUrl: ctx.saleorApiUrl,
token: ctx.appToken,
});
setOrCreateConfig: protectedClientProcedure
.input(z.string().min(1))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe YAGNI but

  • If we accept string, procedure probably should be setting only the token/key
  • If we accept the config, we should accept the object. Can be a FormData, maybe serialized AppConfig?

Copy link
Member Author

@krzysztofzuraw krzysztofzuraw Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently config is one segmentWriteKey so I think having setOrCreateSegmentKey will be better here as I didn't plan to refactor UI form (at least now)

.mutation(async ({ input, ctx }) => {
let config: AppConfig | null;

config = await configManager.get({
saleorApiUrl: ctx.saleorApiUrl,
appId: ctx.appId,
});

const config = await manager.get();
if (!config) {
// there is no config in DynamoDB - create new one and then set `segmentWriteKey`
config = new AppConfig({
segmentWriteKey: input,
});
}

config.setSegmentWriteKey(input);
const setWriteKeyResult = config.setSegmentWriteKey(input);

await manager.set(config);
if (setWriteKeyResult.isErr()) {
logger.error("Error during setting segment write key", {
error: setWriteKeyResult.error,
});

logger.debug("Successfully set config");
throw new TRPCError({
code: "PARSE_ERROR",
message:
"There was an error with setting segment write key. Check if it has at least 1 character.",
});
}

const webhookActivityClient = new WebhooksActivityClient(ctx.apiClient);
const webhookActivityService = new WebhookActivityService(ctx.appId, webhookActivityClient);
await configManager.set({ config, saleorApiUrl: ctx.saleorApiUrl, appId: ctx.appId });

const enableAppWebhooksResult = await webhookActivityService.enableAppWebhooks();
logger.debug("Successfully set config");

if (enableAppWebhooksResult.isErr()) {
logger.error("Error during enabling app webhooks", { error: enableAppWebhooksResult.error });
const webhookActivityClient = new WebhooksActivityClient(ctx.apiClient);
const webhookActivityService = new WebhookActivityService(ctx.appId, webhookActivityClient);

throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "There with enabling app webhooks. Contact Saleor support.",
});
}
}),
const enableAppWebhooksResult = await webhookActivityService.enableAppWebhooks();

if (enableAppWebhooksResult.isErr()) {
logger.error("Error during enabling app webhooks", {
error: enableAppWebhooksResult.error,
});

throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "There is a problem with enabling app webhooks. Contact Saleor support.",
});
}
}),
});
Loading
Loading