Skip to content

Commit

Permalink
[api] do not crash when a request has MANY actions
Browse files Browse the repository at this point in the history
Limiting notification payload to 50 for now
  • Loading branch information
adrianschroeter committed Feb 25, 2015
1 parent a834c5b commit fa3dee7
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 4 deletions.
6 changes: 4 additions & 2 deletions src/api/app/models/bs_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,8 @@ def send_state_change
Event::RequestStatechange.create(self.notify_parameters)
end

ActionNotifyLimit=50

def notify_parameters(ret = {})
ret[:id] = self.id
ret[:description] = self.description
Expand All @@ -726,15 +728,15 @@ def notify_parameters(ret = {})

# Use a nested data structure to support multiple actions in one request
ret[:actions] = []
self.bs_request_actions.each do |a|
self.bs_request_actions[0..ActionNotifyLimit].each do |a|
ret[:actions] << a.notify_params
end
ret
end

def self.actions_summary(payload)
ret = []
payload.with_indifferent_access['actions'].each do |a|
payload.with_indifferent_access['actions'][0..ActionNotifyLimit].each do |a|
str = "#{a['type']} #{a['targetproject']}"
str += "/#{a['targetpackage']}" if a['targetpackage']
str += "/#{a['targetrepository']}" if a['targetrepository']
Expand Down
4 changes: 2 additions & 2 deletions src/api/app/models/event/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ class Event::Request < ::Event::Base
self.abstract_class = true
payload_keys :author, :comment, :description, :id, :actions, :state, :when, :who

DiffLimit = 120

def self.message_id(id)
"<obs-request-#{id}@#{message_domain}>"
end
Expand Down Expand Up @@ -68,8 +70,6 @@ def calculate_diff(a)
end
end

DiffLimit = 120

def payload_with_diff
ret = payload
payload['actions'].each do |a|
Expand Down
25 changes: 25 additions & 0 deletions src/api/test/functional/request_events_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,31 @@ def verify_email(fixture_name, myid, email)
verify_email('request_event', myid, email)
end

test 'very_large_request_event' do
login_Iggy

Timecop.travel(2013, 8, 20, 12, 0, 0)
myid = 0
assert_difference 'ActionMailer::Base.deliveries.size', +1 do
body = "<request>\n"
actions=1000
actions.times do |t|
body += "<action type='add_role'><target project='home:tom'/><person name='Iggy' role='reviewer'/></action>\n"
end
body += "</request>"
raw_post '/request?cmd=create', body
assert_response :success
req = Xmlhash.parse(@response.body)
assert_equal actions, req['action'].count
myid = req['id']
end

email = ActionMailer::Base.deliveries.last

assert_match(/^Request #{myid} created by Iggy \(add_role home:tom, /, email.subject)
assert_equal %w(tschmidt@example.com), email.to # tom is maintainer
end

test 'set_bugowner event' do
login_Iggy

Expand Down

0 comments on commit fa3dee7

Please sign in to comment.