Potlatch 0.11a

app/controllers/amf_controller.rb

@@ -501,13 +501,13 @@ class AmfController < ApplicationController
       new_relation.version = version
 
       if relid <= 0
-        # We're creating the node
+        # We're creating the relation
         new_relation.create_with_history(user)
       elsif visible
-        # We're updating the node
+        # We're updating the relation
         relation.update_from(new_relation, user)
       else
-        # We're deleting the node
+        # We're deleting the relation
         relation.delete_with_history!(new_relation, user)
       end
     end # transaction
@@ -542,7 +542,8 @@ class AmfController < ApplicationController
   # 4. way ID
   # 5. list of nodes in way
   # 6. hash of way tags
-  # 7. array of nodes to change (each one is [lon,lat,id,version,tags])
+  # 7. array of nodes to change (each one is [lon,lat,id,version,tags]),
+  # 8. hash of nodes to delete (id->version).
   # 
   # Returns:
   # 0. '0' (code for success),
@@ -552,7 +553,7 @@ class AmfController < ApplicationController
   # 4. way version,
   # 5. hash of node versions (node=>version)
 
-  def putway(renumberednodes, usertoken, changeset_id, wayversion, originalway, pointlist, attributes, nodes) #:doc:
+  def putway(renumberednodes, usertoken, changeset_id, wayversion, originalway, pointlist, attributes, nodes, deletednodes) #:doc:
 
     # -- Initialise
 	
@@ -561,22 +562,13 @@ class AmfController < ApplicationController
     if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end
 
     originalway = originalway.to_i
-	pointlist.collect! {|a| a.to_i }
+  	pointlist.collect! {|a| a.to_i }
 
     way=nil	# this is returned, so scope it outside the transaction
     nodeversions = {}
     Way.transaction do
 
-      # -- Get unique nodes
+      # -- Update each changed node
-
-      if originalway <= 0
-        uniques = []
-      else
-        way = Way.find(originalway)
-        uniques = way.unshared_node_ids
-      end
-
-      #-- Update each changed node
 
       nodes.each do |a|
         lon = a[0].to_f
@@ -609,9 +601,9 @@ class AmfController < ApplicationController
 
       # -- Save revised way
 
-	  pointlist.collect! {|a|
+	    pointlist.collect! {|a|
-		renumberednodes[a] ? renumberednodes[a]:a
+		    renumberednodes[a] ? renumberednodes[a]:a
-	  } # renumber nodes
+	    } # renumber nodes
       new_way = Way.new
       new_way.tags = attributes
       new_way.nds = pointlist
@@ -620,25 +612,26 @@ class AmfController < ApplicationController
       if originalway <= 0
         new_way.create_with_history(user)
         way=new_way	# so we can get way.id and way.version
-      elsif way.tags!=attributes or way.nds!=pointlist or !way.visible?
+      else
-        way.update_from(new_way, user)
+	      way = Way.find(originalway)
+   		  if way.tags!=attributes or way.nds!=pointlist or !way.visible?
+    	    way.update_from(new_way, user)
+        end
       end
 
-      # -- Delete any unique nodes no longer used
+      # -- Delete unwanted nodes
 
-      uniques=uniques-pointlist
+      deletednodes.each do |id,v|
-      uniques.each do |n|
+        node = Node.find(id.to_i)
-        node = Node.find(n)
-        deleteitemrelations(user, changeset_id, id, 'Node', node.version)
         new_node = Node.new
         new_node.changeset_id = changeset_id
-        new_node.version = node.version
+        new_node.version = v.to_i
-        node.delete_with_history!(new_node, user)
+        node.delete_with_history_unless_used!(new_node, user)
       end
 
     end # transaction
 
-    [0, originalway, way.id, renumberednodes, way.version, nodeversions]
+    [0, originalway, way.id, renumberednodes, way.version, nodeversions, deletednodes]
   rescue OSM::APIChangesetAlreadyClosedError => ex
     return [-2, "Sorry, your changeset #{ex.changeset.id} has been closed (at #{ex.changeset.closed_at})."]
   rescue OSM::APIVersionMismatchError => ex
@@ -747,7 +740,7 @@ class AmfController < ApplicationController
   # of the nodes have been changed by someone else then, there is a problem!
   # Returns 0 (success), unchanged way id.
 
-  def deleteway(usertoken, changeset_id, way_id, way_version, node_id_version) #:doc:
+  def deleteway(usertoken, changeset_id, way_id, way_version, deletednodes) #:doc:
     user = getuser(usertoken)
     unless user then return -1,"You are not logged in, so the way could not be deleted." end
       
@@ -755,29 +748,24 @@ class AmfController < ApplicationController
     # Need a transaction so that if one item fails to delete, the whole delete fails.
     Way.transaction do
 
-      # delete the way
+      # -- Delete the way
+
       old_way = Way.find(way_id)
-      u = old_way.unshared_node_ids
       delete_way = Way.new
       delete_way.version = way_version
       delete_way.changeset_id = changeset_id
       old_way.delete_with_history!(delete_way, user)
 
-      u.each do |node_id|
+      # -- Delete unwanted nodes
-        # delete the node
+
-        node = Node.find(node_id)
+      deletednodes.each do |id,v|
-        delete_node = Node.new
+        node = Node.find(id.to_i)
-        delete_node.changeset_id = changeset_id
+        new_node = Node.new
-        if node_id_version[node_id.to_s]
+        new_node.changeset_id = changeset_id
-          delete_node.version = node_id_version[node_id.to_s]
+        new_node.version = v.to_i
-        else
+        node.delete_with_history_unless_used!(new_node, user)
-          # in case the node wasn't passed (i.e. if it was previously removed
-          # from the way in Potlatch)
-          deleteitemrelations(user, changeset_id, node_id, 'Node', node.version)
-	      delete_node.version = node.version
-	    end
-        node.delete_with_history!(delete_node, user)
       end
+
     end # transaction
     [0, way_id]
   rescue OSM::APIChangesetAlreadyClosedError => ex
@@ -798,28 +786,6 @@ class AmfController < ApplicationController
   # ====================================================================
   # Support functions
 
-  # Remove a node or way from all relations
-  # This is only used by putway and deleteway when deleting nodes removed 
-  # from a way (because Potlatch itself doesn't keep track of these - 
-  # possible FIXME).
-
-  def deleteitemrelations(user, changeset_id, objid, type, version) #:doc:
-    relations = RelationMember.find(:all, 
-									:conditions => ['member_type = ? and member_id = ?', type.classify, objid], 
-									:include => :relation).collect { |rm| rm.relation }.uniq
-
-    relations.each do |rel|
-      rel.members.delete_if { |x| x[0] == type and x[1] == objid }
-      new_rel = Relation.new
-      new_rel.tags = rel.tags
-      new_rel.visible = rel.visible
-      new_rel.version = rel.version
-      new_rel.members = rel.members
-      new_rel.changeset_id = changeset_id
-      rel.update_from(new_rel, user)
-    end
-  end
-
   # Authenticate token
   # (can also be of form user:pass)
   # When we are writing to the api, we need the actual user model, 

app/models/node.rb

@@ -149,6 +149,30 @@ class Node < ActiveRecord::Base
     end
   end
 
+  # As above but doesn't throw an error if node is used elsewhere - instead, just silently
+  # returns without deleting the node. Potlatch needs this in case you ask to delete an
+  # off-screen node while deleting a way - it doesn't know whether the node is used
+  # elsewhere or not.
+  def delete_with_history_unless_used!(new_node, user)
+    unless self.visible
+      raise OSM::APIAlreadyDeletedError.new("node", new_node.id)
+    end
+
+    Node.transaction do
+      check_consistency(self, new_node, user)
+      way = WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", 
+                         :conditions => [ "current_ways.visible = ? AND current_way_nodes.node_id = ?", true, self.id ])
+      rel = RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", 
+                                :conditions => [ "visible = ? AND member_type='Node' and member_id=? ", true, self.id])
+      if way.nil? and rel.nil?
+        self.changeset_id = new_node.changeset_id
+        self.visible = false
+        changeset.update_bbox!(bbox)
+        save_with_history!
+      end
+    end
+  end
+
   def update_from(new_node, user)
     check_consistency(self, new_node, user)
 

app/models/way.rb

@@ -266,18 +266,6 @@ class Way < ActiveRecord::Base
     end
   end
 
-  # Find nodes that belong to this way only
-  def unshared_node_ids
-    node_ids = self.nodes.collect { |node| node.id }
-
-    unless node_ids.empty?
-      way_nodes = WayNode.find(:all, :conditions => "node_id in (#{node_ids.join(',')}) and id != #{self.id}")
-      node_ids = node_ids - way_nodes.collect { |way_node| way_node.node_id }
-    end
-
-    return node_ids
-  end
-
   # Temporary method to match interface to nodes
   def tags_as_hash
     return self.tags