From 17e6e69c39072d8e6cb316b75b311e89c92cca45 Mon Sep 17 00:00:00 2001 From: Frederik Ramm Date: Sat, 13 Apr 2019 01:44:15 +0200 Subject: [PATCH] Move generation of gpx_file XML from model to builder --- app/controllers/api/traces_controller.rb | 3 +- app/controllers/api/users_controller.rb | 7 ++-- app/models/trace.rb | 30 ---------------- app/views/api/traces/_trace.builder | 22 ++++++++++++ app/views/api/traces/trace.builder | 9 +++++ .../controllers/api/traces_controller_test.rb | 36 +++++++++++++++---- 6 files changed, 64 insertions(+), 43 deletions(-) create mode 100644 app/views/api/traces/_trace.builder create mode 100644 app/views/api/traces/trace.builder diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb index 86f1370f6..fd43a1d44 100644 --- a/app/controllers/api/traces_controller.rb +++ b/app/controllers/api/traces_controller.rb @@ -19,7 +19,8 @@ module Api trace = Trace.visible.find(params[:id]) if trace.public? || trace.user == current_user - render :xml => trace.to_xml.to_s + @traces = [trace] + render "trace" else head :forbidden end diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index 3180cabcc..e6922e0a0 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -37,11 +37,8 @@ module Api 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 "api/traces/trace", :content_type => "application/xml" end private diff --git a/app/models/trace.rb b/app/models/trace.rb index 0e8763328..52812cfc0 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -169,36 +169,6 @@ class Trace < ActiveRecord::Base 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 diff --git a/app/views/api/traces/_trace.builder b/app/views/api/traces/_trace.builder new file mode 100644 index 000000000..7efd640f5 --- /dev/null +++ b/app/views/api/traces/_trace.builder @@ -0,0 +1,22 @@ +# basic attributes + +attrs = { + "id" => trace.id, + "name" => trace.name, + "user" => trace.user.display_name, + "visibility" => trace.visibility, + "pending" => trace.inserted ? "false" : "true", + "timestamp" => trace.timestamp.xmlschema +} + +if trace.inserted + attrs["lat"] = trace.latitude.to_s + attrs["lon"] = trace.longitude.to_s +end + +xml.gpx_file(attrs) do |trace_xml_node| + trace_xml_node.description(trace.description) + trace.tags.each do |t| + trace_xml_node.tag(t.tag) + end +end diff --git a/app/views/api/traces/trace.builder b/app/views/api/traces/trace.builder new file mode 100644 index 000000000..1e278f770 --- /dev/null +++ b/app/views/api/traces/trace.builder @@ -0,0 +1,9 @@ +xml.instruct! :xml, :version => "1.0" + +# basic attributes + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + @traces.each do |trace| + osm << render(:partial => "api/traces/trace.builder", :locals => { :trace => trace }) + end +end diff --git a/test/controllers/api/traces_controller_test.rb b/test/controllers/api/traces_controller_test.rb index 24ff6ee66..100bf5772 100644 --- a/test/controllers/api/traces_controller_test.rb +++ b/test/controllers/api/traces_controller_test.rb @@ -250,27 +250,27 @@ module 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" @@ -279,7 +279,7 @@ module Api 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 @@ module Api 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 @@ module Api 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 -- 2.45.1