Skip to content

Commit

Permalink
using conditional check with silent failure on initial synonym creation
Browse files Browse the repository at this point in the history
  • Loading branch information
Courey committed Nov 14, 2024
1 parent ed9f3a2 commit 981fb22
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 39 deletions.
15 changes: 9 additions & 6 deletions __tests__/synonyms.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import * as awsSDKMock from 'aws-sdk-mock'
import crypto from 'crypto'

import type { Circular } from '~/routes/circulars/circulars.lib'
import { createSynonyms, putSynonyms } from '~/routes/synonyms/synonyms.server'
import {
moderatorCreateSynonyms,
putSynonyms,
} from '~/routes/synonyms/synonyms.server'

jest.mock('@architect/functions')
const synonymId = 'abcde-abcde-abcde-abcde-abcde'
Expand Down Expand Up @@ -36,7 +39,7 @@ const exampleCirculars = [
{ Items: [] },
]

describe('createSynonyms', () => {
describe('moderatorCreateSynonyms', () => {
beforeEach(() => {
const mockBatchWrite = jest.fn()
const mockQuery = jest.fn()
Expand Down Expand Up @@ -65,7 +68,7 @@ describe('createSynonyms', () => {
jest.restoreAllMocks()
})

test('createSynonyms should write to DynamoDB', async () => {
test('moderatorCreateSynonyms should write to DynamoDB', async () => {
const mockBatchWriteItem = jest.fn(
(
params: DynamoDB.DocumentClient.BatchWriteItemInput,
Expand All @@ -81,12 +84,12 @@ describe('createSynonyms', () => {
awsSDKMock.mock('DynamoDB', 'batchWriteItem', mockBatchWriteItem)

const synonymousEventIds = ['eventId1', 'eventId2']
const result = await createSynonyms(synonymousEventIds)
const result = await moderatorCreateSynonyms(synonymousEventIds)

expect(result).toBe(synonymId)
})

test('createSynonyms with nonexistent eventId throws Response 400', async () => {
test('moderatorCreateSynonyms with nonexistent eventId throws Response 400', async () => {
const mockBatchWriteItem = jest.fn(
(
params: DynamoDB.DocumentClient.BatchWriteItemInput,
Expand All @@ -103,7 +106,7 @@ describe('createSynonyms', () => {

const synonymousEventIds = ['eventId1', 'nope']
try {
await createSynonyms(synonymousEventIds)
await moderatorCreateSynonyms(synonymousEventIds)
} catch (error) {
// eslint-disable-next-line jest/no-conditional-expect
expect(error).toBeInstanceOf(Response)
Expand Down
8 changes: 2 additions & 6 deletions app/email-incoming/circulars/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ import {
import { sendEmail } from '~/lib/email.server'
import { hostname, origin } from '~/lib/env.server'
import { putRaw, submitterGroup } from '~/routes/circulars/circulars.server'
import {
createSynonyms,
synonymExists,
} from '~/routes/synonyms/synonyms.server'
import { tryInitSynonym } from '~/routes/synonyms/synonyms.server'

interface UserData {
email: string
Expand Down Expand Up @@ -103,8 +100,7 @@ export const handler = createEmailIncomingMessageHandler(
// Removes sub as a property if it is undefined from the legacy users
if (!circular.sub) delete circular.sub
const { circularId } = await putRaw(circular)
if (eventId && !(await synonymExists({ eventId })))
await createSynonyms([eventId])
if (eventId) await tryInitSynonym(eventId)

// Send a success email
await sendSuccessEmail({
Expand Down
5 changes: 2 additions & 3 deletions app/routes/circulars/circulars.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import memoizee from 'memoizee'
import { dedent } from 'ts-dedent'

import { type User, getUser } from '../_auth/user.server'
import { createSynonyms, synonymExists } from '../synonyms/synonyms.server'
import { tryInitSynonym } from '../synonyms/synonyms.server'
import {
bodyIsValid,
formatAuthor,
Expand Down Expand Up @@ -313,8 +313,7 @@ export async function put(
const eventId = parseEventFromSubject(item.subject)
if (eventId) circular.eventId = eventId
const result = await putRaw(circular)
if (eventId && !(await synonymExists({ eventId })))
await createSynonyms([eventId])
if (eventId) await tryInitSynonym(eventId)
return result
}

Expand Down
4 changes: 2 additions & 2 deletions app/routes/synonyms.new.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { getUser } from './_auth/user.server'
import { moderatorGroup } from './circulars/circulars.server'
import {
autoCompleteEventIds,
createSynonyms,
moderatorCreateSynonyms,
} from './synonyms/synonyms.server'
import DetailsDropdownContent from '~/components/DetailsDropdownContent'
import { getFormDataString } from '~/lib/utils'
Expand All @@ -40,7 +40,7 @@ export async function action({ request }: ActionFunctionArgs) {
const data = await request.formData()
const eventIds = getFormDataString(data, 'synonyms')?.split(',')
if (!eventIds) throw new Response(null, { status: 400 })
const synonymId = await createSynonyms(eventIds)
const synonymId = await moderatorCreateSynonyms(eventIds)
return redirect(`/synonyms/${synonymId}`)
}

Expand Down
52 changes: 30 additions & 22 deletions app/routes/synonyms/synonyms.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { tables } from '@architect/functions'
import type { DynamoDBDocument } from '@aws-sdk/lib-dynamodb'
import { PutItemCommand } from '@aws-sdk/client-dynamodb'
import { type DynamoDBDocument } from '@aws-sdk/lib-dynamodb'
import { search as getSearchClient } from '@nasa-gcn/architect-functions-search'
import crypto from 'crypto'

Expand Down Expand Up @@ -122,20 +123,6 @@ async function getSynonymMembers(eventId: string) {
return Items as Circular[]
}

export async function synonymExists({ eventId }: { eventId: string }) {
const db = await tables()
const { Items } = await db.synonyms.query({
KeyConditionExpression: '#eventId = :eventId',
ExpressionAttributeNames: {
'#eventId': 'eventId',
},
ExpressionAttributeValues: {
':eventId': eventId,
},
})
return Items.length >= 1
}

/*
* If an eventId already has a synonym and is passed in, it will unlink the
* eventId from the old synonym and the only remaining link will be to the
Expand All @@ -144,17 +131,17 @@ export async function synonymExists({ eventId }: { eventId: string }) {
* BatchWriteItem has a limit of 25 items, so the user may not add more than
* 25 synonyms at a time.
*/
export async function createSynonyms(synonymousEventIds: string[]) {
const uuid = crypto.randomUUID()
export async function moderatorCreateSynonyms(synonymousEventIds: string[]) {
if (!synonymousEventIds.length) {
throw new Response('EventIds are required.', { status: 400 })
}
const uuid = crypto.randomUUID()
const db = await tables()
const client = db._doc as unknown as DynamoDBDocument
const TableName = db.name('synonyms')

const isValid = await validateEventIds({ eventIds: synonymousEventIds })
if (!isValid) throw new Response('eventId does not exist', { status: 400 })

await client.batchWrite({
RequestItems: {
[TableName]: synonymousEventIds.map((eventId) => ({
Expand All @@ -167,6 +154,25 @@ export async function createSynonyms(synonymousEventIds: string[]) {
return uuid
}

export async function tryInitSynonym(eventId: string) {
const db = await tables()
const client = db._doc as unknown as DynamoDBDocument
const TableName = db.name('synonyms')
try {
const params = {
TableName,
Item: {
synonymId: { S: crypto.randomUUID() },
eventId: { S: eventId },
},
ConditionExpression: 'attribute_not_exists(eventId)',
}
await client.send(new PutItemCommand(params))
} catch (error) {
if ((error as Error).name !== 'ConditionalCheckFailedException') throw error
}
}

/*
* If an eventId already has a synonym and is passed in, it will unlink the
* eventId from the old synonym and the only remaining link will be to the
Expand Down Expand Up @@ -195,10 +201,11 @@ export async function putSynonyms({
const writes = []
if (subtractions?.length) {
const subtraction_writes = subtractions.map((eventId) => ({
DeleteRequest: {
Key: { eventId },
PutRequest: {
Item: { synonymId: crypto.randomUUID(), eventId },
},
}))

writes.push(...subtraction_writes)
}
if (additions?.length) {
Expand Down Expand Up @@ -233,9 +240,10 @@ export async function deleteSynonyms(synonymId: string) {
':synonymId': synonymId,
},
})

const writes = results.Items.map(({ eventId }) => ({
DeleteRequest: {
Key: { eventId },
PutRequest: {
Item: { synonymId: crypto.randomUUID(), eventId },
},
}))
const params = {
Expand Down

0 comments on commit 981fb22

Please sign in to comment.