CDB-1699 Add new attributes to Geocoding

kind: determines the type of geocoding job to be done, see Geocoding::ALLOWED_KINDS
column_name: column that contains the data to geocode
country_code: ISO 3166-1, will make the geocoder more accurate
geometry_type: internal geocoder can handle polygons, see CartoDB::InternalGeocoder

app/models/geocoding.rb

@@ -1,9 +1,11 @@
 # encoding: UTF-8'
 require_relative '../../services/table-geocoder/lib/table_geocoder'
+require_relative '../../services/table-geocoder/lib/internal_geocoder.rb'
 
 class Geocoding < Sequel::Model
 
   DEFAULT_TIMEOUT = 15.minutes
+  ALLOWED_KINDS   = %w(admin0 admin1 namedplace postalcode high-resolution)
 
   many_to_one :user
   many_to_one :table
@@ -11,42 +13,37 @@ class Geocoding < Sequel::Model
 
   attr_reader :table_geocoder
 
-  attr_accessor :run_timeout
+  attr_accessor :run_timeout, :column_name
 
   def validate
     super
     validates_presence :formatter
     validates_presence :table_id
+    validates_includes ALLOWED_KINDS, :kind
   end # validate
 
   def after_initialize
     super
-    instantiate_table_geocoder
     @run_timeout = DEFAULT_TIMEOUT
   end #after_initialize
 
-  def after_create
-    super
-    instantiate_table_geocoder
-  end # after_create
-
   def before_save
     super
     self.updated_at = Time.now
     cancel if state == 'cancelled'
   end # before_save
 
-  def instantiate_table_geocoder
+  def table_geocoder
+    geocoder_class = (kind == 'high-resolution' ? CartoDB::TableGeocoder : CartoDB::InternalGeocoder)
     config = Cartodb.config[:geocoder].deep_symbolize_keys.merge(
       table_name: table.try(:name),
       formatter:  translate_formatter,
       connection: (user.present? ? user.in_database(as: :superuser) : nil),
       remote_id:  remote_id,
-      kind:       kind,
       max_rows:   max_geocodable_rows
     )
-    @table_geocoder = CartoDB::TableGeocoder.new(config)
-  end # instantiate_table_geocoder
+    @table_geocoder ||= geocoder_class.new(config)
+  end # table_geocoder
 
   def cancel
     table_geocoder.cancel

db/migrate/20140305112732_add_extra_fields_to_geocodings.rb

@@ -0,0 +1,9 @@
+Sequel.migration do
+  change do
+    alter_table :geocodings do
+      add_column :kind,          :text
+      add_column :country_code,  :text
+      add_column :geometry_type, :text
+    end
+  end
+end

spec/factories/geocodings.rb

@@ -1,6 +1,7 @@
 FactoryGirl.define do
 
   factory :geocoding do
+    kind 'high-resolution'
   end
 
 end

spec/models/geocoding_spec.rb

@@ -22,21 +22,43 @@ describe Geocoding do
     it 'links user and geocoding' do
       geocoding.user.should eq @user
     end
+  end
 
-    it 'initializes table geocoder' do
+  describe '#table_geocoder' do
+    it 'returns an instance of TableGeocoder when kind is high-resolution' do
+      geocoding = FactoryGirl.build(:geocoding, user: @user, table: @table, kind: 'high-resolution')
       geocoding.table_geocoder.should be_kind_of(CartoDB::TableGeocoder)
     end
+
+    it 'returns an instance of InternalGeocoder when kind is not high-resolution' do
+      geocoding = FactoryGirl.build(:geocoding, user: @user, table: @table, kind: 'admin0')
+      geocoding.table_geocoder.should be_kind_of(CartoDB::InternalGeocoder)
+    end
+
+    it 'memoizes' do
+      geocoding = FactoryGirl.build(:geocoding, user: @user, table: @table, kind: 'admin0')
+      geocoder = geocoding.table_geocoder
+      geocoder.should be_kind_of(CartoDB::InternalGeocoder)
+      geocoder.should eq geocoding.table_geocoder
+    end
   end
 
   describe '#save' do
     let(:geocoding) { FactoryGirl.build(:geocoding, user: @user, table: @table) }
 
-    it 'validates table_name and formatter' do
+    it 'validates formatter' do
       expect { geocoding.save }.to raise_error(Sequel::ValidationFailed)
+      geocoding.errors[:formatter].join(',').should match /is not present/
+    end
+
+    it 'validates kind' do
+      geocoding.kind = 'nonsense'
+      expect { geocoding.save }.to raise_error(Sequel::ValidationFailed)
+      geocoding.errors[:kind].join(',').should match /is not in range or set/
     end
 
     it 'updates updated_at' do
-      geocoding = FactoryGirl.build(:geocoding, user: @user, table: @table, formatter: 'b')
+      geocoding = FactoryGirl.build(:geocoding, user: @user, table: @table, formatter: 'b', kind: 'admin0')
       expect { geocoding.save }.to change(geocoding, :updated_at)
     end
   end
@@ -62,7 +84,7 @@ describe Geocoding do
 
   describe '#run!' do
     it 'updates processed_rows, cache_hits and state' do
-      geocoding = Geocoding.create(user: @user, table: @table, formatter: 'b')
+      geocoding = FactoryGirl.create(:geocoding, user: @user, table: @table, formatter: 'b')
       geocoding.table_geocoder.stubs(:run).returns true
       geocoding.table_geocoder.stubs(:cache).returns  OpenStruct.new(hits: 5)
       geocoding.table_geocoder.stubs(:process_results).returns true
@@ -76,7 +98,7 @@ describe Geocoding do
     end
 
     it 'marks the geocoding as failed if the geocoding job fails' do
-      geocoding = FactoryGirl.build(:geocoding, user: @user, formatter: 'a', table: @table)
+      geocoding = FactoryGirl.build(:geocoding, user: @user, formatter: 'a', table: @table, formatter: 'b')
       CartoDB::TableGeocoder.any_instance.stubs(:run).raises("Error")
       CartoDB.expects(:notify_exception).times(1)
 
@@ -85,7 +107,7 @@ describe Geocoding do
     end
 
     it 'raises a timeout error if geocoding takes more than 15 minutes to start' do
-      geocoding = Geocoding.create(user: @user, table: @table, formatter: 'b')
+      geocoding = FactoryGirl.create(:geocoding, user: @user, table: @table, formatter: 'b')
       CartoDB::TableGeocoder.any_instance.stubs(:run).returns true
       CartoDB::TableGeocoder.any_instance.stubs(:process_results).returns true
       CartoDB::Geocoder.any_instance.stubs(:status).returns 'submitted'