Merge pull request #74 from PrivateAIM/feature/71-revise-subscription…

…-endpoint

Feature/71 Revise Message Subscription Endpoint

src/server/message/subscription/subscription.controller.ts

@@ -1,10 +1,12 @@
 /* eslint-disable max-classes-per-file */
 // Disabled to allow keeping validation classes close to where they are used!
 import {
-    Body, Controller, Get, HttpCode, Param, Post, Req, Res, UseGuards,
+    BadGatewayException,
+    Body, Controller, Get, HttpCode, HttpException, InternalServerErrorException, NotFoundException, Param, Post, Req, Res, UseGuards,
 } from '@nestjs/common';
-import { IsNotEmpty, IsString, IsUrl } from 'class-validator';
+import { IsUrl } from 'class-validator';
 import { Request, Response } from 'express';
+import { APIClient } from '@privateaim/core';
 import type { SubscriptionDto } from './subscription.service';
 import { MessageSubscriptionService } from './subscription.service';
 import { AuthGuard } from '../../auth/auth.guard';
@@ -14,39 +16,67 @@ import { AuthGuard } from '../../auth/auth.guard';
  * Makes use of auto-validation using `class-validator`.
  */
 class AddSubscriptionRequestBody {
-    @IsString()
-    @IsNotEmpty()
-        analysisId: string;
-
     @IsUrl({ require_tld: false })
         webhookUrl: URL;
     // TODO: might need some auth information as well (left out for brevity at the moment)
 }
 
+// TODO: this somehow needs to be handled by the API Client -> maybe introduce a wrapper later on
+function handleHubApiError(err: any, analysisId: string) {
+    if (err.statusCode !== undefined) {
+        if ((err.statusCode as number) === 404) {
+            throw new NotFoundException(`analysis '${analysisId}' does not exist`);
+        }
+
+        if ((err.statusCode as number) >= 500) {
+            throw new BadGatewayException(`cannot check existence of analysis '${analysisId}'`, {
+                cause: err,
+                description: 'unrecoverable error when requesting central side (hub)',
+            });
+        }
+    } else {
+        throw new InternalServerErrorException(`cannot check existence of analysis '${analysisId}'`, { cause: err });
+    }
+}
+
 /**
  * Bundles API endpoints related to message subscriptions.
  */
-@Controller('messages')
+@Controller('analyses/:id/messages')
 @UseGuards(AuthGuard)
 export class MessageSubscriptionController {
     private readonly subscriptionService: MessageSubscriptionService;
 
-    constructor(subscriptionService: MessageSubscriptionService) {
+    private readonly hubApiClient: APIClient;
+
+    constructor(subscriptionService: MessageSubscriptionService, hubApiClient: APIClient) {
         this.subscriptionService = subscriptionService;
+        this.hubApiClient = hubApiClient;
     }
 
     @Post('subscriptions')
     @HttpCode(201)
-    async subscribe(@Body() data: AddSubscriptionRequestBody, @Req() req: Request, @Res() res: Response) {
-        const { id } = await this.subscriptionService.addSubscription({
-            analysisId: data.analysisId,
-            webhookUrl: data.webhookUrl,
-        });
-        const subscriptionResourceLocation = `${req.protocol}://${req.get('Host')}${req.originalUrl}/${id}`;
-        res.header('Location', subscriptionResourceLocation);
-        res.json({
-            subscriptionId: id,
-        });
+    async subscribe(@Param('id') analysisId: string, @Body() data: AddSubscriptionRequestBody, @Req() req: Request, @Res() res: Response) {
+        return this.hubApiClient.analysis.getOne(analysisId)
+            .catch((err) => handleHubApiError(err, analysisId))
+            .then(() => this.subscriptionService.addSubscription({
+                analysisId,
+                webhookUrl: data.webhookUrl,
+            }))
+            .catch((err) => {
+                if (err instanceof HttpException) {
+                    throw err;
+                } else {
+                    throw new InternalServerErrorException(`cannot save subscription for analysis '${analysisId}'`, { cause: err });
+                }
+            })
+            .then(({ id }) => {
+                const subscriptionResourceLocation = `${req.protocol}://${req.get('Host')}${req.originalUrl}/${id}`;
+                res.header('Location', subscriptionResourceLocation);
+                res.json({
+                    subscriptionId: id,
+                });
+            });
     }
 
     @Get('subscriptions/:id')

src/server/message/subscription/subscription.module.ts

@@ -1,5 +1,9 @@
 import { Module } from '@nestjs/common';
 import { MongooseModule } from '@nestjs/mongoose';
+import { APIClient } from '@privateaim/core';
+import { ConfigService } from '@nestjs/config';
+import type { ClientResponseErrorTokenHookOptions } from '@authup/core';
+import { mountClientResponseErrorTokenHook } from '@authup/core';
 import { SubscriptionSchema } from './persistence/subscription.schema';
 import { MessageSubscriptionService } from './subscription.service';
 import { MessageSubscriptionController } from './subscription.controller';
@@ -10,7 +14,30 @@ import { MessageSubscriptionController } from './subscription.controller';
 @Module({
     imports: [MongooseModule.forFeature([{ name: 'subscription', schema: SubscriptionSchema }])],
     controllers: [MessageSubscriptionController],
-    providers: [MessageSubscriptionService],
+    providers: [
+        {
+            provide: APIClient,
+            useFactory: async (configService: ConfigService) => {
+                const hookOptions: ClientResponseErrorTokenHookOptions = {
+                    baseURL: configService.getOrThrow<string>('hub.auth.baseUrl'),
+                    tokenCreator: {
+                        type: 'robot',
+                        id: configService.getOrThrow<string>('hub.auth.robotId'),
+                        secret: configService.getOrThrow<string>('hub.auth.robotSecret'),
+                    },
+                };
+
+                const hubClient = new APIClient({
+                    baseURL: configService.get<string>('hub.baseUrl'),
+                });
+                mountClientResponseErrorTokenHook(hubClient, hookOptions);
+
+                return hubClient;
+            },
+            inject: [ConfigService],
+        },
+        MessageSubscriptionService,
+    ],
     exports: [MessageSubscriptionService],
 })
 export class MessageSubscriptionModule { }

test/unit/server/message/subscription/subscription.e2e.spec.ts

@@ -1,11 +1,14 @@
+/* eslint-disable prefer-promise-reject-errors */
 import request from 'supertest';
 import type { INestApplication } from '@nestjs/common';
-import { ValidationPipe } from '@nestjs/common';
+import { HttpStatus, ValidationPipe } from '@nestjs/common';
 import { Test } from '@nestjs/testing';
 import { MongooseModule } from '@nestjs/mongoose';
 import type { StartedTestContainer } from 'testcontainers';
 import { GenericContainer, Wait } from 'testcontainers';
 import { APP_PIPE } from '@nestjs/core';
+import type { Analysis } from '@privateaim/core';
+import { APIClient } from '@privateaim/core';
 import { MessageSubscriptionModule } from '../../../../../src/server/message/subscription/subscription.module';
 import { AuthGuard } from '../../../../../src/server/auth/auth.guard';
 
@@ -14,6 +17,7 @@ const MONGO_DB_TEST_DB_NAME: string = 'message-broker-subscriptions-test';
 describe('Message Subscription Module', () => {
     let mongodbEnv: StartedTestContainer;
     let app: INestApplication;
+    let hubApiClient: APIClient;
 
     beforeAll(async () => {
         mongodbEnv = await new GenericContainer('mongo:7.0.5@sha256:fcde2d71bf00b592c9cabab1d7d01defde37d69b3d788c53c3bc7431b6b15de8')
@@ -26,6 +30,8 @@ describe('Message Subscription Module', () => {
 
         const mongoDbConnString = `mongodb://${mongodbEnv.getHost()}:${mongodbEnv.getFirstMappedPort()}/`;
 
+        hubApiClient = new APIClient({});
+
         const moduleRef = await Test.createTestingModule({
             imports: [
                 MongooseModule.forRootAsync({
@@ -45,25 +51,33 @@ describe('Message Subscription Module', () => {
         })
             .overrideGuard(AuthGuard)
             .useValue({ canActivate: () => true })
+            .overrideProvider(APIClient)
+            .useValue(hubApiClient)
             .compile();
 
         app = moduleRef.createNestApplication();
         await app.init();
     }, 300000); // timeout takes into account that this image might have to be pulled first
 
+    beforeEach(() => {
+        jest.clearAllMocks();
+    });
+
     afterAll(async () => {
         await app.close();
         await mongodbEnv.stop();
     });
 
     it('/POST subscriptions should persist a new subscription', async () => {
-        const testAnalysisId = 'foo';
+        const testAnalysisId = 'd985ddb4-e0af-407f-afd0-6d002813d29c';
         const testWebhookUrl = 'http://localhost/bar';
 
+        jest.spyOn(hubApiClient.analysis, 'getOne').mockImplementation(() => Promise.resolve(jest.fn() as unknown as Analysis));
+
         const subscriptionId = await request(app.getHttpServer())
-            .post('/messages/subscriptions')
-            .send({ analysisId: testAnalysisId, webhookUrl: testWebhookUrl })
-            .expect(201)
+            .post(`/analyses/${testAnalysisId}/messages/subscriptions`)
+            .send({ webhookUrl: testWebhookUrl })
+            .expect(HttpStatus.CREATED)
             .then((res) => {
                 const { location } = res.header;
                 const { subscriptionId } = res.body;
@@ -73,8 +87,8 @@ describe('Message Subscription Module', () => {
             });
 
         await request(app.getHttpServer())
-            .get(`/messages/subscriptions/${subscriptionId}`)
-            .expect(200)
+            .get(`/analyses/${testAnalysisId}/messages/subscriptions/${subscriptionId}`)
+            .expect(HttpStatus.OK)
             .then((res) => {
                 expect(res.body.id).toBe(subscriptionId);
                 expect(res.body.analysisId).toBe(testAnalysisId);
@@ -83,14 +97,45 @@ describe('Message Subscription Module', () => {
     });
 
     it.each([
-        ['', 'http://localhost/foo'],
-        [1, 'http://localhost/foo'],
-        ['analysis-1', ''],
-        ['analysis-1', 'not:a-domain'],
-    ])('/POST subscriptions should return an error on malformed body', async (analysisId, webhookUrl) => {
+        [''],
+        ['not:a-domain'],
+    ])('/POST subscriptions should return an error on malformed body', async (webhookUrl) => {
+        await request(app.getHttpServer())
+            .post('/analyses/foo/messages/subscriptions')
+            .send({ webhookUrl })
+            .expect(HttpStatus.BAD_REQUEST);
+    });
+
+    it('/POST subscriptions returns 404 if analysis does not exist', async () => {
+        const testAnalysisId = 'b2b1a935-3f9f-452d-83c2-d5a21a5cd616';
+        const testWebhookUrl = 'http://localhost/bar';
+
+        jest.spyOn(hubApiClient.analysis, 'getOne').mockImplementation(() => Promise.reject({
+            statusCode: 404,
+        }));
+
+        await request(app.getHttpServer())
+            .post(`/analyses/${testAnalysisId}/messages/subscriptions`)
+            .send({ webhookUrl: testWebhookUrl })
+            .expect(HttpStatus.NOT_FOUND);
+    });
+
+    it.each([
+        [500],
+        [501],
+        [502],
+        [503],
+    ])('/POST subscriptions returns 502 if central side ', async (httpStatusCode) => {
+        const testAnalysisId = 'b2b1a935-3f9f-452d-83c2-d5a21a5cd616';
+        const testWebhookUrl = 'http://localhost/bar';
+
+        jest.spyOn(hubApiClient.analysis, 'getOne').mockImplementation(() => Promise.reject({
+            statusCode: httpStatusCode,
+        }));
+
         await request(app.getHttpServer())
-            .post('/messages/subscriptions')
-            .send({ analysisId, webhookUrl })
-            .expect(400);
+            .post(`/analyses/${testAnalysisId}/messages/subscriptions`)
+            .send({ webhookUrl: testWebhookUrl })
+            .expect(HttpStatus.BAD_GATEWAY);
     });
 });