From 74ebf8bcae6ea80453bb29c0497534f34a4a1127 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Sep 2024 20:36:24 +0200 Subject: [PATCH] events-backend: minor review fixes Signed-off-by: Patrik Oldsberg --- .../src/service/EventsPlugin.test.ts | 2 +- .../src/service/hub/DatabaseEventBusStore.ts | 23 +++++++++-------- .../src/service/hub/createEventBusRouter.ts | 25 +++++++++---------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/plugins/events-backend/src/service/EventsPlugin.test.ts b/plugins/events-backend/src/service/EventsPlugin.test.ts index 37b4cfd961..2c0befe1fd 100644 --- a/plugins/events-backend/src/service/EventsPlugin.test.ts +++ b/plugins/events-backend/src/service/EventsPlugin.test.ts @@ -139,7 +139,7 @@ describe('eventsPlugin', () => { } const databases = TestDatabases.create({ - ids: ['SQLITE_3', 'POSTGRES_9', 'POSTGRES_13', 'POSTGRES_16'], + ids: ['SQLITE_3', 'MYSQL_8', 'POSTGRES_9', 'POSTGRES_13', 'POSTGRES_16'], }); async function mockKnexFactory(databaseId: TestDatabaseId) { diff --git a/plugins/events-backend/src/service/hub/DatabaseEventBusStore.ts b/plugins/events-backend/src/service/hub/DatabaseEventBusStore.ts index c94fa2aa2c..0f4085e794 100644 --- a/plugins/events-backend/src/service/hub/DatabaseEventBusStore.ts +++ b/plugins/events-backend/src/service/hub/DatabaseEventBusStore.ts @@ -269,7 +269,6 @@ export class DatabaseEventBusStore implements EventBusStore { await db.migrate.latest({ directory: migrationsDir, }); - options.logger.info('DatabaseEventBusStore migrations ran successfully'); } const listener = new DatabaseEventBusListener(db.client, options.logger); @@ -288,7 +287,7 @@ export class DatabaseEventBusStore implements EventBusStore { frequency: { seconds: 10 }, timeout: { minutes: 1 }, initialDelay: { seconds: 10 }, - fn: store.#cleanup, + fn: () => store.#cleanup(), }); options.lifecycle.addShutdownHook(async () => { @@ -532,7 +531,7 @@ export class DatabaseEventBusStore implements EventBusStore { ); } - #cleanup = async () => { + async #cleanup() { try { const eventCount = await this.#db(TABLE_EVENTS) .delete() @@ -549,9 +548,11 @@ export class DatabaseEventBusStore implements EventBusStore { // If events are outside the max age they will always be deleted .orWhere('created_at', '<', new Date(Date.now() - this.#windowMaxAge)); - this.#logger.info( - `Event cleanup resulted in ${eventCount} old events being deleted`, - ); + if (eventCount > 0) { + this.#logger.info( + `Event cleanup resulted in ${eventCount} old events being deleted`, + ); + } } catch (error) { this.#logger.error('Event cleanup failed', error); } @@ -572,11 +573,13 @@ export class DatabaseEventBusStore implements EventBusStore { .where('read_until', '<', minId - 1); } - this.#logger.info( - `Subscription cleanup resulted in ${subscriberCount} stale subscribers being deleted`, - ); + if (subscriberCount > 0) { + this.#logger.info( + `Subscription cleanup resulted in ${subscriberCount} stale subscribers being deleted`, + ); + } } catch (error) { this.#logger.error('Subscription cleanup failed', error); } - }; + } } diff --git a/plugins/events-backend/src/service/hub/createEventBusRouter.ts b/plugins/events-backend/src/service/hub/createEventBusRouter.ts index b1e99596b1..485f1b0e58 100644 --- a/plugins/events-backend/src/service/hub/createEventBusRouter.ts +++ b/plugins/events-backend/src/service/hub/createEventBusRouter.ts @@ -22,7 +22,6 @@ import { SchedulerService, } from '@backstage/backend-plugin-api'; import { Handler } from 'express'; -import Router from 'express-promise-router'; import { createOpenApiRouter } from '../../schema/openapi.generated'; import { MemoryEventBusStore } from './MemoryEventBusStore'; import { DatabaseEventBusStore } from './DatabaseEventBusStore'; @@ -145,14 +144,11 @@ export async function createEventBusRouter(options: { }): Promise { const { httpAuth, notifyTimeoutMs = DEFAULT_NOTIFY_TIMEOUT_MS } = options; const logger = options.logger.child({ type: 'EventBus' }); - const router = Router(); const store = await createEventBusStore(options); const apiRouter = await createOpenApiRouter(); - router.use(apiRouter); - apiRouter.post('/bus/v1/events', async (req, res) => { const credentials = await httpAuth.credentials(req, { allow: ['service'], @@ -164,22 +160,25 @@ export async function createEventBusRouter(options: { topic, eventPayload: req.body.event.payload, } as EventParams, - notifiedSubscribers: req.body.notifiedSubscribers, + notifiedSubscribers, }); if (result) { - logger.info(`Published event to '${topic}' with ID '${result.eventId}'`, { - subject: credentials.principal.subject, - }); + logger.debug( + `Published event to '${topic}' with ID '${result.eventId}'`, + { + subject: credentials.principal.subject, + }, + ); res.status(201).end(); } else { if (notifiedSubscribers) { const notified = `'${notifiedSubscribers.join("', '")}'`; - logger.info( + logger.debug( `Skipped publishing of event to '${topic}', subscribers have already been notified: ${notified}`, { subject: credentials.principal.subject }, ); } else { - logger.info( + logger.debug( `Skipped publishing of event to '${topic}', no subscribers present`, { subject: credentials.principal.subject }, ); @@ -216,7 +215,7 @@ export async function createEventBusRouter(options: { try { const { events } = await store.readSubscription(id); - logger.info( + logger.debug( `Reading subscription '${id}' resulted in ${events.length} events`, { subject: credentials.principal.subject }, ); @@ -234,7 +233,7 @@ export async function createEventBusRouter(options: { try { const { topic } = await listener.waitForUpdate(); - logger.info( + logger.debug( `Received notification for subscription '${id}' for topic '${topic}'`, { subject: credentials.principal.subject }, ); @@ -266,7 +265,7 @@ export async function createEventBusRouter(options: { await store.upsertSubscription(id, req.body.topics); - logger.info( + logger.debug( `New subscription '${id}' for topics '${req.body.topics.join("', '")}'`, { subject: credentials.principal.subject }, );