FIX: Don't use exceptions to catch conflicts

If a database exception is raised ActiveRecord will always rollback
even if caught.

Instead we build the query in manual SQL and DO NOTHING when there's a
conflict. If we detect nothing was done, perform an update.
This commit is contained in:
Robin Ward 2019-07-06 14:42:03 -04:00
parent feb828172b
commit a075fd46fd
2 changed files with 16 additions and 6 deletions

View File

@ -66,6 +66,10 @@ module HasCustomFields
attr_accessor :preloaded_custom_fields attr_accessor :preloaded_custom_fields
def custom_fields_fk
@custom_fields_fk ||= "#{_custom_fields.reflect_on_all_associations(:belongs_to)[0].name}_id"
end
# To avoid n+1 queries, use this function to retrieve lots of custom fields in one go # To avoid n+1 queries, use this function to retrieve lots of custom fields in one go
# and create a "sideloaded" version for easy querying by id. # and create a "sideloaded" version for easy querying by id.
def self.custom_fields_for_ids(ids, whitelisted_fields) def self.custom_fields_for_ids(ids, whitelisted_fields)
@ -249,13 +253,18 @@ module HasCustomFields
end end
end end
# We support unique indexes on certain fields. In the event two concurrenct processes attempt to
# update the same custom field we should catch the error and perform an update instead.
def create_singular(name, value, field_type = nil) def create_singular(name, value, field_type = nil)
write_value = value.is_a?(Hash) || field_type == :json ? value.to_json : value write_value = value.is_a?(Hash) || field_type == :json ? value.to_json : value
_custom_fields.create!(name: name, value: write_value) write_value = 't' if write_value.is_a?(TrueClass)
rescue ActiveRecord::RecordNotUnique write_value = 'f' if write_value.is_a?(FalseClass)
# We support unique indexes on certain fields. In the event two concurrenct processes attempt to row_count = DB.exec(<<~SQL, name: name, value: write_value, id: id)
# update the same custom field we should catch the error and perform an update instead. INSERT INTO #{_custom_fields.table_name} (#{custom_fields_fk}, name, value, created_at, updated_at)
_custom_fields.where(name: name).update_all(value: write_value) VALUES (:id, :name, :value, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
ON CONFLICT DO NOTHING
SQL
_custom_fields.where(name: name).update_all(value: write_value) if row_count == 0
end end
protected protected

View File

@ -7,7 +7,7 @@ describe HasCustomFields do
context "custom_fields" do context "custom_fields" do
before do before do
DB.exec("create temporary table custom_fields_test_items(id SERIAL primary key)") DB.exec("create temporary table custom_fields_test_items(id SERIAL primary key)")
DB.exec("create temporary table custom_fields_test_item_custom_fields(id SERIAL primary key, custom_fields_test_item_id int, name varchar(256) not null, value text)") DB.exec("create temporary table custom_fields_test_item_custom_fields(id SERIAL primary key, custom_fields_test_item_id int, name varchar(256) not null, value text, created_at TIMESTAMP, updated_at TIMESTAMP)")
DB.exec(<<~SQL) DB.exec(<<~SQL)
CREATE UNIQUE INDEX ON custom_fields_test_item_custom_fields (custom_fields_test_item_id) CREATE UNIQUE INDEX ON custom_fields_test_item_custom_fields (custom_fields_test_item_id)
WHERE NAME = 'rare' WHERE NAME = 'rare'
@ -282,6 +282,7 @@ describe HasCustomFields do
item0 = CustomFieldsTestItem.new item0 = CustomFieldsTestItem.new
item0.custom_fields = { "rare" => "gem" } item0.custom_fields = { "rare" => "gem" }
item0.save item0.save
expect(item0.reload.custom_fields['rare']).to eq("gem")
item0.create_singular('rare', "diamond") item0.create_singular('rare', "diamond")
expect(item0.reload.custom_fields['rare']).to eq("diamond") expect(item0.reload.custom_fields['rare']).to eq("diamond")