From e72676e83a3c638ddacbb89cfe15fe9b1b83b92f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 15 Jan 2024 04:47:25 -0500 Subject: [PATCH] Improve `api/v1/markers#create` performance against simultaneous requests (#28718) --- app/controllers/api/v1/markers_controller.rb | 2 +- .../api/v1/markers_spec.rb} | 24 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) rename spec/{controllers/api/v1/markers_controller_spec.rb => requests/api/v1/markers_spec.rb} (62%) diff --git a/app/controllers/api/v1/markers_controller.rb b/app/controllers/api/v1/markers_controller.rb index f8dfba8a94..8eaf7767df 100644 --- a/app/controllers/api/v1/markers_controller.rb +++ b/app/controllers/api/v1/markers_controller.rb @@ -19,7 +19,7 @@ class Api::V1::MarkersController < Api::BaseController @markers = {} resource_params.each_pair do |timeline, timeline_params| - @markers[timeline] = current_user.markers.find_or_initialize_by(timeline: timeline) + @markers[timeline] = current_user.markers.find_or_create_by(timeline: timeline) @markers[timeline].update!(timeline_params) end end diff --git a/spec/controllers/api/v1/markers_controller_spec.rb b/spec/requests/api/v1/markers_spec.rb similarity index 62% rename from spec/controllers/api/v1/markers_controller_spec.rb rename to spec/requests/api/v1/markers_spec.rb index e954bbd1b6..a1ca4ba754 100644 --- a/spec/controllers/api/v1/markers_controller_spec.rb +++ b/spec/requests/api/v1/markers_spec.rb @@ -2,20 +2,18 @@ require 'rails_helper' -RSpec.describe Api::V1::MarkersController do - render_views +RSpec.describe 'API Markers' do + let(:user) { Fabricate(:user) } + let(:scopes) { 'read:statuses write:statuses' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } - let!(:user) { Fabricate(:user) } - let!(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses write:statuses') } - - before { allow(controller).to receive(:doorkeeper_token) { token } } - - describe 'GET #index' do + describe 'GET /api/v1/markers' do before do Fabricate(:marker, timeline: 'home', last_read_id: 123, user: user) Fabricate(:marker, timeline: 'notifications', last_read_id: 456, user: user) - get :index, params: { timeline: %w(home notifications) } + get '/api/v1/markers', headers: headers, params: { timeline: %w(home notifications) } end it 'returns markers', :aggregate_failures do @@ -29,10 +27,10 @@ RSpec.describe Api::V1::MarkersController do end end - describe 'POST #create' do + describe 'POST /api/v1/markers' do context 'when no marker exists' do before do - post :create, params: { home: { last_read_id: '69420' } } + post '/api/v1/markers', headers: headers, params: { home: { last_read_id: '69420' } } end it 'creates a marker', :aggregate_failures do @@ -44,8 +42,8 @@ RSpec.describe Api::V1::MarkersController do context 'when a marker exists' do before do - post :create, params: { home: { last_read_id: '69420' } } - post :create, params: { home: { last_read_id: '70120' } } + post '/api/v1/markers', headers: headers, params: { home: { last_read_id: '69420' } } + post '/api/v1/markers', headers: headers, params: { home: { last_read_id: '70120' } } end it 'updates a marker', :aggregate_failures do