Merge branch 'interface_cleanup'

lib/geocoder/orms/active_record.rb

 require 'geocoder/orms/base'
+require 'geocoder/orms/active_record_legacy'
 
 ##
 # Add geocoding functionality to any ActiveRecord object.
@@ -6,6 +7,7 @@ require 'geocoder/orms/base'
 module Geocoder::Orm
   module ActiveRecord
     include Base
+    include ActiveRecord::Legacy
 
     ##
     # Implementation of 'included' hook method.
@@ -115,7 +117,7 @@ module Geocoder::Orm
           ["#{lat_attr} BETWEEN ? AND ? AND #{lon_attr} BETWEEN ? AND ?"] +
           coordinate_bounds(latitude, longitude, radius)
         if obj = options[:exclude]
-          conditions[0] << " AND id != ?"
+          conditions[0] << " AND #{table_name}.id != ?"
           conditions << obj.id
         end
         {
@@ -145,46 +147,34 @@ module Geocoder::Orm
     end
 
     ##
-    # Fetch coordinates and assign +latitude+ and +longitude+. Also returns
-    # coordinates as an array: <tt>[lat, lon]</tt>.
+    # Look up coordinates and assign to +latitude+ and +longitude+ attributes
+    # (or other as specified in +geocoded_by+). Returns coordinates (array).
     #
-    def fetch_coordinates(save = false)
-      geocode do |o,r|
+    def geocode
+      geocode! do |o,r|
         unless r.latitude.nil? or r.longitude.nil?
-          method = (save ? "update" : "write") + "_attribute"
-          o.send method, self.class.geocoder_options[:latitude],  r.latitude
-          o.send method, self.class.geocoder_options[:longitude], r.longitude
+          o.send :write_attribute, self.class.geocoder_options[:latitude],  r.latitude
+          o.send :write_attribute, self.class.geocoder_options[:longitude], r.longitude
         end
         r.coordinates
       end
     end
 
-    ##
-    # Fetch coordinates and update (save) +latitude+ and +longitude+ data.
-    #
-    def fetch_coordinates!
-      fetch_coordinates(true)
-    end
+    #alias_method :fetch_coordinates, :geocode
 
     ##
-    # Fetch address and assign +address+ attribute. Also returns
-    # address as a string.
+    # Look up address and assign to +address+ attribute (or other as specified
+    # in +reverse_geocoded_by+). Returns address (string).
     #
-    def fetch_address(save = false)
-      geocode do |o,r|
+    def reverse_geocode
+      reverse_geocode! do |o,r|
         unless r.address.nil?
-          method = (save ? "update" : "write") + "_attribute"
-          o.send method, self.class.geocoder_options[:fetched_address], r.address
+          o.send :write_attribute, self.class.geocoder_options[:fetched_address], r.address
         end
         r.address
       end
     end
 
-    ##
-    # Fetch address and update (save) +address+ data.
-    #
-    def fetch_address!
-      fetch_address(true)
-    end
+    #alias_method :fetch_address, :reverse_geocode
   end
 end

lib/geocoder/orms/active_record_legacy.rb

+module Geocoder::Orm::ActiveRecord
+  module Legacy
+
+    ##
+    # Fetch coordinates and update (save) +latitude+ and +longitude+ data.
+    #
+    def fetch_coordinates!
+      warn "DEPRECATION WARNING: The 'fetch_coordinates!' method is deprecated and will be removed in geocoder v1.0. " +
+        "Please use 'geocode' instead and then save your objects manually."
+      geocode! do |o,r|
+        unless r.latitude.nil? or r.longitude.nil?
+          o.send :update_attribute, self.class.geocoder_options[:latitude],  r.latitude
+          o.send :update_attribute, self.class.geocoder_options[:longitude], r.longitude
+        end
+        r.coordinates
+      end
+    end
+
+    def fetch_coordinates(*args)
+      warn "DEPRECATION WARNING: The 'fetch_coordinates' method will cease taking " +
+        "an argument in geocoder v1.0. Please save your objects manually." if args.size > 0
+      geocode! do |o,r|
+        unless r.latitude.nil? or r.longitude.nil?
+          method = ((args.size > 0 && args.first) ? "update" : "write" ) + "_attribute"
+          o.send method, self.class.geocoder_options[:latitude],  r.latitude
+          o.send method, self.class.geocoder_options[:longitude], r.longitude
+        end
+        r.coordinates
+      end
+    end
+
+    ##
+    # Fetch address and update (save) +address+ data.
+    #
+    def fetch_address!
+      warn "DEPRECATION WARNING: The 'fetch_address!' method is deprecated and will be removed in geocoder v1.0. " +
+        "Please use 'reverse_geocode' instead and then save your objects manually."
+      reverse_geocode! do |o,r|
+        unless r.address.nil?
+          o.send :update_attribute, self.class.geocoder_options[:fetched_address], r.address
+        end
+        r.address
+      end
+    end
+
+    def fetch_address(*args)
+      warn "DEPRECATION WARNING: The 'fetch_address' method will cease taking " +
+        "an argument in geocoder v1.0. Please save your objects manually." if args.size > 0
+      reverse_geocode! do |o,r|
+        unless r.latitude.nil? or r.longitude.nil?
+          method = ((args.size > 0 && args.first) ? "update" : "write" ) + "_attribute"
+          o.send method, self.class.geocoder_options[:fetched_address], r.address
+        end
+        r.coordinates
+      end
+    end
+  end
+end

lib/geocoder/orms/base.rb

@@ -32,6 +32,41 @@ module Geocoder
         self.class.near(read_coordinates, radius, options)
       end
 
+      ##
+      # Look up coordinates and assign to +latitude+ and +longitude+ attributes
+      # (or other as specified in +geocoded_by+). Returns coordinates (array).
+      #
+      def geocode
+        fail
+      end
+
+      ##
+      # Look up address and assign to +address+ attribute (or other as specified
+      # in +reverse_geocoded_by+). Returns address (string).
+      #
+      def reverse_geocode
+        fail
+      end
+
+      ##
+      # Look up coordinates and execute block, if given. Block should take two
+      # arguments: the object and a Geocoder::Result object.
+      #
+      def geocode!(&block)
+        do_lookup(false, &block)
+      end
+
+      ##
+      # Look up address and execute block, if given. Block should take two
+      # arguments: the object and a Geocoder::Result object.
+      #
+      def reverse_geocode!(&block)
+        do_lookup(true, &block)
+      end
+
+
+      private # --------------------------------------------------------------
+
       ##
       # Look up geographic data based on object attributes (configured in
       # geocoded_by or reverse_geocoded_by) and handle the result with the
@@ -39,25 +74,31 @@ module Geocoder
       # given two-arguments: the object being geocoded and a
       # Geocoder::Result object with the geocoding results).
       #
-      def geocode
+      def do_lookup(reverse = false)
         options = self.class.geocoder_options
-        args = options[:user_address] ?
-          [:user_address] : [:latitude, :longitude]
+        if reverse and options[:reverse_geocode]
+          args = [:latitude, :longitude]
+        elsif !reverse and options[:geocode]
+          args = [:user_address]
+        else
+          return
+        end
         args.map!{ |a| send(options[a]) }
 
-        # passing a block to this method overrides the one given in the model
         if result = Geocoder.search(*args)
+          # first execute block passed directly to this method
           if block_given?
-            yield(self, result)
-          else
-            self.class.geocoder_options[:block].call(self, result)
+            value = yield(self, result)
+          end
+          # then execute block specified in configuration
+          block_key = reverse ? :reverse_block : :geocode_block
+          if custom_block = options[block_key]
+            value = custom_block.call(self, result)
           end
+          value
         end
       end
 
-
-      private # --------------------------------------------------------------
-
       ##
       # Read the coordinates [lat,lon] of the object.
       # Looks at user config to determine attributes.

lib/geocoder/railtie.rb

@@ -32,10 +32,11 @@ module Geocoder
     #
     def geocoded_by(address_attr, options = {}, &block)
       geocoder_init(
-        :user_address => address_attr,
-        :latitude  => options[:latitude]  || :latitude,
-        :longitude => options[:longitude] || :longitude,
-        :block => block
+        :geocode       => true,
+        :user_address  => address_attr,
+        :latitude      => options[:latitude]  || :latitude,
+        :longitude     => options[:longitude] || :longitude,
+        :geocode_block => block
       )
     end
 
@@ -44,10 +45,11 @@ module Geocoder
     #
     def reverse_geocoded_by(latitude_attr, longitude_attr, options = {}, &block)
       geocoder_init(
+        :reverse_geocode => true,
         :fetched_address => options[:address] || :address,
-        :latitude  => latitude_attr,
-        :longitude => longitude_attr,
-        :block => block
+        :latitude        => latitude_attr,
+        :longitude       => longitude_attr,
+        :reverse_block   => block
       )
     end
 

test/geocoder_test.rb

@@ -23,7 +23,7 @@ class GeocoderTest < Test::Unit::TestCase
   def test_does_not_choke_on_nil_address
     v = Venue.new("Venue", nil)
     assert_nothing_raised do
-      v.fetch_coordinates
+      v.geocode
     end
   end
 
@@ -42,42 +42,55 @@ class GeocoderTest < Test::Unit::TestCase
 
   # --- general ---
 
-  def test_fetch_coordinates_assigns_and_returns_coordinates
+  def test_geocode_assigns_and_returns_coordinates
     v = Venue.new(*venue_params(:msg))
     coords = [40.750354, -73.993371]
-    assert_equal coords, v.fetch_coordinates
+    assert_equal coords, v.geocode
     assert_equal coords, [v.latitude, v.longitude]
   end
 
-  def test_fetch_address_assigns_and_returns_address
+  def test_reverse_geocode_assigns_and_returns_address
     v = Landmark.new(*landmark_params(:msg))
     address = "4 Penn Plaza, New York, NY 10001, USA"
-    assert_equal address, v.fetch_address
+    assert_equal address, v.reverse_geocode
     assert_equal address, v.address
   end
 
-  def test_geocode_fetches_and_assigns_custom_coordinates
+  def test_geocode_bang_fetches_and_assigns_custom_coordinates
     e = Event.new(*venue_params(:msg))
     coords = [40.750354, -73.993371]
-    e.geocode
+    e.geocode!
     assert_equal coords.map{ |c| c.to_s }.join(','), e.coordinates
   end
 
-  def test_geocode_fetches_and_assigns_custom_address_components
+  def test_geocode_bang_doesnt_auto_assign_coordinates
+    e = Event.new(*venue_params(:msg))
+    e.geocode!
+    assert_nil e.latitude
+    assert_nil e.longitude
+  end
+
+  def test_reverse_geocode_bang_fetches_and_assigns_custom_address_components
     e = Party.new(*landmark_params(:msg))
-    e.geocode
+    e.reverse_geocode!
     assert_equal "US", e.country
   end
 
-  def test_fetch_forward_and_reverse_geocoding_on_same_model
+  def test_reverse_geocode_bang_doesnt_auto_assign_address
+    e = Party.new(*landmark_params(:msg))
+    e.reverse_geocode!
+    assert_nil e.address
+  end
+
+  def test_forward_and_reverse_geocoding_on_same_model
     g = GasStation.new("Exxon")
     g.address = "404 New St, Middletown, CT"
-    g.fetch_coordinates
+    g.geocode
     assert_not_nil g.lat
     assert_not_nil g.lon
 
     assert_nil g.location
-    g.fetch_address
+    g.reverse_geocode
     assert_not_nil g.location
   end