]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/3324'
authorTom Hughes <tom@compton.nu>
Wed, 15 Sep 2021 17:38:25 +0000 (18:38 +0100)
committerTom Hughes <tom@compton.nu>
Wed, 15 Sep 2021 17:38:25 +0000 (18:38 +0100)
13 files changed:
app/controllers/api/changesets_controller.rb
app/models/concerns/object_metadata.rb [deleted file]
app/models/node.rb
app/models/old_node.rb
app/models/old_relation.rb
app/models/old_way.rb
app/models/relation.rb
app/models/way.rb
app/views/api/changesets/download.xml.builder [new file with mode: 0644]
test/controllers/api/changesets_controller_test.rb
test/controllers/api/old_nodes_controller_test.rb
test/models/old_node_test.rb
test/test_helper.rb

index 34627a1187acca98b026da44c4c3ce5ecc0e0428..63fda31bdc297e0e509437da3d6ada61584197bd 100644 (file)
@@ -11,6 +11,8 @@ module Api
     before_action :require_public_data, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
     before_action :check_api_writable, :only => [:create, :update, :upload, :subscribe, :unsubscribe]
     before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :subscribe, :unsubscribe]
+    before_action :set_request_formats, :only => [:download]
+
     around_action :api_call_handle_error
     around_action :api_call_timeout, :except => [:upload]
 
@@ -122,35 +124,29 @@ module Api
         end
       end
 
-      # create changeset and user caches
-      changeset_cache = {}
-      user_display_name_cache = {}
-
-      # create an osmChange document for the output
-      result = OSM::API.new.get_xml_doc
-      result.root.name = "osmChange"
-
       # generate an output element for each operation. note: we avoid looking
       # at the history because it is simpler - but it would be more correct to
       # check these assertions.
+      @created = []
+      @modified = []
+      @deleted = []
+
       elements.each do |elt|
-        result.root <<
-          if elt.version == 1
-            # first version, so it must be newly-created.
-            created = XML::Node.new "create"
-            created << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          elsif elt.visible
-            # must be a modify
-            modified = XML::Node.new "modify"
-            modified << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          else
-            # if the element isn't visible then it must have been deleted
-            deleted = XML::Node.new "delete"
-            deleted << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          end
+        if elt.version == 1
+          # first version, so it must be newly-created.
+          @created << elt
+        elsif elt.visible
+          # must be a modify
+          @modified << elt
+        else
+          # if the element isn't visible then it must have been deleted
+          @deleted << elt
+        end
       end
 
-      render :xml => result.to_s
+      respond_to do |format|
+        format.xml
+      end
     end
 
     ##
diff --git a/app/models/concerns/object_metadata.rb b/app/models/concerns/object_metadata.rb
deleted file mode 100644 (file)
index dcfde88..0000000
+++ /dev/null
@@ -1,43 +0,0 @@
-module ObjectMetadata
-  extend ActiveSupport::Concern
-
-  def add_metadata_to_xml_node(el, osm, changeset_cache, user_display_name_cache)
-    el["changeset"] = osm.changeset_id.to_s
-    el["redacted"] = osm.redaction.id.to_s if osm.redacted?
-    el["timestamp"] = osm.timestamp.xmlschema
-    el["version"] = osm.version.to_s
-    el["visible"] = osm.visible.to_s
-
-    if changeset_cache.key?(osm.changeset_id)
-      # use the cache if available
-    else
-      changeset_cache[osm.changeset_id] = osm.changeset.user_id
-    end
-
-    user_id = changeset_cache[osm.changeset_id]
-
-    if user_display_name_cache.key?(user_id)
-      # use the cache if available
-    elsif osm.changeset.user.data_public?
-      user_display_name_cache[user_id] = osm.changeset.user.display_name
-    else
-      user_display_name_cache[user_id] = nil
-    end
-
-    unless user_display_name_cache[user_id].nil?
-      el["user"] = user_display_name_cache[user_id]
-      el["uid"] = user_id.to_s
-    end
-  end
-
-  def add_tags_to_xml_node(el, tags)
-    tags.each do |tag|
-      tag_el = XML::Node.new("tag")
-
-      tag_el["k"] = tag.k
-      tag_el["v"] = tag.v
-
-      el << tag_el
-    end
-  end
-end
index 5e799c8d9374b1562d61035b8b710f81f27a64de..8bfac993b5bd9fdd2d512a4007dfff1304cae2ec 100644 (file)
@@ -27,7 +27,6 @@ class Node < ApplicationRecord
   include GeoRecord
   include ConsistencyValidations
   include NotRedactable
-  include ObjectMetadata
 
   self.table_name = "current_nodes"
 
index 620cc24d227029091e610bfccb0813bb792c0449..81d8e331fcf5e1fc507e5a19119cfc5773e9ab5f 100644 (file)
@@ -27,7 +27,6 @@
 class OldNode < ApplicationRecord
   include GeoRecord
   include ConsistencyValidations
-  include ObjectMetadata
 
   self.table_name = "nodes"
   self.primary_keys = "node_id", "version"
@@ -69,28 +68,6 @@ class OldNode < ApplicationRecord
     old_node
   end
 
-  def to_xml
-    doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node
-    doc
-  end
-
-  def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
-    el = XML::Node.new "node"
-    el["id"] = node_id.to_s
-
-    add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
-
-    if visible?
-      el["lat"] = lat.to_s
-      el["lon"] = lon.to_s
-    end
-
-    add_tags_to_xml_node(el, old_tags)
-
-    el
-  end
-
   def save_with_dependencies!
     save!
 
index 51aeb9c4c533f296eaa5a1eaa7a113cca93b46e9..d0a03ac25b01baf942480f6b6e5f3124d846575c 100644 (file)
@@ -22,7 +22,6 @@
 
 class OldRelation < ApplicationRecord
   include ConsistencyValidations
-  include ObjectMetadata
 
   self.table_name = "relations"
   self.primary_keys = "relation_id", "version"
@@ -88,31 +87,6 @@ class OldRelation < ApplicationRecord
 
   attr_writer :members, :tags
 
-  def to_xml
-    doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node
-    doc
-  end
-
-  def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
-    el = XML::Node.new "relation"
-    el["id"] = relation_id.to_s
-
-    add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
-
-    old_members.each do |member|
-      member_el = XML::Node.new "member"
-      member_el["type"] = member.member_type.to_s.downcase
-      member_el["ref"] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML
-      member_el["role"] = member.member_role.to_s
-      el << member_el
-    end
-
-    add_tags_to_xml_node(el, old_tags)
-
-    el
-  end
-
   # Temporary method to match interface to relations
   def relation_members
     old_members
index 3260f2a1f57acbed3755fe691949d8e14fa1fbbc..9acf8665d2a5fa144c927b9da2763874599f1a95 100644 (file)
@@ -22,7 +22,6 @@
 
 class OldWay < ApplicationRecord
   include ConsistencyValidations
-  include ObjectMetadata
 
   self.table_name = "ways"
   self.primary_keys = "way_id", "version"
@@ -86,23 +85,6 @@ class OldWay < ApplicationRecord
 
   attr_writer :nds, :tags
 
-  def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
-    el = XML::Node.new "way"
-    el["id"] = way_id.to_s
-
-    add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
-
-    old_nodes.each do |nd| # FIXME: need to make sure they come back in the right order
-      node_el = XML::Node.new "nd"
-      node_el["ref"] = nd.node_id.to_s
-      el << node_el
-    end
-
-    add_tags_to_xml_node(el, old_tags)
-
-    el
-  end
-
   # Temporary method to match interface to ways
   def way_nodes
     old_nodes
index 365ea533e11259aa3f9ae9ebc495cb990b400c10..25564940bdd1a4013cf2ae3a533b3fdf2afc4bde 100644 (file)
@@ -22,7 +22,6 @@ class Relation < ApplicationRecord
 
   include ConsistencyValidations
   include NotRedactable
-  include ObjectMetadata
 
   self.table_name = "current_relations"
 
index 7bb82b281bc4e10603ea2ffc29eee928c4ff9709..724965ddfe4bc1a24b4de482bf6529ca131eeb90 100644 (file)
@@ -22,7 +22,6 @@ class Way < ApplicationRecord
 
   include ConsistencyValidations
   include NotRedactable
-  include ObjectMetadata
 
   self.table_name = "current_ways"
 
diff --git a/app/views/api/changesets/download.xml.builder b/app/views/api/changesets/download.xml.builder
new file mode 100644 (file)
index 0000000..1e400cd
--- /dev/null
@@ -0,0 +1,19 @@
+xml.instruct! :xml, :version => "1.0"
+
+xml.osmChange(OSM::API.new.xml_root_attributes) do |osm|
+  @created.each do |elt|
+    osm.create do |create|
+      create << render(elt)
+    end
+  end
+  @modified.each do |elt|
+    osm.modify do |modify|
+      modify << render(elt)
+    end
+  end
+  @deleted.each do |elt|
+    osm.delete do |delete|
+      delete << render(elt)
+    end
+  end
+end
index 8e8f4c1850523aa23ab5f3f9d3490879f0b51bf3..f3de0682b53ad1ca28da38b400f1436f1fb7dd7f 100644 (file)
@@ -1428,7 +1428,6 @@ module Api
       get changeset_download_path(changeset)
 
       assert_response :success
-      assert_template nil
       # print @response.body
       # FIXME: needs more assert_select tests
       assert_select "osmChange[version='#{Settings.api_version}'][generator='#{Settings.generator}']" do
index e85bc86f85044592d895fc2a6186db818464a7ae..8fc19145c066c29f6d6b896db5a2c291eddc845b 100644 (file)
@@ -187,6 +187,15 @@ module Api
       check_current_version(node_with_versions)
     end
 
+    # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
+    def test_lat_lon_xml_format
+      old_node = create(:old_node, :latitude => (0.00004 * OldNode::SCALE).to_i, :longitude => (0.00008 * OldNode::SCALE).to_i)
+
+      get api_node_history_path(:id => old_node.node_id, :version => old_node.version)
+      assert_match(/lat="0.0000400"/, response.body)
+      assert_match(/lon="0.0000800"/, response.body)
+    end
+
     ##
     # test the redaction of an old version of a node, while not being
     # authorised.
index eade6130a11873a0768e1034909ad61713172ad0..cc0e0d8c234390768c8ac501c7424bf59ecd42cf 100644 (file)
@@ -68,14 +68,6 @@ class OldNodeTest < ActiveSupport::TestCase
     assert_in_delta 76.543 * OldNode::SCALE, node.longitude, 0.000001
   end
 
-  # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
-  def test_lat_lon_xml_format
-    old_node = build(:old_node, :latitude => 0.00004 * OldNode::SCALE, :longitude => 0.00008 * OldNode::SCALE)
-
-    assert_match(/lat="0.0000400"/, old_node.to_xml.to_s)
-    assert_match(/lon="0.0000800"/, old_node.to_xml.to_s)
-  end
-
   def test_node_tags
     node_v1 = create(:old_node, :version => 1)
     node_v2 = create(:old_node, :node_id => node_v1.node_id, :version => 2)
index 6c8a798aac1d4106b6ac29dec0957c216d7323d6..421ae8698847a0d1cbe7090045b69e37d37db389 100644 (file)
@@ -264,14 +264,14 @@ module ActiveSupport
       el = XML::Node.new "node"
       el["id"] = node.id.to_s
 
-      OMHelper.add_metadata_to_xml_node(el, node, {}, {})
+      add_metadata_to_xml_node(el, node, {}, {})
 
       if node.visible?
         el["lat"] = node.lat.to_s
         el["lon"] = node.lon.to_s
       end
 
-      OMHelper.add_tags_to_xml_node(el, node.node_tags)
+      add_tags_to_xml_node(el, node.node_tags)
 
       el
     end
@@ -286,7 +286,7 @@ module ActiveSupport
       el = XML::Node.new "way"
       el["id"] = way.id.to_s
 
-      OMHelper.add_metadata_to_xml_node(el, way, {}, {})
+      add_metadata_to_xml_node(el, way, {}, {})
 
       # make sure nodes are output in sequence_id order
       ordered_nodes = []
@@ -302,7 +302,7 @@ module ActiveSupport
         el << node_el
       end
 
-      OMHelper.add_tags_to_xml_node(el, way.way_tags)
+      add_tags_to_xml_node(el, way.way_tags)
 
       el
     end
@@ -317,7 +317,7 @@ module ActiveSupport
       el = XML::Node.new "relation"
       el["id"] = relation.id.to_s
 
-      OMHelper.add_metadata_to_xml_node(el, relation, {}, {})
+      add_metadata_to_xml_node(el, relation, {}, {})
 
       relation.relation_members.each do |member|
         member_el = XML::Node.new "member"
@@ -327,13 +327,49 @@ module ActiveSupport
         el << member_el
       end
 
-      OMHelper.add_tags_to_xml_node(el, relation.relation_tags)
+      add_tags_to_xml_node(el, relation.relation_tags)
 
       el
     end
 
-    class OMHelper
-      extend ObjectMetadata
+    def add_metadata_to_xml_node(el, osm, changeset_cache, user_display_name_cache)
+      el["changeset"] = osm.changeset_id.to_s
+      el["redacted"] = osm.redaction.id.to_s if osm.redacted?
+      el["timestamp"] = osm.timestamp.xmlschema
+      el["version"] = osm.version.to_s
+      el["visible"] = osm.visible.to_s
+
+      if changeset_cache.key?(osm.changeset_id)
+        # use the cache if available
+      else
+        changeset_cache[osm.changeset_id] = osm.changeset.user_id
+      end
+
+      user_id = changeset_cache[osm.changeset_id]
+
+      if user_display_name_cache.key?(user_id)
+        # use the cache if available
+      elsif osm.changeset.user.data_public?
+        user_display_name_cache[user_id] = osm.changeset.user.display_name
+      else
+        user_display_name_cache[user_id] = nil
+      end
+
+      unless user_display_name_cache[user_id].nil?
+        el["user"] = user_display_name_cache[user_id]
+        el["uid"] = user_id.to_s
+      end
+    end
+
+    def add_tags_to_xml_node(el, tags)
+      tags.each do |tag|
+        tag_el = XML::Node.new("tag")
+
+        tag_el["k"] = tag.k
+        tag_el["v"] = tag.v
+
+        el << tag_el
+      end
     end
   end
 end