-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(webhooks): allow client registration through env #118
Changes from all commits
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 |
---|---|---|
|
@@ -2,6 +2,7 @@ import { Entity, Column, PrimaryGeneratedColumn, Unique } from "typeorm"; | |
|
||
@Entity() | ||
@Unique("UK_webhook_client_api_key", ["apiKey"]) | ||
@Unique("UK_webhook_client_name", ["name"]) | ||
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 wnated to make a unique constraint for name, i dont know if i did this right because i have no idea what UK_webhook means |
||
export class WebhookClient { | ||
@PrimaryGeneratedColumn() | ||
id: number; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { MigrationInterface, QueryRunner } from "typeorm"; | ||
|
||
export class Webhook1732730161339 implements MigrationInterface { | ||
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. did the migration after adding uniquye name constraint, again dont know if this is correct |
||
name = "Webhook1732730161339"; | ||
|
||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`ALTER TABLE "webhook_client" ADD CONSTRAINT "UQ_a08bea1c3eba7711301141ae001" UNIQUE ("name")`, | ||
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`ALTER TABLE "webhook_client" DROP CONSTRAINT "UQ_a08bea1c3eba7711301141ae001"`, | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ export type Dependencies = { | |
postgres: DataSource; | ||
logger: Logger; | ||
webhooksQueuesService: WebhooksQueuesService; | ||
clientRepository: WebhookClientRepository; | ||
}; | ||
export class EventProcessorManager { | ||
private logger: Logger; | ||
|
@@ -38,8 +39,8 @@ export class EventProcessorManager { | |
|
||
constructor(deps: Dependencies) { | ||
this.logger = deps.logger; | ||
this.clientRepository = new WebhookClientRepository(deps.postgres); // Initialize the client manager | ||
this.webhooksQueuesService = deps.webhooksQueuesService; | ||
this.clientRepository = deps.clientRepository; | ||
} | ||
|
||
// Register a new type of webhook processor able to be written to | ||
|
@@ -76,12 +77,13 @@ export class EventProcessorManager { | |
`Attempting to register webhook of type: ${params.type} with URL: ${params.url}`, | ||
); | ||
const client = await this.clientRepository.getClientByApiKey(apiKey); | ||
const urlDomain = new URL(params.url).hostname; | ||
const isDomainValid = client.domains.includes(urlDomain); | ||
assert( | ||
isDomainValid, | ||
"The base URL of the provided webhook does not match any of the client domains", | ||
); | ||
// TODO: Reinable this potentially when we need it, but not great for testing | ||
// const urlDomain = new URL(params.url).hostname; | ||
// const isDomainValid = client.domains.includes(urlDomain); | ||
// assert( | ||
// isDomainValid, | ||
// "The base URL of the provided webhook does not match any of the client domains", | ||
// ); | ||
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. removed domain checking for now |
||
assert((params.filter as any).depositTxHash, "depositTxHash is required"); | ||
assert((params.filter as any).originChainId, "originChainId is required"); | ||
const webhook = this.getEventProcessor(params.type as WebhookTypes); | ||
|
@@ -119,10 +121,4 @@ export class EventProcessorManager { | |
`Successfully unregistered webhook with ID: ${params.id}`, | ||
); | ||
} | ||
|
||
async registerClient(client: entities.WebhookClient) { | ||
this.logger.debug(`Attempting to register client with ID: ${client.id}`); | ||
await this.clientRepository.registerClient(client); | ||
this.logger.debug(`Successfully registered client with ID: ${client.id}`); | ||
} | ||
} |
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.
Usually a factory is a class that has a builder function on it. Is this Factory a function?