dont eager load tags (false primary key fucks all sorts of things up) and move delete...
authorSteve Coast <steve@asklater.com>
Fri, 25 Jan 2008 14:32:45 +0000 (14:32 +0000)
committerSteve Coast <steve@asklater.com>
Fri, 25 Jan 2008 14:32:45 +0000 (14:32 +0000)
app/controllers/amf_controller.rb
app/controllers/way_controller.rb
app/models/way.rb
app/models/way_tag.rb
lib/osm.rb

index 876dbd10e04a4cf15dc243b382b3e21f710de0b1..471e2fadabdbbcccca7b19e5916e2d1b83c9ea22 100644 (file)
@@ -475,21 +475,21 @@ class AmfController < ApplicationController
   end
 
   # ----- getpoi
-  #              read POI from database
+  # read POI from database
   #              (only called on revert: POIs are usually read by whichways)
   #              in:   [0] node id, [1] baselong, [2] basey, [3] masterscale
   #              does: reads POI
   #              out:  [0] id (unchanged), [1] projected long, [2] projected lat,
   #                            [3] hash of tags
   def getpoi(args) #:doc:
-    id,baselong,basey,masterscale=args; id=id.to_i
-    poi=ActiveRecord::Base.connection.select_one("SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lng,tags "+
-    "FROM current_nodes WHERE visible=1 AND id=#{id}")
-    if poi.nil? then return [nil,nil,nil,''] end
-    [id,
-      long2coord(poi['lng'].to_f,baselong,masterscale),
-      lat2coord(poi['lat'].to_f,basey,masterscale),
-      tag2array(poi['tags'])]
+    id,baselong,basey,masterscale = args
+    
+    n = Node.find(id.to_i)
+    if n
+      return [n.id, n.long_potlatch(baselong,masterscale), n.lat_potlatch(basey,masterscale), n.tags_as_hash]
+    else
+      return [nil,nil,nil,'']
+    end
   end
 
   # ----- deleteway
index 25a1231dbc5dae8b1069a682275e72b2f63d98f0..c3640609a695ec755c762182679c6a12149c14db 100644 (file)
@@ -69,30 +69,21 @@ class WayController < ApplicationController
     end
   end
 
+  # This is the API call to delete a way
   def delete
     begin
       way = Way.find(params[:id])
-
-      if way.visible
-        # omg FIXME
-        if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='way' and member_id=?", params[:id]])
-          render :text => "", :status => :precondition_failed
-        else
-          way.user_id = @user.id
-          way.tags = []
-          way.nds = []
-          way.visible = false
-         way.save_with_history!
-
-         render :nothing => true
-        end
-      else
-        render :text => "", :status => :gone
-      end
+      way.delete_with_relations_and_history(@user)
+
+      # if we get here, all is fine, otherwise something will catch below.  
+      render :nothing => true
+      return
+    rescue OSM::APIAlreadyDeletedError
+      render :text => "", :status => :gone
+    rescue OSM::APIPreconditionFailedError
+      render :text => "", :status => :precondition_failed
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
-    rescue => ex
-      puts ex
     end
   end
 
index 2c9ce9f72d06e0560632953a1fafd9b069ff26b6..924fffe5c84b99775a37fec5b116f5d9a0ec4ec0 100644 (file)
@@ -52,7 +52,9 @@ class Way < ActiveRecord::Base
   
   # You can't pull in all the tags too unless we put a sequence_id on the way_tags table and have a multipart key
   def self.find_eager(id)
-    way = Way.find(id, :include => [:way_tags, {:way_nodes => :node}])
+    way = Way.find(id, :include => {:way_nodes => :node})
+    #If waytag had a multipart key that was real, you could do this:
+    #way = Way.find(id, :include => [:way_tags, {:way_nodes => :node}])
   end
 
   # Find a way given it's ID, and in a single SQL call also grab its nodes and tags
@@ -204,4 +206,22 @@ class Way < ActiveRecord::Base
     return true
   end
 
+  # Delete the way and it's relations, but don't really delete it - set its visibility to false and update the history etc to maintain wiki-like functionality.
+  def delete_with_relations_and_history(user)
+    if self.visible
+      # omg FIXME
+      if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id",
+                             :conditions => [ "visible = 1 AND member_type='way' and member_id=?", self.id])
+        raise OSM::APIPreconditionFailedError
+      else
+        self.user_id = user.id
+        self.tags = []
+        self.nds = []
+        self.visible = false
+        self.save_with_history!
+      end
+    else
+      raise OSM::APIAlreadyDeletedError
+    end
+  end
 end
index 3f6ac8ce3b5427b5128228e3f49cd218aa6fb84c..4548674d4b950607e2396ecc079da2e3951bba4c 100644 (file)
@@ -1,8 +1,9 @@
 class WayTag < ActiveRecord::Base
   set_table_name 'current_way_tags'
 
-  # false multipart key
-  set_primary_keys :id, :k, :v
+  # False multipart keys. The following would be a hack:
+  # set_primary_keys :id, :k, :v
+  # FIXME add a real multipart key to waytags so that we can do eager loadin
 
   belongs_to :way, :foreign_key => 'id'
 end
index 24df7a35ddf11efe5ea2aba1a83e8ee66cd4c7be..bd935102658576295b2670ca46a8b28de89e94b4 100644 (file)
@@ -1,13 +1,6 @@
+# The OSM module provides support functions for OSM.
 module OSM
 
-  # This piece of magic reads a GPX with SAX and spits out
-  # lat/lng and stuff
-  #
-  # This would print every latitude value:
-  #
-  # gpx = OSM::GPXImporter.new('somefile.gpx')
-  # gpx.points {|p| puts p['latitude']}
-
   require 'time'
   require 'rexml/parsers/sax2parser'
   require 'rexml/text'
@@ -15,11 +8,27 @@ module OSM
   require 'digest/md5'
   require 'RMagick'
 
+  # The base class for API Errors.
+  class APIError < RuntimeError
+  end
+
+  # Raised when an API object is not found.
+  class APINotFoundError < APIError
+  end
+
+  # Raised when a precondition to an API action fails sanity check.
+  class APIPreconditionFailedError < APIError
+  end
+
+  # Raised when to delete an already-deleted object.
+  class APIAlreadyDeletedError < APIError
+  end
+
+  # Helper methods for going to/from mercator and lat/lng.
   class Mercator
     include Math
 
     #init me with your bounding box and the size of your image
-
     def initialize(min_lat, min_lon, max_lat, max_lon, width, height)
       xsize = xsheet(max_lon) - xsheet(min_lon)
       ysize = ysheet(max_lat) - ysheet(min_lat)
@@ -62,6 +71,13 @@ module OSM
   end
 
 
+  # This piece of magic reads a GPX with SAX and spits out
+  # lat/lng and stuff
+  #
+  # This would print every latitude value:
+  #
+  # gpx = OSM::GPXImporter.new('somefile.gpx')
+  # gpx.points {|p| puts p['latitude']}
   class GPXImporter
     # FIXME swap REXML for libXML
     attr_reader :possible_points