chore(events): apply post-merge comments

Introduces a new interface `RequestDetails` to abstract `Request`
providing access to request body and headers.

**BREAKING:** Replace `request: Request` with `request: RequestDetails` at `RequestValidator`.

**BREAKING:** Remove required field `router` at `HttpPostIngressEventPublisher.fromConfig`
  and replace it with `bind(router: Router)`.
  Additionally, the path prefix `/http` will be added inside `HttpPostIngressEventPublisher`.

Relates-to: PR #13931
Signed-off-by: Patrick Jungermann <Patrick.Jungermann@gmail.com>

.changeset/fluffy-walls-approve.md

@@ -0,0 +1,21 @@
+---
+'@backstage/plugin-events-backend': minor
+---
+
+**BREAKING:** Remove required field `router` at `HttpPostIngressEventPublisher.fromConfig`
+and replace it with `bind(router: Router)`.
+Additionally, the path prefix `/http` will be added inside `HttpPostIngressEventPublisher`.
+
+```diff
+// at packages/backend/src/plugins/events.ts
+   const eventsRouter = Router();
+-  const httpRouter = Router();
+-  eventsRouter.use('/http', httpRouter);
+
+   const http = HttpPostIngressEventPublisher.fromConfig({
+     config: env.config,
+     logger: env.logger,
+-    router: httpRouter,
+   });
++  http.bind(eventsRouter);
+```

.changeset/nine-ears-whisper.md

@@ -0,0 +1,9 @@
+---
+'@backstage/plugin-events-backend': patch
+'@backstage/plugin-events-node': minor
+---
+
+Introduce a new interface `RequestDetails` to abstract `Request`
+providing access to request body and headers.
+
+**BREAKING:** Replace `request: Request` with `request: RequestDetails` at `RequestValidator`.

packages/backend/src/plugins/events.ts

@@ -27,14 +27,12 @@ export default async function createPlugin(
   subscribers: EventSubscriber[],
 ): Promise<Router> {
   const eventsRouter = Router();
-  const httpRouter = Router();
-  eventsRouter.use('/http', httpRouter);
 
   const http = HttpPostIngressEventPublisher.fromConfig({
     config: env.config,
     logger: env.logger,
-    router: httpRouter,
   });
+  http.bind(eventsRouter);
 
   await new EventsBackend(env.logger)
     .addPublishers(http)

plugins/events-backend/README.md

@@ -170,8 +170,8 @@ const http = HttpPostIngressEventPublisher.fromConfig({
     },
   },
   logger: env.logger,
-  router: httpRouter,
 });
+http.bind(router);
 
 await new EventsBackend(env.logger)
   .addPublishers(http)

plugins/events-backend/api-report.md

@@ -33,6 +33,8 @@ export const eventsPlugin: (options?: undefined) => BackendFeature;
 
 // @public
 export class HttpPostIngressEventPublisher implements EventPublisher {
+  // (undocumented)
+  bind(router: express.Router): void;
   // (undocumented)
   static fromConfig(env: {
     config: Config;
@@ -40,7 +42,6 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
       [topic: string]: Omit<HttpPostIngressOptions, 'topic'>;
     };
     logger: Logger;
-    router: express.Router;
   }): HttpPostIngressEventPublisher;
   // (undocumented)
   setEventBroker(eventBroker: EventBroker): Promise<void>;

plugins/events-backend/src/service/EventsPlugin.ts

@@ -90,14 +90,11 @@ export const eventsPlugin = createBackendPlugin({
     env.registerInit({
       deps: {
         config: configServiceRef,
-        httpRouter: httpRouterServiceRef,
         logger: loggerServiceRef,
+        router: httpRouterServiceRef,
       },
-      async init({ config, httpRouter, logger }) {
+      async init({ config, logger, router }) {
         const winstonLogger = loggerToWinstonLogger(logger);
-        const eventsRouter = Router();
-        const router = Router();
-        eventsRouter.use('/http', router);
 
         const ingresses = Object.fromEntries(
           extensionPoint.httpPostIngresses.map(ingress => [
@@ -108,23 +105,20 @@ export const eventsPlugin = createBackendPlugin({
 
         const http = HttpPostIngressEventPublisher.fromConfig({
           config,
-          logger: winstonLogger,
-          router,
           ingresses,
+          logger: winstonLogger,
         });
+        const eventsRouter = Router();
+        http.bind(eventsRouter);
+        router.use(eventsRouter);
 
-        if (!extensionPoint.eventBroker) {
-          extensionPoint.setEventBroker(new InMemoryEventBroker(winstonLogger));
-        }
+        const eventBroker =
+          extensionPoint.eventBroker ?? new InMemoryEventBroker(winstonLogger);
 
-        extensionPoint.eventBroker!.subscribe(extensionPoint.subscribers);
+        eventBroker.subscribe(extensionPoint.subscribers);
         [extensionPoint.publishers, http]
           .flat()
-          .forEach(publisher =>
-            publisher.setEventBroker(extensionPoint.eventBroker!),
-          );
-
-        httpRouter.use(eventsRouter);
+          .forEach(publisher => publisher.setEventBroker(eventBroker));
       },
     });
   },

plugins/events-backend/src/service/http/HttpPostIngressEventPublisher.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { errorHandler, getVoidLogger } from '@backstage/backend-common';
+import { getVoidLogger } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
 import { TestEventBroker } from '@backstage/plugin-events-backend-test-utils';
 import express from 'express';
@@ -35,37 +35,35 @@ describe('HttpPostIngressEventPublisher', () => {
     });
 
     const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
     const app = express().use(router);
 
     const publisher = HttpPostIngressEventPublisher.fromConfig({
       config,
-      logger,
-      router,
       ingresses: {
         testB: {},
       },
+      logger,
     });
+    publisher.bind(router);
 
     const eventBroker = new TestEventBroker();
     await publisher.setEventBroker(eventBroker);
 
     const notFoundResponse = await request(app)
-      .post('/unknown')
+      .post('/http/unknown')
       .timeout(100)
       .send({ test: 'data' });
     expect(notFoundResponse.status).toBe(404);
 
     const response1 = await request(app)
-      .post('/testA')
+      .post('/http/testA')
       .set('X-Custom-Header', 'test-value')
       .timeout(100)
       .send({ testA: 'data' });
     expect(response1.status).toBe(202);
 
     const response2 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Custom-Header', 'test-value')
       .timeout(100)
       .send({ testB: 'data' });
@@ -100,14 +98,10 @@ describe('HttpPostIngressEventPublisher', () => {
     });
 
     const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
     const app = express().use(router);
 
     const publisher = HttpPostIngressEventPublisher.fromConfig({
       config,
-      logger,
-      router,
       ingresses: {
         testB: {
           validator: async (req, context) => {
@@ -148,26 +142,28 @@ describe('HttpPostIngressEventPublisher', () => {
           },
         },
       },
+      logger,
     });
+    publisher.bind(router);
 
     const eventBroker = new TestEventBroker();
     await publisher.setEventBroker(eventBroker);
 
     const response1 = await request(app)
-      .post('/testA')
+      .post('/http/testA')
       .timeout(100)
       .send({ test: 'data' });
     expect(response1.status).toBe(202);
 
     const response2 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .timeout(100)
       .send({ test: 'data' });
     expect(response2.status).toBe(400);
     expect(response2.body).toEqual({ message: 'wrong signature' });
 
     const response3 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Test-Signature', 'wrong')
       .timeout(100)
       .send({ test: 'data' });
@@ -175,21 +171,21 @@ describe('HttpPostIngressEventPublisher', () => {
     expect(response3.body).toEqual({ message: 'wrong signature' });
 
     const response4 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Test-Signature', 'testB-signature')
       .timeout(100)
       .send({ test: 'data' });
     expect(response4.status).toBe(202);
 
     const response5 = await request(app)
-      .post('/testC')
+      .post('/http/testC')
       .timeout(100)
       .send({ test: 'data' });
     expect(response5.status).toBe(404);
     expect(response5.body).toEqual({});
 
     const response6 = await request(app)
-      .post('/testD')
+      .post('/http/testD')
       .timeout(100)
       .send({ test: 'data' });
     expect(response6.status).toBe(403);
@@ -210,15 +206,10 @@ describe('HttpPostIngressEventPublisher', () => {
   it('without configuration', async () => {
     const config = new ConfigReader({});
 
-    const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
-
     expect(() =>
       HttpPostIngressEventPublisher.fromConfig({
         config,
         logger,
-        router,
       }),
     ).not.toThrow();
   });

plugins/events-backend/src/service/http/HttpPostIngressEventPublisher.ts

@@ -41,7 +41,6 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
     config: Config;
     ingresses?: { [topic: string]: Omit<HttpPostIngressOptions, 'topic'> };
     logger: Logger;
-    router: express.Router;
   }): HttpPostIngressEventPublisher {
     const topics =
       env.config.getOptionalStringArray('events.http.topics') ?? [];
@@ -55,15 +54,18 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
       }
     });
 
-    return new HttpPostIngressEventPublisher(env.logger, env.router, ingresses);
+    return new HttpPostIngressEventPublisher(env.logger, ingresses);
   }
 
   private constructor(
-    private logger: Logger,
-    router: express.Router,
-    ingresses: { [topic: string]: Omit<HttpPostIngressOptions, 'topic'> },
-  ) {
-    router.use(this.createRouter(ingresses));
+    private readonly logger: Logger,
+    private readonly ingresses: {
+      [topic: string]: Omit<HttpPostIngressOptions, 'topic'>;
+    },
+  ) {}
+
+  bind(router: express.Router): void {
+    router.use('/http', this.createRouter(this.ingresses));
   }
 
   async setEventBroker(eventBroker: EventBroker): Promise<void> {
@@ -92,8 +94,12 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
     const path = `/${topic}`;
 
     router.post(path, async (request, response) => {
+      const requestDetails = {
+        body: request.body,
+        headers: request.headers,
+      };
       const context = new RequestValidationContextImpl();
-      await validator?.(request, context);
+      await validator?.(requestDetails, context);
       if (context.wasRejected()) {
         response
           .status(context.rejectionDetails!.status)

plugins/events-node/api-report.md

@@ -4,7 +4,6 @@
 
 ```ts
 import { ExtensionPoint } from '@backstage/backend-plugin-api';
-import { Request as Request_2 } from 'express';
 
 // @public
 export interface EventBroker {
@@ -74,6 +73,12 @@ export interface HttpPostIngressOptions {
   validator?: RequestValidator;
 }
 
+// @public (undocumented)
+export interface RequestDetails {
+  body: unknown;
+  headers: Record<string, string | string[] | undefined>;
+}
+
 // @public
 export interface RequestRejectionDetails {
   // (undocumented)
@@ -89,7 +94,7 @@ export interface RequestValidationContext {
 
 // @public
 export type RequestValidator = (
-  request: Request_2,
+  request: RequestDetails,
   context: RequestValidationContext,
 ) => Promise<void>;
 

plugins/events-node/package.json

@@ -24,9 +24,7 @@
     "postpack": "backstage-cli package postpack"
   },
   "dependencies": {
-    "@backstage/backend-plugin-api": "workspace:^",
-    "@types/express": "^4.17.6",
-    "express": "^4.17.1"
+    "@backstage/backend-plugin-api": "workspace:^"
   },
   "devDependencies": {
     "@backstage/cli": "workspace:^"

plugins/events-node/src/api/http/validation/RequestDetails.ts

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2022 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @public
+ */
+export interface RequestDetails {
+  /**
+   * Request body. JSON payloads have been parsed already.
+   */
+  body: unknown;
+  /**
+   * Key-value pairs of header names and values. Header names are lower-cased.
+   */
+  headers: Record<string, string | string[] | undefined>;
+}

plugins/events-node/src/api/http/validation/RequestValidator.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Request } from 'express';
+import { RequestDetails } from './RequestDetails';
 import { RequestValidationContext } from './RequestValidationContext';
 
 /**
@@ -29,6 +29,6 @@ import { RequestValidationContext } from './RequestValidationContext';
  * @public
  */
 export type RequestValidator = (
-  request: Request,
+  request: RequestDetails,
   context: RequestValidationContext,
 ) => Promise<void>;

plugins/events-node/src/api/http/validation/index.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+export type { RequestDetails } from './RequestDetails';
 export type { RequestRejectionDetails } from './RequestRejectionDetails';
 export type { RequestValidationContext } from './RequestValidationContext';
 export type { RequestValidator } from './RequestValidator';

yarn.lock

@@ -5232,8 +5232,6 @@ __metadata:
   dependencies:
     "@backstage/backend-plugin-api": "workspace:^"
     "@backstage/cli": "workspace:^"
-    "@types/express": ^4.17.6
-    express: ^4.17.1
   languageName: unknown
   linkType: soft