From ea8a78cc2294cc6825c5fc1ec479a3f81689c182 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 9 Dec 2025 01:01:03 -0800
Subject: [PATCH] Fix stale-read race when creating push subscriptions

(Caught one such uniqueness exception in the wild)

* 200 OK -> 204 No Content, default status
* No need to touch the subscription when found
* Drop superflous test
---
 .../users/push_subscriptions_controller.rb    |  8 +-------
 .../push_subscriptions_controller_test.rb     | 20 +------------------
 2 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/app/controllers/users/push_subscriptions_controller.rb b/app/controllers/users/push_subscriptions_controller.rb
index bf5e5f87e..b511a6b19 100644
--- a/app/controllers/users/push_subscriptions_controller.rb
+++ b/app/controllers/users/push_subscriptions_controller.rb
@@ -5,13 +5,7 @@ class Users::PushSubscriptionsController < ApplicationController
   end
 
   def create
-    if subscription = @push_subscriptions.find_by(push_subscription_params)
-      subscription.touch
-    else
-      @push_subscriptions.create! push_subscription_params.merge(user_agent: request.user_agent)
-    end
-
-    head :ok
+    @push_subscriptions.create_with(user_agent: request.user_agent).create_or_find_by!(push_subscription_params)
   end
 
   def destroy
diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb
index c41d8313e..4b4dd0a09 100644
--- a/test/controllers/users/push_subscriptions_controller_test.rb
+++ b/test/controllers/users/push_subscriptions_controller_test.rb
@@ -14,30 +14,12 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
     post user_push_subscriptions_path(users(:david)),
       params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" }
 
-    assert_response :ok
+    assert_response :no_content
 
     assert_equal subscription_params, users(:david).push_subscriptions.last.attributes.slice("endpoint", "p256dh_key", "auth_key")
     assert_equal "Mozilla/5.0", users(:david).push_subscriptions.last.user_agent
   end
 
-  test "touch existing subscription" do
-    existing_subscription = users(:david).push_subscriptions.create!(
-      endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
-      p256dh_key: "123",
-      auth_key: "456"
-    )
-
-    assert_no_difference -> { users(:david).push_subscriptions.count } do
-      assert_changes -> { existing_subscription.reload.updated_at } do
-        post user_push_subscriptions_path(users(:david)), params: {
-          push_subscription: existing_subscription.attributes.slice("endpoint", "p256dh_key", "auth_key")
-        }
-      end
-    end
-
-    assert_response :ok
-  end
-
   test "destroy a push subscription" do
     subscription = users(:david).push_subscriptions.create!(
       endpoint: "https://fcm.googleapis.com/fcm/send/abc123",