FIX: Gap Of Uploading The Same File Key Twice

CLAUDE.md

@@ -63,8 +63,8 @@ Black-box BDD covering the video upload feature: its own Go module importing
 nothing from `cms/`, talking to a running service over HTTP only, so the same
 scenarios run against compose and against a deployed environment. Skips (does
 not fail) when nothing is serving. Uploading is a plain `PUT` to the presigned
-URL, no AWS SDK. Scenarios tagged `@known-gap` deliberately pin current
-behaviour that differs from the documented contract — see `tests/README.md`.
+URL, no AWS SDK. Scenarios can be tagged `@known-gap` to pin current behaviour
+that differs from the documented contract — see `tests/README.md`.
 
 Note compose starts `cms` in server mode only: the migrate container exists in
 the ECS task definition, not in `docker-compose.yml`, so a freshly created

cms/docs/docs.go

@@ -84,6 +84,12 @@ const docTemplate = `{
                             "$ref": "#/definitions/api.ProblemDetails"
                         }
                     },
+                    "409": {
+                        "description": "key has already been registered by an earlier request",
+                        "schema": {
+                            "$ref": "#/definitions/api.ProblemDetails"
+                        }
+                    },
                     "422": {
                         "description": "title was empty, key was missing or not a key issued by this API, or categoryIds was empty",
                         "schema": {
@@ -91,7 +97,7 @@ const docTemplate = `{
                         }
                     },
                     "500": {
-                        "description": "Categories could not be read, the transcoding job could not be queued, or the video record could not be saved",
+                        "description": "Categories could not be read, the key could not be checked, the transcoding job could not be queued, or the video record could not be saved",
                         "schema": {
                             "$ref": "#/definitions/api.ProblemDetails"
                         }

cms/docs/swagger.json

@@ -77,6 +77,12 @@
                             "$ref": "#/definitions/api.ProblemDetails"
                         }
                     },
+                    "409": {
+                        "description": "key has already been registered by an earlier request",
+                        "schema": {
+                            "$ref": "#/definitions/api.ProblemDetails"
+                        }
+                    },
                     "422": {
                         "description": "title was empty, key was missing or not a key issued by this API, or categoryIds was empty",
                         "schema": {
@@ -84,7 +90,7 @@
                         }
                     },
                     "500": {
-                        "description": "Categories could not be read, the transcoding job could not be queued, or the video record could not be saved",
+                        "description": "Categories could not be read, the key could not be checked, the transcoding job could not be queued, or the video record could not be saved",
                         "schema": {
                             "$ref": "#/definitions/api.ProblemDetails"
                         }

cms/docs/swagger.yaml

@@ -190,14 +190,19 @@ paths:
           description: categoryIds referenced a category that does not exist
           schema:
             $ref: '#/definitions/api.ProblemDetails'
+        "409":
+          description: key has already been registered by an earlier request
+          schema:
+            $ref: '#/definitions/api.ProblemDetails'
         "422":
           description: title was empty, key was missing or not a key issued by this
             API, or categoryIds was empty
           schema:
             $ref: '#/definitions/api.ProblemDetails'
         "500":
-          description: Categories could not be read, the transcoding job could not
-            be queued, or the video record could not be saved
+          description: Categories could not be read, the key could not be checked,
+            the transcoding job could not be queued, or the video record could not
+            be saved
           schema:
             $ref: '#/definitions/api.ProblemDetails'
       summary: Register an uploaded video

cms/internal/db/repositories/videos.go

@@ -14,6 +14,19 @@ type VideoRepository struct {
 
 var VideoRepo VideoRepository
 
+// VideoExistsWithStorageKey reports whether a video has already been
+// registered under a storage key. It is served by videos_storage_key_key, the
+// index Postgres builds for the column's UNIQUE constraint.
+func (svc VideoRepository) VideoExistsWithStorageKey(ctx context.Context, storageKey string) (bool, error) {
+	var exists bool
+	err := svc.SQLDB.QueryRowContext(ctx,
+		`SELECT EXISTS (SELECT 1 FROM videos WHERE storage_key = $1)`, storageKey).Scan(&exists)
+	if err != nil {
+		return false, err
+	}
+	return exists, nil
+}
+
 func (svc VideoRepository) CreateVideo(ctx context.Context, v models.Video) (models.Video, error) {
 	tx, err := svc.SQLDB.BeginTx(ctx, nil)
 	if err != nil {

cms/internal/handlers/videos.go

@@ -113,8 +113,9 @@ func PresignVideoUpload(w http.ResponseWriter, r *http.Request) {
 //	@Success		201		{object}	api.CompleteResponse
 //	@Failure		400		{object}	api.ProblemDetails	"Request body was not valid JSON"
 //	@Failure		404		{object}	api.ProblemDetails	"categoryIds referenced a category that does not exist"
+//	@Failure		409		{object}	api.ProblemDetails	"key has already been registered by an earlier request"
 //	@Failure		422		{object}	api.ProblemDetails	"title was empty, key was missing or not a key issued by this API, or categoryIds was empty"
-//	@Failure		500		{object}	api.ProblemDetails	"Categories could not be read, the transcoding job could not be queued, or the video record could not be saved"
+//	@Failure		500		{object}	api.ProblemDetails	"Categories could not be read, the key could not be checked, the transcoding job could not be queued, or the video record could not be saved"
 //	@Router			/api/videos [post]
 func CompleteVideoUpload(w http.ResponseWriter, r *http.Request) {
 	r.Body = http.MaxBytesReader(w, r.Body, maxJSONBodySize)
@@ -166,6 +167,24 @@ func CompleteVideoUpload(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Ahead of queueing, because a key that is already registered would
+	// otherwise get a second transcoding job before the insert failed on the
+	// storage_key UNIQUE constraint. That constraint is still the backstop:
+	// this check and the insert are not one atomic step, so two concurrent
+	// requests carrying the same key can both get past here.
+	alreadyRegistered, err := repositories.VideoRepo.VideoExistsWithStorageKey(r.Context(), key)
+	if err != nil {
+		log.Printf("Something Went Wrong Checking Whether The Video Was Already Registered: %s", err)
+		writeProblem(w, http.StatusInternalServerError, "Something Went Wrong Checking Whether The Video Was Already Registered",
+			"The submitted key could not be checked against the existing video records, so no video record was created and no transcoding job was queued. This is a server-side fault; the uploaded file is still in storage, so retry this request with the same 'key'.")
+		return
+	}
+	if alreadyRegistered {
+		writeProblem(w, http.StatusConflict, "Video Already Registered",
+			fmt.Sprintf("The 'key' %q has already been registered, so a video record and a transcoding job exist for that file. Registering it a second time would queue a duplicate job. If you meant to add a different video, request a fresh key from POST /api/videos/presign and upload the file to it.", key))
+		return
+	}
+
 	jobID, err := services.MediaConvertClient.QueueEncodingJob(r.Context(), key)
 	if err != nil {
 		log.Printf("Something Went Wrong On Creation Of Transcoding Job: %s", err)

tests/README.md

@@ -66,20 +66,8 @@ Both steps of the upload flow, end to end:
   category, and a storage key this API never issued
 - answering 404 for a category id no category has, kept distinct from the 500
   the category lookup itself failing produces
-
-## `@known-gap`
-
-One scenario is tagged `@known-gap`. It passes — it pins down what the service
-does **today**, where that differs from what it is documented to do:
-
-- **registering the same upload twice answers 500.** `videos.storage_key` is
-  `UNIQUE`, so the retry the API's own error message invites ("retry this
-  request with the same 'key'") fails the insert — after a second MediaConvert
-  job has already been queued for the same file.
-
-Fixing it means changing the scenario alongside the handler. It is tagged so it
-is easy to find, and so it can be excluded with
-`gobdd.WithIgnoredTags("@known-gap")` if that is ever wanted.
+- answering 409 for a key that was already registered, rather than queueing a
+  second transcoding job for the same file
 
 ## Adding a scenario
 

tests/features/video_upload.feature

@@ -97,8 +97,6 @@ Feature: Video upload
       | ../videos/escape.mp4            |
       | videos-of-someone-else/clip.mp4 |
 
-  # An id no category has is the client's mistake, and is answered separately
-  # from the category lookup failing, which is the server's and stays a 500.
   Scenario: Registering a video under a category id that does not exist
     Given I have requested an upload slot for "mystery.mp4" of type "video/mp4"
     And I have uploaded the file to the upload URL
@@ -106,14 +104,12 @@ Feature: Video upload
     Then the request is rejected with status 404
     And the problem title is "Catagory Does Not Exist"
 
-  # Registration is not idempotent: videos.storage_key is UNIQUE, so a retry
-  # with a key that was already registered fails the insert — after a second
-  # MediaConvert job has already been queued for the same file.
-  @known-gap
+  # A key is registered once. The second attempt is turned away before the
+  # transcoding job is queued, so it cannot leave a duplicate job behind.
   Scenario: Registering the same upload twice
     Given I have requested an upload slot for "twice.mp4" of type "video/mp4"
     And I have uploaded the file to the upload URL
     And I have registered the uploaded video titled "Filed Once" under categories "other"
     When I register the uploaded video titled "Filed Twice" under categories "other"
-    Then the request is rejected with status 500
-    And the problem title is "Something Went Wrong Saving The Video Record"
+    Then the request is rejected with status 409
+    And the problem title is "Video Already Registered"