diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 11fea0f..484cfe6 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1 +1 @@ -{".":"1.0.0"} +{ ".": "1.0.0" } diff --git a/package-lock.json b/package-lock.json index 7abfba2..bb02ef1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "feedback-api", - "version": "1.0.0", + "version": "0.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "feedback-api", - "version": "1.0.0", + "version": "0.2.1", "hasInstallScript": true, "license": "ISC", "dependencies": { @@ -25,7 +25,9 @@ "@map-colonies/tsconfig": "^2.0.0", "@opentelemetry/api": "^1.9.1", "@opentelemetry/api-metrics": "0.23.0", + "@sinclair/typebox": "^0.34.49", "@types/body-parser": "^1.19.6", + "ajv": "^8.20.0", "compression": "^1.8.0", "express": "^4.21.2", "express-openapi-validator": "^5.6.2", @@ -938,14 +940,14 @@ } }, "node_modules/@emnapi/core": { - "version": "1.10.0", - "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.10.0.tgz", - "integrity": "sha512-yq6OkJ4p82CAfPl0u9mQebQHKPJkY7WrIuk205cTYnYe+k2Z8YBh11FrbRG/H6ihirqcacOgl2BIO8oyMQLeXw==", + "version": "1.11.1", + "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.11.1.tgz", + "integrity": "sha512-RSvbQmHzdKzNsLYa/wHrbc3KN4sYLKAdPZxqiM2HATqv/SBk2/ENSHpvXGaLOMcsAyz0poEGqkmmKYG3OWiJEQ==", "dev": true, "license": "MIT", "optional": true, "dependencies": { - "@emnapi/wasi-threads": "1.2.1", + "@emnapi/wasi-threads": "1.2.2", "tslib": "^2.4.0" } }, @@ -958,9 +960,9 @@ "optional": true }, "node_modules/@emnapi/runtime": { - "version": "1.10.0", - "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.10.0.tgz", - "integrity": "sha512-ewvYlk86xUoGI0zQRNq/mC+16R1QeDlKQy21Ki3oSYXNgLb45GV1P6A0M+/s6nyCuNDqe5VpaY84BzXGwVbwFA==", + "version": "1.11.1", + "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.11.1.tgz", + "integrity": "sha512-vgj7R3y3Wgx24IQaGPA/R6YFXLHVMOZ0uVEyIQPaWs+rd1AzfEMXlAC22FYwO1XkKR6NPsq7mUandH8oIRdZFw==", "dev": true, "license": "MIT", "optional": true, @@ -977,9 +979,9 @@ "optional": true }, "node_modules/@emnapi/wasi-threads": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.1.tgz", - "integrity": "sha512-uTII7OYF+/Mes/MrcIOYp5yOtSMLBWSIoLPpcgwipoiKbli6k322tcoFsxoIIxPDqW01SQGAgko4EzZi2BNv2w==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.2.tgz", + "integrity": "sha512-c95qOXkHdydNKhscBTebqEC1CVAZpyqOfVfBzQ1qgzyl3gfeldUjIggDbIZgDKsHLgnsM+igH7TJ/eAasaVuMA==", "dev": true, "license": "MIT", "optional": true, @@ -1944,6 +1946,13 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/@jest/schemas/node_modules/@sinclair/typebox": { + "version": "0.27.10", + "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.10.tgz", + "integrity": "sha512-MTBk/3jGLNB2tVxv6uLlFh1iu64iYOQ2PbdOSK3NW8JZsmlaOh2q6sdtKowBhfw8QFLmYNzTW4/oK4uATIi6ZA==", + "dev": true, + "license": "MIT" + }, "node_modules/@jest/source-map": { "version": "29.6.3", "resolved": "https://registry.npmjs.org/@jest/source-map/-/source-map-29.6.3.tgz", @@ -5084,48 +5093,6 @@ "node": "^20.19.0 || >=22.12.0" } }, - "node_modules/@rolldown/binding-wasm32-wasi/node_modules/@emnapi/core": { - "version": "1.11.1", - "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.11.1.tgz", - "integrity": "sha512-RSvbQmHzdKzNsLYa/wHrbc3KN4sYLKAdPZxqiM2HATqv/SBk2/ENSHpvXGaLOMcsAyz0poEGqkmmKYG3OWiJEQ==", - "dev": true, - "license": "MIT", - "optional": true, - "dependencies": { - "@emnapi/wasi-threads": "1.2.2", - "tslib": "^2.4.0" - } - }, - "node_modules/@rolldown/binding-wasm32-wasi/node_modules/@emnapi/runtime": { - "version": "1.11.1", - "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.11.1.tgz", - "integrity": "sha512-vgj7R3y3Wgx24IQaGPA/R6YFXLHVMOZ0uVEyIQPaWs+rd1AzfEMXlAC22FYwO1XkKR6NPsq7mUandH8oIRdZFw==", - "dev": true, - "license": "MIT", - "optional": true, - "dependencies": { - "tslib": "^2.4.0" - } - }, - "node_modules/@rolldown/binding-wasm32-wasi/node_modules/@emnapi/wasi-threads": { - "version": "1.2.2", - "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.2.tgz", - "integrity": "sha512-c95qOXkHdydNKhscBTebqEC1CVAZpyqOfVfBzQ1qgzyl3gfeldUjIggDbIZgDKsHLgnsM+igH7TJ/eAasaVuMA==", - "dev": true, - "license": "MIT", - "optional": true, - "dependencies": { - "tslib": "^2.4.0" - } - }, - "node_modules/@rolldown/binding-wasm32-wasi/node_modules/tslib": { - "version": "2.8.1", - "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", - "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", - "dev": true, - "license": "0BSD", - "optional": true - }, "node_modules/@rolldown/binding-win32-arm64-msvc": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@rolldown/binding-win32-arm64-msvc/-/binding-win32-arm64-msvc-1.1.4.tgz", @@ -5204,10 +5171,9 @@ } }, "node_modules/@sinclair/typebox": { - "version": "0.27.10", - "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.10.tgz", - "integrity": "sha512-MTBk/3jGLNB2tVxv6uLlFh1iu64iYOQ2PbdOSK3NW8JZsmlaOh2q6sdtKowBhfw8QFLmYNzTW4/oK4uATIi6ZA==", - "dev": true, + "version": "0.34.52", + "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.34.52.tgz", + "integrity": "sha512-XiMQh7qqVlxZzcVD+kkGMNGMzcTrDMLWI7S4x7z1MkCkbDPrekpZXEUK0eZqZFMuHQg2a2DZOcDIh9o5v3Gonw==", "license": "MIT" }, "node_modules/@sinonjs/commons": { @@ -6267,6 +6233,48 @@ "node": ">=14.0.0" } }, + "node_modules/@unrs/resolver-binding-wasm32-wasi/node_modules/@emnapi/core": { + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.10.0.tgz", + "integrity": "sha512-yq6OkJ4p82CAfPl0u9mQebQHKPJkY7WrIuk205cTYnYe+k2Z8YBh11FrbRG/H6ihirqcacOgl2BIO8oyMQLeXw==", + "dev": true, + "license": "MIT", + "optional": true, + "dependencies": { + "@emnapi/wasi-threads": "1.2.1", + "tslib": "^2.4.0" + } + }, + "node_modules/@unrs/resolver-binding-wasm32-wasi/node_modules/@emnapi/runtime": { + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.10.0.tgz", + "integrity": "sha512-ewvYlk86xUoGI0zQRNq/mC+16R1QeDlKQy21Ki3oSYXNgLb45GV1P6A0M+/s6nyCuNDqe5VpaY84BzXGwVbwFA==", + "dev": true, + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, + "node_modules/@unrs/resolver-binding-wasm32-wasi/node_modules/@emnapi/wasi-threads": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.1.tgz", + "integrity": "sha512-uTII7OYF+/Mes/MrcIOYp5yOtSMLBWSIoLPpcgwipoiKbli6k322tcoFsxoIIxPDqW01SQGAgko4EzZi2BNv2w==", + "dev": true, + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, + "node_modules/@unrs/resolver-binding-wasm32-wasi/node_modules/tslib": { + "version": "2.8.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", + "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", + "dev": true, + "license": "0BSD", + "optional": true + }, "node_modules/@unrs/resolver-binding-win32-arm64-msvc": { "version": "1.12.2", "resolved": "https://registry.npmjs.org/@unrs/resolver-binding-win32-arm64-msvc/-/resolver-binding-win32-arm64-msvc-1.12.2.tgz", diff --git a/package.json b/package.json index 71918e5..885faa7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "feedback-api", - "version": "1.0.0", + "version": "0.2.1", "description": "Feedback-api service for MapColonies", "main": "./src/index.ts", "scripts": { @@ -55,7 +55,9 @@ "@map-colonies/tsconfig": "^2.0.0", "@opentelemetry/api": "^1.9.1", "@opentelemetry/api-metrics": "0.23.0", + "@sinclair/typebox": "^0.34.49", "@types/body-parser": "^1.19.6", + "ajv": "^8.20.0", "compression": "^1.8.0", "express": "^4.21.2", "express-openapi-validator": "^5.6.2", diff --git a/src/common/errors.ts b/src/common/errors.ts index c0ab799..45d32ab 100644 --- a/src/common/errors.ts +++ b/src/common/errors.ts @@ -24,3 +24,11 @@ export class BadRequestError extends Error implements HttpError { super(message); } } + +export class GeocodingResponseParseError extends Error { + public constructor(message: string) { + super(message); + Object.setPrototypeOf(this, GeocodingResponseParseError.prototype); + this.name = 'GeocodingResponseParseError'; + } +} diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index d253b58..db1cd2d 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -1,3 +1,4 @@ +import { Type, type Static } from '@sinclair/typebox'; import type { vectorFeedbackApiV1Type } from '@map-colonies/schemas'; export interface OpenApiConfig { @@ -17,11 +18,13 @@ export interface FeedbackResponse { geocodingResponse: GeocodingResponse; } -export interface GeocodingResponse { - userId?: string; - apiKey: string; - site: string; - response: JSON; - respondedAt: Date; // from Geocoding - wasUsed?: boolean; -} +export const geocodingResponseSchema = Type.Object({ + userId: Type.Optional(Type.String()), + apiKey: Type.String(), + site: Type.String(), + response: Type.Any(), + respondedAt: Type.Unsafe({ type: 'string' }), + wasUsed: Type.Optional(Type.Boolean()), +}); + +export type GeocodingResponse = Static; diff --git a/src/common/utils.ts b/src/common/utils.ts index 9af5d25..89c02a7 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -1,4 +1,17 @@ -import { TimeoutError } from './errors'; +import ajvLib from 'ajv'; +import { GeocodingResponseParseError, TimeoutError } from './errors'; +import { geocodingResponseSchema, type GeocodingResponse } from './interfaces'; + +const ajv = new ajvLib(); +const validateGeocodingResponse = ajv.compile(geocodingResponseSchema); + +export function parseGeocodingResponse(data: string): GeocodingResponse | GeocodingResponseParseError { + const parsed: unknown = JSON.parse(data); + if (!validateGeocodingResponse(parsed)) { + return new GeocodingResponseParseError(`Invalid geocoding response shape: ${ajv.errorsText(validateGeocodingResponse.errors)}`); + } + return parsed; +} export const promiseTimeout = async (ms: number, promise: Promise): Promise => { // create a promise that rejects in milliseconds diff --git a/src/feedback/models/feedbackManager.ts b/src/feedback/models/feedbackManager.ts index 72fb75f..e7f0397 100644 --- a/src/feedback/models/feedbackManager.ts +++ b/src/feedback/models/feedbackManager.ts @@ -3,7 +3,8 @@ import { inject, injectable } from 'tsyringe'; import { type Producer } from 'kafkajs'; import { SERVICES } from '@common/constants'; import { FeedbackResponse, GeocodingResponse } from '@common/interfaces'; -import { NotFoundError, BadRequestError } from '@common/errors'; +import { parseGeocodingResponse } from '@common/utils'; +import { NotFoundError, BadRequestError, GeocodingResponseParseError } from '@common/errors'; import { type ConfigType } from '@src/common/config'; import { type RedisClient } from '../../redis'; import { IFeedbackModel } from './feedback'; @@ -20,12 +21,11 @@ export class FeedbackManager { public async createFeedback(feedback: IFeedbackModel, apiKey: string): Promise { const requestId = feedback.request_id; const userId = feedback.user_id; - const raw = this.config.get('application.userValidation'); - const userValidation = Array.isArray(raw) ? raw : (JSON.parse(raw) as string[]); + const userValidation = this.config.get('application.userValidation'); const ttl = this.config.get('redis.ttl'); const prefix = this.config.get('redis.prefix'); - const validateUser = !userValidation.some((validEnding) => validEnding !== '' && userId.endsWith(validEnding)); + const validateUser = !userValidation.some((validEnding: string) => validEnding !== '' && userId.endsWith(validEnding)); if (validateUser) { throw new BadRequestError(`user_id not valid. valid user_id ends with "${JSON.stringify(userValidation)}"`); } @@ -49,9 +49,18 @@ export class FeedbackManager { public async getGeocodingResponse(requestId: string, userId: string, apiKey: string): Promise { try { - const redisResponse = (await this.redisClient.get(requestId)) as string; - if (redisResponse) { - const geocodingResponse = JSON.parse(redisResponse) as GeocodingResponse; + const redisResponse = await this.redisClient.get(requestId); + if (redisResponse != null) { + const geocodingResponse = parseGeocodingResponse(redisResponse); + + if (geocodingResponse instanceof GeocodingResponseParseError) { + this.logger.error({ + msg: `Error parsing geocoding response for requestId: ${requestId}, error: ${geocodingResponse.message} }`, + err: geocodingResponse, + }); + throw geocodingResponse; + } + geocodingResponse.userId = userId; geocodingResponse.apiKey = apiKey; geocodingResponse.wasUsed = true; diff --git a/src/redis/subscribe.ts b/src/redis/subscribe.ts index 85d0c54..b086837 100644 --- a/src/redis/subscribe.ts +++ b/src/redis/subscribe.ts @@ -4,7 +4,8 @@ import type { Producer } from 'kafkajs'; import type { ConfigType } from '@src/common/config'; import { REDIS_SUB, SERVICES } from '../common/constants'; import type { FeedbackResponse, GeocodingResponse } from '../common/interfaces'; -import { NotFoundError } from '../common/errors'; +import { parseGeocodingResponse } from '../common/utils'; +import { GeocodingResponseParseError, NotFoundError } from '../common/errors'; import type { RedisClient } from '../redis/index'; const TTL_PREFIX = 'ttl:'; @@ -65,10 +66,15 @@ export const redisSubscribe = async (deps: DependencyContainer): Promise(partial: Partial>): MockedObject => partial as MockedObject; + +export const makeGeocodingResponseJson = (overrides: Partial = {}): string => + JSON.stringify({ + apiKey: 'test-api-key', + site: 'test-site', + response: {}, + respondedAt: '2024-01-01T00:00:00.000Z', + ...overrides, + }); diff --git a/tests/integration/feedback/feedback.spec.ts b/tests/integration/feedback/feedback.spec.ts index 6cd5a74..197adb7 100644 --- a/tests/integration/feedback/feedback.spec.ts +++ b/tests/integration/feedback/feedback.spec.ts @@ -70,7 +70,7 @@ describe('feedback', function () { userId: '1', apiKey: '1', site: 'test', - response: JSON.parse('["USA"]') as JSON, + response: ['USA'], respondedAt: new Date('2024-08-29T14:39:10.602Z'), }; const redisKey = crypto.randomUUID(); @@ -91,7 +91,7 @@ describe('feedback', function () { const geocodingResponse: GeocodingResponse = { apiKey: '1', site: 'test', - response: JSON.parse('["USA"]') as JSON, + response: ['USA'], respondedAt: new Date('2024-08-29T14:39:10.602Z'), }; const redisKey = crypto.randomUUID(); @@ -113,7 +113,7 @@ describe('feedback', function () { const geocodingResponse: GeocodingResponse = { apiKey: '1', site: 'test', - response: JSON.parse('["USA"]') as JSON, + response: ['USA'], respondedAt: new Date(), }; await redisClient.set(requestId, JSON.stringify(geocodingResponse)); @@ -151,7 +151,7 @@ describe('feedback', function () { userId: '1', apiKey: '1', site: 'test', - response: JSON.parse('["USA"]') as JSON, + response: ['USA'], respondedAt: new Date('2024-08-29T14:39:10.602Z'), }; const redisKey = crypto.randomUUID(); @@ -240,7 +240,7 @@ describe('feedback', function () { geocodingResponse: { apiKey: '1', site: 'test', - response: JSON.parse('["USA"]') as JSON, + response: ['USA'], respondedAt: new Date(), }, }; diff --git a/tests/unit/feedback/models/feedbackModel.spec.ts b/tests/unit/feedback/models/feedbackModel.spec.ts index 76a049a..2ccd76b 100644 --- a/tests/unit/feedback/models/feedbackModel.spec.ts +++ b/tests/unit/feedback/models/feedbackModel.spec.ts @@ -11,7 +11,7 @@ import type { IFeedbackModel } from '@src/feedback/models/feedback'; import { BadRequestError, NotFoundError } from '@src/common/errors'; import type { ConfigType } from '@src/common/config'; import type { RedisClient } from '@src/redis'; -import { createMock } from '../../../helpers/createMock'; +import { createMock, makeGeocodingResponseJson } from '../../../helpers/createMock'; type MockConfig = { [K in keyof ConfigType]: Mock; @@ -96,14 +96,14 @@ describe('FeedbackManager', () => { describe('#createFeadback', () => { it('should create feedback without errors', async () => { const feedbackRequest = makeFeedbackRequest(); - (mockedRedis.get as Mock).mockResolvedValue('{ "geocodingResponse": "completed" }'); + (mockedRedis.get as Mock).mockResolvedValue(makeGeocodingResponseJson()); const feedback = await feedbackManager.createFeedback(feedbackRequest, 'token'); expect(feedback.requestId).toBe(feedbackRequest.request_id); expect(feedback.chosenResultId).toBe(feedbackRequest.chosen_result_id); expect(feedback.userId).toBe(feedbackRequest.user_id); - expect(feedback.geocodingResponse).toMatchObject({ geocodingResponse: 'completed' }); + expect(feedback.geocodingResponse).toMatchObject({ site: 'test-site' }); expect(mockedRedis.get).toHaveBeenCalledTimes(1); }); @@ -117,8 +117,7 @@ describe('FeedbackManager', () => { }); const mockedManager = makeManager(mockedConfig); - (mockedRedis.get as Mock).mockResolvedValue('{ "geocodingResponse": "completed" }'); - + (mockedRedis.get as Mock).mockResolvedValue(makeGeocodingResponseJson()); const feedback = await mockedManager.createFeedback(feedbackRequest, 'token'); const expectedKey = `feedback-test:${feedbackRequest.request_id}`; @@ -135,7 +134,7 @@ describe('FeedbackManager', () => { }); const mockedManager = makeManager(mockedConfig); - (mockedRedis.get as Mock).mockResolvedValue('{ "geocodingResponse": "completed" }'); + (mockedRedis.get as Mock).mockResolvedValue(makeGeocodingResponseJson()); await mockedManager.createFeedback(feedbackRequest, 'token'); @@ -156,7 +155,8 @@ describe('FeedbackManager', () => { it('should not be able to upload feedback to kafka', async () => { const feedbackRequest = makeFeedbackRequest(); - (mockedRedis.get as Mock).mockResolvedValue('{ "geocodingResponse": "completed" }'); + (mockedRedis.get as Mock).mockResolvedValue(makeGeocodingResponseJson()); + (mockProducer.send as Mock).mockRejectedValue(new Error('Kafka error')); await expect(feedbackManager.createFeedback(feedbackRequest, 'token')).rejects.toThrow(new Error('Kafka error')); diff --git a/tests/unit/redis/subscribe.spec.ts b/tests/unit/redis/subscribe.spec.ts index 23b6f94..72a3099 100644 --- a/tests/unit/redis/subscribe.spec.ts +++ b/tests/unit/redis/subscribe.spec.ts @@ -4,7 +4,7 @@ import type { DependencyContainer } from 'tsyringe'; import { describe, beforeEach, it, expect, vi, type Mock } from 'vitest'; import { redisSubscribe } from '@src/redis/subscribe'; import { REDIS_SUB, SERVICES } from '@src/common/constants'; -import { createMock } from '../../helpers/createMock'; +import { createMock, makeGeocodingResponseJson } from '../../helpers/createMock'; type SetCallback = (message: string) => Promise; type ExpiredCallback = (message: string) => Promise; @@ -154,21 +154,21 @@ describe('redisSubscribe', () => { beforeEach(async () => setup()); it('should send no-chosen result to Kafka when wasUsed is false', async () => { - mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: false })); + mockRedisClient.get.mockResolvedValue(makeGeocodingResponseJson({ wasUsed: false })); await expiredCallback('ttl:some-uuid'); expect(mockProducer.send).toHaveBeenCalled(); }); it('should NOT send to Kafka when wasUsed is true', async () => { - mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: true })); + mockRedisClient.get.mockResolvedValue(makeGeocodingResponseJson({ wasUsed: true })); await expiredCallback('ttl:some-uuid'); expect(mockProducer.send).not.toHaveBeenCalled(); }); it('should always delete the original key after expiry', async () => { - mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: true })); + mockRedisClient.get.mockResolvedValue(makeGeocodingResponseJson({ wasUsed: true })); await expiredCallback('ttl:some-uuid'); expect(mockRedisClient.del).toHaveBeenCalledWith('some-uuid');