From: Tom Hughes Date: Wed, 26 Jun 2019 13:30:30 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/2207' X-Git-Tag: live~3579 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/cbea796ef412840af9a3ec2fd26bf08657fc4cd3?ds=sidebyside;hp=-c Merge remote-tracking branch 'upstream/pull/2207' --- cbea796ef412840af9a3ec2fd26bf08657fc4cd3 diff --combined app/controllers/api/users_controller.rb index d765b4904,43d5ec908..d3387bd5f --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@@ -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 @@@ -33,15 -33,12 +33,12 @@@ @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 9d710d1ce,52812cfc0..adaba52ae --- a/app/models/trace.rb +++ b/app/models/trace.rb @@@ -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(", ") @@@ -164,36 -169,6 +164,6 @@@ 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 @@@ -203,8 -178,8 +173,8 @@@ 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) @@@ -275,7 -250,7 +245,7 @@@ 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 @@@ -338,12 -313,4 +308,12 @@@ 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 diff --combined test/controllers/api/traces_controller_test.rb index 499fefbd7,100bf5772..820829aad --- a/test/controllers/api/traces_controller_test.rb +++ b/test/controllers/api/traces_controller_test.rb @@@ -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 @@@ -250,27 -250,27 +250,27 @@@ 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" @@@ -279,7 -279,7 +279,7 @@@ 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 @@@ -292,7 -292,7 +292,7 @@@ 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) @@@ -339,5 -339,27 +339,27 @@@ 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