Commit f2ae886550605e2f2cf88746e7f38df794ab6613
1 parent
a95a3e98
Exists in
master
and in
1 other branch
rework CSV export architecture
in order to work with multiple app servers, don't export to file. instead export to redis.
Showing
3 changed files
with
40 additions
and
80 deletions
Show diff stats
app/controllers/questions_controller.rb
@@ -73,8 +73,8 @@ class QuestionsController < InheritedResources::Base | @@ -73,8 +73,8 @@ class QuestionsController < InheritedResources::Base | ||
73 | 73 | ||
74 | # puts "redis key is::::: #{redis_key}" | 74 | # puts "redis key is::::: #{redis_key}" |
75 | 75 | ||
76 | - @question.send_later :export_and_delete, type, | ||
77 | - :response_type => response_type, :redis_key => redis_key, :delete_at => 3.days.from_now | 76 | + @question.send_later :export, type, |
77 | + :response_type => response_type, :redis_key => redis_key | ||
78 | 78 | ||
79 | 79 | ||
80 | render :text => "Ok! Please wait for the response (as specified by your response_type)" | 80 | render :text => "Ok! Please wait for the response (as specified by your response_type)" |
app/models/question.rb
@@ -512,36 +512,27 @@ class Question < ActiveRecord::Base | @@ -512,36 +512,27 @@ class Question < ActiveRecord::Base | ||
512 | end | 512 | end |
513 | 513 | ||
514 | 514 | ||
515 | - def export_and_delete(type, options={}) | ||
516 | - delete_at = options.delete(:delete_at) | ||
517 | - filename = export(type, options) | ||
518 | - | ||
519 | - File.send_at(delete_at, :delete, filename) | ||
520 | - filename | ||
521 | - end | ||
522 | - | ||
523 | def export(type, options = {}) | 515 | def export(type, options = {}) |
524 | 516 | ||
525 | case type | 517 | case type |
526 | when 'votes' | 518 | when 'votes' |
527 | - outfile = "ideamarketplace_#{self.id}_votes.csv" | 519 | + outfile = "ideamarketplace_#{self.id}_votes" |
528 | 520 | ||
529 | headers = ['Vote ID', 'Session ID', 'Ideamarketplace ID','Winner ID', 'Winner Text', 'Loser ID', 'Loser Text', 'Prompt ID', 'Left Choice ID', 'Right Choice ID', 'Created at', 'Updated at', 'Response Time (s)', 'Missing Response Time Explanation', 'Session Identifier', 'Valid'] | 521 | headers = ['Vote ID', 'Session ID', 'Ideamarketplace ID','Winner ID', 'Winner Text', 'Loser ID', 'Loser Text', 'Prompt ID', 'Left Choice ID', 'Right Choice ID', 'Created at', 'Updated at', 'Response Time (s)', 'Missing Response Time Explanation', 'Session Identifier', 'Valid'] |
530 | 522 | ||
531 | when 'ideas' | 523 | when 'ideas' |
532 | - outfile = "ideamarketplace_#{self.id}_ideas.csv" | 524 | + outfile = "ideamarketplace_#{self.id}_ideas" |
533 | headers = ['Ideamarketplace ID','Idea ID', 'Idea Text', 'Wins', 'Losses', 'Times involved in Cant Decide', 'Score', 'User Submitted', 'Session ID', 'Created at', 'Last Activity', 'Active', 'Appearances on Left', 'Appearances on Right'] | 525 | headers = ['Ideamarketplace ID','Idea ID', 'Idea Text', 'Wins', 'Losses', 'Times involved in Cant Decide', 'Score', 'User Submitted', 'Session ID', 'Created at', 'Last Activity', 'Active', 'Appearances on Left', 'Appearances on Right'] |
534 | when 'non_votes' | 526 | when 'non_votes' |
535 | - outfile = "ideamarketplace_#{self.id}_non_votes.csv" | 527 | + outfile = "ideamarketplace_#{self.id}_non_votes" |
536 | headers = ['Record Type', 'Record ID', 'Session ID', 'Ideamarketplace ID','Left Choice ID', 'Left Choice Text', 'Right Choice ID', 'Right Choice Text', 'Prompt ID', 'Reason', 'Created at', 'Updated at', 'Response Time (s)', 'Missing Response Time Explanation', 'Session Identifier', 'Valid'] | 528 | headers = ['Record Type', 'Record ID', 'Session ID', 'Ideamarketplace ID','Left Choice ID', 'Left Choice Text', 'Right Choice ID', 'Right Choice Text', 'Prompt ID', 'Reason', 'Created at', 'Updated at', 'Response Time (s)', 'Missing Response Time Explanation', 'Session Identifier', 'Valid'] |
537 | else | 529 | else |
538 | raise "Unsupported export type: #{type}" | 530 | raise "Unsupported export type: #{type}" |
539 | end | 531 | end |
540 | 532 | ||
541 | - filename = File.join(File.expand_path(Rails.root), "public", "system", "exports", self.id.to_s, Digest::SHA1.hexdigest(outfile + rand(10000000).to_s) + "_" + outfile) | 533 | + name = outfile + "_" + Digest::SHA1.hexdigest(outfile + rand(10000000).to_s) + ".csv" |
542 | 534 | ||
543 | - FileUtils::mkdir_p(File.dirname(filename)) | ||
544 | - csv_data = FasterCSV.open(filename, "w") do |csv| | 535 | + csv_data = FasterCSV.generate do |csv| |
545 | csv << headers | 536 | csv << headers |
546 | case type | 537 | case type |
547 | when 'votes' | 538 | when 'votes' |
@@ -600,18 +591,23 @@ class Question < ActiveRecord::Base | @@ -600,18 +591,23 @@ class Question < ActiveRecord::Base | ||
600 | 591 | ||
601 | end | 592 | end |
602 | 593 | ||
594 | + | ||
603 | if options[:response_type] == 'redis' | 595 | if options[:response_type] == 'redis' |
596 | + # let's compress this for redis | ||
597 | + # it should get removed from redis relatively quickly | ||
598 | + zlib = Zlib::Deflate.new | ||
599 | + zlibcsv = zlib.deflate(csv_data, Zlib::FINISH) | ||
600 | + zlib.close | ||
604 | 601 | ||
605 | if options[:redis_key].nil? | 602 | if options[:redis_key].nil? |
606 | raise "No :redis_key specified" | 603 | raise "No :redis_key specified" |
607 | end | 604 | end |
608 | #The client should use blpop to listen for a key | 605 | #The client should use blpop to listen for a key |
609 | #The client is responsible for deleting the redis key (auto expiration results failure in testing) | 606 | #The client is responsible for deleting the redis key (auto expiration results failure in testing) |
610 | - $redis.lpush(options[:redis_key], filename) | ||
611 | - #TODO implement response_type == 'email' for use by customers of the API (not local) | 607 | + $redis.lpush(options[:redis_key], zlibcsv) |
608 | + else | ||
609 | + return csv_data | ||
612 | end | 610 | end |
613 | - | ||
614 | - filename | ||
615 | end | 611 | end |
616 | 612 | ||
617 | def get_first_unanswered_appearance(visitor, offset=0) | 613 | def get_first_unanswered_appearance(visitor, offset=0) |
spec/models/question_spec.rb
@@ -365,86 +365,60 @@ describe Question do | @@ -365,86 +365,60 @@ describe Question do | ||
365 | 365 | ||
366 | 366 | ||
367 | it "should export vote data to a csv file" do | 367 | it "should export vote data to a csv file" do |
368 | - filename = @aoi_question.export('votes') | 368 | + csv = @aoi_question.export('votes') |
369 | 369 | ||
370 | - filename.should_not be nil | ||
371 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_votes[.]csv$/ | ||
372 | - File.exists?(filename).should be_true | ||
373 | # Not specifying exact file syntax, it's likely to change frequently | 370 | # Not specifying exact file syntax, it's likely to change frequently |
374 | # | 371 | # |
375 | - rows = FasterCSV.read(filename) | 372 | + rows = FasterCSV.parse(csv) |
376 | rows.first.should include("Vote ID") | 373 | rows.first.should include("Vote ID") |
377 | rows.first.should_not include("Idea ID") | 374 | rows.first.should_not include("Idea ID") |
378 | - File.delete(filename).should be_true | ||
379 | 375 | ||
380 | end | 376 | end |
381 | 377 | ||
382 | - it "should notify redis after completing an export, if redis option set" do | 378 | + it "should export zlibed csv to redis after completing an export, if redis option set" do |
383 | redis_key = "test_key123" | 379 | redis_key = "test_key123" |
384 | $redis.del(redis_key) # clear if key exists already | 380 | $redis.del(redis_key) # clear if key exists already |
385 | - filename = @aoi_question.export('votes', :response_type => 'redis', :redis_key => redis_key) | ||
386 | - | ||
387 | - filename.should_not be nil | ||
388 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_votes[.]csv$/ | ||
389 | - File.exists?(filename).should be_true | ||
390 | - $redis.lpop(redis_key).should == filename | 381 | + csv = @aoi_question.export('votes') |
382 | + @aoi_question.export('votes', :response_type => 'redis', :redis_key => redis_key) | ||
383 | + | ||
384 | + zlibcsv = $redis.lpop(redis_key) | ||
385 | + zstream = Zlib::Inflate.new | ||
386 | + buf = zstream.inflate(zlibcsv) | ||
387 | + zstream.finish | ||
388 | + zstream.close | ||
389 | + buf.should == csv | ||
391 | $redis.del(redis_key) # clean up | 390 | $redis.del(redis_key) # clean up |
392 | - File.delete(filename).should be_true | ||
393 | 391 | ||
394 | end | 392 | end |
395 | it "should email question owner after completing an export, if email option set" do | 393 | it "should email question owner after completing an export, if email option set" do |
396 | #TODO | 394 | #TODO |
397 | end | 395 | end |
398 | 396 | ||
399 | - it "should export non vote data to a csv file" do | ||
400 | - filename = @aoi_question.export('non_votes') | ||
401 | - | ||
402 | - filename.should_not be nil | ||
403 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_non_votes[.]csv$/ | ||
404 | - File.exists?(filename).should be_true | 397 | + it "should export non vote data to a string" do |
398 | + csv = @aoi_question.export('non_votes') | ||
405 | 399 | ||
406 | - # Not specifying exact file syntax, it's likely to change frequently | ||
407 | - # | ||
408 | - rows = FasterCSV.read(filename) | 400 | + rows = FasterCSV.parse(csv) |
409 | rows.first.should include("Record ID") | 401 | rows.first.should include("Record ID") |
410 | rows.first.should include("Record Type") | 402 | rows.first.should include("Record Type") |
411 | rows.first.should_not include("Idea ID") | 403 | rows.first.should_not include("Idea ID") |
412 | # ensure we have more than just the header row | 404 | # ensure we have more than just the header row |
413 | rows.length.should be > 1 | 405 | rows.length.should be > 1 |
414 | - File.delete(filename).should_not be_nil | ||
415 | - | ||
416 | - | ||
417 | end | 406 | end |
418 | 407 | ||
419 | - it "should export idea data to a csv file" do | ||
420 | - filename = @aoi_question.export('ideas') | 408 | + it "should export idea data to a string" do |
409 | + csv = @aoi_question.export('ideas') | ||
421 | 410 | ||
422 | - filename.should_not be nil | ||
423 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_ideas[.]csv$/ | ||
424 | - File.exists?(filename).should be_true | ||
425 | # Not specifying exact file syntax, it's likely to change frequently | 411 | # Not specifying exact file syntax, it's likely to change frequently |
426 | # | 412 | # |
427 | - rows = FasterCSV.read(filename) | 413 | + rows = FasterCSV.parse(csv) |
428 | rows.first.should include("Idea ID") | 414 | rows.first.should include("Idea ID") |
429 | rows.first.should_not include("Skip ID") | 415 | rows.first.should_not include("Skip ID") |
430 | - File.delete(filename).should_not be_nil | ||
431 | - | ||
432 | end | 416 | end |
433 | 417 | ||
434 | it "should raise an error when given an unsupported export type" do | 418 | it "should raise an error when given an unsupported export type" do |
435 | lambda { @aoi_question.export("blahblahblah") }.should raise_error | 419 | lambda { @aoi_question.export("blahblahblah") }.should raise_error |
436 | end | 420 | end |
437 | 421 | ||
438 | - it "should export data and schedule a job to delete export after X days" do | ||
439 | - Delayed::Job.delete_all | ||
440 | - filename = @aoi_question.export_and_delete('votes', :delete_at => 2.days.from_now) | ||
441 | - | ||
442 | - Delayed::Job.count.should == 1 | ||
443 | - Delayed::Job.delete_all | ||
444 | - File.delete(filename).should_not be_nil | ||
445 | - | ||
446 | - end | ||
447 | - | ||
448 | after(:all) { truncate_all } | 422 | after(:all) { truncate_all } |
449 | end | 423 | end |
450 | 424 | ||
@@ -499,15 +473,12 @@ describe Question do | @@ -499,15 +473,12 @@ describe Question do | ||
499 | end | 473 | end |
500 | end | 474 | end |
501 | 475 | ||
502 | - it "should export idea data to a csv file with proper escaping" do | ||
503 | - filename = @aoi_question.export('ideas') | 476 | + it "should export idea data to a string with proper escaping" do |
477 | + csv = @aoi_question.export('ideas') | ||
504 | 478 | ||
505 | - filename.should_not be nil | ||
506 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_ideas[.]csv$/ | ||
507 | - File.exists?(filename).should be_true | ||
508 | # Not specifying exact file syntax, it's likely to change frequently | 479 | # Not specifying exact file syntax, it's likely to change frequently |
509 | # | 480 | # |
510 | - rows = FasterCSV.read(filename) | 481 | + rows = FasterCSV.parse(csv) |
511 | rows.first.should include("Idea ID") | 482 | rows.first.should include("Idea ID") |
512 | rows.first.should_not include("Skip ID") | 483 | rows.first.should_not include("Skip ID") |
513 | 484 | ||
@@ -516,20 +487,14 @@ describe Question do | @@ -516,20 +487,14 @@ describe Question do | ||
516 | # Idea Text | 487 | # Idea Text |
517 | row[2].should =~ /^foo.bar$/m | 488 | row[2].should =~ /^foo.bar$/m |
518 | end | 489 | end |
519 | - | ||
520 | - File.delete(filename).should_not be_nil | ||
521 | - | ||
522 | end | 490 | end |
523 | 491 | ||
524 | - it "should export vote data to a csv file with proper escaping" do | ||
525 | - filename = @aoi_question.export('votes') | 492 | + it "should export vote data to a string with proper escaping" do |
493 | + csv = @aoi_question.export('votes') | ||
526 | 494 | ||
527 | - filename.should_not be nil | ||
528 | - filename.should match /.*ideamarketplace_#{@aoi_question.id}_votes[.]csv$/ | ||
529 | - File.exists?(filename).should be_true | ||
530 | # Not specifying exact file syntax, it's likely to change frequently | 495 | # Not specifying exact file syntax, it's likely to change frequently |
531 | # | 496 | # |
532 | - rows = FasterCSV.read(filename) | 497 | + rows = FasterCSV.parse(csv) |
533 | rows.first.should include("Vote ID") | 498 | rows.first.should include("Vote ID") |
534 | rows.first.should_not include("Idea ID") | 499 | rows.first.should_not include("Idea ID") |
535 | 500 | ||
@@ -540,7 +505,6 @@ describe Question do | @@ -540,7 +505,6 @@ describe Question do | ||
540 | # Loser Text | 505 | # Loser Text |
541 | row[6].should =~ /^foo.bar$/m | 506 | row[6].should =~ /^foo.bar$/m |
542 | end | 507 | end |
543 | - File.delete(filename).should be_true | ||
544 | 508 | ||
545 | end | 509 | end |
546 | 510 |