]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/2207'
authorTom Hughes <tom@compton.nu>
Wed, 26 Jun 2019 13:30:30 +0000 (14:30 +0100)
committerTom Hughes <tom@compton.nu>
Wed, 26 Jun 2019 13:30:30 +0000 (14:30 +0100)
1  2 
app/controllers/api/users_controller.rb
app/models/trace.rb
test/controllers/api/traces_controller_test.rb

index d765b4904e43774dfd879241954dca1366cf12fd,43d5ec90883752b3212a9dfb73e32e5633b53de9..d3387bd5ffaaa1f48a0e32dff1b32d25ae3eb639
@@@ -13,7 -13,7 +13,7 @@@ module Ap
  
      def show
        if @user.visible?
 -        render :action => :show, :content_type => "text/xml"
 +        render :content_type => "text/xml"
        else
          head :gone
        end
  
        @users = User.visible.find(ids)
  
 -      render :action => :index, :content_type => "text/xml"
 +      render :content_type => "text/xml"
      end
  
      def gpx_files
-       doc = OSM::API.new.get_xml_doc
-       current_user.traces.reload.each do |trace|
-         doc.root << trace.to_xml_node
-       end
-       render :xml => doc.to_s
+       @traces = current_user.traces.reload
+       render :content_type => "application/xml"
      end
  
      private
diff --combined app/models/trace.rb
index 9d710d1ceca99b540065aa6822be470c40cb3702,52812cfc0d37ea9c250eec3d267e781d3f2a29b8..adaba52ae23b87b301598f5d32e26dc8787224e7
@@@ -2,11 -2,11 +2,11 @@@
  #
  # Table name: gpx_files
  #
 -#  id          :integer          not null, primary key
 -#  user_id     :integer          not null
 +#  id          :bigint(8)        not null, primary key
 +#  user_id     :bigint(8)        not null
  #  visible     :boolean          default(TRUE), not null
  #  name        :string           default(""), not null
 -#  size        :integer
 +#  size        :bigint(8)
  #  latitude    :float
  #  longitude   :float
  #  timestamp   :datetime         not null
@@@ -43,7 -43,12 +43,7 @@@ class Trace < ActiveRecord::Bas
    validates :timestamp, :presence => true
    validates :visibility, :inclusion => %w[private public trackable identifiable]
  
 -  def destroy
 -    super
 -    FileUtils.rm_f(trace_name)
 -    FileUtils.rm_f(icon_picture_name)
 -    FileUtils.rm_f(large_picture_name)
 -  end
 +  after_destroy :remove_files
  
    def tagstring
      tags.collect(&:tag).join(", ")
      extension
    end
  
-   def to_xml
-     doc = OSM::API.new.get_xml_doc
-     doc.root << to_xml_node
-     doc
-   end
-   def to_xml_node
-     el1 = XML::Node.new "gpx_file"
-     el1["id"] = id.to_s
-     el1["name"] = name.to_s
-     el1["lat"] = latitude.to_s if inserted
-     el1["lon"] = longitude.to_s if inserted
-     el1["user"] = user.display_name
-     el1["visibility"] = visibility
-     el1["pending"] = inserted ? "false" : "true"
-     el1["timestamp"] = timestamp.xmlschema
-     el2 = XML::Node.new "description"
-     el2 << description
-     el1 << el2
-     tags.each do |tag|
-       el2 = XML::Node.new("tag")
-       el2 << tag.tag
-       el1 << el2
-     end
-     el1
-   end
    def update_from_xml(xml, create = false)
      p = XML::Parser.string(xml, :options => XML::Parser::Options::NOERROR)
      doc = p.parse
      end
  
      raise OSM::APIBadXMLError.new("trace", xml, "XML doesn't contain an osm/gpx_file element.")
 -  rescue LibXML::XML::Error, ArgumentError => ex
 -    raise OSM::APIBadXMLError.new("trace", xml, ex.message)
 +  rescue LibXML::XML::Error, ArgumentError => e
 +    raise OSM::APIBadXMLError.new("trace", xml, e.message)
    end
  
    def update_from_xml_node(pt, create = false)
    def import
      logger.info("GPX Import importing #{name} (#{id}) from #{user.email}")
  
 -    gpx = ::GPX::File.new(xml_file)
 +    gpx = ::GPX::File.new(trace_name)
  
      f_lat = 0
      f_lon = 0
  
      gpx
    end
 +
 +  private
 +
 +  def remove_files
 +    FileUtils.rm_f(trace_name)
 +    FileUtils.rm_f(icon_picture_name)
 +    FileUtils.rm_f(large_picture_name)
 +  end
  end
index 499fefbd7e2ee507dc708877350c3a6dd81f5580,100bf5772afa1d9a34f3becc0136b5db5ff271a1..820829aadc6f21a79e230bd3207867834262a53e
@@@ -153,7 -153,7 +153,7 @@@ module Ap
        # And finally we should be able to do it with the owner of the trace
        basic_authorization anon_trace_file.user.display_name, "test"
        get :data, :params => { :id => anon_trace_file.id }
 -      check_trace_data anon_trace_file, "66179ca44f1e93d8df62e2b88cbea732"
 +      check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701"
      end
  
      # Test downloading a trace that doesn't exist through the api
        anon_trace_file = create(:trace, :visibility => "private")
  
        # First with no auth
-       put :update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
+       put :update, :params => { :id => public_trace_file.id }, :body => create_trace_xml(public_trace_file)
        assert_response :unauthorized
  
        # Now with some other user, which should fail
        basic_authorization create(:user).display_name, "test"
-       put :update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
+       put :update, :params => { :id => public_trace_file.id }, :body => create_trace_xml(public_trace_file)
        assert_response :forbidden
  
        # Now with a trace which doesn't exist
        basic_authorization create(:user).display_name, "test"
-       put :update, :params => { :id => 0 }, :body => public_trace_file.to_xml.to_s
+       put :update, :params => { :id => 0 }, :body => create_trace_xml(public_trace_file)
        assert_response :not_found
  
        # Now with a trace which did exist but has been deleted
        basic_authorization deleted_trace_file.user.display_name, "test"
-       put :update, :params => { :id => deleted_trace_file.id }, :body => deleted_trace_file.to_xml.to_s
+       put :update, :params => { :id => deleted_trace_file.id }, :body => create_trace_xml(deleted_trace_file)
        assert_response :not_found
  
        # Now try an update with the wrong ID
        basic_authorization public_trace_file.user.display_name, "test"
-       put :update, :params => { :id => public_trace_file.id }, :body => anon_trace_file.to_xml.to_s
+       put :update, :params => { :id => public_trace_file.id }, :body => create_trace_xml(anon_trace_file)
        assert_response :bad_request,
                        "should not be able to update a trace with a different ID from the XML"
  
        t = public_trace_file
        t.description = "Changed description"
        t.visibility = "private"
-       put :update, :params => { :id => t.id }, :body => t.to_xml.to_s
+       put :update, :params => { :id => t.id }, :body => create_trace_xml(t)
        assert_response :success
        nt = Trace.find(t.id)
        assert_equal nt.description, t.description
        trace = tracetag.trace
        basic_authorization trace.user.display_name, "test"
  
-       put :update, :params => { :id => trace.id }, :body => trace.to_xml.to_s
+       put :update, :params => { :id => trace.id }, :body => create_trace_xml(trace)
        assert_response :success
  
        updated = Trace.find(trace.id)
        assert_equal content_type, response.content_type
        assert_equal "attachment; filename=\"#{trace.id}.#{extension}\"", @response.header["Content-Disposition"]
      end
+     ##
+     # build XML for traces
+     # this builds a minimum viable XML for the tests in this suite
+     def create_trace_xml(trace)
+       root = XML::Document.new
+       root.root = XML::Node.new "osm"
+       trc = XML::Node.new "gpx_file"
+       trc["id"] = trace.id.to_s
+       trc["visibility"] = trace.visibility
+       trc["visible"] = trace.visible.to_s
+       desc = XML::Node.new "description"
+       desc << trace.description
+       trc << desc
+       trace.tags.each do |tag|
+         t = XML::Node.new "tag"
+         t << tag.tag
+         trc << t
+       end
+       root.root << trc
+       root.to_s
+     end
    end
  end