From bb7f9698f39b671c60a0bd78afdb06a1bfea5efe Mon Sep 17 00:00:00 2001 From: Courey Elliott Date: Wed, 31 Jul 2024 15:42:08 -0500 Subject: [PATCH 1/4] updated synonymGroups table-stream with tests formatting additional test cleaning up additional testing with adjustments removed dynamodb calls and adjusted mocks adds additional test for no duplicate eventId adjusting logic so that nothing happens if there is no data to save instead of creating a new synonym group cleaning up tests bailing if somehow there is no data for the group using dynamodb as source of truth again and updating tests --- __tests__/table-streams/synonyms.ts | 204 +++++++++++++++++++++++++ app/routes/synonyms/synonyms.lib.ts | 8 + app/routes/synonyms/synonyms.server.ts | 6 +- app/table-streams/synonyms/index.ts | 26 ++-- sandbox-invoke-mocks.js | 14 ++ sandbox-search.js | 14 +- 6 files changed, 257 insertions(+), 15 deletions(-) create mode 100644 __tests__/table-streams/synonyms.ts diff --git a/__tests__/table-streams/synonyms.ts b/__tests__/table-streams/synonyms.ts new file mode 100644 index 000000000..a24f9e6d6 --- /dev/null +++ b/__tests__/table-streams/synonyms.ts @@ -0,0 +1,204 @@ +import { tables } from '@architect/functions' +import { search } from '@nasa-gcn/architect-functions-search' +import type { DynamoDBRecord } from 'aws-lambda' + +import type { Synonym } from '~/routes/synonyms/synonyms.lib' +import { handler } from '~/table-streams/synonyms/index' + +const synonymId = 'abcde-abcde-abcde-abcde-abcde' +const eventId = 'GRB 123' +const existingEventId = 'GRB 99999999' + +const putData = { + index: 'synonym-groups', + id: synonymId, + body: { + synonymId, + eventIds: [] as string[], + }, +} + +jest.mock('@nasa-gcn/architect-functions-search', () => ({ + search: jest.fn(), +})) + +jest.mock('@architect/functions', () => ({ + tables: jest.fn(), +})) + +const mockIndex = jest.fn() +const mockDelete = jest.fn() +const mockGet = jest.fn() +const mockQuery = jest.fn() + +const mockStreamEvent = { + Records: [ + { + eventID: '1', + eventName: 'INSERT', + eventVersion: '1.0', + eventSource: 'aws:dynamodb', + awsRegion: 'us-west-2', + dynamodb: { + Keys: { + synonymId: { + S: synonymId, + }, + eventId: { + S: eventId, + }, + }, + NewImage: { + synonymId: { + S: synonymId, + }, + eventId: { + S: eventId, + }, + }, + SequenceNumber: '111', + SizeBytes: 26, + StreamViewType: 'NEW_IMAGE', + }, + eventSourceARN: + 'arn:aws:dynamodb:us-west-2:123456789012:table/synonym-groups/stream/2020-01-01T00:00:00.000', + } as DynamoDBRecord, + ], +} + +afterEach(() => { + jest.clearAllMocks() +}) + +describe('testing put synonymGroup table-stream', () => { + test('insert new synonym group', async () => { + const mockItems = [{ synonymId, eventId }] + const mockClient = { + synonyms: { + query: mockQuery, + }, + } + mockQuery.mockResolvedValue({ Items: mockItems }) + ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) + putData.body.eventIds = [eventId] + mockGet.mockResolvedValue(null) + ;(search as unknown as jest.Mock).mockReturnValue({ + index: mockIndex, + delete: mockDelete, + get: mockGet, + }) + + await handler(mockStreamEvent) + + expect(mockIndex).toHaveBeenCalledWith(putData) + }) + + test('insert into existing synonym group', async () => { + const mockItems = [ + { synonymId, eventId: existingEventId }, + { synonymId, eventId }, + ] + const mockClient = { + synonyms: { + query: mockQuery, + }, + } + mockQuery.mockResolvedValue({ Items: mockItems }) + ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) + putData.body.eventIds = [existingEventId, eventId] + mockGet.mockResolvedValue({ + body: { _source: { synonymId, eventIds: [existingEventId] } }, + }) + ;(search as unknown as jest.Mock).mockReturnValue({ + index: mockIndex, + delete: mockDelete, + get: mockGet, + }) + + await handler(mockStreamEvent) + + expect(mockIndex).toHaveBeenCalledWith(putData) + }) + + test('insert only once', async () => { + const mockItems = [ + { synonymId, eventId: existingEventId }, + { synonymId, eventId }, + ] + const mockClient = { + synonyms: { + query: mockQuery, + }, + } + mockQuery.mockResolvedValue({ Items: mockItems }) + ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) + putData.body.eventIds = [existingEventId, eventId] + mockGet.mockResolvedValue({ + body: { _source: { synonymId, eventIds: [existingEventId, eventId] } }, + }) + ;(search as unknown as jest.Mock).mockReturnValue({ + index: mockIndex, + delete: mockDelete, + get: mockGet, + }) + + await handler(mockStreamEvent) + + expect(mockIndex).toHaveBeenCalledWith(putData) + }) +}) + +describe('testing delete synonymGroup table-stream', () => { + test('remove one eventId while leaving others', async () => { + const mockItems = [{ synonymId, eventId: existingEventId }] + const mockClient = { + synonyms: { + query: mockQuery, + }, + } + mockQuery.mockResolvedValue({ Items: mockItems }) + ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) + mockStreamEvent.Records[0].eventName = 'REMOVE' + putData.body.eventIds = [existingEventId] + mockGet.mockResolvedValue({ + body: { _source: { synonymId, eventIds: [existingEventId, eventId] } }, + }) + ;(search as unknown as jest.Mock).mockReturnValue({ + index: mockIndex, + delete: mockDelete, + get: mockGet, + }) + + await handler(mockStreamEvent) + + expect(mockIndex).toHaveBeenCalledWith(putData) + }) + + test('remove final synonym and delete synonym group', async () => { + const mockItems = [] as Synonym[] + const mockClient = { + synonyms: { + query: mockQuery, + }, + } + mockQuery.mockResolvedValue({ Items: mockItems }) + ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) + mockStreamEvent.Records[0].eventName = 'REMOVE' + const deleteData = { + index: 'synonym-groups', + id: synonymId, + } + mockGet.mockResolvedValue({ + body: { _source: { synonymId, eventIds: [eventId] } }, + }) + ;(search as unknown as jest.Mock).mockReturnValue({ + index: mockIndex, + delete: mockDelete, + get: mockGet, + }) + + await handler(mockStreamEvent) + + expect(mockDelete).toHaveBeenCalledWith(deleteData) + }) +}) diff --git a/app/routes/synonyms/synonyms.lib.ts b/app/routes/synonyms/synonyms.lib.ts index 638fe925b..a0e3a40ac 100644 --- a/app/routes/synonyms/synonyms.lib.ts +++ b/app/routes/synonyms/synonyms.lib.ts @@ -5,7 +5,15 @@ * * SPDX-License-Identifier: Apache-2.0 */ + +/* Data structure in DynamoDB */ export interface Synonym { eventId: string synonymId: string } + +/* Layout of materialized view in OpenSearch */ +export interface SynonymGroup { + synonymId: string + eventIds: string[] +} diff --git a/app/routes/synonyms/synonyms.server.ts b/app/routes/synonyms/synonyms.server.ts index 1fbc57e98..684d30893 100644 --- a/app/routes/synonyms/synonyms.server.ts +++ b/app/routes/synonyms/synonyms.server.ts @@ -6,7 +6,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import { tables } from '@architect/functions' -import type { DynamoDBDocument } from '@aws-sdk/lib-dynamodb/dist-types/DynamoDBDocument' +import type { DynamoDBDocument } from '@aws-sdk/lib-dynamodb' import { search as getSearchClient } from '@nasa-gcn/architect-functions-search' import crypto from 'crypto' @@ -54,7 +54,7 @@ export async function searchSynonymsByEventId({ if (eventId) { query.bool.should.push({ match: { - eventId: { + eventIds: { query: eventId, fuzziness: 'AUTO', }, @@ -70,7 +70,7 @@ export async function searchSynonymsByEventId({ }, }, } = await client.search({ - index: 'synonyms', + index: 'synonym-groups', from: page && limit && (page - 1) * limit, size: limit, body: { diff --git a/app/table-streams/synonyms/index.ts b/app/table-streams/synonyms/index.ts index c118bdac1..13bcab584 100644 --- a/app/table-streams/synonyms/index.ts +++ b/app/table-streams/synonyms/index.ts @@ -15,9 +15,10 @@ import type { } from 'aws-lambda' import { createTriggerHandler } from '~/lib/lambdaTrigger.server' -import type { Synonym } from '~/routes/synonyms/synonyms.lib' +import type { Synonym, SynonymGroup } from '~/routes/synonyms/synonyms.lib' +import { getSynonymsByUuid } from '~/routes/synonyms/synonyms.server' -const index = 'synonyms' +const index = 'synonym-groups' function unmarshallTrigger(item?: Record) { return unmarshall(item as Record) @@ -34,26 +35,31 @@ async function removeIndex(id: string) { } } -async function putIndex(synonym: Synonym) { +async function putIndex(synonymGroup: SynonymGroup) { const client = await getSearchClient() await client.index({ index, - body: synonym, + id: synonymGroup.synonymId, + body: synonymGroup, }) } export const handler = createTriggerHandler( async ({ eventName, dynamodb }: DynamoDBRecord) => { - const id = unmarshallTrigger(dynamodb!.Keys).id as string + if (!eventName || !dynamodb) return const promises = [] - - if (eventName === 'REMOVE') { + const id = unmarshallTrigger(dynamodb!.Keys).synonymId + const synonym = unmarshallTrigger(dynamodb!.NewImage) as Synonym + const dynamoSynonyms = await getSynonymsByUuid(synonym.synonymId) + const group = { + synonymId: synonym.synonymId, + eventIds: dynamoSynonyms.map((synonym) => synonym.eventId), + } + if (eventName === 'REMOVE' && group.eventIds.length === 0) { promises.push(removeIndex(id)) } /* (eventName === 'INSERT' || eventName === 'MODIFY') */ else { - const synonym = unmarshallTrigger(dynamodb!.NewImage) as Synonym - promises.push(putIndex(synonym)) + promises.push(putIndex(group)) } - await Promise.all(promises) } ) diff --git a/sandbox-invoke-mocks.js b/sandbox-invoke-mocks.js index fdf001dda..bfe93f55f 100644 --- a/sandbox-invoke-mocks.js +++ b/sandbox-invoke-mocks.js @@ -217,6 +217,20 @@ export default { body: 'This is a test', }, }, + synonyms: { + INSERT: { + eventId: 'GRB 230727A', + synonymId: 'fe00636d-ae9a-45fe-9941-9cbc6e84da04', + }, + REMOVE: { + eventId: 'GRB 230727A', + synonymId: 'fe00636d-ae9a-45fe-9941-9cbc6e84da04', + }, + MODIFY: { + eventId: 'GRB 230727A', + synonymId: 'fe00636d-ae9a-45fe-9941-9cbc6e84da04', + }, + }, 'circulars-kafka-distribution': { INSERT: { circularId: 40000, diff --git a/sandbox-search.js b/sandbox-search.js index bfae389dc..84f3e1044 100644 --- a/sandbox-search.js +++ b/sandbox-search.js @@ -11,13 +11,23 @@ export default async function () { const text = await readFile('sandbox-seed.json', { encoding: 'utf-8' }) const { circulars, synonyms } = JSON.parse(text) + const groupedSynonyms = synonyms.reduce((accumulator, synonym) => { + ;(accumulator[synonym.synonymId] ??= []).push(synonym.eventId) + return accumulator + }, {}) + + const groups = Object.keys(groupedSynonyms).map((id) => ({ + synonymId: id, + eventIds: groupedSynonyms[id], + })) + return [ ...circulars.flatMap((item) => [ { index: { _index: 'circulars', _id: item.circularId.toString() } }, item, ]), - ...synonyms.flatMap((item) => [ - { index: { _index: 'synonyms', _id: item.id } }, + ...groups.flatMap((item) => [ + { index: { _index: 'synonym-groups', _id: item.synonymId.toString() } }, item, ]), ] From 26c3a2863e2a7389d7aa77d52284ae1f5cec9e6b Mon Sep 17 00:00:00 2001 From: Courey Elliott Date: Thu, 8 Aug 2024 11:53:20 -0400 Subject: [PATCH 2/4] simplifying handler conditional --- __tests__/table-streams/synonyms.ts | 19 ------------------- app/table-streams/synonyms/index.ts | 22 +++++++++------------- 2 files changed, 9 insertions(+), 32 deletions(-) diff --git a/__tests__/table-streams/synonyms.ts b/__tests__/table-streams/synonyms.ts index a24f9e6d6..9eeedd17f 100644 --- a/__tests__/table-streams/synonyms.ts +++ b/__tests__/table-streams/synonyms.ts @@ -28,7 +28,6 @@ jest.mock('@architect/functions', () => ({ const mockIndex = jest.fn() const mockDelete = jest.fn() -const mockGet = jest.fn() const mockQuery = jest.fn() const mockStreamEvent = { @@ -81,11 +80,9 @@ describe('testing put synonymGroup table-stream', () => { mockQuery.mockResolvedValue({ Items: mockItems }) ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) putData.body.eventIds = [eventId] - mockGet.mockResolvedValue(null) ;(search as unknown as jest.Mock).mockReturnValue({ index: mockIndex, delete: mockDelete, - get: mockGet, }) await handler(mockStreamEvent) @@ -106,13 +103,9 @@ describe('testing put synonymGroup table-stream', () => { mockQuery.mockResolvedValue({ Items: mockItems }) ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) putData.body.eventIds = [existingEventId, eventId] - mockGet.mockResolvedValue({ - body: { _source: { synonymId, eventIds: [existingEventId] } }, - }) ;(search as unknown as jest.Mock).mockReturnValue({ index: mockIndex, delete: mockDelete, - get: mockGet, }) await handler(mockStreamEvent) @@ -133,13 +126,9 @@ describe('testing put synonymGroup table-stream', () => { mockQuery.mockResolvedValue({ Items: mockItems }) ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) putData.body.eventIds = [existingEventId, eventId] - mockGet.mockResolvedValue({ - body: { _source: { synonymId, eventIds: [existingEventId, eventId] } }, - }) ;(search as unknown as jest.Mock).mockReturnValue({ index: mockIndex, delete: mockDelete, - get: mockGet, }) await handler(mockStreamEvent) @@ -160,13 +149,9 @@ describe('testing delete synonymGroup table-stream', () => { ;(tables as unknown as jest.Mock).mockResolvedValue(mockClient) mockStreamEvent.Records[0].eventName = 'REMOVE' putData.body.eventIds = [existingEventId] - mockGet.mockResolvedValue({ - body: { _source: { synonymId, eventIds: [existingEventId, eventId] } }, - }) ;(search as unknown as jest.Mock).mockReturnValue({ index: mockIndex, delete: mockDelete, - get: mockGet, }) await handler(mockStreamEvent) @@ -188,13 +173,9 @@ describe('testing delete synonymGroup table-stream', () => { index: 'synonym-groups', id: synonymId, } - mockGet.mockResolvedValue({ - body: { _source: { synonymId, eventIds: [eventId] } }, - }) ;(search as unknown as jest.Mock).mockReturnValue({ index: mockIndex, delete: mockDelete, - get: mockGet, }) await handler(mockStreamEvent) diff --git a/app/table-streams/synonyms/index.ts b/app/table-streams/synonyms/index.ts index 69639d92f..98b38f81e 100644 --- a/app/table-streams/synonyms/index.ts +++ b/app/table-streams/synonyms/index.ts @@ -39,19 +39,15 @@ async function putIndex(synonymGroup: SynonymGroup) { export const handler = createTriggerHandler( async ({ eventName, dynamodb }: DynamoDBRecord) => { if (!eventName || !dynamodb) return - const promises = [] - const id = unmarshallTrigger(dynamodb!.Keys).synonymId - const synonym = unmarshallTrigger(dynamodb!.NewImage) as Synonym - const dynamoSynonyms = await getSynonymsByUuid(synonym.synonymId) - const group = { - synonymId: synonym.synonymId, - eventIds: dynamoSynonyms.map((synonym) => synonym.eventId), + const { synonymId } = unmarshallTrigger(dynamodb!.NewImage) as Synonym + const dynamoSynonyms = await getSynonymsByUuid(synonymId) + if (dynamoSynonyms.length > 0) { + await putIndex({ + synonymId, + eventIds: dynamoSynonyms.map((synonym) => synonym.eventId), + }) + } else { + await removeIndex(synonymId) } - if (eventName === 'REMOVE' && group.eventIds.length === 0) { - promises.push(removeIndex(id)) - } /* (eventName === 'INSERT' || eventName === 'MODIFY') */ else { - promises.push(putIndex(group)) - } - await Promise.all(promises) } ) From bb5d779241846ada9013e6a792d58d538d3df9c0 Mon Sep 17 00:00:00 2001 From: Courey Elliott Date: Fri, 9 Aug 2024 10:42:00 -0400 Subject: [PATCH 3/4] using lodash instead of reducing --- sandbox-search.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sandbox-search.js b/sandbox-search.js index 84f3e1044..7e05d2b6f 100644 --- a/sandbox-search.js +++ b/sandbox-search.js @@ -6,16 +6,12 @@ * SPDX-License-Identifier: Apache-2.0 */ import { readFile } from 'fs/promises' +import _ from 'lodash' export default async function () { const text = await readFile('sandbox-seed.json', { encoding: 'utf-8' }) const { circulars, synonyms } = JSON.parse(text) - - const groupedSynonyms = synonyms.reduce((accumulator, synonym) => { - ;(accumulator[synonym.synonymId] ??= []).push(synonym.eventId) - return accumulator - }, {}) - + const groupedSynonyms = _.groupBy(synonyms, 'synonymId') const groups = Object.keys(groupedSynonyms).map((id) => ({ synonymId: id, eventIds: groupedSynonyms[id], From 7c23c671eb2135deee0770569d9177756914c454 Mon Sep 17 00:00:00 2001 From: Courey Elliott Date: Mon, 12 Aug 2024 09:18:23 -0400 Subject: [PATCH 4/4] nitpicks --- sandbox-search.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sandbox-search.js b/sandbox-search.js index 7e05d2b6f..02a89017a 100644 --- a/sandbox-search.js +++ b/sandbox-search.js @@ -6,17 +6,16 @@ * SPDX-License-Identifier: Apache-2.0 */ import { readFile } from 'fs/promises' -import _ from 'lodash' +import groupBy from 'lodash/groupBy.js' export default async function () { const text = await readFile('sandbox-seed.json', { encoding: 'utf-8' }) const { circulars, synonyms } = JSON.parse(text) - const groupedSynonyms = _.groupBy(synonyms, 'synonymId') - const groups = Object.keys(groupedSynonyms).map((id) => ({ - synonymId: id, - eventIds: groupedSynonyms[id], - })) - + const groups = Object.entries(groupBy(synonyms, 'synonymId')).flatMap( + ([synonymId, values]) => [ + { synonymId, eventIds: values.map(({ eventId }) => eventId) }, + ] + ) return [ ...circulars.flatMap((item) => [ { index: { _index: 'circulars', _id: item.circularId.toString() } },