fix: Race condition protection

Author: LautaroPetaccio

src/entities/CheckScenes/task/handleWorldScenesUndeployment.ts

@@ -41,7 +41,11 @@ export async function handleWorldScenesUndeployment(
       )}`
     )
 
-    await PlaceModel.deleteByWorldIdAndPositions(worldName, basePositions)
+    await PlaceModel.deleteByWorldIdAndPositions(
+      worldName,
+      basePositions,
+      event.timestamp
+    )
 
     loggerExtended.log(
       `Deleted place records for world: ${worldName} at positions: ${basePositions.join(

src/entities/CheckScenes/task/handleWorldUndeployment.ts

@@ -1,8 +1,8 @@
 import { WorldUndeploymentEvent } from "@dcl/schemas/dist/platform/events/world"
 import logger from "decentraland-gatsby/dist/entities/Development/logger"
 
-import { notifyError } from "../../Slack/utils"
 import PlaceModel from "../../Place/model"
+import { notifyError } from "../../Slack/utils"
 
 /**
  * Handles WorldUndeploymentEvent from the worlds content server.
@@ -28,7 +28,7 @@ export async function handleWorldUndeployment(
   try {
     loggerExtended.log(`Processing world undeployment for world: ${worldName}`)
 
-    await PlaceModel.deleteByWorldId(worldName)
+    await PlaceModel.deleteByWorldId(worldName, event.timestamp)
 
     loggerExtended.log(`Deleted all place records for world: ${worldName}`)
   } catch (error: any) {

src/entities/Place/model.ts

@@ -470,29 +470,43 @@ export default class PlaceModel extends Model<PlaceAttributes> {
   }
 
   /**
-   * Delete all place records associated with a world
+   * Delete all place records associated with a world that were deployed
+   * before the given event timestamp. This prevents stale undeployment
+   * events from removing places that were re-deployed after the event
+   * was emitted.
    */
-  static async deleteByWorldId(worldId: string): Promise<void> {
+  static async deleteByWorldId(
+    worldId: string,
+    eventTimestamp: number
+  ): Promise<void> {
     const normalizedWorldId = worldId.toLowerCase()
+    const eventDate = new Date(eventTimestamp)
     const sql = SQL`
       DELETE FROM ${table(this)}
       WHERE "world_id" = ${normalizedWorldId}
+        AND "deployed_at" < ${eventDate}
     `
     await this.namedQuery("delete_by_world_id", sql)
   }
 
   /**
    * Delete place records matching a world and specific base positions
+   * that were deployed before the given event timestamp. This prevents
+   * stale undeployment events from removing places that were re-deployed
+   * after the event was emitted.
    */
   static async deleteByWorldIdAndPositions(
     worldId: string,
-    basePositions: string[]
+    basePositions: string[],
+    eventTimestamp: number
   ): Promise<void> {
     const normalizedWorldId = worldId.toLowerCase()
+    const eventDate = new Date(eventTimestamp)
     const sql = SQL`
       DELETE FROM ${table(this)}
       WHERE "world_id" = ${normalizedWorldId}
         AND "base_position" = ANY(${basePositions})
+        AND "deployed_at" < ${eventDate}
     `
     await this.namedQuery("delete_by_world_id_and_positions", sql)
   }

test/fixtures/undeploymentEvent.ts

@@ -8,13 +8,14 @@ import {
  * Creates a WorldUndeploymentEvent for a full world undeployment.
  */
 export function createWorldUndeploymentEvent(
-  worldName: string
+  worldName: string,
+  options: { timestamp?: number } = {}
 ): WorldUndeploymentEvent {
   return {
     type: Events.Type.WORLD,
     subType: Events.SubType.Worlds.WORLD_UNDEPLOYMENT,
     key: worldName,
-    timestamp: Date.now(),
+    timestamp: options.timestamp ?? Date.now(),
     metadata: {
       worldName,
     },
@@ -26,13 +27,14 @@ export function createWorldUndeploymentEvent(
  */
 export function createWorldScenesUndeploymentEvent(
   worldName: string,
-  scenes: Array<{ entityId: string; baseParcel: string }>
+  scenes: Array<{ entityId: string; baseParcel: string }>,
+  options: { timestamp?: number } = {}
 ): WorldScenesUndeploymentEvent {
   return {
     type: Events.Type.WORLD,
     subType: Events.SubType.Worlds.WORLD_SCENES_UNDEPLOYMENT,
     key: worldName,
-    timestamp: Date.now(),
+    timestamp: options.timestamp ?? Date.now(),
     metadata: {
       worldName,
       scenes,

test/integration/handleWorldScenesUndeployment.test.ts

@@ -270,6 +270,38 @@ describe("when handling the WorldScenesUndeploymentEvent", () => {
     })
   })
 
+  describe("and the WorldScenesUndeploymentEvent has a timestamp older than the deployment", () => {
+    const worldName = "racecondition.dcl.eth"
+
+    beforeEach(async () => {
+      await deployWorldScene({
+        worldName,
+        title: "Re-deployed Scene",
+        base: "10,10",
+        parcels: ["10,10"],
+      })
+    })
+
+    it("should not delete the place record", async () => {
+      // Simulate a stale undeployment event with a timestamp before the deployment
+      const staleTimestamp = Date.now() - 60_000
+      const event = createWorldScenesUndeploymentEvent(
+        worldName,
+        [{ entityId: "entity-stale", baseParcel: "10,10" }],
+        { timestamp: staleTimestamp }
+      )
+      await handleWorldScenesUndeployment(event)
+
+      const response = await supertest(app)
+        .get("/api/places")
+        .query({ names: worldName })
+        .expect(200)
+
+      expect(response.body.data).toHaveLength(1)
+      expect(response.body.data[0].title).toBe("Re-deployed Scene")
+    })
+  })
+
   describe("and the WorldScenesUndeploymentEvent has an empty scenes array", () => {
     it("should return early without errors", async () => {
       const event = createWorldScenesUndeploymentEvent("anyworld.dcl.eth", [])

test/integration/handleWorldUndeployment.test.ts

@@ -167,6 +167,31 @@ describe("when handling the WorldUndeploymentEvent", () => {
     })
   })
 
+  describe("and the WorldUndeploymentEvent has a timestamp older than the deployment", () => {
+    const worldName = "racecondition.dcl.eth"
+
+    beforeEach(async () => {
+      await deployWorldScene({ worldName, title: "Re-deployed Scene" })
+    })
+
+    it("should not delete the place record", async () => {
+      // Simulate a stale undeployment event with a timestamp before the deployment
+      const staleTimestamp = Date.now() - 60_000
+      const event = createWorldUndeploymentEvent(worldName, {
+        timestamp: staleTimestamp,
+      })
+      await handleWorldUndeployment(event)
+
+      const response = await supertest(app)
+        .get("/api/places")
+        .query({ names: worldName })
+        .expect(200)
+
+      expect(response.body.data).toHaveLength(1)
+      expect(response.body.data[0].title).toBe("Re-deployed Scene")
+    })
+  })
+
   describe("and the WorldUndeploymentEvent has an empty world name", () => {
     it("should return early without errors", async () => {
       const event = createWorldUndeploymentEvent("")