From 042ecee695e94abffd88bbe77252a26338839e2b Mon Sep 17 00:00:00 2001
From: Tom Hughes <tom@compton.nu>
Date: Tue, 29 Jul 2014 10:14:08 +0100
Subject: [PATCH] Don't preload the nodes as it seems to break the ordering

Fixes #795
---
 app/controllers/amf_controller.rb       |  4 +-
 test/controllers/amf_controller_test.rb | 79 +++++++++++++++++++++----
 2 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index ad337d8a7..9dd0e909e 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -321,12 +321,12 @@ class AmfController < ApplicationController
         # Ideally we would do ":include => :nodes" here but if we do that
         # then rails only seems to return the first copy of a node when a
         # way includes a node more than once
-        way = Way.where(:id => wayid).preload(:nodes => :node_tags).first
+        way = Way.where(:id => wayid).first
 
         # check case where way has been deleted or doesn't exist
         return [-4, 'way', wayid] if way.nil? or !way.visible
 
-        points = way.nodes.collect do |node|
+        points = way.nodes.preload(:node_tags).collect do |node|
           nodetags=node.tags
           nodetags.delete('created_by')
           [node.lon, node.lat, node.id, nodetags, node.version]
diff --git a/test/controllers/amf_controller_test.rb b/test/controllers/amf_controller_test.rb
index 36b76c329..073b82235 100644
--- a/test/controllers/amf_controller_test.rb
+++ b/test/controllers/amf_controller_test.rb
@@ -26,8 +26,13 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response                                         
     way = amf_result("/1")
-    assert_equal way[0], 0
-    assert_equal way[2], id
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 1, way[3].length
+    assert_equal 3, way[3][0][2]
+    assert_equal 1, way[5]
+    assert_equal 2, way[6]
   end
 
   def test_getway_invisible
@@ -38,10 +43,64 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response
     way = amf_result("/1")
-    assert_equal way[0], -4
-    assert_equal way[1], "way"
-    assert_equal way[2], id
-    assert way[3].nil? and way[4].nil?
+    assert_equal -4, way[0], -4
+    assert_equal "way", way[1]
+    assert_equal id, way[2]
+    assert way[3].nil? and way[4].nil? and way[5].nil? and way[6].nil?
+  end
+
+  def test_getway_with_versions
+    # check a way with multiple versions
+    id = current_ways(:way_with_versions).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 1, way[3].length
+    assert_equal 15, way[3][0][2]
+    assert_equal 4, way[5]
+    assert_equal 2, way[6]
+  end
+
+  def test_getway_with_duplicate_nodes
+    # check a way with duplicate nodes
+    id = current_ways(:way_with_duplicate_nodes).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 2, way[3].length
+    assert_equal 4, way[3][0][2]
+    assert_equal 4, way[3][1][2]
+    assert_equal 1, way[5]
+    assert_equal 2, way[6]
+  end
+
+  def test_getway_with_multiple_nodes
+    # check a way with multiple nodes
+    id = current_ways(:way_with_multiple_nodes).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 3, way[3].length
+    assert_equal 4, way[3][0][2]
+    assert_equal 15, way[3][1][2]
+    assert_equal 6, way[3][2][2]
+    assert_equal 2, way[5]
+    assert_equal 2, way[6]
   end
 
   def test_getway_nonexistent
@@ -51,10 +110,10 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response
     way = amf_result("/1")
-    assert_equal way[0], -4
-    assert_equal way[1], "way"
-    assert_equal way[2], 0
-    assert way[3].nil? and way[4].nil?
+    assert_equal -4, way[0]
+    assert_equal "way", way[1]
+    assert_equal 0, way[2]
+    assert way[3].nil? and way[4].nil? and way[5].nil? and way[6].nil?
   end
 
   def test_whichways
-- 
2.43.2