From eae35db92d27aa9a321addef0752115744c7acf4 Mon Sep 17 00:00:00 2001 From: Nuwan Date: Tue, 13 Apr 2021 00:20:46 +0530 Subject: [PATCH 1/3] refactor crash_dump in users api --- ruby/lib/jam_ruby.rb | 1 + .../lib/s3_analytics_manager_mixin.rb | 19 ++++++++++ ruby/lib/jam_ruby/models/crash_dump.rb | 35 ++++++++++++++++--- web/app/controllers/api_users_controller.rb | 27 +++++++------- web/config/application.rb | 1 + web/config/environments/test.rb | 1 + .../controllers/api_users_controller_spec.rb | 27 ++++++++++++++ web/spec/support/uses_temp_files.rb | 33 +++++++++++++++++ 8 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 ruby/lib/jam_ruby/lib/s3_analytics_manager_mixin.rb create mode 100644 web/spec/support/uses_temp_files.rb diff --git a/ruby/lib/jam_ruby.rb b/ruby/lib/jam_ruby.rb index 83da05d40..8433ab63c 100755 --- a/ruby/lib/jam_ruby.rb +++ b/ruby/lib/jam_ruby.rb @@ -44,6 +44,7 @@ require "jam_ruby/errors/conflict_error" require "jam_ruby/errors/pay_pal_client_error" require "jam_ruby/lib/app_config" require "jam_ruby/lib/s3_manager_mixin" +require "jam_ruby/lib/s3_analytics_manager_mixin" require "jam_ruby/lib/s3_public_manager_mixin" require "jam_ruby/lib/module_overrides" require "jam_ruby/lib/s3_util" diff --git a/ruby/lib/jam_ruby/lib/s3_analytics_manager_mixin.rb b/ruby/lib/jam_ruby/lib/s3_analytics_manager_mixin.rb new file mode 100644 index 000000000..4b201de85 --- /dev/null +++ b/ruby/lib/jam_ruby/lib/s3_analytics_manager_mixin.rb @@ -0,0 +1,19 @@ +module JamRuby + module S3AnalyticsManagerMixin + extend ActiveSupport::Concern + include AppConfig + + included do + end + + module ClassMethods + def s3_manager(options={:bucket => nil, :public => false}) + @s3_manager ||= S3Manager.new(options[:bucket] ? options[:bucket] : APP_CONFIG.aws_analytics_bucket, APP_CONFIG.aws_access_key_id, APP_CONFIG.aws_secret_access_key) + end + end + + def s3_manager(options={:bucket => nil, :public => false}) + @s3_manager ||= S3Manager.new(options[:bucket] ? options[:bucket] : app_config.aws_analytics_bucket, app_config.aws_access_key_id, app_config.aws_secret_access_key) + end + end +end \ No newline at end of file diff --git a/ruby/lib/jam_ruby/models/crash_dump.rb b/ruby/lib/jam_ruby/models/crash_dump.rb index f71d8c21d..c9c57f8da 100644 --- a/ruby/lib/jam_ruby/models/crash_dump.rb +++ b/ruby/lib/jam_ruby/models/crash_dump.rb @@ -1,7 +1,7 @@ module JamRuby class CrashDump < ActiveRecord::Base - include JamRuby::S3ManagerMixin + include JamRuby::S3AnalyticsManagerMixin self.table_name = "crash_dumps" @@ -9,15 +9,17 @@ module JamRuby belongs_to :user, :inverse_of => :crash_dumps, :class_name => "JamRuby::User" - validates :client_type, presence: true - validates :client_version, presence: true + validates :client_type, :client_version, :description, presence: true + #validates :client_version, presence: true attr_accessor :user_email before_validation(:on => :create) do self.created_at ||= Time.now self.id = SecureRandom.uuid - self.uri = "dumps/#{created_at.strftime('%Y-%m-%d')}/#{self.id}.zip" + #self.uri = "dumps/#{created_at.strftime('%Y-%m-%d')}/#{self.id}.zip" + type = description.gsub(/[^a-zA-Z0-9_.-]/, '') + self.uri = "stats/#{type}/#{created_at.strftime('%Y-%m-%d')}/#{self.id}.zip" end def user_email @@ -26,7 +28,30 @@ module JamRuby end def sign_url(expiration_time = 3600 * 24 * 7, secure=true) - s3_manager.sign_url(self[:ri], {:expires => expiration_time, :secure => secure}) + s3_manager.sign_url(self[:uri], {:expires => expiration_time, :secure => secure}) end + + def read_url + expire = Time.now + 5.years + s3_bucket.objects[uri].url_for(:read, + :expires => expire, + :'response_content_type' => 'application/octet-stream').to_s + end + + def write_url + s3_bucket.objects[uri].url_for(:write, + :expires => Rails.application.config.crash_dump_data_signed_url_timeout, + :'response_content_type' => 'application/octet-stream').to_s + + end + + private + + def s3_bucket + s3 = AWS::S3.new(:access_key_id => Rails.application.config.aws_access_key_id, + :secret_access_key => Rails.application.config.aws_secret_access_key) + s3.buckets[Rails.application.config.aws_analytics_bucket] + end + end end diff --git a/web/app/controllers/api_users_controller.rb b/web/app/controllers/api_users_controller.rb index 5068c4f11..051fe3426 100644 --- a/web/app/controllers/api_users_controller.rb +++ b/web/app/controllers/api_users_controller.rb @@ -684,20 +684,23 @@ class ApiUsersController < ApiController # This part is the piece that really needs to be decomposed into a library... if Rails.application.config.storage_type == :fog - s3 = AWS::S3.new(:access_key_id => Rails.application.config.aws_access_key_id, - :secret_access_key => Rails.application.config.aws_secret_access_key) - bucket = s3.buckets[Rails.application.config.aws_bucket] - uri = @dump.uri - expire = Time.now + 5.years - read_url = bucket.objects[uri].url_for(:read, - :expires => expire, - :'response_content_type' => 'application/octet-stream').to_s - #@dump.update_attribute(:uri, read_url) + # s3 = AWS::S3.new(:access_key_id => Rails.application.config.aws_access_key_id, + # :secret_access_key => Rails.application.config.aws_secret_access_key) + # bucket = s3.buckets[Rails.application.config.aws_bucket] + # uri = @dump.uri + # expire = Time.now + 5.years + # read_url = bucket.objects[uri].url_for(:read, + # :expires => expire, + # :'response_content_type' => 'application/octet-stream').to_s + # #@dump.update_attribute(:uri, read_url) - write_url = bucket.objects[uri].url_for(:write, - :expires => Rails.application.config.crash_dump_data_signed_url_timeout, - :'response_content_type' => 'application/octet-stream').to_s + # write_url = bucket.objects[uri].url_for(:write, + # :expires => Rails.application.config.crash_dump_data_signed_url_timeout, + # :'response_content_type' => 'application/octet-stream').to_s + read_url = @dump.read_url + write_url = @dump.write_url + logger.debug("crash_dump can read from url #{read_url}") description = @dump.description diff --git a/web/config/application.rb b/web/config/application.rb index 723829681..5112d4ae9 100644 --- a/web/config/application.rb +++ b/web/config/application.rb @@ -157,6 +157,7 @@ if defined?(Bundler) config.aws_region = 'us-east-1' config.aws_bucket = 'jamkazam-dev' config.aws_bucket_public = 'jamkazam-dev-public' + config.aws_analytics_bucket = 'jamkazam-analytics-dev' config.aws_cache = '315576000' config.aws_fullhost = "#{config.aws_bucket_public}.s3.amazonaws.com" config.aws_bucket_jamtracks = 'jamkazam-jamtracks' diff --git a/web/config/environments/test.rb b/web/config/environments/test.rb index c593f5261..2acd9605f 100644 --- a/web/config/environments/test.rb +++ b/web/config/environments/test.rb @@ -77,6 +77,7 @@ SampleApp::Application.configure do config.aws_bucket = 'jamkazam-testing' config.aws_bucket_public = 'jamkazam-testing' + config.aws_analytics_bucket = 'jamkazam-analytics-test' config.aws_access_key_id = 'AKIAJESQY24TOT542UHQ' # credentials for jamkazam-tester user, who has access to this bucket config.aws_secret_access_key = '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' config.aws_bucket_jamtracks = 'jamkazam-jamtracks-test' diff --git a/web/spec/controllers/api_users_controller_spec.rb b/web/spec/controllers/api_users_controller_spec.rb index c7d1ce941..e7d1a4e87 100644 --- a/web/spec/controllers/api_users_controller_spec.rb +++ b/web/spec/controllers/api_users_controller_spec.rb @@ -309,4 +309,31 @@ describe ApiUsersController, type: :controller do conn.user.last_jam_audio_latency.should == 5 end end + + describe "crash_dump" do + include UsesTempFiles + CRASH_TEMP_FILE='crash.txt' + in_directory_with_file(CRASH_TEMP_FILE) + + def put_file_to_aws(url, contents) + begin + RestClient.put( url, contents) + rescue => e + puts e.response + raise e + end + + end + + before(:all) do + Rails.application.config.storage_type = :fog + end + + it "307 Temporary Redirect to s3" do + content_for_file("this is a crash file") + put :crash_dump, client_type: 'MacOSX', version: '1.0', client_version: '1.0', client_id: '1', session_id: '1', timestamp: 1618246569, fsize: '10K', description: "memory_limit_exceeded", crash_context: "Blahblahblab" + expect(response).to have_http_status(307) + put_file_to_aws(response.location, File.read(CRASH_TEMP_FILE)) + end + end end diff --git a/web/spec/support/uses_temp_files.rb b/web/spec/support/uses_temp_files.rb new file mode 100644 index 000000000..263bfed1b --- /dev/null +++ b/web/spec/support/uses_temp_files.rb @@ -0,0 +1,33 @@ +#http://gabebw.wordpress.com/2011/03/21/temp-files-in-rspec/ + +# this will make a folder jam-ruby/spec/tmp if used in an rspec test, and delete it after +# our .gitignore would also keep spec/tmp out, if somehow it did not get deleted. +module UsesTempFiles + def self.included(example_group) + example_group.extend(self) + end + + def in_directory_with_file(file) + before do + @pwd = Dir.pwd + @tmp_dir = File.join(File.dirname(__FILE__), 'tmp') + FileUtils.mkdir_p(@tmp_dir) + Dir.chdir(@tmp_dir) + + FileUtils.mkdir_p(File.dirname(file)) + FileUtils.touch(file) + end + + define_method(:content_for_file) do |content| + f = File.new(File.join(@tmp_dir, file), 'a+') + f.write(content) + f.flush # VERY IMPORTANT + f.close + end + + after do + Dir.chdir(@pwd) + FileUtils.rm_rf(@tmp_dir) + end + end +end \ No newline at end of file From ba9a91eb1ffa106cf3acd8fa7807fa31626a309d Mon Sep 17 00:00:00 2001 From: Nuwan Date: Tue, 13 Apr 2021 01:19:03 +0530 Subject: [PATCH 2/3] crash_dump fix test error in content_type --- web/spec/controllers/api_users_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/spec/controllers/api_users_controller_spec.rb b/web/spec/controllers/api_users_controller_spec.rb index e7d1a4e87..edee44cff 100644 --- a/web/spec/controllers/api_users_controller_spec.rb +++ b/web/spec/controllers/api_users_controller_spec.rb @@ -317,7 +317,7 @@ describe ApiUsersController, type: :controller do def put_file_to_aws(url, contents) begin - RestClient.put( url, contents) + RestClient.put( url, contents, {content_type: nil}) rescue => e puts e.response raise e From 098e828d0f94f7e766ab4ae9fc7f24f26b78d5d7 Mon Sep 17 00:00:00 2001 From: Nuwan Date: Fri, 16 Apr 2021 05:16:22 +0530 Subject: [PATCH 3/3] refactor crash_dump --- ruby/lib/jam_ruby/models/crash_dump.rb | 14 ++++---------- ruby/spec/jam_ruby/models/crash_dump_spec.rb | 10 ++++++---- web/app/controllers/api_users_controller.rb | 14 -------------- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/ruby/lib/jam_ruby/models/crash_dump.rb b/ruby/lib/jam_ruby/models/crash_dump.rb index c9c57f8da..012eaa1da 100644 --- a/ruby/lib/jam_ruby/models/crash_dump.rb +++ b/ruby/lib/jam_ruby/models/crash_dump.rb @@ -10,14 +10,12 @@ module JamRuby belongs_to :user, :inverse_of => :crash_dumps, :class_name => "JamRuby::User" validates :client_type, :client_version, :description, presence: true - #validates :client_version, presence: true attr_accessor :user_email before_validation(:on => :create) do self.created_at ||= Time.now self.id = SecureRandom.uuid - #self.uri = "dumps/#{created_at.strftime('%Y-%m-%d')}/#{self.id}.zip" type = description.gsub(/[^a-zA-Z0-9_.-]/, '') self.uri = "stats/#{type}/#{created_at.strftime('%Y-%m-%d')}/#{self.id}.zip" end @@ -32,17 +30,13 @@ module JamRuby end def read_url - expire = Time.now + 5.years - s3_bucket.objects[uri].url_for(:read, - :expires => expire, - :'response_content_type' => 'application/octet-stream').to_s + s3_manager.sign_url(self[:uri], { :expires => Time.now + 5.years, + :'response_content_type' => 'application/octet-stream'}, :read) end def write_url - s3_bucket.objects[uri].url_for(:write, - :expires => Rails.application.config.crash_dump_data_signed_url_timeout, - :'response_content_type' => 'application/octet-stream').to_s - + s3_manager.sign_url(self[:uri], { :expires => Rails.application.config.crash_dump_data_signed_url_timeout, + :'response_content_type' => 'application/octet-stream'}, :write) end private diff --git a/ruby/spec/jam_ruby/models/crash_dump_spec.rb b/ruby/spec/jam_ruby/models/crash_dump_spec.rb index c395f1038..e8f4bf123 100644 --- a/ruby/spec/jam_ruby/models/crash_dump_spec.rb +++ b/ruby/spec/jam_ruby/models/crash_dump_spec.rb @@ -4,15 +4,17 @@ describe CrashDump do before do end - it "should fail to save a crash dump without a client_type and client_version" do - CrashDump.new(:client_type => "", :client_version => "version").should_not be_valid - CrashDump.new(:client_type => "type", :client_version => "").should_not be_valid + it "should fail to save a crash dump without a client_type, client_version or description" do + CrashDump.new(:client_type => "", :client_version => "version", :description => "").should_not be_valid + CrashDump.new(:client_type => "type", :client_version => "", :description => "").should_not be_valid + CrashDump.new(:client_type => "type", :client_version => "1.0", :description => "").should_not be_valid end - it "should be able to save a crash dump with JUST a client_type and client_version" do + it "should be able to save a crash dump with JUST a client_type, client_version and description" do @cd = CrashDump.new @cd.client_type = "Win32" @cd.client_version = "version" + @cd.description = "crash" @cd.should be_valid @cd.save diff --git a/web/app/controllers/api_users_controller.rb b/web/app/controllers/api_users_controller.rb index 051fe3426..a71815375 100644 --- a/web/app/controllers/api_users_controller.rb +++ b/web/app/controllers/api_users_controller.rb @@ -684,20 +684,6 @@ class ApiUsersController < ApiController # This part is the piece that really needs to be decomposed into a library... if Rails.application.config.storage_type == :fog - # s3 = AWS::S3.new(:access_key_id => Rails.application.config.aws_access_key_id, - # :secret_access_key => Rails.application.config.aws_secret_access_key) - # bucket = s3.buckets[Rails.application.config.aws_bucket] - # uri = @dump.uri - # expire = Time.now + 5.years - # read_url = bucket.objects[uri].url_for(:read, - # :expires => expire, - # :'response_content_type' => 'application/octet-stream').to_s - # #@dump.update_attribute(:uri, read_url) - - # write_url = bucket.objects[uri].url_for(:write, - # :expires => Rails.application.config.crash_dump_data_signed_url_timeout, - # :'response_content_type' => 'application/octet-stream').to_s - read_url = @dump.read_url write_url = @dump.write_url