From 3b405562ccd8c7238daed08d63a9fb10ba420339 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 2 Nov 2013 22:32:00 +0100 Subject: [PATCH] Improved fetching of records (race conditions). --- .../_application_controller_generic.js.coffee | 6 +- .../app/controllers/widget/user.js.coffee | 2 - .../app/lib/app_post/auth.js.coffee | 3 +- .../app/models/_application_model.js.coffee | 1 + .../app/models/ticket_article.js.coffee | 4 +- .../javascripts/app/models/user.js.coffee | 3 +- app/controllers/users_controller.rb | 47 ++++++++++++++++ app/models/user.rb | 55 ++++++++++++++++--- config/routes/user.rb | 1 + db/migrate/20131102000000_change_user2.rb | 14 +++++ lib/user_agent.rb | 19 ++++--- 11 files changed, 130 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20131102000000_change_user2.rb diff --git a/app/assets/javascripts/app/controllers/_application_controller_generic.js.coffee b/app/assets/javascripts/app/controllers/_application_controller_generic.js.coffee index 3a88da7c6..0b98b9a6b 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_generic.js.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_generic.js.coffee @@ -42,7 +42,8 @@ class App.ControllerGenericNew extends App.ControllerModal object.save( success: -> if ui.callback - ui.callback( @ ) + item = App[ ui.genericObject ].retrieve(@id) + ui.callback( item ) ui.modalHide() error: -> @@ -88,7 +89,8 @@ class App.ControllerGenericEdit extends App.ControllerModal @item.save( success: -> if ui.callback - ui.callback( @ ) + item = App[ ui.genericObject ].retrieve(@id) + ui.callback( item ) ui.modalHide() error: => diff --git a/app/assets/javascripts/app/controllers/widget/user.js.coffee b/app/assets/javascripts/app/controllers/widget/user.js.coffee index 9016b5023..7428a165d 100644 --- a/app/assets/javascripts/app/controllers/widget/user.js.coffee +++ b/app/assets/javascripts/app/controllers/widget/user.js.coffee @@ -21,8 +21,6 @@ class App.WidgetUser extends App.ControllerDrox App.User.unsubscribe(@subscribeId) render: (user) => - if !user - user = @u # get display data userData = [] diff --git a/app/assets/javascripts/app/lib/app_post/auth.js.coffee b/app/assets/javascripts/app/lib/app_post/auth.js.coffee index 288ad5c5e..cd59209a6 100644 --- a/app/assets/javascripts/app/lib/app_post/auth.js.coffee +++ b/app/assets/javascripts/app/lib/app_post/auth.js.coffee @@ -72,8 +72,7 @@ class App.Auth return false; # set avatar - if !data.session.image - data.session.image = 'http://placehold.it/48x48' + data.session.image = App.Config.get('api_path') + '/users/image/' + data.session.image # update config for key, value of data.config diff --git a/app/assets/javascripts/app/models/_application_model.js.coffee b/app/assets/javascripts/app/models/_application_model.js.coffee index 1c56f2779..f49560355 100644 --- a/app/assets/javascripts/app/models/_application_model.js.coffee +++ b/app/assets/javascripts/app/models/_application_model.js.coffee @@ -131,6 +131,7 @@ class App.Model extends Spine.Model if @RETRIEVE_CALLBACK[ record.id ] for key, callback of @RETRIEVE_CALLBACK[ record.id ] data = App[ @className ].find( record.id ) + data = @_fillUp( data ) callback( data ) delete @RETRIEVE_CALLBACK[ record.id ][ key ] if _.isEmpty @RETRIEVE_CALLBACK[ record.id ] diff --git a/app/assets/javascripts/app/models/ticket_article.js.coffee b/app/assets/javascripts/app/models/ticket_article.js.coffee index 09360a160..859c6aa96 100644 --- a/app/assets/javascripts/app/models/ticket_article.js.coffee +++ b/app/assets/javascripts/app/models/ticket_article.js.coffee @@ -24,9 +24,9 @@ class App.TicketArticle extends App.Model # add created & updated if data.created_by_id - data.created_by = App.User.find( data.created_by_id ) + data.created_by = App.User.retrieve( data.created_by_id ) if data.updated_by_id - data.updated_by = App.User.find( data.updated_by_id ) + data.updated_by = App.User.retrieve( data.updated_by_id ) # add relations data.article_type = App.TicketArticleType.find( data.ticket_article_type_id ) diff --git a/app/assets/javascripts/app/models/user.js.coffee b/app/assets/javascripts/app/models/user.js.coffee index 2868a32e7..29f43331d 100644 --- a/app/assets/javascripts/app/models/user.js.coffee +++ b/app/assets/javascripts/app/models/user.js.coffee @@ -44,8 +44,7 @@ class App.User extends App.Model data['accounts'][account]['link'] = 'https://www.facebook.com/profile.php?id=' + data['accounts'][account]['uid'] # set image url - if !data.image - data.image = 'http://placehold.it/48x48' + data.image = @apiPath + '/users/image/' + data.image data diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8944f0021..58358015b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -570,4 +570,51 @@ curl http://localhost/api/v1/users/account.json -v -u #{login}:#{password} -H "C render :json => { :message => 'ok' }, :status => :ok end +=begin + +Resource: +GET /api/v1/users/image/8d6cca1c6bdc226cf2ba131e264ca2c7 + +Response: + + +Test: +curl http://localhost/api/v1/users/image/8d6cca1c6bdc226cf2ba131e264ca2c7 -v -u #{login}:#{password} + +=end + + def image + + # cache image + response.headers['Expires'] = 1.year.from_now.httpdate + response.headers["Cache-Control"] = "cache, store, max-age=31536000, must-revalidate" + response.headers["Pragma"] = "cache" + + # serve user image + user = User.where( :image => params[:hash] ).first + if user + # find file + list = Store.list( :object => 'User::Image', :o_id => user.id ) + if list && list[0] + file = Store.find( list[0] ) + send_data( + file.store_file.data, + :filename => file.filename, + :type => file.preferences['Content-Type'] || file.preferences['Mime-Type'], + :disposition => 'inline' + ) + return + end + end + + # serve defalt image + image = 'R0lGODdhMAAwAOMAAMzMzJaWlr6+vqqqqqOjo8XFxbe3t7GxsZycnAAAAAAAAAAAAAAAAAAAAAAAAAAAACwAAAAAMAAwAAAEcxDISau9OOvNu/9gKI5kaZ5oqq5s675wLM90bd94ru98TwuAA+KQAQqJK8EAgBAgMEqmkzUgBIeSwWGZtR5XhSqAULACCoGCJGwlm1MGQrq9RqgB8fm4ZTUgDBIEcRR9fz6HiImKi4yNjo+QkZKTlJWWkBEAOw==' + send_data( + Base64.decode64(image), + :filename => 'image.gif', + :type => 'image/gif', + :disposition => 'inline', + ) + end + end diff --git a/app/models/user.rb b/app/models/user.rb index 0863c2b1b..777e51d63 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,13 +1,15 @@ # Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/ +require 'digest/md5' + class User < ApplicationModel include User::Assets extend User::Search before_create :check_name, :check_email, :check_login, :check_image, :check_password before_update :check_password, :check_image, :check_email, :check_login_update - after_create :notify_clients_after_create - after_update :notify_clients_after_update + after_create :check_image_load, :notify_clients_after_create + after_update :check_image_load, :notify_clients_after_update after_destroy :notify_clients_after_destroy has_and_belongs_to_many :groups, :after_add => :cache_update, :after_remove => :cache_update @@ -22,12 +24,15 @@ class User < ApplicationModel activity_stream_support( :role => 'Admin', :ignore_attributes => { - :last_login => true, - } + :last_login => true, + :image => true, + :image_source => true, } ) history_support( :ignore_attributes => { - :password => true, + :password => true, + :image => true, + :image_source => true, } ) @@ -530,15 +535,49 @@ returns end def check_image - require 'digest/md5' - if !self.image || self.image == '' + if !self.image_source || self.image_source == '' || self.image_source =~ /gravatar.com/i if self.email hash = Digest::MD5.hexdigest(self.email) - self.image = "http://www.gravatar.com/avatar/#{hash}?s=48" + self.image_source = "http://www.gravatar.com/avatar/#{hash}?s=48&d=404" end end end + def check_image_load + + return if !self.image_source + return if self.image_source !~ /http/i + + # download image + response = UserAgent.request( self.image_source ) + if !response.success? + self.update_column( :image, 'none' ) + puts "WARNING: Can't fetch '#{url}', http code: #{response.code.to_s}" + #raise "Can't fetch '#{url}', http code: #{response.code.to_s}" + return + end + + # store image local + hash = Digest::MD5.hexdigest( response.body ) + + # check if image has changed + return if self.image != hash + + # save new image + self.update_column( :image, hash ) + Store.remove( :object => 'User::Image', :o_id => self.id ) + Store.add( + :object => 'User::Image', + :o_id => self.id, + :data => response.body, + :filename => 'image', + :preferences => { + 'Content-Type' => response.content_type + }, + :created_by_id => self.updated_by_id, + ) + end + def check_password # set old password again if not given diff --git a/config/routes/user.rb b/config/routes/user.rb index 7e019ef67..d32ad1fbc 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -13,5 +13,6 @@ Zammad::Application.routes.draw do match api_path + '/users/history/:id', :to => 'users#history', :via => :get match api_path + '/users', :to => 'users#create', :via => :post match api_path + '/users/:id', :to => 'users#update', :via => :put + match api_path + '/users/image/:hash', :to => 'users#image', :via => :get end \ No newline at end of file diff --git a/db/migrate/20131102000000_change_user2.rb b/db/migrate/20131102000000_change_user2.rb new file mode 100644 index 000000000..013a2ed12 --- /dev/null +++ b/db/migrate/20131102000000_change_user2.rb @@ -0,0 +1,14 @@ +class ChangeUser2 < ActiveRecord::Migration + def up + add_column :users, :image_source, :string, :limit => 200, :null => true + add_index :users, [:image] + User.all.each {|user| + puts "Update user #{user.login}" + user.image_source = user.image + user.save + } + end + + def down + end +end diff --git a/lib/user_agent.rb b/lib/user_agent.rb index afad3aff3..c63e1bb01 100644 --- a/lib/user_agent.rb +++ b/lib/user_agent.rb @@ -116,9 +116,10 @@ returns when Net::HTTPOK return Result.new( - :body => response.body, - :success => true, - :code => response.code, + :body => response.body, + :content_type => response['Content-Type'], + :success => true, + :code => response.code, ) end @@ -171,10 +172,11 @@ returns class Result def initialize(options) - @success = options[:success] - @body = options[:body] - @code = options[:code] - @error = options[:error] + @success = options[:success] + @body = options[:body] + @code = options[:code] + @content_type = options[:content_type] + @error = options[:error] end def error @error @@ -188,5 +190,8 @@ returns def code @code end + def content_type + @content_type + end end end