Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Courey/update table stream #2781

Merged
merged 12 commits into from
Jan 17, 2025
Prev Previous commit
Next Next commit
refactoring tests so it's more explicit
Courey committed Dec 13, 2024
commit 539c245591d9cf4adabbc56823bad262c8440a34
25 changes: 10 additions & 15 deletions __tests__/table-streams/synonyms.ts
Original file line number Diff line number Diff line change
@@ -29,10 +29,6 @@ const putData = {
},
}

const mockPreviousItems = [
{ synonymId: previousSynonymId, eventId, slug: eventSlug },
]

const deleteData = {
index: 'synonym-groups',
id: previousSynonymId,
@@ -109,12 +105,14 @@ describe('testing synonymGroup table-stream', () => {
test('insert initial synonym record where no previous opensearch record exists', async () => {
putData.body.eventIds = [eventId]
putData.body.slugs.push(eventSlug)
const mockItems = [{ synonymId, eventId, slug: eventSlug }]

const mockSearch = jest
.fn()
.mockReturnValue({ body: { hits: { hits: [] } } })

mockQuery.mockResolvedValue({ Items: mockItems })
mockQuery.mockResolvedValue({
Items: [{ synonymId, eventId, slug: eventSlug }],
})

const mockClient = {
synonyms: {
@@ -132,16 +130,19 @@ describe('testing synonymGroup table-stream', () => {
await handler(mockStreamEvent)

expect(mockIndex).toHaveBeenCalledWith(putData)
expect(mockIndex).toHaveBeenCalledTimes(1)
expect(mockDelete).not.toHaveBeenCalled()
})

test('insert into existing synonym group with removal of previous now unused group', async () => {
putData.body.eventIds = [existingEventId, eventId]
putData.body.slugs = [existingEventSlug, eventSlug]

const mockItems = [
{ synonymId, eventId: existingEventId, slug: existingEventSlug },
{ synonymId, eventId, slug: eventSlug },
]

const mockSearch = jest.fn().mockReturnValue({
body: {
hits: {
@@ -160,16 +161,18 @@ describe('testing synonymGroup table-stream', () => {

const implementedMockQuery = mockQuery.mockImplementation((query) => {
if (query.ExpressionAttributeValues[':synonymId'] == previousSynonymId) {
return { Items: mockPreviousItems }
return { Items: [] }
} else {
return { Items: mockItems }
}
})

;(search as unknown as jest.Mock).mockReturnValue({
index: mockIndex,
delete: mockDelete,
search: mockSearch,
})

const mockClient = {
synonyms: {
query: implementedMockQuery,
@@ -178,14 +181,6 @@ describe('testing synonymGroup table-stream', () => {

;(tables as unknown as jest.Mock).mockResolvedValue(mockClient)

mockQuery.mockImplementation((query) => {
if (query.ExpressionAttributeValues[':synonymId'] == previousSynonymId) {
return { Items: mockPreviousItems }
} else {
return { Items: mockItems }
}
})

await handler(mockStreamEvent)

// the new group opensearch record is updated
4 changes: 2 additions & 2 deletions app/table-streams/synonyms/index.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be much simpler, with fewer if statements. You know that if a synonym ID is present in either the old image or the new image, then you can update its OpenSearch items correctly with the following code:

          const synonyms = await getSynonymsByUuid(synonymId)
          if (synonyms.length > 0) {
            await putIndex({
              synonymId,
              eventIds: synonyms.map((synonym) => synonym.eventId),
              slugs: synonyms.map((synonym) => synonym.slug),
            })
          } else {
            await removeIndex(synonymId)
          }

Then all you need to do is execute the code above in a loop over the old image and the new image:

export const handler = createTriggerHandler(
  async ({ eventName, dynamodb }: DynamoDBRecord) => {
    invariant(eventName)
    invariant(dynamodb)
    await Promise.all(
      [dynamodb.OldImage, dynamodb.NewImage]
        .filter((image) => image !== undefined)
        .map(async (image) => {
          const { synonymId } = unmarshallTrigger(image) as Synonym
          const synonyms = await getSynonymsByUuid(synonymId)
          if (synonyms.length > 0) {
            await putIndex({
              synonymId,
              eventIds: synonyms.map((synonym) => synonym.eventId),
              slugs: synonyms.map((synonym) => synonym.slug),
            })
          } else {
            await removeIndex(synonymId)
          }
        })
    )
  }
)

Original file line number Diff line number Diff line change
@@ -55,17 +55,17 @@
: null
const dynamoPreviousGroup = previousSynonymId
? (await getSynonymsByUuid(previousSynonymId)).filter(
(synonym) => synonym.eventId != eventId

Check warning on line 58 in app/table-streams/synonyms/index.ts

Codecov / codecov/patch

app/table-streams/synonyms/index.ts#L58

Added line #L58 was not covered by tests
)
: []

if (dynamoPreviousGroup.length === 0 && previousSynonymId) {
if (previousSynonymId && dynamoPreviousGroup.length === 0) {
await removeIndex(previousSynonymId)
} else {
} else if (previousSynonymId && dynamoPreviousGroup.length > 0) {
await putIndex({

Check warning on line 65 in app/table-streams/synonyms/index.ts

Codecov / codecov/patch

app/table-streams/synonyms/index.ts#L65

Added line #L65 was not covered by tests
synonymId: previousSynonymId,
eventIds: dynamoPreviousGroup.map((synonym) => synonym.eventId),
slugs: dynamoPreviousGroup.map((synonym) => synonym.slug),

Check warning on line 68 in app/table-streams/synonyms/index.ts

Codecov / codecov/patch

app/table-streams/synonyms/index.ts#L67-L68

Added lines #L67 - L68 were not covered by tests
})
}