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

Add redis lock to new ships #875

Merged
merged 3 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified bun.lockb
Binary file not shown.
40 changes: 40 additions & 0 deletions lib/redis-lock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use server'
import { kv } from '@vercel/kv'
import { v4 as uuidv4 } from 'uuid'

const LOCK_TIMEOUT = 30 * 1000 // 30 seconds

export async function aquireLock(key: string): Promise<string | null> {
const lockKey = `lock:${key}`
const lockValue = uuidv4()
const acquired = await kv.set(lockKey, lockValue, {
nx: true,
px: LOCK_TIMEOUT,
})
return acquired ? lockValue : null
}

export async function releaseLock(
lockKey: string,
lockValue: string,
): Promise<void> {
const currentLockValue = await kv.get(lockKey)
if (currentLockValue === lockValue) {
await kv.del(lockKey)
}
}

export async function withLock<T>(
key: string,
action: () => Promise<T>,
): Promise<T | null> {
const lockValue = await aquireLock(key)
if (!lockValue) {
throw new Error(`Failed to acquire lock for key: ${key}`)
}
try {
return await action()
} finally {
await releaseLock(`lock:${key}`, lockValue)
}
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
"sharp": "^0.33.5",
"shepherd.js": "^14.1.0",
"tailwind-merge": "^2.5.2",
"tailwindcss-animate": "^1.0.7"
"tailwindcss-animate": "^1.0.7",
"uuid": "^11.0.3"
},
"devDependencies": {
"@types/node": "^20",
Expand Down
2 changes: 1 addition & 1 deletion src/app/harbor/shipyard/new-ship-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default function NewShipForm({

const isTutorial = sessionStorage?.getItem('tutorial') === 'true'
if (!isTutorial) {
await createShip(formData)
await createShip(formData, false)
}
confettiRef.current?.addConfetti()
closeForm()
Expand Down
231 changes: 120 additions & 111 deletions src/app/harbor/shipyard/ship-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { fetchShips, person } from '@/app/utils/data'
import { getWakaSessions } from '@/app/utils/waka'
import type { Ship } from '@/app/utils/data'
import Airtable from 'airtable'
import { withLock } from '../../../../lib/redis-lock'

const peopleTableName = 'people'
const shipsTableName = 'ships'
Expand Down Expand Up @@ -41,42 +42,44 @@ export async function createShip(formData: FormData, isTutorial: boolean) {
}

const slackId = session.slackId
const entrantId = await getSelfPerson(slackId).then((p) => p.id)
return await withLock(`ship:${slackId}`, async () => {
const entrantId = await getSelfPerson(slackId).then((p) => p.id)

const isShipUpdate = formData.get('isShipUpdate')
const isShipUpdate = formData.get('isShipUpdate')

base()(shipsTableName).create(
[
{
// @ts-expect-error No overload matches this call - but it does
fields: {
title: formData.get('title'),
entrant: [entrantId],
repo_url: formData.get('repo_url'),
readme_url: formData.get('readme_url'),
deploy_url: formData.get('deployment_url'),
screenshot_url: formData.get('screenshot_url'),
ship_type: isShipUpdate ? 'update' : 'project',
update_description: isShipUpdate
? formData.get('updateDescription')
: null,
wakatime_project_name: formData.get('wakatime_project_name'),
project_source: isTutorial ? 'tutorial' : 'high_seas',
base()(shipsTableName).create(
[
{
// @ts-expect-error No overload matches this call - but it does
fields: {
title: formData.get('title'),
entrant: [entrantId],
repo_url: formData.get('repo_url'),
readme_url: formData.get('readme_url'),
deploy_url: formData.get('deployment_url'),
screenshot_url: formData.get('screenshot_url'),
ship_type: isShipUpdate ? 'update' : 'project',
update_description: isShipUpdate
? formData.get('updateDescription')
: null,
wakatime_project_name: formData.get('wakatime_project_name'),
project_source: isTutorial ? 'tutorial' : 'high_seas',
},
},
],
(err: Error, records: any) => {
if (err) console.error(err)
},
],
(err: Error, records: any) => {
if (err) console.error(err)
},
)
)
})
}

// @malted: I'm confident this is secure.
export async function createShipUpdate(
dangerousReshippedFromShipId: string,
credited_hours: number,
formData: FormData,
): Promise<Ship> {
): Promise<Ship | null> {
const session = await getSession()
if (!session) {
const error = new Error(
Expand All @@ -87,102 +90,108 @@ export async function createShipUpdate(
}

const slackId = session.slackId
const entrantId = await getSelfPerson(slackId).then((p) => p.id)

// This pattern makes sure the ship data is not fraudulent
const ships = await fetchShips(slackId)
return withLock(`update:${slackId}`, async () => {
const entrantId = await getSelfPerson(slackId).then((p) => p.id)

const reshippedFromShip = ships.find(
(ship: Ship) => ship.id === dangerousReshippedFromShipId,
)
if (!reshippedFromShip) {
const error = new Error('Invalid reshippedFromShipId!')
console.error(error)
throw error
}
// This pattern makes sure the ship data is not fraudulent
const ships = await fetchShips(slackId)

/* Two things are happening here.
* Firstly, the new ship of ship_type "update" needs to be created.
* - This will have all the same fields as the reshipped ship.
* - The update_descripton will be the new entered form field though.
* - The reshipped_from field should have the record ID of the reshipped ship
* Secondly, the reshipped_to field on the reshipped ship should be updated to be the new update ship's record ID.
*/
const reshippedFromShip = ships.find(
(ship: Ship) => ship.id === dangerousReshippedFromShipId,
)
if (!reshippedFromShip) {
const error = new Error('Invalid reshippedFromShipId!')
console.error(error)
throw error
}

/* Two things are happening here.
* Firstly, the new ship of ship_type "update" needs to be created.
* - This will have all the same fields as the reshipped ship.
* - The update_descripton will be the new entered form field though.
* - The reshipped_from field should have the record ID of the reshipped ship
* Secondly, the reshipped_to field on the reshipped ship should be updated to be the new update ship's record ID.
*/

// Step 1:
const res: { id: string; fields: any } = await new Promise(
(resolve, reject) => {
base()(shipsTableName).create(
[
{
// @ts-expect-error No overload matches this call - but it does
fields: {
...shipToFields(reshippedFromShip, entrantId),
ship_type: 'update',
update_description: formData.get('update_description'),
reshipped_from: [reshippedFromShip.id],
reshipped_from_all: reshippedFromShip.reshippedFromAll
? [...reshippedFromShip.reshippedFromAll, reshippedFromShip.id]
: [reshippedFromShip.id],
credited_hours,
// Step 1:
const res: { id: string; fields: any } = await new Promise(
(resolve, reject) => {
base()(shipsTableName).create(
[
{
// @ts-expect-error No overload matches this call - but it does
fields: {
...shipToFields(reshippedFromShip, entrantId),
ship_type: 'update',
update_description: formData.get('update_description'),
reshipped_from: [reshippedFromShip.id],
reshipped_from_all: reshippedFromShip.reshippedFromAll
? [
...reshippedFromShip.reshippedFromAll,
reshippedFromShip.id,
]
: [reshippedFromShip.id],
credited_hours,
},
},
},
],
(err: Error, records: any) => {
if (err) {
console.error('createShipUpdate step 1:', err)
throw err
}
if (records) {
// Step 2
if (records.length !== 1) {
const error = new Error(
'createShipUpdate: step 1 created records result length is not 1',
)
console.error(error)
reject(error)
],
(err: Error, records: any) => {
if (err) {
console.error('createShipUpdate step 1:', err)
throw err
}
const reshippedToShip = records[0]
if (records) {
// Step 2
if (records.length !== 1) {
const error = new Error(
'createShipUpdate: step 1 created records result length is not 1',
)
console.error(error)
reject(error)
}
const reshippedToShip = records[0]

// Update previous ship to point reshipped_to to the newly created update record
base()(shipsTableName).update([
{
id: reshippedFromShip.id,
fields: {
reshipped_to: [reshippedToShip.id],
reshipped_all: [reshippedToShip, reshippedFromShip],
// Update previous ship to point reshipped_to to the newly created update record
base()(shipsTableName).update([
{
id: reshippedFromShip.id,
fields: {
reshipped_to: [reshippedToShip.id],
reshipped_all: [reshippedToShip, reshippedFromShip],
},
},
},
])
])

resolve(reshippedToShip)
} else {
console.error('AAAFAUKCSCSAEVTNOESIFNVFEINTTET🤬🤬🤬')
reject(new Error('createShipUpdate: step 1 created no records'))
}
},
)
},
)
resolve(reshippedToShip)
} else {
console.error('AAAFAUKCSCSAEVTNOESIFNVFEINTTET🤬🤬🤬')
reject(new Error('createShipUpdate: step 1 created no records'))
}
},
)
},
)

return {
...reshippedFromShip,
id: res.id,
repoUrl: reshippedFromShip.repoUrl,
readmeUrl: reshippedFromShip.readmeUrl,
screenshotUrl: reshippedFromShip.screenshotUrl,
deploymentUrl: reshippedFromShip.deploymentUrl,
shipType: 'update',
shipStatus: 'staged',
updateDescription: formData.get('update_description')?.toString() || null,
reshippedFromId: reshippedFromShip.id,
reshippedFromAll: reshippedFromShip.reshippedFromAll
? [...reshippedFromShip.reshippedFromAll, reshippedFromShip.id]
: [reshippedFromShip.id],
credited_hours,
total_hours: (reshippedFromShip.total_hours ?? 0) + credited_hours,
wakatimeProjectNames: reshippedFromShip.wakatimeProjectNames,
}
return {
...reshippedFromShip,
id: res.id,
repoUrl: reshippedFromShip.repoUrl,
readmeUrl: reshippedFromShip.readmeUrl,
screenshotUrl: reshippedFromShip.screenshotUrl,
deploymentUrl: reshippedFromShip.deploymentUrl,
shipType: 'update',
shipStatus: 'staged',
updateDescription: formData.get('update_description')?.toString() || null,
reshippedFromId: reshippedFromShip.id,
reshippedFromAll: reshippedFromShip.reshippedFromAll
? [...reshippedFromShip.reshippedFromAll, reshippedFromShip.id]
: [reshippedFromShip.id],
credited_hours,
total_hours: (reshippedFromShip.total_hours ?? 0) + credited_hours,
wakatimeProjectNames: reshippedFromShip.wakatimeProjectNames,
}
})
}

export async function updateShip(ship: Ship) {
Expand Down