diff --git a/app/controllers/api/json/tables_controller.rb b/app/controllers/api/json/tables_controller.rb index cbde110ede..1fdb5a8ef2 100644 --- a/app/controllers/api/json/tables_controller.rb +++ b/app/controllers/api/json/tables_controller.rb @@ -143,13 +143,15 @@ class Api::Json::TablesController < ApplicationController unless params[:column].blank? || params[:column].empty? begin if params[:what] == 'add' - @table.add_column!(params[:column]) + resp = @table.add_column!(params[:column]) + render :json => resp.to_json, :status => 200 and return elsif params[:what] == 'drop' @table.drop_column!(params[:column]) + render :json => ''.to_json, :status => 200 and return else - @table.modify_column!(params[:column]) + resp = @table.modify_column!(params[:column]) + render :json => resp.to_json, :status => 200 and return end - render :json => ''.to_json, :status => 200 rescue => e render :json => { :errors => [e.message.split("\n").first] }.to_json, :status => 400 and return end diff --git a/app/models/table.rb b/app/models/table.rb index b88791cbee..7220a25407 100644 --- a/app/models/table.rb +++ b/app/models/table.rb @@ -151,8 +151,9 @@ class Table < Sequel::Model(:user_tables) def add_column!(options) owner.in_database do |user_database| - user_database.add_column name.to_sym, options[:name].to_s, options[:type] + user_database.add_column name.to_sym, options[:name].to_s.sanitize, options[:type] end + return {:name => options[:name].to_s.sanitize, :type => options[:type]} end def drop_column!(options) @@ -162,27 +163,32 @@ class Table < Sequel::Model(:user_tables) end def modify_column!(options) + new_name = options[:name] + new_type = options[:type] owner.in_database do |user_database| if options[:old_name] && options[:new_name] - user_database.rename_column name.to_sym, options[:old_name].to_sym, options[:new_name].to_sym + user_database.rename_column name.to_sym, options[:old_name].to_sym, options[:new_name].sanitize.to_sym + new_name = options[:new_name].sanitize end if options[:type] + column_name = (options[:new_name] || options[:name]).sanitize begin - user_database.set_column_type name.to_sym, (options[:new_name] || options[:name]).to_sym, options[:type] + user_database.set_column_type name.to_sym, column_name.to_sym, options[:type] rescue => e message = e.message.split("\n").first if message =~ /cannot be cast to type/ user_database.transaction do random_name = "new_column_#{rand(10)*Time.now.to_i}" user_database.add_column name.to_sym, random_name, options[:type] - user_database["UPDATE #{name} SET #{random_name}=#{(options[:new_name] || options[:name])}::#{options[:type]}"] - user_database.drop_column name.to_sym, (options[:new_name] || options[:name]).to_sym - user_database.rename_column name.to_sym, random_name, (options[:new_name] || options[:name]).to_sym + user_database["UPDATE #{name} SET #{random_name}=#{column_name}::#{options[:type]}"] + user_database.drop_column name.to_sym, column_name.to_sym + user_database.rename_column name.to_sym, random_name, column_name.to_sym end end end end end + return {:name => new_name, :type => new_type} end def to_json(options = {}) diff --git a/spec/acceptance/api/tables_spec.rb b/spec/acceptance/api/tables_spec.rb index 686428aa65..379783b389 100644 --- a/spec/acceptance/api/tables_spec.rb +++ b/spec/acceptance/api/tables_spec.rb @@ -153,17 +153,21 @@ feature "Tables JSON API" do } } response.status.should == 200 + json_response = JSON(response.body) + json_response.should == {"name" => "postal_code", "type" => 'integer'} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"postal code", "integer"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:postal_code, "integer"]] put_json "/api/json/tables/#{table.id}/update_schema", { :what => "modify", :column => { - :type => "text", :name => "postal code" + :type => "text", :name => "postal_code" } } response.status.should == 200 + json_response = JSON(response.body) + json_response.should == {"name" => "postal_code", "type" => 'text'} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"postal code", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:postal_code, "text"]] put_json "/api/json/tables/#{table.id}/update_schema", { :what => "add", :column => { @@ -176,7 +180,7 @@ feature "Tables JSON API" do put_json "/api/json/tables/#{table.id}/update_schema", { :what => "drop", :column => { - :name => "postal code" + :name => "postal_code" } } response.status.should == 200 @@ -185,17 +189,17 @@ feature "Tables JSON API" do put_json "/api/json/tables/#{table.id}/update_schema", { :what => "drop", :column => { - :name => "postal code" + :name => "postal_code" } } response.status.should == 400 table.reload json_response = JSON(response.body) - json_response['errors'].should == ["PGError: ERROR: column \"postal code\" of relation \"#{table.name}\" does not exist"] + json_response['errors'].should == ["PGError: ERROR: column \"postal_code\" of relation \"#{table.name}\" does not exist"] put_json "/api/json/tables/#{table.id}/update_schema", { :what => "wadus", :column => { - :name => "postal code" + :name => "postal_code" } } response.status.should == 400 @@ -218,7 +222,11 @@ feature "Tables JSON API" do authenticate_api user - post_json "/api/json/tables/#{table.id}/rows", { :name => "Name 123", :description => "The description", :location => Point.from_x_y(1,1).as_ewkt } + post_json "/api/json/tables/#{table.id}/rows", { + :name => "Name 123", + :description => "The description", + :location => Point.from_x_y(1,1).as_ewkt + } response.status.should == 200 table.reload table.rows_counted.should == 1 diff --git a/spec/models/table_spec.rb b/spec/models/table_spec.rb index 12f9652b23..44d196ba08 100644 --- a/spec/models/table_spec.rb +++ b/spec/models/table_spec.rb @@ -153,31 +153,35 @@ describe Table do }.should raise_error table.reload - table.add_column!(:name => "my new column", :type => "integer") + resp = table.add_column!(:name => "my new column", :type => "integer") + resp.should == {:name => 'my_new_column', :type => 'integer'} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column", "integer"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column, "integer"]] - table.modify_column!(:old_name => "my new column", :new_name => "my new column new name", :type => "text") + resp = table.modify_column!(:old_name => "my_new_column", :new_name => "my new column new name", :type => "text") + resp.should == {:name => 'my_new_column_new_name', :type => 'text'} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column new name", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column_new_name, "text"]] - table.modify_column!(:old_name => "my new column new name", :new_name => "my new column") + resp = table.modify_column!(:old_name => "my_new_column_new_name", :new_name => "my new column") + resp.should == {:name => 'my_new_column', :type => nil} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column, "text"]] - table.modify_column!(:name => "my new column", :type => "text") + resp = table.modify_column!(:name => "my_new_column", :type => "text") + resp.should == {:name => 'my_new_column', :type => 'text'} table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column, "text"]] table.drop_column!(:name => "location") table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:description, "text"], [:"my new column", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:description, "text"], [:my_new_column, "text"]] lambda { table.drop_column!(:name => "location") }.should raise_error table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:description, "text"], [:"my new column", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:description, "text"], [:my_new_column, "text"]] end it "should be able to modify it's schema with castings that the DB engine doesn't support" do @@ -186,11 +190,11 @@ describe Table do table.add_column!(:name => "my new column", :type => "text") table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column", "text"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column, "text"]] - table.modify_column!(:old_name => "my new column", :new_name => "my new column new name", :type => "integer", :force_value => "NULL") + table.modify_column!(:old_name => "my_new_column", :new_name => "my new column new name", :type => "integer", :force_value => "NULL") table.reload - table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:"my new column new name", "integer"]] + table.schema.should == [[:id, "integer"], [:name, "text"], [:location, "geometry"], [:description, "text"], [:my_new_column_new_name, "integer"]] end it "should be able to insert a new row" do