Give a nice error message when parsing the nwr tags and they are missing the k and...

diff --git a/app/models/node.rb b/app/models/node.rb
index eeb6e6da5d0af7e784dd81c175e0d0dc80db3878..dd8d96d12aa2116b56be6e2a3b1d986c3a884f9f 100644 (file)
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -105,6 +105,8 @@ class Node < ActiveRecord::Base
     node.visible = true
 
     pt.find('tag').each do |tag|
+      raise OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag['k'].nil?
+      raise OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag['v'].nil?
       node.add_tag_key_val(tag['k'],tag['v'])
     end
 

diff --git a/app/models/relation.rb b/app/models/relation.rb
index 28fa326ba9142374509be04d06dfd4bc93d92354..76cd86729cdc8ed3d3ce16600b799eb9b5813bdc 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -60,6 +60,8 @@ class Relation < ActiveRecord::Base
     relation.visible = true
 
     pt.find('tag').each do |tag|
+      raise OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag['k'].nil?
+      raise OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag['v'].nil?
       relation.add_tag_keyval(tag['k'], tag['v'])
     end
 

diff --git a/app/models/way.rb b/app/models/way.rb
index f9d36be93811cf32c071c5d1af518333ab54fb2c..8788bd671b1e2ffdd9b1c9bd82c8f371edabc45a 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -61,6 +61,8 @@ class Way < ActiveRecord::Base
     way.visible = true
 
     pt.find('tag').each do |tag|
+      raise OSM::APIBadXMLError.new("way", pt, "tag is missing key") if tag['k'].nil?
+      raise OSM::APIBadXMLError.new("way", pt, "tag is missing value") if tag['v'].nil?
       way.add_tag_keyval(tag['k'], tag['v'])
     end
 

diff --git a/test/unit/node_test.rb b/test/unit/node_test.rb
index 8e71233e64eb59b9cd76932303443511692a9e4f..dd907f0f48e79bd4b6f7b17f7fbba4ab2fd416e1 100644 (file)
--- a/test/unit/node_test.rb
+++ b/test/unit/node_test.rb
@@ -269,4 +269,40 @@ class NodeTest < ActiveSupport::TestCase
     }
     assert_match /Must specify a string with one or more characters/, message_update.message
   end
+  
+  def test_from_xml_no_k_v
+    nokv = "<osm><node id='23' lat='12.3' lon='23.4' changeset='12' version='23'><tag /></node></osm>"
+    message_create = assert_raise(OSM::APIBadXMLError) {
+      Node.from_xml(nokv, true)
+    }
+    assert_match /tag is missing key/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
+      Node.from_xml(nokv, false)
+    }
+    assert_match /tag is missing key/, message_update.message
+  end
+  
+  def test_from_xml_no_v
+    no_v = "<osm><node id='23' lat='23.43' lon='23.32' changeset='23' version='32'><tag k='key' /></node></osm>"
+    message_create = assert_raise(OSM::APIBadXMLError) {
+      Node.from_xml(no_v, true)
+    }
+    assert_match /tag is missing value/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
+      Node.from_xml(no_v, false)
+    }
+    assert_match /tag is missing value/, message_update.message
+  end
+  
+  def test_from_xml_duplicate_k
+    dupk = "<osm><node id='23' lat='23.2' lon='23' changeset='34' version='23'><tag k='dup' v='test' /><tag k='dup' v='tester' /></node></osm>"
+    message_create = assert_raise(OSM::APIDuplicateTagsError) {
+      Node.from_xml(dupk, true)
+    }
+    assert_equal "Element node/ has duplicate tags with key dup", message_create.message
+    message_update = assert_raise(OSM::APIDuplicateTagsError) {
+      Node.from_xml(dupk, false)
+    }
+    assert_equal "Element node/23 has duplicate tags with key dup", message_update.message
+  end
 end

diff --git a/test/unit/relation_test.rb b/test/unit/relation_test.rb
index a878be8cb0b7645a53c9eeb74a775426c6cb9654..b1fbc0fcd6c4ced1473b083bf31a40149da90b3f 100644 (file)
--- a/test/unit/relation_test.rb
+++ b/test/unit/relation_test.rb
@@ -69,14 +69,30 @@ class RelationTest < ActiveSupport::TestCase
   
   def test_from_xml_no_k_v
     nokv = "<osm><relation id='23' changeset='23' version='23'><tag /></relation></osm>"
-    assert_nothing_raised(OSM::APIBadUserInput, OSM::APIBadXMLError) {
+    message_create = assert_raise(OSM::APIBadXMLError) {
       Relation.from_xml(nokv, true)
+    }
+    assert_match /tag is missing key/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
       Relation.from_xml(nokv, false)
     }
+    assert_match /tag is missing key/, message_update.message
+  end
+  
+  def test_from_xml_no_v
+    no_v = "<osm><relation id='23' changeset='23' version='23'><tag k='key' /></relation></osm>"
+    message_create = assert_raise(OSM::APIBadXMLError) {
+      Relation.from_xml(no_v, true)
+    }
+    assert_match /tag is missing value/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
+      Relation.from_xml(no_v, false)
+    }
+    assert_match /tag is missing value/, message_update.message
   end
   
   def test_from_xml_duplicate_k
-    dupk = "<osm><relation id='23' changeset='23' version='23'><tag k='dup' v='test'/><tag k='dup' v='test'/></relation></osm>"
+    dupk = "<osm><relation id='23' changeset='23' version='23'><tag k='dup' v='test'/><tag k='dup' v='tester'/></relation></osm>"
     message_create = assert_raise(OSM::APIDuplicateTagsError) {
       Relation.from_xml(dupk, true)
     }

diff --git a/test/unit/way_test.rb b/test/unit/way_test.rb
index bdf4ab3657875bc7b716e30531b8181afbb5e1b5..c13c9755fa0de2f52518510a9d4fc2ec41beb71a 100644 (file)
--- a/test/unit/way_test.rb
+++ b/test/unit/way_test.rb
@@ -96,4 +96,40 @@ class WayTest < ActiveSupport::TestCase
     }
     assert_match /Must specify a string with one or more characters/, message_update.message
   end
+  
+  def test_from_xml_no_k_v
+    nokv = "<osm><way id='23' changeset='23' version='23'><tag /></way></osm>"
+    message_create = assert_raise(OSM::APIBadXMLError) {
+      Way.from_xml(nokv, true)
+    }
+    assert_match /tag is missing key/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
+      Way.from_xml(nokv, false)
+    }
+    assert_match /tag is missing key/, message_update.message
+  end
+  
+  def test_from_xml_no_v
+    no_v = "<osm><way id='23' changeset='23' version='23'><tag k='key' /></way></osm>"
+    message_create = assert_raise(OSM::APIBadXMLError) {
+      Way.from_xml(no_v, true)
+    }
+    assert_match /tag is missing value/, message_create.message
+    message_update = assert_raise(OSM::APIBadXMLError) {
+      Way.from_xml(no_v, false)
+    }
+    assert_match /tag is missing value/, message_update.message
+  end
+  
+  def test_from_xml_duplicate_k
+    dupk = "<osm><way id='23' changeset='23' version='23'><tag k='dup' v='test' /><tag k='dup' v='tester' /></way></osm>"
+    message_create = assert_raise(OSM::APIDuplicateTagsError) {
+      Way.from_xml(dupk, true)
+    }
+    assert_equal "Element way/ has duplicate tags with key dup", message_create.message
+    message_update = assert_raise(OSM::APIDuplicateTagsError) {
+      Way.from_xml(dupk, false)
+    }
+    assert_equal "Element way/23 has duplicate tags with key dup", message_update.message
+  end
 end