From 44eba0bb608d2cef9651cc25eeb06e3e8841ee7d Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 18 Oct 2018 10:52:45 +0800 Subject: [PATCH] FIX: Don't rescue `PG::UniqueViolation` within a transaction. Also acquire a transaction per link instead of failing when any of the links can't be processed. This prevents ActiveRecord from rolling back the transaction and the next SQL statement sent to PG will fail. This is however hard to test as it only happens when there are two competing process trying to process this method at the same time. --- app/models/topic_link.rb | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 22f8feeab73..229e2eb835a 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -112,24 +112,23 @@ SQL return if post.blank? || post.whisper? added_urls = [] - TopicLink.transaction do + reflected_ids = [] - added_urls = [] - reflected_ids = [] - - PrettyText - .extract_links(post.cooked) - .map do |u| - uri = begin - URI.parse(u.url) - rescue URI::Error - end - - [u, uri] + PrettyText + .extract_links(post.cooked) + .map do |u| + uri = begin + URI.parse(u.url) + rescue URI::Error end - .reject { |_, p| p.nil? || "mailto".freeze == p.scheme } - .uniq { |_, p| p } - .each do |link, parsed| + + [u, uri] + end + .reject { |_, p| p.nil? || "mailto".freeze == p.scheme } + .uniq { |_, p| p } + .each do |link, parsed| + + TopicLink.transaction do begin url = link.url internal = false @@ -185,7 +184,7 @@ SQL link_post_id: reflected_post.try(:id), quote: link.is_quote, extension: file_extension) - rescue ActiveRecord::RecordNotUnique, PG::UniqueViolation + rescue ActiveRecord::RecordNotUnique # it's fine end end