-
Notifications
You must be signed in to change notification settings - Fork 337
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
Add DynamoDB APL to Segment app #1690
Changes from 2 commits
092b929
9106a77
e2282dd
3dfe0dc
2d35b77
72680ea
7682437
e67ddcf
af4d5f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
services: | ||
dynamodb: | ||
command: "-jar DynamoDBLocal.jar -sharedDb -dbPath ./data" | ||
image: "amazon/dynamodb-local:latest" | ||
ports: | ||
- "8000:8000" | ||
volumes: | ||
- "./docker/dynamodb:/home/dynamodblocal/data" | ||
working_dir: /home/dynamodblocal | ||
volumes: | ||
dynamodb: | ||
driver: local |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
#!/bin/bash | ||
if ! aws dynamodb describe-table --table-name segment-main-table --endpoint-url http://localhost:8000 --region localhost >/dev/null 2>&1; then | ||
aws dynamodb create-table --table-name segment-main-table \ | ||
--attribute-definitions AttributeName=PK,AttributeType=S AttributeName=SK,AttributeType=S \ | ||
--key-schema AttributeName=PK,KeyType=HASH AttributeName=SK,KeyType=RANGE \ | ||
--provisioned-throughput ReadCapacityUnits=5,WriteCapacityUnits=5 \ | ||
--endpoint-url http://localhost:8000 \ | ||
--region localhost | ||
else | ||
echo "Table segment-main-table already exists - creation is skipped" | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import { APL, AplConfiguredResult, AplReadyResult, AuthData } from "@saleor/app-sdk/APL"; | ||
|
||
import { BaseError } from "@/errors"; | ||
import { SegmentAPLRepository } from "@/modules/db/segment-apl-repository"; | ||
import { SegmentAPLEntityType } from "@/modules/db/segment-main-table"; | ||
|
||
export class DynamoAPL implements APL { | ||
private segmentAPLRepository: SegmentAPLRepository; | ||
|
||
static SetAuthDataError = BaseError.subclass("SetAuthDataError"); | ||
static DeleteAuthDataError = BaseError.subclass("DeleteAuthDataError"); | ||
static MissingEnvVariablesError = BaseError.subclass("MissingEnvVariablesError"); | ||
|
||
constructor({ segmentAPLEntity }: { segmentAPLEntity: SegmentAPLEntityType }) { | ||
this.segmentAPLRepository = new SegmentAPLRepository({ segmentAPLEntity }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why dont you inject repo? |
||
} | ||
|
||
async get(saleorApiUrl: string): Promise<AuthData | undefined> { | ||
const getEntryResult = await this.segmentAPLRepository.getEntry({ | ||
saleorApiUrl, | ||
}); | ||
|
||
if (getEntryResult.isErr()) { | ||
// TODO: should we throw here? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, because APL interface is allowing undefined as a non-existing value but, we should check:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we have a easy way of knowing if there is connection error - I checked and toolbox doesn't expose that info if it throws error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense - I will update implementation to add such logic |
||
return undefined; | ||
} | ||
|
||
return getEntryResult.value; | ||
} | ||
|
||
async set(authData: AuthData): Promise<void> { | ||
const setEntryResult = await this.segmentAPLRepository.setEntry({ | ||
authData, | ||
}); | ||
|
||
if (setEntryResult.isErr()) { | ||
lkostrowski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new DynamoAPL.SetAuthDataError("Failed to set APL entry", { | ||
cause: setEntryResult.error, | ||
}); | ||
} | ||
|
||
return undefined; | ||
} | ||
|
||
async delete(saleorApiUrl: string): Promise<void> { | ||
const deleteEntryResult = await this.segmentAPLRepository.deleteEntry({ | ||
saleorApiUrl, | ||
}); | ||
|
||
if (deleteEntryResult.isErr()) { | ||
throw new DynamoAPL.DeleteAuthDataError("Failed to delete APL entry", { | ||
cause: deleteEntryResult.error, | ||
}); | ||
} | ||
|
||
return undefined; | ||
} | ||
|
||
async getAll(): Promise<AuthData[]> { | ||
const getAllEntriesResult = await this.segmentAPLRepository.getAllEntries(); | ||
|
||
if (getAllEntriesResult.isErr()) { | ||
// TODO: should we throw here? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above- depending on the error type |
||
return []; | ||
} | ||
|
||
return getAllEntriesResult.value; | ||
} | ||
|
||
async isReady(): Promise<AplReadyResult> { | ||
const ready = this.envVariablesRequriedByDynamoDBExist(); | ||
|
||
return ready | ||
? { | ||
ready: true, | ||
} | ||
: { | ||
ready: false, | ||
error: new DynamoAPL.MissingEnvVariablesError("Missing DynamoDB env variables"), | ||
}; | ||
} | ||
|
||
async isConfigured(): Promise<AplConfiguredResult> { | ||
const configured = this.envVariablesRequriedByDynamoDBExist(); | ||
|
||
return configured | ||
? { | ||
configured: true, | ||
} | ||
: { | ||
configured: false, | ||
error: new DynamoAPL.MissingEnvVariablesError("Missing DynamoDB env variables"), | ||
}; | ||
} | ||
|
||
private envVariablesRequriedByDynamoDBExist() { | ||
const variables = [ | ||
"DYNAMODB_MAIN_TABLE_NAME", | ||
"AWS_REGION", | ||
"AWS_ACCESS_KEY_ID", | ||
"AWS_SECRET_ACCESS_KEY", | ||
]; | ||
|
||
// eslint-disable-next-line node/no-process-env | ||
return variables.every((variable) => !!process.env[variable]); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { DynamoDBClient } from "@aws-sdk/client-dynamodb"; | ||
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; | ||
|
||
export const createDynamoDBClient = () => { | ||
const client = new DynamoDBClient(); | ||
|
||
return client; | ||
}; | ||
|
||
export const createDynamoDBDocumentClient = (client: DynamoDBClient) => { | ||
return DynamoDBDocumentClient.from(client); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { AuthData } from "@saleor/app-sdk/APL"; | ||
import { FormattedItem, type PutItemInput } from "dynamodb-toolbox"; | ||
|
||
import { SegmentAPLEntityType, SegmentMainTable } from "@/modules/db/segment-main-table"; | ||
|
||
export class SegmentAPLMapper { | ||
dynamoDBEntityToAuthData(entity: FormattedItem<SegmentAPLEntityType>): AuthData { | ||
return { | ||
domain: entity.domain, | ||
token: entity.token, | ||
saleorApiUrl: entity.saleorApiUrl, | ||
appId: entity.appId, | ||
jwks: entity.jwks, | ||
}; | ||
} | ||
|
||
authDataToDynamoPutEntity(authData: AuthData): PutItemInput<SegmentAPLEntityType> { | ||
return { | ||
PK: SegmentMainTable.getAPLPrimaryKey({ saleorApiUrl: authData.saleorApiUrl }), | ||
SK: SegmentMainTable.getAPLSortKey(), | ||
domain: authData.domain, | ||
token: authData.token, | ||
saleorApiUrl: authData.saleorApiUrl, | ||
appId: authData.appId, | ||
jwks: authData.jwks, | ||
}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick, but the class is APL and app context is Segment, so you "know" the outer context - you dont have to prefix it. Calling variable "repo" or "repository" is verbose enough