From: Dave Stubbs <osm@randomjunk.co.uk>
Date: Sun, 9 Nov 2008 17:41:38 +0000 (+0000)
Subject: a few more amf tests and associated bug fixes
X-Git-Tag: live~7563^2~172
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/436470b0576574971ea13b9754142047a6d109df?hp=b6c354b31b8e7963bbd384045cb72cd3a88c613e

a few more amf tests and associated bug fixes
---

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index 0e77c11f7..f90a62a9a 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -61,6 +61,7 @@ class AmfController < ApplicationController
 	  index=AMF.getstring(req)				#  | get index in response sequence
 	  bytes=AMF.getlong(req)				#  | get total size in bytes
 	  args=AMF.getvalue(req)				#  | get response (probably an array)
+      logger.info "Executing AMF #{message}:#{index}"
 
 	  case message
 		when 'getpresets';			results[index]=AMF.putdata(index,getpresets())
@@ -75,6 +76,7 @@ class AmfController < ApplicationController
 		when 'getpoi';				results[index]=AMF.putdata(index,getpoi(*args))
 	  end
 	end
+    logger.info("encoding AMF results")
     sendresponse(results)
   end
 
@@ -218,23 +220,26 @@ class AmfController < ApplicationController
 
   # Get an old version of a way, and all constituent nodes.
   #
-  # For undelete (version=0), always uses the most recent version of each node, 
-  # even if it's moved.  For revert (version=1+), uses the node in existence 
+  # For undelete (version<0), always uses the most recent version of each node, 
+  # even if it's moved.  For revert (version >= 0), uses the node in existence 
   # at the time, generating a new id if it's still visible and has been moved/
   # retagged.
 
   def getway_old(id, version) #:doc:
 	if version < 0
 	  old_way = OldWay.find(:first, :conditions => ['visible = ? AND id = ?', true, id], :order => 'version DESC')
-	  points = old_way.get_nodes_undelete
+	  points = old_way.get_nodes_undelete unless old_way.nil?
 	else
 	  old_way = OldWay.find(:first, :conditions => ['id = ? AND version = ?', id, version])
-	  points = old_way.get_nodes_revert
+	  points = old_way.get_nodes_revert unless old_way.nil?
 	end
 
-	old_way.tags['history'] = "Retrieved from v#{old_way.version}"
-
-	[0, id, points, old_way.tags, old_way.version]
+    if old_way.nil?
+      return [0, id, [], {}, -1]
+    else
+	  old_way.tags['history'] = "Retrieved from v#{old_way.version}"
+	  return [0, id, points, old_way.tags, old_way.version]
+    end
   end
   
   # Find history of a way. Returns 'way', id, and 
diff --git a/app/models/old_node.rb b/app/models/old_node.rb
index 03aff0fc6..badcd74a2 100644
--- a/app/models/old_node.rb
+++ b/app/models/old_node.rb
@@ -92,11 +92,7 @@ class OldNode < ActiveRecord::Base
   end
 
   def tags_as_hash 
-    hash = {} 
-    Tags.split(self.tags) do |k,v| 
-      hash[k] = v 
-    end 
-    hash 
+    return self.tags
   end 
  
   # Pretend we're not in any ways 
diff --git a/test/functional/amf_controller_test.rb b/test/functional/amf_controller_test.rb
index e0375c92f..9a12a01c4 100644
--- a/test/functional/amf_controller_test.rb
+++ b/test/functional/amf_controller_test.rb
@@ -154,6 +154,43 @@ class AmfControllerTest < ActionController::TestCase
     assert rel[1].empty? and rel[2].empty?
   end
 
+  def test_getway_old
+    # try to get the last visible version (specified by <0) (should be current version)
+    latest = current_ways(:way_with_versions)
+    # try to get version 1
+    v1 = ways(:way_with_versions_v1)
+    {latest => -1, v1 => v1.version}.each do |way, v|
+      amf_content "getway_old", "/1", [way.id, v]
+      post :amf_read
+      assert_response :success
+      amf_parse_response
+      returned_way = amf_result("/1")
+      assert_equal returned_way[1], way.id
+      assert_equal returned_way[4], way.version
+    end
+  end
+
+  def test_getway_old_nonexistent
+    # try to get the last version+10 (shoudn't exist)
+    latest = current_ways(:way_with_versions)
+    # try to get last visible version of non-existent way
+    # try to get specific version of non-existent way
+    {nil => -1, nil => 1, latest => latest.version + 10}.each do |way, v|
+      amf_content "getway_old", "/1", [way.nil? ? 0 : way.id, v]
+      post :amf_read
+      assert_response :success
+      amf_parse_response
+      returned_way = amf_result("/1")
+      assert returned_way[2].empty?
+      assert returned_way[3].empty?
+      assert returned_way[4] < 0
+    end
+  end
+
+
+  # ************************************************************
+  # AMF Helper functions
+
   # Get the result record for the specified ID
   # It's an assertion FAIL if the record does not exist
   def amf_result ref