diff --git a/imports/client/components/PuzzleModalForm.tsx b/imports/client/components/PuzzleModalForm.tsx index 757ae9299..cceebc352 100644 --- a/imports/client/components/PuzzleModalForm.tsx +++ b/imports/client/components/PuzzleModalForm.tsx @@ -1,3 +1,4 @@ +import { Meteor } from "meteor/meteor"; import React, { Suspense, useCallback, @@ -9,6 +10,7 @@ import React, { } from "react"; import Alert from "react-bootstrap/Alert"; import Col from "react-bootstrap/Col"; +import FormCheck from "react-bootstrap/FormCheck"; import type { FormControlProps } from "react-bootstrap/FormControl"; import FormControl from "react-bootstrap/FormControl"; import FormGroup from "react-bootstrap/FormGroup"; @@ -38,6 +40,7 @@ export interface PuzzleModalFormSubmitPayload { tags: string[]; docType?: GdriveMimeTypesType; expectedAnswerCount: number; + allowDuplicateUrls?: boolean; } enum PuzzleModalFormSubmitState { @@ -93,6 +96,9 @@ const PuzzleModalForm = React.forwardRef( const [expectedAnswerCount, setExpectedAnswerCount] = useState( puzzle ? puzzle.expectedAnswerCount : 1, ); + const [allowDuplicateUrls, setAllowDuplicateUrls] = useState< + boolean | undefined + >(puzzle ? undefined : false); const [submitState, setSubmitState] = useState( PuzzleModalFormSubmitState.IDLE, ); @@ -157,6 +163,13 @@ const PuzzleModalForm = React.forwardRef( setExpectedAnswerCountDirty(true); }, []); + const onAllowDuplicateUrlsChange = useCallback( + (event: React.ChangeEvent) => { + setAllowDuplicateUrls(event.currentTarget.checked); + }, + [], + ); + const onFormSubmit = useCallback( (callback: () => void) => { setSubmitState(PuzzleModalFormSubmitState.SUBMITTING); @@ -170,9 +183,24 @@ const PuzzleModalForm = React.forwardRef( if (docType) { payload.docType = docType; } + if (allowDuplicateUrls) { + payload.allowDuplicateUrls = allowDuplicateUrls; + } onSubmit(payload, (error) => { if (error) { - setErrorMessage(error.message); + if ( + error instanceof Meteor.Error && + typeof error.error === "number" && + error.error === 409 + ) { + setErrorMessage( + "A puzzle already exists with this URL - did someone else already add this" + + ' puzzle? To force creation anyway, check the "Allow puzzles with identical' + + ' URLs" box above and try again.', + ); + } else { + setErrorMessage(error.message); + } setSubmitState(PuzzleModalFormSubmitState.FAILED); } else { setSubmitState(PuzzleModalFormSubmitState.IDLE); @@ -181,11 +209,21 @@ const PuzzleModalForm = React.forwardRef( setUrlDirty(false); setTagsDirty(false); setExpectedAnswerCountDirty(false); + setAllowDuplicateUrls(false); callback(); } }); }, - [onSubmit, huntId, title, url, tags, expectedAnswerCount, docType], + [ + onSubmit, + huntId, + title, + url, + tags, + expectedAnswerCount, + docType, + allowDuplicateUrls, + ], ); const show = useCallback(() => { @@ -278,6 +316,17 @@ const PuzzleModalForm = React.forwardRef( ) : null; + const allowDuplicateUrlsCheckbox = + !puzzle && typeof allowDuplicateUrls === "boolean" ? ( + + ) : null; + return ( + {allowDuplicateUrlsCheckbox} diff --git a/imports/lib/models/Model.ts b/imports/lib/models/Model.ts index 4b031791f..259bf5120 100644 --- a/imports/lib/models/Model.ts +++ b/imports/lib/models/Model.ts @@ -4,6 +4,7 @@ import type { IndexDirection, IndexSpecification, CreateIndexesOptions, + ClientSession, } from "mongodb"; import { z } from "zod"; import { IsInsert, IsUpdate, IsUpsert, stringId } from "./customTypes"; @@ -447,6 +448,7 @@ class Model< doc: z.input, options: { bypassSchema?: boolean | undefined; + session?: ClientSession | undefined; } = {}, ): Promise> { const { bypassSchema } = options; @@ -456,9 +458,10 @@ class Model< raw = { ...doc, _id: this.collection._makeNewID() }; } try { - await this.collection - .rawCollection() - .insertOne(raw, { bypassDocumentValidation: true }); + await this.collection.rawCollection().insertOne(raw, { + bypassDocumentValidation: true, + session: options.session, + }); return raw._id; } catch (e) { formatValidationError(e); diff --git a/imports/methods/createPuzzle.ts b/imports/methods/createPuzzle.ts index c3a8a2163..505d5b27b 100644 --- a/imports/methods/createPuzzle.ts +++ b/imports/methods/createPuzzle.ts @@ -9,6 +9,7 @@ export default new TypedMethod< tags: string[]; expectedAnswerCount: number; docType: GdriveMimeTypesType; + allowDuplicateUrls?: boolean; }, string >("Puzzles.methods.create"); diff --git a/imports/methods/updatePuzzle.ts b/imports/methods/updatePuzzle.ts index bab00affa..2144df81f 100644 --- a/imports/methods/updatePuzzle.ts +++ b/imports/methods/updatePuzzle.ts @@ -7,6 +7,7 @@ export default new TypedMethod< url?: string; tags: string[]; expectedAnswerCount: number; + allowDuplicateUrls?: boolean; }, void >("Puzzles.methods.update"); diff --git a/imports/server/gdrive.ts b/imports/server/gdrive.ts index 4c9b147db..70fdfee4e 100644 --- a/imports/server/gdrive.ts +++ b/imports/server/gdrive.ts @@ -86,6 +86,16 @@ async function createDocument( return fileId; } +async function deleteDocument(id: string) { + await checkClientOk(); + if (!GoogleClient.drive) + throw new Meteor.Error(500, "Google integration is disabled"); + + await GoogleClient.drive.files.delete({ + fileId: id, + }); +} + export async function moveDocument(id: string, newParentId: string) { await checkClientOk(); if (!GoogleClient.drive) @@ -301,3 +311,16 @@ export async function ensureDocument( return doc!; } + +export async function deleteUnusedDocument(puzzle: { _id: string }) { + const doc = await Documents.findOneAsync({ puzzle: puzzle._id }); + if (!doc) { + return; + } + + await checkClientOk(); + await withLock(`puzzle:${puzzle._id}:documents`, async () => { + await deleteDocument(doc.value.id); + await Documents.removeAsync(doc._id); + }); +} diff --git a/imports/server/methods/createPuzzle.ts b/imports/server/methods/createPuzzle.ts index 0fef6719b..b7d66bb54 100644 --- a/imports/server/methods/createPuzzle.ts +++ b/imports/server/methods/createPuzzle.ts @@ -1,5 +1,6 @@ import { check, Match } from "meteor/check"; import { Meteor } from "meteor/meteor"; +import { MongoInternals } from "meteor/mongo"; import { Random } from "meteor/random"; import Flags from "../../Flags"; import Logger from "../../Logger"; @@ -11,7 +12,7 @@ import Puzzles from "../../lib/models/Puzzles"; import { userMayWritePuzzlesForHunt } from "../../lib/permission_stubs"; import createPuzzle from "../../methods/createPuzzle"; import GlobalHooks from "../GlobalHooks"; -import { ensureDocument } from "../gdrive"; +import { deleteUnusedDocument, ensureDocument } from "../gdrive"; import getOrCreateTagByName from "../getOrCreateTagByName"; import GoogleClient from "../googleClientRefresher"; import defineMethod from "./defineMethod"; @@ -27,11 +28,20 @@ defineMethod(createPuzzle, { docType: Match.OneOf( ...(Object.keys(GdriveMimeTypes) as GdriveMimeTypesType[]), ), + allowDuplicateUrls: Match.Optional(Boolean), }); return arg; }, - async run({ huntId, title, tags, expectedAnswerCount, docType, url }) { + async run({ + huntId, + title, + tags, + expectedAnswerCount, + docType, + url, + allowDuplicateUrls, + }) { check(this.userId, String); const hunt = await Hunts.findOneAsync(huntId); @@ -81,7 +91,39 @@ defineMethod(createPuzzle, { await ensureDocument(fullPuzzle, docType); } - await Puzzles.insertAsync(fullPuzzle); + // In a transaction, look for a puzzle with the same URL. If present, we + // reject the insertion unless the client overrides it. + const client = MongoInternals.defaultRemoteCollectionDriver().mongo.client; + const session = client.startSession(); + try { + await session.withTransaction(async () => { + if (url) { + const existingPuzzleWithUrl = await Puzzles.collection + .rawCollection() + .findOne({ hunt: huntId, url }, { session }); + if (existingPuzzleWithUrl && !allowDuplicateUrls) { + throw new Meteor.Error( + 409, + `Puzzle with URL ${url} already exists`, + ); + } + } + await Puzzles.insertAsync(fullPuzzle, { session }); + }); + } catch (error) { + // In the case of any error, try to delete the document we created before the transaction. + // If that fails too, let the original error propagate. + try { + await deleteUnusedDocument(fullPuzzle); + } catch (deleteError) { + Logger.warn("Unable to clean up document on failed puzzle creation", { + error: deleteError, + }); + } + throw error; + } finally { + await session.endSession(); + } // Run any puzzle-creation hooks, like creating a default document // attachment or announcing the puzzle to Slack. diff --git a/imports/server/methods/updatePuzzle.ts b/imports/server/methods/updatePuzzle.ts index 1927b5ec4..8021f4f83 100644 --- a/imports/server/methods/updatePuzzle.ts +++ b/imports/server/methods/updatePuzzle.ts @@ -22,6 +22,9 @@ defineMethod(updatePuzzle, { url: Match.Optional(String), tags: [String], expectedAnswerCount: Number, + // We accept this argument since it's provided by the form, but it's not checked here - only + // during puzzle creation, to avoid duplicates when creating new puzzles. + allowDuplicateUrls: Match.Optional(Boolean), }); return arg;