From c77589354939517dc8ea3d63c4ffd6855ef82702 Mon Sep 17 00:00:00 2001 From: Fernando Espinosa Date: Mon, 2 Jul 2012 17:51:30 +0200 Subject: [PATCH] Moves upload controller to api/imports namespace --- .../json/imports/uploads_controller.rb} | 6 +- app/models/user.rb | 14 ++-- config/routes.rb | 17 ++-- lib/resque/importer_jobs.rb | 18 ++++ spec/acceptance/api/tables_spec.rb | 84 +++++++++---------- spec/controllers/upload_controller_spec.rb | 52 ------------ spec/requests/api_authentication_spec.rb | 20 ++--- spec/requests/imports/uploads_spec.rb | 37 ++++++++ spec/support/factories/users.rb | 2 +- spec/support/helpers.rb | 4 +- 10 files changed, 131 insertions(+), 123 deletions(-) rename app/controllers/{upload_controller.rb => api/json/imports/uploads_controller.rb} (92%) create mode 100644 lib/resque/importer_jobs.rb delete mode 100644 spec/controllers/upload_controller_spec.rb create mode 100644 spec/requests/imports/uploads_spec.rb diff --git a/app/controllers/upload_controller.rb b/app/controllers/api/json/imports/uploads_controller.rb similarity index 92% rename from app/controllers/upload_controller.rb rename to app/controllers/api/json/imports/uploads_controller.rb index 90929b1bae..0036a11757 100644 --- a/app/controllers/upload_controller.rb +++ b/app/controllers/api/json/imports/uploads_controller.rb @@ -1,6 +1,4 @@ -# coding: UTF-8 - -class UploadController < ApplicationController +class Api::Json::Imports::UploadsController < Api::ApplicationController if Rails.env.production? ssl_required :create @@ -8,7 +6,6 @@ class UploadController < ApplicationController skip_before_filter :verify_authenticity_token before_filter :api_or_user_authorization_required - skip_before_filter :check_domain def create begin @@ -43,4 +40,5 @@ class UploadController < ApplicationController api_authorization_required || login_required end private :api_or_user_authorization_required + end diff --git a/app/models/user.rb b/app/models/user.rb index e2e0c0248e..ddae1a38cf 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -239,14 +239,14 @@ class User < Sequel::Model def get_map_key $users_metadata.HMGET(key, 'map_key').first end - + def regenerate_map_key # GET CURRENT KEY old_key = self.get_map_key - + # SET NEW KEY self.set_map_key - + # REMOVE OLD KEY FROM KEY SET $users_metadata.SREM "#{key}:map_key", old_key end @@ -375,9 +375,9 @@ class User < Sequel::Model puts "Loading functions in db '#{database_name}' (#{username})" in_database(:as => :superuser) do |user_database| user_database.transaction do - glob = RAILS_ROOT + '/lib/sql/*.sql' + glob = Rails.root.join('/lib/sql/*.sql') - Dir.glob(glob).each do |f| + Dir.glob(glob).each do |f| @sql = File.new(f).read user_database.run(@sql) end @@ -392,10 +392,10 @@ class User < Sequel::Model user_database.transaction do config = ::Rails::Sequel.configuration.environment_for(Rails.env) env = " PGUSER=#{database_username}" - env += " PGPORT=#{config['port']}" + env += " PGPORT=#{config['port']}" env += " PGHOST=#{config['host']}" env += " PGPASSWORD=#{database_password}" - glob = RAILS_ROOT + '/lib/sql/test/*.sql' + glob = Rails.root.join('/lib/sql/test/*.sql') #puts " Scanning #{glob}" Dir.glob(glob).each do |f| tname = File.basename(f, '.sql') diff --git a/config/routes.rb b/config/routes.rb index 35420cb35d..e1d0b5e238 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,11 +14,11 @@ CartoDB::Application.routes.draw do match '/dashboard' => 'tables#index', :as => :dashboard # TO IMPLEMENT #match '/dashboard/public' => 'tables#index_public', :as => :dashboard_public - + resources :tables, :only => [:show] do get 'embed_map', :on => :member get 'public' => 'tables#show_public', :on => :member - end + end match '/your_apps/oauth' => 'client_applications#oauth', :as => :oauth_credentials match '/your_apps/api_key' => 'client_applications#api_key', :as => :api_key_credentials post '/your_apps/api_key/regenerate' => 'client_applications#regenerate_api_key', :as => :regenerate_api_key @@ -29,7 +29,7 @@ CartoDB::Application.routes.draw do post '/' => 'users#create', :as => :users resources :users, :only => [:create, :update, :destroy] end - + scope :oauth, :path => :oauth do match '/authorize' => 'oauth#authorize', :as => :authorize match '/request_token' => 'oauth#request_token', :as => :request_token @@ -37,7 +37,7 @@ CartoDB::Application.routes.draw do get '/identity' => 'sessions#show' end - scope "/api" do + scope "/api" do namespace CartoDB::API::VERSION_1, :format => :json, :module => "api/json" do get '/column_types' => 'meta#column_types' get '/tables' => 'tables#index' @@ -56,7 +56,7 @@ CartoDB::Application.routes.draw do get '/tables/:table_id/export/shp' => 'export_tables#show', :format => :shp get '/tables/:table_id/export/kml' => 'export_tables#show', :format => :kml get '/tables/:table_id/export/sql' => 'export_tables#show', :format => :sql - + get '/tables/:table_id/records' => 'records#index' post '/tables/:table_id/records' => 'records#create' get '/tables/:table_id/records/pending_addresses' => 'records#pending_addresses' @@ -71,6 +71,11 @@ CartoDB::Application.routes.draw do get '/tables/:table_id/records/:record_id/columns/:id' => 'records#show_column' put '/tables/:table_id/records/:record_id/columns/:id' => 'records#update_column' get '/queries' => 'queries#run' + + namespace 'imports' do + post '/uploads' => 'uploads#create' + end + end end -end \ No newline at end of file +end diff --git a/lib/resque/importer_jobs.rb b/lib/resque/importer_jobs.rb new file mode 100644 index 0000000000..364135c7ea --- /dev/null +++ b/lib/resque/importer_jobs.rb @@ -0,0 +1,18 @@ +module Resque + module ImporterJobs + @queue = :import_jobs + + def self.before_perform + + end + + def self.perform(table) + table.save! + end + + def self.after_perform + + end + + end +end diff --git a/spec/acceptance/api/tables_spec.rb b/spec/acceptance/api/tables_spec.rb index c865ceca35..b8d0281b4a 100644 --- a/spec/acceptance/api/tables_spec.rb +++ b/spec/acceptance/api/tables_spec.rb @@ -39,7 +39,7 @@ feature "API 1.0 tables management" do response.body[:tables].size.should == 2 end end - + scenario "Get tables from a tag" do another_user = create_user @@ -63,12 +63,12 @@ feature "API 1.0 tables management" do response.status.should be_success response.body[:tables].map{ |t| t['name'] }.should == ["my_table_1"] end - + get_json api_tables_tag_url("tag 2", :page => 1, :per_page => 10) do |response| response.status.should be_success response.body[:tables].map{ |t| t['name'] }.should == ["my_table_1"] end - + get_json api_tables_tag_url("tag 4") do |response| response.status.should be_success response.body[:tables].map{ |t| t['name'] }.should be_empty @@ -87,7 +87,7 @@ feature "API 1.0 tables management" do scenario "Create a new table specifing a name and a schema" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, {:name => "My new imported table", :schema => "bla bla blat"} do |response| response.status.should == 400 end @@ -101,10 +101,10 @@ feature "API 1.0 tables management" do ]).should be_empty end end - + scenario "Create a new table specifying a geometry of type point" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, {:name => "My new imported table", :the_geom_type => "Point" } do |response| response.status.should be_success response.body[:name].should == "my_new_imported_table" @@ -117,7 +117,7 @@ feature "API 1.0 tables management" do scenario "Create a new table specifying a geometry of type polygon" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, {:name => "My new imported table", :the_geom_type => "Polygon" } do |response| response.status.should be_success response.body[:name].should == "my_new_imported_table" @@ -127,10 +127,10 @@ feature "API 1.0 tables management" do ]).should be_empty end end - + scenario "Create a new table specifying a geometry of type line" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, {:name => "My new imported table", :the_geom_type => "Line" } do |response| response.status.should be_success response.body[:name].should == "my_new_imported_table" @@ -143,7 +143,7 @@ feature "API 1.0 tables management" do scenario "Create a new table specifing an schema and a file from which import data" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, { :name => "Twitts", :schema => "url varchar(255) not null, login varchar(255), country varchar(255), \"followers count\" integer, foo varchar(255)", @@ -193,7 +193,7 @@ feature "API 1.0 tables management" do put_json api_table_url(table1.name), {:name => ""} do |response| response.status.should be_success end - + get_json api_table_url(table1.name) do |response| response.status.should be_success end @@ -201,7 +201,7 @@ feature "API 1.0 tables management" do scenario "Delete a table of mine" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + table1 = create_table :user_id => @user.id, :name => 'My table #1', :privacy => Table::PRIVATE, :tags => "tag 1, tag 2,tag 3, tag 3" delete_json api_table_url(table1.name) do |response| @@ -211,7 +211,7 @@ feature "API 1.0 tables management" do scenario "Delete a table of another user" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).never - + another_user = create_user table1 = create_table :user_id => another_user.id, :name => 'My table #1', :privacy => Table::PRIVATE, :tags => "tag 1, tag 2,tag 3, tag 3" @@ -272,17 +272,17 @@ feature "API 1.0 tables management" do response.body[:schema].should include(["aggregated_address", "string", "address"]) end end - + scenario "Create a new table importing file twitters.csv" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, { :file => "#{Rails.root}/db/fake_data/twitters.csv" } do |response| response.status.should be_success response.body[:name].should == "twitters" schema_differences = (response.body[:schema] - [ - ["cartodb_id", "number"], ["url", "string"], ["login", "string"], ["country", "string"], ["followers_count", "string"], + ["cartodb_id", "number"], ["url", "string"], ["login", "string"], ["country", "string"], ["followers_count", "string"], ["field_5", "string"], ["created_at", "date"], ["updated_at", "date"], ["the_geom", "geometry", "geometry", "point"] ]) @@ -292,29 +292,29 @@ feature "API 1.0 tables management" do scenario "Create a new table importing file ngos.xlsx" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, { :file => "#{Rails.root}/db/fake_data/ngos.xlsx" } do |response| response.status.should be_success response.body[:name].should == "ngos" schema_differences = (response.body[:schema] - [ - ["cartodb_id", "number"], ["organization", "string"], ["website", "string"], ["about", "string"], ["organization_s_work_in_haiti", "string"], - ["calculation_of_number_of_people_reached", "string"], ["private_funding", "string"], ["relief", "string"], ["reconstruction", "string"], - ["private_funding_spent", "string"], ["spent_on_relief", "string"], ["spent_on_reconstruction", "string"], ["usg_funding", "string"], - ["usg_funding_spent", "string"], ["other_funding", "string"], ["other_funding_spent", "string"], ["international_staff", "string"], ["national_staff", "string"], - ["us_contact_name", "string"], ["us_contact_title", "string"], ["us_contact_phone", "string"], ["us_contact_e_mail", "string"], ["media_contact_name", "string"], - ["media_contact_title", "string"], ["media_contact_phone", "string"], ["media_contact_e_mail", "string"], ["donation_phone_number", "string"], ["donation_address_line_1", "string"], + ["cartodb_id", "number"], ["organization", "string"], ["website", "string"], ["about", "string"], ["organization_s_work_in_haiti", "string"], + ["calculation_of_number_of_people_reached", "string"], ["private_funding", "string"], ["relief", "string"], ["reconstruction", "string"], + ["private_funding_spent", "string"], ["spent_on_relief", "string"], ["spent_on_reconstruction", "string"], ["usg_funding", "string"], + ["usg_funding_spent", "string"], ["other_funding", "string"], ["other_funding_spent", "string"], ["international_staff", "string"], ["national_staff", "string"], + ["us_contact_name", "string"], ["us_contact_title", "string"], ["us_contact_phone", "string"], ["us_contact_e_mail", "string"], ["media_contact_name", "string"], + ["media_contact_title", "string"], ["media_contact_phone", "string"], ["media_contact_e_mail", "string"], ["donation_phone_number", "string"], ["donation_address_line_1", "string"], ["address_line_2", "string"], ["city", "string"], ["state", "string"], ["zip_code", "string"], ["donation_website", "string"], ["created_at", "date"], ["updated_at", "date"], ["the_geom", "geometry", "geometry", "point"] ]) schema_differences.should be_empty, "difference: #{schema_differences.inspect}" end end - + scenario "Create a new table importing file EjemploVizzuality.zip" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, { :file => "#{Rails.root}/db/fake_data/EjemploVizzuality.zip", :srid => CartoDB::SRID @@ -322,18 +322,18 @@ feature "API 1.0 tables management" do response.status.should be_success response.body[:name].should == "vizzuality" schema_differences = (response.body[:schema] - [ - ["cartodb_id", "number"], ["gid", "number"], ["subclass", "string"], ["x", "number"], ["y", "number"], ["length", "string"], ["area", "string"], - ["angle", "number"], ["name", "string"], ["pid", "number"], ["lot_navteq", "string"], ["version_na", "string"], ["vitesse_sp", "number"], - ["id", "number"], ["nombrerest", "string"], ["tipocomida", "string"], + ["cartodb_id", "number"], ["gid", "number"], ["subclass", "string"], ["x", "number"], ["y", "number"], ["length", "string"], ["area", "string"], + ["angle", "number"], ["name", "string"], ["pid", "number"], ["lot_navteq", "string"], ["version_na", "string"], ["vitesse_sp", "number"], + ["id", "number"], ["nombrerest", "string"], ["tipocomida", "string"], ["the_geom", "geometry", "geometry", "multipolygon"], ["created_at", "date"], ["updated_at", "date"] ]) schema_differences.should be_empty, "difference: #{schema_differences.inspect}" - end + end end - + scenario "Create a new table importing file shp not working.zip" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).once - + post_json api_tables_url, { :file => "#{Rails.root}/db/fake_data/shp\ not\ working.zip", :srid => CartoDB::SRID @@ -341,30 +341,30 @@ feature "API 1.0 tables management" do response.status.should be_success response.body[:name].should == "constru" schema_differences = (response.body[:schema] - [ - ["cartodb_id", "number"], ["gid", "number"], ["mapa", "number"], ["delegacio", "number"], ["municipio", "number"], ["masa", "string"], - ["tipo", "string"], ["parcela", "string"], ["constru", "string"], ["coorx", "number"], ["coory", "number"], ["numsymbol", "number"], - ["area", "number"], ["fechaalta", "number"], ["fechabaja", "number"], ["ninterno", "number"], ["hoja", "string"], ["refcat", "string"], + ["cartodb_id", "number"], ["gid", "number"], ["mapa", "number"], ["delegacio", "number"], ["municipio", "number"], ["masa", "string"], + ["tipo", "string"], ["parcela", "string"], ["constru", "string"], ["coorx", "number"], ["coory", "number"], ["numsymbol", "number"], + ["area", "number"], ["fechaalta", "number"], ["fechabaja", "number"], ["ninterno", "number"], ["hoja", "string"], ["refcat", "string"], ["the_geom", "geometry", "geometry", "multipolygon"], ["created_at", "date"], ["updated_at", "date"] ]) schema_differences.should be_empty, "difference: #{schema_differences.inspect}" - end + end end - - + + scenario "Create a table, remove a table, and recreate it with the same name" do CartoDB::QueriesThreshold.expects(:incr).with(@user.id, "other", any_parameters).times(3) - + post_json api_tables_url, {:file => "#{Rails.root}/db/fake_data/twitters.csv"} do |response| response.status.should be_success - end - + end + delete_json api_table_url("twitters") do |response| response.status.should be_success end post_json api_tables_url, {:name => "wadus", :file => "#{Rails.root}/db/fake_data/twitters.csv"} do |response| response.status.should be_success - end + end end scenario "Download a table in shp format" do @@ -398,4 +398,4 @@ feature "API 1.0 tables management" do end end -end \ No newline at end of file +end diff --git a/spec/controllers/upload_controller_spec.rb b/spec/controllers/upload_controller_spec.rb deleted file mode 100644 index 7ec04c1325..0000000000 --- a/spec/controllers/upload_controller_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'spec_helper' - -describe UploadController do - - describe "POST create" do - - it "stores data from request body as a temp file" do - request.env['warden'] = stub( - :authenticated? => true, - :authenticate! => true, - :user => stub(:id => 1, :username => 'test')) - request.host = 'test.localhost.lan' - - file = File.open(Rails.root.join('spec/support/whs_features.csv'), 'r+') - - post :create, :body => file.read, :qqfile => File.basename(file.path) - - response.code.should be == '200' - puts response.body - - response_json = JSON.parse(response.body) - response_json.should_not be_nil - response_json['file_uri'].should match(/uploads\/.*\/whs_features.csv/) - response_json['success'].should be_true - - end - - it "stores data from 'file' param as a temp file" do - request.env['warden'] = stub( - :authenticated? => true, - :authenticate! => true, - :user => stub(:id => 1, :username => 'test')) - request.host = 'test.localhost.lan' - - file = File.open(Rails.root.join('spec/support/whs_features.csv'), 'r+') - file = ActionDispatch::Http::UploadedFile.new(:tempfile => file, :filename => File.basename(file)) - - post :create, :file => file - - response.code.should be == '200' - - response_json = JSON.parse(response.body) - response_json.should_not be_nil - response_json['file_uri'].should match(/uploads\/.*\/whs_features.csv/) - response_json['success'].should be_true - end - - end - -end - - diff --git a/spec/requests/api_authentication_spec.rb b/spec/requests/api_authentication_spec.rb index c12d00aa7d..baf6abe914 100644 --- a/spec/requests/api_authentication_spec.rb +++ b/spec/requests/api_authentication_spec.rb @@ -9,20 +9,20 @@ describe "API Authentication" do }) @access_token = AccessToken.create(:user => @user, :client_application => @user.client_application) end - + it "should not authorize requests without signature" do get "http://vizzuality.testhost.lan/api/v1/tables" status.should == 401 end describe "Standard OAuth" do - + before(:each) do # We need to specify the complete url in both the prepare_oauth_request method call which we use to build a request from which to take the Authorization header # and also when making the request otherwise get/post integration test methods will use example.org @request_url = "http://vizzuality.testhost.lan/api/v1/tables" end - + it "should authorize requests properly signed" do req = prepare_oauth_request(@oauth_consumer, @request_url, :token => @access_token) get @request_url, {}, {"Authorization" => req["Authorization"]} @@ -41,29 +41,29 @@ describe "API Authentication" do @request_url = "http://vizzuality.testhost.lan/oauth/access_token" @xauth_params = { :x_auth_username => @user.email, :x_auth_password => "clientex", :x_auth_mode => 'client_auth' } end - + it "should return 401 if request is not signed" do post @request_url, @xauth_params status.should == 401 end - + it "should not return an access token with invalid xAuth params" do @xauth_params.merge!(:x_auth_password => "invalid") req = prepare_oauth_request(@oauth_consumer, @request_url, :form_data => @xauth_params) - + post @request_url, @xauth_params, {"Authorization" => req["Authorization"]} status.should == 401 end - + it "should return access tokens with valid xAuth params" do # Not exactly sure why requests come with SERVER_NAME = "example.org" req = prepare_oauth_request(@oauth_consumer, @request_url, :form_data => @xauth_params) - + post @request_url, @xauth_params, {"Authorization" => req["Authorization"]} status.should == 200 values = response.body.split('&').inject({}) { |h,v| h[v.split("=")[0]] = v.split("=")[1]; h } - + new_access_token = OAuth::AccessToken.new(@oauth_consumer, values["oauth_token"], values["oauth_token_secret"]) tables_uri = "http://vizzuality.testhost.lan/api/v1/tables" @@ -72,5 +72,5 @@ describe "API Authentication" do status.should == 200 end end - + end diff --git a/spec/requests/imports/uploads_spec.rb b/spec/requests/imports/uploads_spec.rb new file mode 100644 index 0000000000..5d4eece479 --- /dev/null +++ b/spec/requests/imports/uploads_spec.rb @@ -0,0 +1,37 @@ +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') + +describe "Imports API uploads" do + + before(:each) do + @user = create_user(:username => 'test', :email => "client@example.com", :password => "clientex") + @user.set_map_key + end + + it 'allow to upload using the request body' do + file = File.open(Rails.root.join('spec/support/whs_features.csv'), 'r+') + + post v1_imports_uploads_url(:host => 'test.localhost.lan'), {:body => file.read, :qqfile => File.basename(file.path), :api_key => @user.get_map_key} + + response.code.should be == '200' + + response_json = JSON.parse(response.body) + response_json.should_not be_nil + response_json['file_uri'].should match(/uploads\/.*\/whs_features.csv/) + response_json['success'].should be_true + end + + it 'allow to upload using a file param' do + + file = Rack::Test::UploadedFile.new(Rails.root.join('spec/support/whs_features.csv'), 'text/plain') + + post v1_imports_uploads_url(:host => 'test.localhost.lan'), {:file => file, :api_key => @user.get_map_key} + + response.code.should be == '200' + + response_json = JSON.parse(response.body) + response_json.should_not be_nil + response_json['file_uri'].should match(/uploads\/.*\/whs_features.csv/) + response_json['success'].should be_true + end + +end diff --git a/spec/support/factories/users.rb b/spec/support/factories/users.rb index 8de10e03f6..e7d1a45b61 100644 --- a/spec/support/factories/users.rb +++ b/spec/support/factories/users.rb @@ -38,4 +38,4 @@ module CartoDB user.save end end -end \ No newline at end of file +end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 13679c5204..caa6106abf 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -1,4 +1,5 @@ module HelperMethods + def prepare_oauth_request(consumer, url, options={}) url = URI.parse(url) http = Net::HTTP.new(url.host, url.port) @@ -11,4 +12,5 @@ module HelperMethods req.oauth!(http, consumer, options[:token]) req end -end \ No newline at end of file + +end