* VRFS-1100 - can only download 100 times before 404 given for client downloads, VRFS-862 - quick change to unblock

This commit is contained in:
Seth Call 2014-02-24 16:55:56 +00:00
parent bab83c91c1
commit 0522c20e60
24 changed files with 228 additions and 26 deletions

View File

@ -121,3 +121,4 @@ scores_mod_connections.sql
scores_create_schemas_and_extensions.sql scores_create_schemas_and_extensions.sql
scores_create_tables.sql scores_create_tables.sql
remove_is_downloadable.sql remove_is_downloadable.sql
track_download_counts.sql

View File

@ -0,0 +1,5 @@
ALTER TABLE recorded_tracks ADD COLUMN download_count INTEGER NOT NULL DEFAULT 0;
ALTER TABLE recorded_tracks ADD COLUMN last_downloaded_at TIMESTAMP;
ALTER TABLE mixes ADD COLUMN download_count INTEGER NOT NULL DEFAULT 0;
ALTER TABLE mixes ADD COLUMN last_downloaded_at TIMESTAMP;

View File

@ -20,6 +20,7 @@ module JamRuby
validates_uniqueness_of :user_id, :scope => :recording_id validates_uniqueness_of :user_id, :scope => :recording_id
validate :user_belongs_to_recording validate :user_belongs_to_recording
before_create :generate_share_token before_create :generate_share_token
SHARE_TOKEN_LENGTH = 8 SHARE_TOKEN_LENGTH = 8
@ -67,7 +68,6 @@ module JamRuby
!ClaimedRecording.find_by_user_id_and_recording_id(some_user.id, recording_id).nil? !ClaimedRecording.find_by_user_id_and_recording_id(some_user.id, recording_id).nil?
end end
def remove_non_alpha_num(token) def remove_non_alpha_num(token)
token.gsub(/[^0-9A-Za-z]/, '') token.gsub(/[^0-9A-Za-z]/, '')
end end

View File

@ -10,14 +10,24 @@ module JamRuby
attr_accessible :ogg_url, :should_retry, as: :admin attr_accessible :ogg_url, :should_retry, as: :admin
attr_accessor :is_skip_mount_uploader attr_accessor :is_skip_mount_uploader
attr_writer :current_user
belongs_to :recording, :class_name => "JamRuby::Recording", :inverse_of => :mixes, :foreign_key => 'recording_id' belongs_to :recording, :class_name => "JamRuby::Recording", :inverse_of => :mixes, :foreign_key => 'recording_id'
validates :download_count, presence: true
validate :verify_download_count
skip_callback :save, :before, :store_picture!, if: :is_skip_mount_uploader skip_callback :save, :before, :store_picture!, if: :is_skip_mount_uploader
mount_uploader :ogg_url, MixUploader mount_uploader :ogg_url, MixUploader
def verify_download_count
if (self.download_count < 0 || self.download_count > APP_CONFIG.max_audio_downloads) && !@current_user.admin
errors.add(:download_count, "must be less than or equal to 100")
end
end
before_validation do before_validation do
# this should be an activeadmin only path, because it's using the mount_uploader (whereas the client does something completely different) # this should be an activeadmin only path, because it's using the mount_uploader (whereas the client does something completely different)
if !is_skip_mount_uploader && ogg_url.present? && ogg_url.respond_to?(:file) && ogg_url_changed? if !is_skip_mount_uploader && ogg_url.present? && ogg_url.respond_to?(:file) && ogg_url_changed?
@ -67,7 +77,6 @@ module JamRuby
!ClaimedRecording.find_by_user_id_and_recording_id(some_user.id, recording_id).nil? !ClaimedRecording.find_by_user_id_and_recording_id(some_user.id, recording_id).nil?
end end
def errored(reason, detail) def errored(reason, detail)
self.error_reason = reason self.error_reason = reason
self.error_detail = detail self.error_detail = detail
@ -148,6 +157,11 @@ module JamRuby
Mix.construct_filename(self.created_at, self.recording_id, self.id, type) Mix.construct_filename(self.created_at, self.recording_id, self.id, type)
end end
def update_download_count(count=1)
self.download_count = self.download_count + count
self.last_downloaded_at = Time.now
end
private private
def delete_s3_files def delete_s3_files

View File

@ -12,6 +12,7 @@ module JamRuby
attr_writer :is_skip_mount_uploader attr_writer :is_skip_mount_uploader
attr_accessible :discard, :user, :user_id, :instrument_id, :sound, :client_id, :track_id, :client_track_id, :url, as: :admin attr_accessible :discard, :user, :user_id, :instrument_id, :sound, :client_id, :track_id, :client_track_id, :url, as: :admin
attr_writer :current_user
SOUND = %w(mono stereo) SOUND = %w(mono stereo)
MAX_PART_FAILURES = 3 MAX_PART_FAILURES = 3
@ -31,11 +32,13 @@ module JamRuby
validates :length, length: {minimum: 1, maximum: 1024 * 1024 * 256 }, if: :upload_starting? # 256 megs max. is this reasonable? surely... validates :length, length: {minimum: 1, maximum: 1024 * 1024 * 256 }, if: :upload_starting? # 256 megs max. is this reasonable? surely...
validates :user, presence: true validates :user, presence: true
validates :instrument, presence: true validates :instrument, presence: true
validates :download_count, presence: true
before_destroy :delete_s3_files before_destroy :delete_s3_files
validate :validate_fully_uploaded validate :validate_fully_uploaded
validate :validate_part_complete validate :validate_part_complete
validate :validate_too_many_upload_failures validate :validate_too_many_upload_failures
validate :verify_download_count
before_save :sanitize_active_admin before_save :sanitize_active_admin
skip_callback :save, :before, :store_picture!, if: :is_skip_mount_uploader? skip_callback :save, :before, :store_picture!, if: :is_skip_mount_uploader?
@ -97,6 +100,12 @@ module JamRuby
end end
end end
def verify_download_count
if (self.download_count < 0 || self.download_count > APP_CONFIG.max_audio_downloads) && !@current_user.admin
errors.add(:download_count, "must be less than or equal to 100")
end
end
def sanitize_active_admin def sanitize_active_admin
self.user_id = nil if self.user_id == '' self.user_id = nil if self.user_id == ''
end end
@ -187,6 +196,11 @@ module JamRuby
RecordedTrack.construct_filename(self.created_at, self.recording.id, self.client_track_id) RecordedTrack.construct_filename(self.created_at, self.recording.id, self.client_track_id)
end end
def update_download_count(count=1)
self.download_count = self.download_count + count
self.last_downloaded_at = Time.now
end
private private
def delete_s3_files def delete_s3_files

View File

@ -171,10 +171,28 @@ FactoryGirl.define do
association :user, factory: :user association :user, factory: :user
before(:create) { |claimed_recording| before(:create) { |claimed_recording|
claimed_recording.recording = FactoryGirl.create(:recording_with_track, owner: claimed_recording.user) unless claimed_recording.recording claimed_recording.recording = FactoryGirl.create(:recording_with_track, owner: claimed_recording.user) unless claimed_recording.recording
} }
end
factory :mix, :class => JamRuby::Mix do
started_at Time.now
completed_at Time.now
ogg_md5 'abc'
ogg_length 1
sequence(:ogg_url) { |n| "recordings/ogg/#{n}" }
mp3_md5 'abc'
mp3_length 1
sequence(:mp3_url) { |n| "recordings/mp3/#{n}" }
completed true
before(:create) {|mix|
user = FactoryGirl.create(:user)
mix.recording = FactoryGirl.create(:recording_with_track, owner: user)
mix.recording.claimed_recordings << FactoryGirl.create(:claimed_recording, user: user, recording: mix.recording)
}
end end
factory :musician_instrument, :class => JamRuby::MusicianInstrument do factory :musician_instrument, :class => JamRuby::MusicianInstrument do

View File

@ -63,6 +63,16 @@ describe Mix do
recordings.length.should == 0 recordings.length.should == 0
end end
describe "download count" do
it "will fail if too high" do
mix = FactoryGirl.create(:mix)
mix.current_user = mix.recording.owner
mix.update_download_count(APP_CONFIG.max_audio_downloads + 1)
mix.save
mix.errors[:download_count].should == ["must be less than or equal to 100"]
end
end
end end

View File

@ -13,6 +13,9 @@ SpecDb::recreate_database
# initialize ActiveRecord's db connection # initialize ActiveRecord's db connection
ActiveRecord::Base.establish_connection(YAML::load(File.open('config/database.yml'))["test"]) ActiveRecord::Base.establish_connection(YAML::load(File.open('config/database.yml'))["test"])
# so jam_ruby models that use APP_CONFIG in metadata will load. this is later stubbed pre test run
APP_CONFIG = app_config
require 'jam_ruby' require 'jam_ruby'
require 'factory_girl' require 'factory_girl'
require 'rubygems' require 'rubygems'

View File

@ -101,6 +101,10 @@ def app_config
'315576000' '315576000'
end end
def max_audio_downloads
100
end
private private
def audiomixer_workspace_path def audiomixer_workspace_path

View File

@ -487,7 +487,7 @@
* Load available drivers and populate the driver select box. * Load available drivers and populate the driver select box.
*/ */
function loadAudioDrivers() { function loadAudioDrivers() {
var drivers = jamClient.FTUEGetDevices(); var drivers = jamClient.FTUEGetDevices(false);
var driverOptionFunc = function (driverKey, index, list) { var driverOptionFunc = function (driverKey, index, list) {
optionsHtml += '<option title="' + drivers[driverKey] + '"value="' + driverKey + '">' + optionsHtml += '<option title="' + drivers[driverKey] + '"value="' + driverKey + '">' +

View File

@ -12,13 +12,6 @@
var entity = null; var entity = null;
var remainingCap = 140 - 22 - 1; // 140 tweet max, minus 22 for link size, minus 1 for space var remainingCap = 140 - 22 - 1; // 140 tweet max, minus 22 for link size, minus 1 for space
var textMap = {
LIVE_SESSION: "LIVE SESSION",
SESSION: "SESSION",
RECORDING: "RECORDING",
RECORDED: "RECORDED"
};
function showSpinner() { function showSpinner() {
$(dialogId + ' .dialog-inner').hide(); $(dialogId + ' .dialog-inner').hide();
var spinner = $('<div class="spinner spinner-large"></div>') var spinner = $('<div class="spinner spinner-large"></div>')

View File

@ -33,8 +33,7 @@ class ApiController < ApplicationController
def respond_with_model(model, options = {}) def respond_with_model(model, options = {})
if model.errors.any? if model.errors.any?
response.status = :unprocessable_entity respond_with model, status: :unprocessable_entity, layout: nil
respond_with model, layout: nil
else else
status = options[:new] && options[:new] == true ? 201 : 200 status = options[:new] && options[:new] == true ? 201 : 200
redirect_on_success = options[:location] redirect_on_success = options[:location]

View File

@ -24,9 +24,18 @@ class ApiMixesController < ApiController
def download def download
@mix = Mix.find(params[:id]) @mix = Mix.find(params[:id])
raise PermissionError, "You can only download a mix you didn't claim" unless @mix.can_download? current_user raise PermissionError, "You can only download a mix you have claimed" unless @mix.can_download? current_user
redirect_to @mix.sign_url @mix.current_user = current_user
@mix.update_download_count
@mix.valid?
if !@mix.errors.any?
@mix.save!
redirect_to @mix.sign_url
else
render :json => { :message => "download limit surpassed" }, :status => 404
end
end end
private private

View File

@ -38,7 +38,18 @@ class ApiRecordingsController < ApiController
def download def download
raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless @recorded_track.can_download?(current_user) raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless @recorded_track.can_download?(current_user)
redirect_to @recorded_track.sign_url @recorded_track.current_user = current_user
@recorded_track.update_download_count
@recorded_track.valid?
if !@recorded_track.errors.any?
@recorded_track.save!
redirect_to @recorded_track.sign_url
else
render :json => { :message => "download limit surpassed" }, :status => 404
end
end end
def start def start

View File

@ -0,0 +1 @@
object @mix

View File

@ -69,7 +69,6 @@
</div> </div>
<%= render "clients/invitationDialog" %> <%= render "clients/invitationDialog" %>
<%= render "clients/shareDialog" %>
<%= render "users/signupDialog" %> <%= render "users/signupDialog" %>
<%= render "users/signinDialog" %> <%= render "users/signinDialog" %>
<%= render "users/videoDialog" %> <%= render "users/videoDialog" %>

View File

@ -204,6 +204,8 @@ if defined?(Bundler)
config.twitter_app_id = ENV['TWITTER_APP_ID'] || 'nQj2oEeoJZxECC33tiTuIg' config.twitter_app_id = ENV['TWITTER_APP_ID'] || 'nQj2oEeoJZxECC33tiTuIg'
config.twitter_app_secret = ENV['TWITTER_APP_SECRET'] || 'Azcy3QqfzYzn2fsojFPYXcn72yfwa0vG6wWDrZ3KT8' config.twitter_app_secret = ENV['TWITTER_APP_SECRET'] || 'Azcy3QqfzYzn2fsojFPYXcn72yfwa0vG6wWDrZ3KT8'
config.autocheck_create_session_agreement = false; config.autocheck_create_session_agreement = false
config.max_audio_downloads = 100
end end
end end

View File

@ -3,7 +3,6 @@ require 'spec_helper'
describe ApiCorporateController do describe ApiCorporateController do
render_views render_views
before(:each) do before(:each) do
CorpMailer.deliveries.clear CorpMailer.deliveries.clear
end end

View File

@ -0,0 +1,53 @@
require 'spec_helper'
describe ApiMixesController do
render_views
let(:mix) { FactoryGirl.create(:mix) }
before(:each) do
controller.current_user = nil
end
describe "download" do
it "is possible" do
controller.current_user = mix.recording.owner
get :download, {id: mix.id}
response.status.should == 302
mix.reload
mix.download_count.should == 1
get :download, {id: mix.id}
response.status.should == 302
mix.reload
mix.download_count.should == 2
end
it "prevents download after limit is reached" do
mix.download_count = APP_CONFIG.max_audio_downloads
mix.save!
controller.current_user = mix.recording.owner
get :download, {format:'json', id: mix.id}
response.status.should == 404
JSON.parse(response.body, symbolize_names: true)[:message].should == "download limit surpassed"
end
it "lets admins surpass limit" do
mix.download_count = APP_CONFIG.max_audio_downloads
mix.save!
mix.recording.owner.admin = true
mix.recording.owner.save!
controller.current_user = mix.recording.owner
get :download, {format:'json', id: mix.id}
response.status.should == 302
mix.reload
mix.download_count.should == 101
end
end
end

View File

@ -101,6 +101,8 @@ describe ApiRecordingsController do
end end
describe "download" do describe "download" do
let(:mix) { FactoryGirl.create(:mix) }
it "should only allow a user to download a track if they have claimed the recording" do it "should only allow a user to download a track if they have claimed the recording" do
post :start, { :format => 'json', :music_session_id => @music_session.id } post :start, { :format => 'json', :music_session_id => @music_session.id }
response_body = JSON.parse(response.body) response_body = JSON.parse(response.body)
@ -108,5 +110,51 @@ describe ApiRecordingsController do
post :stop, { :format => 'json', :id => recording.id } post :stop, { :format => 'json', :id => recording.id }
response.should be_success response.should be_success
end end
it "is possible" do
mix.touch
recorded_track = mix.recording.recorded_tracks[0]
controller.current_user = mix.recording.owner
get :download, {id: recorded_track.recording.id, track_id: recorded_track.client_track_id}
response.status.should == 302
recorded_track.reload
recorded_track.download_count.should == 1
get :download, {id: recorded_track.recording.id, track_id: recorded_track.client_track_id}
response.status.should == 302
recorded_track.reload
recorded_track.download_count.should == 2
end
it "prevents download after limit is reached" do
mix.touch
recorded_track = mix.recording.recorded_tracks[0]
recorded_track.download_count = APP_CONFIG.max_audio_downloads
recorded_track.save!
controller.current_user = recorded_track.user
get :download, {format:'json', id: recorded_track.recording.id, track_id: recorded_track.client_track_id}
response.status.should == 404
JSON.parse(response.body, symbolize_names: true)[:message].should == "download limit surpassed"
end
it "lets admins surpass limit" do
mix.touch
recorded_track = mix.recording.recorded_tracks[0]
recorded_track.download_count = APP_CONFIG.max_audio_downloads
recorded_track.save!
recorded_track.user.admin = true
recorded_track.user.save!
controller.current_user = recorded_track.user
get :download, {format:'json', id: recorded_track.recording.id, track_id: recorded_track.client_track_id}
response.status.should == 302
recorded_track.reload
recorded_track.download_count.should == 101
end
end end
end end

View File

@ -374,4 +374,23 @@ FactoryGirl.define do
factory :music_session_like, :class => JamRuby::MusicSessionLiker do factory :music_session_like, :class => JamRuby::MusicSessionLiker do
end end
factory :mix, :class => JamRuby::Mix do
started_at Time.now
completed_at Time.now
ogg_md5 'abc'
ogg_length 1
sequence(:ogg_url) { |n| "recordings/ogg/#{n}" }
mp3_md5 'abc'
mp3_length 1
sequence(:mp3_url) { |n| "recordings/mp3/#{n}" }
completed true
before(:create) {|mix|
user = FactoryGirl.create(:user)
mix.recording = FactoryGirl.create(:recording_with_track, owner: user)
mix.recording.claimed_recordings << FactoryGirl.create(:claimed_recording, user: user, recording: mix.recording)
}
end
end end

View File

@ -39,7 +39,7 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature
in_client(creator) do in_client(creator) do
find('#session-leave').trigger(:click) find('#session-leave').trigger(:click)
find('#btn-accept').trigger(:click) find('#btn-accept-leave-session').trigger(:click)
expect(page).to have_selector('h2', text: 'feed') expect(page).to have_selector('h2', text: 'feed')
end end
@ -76,7 +76,7 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature
in_client(creator) do in_client(creator) do
find('#session-leave').trigger(:click) find('#session-leave').trigger(:click)
find('#btn-accept').trigger(:click) find('#btn-accept-leave-session').trigger(:click)
expect(page).to have_selector('h2', text: 'feed') expect(page).to have_selector('h2', text: 'feed')
end end

View File

@ -1,6 +1,6 @@
require 'simplecov' require 'simplecov'
require 'rubygems' require 'rubygems'
require 'spork' #require 'spork'
require 'omniauth' require 'omniauth'
#uncomment the following line to use spork with the debugger #uncomment the following line to use spork with the debugger
#require 'spork/ext/ruby-debug' #require 'spork/ext/ruby-debug'
@ -40,7 +40,7 @@ Thread.new {
end end
} }
Spork.prefork do #Spork.prefork do
# Loading more in this block will cause your tests to run faster. However, # Loading more in this block will cause your tests to run faster. However,
# if you change any configuration or code from libraries loaded here, you'll # if you change any configuration or code from libraries loaded here, you'll
# need to restart spork for it take effect. # need to restart spork for it take effect.
@ -155,12 +155,12 @@ Spork.prefork do
wipe_s3_test_bucket wipe_s3_test_bucket
end end
end end
end #end
Spork.each_run do #Spork.each_run do
# This code will be run each time you run your specs. # This code will be run each time you run your specs.
end #end