Merge branch 'master' into openid

Conflicts:
	app/controllers/user_controller.rb
This commit is contained in:
Tom Hughes 2010-10-17 10:59:29 +01:00
commit e09b187cae
13 changed files with 259 additions and 68 deletions

View file

@ -4,22 +4,22 @@ class TraceController < ApplicationController
before_filter :authorize_web
before_filter :set_locale
before_filter :require_user, :only => [:mine, :create, :edit, :delete]
before_filter :authorize, :only => [:api_details, :api_data, :api_create]
before_filter :check_database_readable, :except => [:api_details, :api_data, :api_create]
before_filter :check_database_writable, :only => [:create, :edit, :delete]
before_filter :check_api_readable, :only => [:api_details, :api_data]
before_filter :check_api_writable, :only => [:api_create]
before_filter :require_allow_read_gpx, :only => [:api_details, :api_data]
before_filter :require_allow_write_gpx, :only => [:api_create]
before_filter :authorize, :only => [:api_create, :api_read, :api_update, :api_delete, :api_data]
before_filter :check_database_readable, :except => [:api_read, :api_data]
before_filter :check_database_writable, :only => [:create, :edit, :delete, :api_create, :api_update, :api_delete]
before_filter :check_api_readable, :only => [:api_read, :api_data]
before_filter :check_api_writable, :only => [:api_create, :api_update, :api_delete]
before_filter :require_allow_read_gpx, :only => [:api_read, :api_data]
before_filter :require_allow_write_gpx, :only => [:api_create, :api_update, :api_delete]
before_filter :offline_warning, :only => [:mine, :view]
before_filter :offline_redirect, :only => [:create, :edit, :delete, :data, :api_data, :api_create]
around_filter :api_call_handle_error, :only => [:api_details, :api_data, :api_create]
before_filter :offline_redirect, :only => [:create, :edit, :delete, :data, :api_create, :api_delete, :api_data]
around_filter :api_call_handle_error, :only => [:api_create, :api_read, :api_update, :api_delete, :api_data]
caches_action :list, :unless => :logged_in?, :layout => false
caches_action :view, :layout => false
caches_action :georss, :layout => true
cache_sweeper :trace_sweeper, :only => [:create, :edit, :delete, :api_create], :unless => STATUS == :database_offline
cache_sweeper :tracetag_sweeper, :only => [:create, :edit, :delete, :api_create], :unless => STATUS == :database_offline
cache_sweeper :trace_sweeper, :only => [:create, :edit, :delete, :api_create, :api_update, :api_delete], :unless => STATUS == :database_offline
cache_sweeper :tracetag_sweeper, :only => [:create, :edit, :delete, :api_create, :api_update, :api_delete], :unless => STATUS == :database_offline
# Counts and selects pages of GPX traces for various criteria (by user, tags, public etc.).
# target_user - if set, specifies the user to fetch traces for. if not set will fetch all traces
@ -280,16 +280,48 @@ class TraceController < ApplicationController
render :nothing => true, :status => :not_found
end
def api_details
trace = Trace.find(params[:id])
def api_read
trace = Trace.find(params[:id], :conditions => { :visible => true })
if trace.public? or trace.user == @user
render :text => trace.to_xml.to_s, :content_type => "text/xml"
else
render :nothing => true, :status => :forbidden
end
rescue ActiveRecord::RecordNotFound
render :nothing => true, :status => :not_found
end
def api_update
trace = Trace.find(params[:id], :conditions => { :visible => true })
if trace.user == @user
new_trace = Trace.from_xml(request.raw_post)
unless new_trace and new_trace.id == trace.id
raise OSM::APIBadUserInput.new("The id in the url (#{trace.id}) is not the same as provided in the xml (#{new_trace.id})")
end
trace.description = new_trace.description
trace.tags = new_trace.tags
trace.visibility = new_trace.visibility
trace.save!
render :nothing => true, :status => :ok
else
render :nothing => true, :status => :forbidden
end
end
def api_delete
trace = Trace.find(params[:id], :conditions => { :visible => true })
if trace.user == @user
trace.visible = false
trace.save!
render :nothing => true, :status => :ok
else
render :nothing => true, :status => :forbidden
end
end
def api_data
@ -304,8 +336,6 @@ class TraceController < ApplicationController
else
render :nothing => true, :status => :forbidden
end
rescue ActiveRecord::RecordNotFound
render :nothing => true, :status => :not_found
end
def api_create

View file

@ -55,8 +55,14 @@ class UserController < ApplicationController
if @user
if @user.invalid?
# Something is wrong, so rerender the form
render :action => :new
if @user.new_record?
# Something is wrong with a new user, so rerender the form
render :action => :new
else
# Error in existing user, so go to account settings
flash[:errors] = @user.errors
redirect_to :action => :account, :display_name => @user.display_name
end
elsif @user.terms_agreed?
# Already agreed to terms, so just show settings
redirect_to :action => :account, :display_name => @user.display_name
@ -153,6 +159,13 @@ class UserController < ApplicationController
openid_verify(nil, @user) do |user|
update_user(user)
end
else
if flash[:errors]
flash[:errors].each do |attr,msg|
attr = "new_email" if attr == "email" and !@user.new_email.nil?
@user.errors.add(attr,msg)
end
end
end
end

View file

@ -149,15 +149,72 @@ class Trace < ActiveRecord::Base
el1 = XML::Node.new 'gpx_file'
el1['id'] = self.id.to_s
el1['name'] = self.name.to_s
el1['lat'] = self.latitude.to_s
el1['lon'] = self.longitude.to_s
el1['lat'] = self.latitude.to_s if self.inserted
el1['lon'] = self.longitude.to_s if self.inserted
el1['user'] = self.user.display_name
el1['visibility'] = self.visibility
el1['pending'] = (!self.inserted).to_s
el1['timestamp'] = self.timestamp.xmlschema
el2 = XML::Node.new 'description'
el2 << self.description
el1 << el2
self.tags.each do |tag|
el2 = XML::Node.new('tag')
el2 << tag.tag
el1 << el2
end
return el1
end
# Read in xml as text and return it's Node object representation
def self.from_xml(xml, create=false)
begin
p = XML::Parser.string(xml)
doc = p.parse
doc.find('//osm/gpx_file').each do |pt|
return Trace.from_xml_node(pt, create)
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)
end
end
def self.from_xml_node(pt, create=false)
trace = Trace.new
raise OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt['visibility'].nil?
trace.visibility = pt['visibility']
unless create
raise OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt['id'].nil?
trace.id = pt['id'].to_i
# .to_i will return 0 if there is no number that can be parsed.
# We want to make sure that there is no id with zero anyway
raise OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if trace.id == 0
end
# We don't care about the time, as it is explicitly set on create/update/delete
# We don't care about the visibility as it is implicit based on the action
# and set manually before the actual delete
trace.visible = true
description = pt.find('description').first
raise OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil?
trace.description = description.content
pt.find('tag').each do |tag|
trace.tags.build(:tag => tag.content)
end
return trace
end
def xml_file
# TODO *nix specific, could do to work on windows... would be functionally inferior though - check for '.gz'
filetype = `/usr/bin/file -bz #{trace_name}`.chomp

View file

@ -17,11 +17,12 @@
</noscript>
<div id="map">
<div id="permalink">
<a href="/" id="permalinkanchor"><%= t 'site.index.permalink' %></a><br/>
<a href="/" id="shortlinkanchor"><%= t 'site.index.shortlink' %></a>
</div>
</div>
</div>
<div id="permalink">
<a href="/" id="permalinkanchor"><%= t 'site.index.permalink' %></a><br/>
<a href="/" id="shortlinkanchor"><%= t 'site.index.shortlink' %></a>
</div>
<div id="attribution">
<table width="100%">
@ -40,7 +41,7 @@
</div>
<%
if params['mlon'] and params['mlat']
if params['mlon'] and params['mlat']
marker = true
mlon = h(params['mlon'])
mlat = h(params['mlat'])
@ -68,7 +69,7 @@ if params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat
minlat = h(params['minlat'])
maxlon = h(params['maxlon'])
maxlat = h(params['maxlat'])
box = true if params['box']=="yes"
box = true if params['box']=="yes"
object_zoom = false
end
@ -80,14 +81,14 @@ if params['lon'] and params['lat']
layers = h(params['layers'])
object_zoom = false
elsif params['mlon'] and params['mlat']
lon = h(params['mlon'])
lon = h(params['mlon'])
lat = h(params['mlat'])
zoom = h(params['zoom'] || '12')
layers = h(params['layers'])
object_zoom = false
elsif cookies.key?("_osm_location")
lon,lat,zoom,layers = cookies["_osm_location"].split("|")
elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil?
elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil?
lon = @user.home_lon
lat = @user.home_lat
zoom = '10'
@ -101,12 +102,12 @@ else
maxlon = session[:location][:maxlon]
maxlat = session[:location][:maxlat]
else
lon = '-0.1'
lat = '51.5'
zoom = h(params['zoom'] || '5')
lon = '-0.1'
lat = '51.5'
zoom = h(params['zoom'] || '5')
end
layers = h(params['layers'])
layers = h(params['layers'])
end
%>
@ -229,7 +230,7 @@ end
updatelinks(lonlat.lon, lonlat.lat, zoom, layers, extents.left, extents.bottom, extents.right, extents.top, objtype, objid);
expiry.setYear(expiry.getFullYear() + 10);
expiry.setYear(expiry.getFullYear() + 10);
document.cookie = "_osm_location=" + lonlat.lon + "|" + lonlat.lat + "|" + zoom + "|" + layers + "; expires=" + expiry.toGMTString();
}
@ -241,7 +242,7 @@ end
content.style.width = document.documentElement.clientWidth - content.offsetLeft - rightMargin;
content.style.height = document.documentElement.clientHeight - content.offsetTop - bottomMargin;
}
function resizeMap() {
var centre = map.getCenter();
var zoom = map.getZoom();
@ -265,7 +266,7 @@ end
resizeMap();
}
mapInit();
window.onload = handleResize;

View file

@ -1,5 +1,4 @@
<div id="mapkey">
<h3><%= t "site.key.table.heading", :zoom_level => params[:zoom] %></h3>
<table class="mapkey-table">
<% YAML.load_file("#{RAILS_ROOT}/config/key.yml").each do |name,data| %>
<% if params[:layer] == name %>

View file

@ -1246,9 +1246,8 @@ en:
search_help: "examples: 'Alkmaar', 'Regent Street, Cambridge', 'CB2 5AQ', or 'post offices near Lünen' <a href='http://wiki.openstreetmap.org/wiki/Search'>more examples...</a>"
key:
map_key: "Map key"
map_key_tooltip: "Map key for the mapnik rendering at this zoom level"
map_key_tooltip: "Key for the map"
table:
heading: "Legend for z{{zoom_level}}"
entry:
motorway: "Motorway"
trunk: "Trunk road"

View file

@ -62,9 +62,12 @@ ActionController::Routing::Routes.draw do |map|
map.connect "api/#{API_VERSION}/user/gpx_files", :controller => 'user', :action => 'api_gpx_files'
map.connect "api/#{API_VERSION}/gpx/create", :controller => 'trace', :action => 'api_create'
map.connect "api/#{API_VERSION}/gpx/:id/details", :controller => 'trace', :action => 'api_details'
map.connect "api/#{API_VERSION}/gpx/:id/data", :controller => 'trace', :action => 'api_data'
map.connect "api/#{API_VERSION}/gpx/:id/data.:format", :controller => 'trace', :action => 'api_data'
map.connect "api/#{API_VERSION}/gpx/:id", :controller => 'trace', :action => 'api_read', :id => /\d+/, :conditions => { :method => :get }
map.connect "api/#{API_VERSION}/gpx/:id", :controller => 'trace', :action => 'api_update', :id => /\d+/, :conditions => { :method => :put }
map.connect "api/#{API_VERSION}/gpx/:id", :controller => 'trace', :action => 'api_delete', :id => /\d+/, :conditions => { :method => :delete }
map.connect "api/#{API_VERSION}/gpx/:id/details", :controller => 'trace', :action => 'api_read', :id => /\d+/
map.connect "api/#{API_VERSION}/gpx/:id/data", :controller => 'trace', :action => 'api_data', :id => /\d+/
map.connect "api/#{API_VERSION}/gpx/:id/data.:format", :controller => 'trace', :action => 'api_data', :id => /\d+/
# AMF (ActionScript) API

View file

@ -12,7 +12,7 @@ logger = ActiveRecord::Base.logger
while(true) do
ActiveRecord::Base.logger.info("GPX Import daemon wake @ #{Time.now}.")
Trace.find(:all, :conditions => "inserted = 0 and visible = 1", :order => "id").each do |trace|
Trace.find(:all, :conditions => { :inserted => false, :visible => true }, :order => "id").each do |trace|
Signal.trap("TERM") do
terminated = true
end
@ -38,7 +38,7 @@ while(true) do
exit if terminated
end
Trace.find(:all, :conditions => "visible = 0", :order => "id").each do |trace|
Trace.find(:all, :conditions => { :visible => false }, :order => "id").each do |trace|
Signal.trap("TERM") do
terminated = true
end

View file

@ -23,7 +23,7 @@ module GPX
point = nil
while reader.read > 0
while reader.read
if reader.node_type == XML::Reader::TYPE_ELEMENT
if reader.name == "trkpt"
point = TrkPt.new(@tracksegs, reader["lat"].to_f, reader["lon"].to_f)

View file

@ -335,12 +335,12 @@ hr {
display: none !important;
}
#map #permalink {
z-index:10000;
position:absolute;
bottom:15px;
right:15px;
font-size:smaller;
#permalink {
z-index: 10000;
position: absolute;
bottom: 15px;
right: 15px;
font-size: smaller;
text-align: right;
}

View file

@ -14,7 +14,7 @@ public_trace_file:
anon_trace_file:
id: 2
user_id: 2
visible: false
visible: true
name: Private Trace.gpx
size: 123
latitude: 51.3
@ -27,7 +27,7 @@ anon_trace_file:
trackable_trace_file:
id: 3
user_id: 2
visible: false
visible: true
name: Trackable Trace.gpx
size: 123
latitude: 51.51
@ -40,7 +40,7 @@ trackable_trace_file:
identifiable_trace_file:
id: 4
user_id: 2
visible: false
visible: true
name: Identifiable Trace.gpx
size: 123
latitude: 51.512
@ -49,3 +49,16 @@ identifiable_trace_file:
visibility: "identifiable"
description: This trace shows trksegs, timestamps and user details.
inserted: true
deleted_trace_file:
id: 5
user_id: 2
visible: false
name: Deleted Trace.gpx
size: 123
latitude: 51.512
longitude: 0.142
timestamp: "2009-07-30 17:48:34"
visibility: "public"
description: This is a trace that has been deleted.
inserted: true

View file

@ -4,7 +4,6 @@ class TraceControllerTest < ActionController::TestCase
fixtures :users, :gpx_files
set_fixture_class :gpx_files => 'Trace'
# Check that the list of changesets is displayed
def test_list
get :list
@ -39,48 +38,125 @@ class TraceControllerTest < ActionController::TestCase
end
# Check getting a specific trace through the api
def test_api_details
def test_api_read
# First with no auth
get :api_details, :id => gpx_files(:public_trace_file).id
get :api_read, :id => gpx_files(:public_trace_file).id
assert_response :unauthorized
# Now with some other user, which should work since the trace is public
basic_authorization(users(:public_user).display_name, "test")
get :api_details, :id => gpx_files(:public_trace_file).id
get :api_read, :id => gpx_files(:public_trace_file).id
assert_response :success
# And finally we should be able to do it with the owner of the trace
basic_authorization(users(:normal_user).display_name, "test")
get :api_details, :id => gpx_files(:public_trace_file).id
get :api_read, :id => gpx_files(:public_trace_file).id
assert_response :success
end
# Check an anoymous trace can't be specifically fetched by another user
def test_api_details_anon
def test_api_read_anon
# Furst with no auth
get :api_details, :id => gpx_files(:anon_trace_file).id
get :api_read, :id => gpx_files(:anon_trace_file).id
assert_response :unauthorized
# Now try with another user, which shouldn't work since the trace is anon
basic_authorization(users(:normal_user).display_name, "test")
get :api_details, :id => gpx_files(:anon_trace_file).id
get :api_read, :id => gpx_files(:anon_trace_file).id
assert_response :forbidden
# And finally we should be able to get the trace details with the trace owner
basic_authorization(users(:public_user).display_name, "test")
get :api_details, :id => gpx_files(:anon_trace_file).id
get :api_read, :id => gpx_files(:anon_trace_file).id
assert_response :success
end
# Check the api details for a trace that doesn't exist
def test_api_details_not_found
def test_api_read_not_found
# Try first with no auth, as it should requure it
get :api_details, :id => 0
get :api_read, :id => 0
assert_response :unauthorized
# Login, and try again
basic_authorization(users(:public_user).display_name, "test")
get :api_details, :id => 0
get :api_read, :id => 0
assert_response :not_found
# Now try a trace which did exist but has been deleted
basic_authorization(users(:public_user).display_name, "test")
get :api_read, :id => 5
assert_response :not_found
end
# Check updating a trace through the api
def test_api_update
# First with no auth
content gpx_files(:public_trace_file).to_xml
put :api_update, :id => gpx_files(:public_trace_file).id
assert_response :unauthorized
# Now with some other user, which should fail
basic_authorization(users(:public_user).display_name, "test")
content gpx_files(:public_trace_file).to_xml
put :api_update, :id => gpx_files(:public_trace_file).id
assert_response :forbidden
# Now with a trace which doesn't exist
basic_authorization(users(:public_user).display_name, "test")
content gpx_files(:public_trace_file).to_xml
put :api_update, :id => 0
assert_response :not_found
# Now with a trace which did exist but has been deleted
basic_authorization(users(:public_user).display_name, "test")
content gpx_files(:deleted_trace_file).to_xml
put :api_update, :id => gpx_files(:deleted_trace_file).id
assert_response :not_found
# Now try an update with the wrong ID
basic_authorization(users(:normal_user).display_name, "test")
content gpx_files(:anon_trace_file).to_xml
put :api_update, :id => gpx_files(:public_trace_file).id
assert_response :bad_request,
"should not be able to update a trace with a different ID from the XML"
# And finally try an update that should work
basic_authorization(users(:normal_user).display_name, "test")
t = gpx_files(:public_trace_file)
t.description = "Changed description"
t.visibility = "private"
content t.to_xml
put :api_update, :id => t.id
assert_response :success
nt = Trace.find(t.id)
assert_equal nt.description, t.description
assert_equal nt.visibility, t.visibility
end
# Check deleting a trace through the api
def test_api_delete
# First with no auth
delete :api_delete, :id => gpx_files(:public_trace_file).id
assert_response :unauthorized
# Now with some other user, which should fail
basic_authorization(users(:public_user).display_name, "test")
delete :api_delete, :id => gpx_files(:public_trace_file).id
assert_response :forbidden
# Now with a trace which doesn't exist
basic_authorization(users(:public_user).display_name, "test")
delete :api_delete, :id => 0
assert_response :not_found
# And finally we should be able to do it with the owner of the trace
basic_authorization(users(:normal_user).display_name, "test")
delete :api_delete, :id => gpx_files(:public_trace_file).id
assert_response :success
# Try it a second time, which should fail
basic_authorization(users(:normal_user).display_name, "test")
delete :api_delete, :id => gpx_files(:public_trace_file).id
assert_response :not_found
end
end

View file

@ -4,7 +4,7 @@ class TraceTest < ActiveSupport::TestCase
api_fixtures
def test_trace_count
assert_equal 4, Trace.count
assert_equal 5, Trace.count
end
end