Added database locking to prevent race conditions.

This commit is contained in:
Martin Edenhofer 2016-09-08 21:18:26 +02:00
parent 18cc4af6b8
commit eceb9889b1
6 changed files with 160 additions and 109 deletions

View file

@ -535,11 +535,14 @@ class ApplicationController < ActionController::Base
clean_params = object.param_association_lookup(params) clean_params = object.param_association_lookup(params)
clean_params = object.param_cleanup(clean_params, true) clean_params = object.param_cleanup(clean_params, true)
# save object generic_object.with_lock do
generic_object.update_attributes!(clean_params)
# set relations # set attributes
generic_object.param_set_associations(params) generic_object.update_attributes!(clean_params)
# set relations
generic_object.param_set_associations(params)
end
if params[:expand] if params[:expand]
render json: generic_object.attributes_with_relation_names, status: :ok render json: generic_object.attributes_with_relation_names, status: :ok

View file

@ -114,25 +114,26 @@ class TicketsController < ApplicationController
# create ticket # create ticket
ticket.save! ticket.save!
ticket.with_lock do
# create tags if given # create tags if given
if params[:tags] && !params[:tags].empty? if params[:tags] && !params[:tags].empty?
tags = params[:tags].split(/,/) tags = params[:tags].split(/,/)
tags.each { |tag| tags.each { |tag|
Tag.tag_add( Tag.tag_add(
object: 'Ticket', object: 'Ticket',
o_id: ticket.id, o_id: ticket.id,
item: tag, item: tag,
created_by_id: current_user.id, created_by_id: current_user.id,
) )
} }
end
# create article if given
if params[:article]
article_create(ticket, params[:article])
end
end end
# create article if given
if params[:article]
article_create(ticket, params[:article])
end
# create links (e. g. in case of ticket split) # create links (e. g. in case of ticket split)
# links: { # links: {
# Ticket: { # Ticket: {
@ -191,10 +192,11 @@ class TicketsController < ApplicationController
} }
end end
ticket.update_attributes!(clean_params) ticket.with_lock do
ticket.update_attributes!(clean_params)
if params[:article] if params[:article]
article_create(ticket, params[:article]) article_create(ticket, params[:article])
end
end end
if params[:expand] if params[:expand]

View file

@ -254,29 +254,31 @@ class UsersController < ApplicationController
# permission check # permission check
permission_check_by_permission(params) permission_check_by_permission(params)
user.update_attributes(clean_params) user.with_lock do
user.update_attributes(clean_params)
# only allow Admin's # only allow Admin's
if current_user.permissions?('admin.user') && (params[:role_ids] || params[:roles]) if current_user.permissions?('admin.user') && (params[:role_ids] || params[:roles])
user.role_ids = params[:role_ids] user.role_ids = params[:role_ids]
user.param_set_associations({ role_ids: params[:role_ids], roles: params[:roles] }) user.param_set_associations({ role_ids: params[:role_ids], roles: params[:roles] })
end end
# only allow Admin's # only allow Admin's
if current_user.permissions?('admin.user') && (params[:group_ids] || params[:groups]) if current_user.permissions?('admin.user') && (params[:group_ids] || params[:groups])
user.group_ids = params[:group_ids] user.group_ids = params[:group_ids]
user.param_set_associations({ group_ids: params[:group_ids], groups: params[:groups] }) user.param_set_associations({ group_ids: params[:group_ids], groups: params[:groups] })
end end
# only allow Admin's and Agent's # only allow Admin's and Agent's
if current_user.permissions?(['admin.user', 'ticket.agent']) && (params[:organization_ids] || params[:organizations]) if current_user.permissions?(['admin.user', 'ticket.agent']) && (params[:organization_ids] || params[:organizations])
user.param_set_associations({ organization_ids: params[:organization_ids], organizations: params[:organizations] }) user.param_set_associations({ organization_ids: params[:organization_ids], organizations: params[:organizations] })
end end
if params[:expand] if params[:expand]
user = User.find(user.id).attributes_with_relation_names user = User.find(user.id).attributes_with_relation_names
render json: user, status: :ok render json: user, status: :ok
return return
end
end end
# get new data # get new data
@ -704,7 +706,7 @@ curl http://localhost/api/v1/users/password_change.json -v -u #{login}:#{passwor
render json: { message: 'failed', notice: ['Current password needed!'] }, status: :ok render json: { message: 'failed', notice: ['Current password needed!'] }, status: :ok
return return
end end
user = User.authenticate( current_user.login, params[:password_old] ) user = User.authenticate(current_user.login, params[:password_old])
if !user if !user
render json: { message: 'failed', notice: ['Current password is wrong!'] }, status: :ok render json: { message: 'failed', notice: ['Current password is wrong!'] }, status: :ok
return return
@ -763,10 +765,12 @@ curl http://localhost/api/v1/users/preferences.json -v -u #{login}:#{password} -
if params[:user] if params[:user]
user = User.find(current_user.id) user = User.find(current_user.id)
params[:user].each { |key, value| user.with_lock do
user.preferences[key.to_sym] = value params[:user].each { |key, value|
} user.preferences[key.to_sym] = value
user.save }
user.save
end
end end
render json: { message: 'ok' }, status: :ok render json: { message: 'ok' }, status: :ok
end end

View file

@ -442,10 +442,10 @@ retrns
# get ticket# based on email headers # get ticket# based on email headers
if mail[ 'x-zammad-ticket-id'.to_sym ] if mail[ 'x-zammad-ticket-id'.to_sym ]
ticket = Ticket.find_by( id: mail[ 'x-zammad-ticket-id'.to_sym ] ) ticket = Ticket.find_by(id: mail[ 'x-zammad-ticket-id'.to_sym ])
end end
if mail[ 'x-zammad-ticket-number'.to_sym ] if mail[ 'x-zammad-ticket-number'.to_sym ]
ticket = Ticket.find_by( number: mail[ 'x-zammad-ticket-number'.to_sym ] ) ticket = Ticket.find_by(number: mail[ 'x-zammad-ticket-number'.to_sym ])
end end
# set ticket state to open if not new # set ticket state to open if not new
@ -500,7 +500,6 @@ retrns
priority_id: Ticket::Priority.find_by(name: '2 normal').id, priority_id: Ticket::Priority.find_by(name: '2 normal').id,
preferences: preferences, preferences: preferences,
) )
set_attributes_by_x_headers(ticket, 'ticket', mail) set_attributes_by_x_headers(ticket, 'ticket', mail)
# create ticket # create ticket
@ -508,45 +507,47 @@ retrns
end end
# set attributes # set attributes
article = Ticket::Article.new( ticket.with_lock do
ticket_id: ticket.id, article = Ticket::Article.new(
type_id: Ticket::Article::Type.find_by(name: 'email').id, ticket_id: ticket.id,
sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, type_id: Ticket::Article::Type.find_by(name: 'email').id,
content_type: mail[:content_type], sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id,
body: mail[:body], content_type: mail[:content_type],
from: mail[:from], body: mail[:body],
to: mail[:to], from: mail[:from],
cc: mail[:cc], to: mail[:to],
subject: mail[:subject], cc: mail[:cc],
message_id: mail[:message_id], subject: mail[:subject],
internal: false, message_id: mail[:message_id],
) internal: false,
)
# x-headers lookup # x-headers lookup
set_attributes_by_x_headers(article, 'article', mail) set_attributes_by_x_headers(article, 'article', mail)
# create article # create article
article.save! article.save!
# store mail plain # store mail plain
Store.add( Store.add(
object: 'Ticket::Article::Mail', object: 'Ticket::Article::Mail',
o_id: article.id, o_id: article.id,
data: msg, data: msg,
filename: "ticket-#{ticket.number}-#{article.id}.eml", filename: "ticket-#{ticket.number}-#{article.id}.eml",
preferences: {} preferences: {}
) )
# store attachments # store attachments
if mail[:attachments] if mail[:attachments]
mail[:attachments].each do |attachment| mail[:attachments].each do |attachment|
Store.add( Store.add(
object: 'Ticket::Article', object: 'Ticket::Article',
o_id: article.id, o_id: article.id,
data: attachment[:data], data: attachment[:data],
filename: attachment[:filename], filename: attachment[:filename],
preferences: attachment[:preferences] preferences: attachment[:preferences]
) )
end
end end
end end
end end

View file

@ -294,25 +294,30 @@ returns
def self.send(client_id, data) def self.send(client_id, data)
path = "#{@path}/#{client_id}/" path = "#{@path}/#{client_id}/"
filename = "send-#{Time.now.utc.to_f}" filename = "send-#{Time.now.utc.to_f}"
location = "#{path}#{filename}"
check = true check = true
count = 0 count = 0
while check while check
if File.exist?(path + filename) if File.exist?(location)
count += 1 count += 1
filename = "#{filename}-#{count}" location = "#{path}#{filename}-#{count}"
else else
check = false check = false
end end
end end
return false if !File.directory? path return false if !File.directory? path
File.open(path + 'a-' + filename, 'wb') { |file| begin
file.flock(File::LOCK_EX) File.open(location, 'wb') { |file|
file.write data.to_json file.flock(File::LOCK_EX)
file.flock(File::LOCK_UN) file.write data.to_json
file.close file.flock(File::LOCK_UN)
} file.close
return false if !File.exist?(path + 'a-' + filename) }
FileUtils.mv(path + 'a-' + filename, path + filename) rescue => e
log('error', e.inspect)
log('error', "error in writing message file '#{location}'")
return false
end
true true
end end

View file

@ -125,7 +125,7 @@ class TestCase < Test::Unit::TestCase
def screenshot(params) def screenshot(params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
comment = params[:comment] || '' comment = params[:comment] || ''
filename = "tmp/#{Time.zone.now.strftime('screenshot_%Y_%m_%d__%H_%M_%S')}_#{comment}_#{instance.hash}.png" filename = "tmp/#{Time.zone.now.strftime('screenshot_%Y_%m_%d__%H_%M_%S_%L')}_#{comment}#{instance.hash}.png"
log('screenshot', { filename: filename }) log('screenshot', { filename: filename })
instance.save_screenshot(filename) instance.save_screenshot(filename)
end end
@ -415,6 +415,7 @@ class TestCase < Test::Unit::TestCase
log('click', params) log('click', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'click_before')
if params[:css] if params[:css]
begin begin
@ -501,6 +502,7 @@ class TestCase < Test::Unit::TestCase
modal_disappear( modal_disappear(
browser: browser1, browser: browser1,
timeout: 12, # default 6
) )
=end =end
@ -514,7 +516,7 @@ class TestCase < Test::Unit::TestCase
watch_for_disappear( watch_for_disappear(
browser: instance, browser: instance,
css: '.modal', css: '.modal',
timeout: 6, timeout: params[:timeout] || 6,
) )
end end
@ -599,6 +601,7 @@ class TestCase < Test::Unit::TestCase
log('set', params) log('set', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'set_before')
element = instance.find_elements(css: params[:css])[0] element = instance.find_elements(css: params[:css])[0]
if !params[:no_click] if !params[:no_click]
@ -616,11 +619,24 @@ class TestCase < Test::Unit::TestCase
} }
end end
# it's not working stable with ff via selenium, use js
if browser =~ /firefox/i && params[:css] =~ /\[data-name=/
log('set_ff_check', params)
value = instance.find_elements(css: params[:css])[0].text
if value != params[:value]
log('set_ff_check_failed_use_js', params)
value_quoted = quote(params[:value])
puts "DEBUG $('#{params[:css]}').html('#{value_quoted}').trigger('focusout')"
instance.execute_script("$('#{params[:css]}').html('#{value_quoted}').trigger('focusout')")
end
end
if params[:blur] if params[:blur]
instance.execute_script("$('#{params[:css]}').blur()") instance.execute_script("$('#{params[:css]}').blur()")
end end
sleep 0.2 sleep 0.2
screenshot(browser: instance, comment: 'set_after')
end end
=begin =begin
@ -639,6 +655,7 @@ class TestCase < Test::Unit::TestCase
log('select', params) log('select', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'select_before')
# searchable select # searchable select
element = instance.find_elements(css: "#{params[:css]}.js-shadow")[0] element = instance.find_elements(css: "#{params[:css]}.js-shadow")[0]
@ -677,6 +694,7 @@ class TestCase < Test::Unit::TestCase
#puts "select2 - #{params.inspect}" #puts "select2 - #{params.inspect}"
end end
sleep 0.4 sleep 0.4
screenshot(browser: instance, comment: 'select_after')
end end
=begin =begin
@ -695,6 +713,7 @@ class TestCase < Test::Unit::TestCase
log('switch', params) log('switch', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'switch_before')
element = instance.find_elements(css: "#{params[:css]} input[type=checkbox]")[0] element = instance.find_elements(css: "#{params[:css]} input[type=checkbox]")[0]
checked = element.attribute('checked') checked = element.attribute('checked')
@ -720,6 +739,7 @@ class TestCase < Test::Unit::TestCase
raise 'Switch not off!' if checked raise 'Switch not off!' if checked
end end
end end
screenshot(browser: instance, comment: 'switch_after')
end end
=begin =begin
@ -736,11 +756,13 @@ class TestCase < Test::Unit::TestCase
log('check', params) log('check', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'check_before')
instance.execute_script("if (!$('#{params[:css]}').prop('checked')) { $('#{params[:css]}').click() }") instance.execute_script("if (!$('#{params[:css]}').prop('checked')) { $('#{params[:css]}').click() }")
#element = instance.find_elements(css: params[:css])[0] #element = instance.find_elements(css: params[:css])[0]
#checked = element.attribute('checked') #checked = element.attribute('checked')
#element.click if !checked #element.click if !checked
screenshot(browser: instance, comment: 'check_after')
end end
=begin =begin
@ -757,11 +779,13 @@ class TestCase < Test::Unit::TestCase
log('uncheck', params) log('uncheck', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'uncheck_before')
instance.execute_script("if ($('#{params[:css]}').prop('checked')) { $('#{params[:css]}').click() }") instance.execute_script("if ($('#{params[:css]}').prop('checked')) { $('#{params[:css]}').click() }")
#element = instance.find_elements(css: params[:css])[0] #element = instance.find_elements(css: params[:css])[0]
#checked = element.attribute('checked') #checked = element.attribute('checked')
#element.click if checked #element.click if checked
screenshot(browser: instance, comment: 'uncheck_after')
end end
=begin =begin
@ -779,10 +803,12 @@ class TestCase < Test::Unit::TestCase
log('sendkey', params) log('sendkey', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'sendkey_before')
if params[:value].class == Array if params[:value].class == Array
params[:value].each { |key| params[:value].each { |key|
instance.action.send_keys(key).perform instance.action.send_keys(key).perform
} }
screenshot(browser: instance, comment: 'sendkey_after')
return return
end end
instance.action.send_keys(params[:value]).perform instance.action.send_keys(params[:value]).perform
@ -791,6 +817,7 @@ class TestCase < Test::Unit::TestCase
else else
sleep 0.2 sleep 0.2
end end
screenshot(browser: instance, comment: 'sendkey_after')
end end
=begin =begin
@ -825,9 +852,11 @@ class TestCase < Test::Unit::TestCase
end end
if params[:should_not_match] if params[:should_not_match]
if success if success
screenshot(browser: instance, comment: 'match_failed')
raise "should not match '#{params[:value]}' in select list, but is matching" raise "should not match '#{params[:value]}' in select list, but is matching"
end end
elsif !success elsif !success
screenshot(browser: instance, comment: 'match_failed')
raise "not matching '#{params[:value]}' in select list" raise "not matching '#{params[:value]}' in select list"
end end
@ -871,9 +900,11 @@ class TestCase < Test::Unit::TestCase
if match if match
if params[:should_not_match] if params[:should_not_match]
screenshot(browser: instance, comment: 'match_failed')
raise "matching '#{params[:value]}' in content '#{text}' but should not!" raise "matching '#{params[:value]}' in content '#{text}' but should not!"
end end
elsif !params[:should_not_match] elsif !params[:should_not_match]
screenshot(browser: instance, comment: 'match_failed')
raise "not matching '#{params[:value]}' in content '#{text}' but should!" raise "not matching '#{params[:value]}' in content '#{text}' but should!"
end end
sleep 0.2 sleep 0.2
@ -1032,10 +1063,11 @@ class TestCase < Test::Unit::TestCase
if title =~ /#{data[:title]}/i if title =~ /#{data[:title]}/i
assert(true, "matching '#{data[:title]}' in title '#{title}'") assert(true, "matching '#{data[:title]}' in title '#{title}'")
else else
screenshot(browser: instance, comment: 'verify_task_failed')
raise "not matching '#{data[:title]}' in title '#{title}'" raise "not matching '#{data[:title]}' in title '#{title}'"
end end
end end
puts "tv #{params.inspect}"
# verify modified # verify modified
if data.key?(:modified) if data.key?(:modified)
exists = instance.find_elements(css: '.tasks .is-active')[0] exists = instance.find_elements(css: '.tasks .is-active')[0]
@ -1051,15 +1083,19 @@ class TestCase < Test::Unit::TestCase
if is_modified if is_modified
assert(true, "task '#{data[:title]}' is modifed") assert(true, "task '#{data[:title]}' is modifed")
elsif !exists elsif !exists
screenshot(browser: instance, comment: 'verify_task_failed')
raise "task '#{data[:title]}' not exists, should not modified" raise "task '#{data[:title]}' not exists, should not modified"
else else
screenshot(browser: instance, comment: 'verify_task_failed')
raise "task '#{data[:title]}' is not modifed" raise "task '#{data[:title]}' is not modifed"
end end
elsif !is_modified elsif !is_modified
assert(true, "task '#{data[:title]}' is modifed") assert(true, "task '#{data[:title]}' is modifed")
elsif !exists elsif !exists
screenshot(browser: instance, comment: 'verify_task_failed')
raise "task '#{data[:title]}' not exists, should be not modified" raise "task '#{data[:title]}' not exists, should be not modified"
else else
screenshot(browser: instance, comment: 'verify_task_failed')
raise "task '#{data[:title]}' is modifed, but should not" raise "task '#{data[:title]}' is modifed, but should not"
end end
end end
@ -1285,12 +1321,14 @@ wait untill text in selector disabppears
switch_window_focus(params) switch_window_focus(params)
log('shortcut', params) log('shortcut', params)
instance = params[:browser] || @browser instance = params[:browser] || @browser
screenshot(browser: instance, comment: 'shortcut_before')
instance.action.key_down(:control) instance.action.key_down(:control)
.key_down(:alt) .key_down(:alt)
.send_keys(params[:key]) .send_keys(params[:key])
.key_up(:alt) .key_up(:alt)
.key_up(:control) .key_up(:control)
.perform .perform
screenshot(browser: instance, comment: 'shortcut_after')
end end
=begin =begin
@ -1829,14 +1867,6 @@ wait untill text in selector disabppears
clear: true, clear: true,
mute_log: true, mute_log: true,
) )
# it's not working stable via selenium, use js
value = instance.find_elements(css: '.content .newTicket div[data-name=body]')[0].text
#puts "V #{value.inspect}"
if value != data[:body]
body_quoted = quote(data[:body])
instance.execute_script("$('.content.active div[data-name=body]').html('#{body_quoted}').trigger('focusout')")
end
end end
if data[:customer] if data[:customer]
element = instance.find_elements(css: '.active .newTicket input[name="customer_id_completion"]')[0] element = instance.find_elements(css: '.active .newTicket input[name="customer_id_completion"]')[0]
@ -2260,12 +2290,14 @@ wait untill text in selector disabppears
browser: instance, browser: instance,
js: '$(".content.active .sidebar").css("display", "block")', js: '$(".content.active .sidebar").css("display", "block")',
) )
screenshot(browser: instance, comment: 'ticket_open_by_overview')
instance.find_elements(css: ".content.active .sidebar a[href=\"#{params[:link]}\"]")[0].click instance.find_elements(css: ".content.active .sidebar a[href=\"#{params[:link]}\"]")[0].click
sleep 1 sleep 1
execute( execute(
browser: instance, browser: instance,
js: '$(".content.active .sidebar").css("display", "none")', js: '$(".content.active .sidebar").css("display", "none")',
) )
screenshot(browser: instance, comment: 'ticket_open_by_overview_search')
instance.find_elements(partial_link_text: params[:number])[0].click instance.find_elements(partial_link_text: params[:number])[0].click
sleep 1 sleep 1
number = instance.find_elements(css: '.active .ticketZoom-header .ticket-number')[0].text number = instance.find_elements(css: '.active .ticketZoom-header .ticket-number')[0].text
@ -2310,8 +2342,9 @@ wait untill text in selector disabppears
sleep 1 sleep 1
# open ticket # open ticket
screenshot(browser: instance, comment: 'ticket_open_by_search')
#instance.find_element(partial_link_text: params[:number] } ).click #instance.find_element(partial_link_text: params[:number] } ).click
instance.execute_script("$(\".js-global-search-result a:contains('#{params[:value]}') .nav-tab-icon\").click()") instance.execute_script("$(\".js-global-search-result a:contains('#{params[:number]}') .nav-tab-icon\").first().click()")
sleep 1 sleep 1
number = instance.find_elements(css: '.active .ticketZoom-header .ticket-number')[0].text number = instance.find_elements(css: '.active .ticketZoom-header .ticket-number')[0].text
if number !~ /#{params[:number]}/ if number !~ /#{params[:number]}/
@ -2345,6 +2378,7 @@ wait untill text in selector disabppears
sleep 3 sleep 3
# open ticket # open ticket
screenshot(browser: instance, comment: 'ticket_open_by_title_search')
#instance.find_element(partial_link_text: params[:title] } ).click #instance.find_element(partial_link_text: params[:title] } ).click
instance.execute_script("$(\".js-global-search-result a:contains('#{params[:title]}') .nav-tab-icon\").click()") instance.execute_script("$(\".js-global-search-result a:contains('#{params[:title]}') .nav-tab-icon\").click()")
sleep 1 sleep 1
@ -2466,6 +2500,8 @@ wait untill text in selector disabppears
element.clear element.clear
element.send_keys(params[:value]) element.send_keys(params[:value])
sleep 3 sleep 3
screenshot(browser: instance, comment: 'user_open_by_search')
#instance.find_element(partial_link_text: params[:value]).click #instance.find_element(partial_link_text: params[:value]).click
instance.execute_script("$(\".js-global-search-result a:contains('#{params[:value]}') .nav-tab-icon\").click()") instance.execute_script("$(\".js-global-search-result a:contains('#{params[:value]}') .nav-tab-icon\").click()")
sleep 1 sleep 1