Commit 210f4fe7 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'fix/gb/unique-build-trace-constraints' into 'master'

Use `safe_find_or_create_by` to ensure build pending state presence

See merge request gitlab-org/gitlab!46877
parents 59306a42 35dff720
...@@ -48,6 +48,8 @@ class ApplicationRecord < ActiveRecord::Base ...@@ -48,6 +48,8 @@ class ApplicationRecord < ActiveRecord::Base
def self.safe_find_or_create_by!(*args, &block) def self.safe_find_or_create_by!(*args, &block)
safe_find_or_create_by(*args, &block).tap do |record| safe_find_or_create_by(*args, &block).tap do |record|
raise ActiveRecord::RecordNotFound unless record.present?
record.validate! unless record.persisted? record.validate! unless record.persisted?
end end
end end
......
...@@ -163,16 +163,18 @@ module Ci ...@@ -163,16 +163,18 @@ module Ci
end end
def ensure_pending_state def ensure_pending_state
Ci::BuildPendingState.create_or_find_by!( build_state = Ci::BuildPendingState.safe_find_or_create_by(
build_id: build.id, build_id: build.id,
state: params.fetch(:state), state: params.fetch(:state),
trace_checksum: params.fetch(:checksum), trace_checksum: params.fetch(:checksum),
failure_reason: params.dig(:failure_reason) failure_reason: params.dig(:failure_reason)
) )
rescue ActiveRecord::RecordNotFound
unless build_state.present?
metrics.increment_trace_operation(operation: :conflict) metrics.increment_trace_operation(operation: :conflict)
end
build.pending_state build_state || build.pending_state
end end
## ##
......
...@@ -67,7 +67,8 @@ RSpec.describe ApplicationRecord do ...@@ -67,7 +67,8 @@ RSpec.describe ApplicationRecord do
end end
it 'raises a validation error if the record was not persisted' do it 'raises a validation error if the record was not persisted' do
expect { Suggestion.find_or_create_by!(note: nil) }.to raise_error(ActiveRecord::RecordInvalid) expect { Suggestion.safe_find_or_create_by!(note: nil) }
.to raise_error(ActiveRecord::RecordInvalid)
end end
it 'passes a block to find_or_create_by' do it 'passes a block to find_or_create_by' do
...@@ -75,6 +76,14 @@ RSpec.describe ApplicationRecord do ...@@ -75,6 +76,14 @@ RSpec.describe ApplicationRecord do
Suggestion.safe_find_or_create_by!(suggestion_attributes, &block) Suggestion.safe_find_or_create_by!(suggestion_attributes, &block)
end.to yield_with_args(an_object_having_attributes(suggestion_attributes)) end.to yield_with_args(an_object_having_attributes(suggestion_attributes))
end end
it 'raises a record not found error in case of attributes mismatch' do
suggestion = Suggestion.safe_find_or_create_by!(suggestion_attributes)
attributes = suggestion_attributes.merge(outdated: !suggestion.outdated)
expect { Suggestion.safe_find_or_create_by!(attributes) }
.to raise_error(ActiveRecord::RecordNotFound)
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment