From: Tom Hughes Date: Tue, 19 Jan 2016 00:19:09 +0000 (+0000) Subject: Fix rubocop warnings X-Git-Tag: live~4537 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8fe18995964717ff6f88858291c6f464cc40397f Fix rubocop warnings --- diff --git a/.rubocop.yml b/.rubocop.yml index 213eb27aa..62685c8ed 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,7 @@ inherit_from: .rubocop_todo.yml -AllCops: - RunRailsCops: true +Rails: + Enabled: true Style/BracesAroundHashParameters: EnforcedStyle: context_dependent @@ -15,10 +15,16 @@ Style/FileName: - 'script/locale/reload-languages' - 'script/update-spam-blocks' +Style/IfInsideElse: + Enabled: false + Style/GlobalVars: Exclude: - 'lib/quad_tile/extconf.rb' - + +Style/GuardClause: + Enabled: false + Style/HashSyntax: EnforcedStyle: hash_rockets Exclude: @@ -27,3 +33,7 @@ Style/HashSyntax: Style/StringLiterals: EnforcedStyle: double_quotes + +Style/WordArray: + Exclude: + - 'test/models/message_test.rb' diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index f2b592b49..f528919c3 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -189,11 +189,11 @@ class AmfController < ApplicationController def getpresets(usertoken, lang) #:doc: user = getuser(usertoken) - if user && !user.languages.empty? - langs = Locale.list(user.languages) - else - langs = Locale.list(http_accept_language.user_preferred_languages) - end + langs = if user && !user.languages.empty? + Locale.list(user.languages) + else + Locale.list(http_accept_language.user_preferred_languages) + end lang = getlocales.preferred(langs) (real_lang, localised) = getlocalized(lang.to_s) @@ -471,11 +471,11 @@ class AmfController < ApplicationController return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists? query = Trace.visible_to(user) - if searchterm.to_i > 0 - query = query.where(:id => searchterm.to_i) - else - query = query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) - end + query = if searchterm.to_i > 0 + query.where(:id => searchterm.to_i) + else + query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) + end gpxs = query.collect do |gpx| [gpx.id, gpx.name, gpx.description] end @@ -868,11 +868,10 @@ class AmfController < ApplicationController def getuser(token) #:doc: if token =~ /^(.+)\:(.+)$/ - user = User.authenticate(:username => $1, :password => $2) + User.authenticate(:username => $1, :password => $2) else - user = User.authenticate(:token => token) + User.authenticate(:token => token) end - user end def getlocales diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 379bb7e86..d158019f3 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -136,7 +136,7 @@ class ApiController < ApplicationController doc = OSM::API.new.get_xml_doc # add bounds - doc.root << bbox.add_bounds_to(XML::Node.new "bounds") + doc.root << bbox.add_bounds_to(XML::Node.new("bounds")) # get ways # find which ways are needed @@ -258,8 +258,8 @@ class ApiController < ApplicationController api = XML::Node.new "api" version = XML::Node.new "version" - version["minimum"] = "#{API_VERSION}" - version["maximum"] = "#{API_VERSION}" + version["minimum"] = API_VERSION.to_s + version["maximum"] = API_VERSION.to_s api << version area = XML::Node.new "area" area["maximum"] = MAX_REQUEST_AREA.to_s diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9954b775b..98eecd600 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -140,13 +140,13 @@ class ApplicationController < ActionController::Base unless Authenticator.new(self, [:token]).allow? username, passwd = get_auth_data # parse from headers # authenticate per-scheme - if username.nil? - @user = nil # no authentication provided - perhaps first connect (client should retry after 401) - elsif username == "token" - @user = User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth - else - @user = User.authenticate(:username => username, :password => passwd) # basic auth - end + @user = if username.nil? + nil # no authentication provided - perhaps first connect (client should retry after 401) + elsif username == "token" + User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth + else + User.authenticate(:username => username, :password => passwd) # basic auth + end end # have we identified the user? @@ -276,7 +276,7 @@ class ApplicationController < ActionController::Base response.headers["Error"] = message if request.headers["X-Error-Format"] && - request.headers["X-Error-Format"].downcase == "xml" + request.headers["X-Error-Format"].casecmp("xml").zero? result = OSM::API.new.get_xml_doc result.root.name = "osmError" result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}") diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index 0ac45b926..7d2f80eb9 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -58,11 +58,11 @@ class BrowseController < ApplicationController def changeset @type = "changeset" @changeset = Changeset.find(params[:id]) - if @user && @user.moderator? - @comments = @changeset.comments.unscope(:where => :visible).includes(:author) - else - @comments = @changeset.comments.includes(:author) - end + @comments = if @user && @user.moderator? + @changeset.comments.unscope(:where => :visible).includes(:author) + else + @changeset.comments.includes(:author) + end @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page") @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page") @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page") diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 733c20922..137991276 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -186,16 +186,14 @@ class ChangesetController < ApplicationController # 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 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 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 end @@ -277,11 +275,11 @@ class ChangesetController < ApplicationController changesets = conditions_nonempty(Changeset.all) if params[:display_name] - if user.data_public? || user == @user - changesets = changesets.where(:user_id => user.id) - else - changesets = changesets.where("false") - end + changesets = if user.data_public? || user == @user + changesets.where(:user_id => user.id) + else + changesets.where("false") + end elsif params[:bbox] changesets = conditions_bbox(changesets, BoundingBox.from_bbox_params(params)) elsif params[:friends] && @user @@ -459,11 +457,12 @@ class ChangesetController < ApplicationController if bbox bbox.check_boundaries bbox = bbox.to_scaled - return changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?", - bbox.max_lon.to_i, bbox.min_lon.to_i, - bbox.max_lat.to_i, bbox.min_lat.to_i) + + changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?", + bbox.max_lon.to_i, bbox.min_lon.to_i, + bbox.max_lat.to_i, bbox.min_lat.to_i) else - return changesets + changesets end end @@ -471,7 +470,7 @@ class ChangesetController < ApplicationController # restrict changesets to those by a particular user def conditions_user(changesets, user, name) if user.nil? && name.nil? - return changesets + changesets else # shouldn't provide both name and UID fail OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name @@ -497,7 +496,8 @@ class ChangesetController < ApplicationController fail OSM::APINotFoundError if @user.nil? || @user.id != u.id end - return changesets.where(:user_id => u.id) + + changesets.where(:user_id => u.id) end end @@ -506,20 +506,19 @@ class ChangesetController < ApplicationController def conditions_time(changesets, time) if time.nil? return changesets - else + elsif time.count(",") == 1 # if there is a range, i.e: comma separated, then the first is # low, second is high - same as with bounding boxes. - if time.count(",") == 1 - # check that we actually have 2 elements in the array - times = time.split(/,/) - fail OSM::APIBadUserInput.new("bad time range") if times.size != 2 - from, to = times.collect { |t| DateTime.parse(t) } - return changesets.where("closed_at >= ? and created_at <= ?", from, to) - else - # if there is no comma, assume its a lower limit on time - return changesets.where("closed_at >= ?", DateTime.parse(time)) - end + # check that we actually have 2 elements in the array + times = time.split(/,/) + fail OSM::APIBadUserInput.new("bad time range") if times.size != 2 + + from, to = times.collect { |t| DateTime.parse(t) } + return changesets.where("closed_at >= ? and created_at <= ?", from, to) + else + # if there is no comma, assume its a lower limit on time + return changesets.where("closed_at >= ?", DateTime.parse(time)) end # stupid DateTime seems to throw both of these for bad parsing, so # we have to catch both and ensure the correct code path is taken. diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index b6fb455ee..5f29f418d 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -19,13 +19,13 @@ class GeocoderController < ApplicationController @sources.push "osm_nominatim_reverse" @sources.push "geonames_reverse" if defined?(GEONAMES_USERNAME) elsif params[:query] - if params[:query].match(/^\d{5}(-\d{4})?$/) + if params[:query] =~ /^\d{5}(-\d{4})?$/ @sources.push "us_postcode" @sources.push "osm_nominatim" - elsif params[:query].match(/^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i) + elsif params[:query] =~ /^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i @sources.push "uk_postcode" @sources.push "osm_nominatim" - elsif params[:query].match(/^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i) + elsif params[:query] =~ /^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i @sources.push "ca_postcode" @sources.push "osm_nominatim" else @@ -70,7 +70,7 @@ class GeocoderController < ApplicationController response = fetch_text("http://rpc.geocoder.us/service/csv?zip=#{escape_query(query)}") # parse the response - unless response.match(/couldn't find this zip/) + unless response =~ /couldn't find this zip/ data = response.split(/\s*,\s+/) # lat,long,town,state,zip @results.push(:lat => data[0], :lon => data[1], :zoom => POSTCODE_ZOOM, @@ -95,7 +95,7 @@ class GeocoderController < ApplicationController response = fetch_text("http://www.npemap.org.uk/cgi/geocoder.fcgi?format=text&postcode=#{escape_query(query)}") # parse the response - unless response.match(/Error/) + unless response =~ /Error/ dataline = response.split(/\n/)[1] data = dataline.split(/,/) # easting,northing,postcode,lat,long postcode = data[2].delete("'") @@ -171,11 +171,11 @@ class GeocoderController < ApplicationController type = place.attributes["type"].to_s name = place.attributes["display_name"].to_s min_lat, max_lat, min_lon, max_lon = place.attributes["boundingbox"].to_s.split(",") - if type.empty? - prefix_name = "" - else - prefix_name = t "geocoder.search_osm_nominatim.prefix.#{klass}.#{type}", :default => type.tr("_", " ").capitalize - end + prefix_name = if type.empty? + "" + else + t "geocoder.search_osm_nominatim.prefix.#{klass}.#{type}", :default => type.tr("_", " ").capitalize + end if klass == "boundary" && type == "administrative" rank = (place.attributes["place_rank"].to_i + 1) / 2 prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name @@ -339,11 +339,11 @@ class GeocoderController < ApplicationController def nsew_to_decdeg(captures) begin Float(captures[0]) - captures[2].downcase != "s" ? lat = captures[0].to_f : lat = -(captures[0].to_f) - captures[5].downcase != "w" ? lon = captures[3].to_f : lon = -(captures[3].to_f) + lat = captures[2].downcase != "s" ? captures[0].to_f : -captures[0].to_f + lon = captures[5].downcase != "w" ? captures[3].to_f : -captures[3].to_f rescue - captures[0].downcase != "s" ? lat = captures[1].to_f : lat = -(captures[1].to_f) - captures[3].downcase != "w" ? lon = captures[4].to_f : lon = -(captures[4].to_f) + lat = captures[0].downcase != "s" ? captures[1].to_f : -captures[1].to_f + lon = captures[3].downcase != "w" ? captures[4].to_f : -captures[4].to_f end { :lat => lat, :lon => lon } end @@ -351,11 +351,11 @@ class GeocoderController < ApplicationController def ddm_to_decdeg(captures) begin Float(captures[0]) - captures[3].downcase != "s" ? lat = captures[0].to_f + captures[1].to_f / 60 : lat = -(captures[0].to_f + captures[1].to_f / 60) - captures[7].downcase != "w" ? lon = captures[4].to_f + captures[5].to_f / 60 : lon = -(captures[4].to_f + captures[5].to_f / 60) + lat = captures[3].downcase != "s" ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60) + lon = captures[7].downcase != "w" ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60) rescue - captures[0].downcase != "s" ? lat = captures[1].to_f + captures[2].to_f / 60 : lat = -(captures[1].to_f + captures[2].to_f / 60) - captures[4].downcase != "w" ? lon = captures[5].to_f + captures[6].to_f / 60 : lon = -(captures[5].to_f + captures[6].to_f / 60) + lat = captures[0].downcase != "s" ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60) + lon = captures[4].downcase != "w" ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60) end { :lat => lat, :lon => lon } end @@ -363,11 +363,11 @@ class GeocoderController < ApplicationController def dms_to_decdeg(captures) begin Float(captures[0]) - captures[4].downcase != "s" ? lat = captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : lat = -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60) - captures[9].downcase != "w" ? lon = captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : lon = -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60) + lat = captures[4].downcase != "s" ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60) + lon = captures[9].downcase != "w" ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60) rescue - captures[0].downcase != "s" ? lat = captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : lat = -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60) - captures[5].downcase != "w" ? lon = captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : lon = -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60) + lat = captures[0].downcase != "s" ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60) + lon = captures[5].downcase != "w" ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60) end { :lat => lat, :lon => lon } end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index e73b2ba03..d996251b3 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -319,21 +319,19 @@ class NotesController < ApplicationController # Generate a condition to choose which bugs we want based # on their status and the user's request parameters def closed_condition(notes) - if params[:closed] - closed_since = params[:closed].to_i - else - closed_since = 7 - end + closed_since = if params[:closed] + params[:closed].to_i + else + 7 + end if closed_since < 0 - notes = notes.where("status != 'hidden'") + notes.where("status != 'hidden'") elsif closed_since > 0 - notes = notes.where("(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))") + notes.where("(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))") else - notes = notes.where("status = 'open'") + notes.where("status = 'open'") end - - notes end ## diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb index 88691f862..59ebfd631 100644 --- a/app/controllers/oauth_controller.rb +++ b/app/controllers/oauth_controller.rb @@ -41,38 +41,36 @@ class OauthController < ApplicationController if @token.invalidated? @message = t "oauth.oauthorize_failure.invalid" render :action => "authorize_failure" - else - if request.post? - if user_authorizes_token? - @token.authorize!(current_user) - if @token.oauth10? - callback_url = params[:oauth_callback] || @token.client_application.callback_url - else - callback_url = @token.oob? ? @token.client_application.callback_url : @token.callback_url - end - @redirect_url = URI.parse(callback_url) unless callback_url.blank? + elsif request.post? + if user_authorizes_token? + @token.authorize!(current_user) + callback_url = if @token.oauth10? + params[:oauth_callback] || @token.client_application.callback_url + else + @token.oob? ? @token.client_application.callback_url : @token.callback_url + end + @redirect_url = URI.parse(callback_url) unless callback_url.blank? - if @redirect_url.to_s.blank? - render :action => "authorize_success" - else - @redirect_url.query = if @redirect_url.query.blank? - "oauth_token=#{@token.token}" - else - @redirect_url.query + + if @redirect_url.to_s.blank? + render :action => "authorize_success" + else + @redirect_url.query = if @redirect_url.query.blank? + "oauth_token=#{@token.token}" + else + @redirect_url.query + "&oauth_token=#{@token.token}" - end - - unless @token.oauth10? - @redirect_url.query += "&oauth_verifier=#{@token.verifier}" - end + end - redirect_to @redirect_url.to_s + unless @token.oauth10? + @redirect_url.query += "&oauth_verifier=#{@token.verifier}" end - else - @token.invalidate! - @message = t("oauth.oauthorize_failure.denied", :app_name => @token.client_application.name) - render :action => "authorize_failure" + + redirect_to @redirect_url.to_s end + else + @token.invalidate! + @message = t("oauth.oauthorize_failure.denied", :app_name => @token.client_application.name) + render :action => "authorize_failure" end end end diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index bda0a7942..771b1a032 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -30,13 +30,13 @@ class TraceController < ApplicationController end # set title - if target_user.nil? - @title = t "trace.list.public_traces" - elsif @user && @user == target_user - @title = t "trace.list.your_traces" - else - @title = t "trace.list.public_traces_from", :user => target_user.display_name - end + @title = if target_user.nil? + t "trace.list.public_traces" + elsif @user && @user == target_user + t "trace.list.your_traces" + else + t "trace.list.public_traces_from", :user => target_user.display_name + end @title += t "trace.list.tagged_with", :tags => params[:tag] if params[:tag] @@ -45,19 +45,17 @@ class TraceController < ApplicationController # 2 - all traces, not logged in = all public traces # 3 - user's traces, logged in as same user = all user's traces # 4 - user's traces, not logged in as that user = all user's public traces - if target_user.nil? # all traces - if @user - @traces = Trace.visible_to(@user) # 1 - else - @traces = Trace.visible_to_all # 2 - end - else - if @user && @user == target_user - @traces = @user.traces # 3 (check vs user id, so no join + can't pick up non-public traces by changing name) - else - @traces = target_user.traces.visible_to_all # 4 - end - end + @traces = if target_user.nil? # all traces + if @user + Trace.visible_to(@user) # 1 + else + Trace.visible_to_all # 2 + end + elsif @user && @user == target_user + @user.traces # 3 (check vs user id, so no join + can't pick up non-public traces by changing name) + else + target_user.traces.visible_to_all # 4 + end @traces = @traces.tagged(params[:tag]) if params[:tag] @@ -315,11 +313,11 @@ class TraceController < ApplicationController visibility = params[:visibility] if visibility.nil? - if params[:public] && params[:public].to_i.nonzero? - visibility = "public" - else - visibility = "private" - end + visibility = if params[:public] && params[:public].to_i.nonzero? + "public" + else + "private" + end end if params[:file].respond_to?(:read) diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 14e8c9460..8520aa324 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -403,13 +403,11 @@ class UserController < ApplicationController friend.friend_user_id = @new_friend.id if @user.is_friends_with?(@new_friend) flash[:warning] = t "user.make_friend.already_a_friend", :name => @new_friend.display_name + elsif friend.save + flash[:notice] = t "user.make_friend.success", :name => @new_friend.display_name + Notifier.friend_notification(friend).deliver_now else - if friend.save - flash[:notice] = t "user.make_friend.success", :name => @new_friend.display_name - Notifier.friend_notification(friend).deliver_now - else - friend.add_error(t("user.make_friend.failed", :name => @new_friend.display_name)) - end + friend.add_error(t("user.make_friend.failed", :name => @new_friend.display_name)) end if params[:referer] @@ -682,11 +680,11 @@ class UserController < ApplicationController user.home_lat = params[:user][:home_lat] user.home_lon = params[:user][:home_lon] - if params[:user][:preferred_editor] == "default" - user.preferred_editor = nil - else - user.preferred_editor = params[:user][:preferred_editor] - end + user.preferred_editor = if params[:user][:preferred_editor] == "default" + nil + else + params[:user][:preferred_editor] + end if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank? user.auth_provider = nil @@ -778,11 +776,11 @@ class UserController < ApplicationController ## # check signup acls def check_signup_allowed(email = nil) - if email.nil? - domain = nil - else - domain = email.split("@").last - end + domain = if email.nil? + nil + else + email.split("@").last + end if blocked = Acl.no_account_creation(request.remote_ip, domain) logger.info "Blocked signup from #{request.remote_ip} for #{email}" diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index 6c9e108dc..8f2ce5da7 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -2,11 +2,11 @@ require "uri" module BrowseHelper def printable_name(object, version = false) - if object.id.is_a?(Array) - id = object.id[0] - else - id = object.id - end + id = if object.id.is_a?(Array) + object.id[0] + else + object.id + end name = t "printable_name.with_id", :id => id.to_s if version name = t "printable_name.with_version", :id => name, :version => object.version.to_s @@ -92,7 +92,7 @@ module BrowseHelper private - ICON_TAGS = %w(aeroway amenity barrier building highway historic landuse leisure man_made natural railway shop tourism waterway) + ICON_TAGS = %w(aeroway amenity barrier building highway historic landuse leisure man_made natural railway shop tourism waterway).freeze def icon_tags(object) object.tags.find_all { |k, _v| ICON_TAGS.include? k }.sort @@ -123,14 +123,14 @@ module BrowseHelper if key == "wikipedia" # This regex should match Wikipedia language codes, everything # from de to zh-classical - if value =~ /^([a-z-]{2,12}):(.+)$/i - # Value is : so split it up - # Note that value is always left as-is, see: https://trac.openstreetmap.org/ticket/4315 - lang = $1 - else - # Value is <title> so default to English Wikipedia - lang = "en" - end + lang = if value =~ /^([a-z-]{2,12}):(.+)$/i + # Value is <lang>:<title> so split it up + # Note that value is always left as-is, see: https://trac.openstreetmap.org/ticket/4315 + $1 + else + # Value is <title> so default to English Wikipedia + "en" + end elsif key =~ /^wikipedia:(\S+)$/ # Language is in the key, so assume value is the title lang = $1 diff --git a/app/helpers/geocoder_helper.rb b/app/helpers/geocoder_helper.rb index e135917a9..161bb2d6d 100644 --- a/app/helpers/geocoder_helper.rb +++ b/app/helpers/geocoder_helper.rb @@ -2,13 +2,13 @@ module GeocoderHelper def result_to_html(result) html_options = { :class => "set_position", :data => {} } - if result[:type] && result[:id] - url = url_for(:controller => :browse, :action => result[:type], :id => result[:id]) - elsif result[:min_lon] && result[:min_lat] && result[:max_lon] && result[:max_lat] - url = "/?bbox=#{result[:min_lon]},#{result[:min_lat]},#{result[:max_lon]},#{result[:max_lat]}" - else - url = "/#map=#{result[:zoom]}/#{result[:lat]}/#{result[:lon]}" - end + url = if result[:type] && result[:id] + url_for(:controller => :browse, :action => result[:type], :id => result[:id]) + elsif result[:min_lon] && result[:min_lat] && result[:max_lon] && result[:max_lat] + "/?bbox=#{result[:min_lon]},#{result[:min_lat]},#{result[:max_lon]},#{result[:max_lat]}" + else + "/#map=#{result[:zoom]}/#{result[:lat]}/#{result[:lon]}" + end result.each do |key, value| html_options[:data][key.to_s.tr("_", "-")] = value diff --git a/app/models/changeset.rb b/app/models/changeset.rb index bf939e425..691081609 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -146,7 +146,7 @@ class Changeset < ActiveRecord::Base # do the changeset update and the changeset tags update in the # same transaction to ensure consistency. Changeset.transaction do - self.save! + save! tags = self.tags ChangesetTag.delete_all(:changeset_id => id) @@ -166,12 +166,12 @@ class Changeset < ActiveRecord::Base # that would make it more than 24h long, in which case clip to # 24h, as this has been decided is a reasonable time limit. def update_closed_at - if self.is_open? - if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT) - self.closed_at = created_at + MAX_TIME_OPEN - else - self.closed_at = Time.now.getutc + IDLE_TIMEOUT - end + if is_open? + self.closed_at = if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT) + created_at + MAX_TIME_OPEN + else + Time.now.getutc + IDLE_TIMEOUT + end end end diff --git a/app/models/client_application.rb b/app/models/client_application.rb index aa7cb1c34..156eeafc7 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -77,7 +77,7 @@ class ClientApplication < ActiveRecord::Base # can agree or not agree to each of them. PERMISSIONS = [:allow_read_prefs, :allow_write_prefs, :allow_write_diary, :allow_write_api, :allow_read_gpx, :allow_write_gpx, - :allow_write_notes] + :allow_write_notes].freeze def generate_keys self.key = OAuth::Helper.generate_key(40)[0, 40] diff --git a/app/models/node.rb b/app/models/node.rb index f1a89e0e2..20c874931 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -117,7 +117,7 @@ class Node < ActiveRecord::Base # provide repeatable reads for the used-by checks. this means it # shouldn't be possible to get race conditions. Node.transaction do - self.lock! + lock! check_consistency(self, new_node, user) ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id) fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(",")}.") unless ways.empty? @@ -138,7 +138,7 @@ class Node < ActiveRecord::Base def update_from(new_node, user) Node.transaction do - self.lock! + lock! check_consistency(self, new_node, user) # update changeset first @@ -184,7 +184,7 @@ class Node < ActiveRecord::Base add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) - if self.visible? + if visible? el["lat"] = lat.to_s el["lon"] = lon.to_s end @@ -235,7 +235,7 @@ class Node < ActiveRecord::Base Node.transaction do self.version += 1 self.timestamp = t - self.save! + save! # Create a NodeTag tags = self.tags diff --git a/app/models/note.rb b/app/models/note.rb index 31056be49..73207af0f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -59,6 +59,6 @@ class Note < ActiveRecord::Base # Fill in default values for new notes def set_defaults - self.status = "open" unless self.attribute_present?(:status) + self.status = "open" unless attribute_present?(:status) end end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index a9bc0d101..23f7b9907 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -136,17 +136,17 @@ class Notifier < ActionMailer::Base @owner = recipient == comment.note.author @event = comment.event - if comment.author - @commenter = comment.author.display_name - else - @commenter = I18n.t("notifier.note_comment_notification.anonymous") - end - - if @owner - subject = I18n.t("notifier.note_comment_notification.#{@event}.subject_own", :commenter => @commenter) - else - subject = I18n.t("notifier.note_comment_notification.#{@event}.subject_other", :commenter => @commenter) - end + @commenter = if comment.author + comment.author.display_name + else + I18n.t("notifier.note_comment_notification.anonymous") + end + + subject = if @owner + I18n.t("notifier.note_comment_notification.#{@event}.subject_own", :commenter => @commenter) + else + I18n.t("notifier.note_comment_notification.#{@event}.subject_other", :commenter => @commenter) + end mail :to => recipient.email, :subject => subject end @@ -162,11 +162,11 @@ class Notifier < ActionMailer::Base @time = comment.created_at @changeset_author = comment.changeset.user.display_name - if @owner - subject = I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter) - else - subject = I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter) - end + subject = if @owner + I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter) + else + I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter) + end mail :to => recipient.email, :subject => subject end diff --git a/app/models/old_node.rb b/app/models/old_node.rb index 84899d7fb..b103e5c73 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -55,7 +55,7 @@ class OldNode < ActiveRecord::Base add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) - if self.visible? + if visible? el["lat"] = lat.to_s el["lon"] = lon.to_s end diff --git a/app/models/relation.rb b/app/models/relation.rb index cb9621822..d244a6d6c 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -33,7 +33,7 @@ class Relation < ActiveRecord::Base scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) } scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) } - TYPES = %w(node way relation) + TYPES = %w(node way relation).freeze def self.from_xml(xml, create = false) p = XML::Parser.string(xml) @@ -183,7 +183,7 @@ class Relation < ActiveRecord::Base # provide repeatable reads for the used-by checks. this means it # shouldn't be possible to get race conditions. Relation.transaction do - self.lock! + lock! check_consistency(self, new_relation, user) # This will check to see if this relation is used by another relation rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id) @@ -199,7 +199,7 @@ class Relation < ActiveRecord::Base def update_from(new_relation, user) Relation.transaction do - self.lock! + lock! check_consistency(self, new_relation, user) unless new_relation.preconditions_ok?(members) fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.") @@ -215,7 +215,7 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) - unless self.preconditions_ok? + unless preconditions_ok? fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.") end self.version = 0 @@ -289,7 +289,7 @@ class Relation < ActiveRecord::Base t = Time.now.getutc self.version += 1 self.timestamp = t - self.save! + save! tags = self.tags.clone relation_tags.each do |old_tag| @@ -312,7 +312,7 @@ class Relation < ActiveRecord::Base end # if there are left-over tags then they are new and will have to # be added. - tags_changed |= (!tags.empty?) + tags_changed |= !tags.empty? RelationTag.delete_all(:relation_id => id) self.tags.each do |k, v| tag = RelationTag.new @@ -370,7 +370,7 @@ class Relation < ActiveRecord::Base # materially change the rest of the relation. any_relations = changed_members.collect { |_id, type| type == "relation" } - .inject(false) { |a, e| a || e } + .inject(false) { |a, e| a || e } update_members = if tags_changed || any_relations # add all non-relation bounding boxes to the changeset diff --git a/app/models/request_token.rb b/app/models/request_token.rb index a1b30abb3..c0f019486 100644 --- a/app/models/request_token.rb +++ b/app/models/request_token.rb @@ -35,7 +35,7 @@ class RequestToken < OauthToken end def oob? - callback_url.nil? || callback_url.downcase == "oob" + callback_url.nil? || callback_url.casecmp("oob").zero? end def oauth10? diff --git a/app/models/trace.rb b/app/models/trace.rb index b13ce84d2..ed1bf058c 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -28,20 +28,20 @@ class Trace < ActiveRecord::Base end def tagstring=(s) - if s.include? "," - self.tags = s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect {|tag| - tt = Tracetag.new - tt.tag = tag - tt - } - else - # do as before for backwards compatibility: - self.tags = s.split.collect {|tag| - tt = Tracetag.new - tt.tag = tag - tt - } - end + self.tags = if s.include? "," + s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect do|tag| + tt = Tracetag.new + tt.tag = tag + tt + end + else + # do as before for backwards compatibility: + s.split.collect do|tag| + tt = Tracetag.new + tt.tag = tag + tt + end + end end def public? @@ -101,17 +101,17 @@ class Trace < ActiveRecord::Base zipped = filetype =~ /Zip archive/ tarred = filetype =~ /tar archive/ - if gzipped - mimetype = "application/x-gzip" - elsif bzipped - mimetype = "application/x-bzip2" - elsif zipped - mimetype = "application/x-zip" - elsif tarred - mimetype = "application/x-tar" - else - mimetype = "application/gpx+xml" - end + mimetype = if gzipped + "application/x-gzip" + elsif bzipped + "application/x-bzip2" + elsif zipped + "application/x-zip" + elsif tarred + "application/x-tar" + else + "application/gpx+xml" + end mimetype end @@ -123,21 +123,21 @@ class Trace < ActiveRecord::Base zipped = filetype =~ /Zip archive/ tarred = filetype =~ /tar archive/ - if tarred && gzipped - extension = ".tar.gz" - elsif tarred && bzipped - extension = ".tar.bz2" - elsif tarred - extension = ".tar" - elsif gzipped - extension = ".gpx.gz" - elsif bzipped - extension = ".gpx.bz2" - elsif zipped - extension = ".zip" - else - extension = ".gpx" - end + extension = if tarred && gzipped + ".tar.gz" + elsif tarred && bzipped + ".tar.bz2" + elsif tarred + ".tar" + elsif gzipped + ".gpx.gz" + elsif bzipped + ".gpx.bz2" + elsif zipped + ".zip" + else + ".gpx" + end extension end @@ -156,7 +156,7 @@ class Trace < ActiveRecord::Base el1["lon"] = longitude.to_s if inserted el1["user"] = user.display_name el1["visibility"] = visibility - el1["pending"] = (!inserted).to_s + el1["pending"] = inserted ? "false" : "true" el1["timestamp"] = timestamp.xmlschema el2 = XML::Node.new "description" @@ -297,7 +297,7 @@ class Trace < ActiveRecord::Base self.icon_picture = gpx.icon(min_lat, min_lon, max_lat, max_lon) self.size = gpx.actual_points self.inserted = true - self.save! + save! end logger.info "done trace #{id}" diff --git a/app/models/user.rb b/app/models/user.rb index eae917af8..1a0e3415e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -242,7 +242,7 @@ class User < ActiveRecord::Base private def set_defaults - self.creation_time = Time.now.getutc unless self.attribute_present?(:creation_time) + self.creation_time = Time.now.getutc unless attribute_present?(:creation_time) end def encrypt_password diff --git a/app/models/user_role.rb b/app/models/user_role.rb index 9686f7250..6bc7435ae 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -1,7 +1,7 @@ class UserRole < ActiveRecord::Base belongs_to :user - ALL_ROLES = %w(administrator moderator) + ALL_ROLES = %w(administrator moderator).freeze validates :role, :inclusion => ALL_ROLES, :uniqueness => { :scope => :user_id } end diff --git a/app/models/way.rb b/app/models/way.rb index 6d49735f1..0f9650760 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -163,7 +163,7 @@ class Way < ActiveRecord::Base def update_from(new_way, user) Way.transaction do - self.lock! + lock! check_consistency(self, new_way, user) unless new_way.preconditions_ok?(nds) fail OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.") @@ -180,7 +180,7 @@ class Way < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) - unless self.preconditions_ok? + unless preconditions_ok? fail OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.") end self.version = 0 @@ -219,7 +219,7 @@ class Way < ActiveRecord::Base # provide repeatable reads for the used-by checks. this means it # shouldn't be possible to get race conditions. Way.transaction do - self.lock! + lock! check_consistency(self, new_way, user) rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id) fail OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty? @@ -265,7 +265,7 @@ class Way < ActiveRecord::Base Way.transaction do self.version += 1 self.timestamp = t - self.save! + save! tags = self.tags WayTag.delete_all(:way_id => id) diff --git a/config/initializers/abstract_adapter.rb b/config/initializers/abstract_adapter.rb index 5613e0d00..89d384567 100644 --- a/config/initializers/abstract_adapter.rb +++ b/config/initializers/abstract_adapter.rb @@ -4,7 +4,7 @@ if defined?(ActiveRecord::ConnectionAdaptors::AbstractAdapter) class AbstractAdapter protected - alias_method :old_log, :log + alias old_log log def log(sql, name) if block_given? diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index a654f2aa2..610053141 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -1,7 +1,7 @@ require "openid/fetchers" require "openid/util" -CA_BUNDLES = ["/etc/ssl/certs/ca-certificates.crt", "/etc/pki/tls/cert.pem"] +CA_BUNDLES = ["/etc/ssl/certs/ca-certificates.crt", "/etc/pki/tls/cert.pem"].freeze OpenID.fetcher.ca_file = CA_BUNDLES.find { |f| File.exist?(f) } OpenID::Util.logger = Rails.logger diff --git a/config/initializers/streaming.rb b/config/initializers/streaming.rb index 3db2a55f9..0c27ba2e7 100644 --- a/config/initializers/streaming.rb +++ b/config/initializers/streaming.rb @@ -1,7 +1,7 @@ # Hack ActionController::DataStreaming to allow streaming from a file handle module ActionController module DataStreaming - alias_method :old_send_file, :send_file + alias old_send_file send_file def send_file(file, options = {}) if file.is_a?(File) || file.is_a?(Tempfile) diff --git a/config/preinitializer.rb b/config/preinitializer.rb index de8367e9f..07f105722 100644 --- a/config/preinitializer.rb +++ b/config/preinitializer.rb @@ -1,17 +1,15 @@ require "yaml" -if defined?(Rake.application) && Rake.application.top_level_tasks.grep(/^(default$|test(:|$))/).any? - env = "test" -else - env = ENV["RAILS_ENV"] || "development" -end +env = if defined?(Rake.application) && Rake.application.top_level_tasks.grep(/^(default$|test(:|$))/).any? + "test" + else + ENV["RAILS_ENV"] || "development" + end config = YAML.load_file(File.expand_path(env == "test" ? "../example.application.yml" : "../application.yml", __FILE__)) ENV.each do |key, value| - if key.match(/^OSM_(.*)$/) - Object.const_set(Regexp.last_match(1).upcase, value) - end + Object.const_set(Regexp.last_match(1).upcase, value) if key =~ /^OSM_(.*)$/ end config[env].each do |key, value| diff --git a/db/migrate/008_remove_segments.rb b/db/migrate/008_remove_segments.rb index 241a144b0..2ccb0a2ba 100644 --- a/db/migrate/008_remove_segments.rb +++ b/db/migrate/008_remove_segments.rb @@ -15,7 +15,7 @@ class RemoveSegments < ActiveRecord::Migration end conn_opts = ActiveRecord::Base.connection - .instance_eval { @connection_options } + .instance_eval { @connection_options } args = conn_opts.map(&:to_s) + [prefix] fail "#{cmd} failed" unless system cmd, *args diff --git a/db/migrate/023_add_changesets.rb b/db/migrate/023_add_changesets.rb index 9039bb5d6..76ef1f747 100644 --- a/db/migrate/023_add_changesets.rb +++ b/db/migrate/023_add_changesets.rb @@ -29,7 +29,7 @@ class AddChangesets < ActiveRecord::Migration # all the changesets will have the id of the user that made them. # We need to generate a changeset for each user in the database execute "INSERT INTO changesets (id, user_id, created_at, open)" + - "SELECT id, id, creation_time, false from users;" + "SELECT id, id, creation_time, false from users;" @conv_user_tables.each do |tbl| rename_column tbl, :user_id, :changeset_id diff --git a/db/migrate/041_add_fine_o_auth_permissions.rb b/db/migrate/041_add_fine_o_auth_permissions.rb index dcb77a2b6..598cb3c04 100644 --- a/db/migrate/041_add_fine_o_auth_permissions.rb +++ b/db/migrate/041_add_fine_o_auth_permissions.rb @@ -1,6 +1,6 @@ class AddFineOAuthPermissions < ActiveRecord::Migration PERMISSIONS = [:allow_read_prefs, :allow_write_prefs, :allow_write_diary, - :allow_write_api, :allow_read_gpx, :allow_write_gpx] + :allow_write_api, :allow_read_gpx, :allow_write_gpx].freeze def self.up PERMISSIONS.each do |perm| diff --git a/lib/auth.rb b/lib/auth.rb index 636173928..b00df09d0 100644 --- a/lib/auth.rb +++ b/lib/auth.rb @@ -1,6 +1,7 @@ module Auth - PROVIDERS = { "None" => "", "OpenID" => "openid" } - PROVIDERS["Google"] = "google" if defined?(GOOGLE_AUTH_ID) - PROVIDERS["Facebook"] = "facebook" if defined?(FACEBOOK_AUTH_ID) - PROVIDERS["Windows Live"] = "windowslive" if defined?(WINDOWSLIVE_AUTH_ID) + PROVIDERS = { "None" => "", "OpenID" => "openid" }.tap do |providers| + providers["Google"] = "google" if defined?(GOOGLE_AUTH_ID) + providers["Facebook"] = "facebook" if defined?(FACEBOOK_AUTH_ID) + providers["Windows Live"] = "windowslive" if defined?(WINDOWSLIVE_AUTH_ID) + end.freeze end diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index e2dfcd3ff..c01808f54 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -6,8 +6,6 @@ class BoundingBox SCALED_LON_LIMIT = LON_LIMIT * GeoRecord::SCALE SCALED_LAT_LIMIT = LAT_LIMIT * GeoRecord::SCALE - public - def initialize(min_lon, min_lat, max_lon, max_lat) @min_lon = min_lon.to_f unless min_lon.nil? @min_lat = min_lat.to_f unless min_lat.nil? @@ -158,20 +156,22 @@ class BoundingBox "#{min_lon},#{min_lat},#{max_lon},#{max_lat}" end - private - - def self.from_bbox_array(bbox_array) - unless bbox_array - fail OSM::APIBadUserInput.new( - "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat") + class << self + private + + def from_bbox_array(bbox_array) + unless bbox_array + fail OSM::APIBadUserInput.new( + "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat") + end + # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and + # max_lat within their respective boundaries. + min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min + min_lat = [[bbox_array[1].to_f, -LAT_LIMIT].max, +LAT_LIMIT].min + max_lon = [[bbox_array[2].to_f, +LON_LIMIT].min, -LON_LIMIT].max + max_lat = [[bbox_array[3].to_f, +LAT_LIMIT].min, -LAT_LIMIT].max + BoundingBox.new(min_lon, min_lat, max_lon, max_lat) end - # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and - # max_lat within their respective boundaries. - min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min - min_lat = [[bbox_array[1].to_f, -LAT_LIMIT].max, +LAT_LIMIT].min - max_lon = [[bbox_array[2].to_f, +LON_LIMIT].min, -LON_LIMIT].max - max_lat = [[bbox_array[3].to_f, +LAT_LIMIT].min, -LAT_LIMIT].max - BoundingBox.new(min_lon, min_lat, max_lon, max_lat) end def update!(bbox) diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb index ee0c5b7dc..00db5d337 100644 --- a/lib/classic_pagination/pagination.rb +++ b/lib/classic_pagination/pagination.rb @@ -61,7 +61,7 @@ module ActionController DEFAULT_OPTIONS[:group] = nil else # A hash holding options for controllers using macro-style pagination - OPTIONS = {} + OPTIONS = {}.freeze # The default options for pagination DEFAULT_OPTIONS = { @@ -78,7 +78,7 @@ module ActionController :select => nil, :group => nil, :parameter => "page" - } + }.freeze end def self.included(base) #:nodoc: @@ -253,19 +253,19 @@ module ActionController def current_page @current_page ||= self[@current_page_number] end - alias_method :current, :current_page + alias current current_page # Returns a new Page representing the first page in this paginator. def first_page @first_page ||= self[1] end - alias_method :first, :first_page + alias first first_page # Returns a new Page representing the last page in this paginator. def last_page @last_page ||= self[page_count] end - alias_method :last, :last_page + alias last last_page # Returns the number of pages in this paginator. def page_count @@ -277,7 +277,7 @@ module ActionController end end - alias_method :length, :page_count + alias length page_count # Returns true if this paginator contains the page of index +number+. def has_page_number?(number) @@ -310,7 +310,7 @@ module ActionController @number = 1 unless @paginator.has_page_number? @number end attr_reader :paginator, :number - alias_method :to_i, :number + alias to_i number # Compares two Page objects and returns true when they represent the # same page (i.e., their paginators are the same and they have the @@ -416,7 +416,7 @@ module ActionController def pages (@first.number..@last.number).to_a.collect! { |n| @paginator[n] } end - alias_method :to_a, :pages + alias to_a pages end end end diff --git a/lib/classic_pagination/pagination_helper.rb b/lib/classic_pagination/pagination_helper.rb index ae03b82b2..18decb119 100644 --- a/lib/classic_pagination/pagination_helper.rb +++ b/lib/classic_pagination/pagination_helper.rb @@ -13,7 +13,7 @@ module ActionView :always_show_anchors => true, :link_to_current_page => false, :params => {} - } + }.freeze end # Creates a basic HTML link bar for the given +paginator+. Links will be created @@ -113,11 +113,11 @@ module ActionView end window_pages.each do |page| - if current_page == page && !link_to_current_page - html << page.number.to_s - else - html << yield(page.number) - end + html << if current_page == page && !link_to_current_page + page.number.to_s + else + yield(page.number) + end html << " " end diff --git a/lib/diff_reader.rb b/lib/diff_reader.rb index 6ffe3806a..867e28489 100644 --- a/lib/diff_reader.rb +++ b/lib/diff_reader.rb @@ -11,7 +11,7 @@ class DiffReader "node" => Node, "way" => Way, "relation" => Relation - } + }.freeze ## # Construct a diff reader by giving it a bunch of XML +data+ to parse diff --git a/lib/editors.rb b/lib/editors.rb index 1c5427b94..21f646228 100644 --- a/lib/editors.rb +++ b/lib/editors.rb @@ -1,4 +1,4 @@ module Editors - ALL_EDITORS = %w(potlatch potlatch2 id remote) - RECOMMENDED_EDITORS = %w(id potlatch2 remote) + ALL_EDITORS = %w(potlatch potlatch2 id remote).freeze + RECOMMENDED_EDITORS = %w(id potlatch2 remote).freeze end diff --git a/lib/gpx.rb b/lib/gpx.rb index f1c82cf92..3ec11c5c4 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -63,7 +63,7 @@ module GPX highlightgc.stroke("#000000") highlightgc.fill("#000000") - images = frames.times.collect do + images = Array(frames) do Magick::Image.new(width, height) do |image| image.background_color = "white" image.format = "GIF" @@ -81,11 +81,11 @@ module GPX if m > 0 frames.times do |n| - if n == mm - gc = highlightgc.dup - else - gc = linegc.dup - end + gc = if n == mm + highlightgc.dup + else + linegc.dup + end gc.line(px, py, oldpx, oldpy) @@ -148,13 +148,11 @@ module GPX end end - private - TrkPt = Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp) do def valid? latitude && longitude && timestamp && - latitude >= -90 && latitude <= 90 && - longitude >= -180 && longitude <= 180 + latitude >= -90 && latitude <= 90 && + longitude >= -180 && longitude <= 180 end end end diff --git a/lib/password_hash.rb b/lib/password_hash.rb index fe618ba7a..4faac4da8 100644 --- a/lib/password_hash.rb +++ b/lib/password_hash.rb @@ -7,7 +7,7 @@ module PasswordHash SALT_BYTE_SIZE = 32 HASH_BYTE_SIZE = 32 PBKDF2_ITERATIONS = 1000 - DIGEST_ALGORITHM = "sha512" + DIGEST_ALGORITHM = "sha512".freeze def self.create(password) salt = SecureRandom.base64(SALT_BYTE_SIZE) @@ -45,8 +45,6 @@ module PasswordHash false end - private - def self.hash(password, salt, iterations, size, algorithm) digest = OpenSSL::Digest.new(algorithm) pbkdf2 = OpenSSL::PKCS5.pbkdf2_hmac(password, salt, iterations, size, digest) diff --git a/lib/potlatch.rb b/lib/potlatch.rb index a077d4c4d..6e8c88bb8 100644 --- a/lib/potlatch.rb +++ b/lib/potlatch.rb @@ -30,9 +30,7 @@ module Potlatch # Return numeric array def self.getarray(s) - getlong(s).times.collect do - getvalue(s) - end + Array.new(getlong(s)) { getvalue(s) } end # Return object/hash @@ -251,11 +249,11 @@ module Potlatch tag = $1 type = $2 values = $3 - if values == "-" - autotags[type][tag] = [] - else - autotags[type][tag] = values.split(",").sort.reverse - end + autotags[type][tag] = if values == "-" + [] + else + values.split(",").sort.reverse + end end end diff --git a/lib/potlatch2.rb b/lib/potlatch2.rb index d2ccd546a..bf2894968 100644 --- a/lib/potlatch2.rb +++ b/lib/potlatch2.rb @@ -93,5 +93,5 @@ module Potlatch2 "yi" => "yi", "zh" => "zh_CN", "zh-TW" => "zh_TW" - } + }.freeze end diff --git a/lib/quova.rb b/lib/quova.rb index b09f8acea..cb6b93654 100644 --- a/lib/quova.rb +++ b/lib/quova.rb @@ -16,7 +16,7 @@ end module Quova ## # Access details for WSDL description - WSDL_URL = "https://webservices.quova.com/OnDemand/GeoPoint/v1/default.asmx?WSDL" + WSDL_URL = "https://webservices.quova.com/OnDemand/GeoPoint/v1/default.asmx?WSDL".freeze WSDL_USER = QUOVA_USERNAME WSDL_PASS = QUOVA_PASSWORD diff --git a/lib/redactable.rb b/lib/redactable.rb index a42164ec7..ca98b714e 100644 --- a/lib/redactable.rb +++ b/lib/redactable.rb @@ -13,10 +13,10 @@ module Redactable def redact!(redaction) # check that this version isn't the current version - fail OSM::APICannotRedactError.new if self.is_latest_version? + fail OSM::APICannotRedactError.new if is_latest_version? # make the change self.redaction = redaction - self.save! + save! end end diff --git a/lib/session_persistence.rb b/lib/session_persistence.rb index 2244a3e63..d4dd1cc1d 100644 --- a/lib/session_persistence.rb +++ b/lib/session_persistence.rb @@ -20,13 +20,17 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. module SessionPersistence - private + class << self + private - # Install filter when we are included - def self.included(controller) - controller.after_filter :persist_session + # Install filter when we are included + def included(controller) + controller.after_filter :persist_session + end end + private + # Override this method if you don't want to use session[:_remember_for]. def session_persistence_key :_remember_for diff --git a/lib/short_link.rb b/lib/short_link.rb index 43d64041e..ec83ce33f 100644 --- a/lib/short_link.rb +++ b/lib/short_link.rb @@ -10,78 +10,80 @@ module ShortLink # URL-friendly. ARRAY = ("A".."Z").to_a + ("a".."z").to_a + ("0".."9").to_a + ["_", "~"] - ## - # Given a string encoding a location, returns the [lon, lat, z] tuple of that - # location. - def self.decode(str) - x = 0 - y = 0 - z = 0 - z_offset = 0 + class << self + ## + # Given a string encoding a location, returns the [lon, lat, z] tuple of that + # location. + def decode(str) + x = 0 + y = 0 + z = 0 + z_offset = 0 - # keep support for old shortlinks which use the @ character, now - # replaced by the ~ character because twitter is horribly broken - # and we can't have that. - str.tr!("@", "~") + # keep support for old shortlinks which use the @ character, now + # replaced by the ~ character because twitter is horribly broken + # and we can't have that. + str.tr!("@", "~") - str.each_char do |c| - t = ARRAY.index c - if t.nil? - z_offset -= 1 - else - 3.times do - x <<= 1 - x |= 1 unless (t & 32).zero? - t <<= 1 + str.each_char do |c| + t = ARRAY.index c + if t.nil? + z_offset -= 1 + else + 3.times do + x <<= 1 + x |= 1 unless (t & 32).zero? + t <<= 1 - y <<= 1 - y |= 1 unless (t & 32).zero? - t <<= 1 + y <<= 1 + y |= 1 unless (t & 32).zero? + t <<= 1 + end + z += 3 end - z += 3 end + # pack the coordinates out to their original 32 bits. + x <<= (32 - z) + y <<= (32 - z) + + # project the parameters back to their coordinate ranges. + [(x * 360.0 / 2**32) - 180.0, + (y * 180.0 / 2**32) - 90.0, + z - 8 - (z_offset % 3)] end - # pack the coordinates out to their original 32 bits. - x <<= (32 - z) - y <<= (32 - z) - # project the parameters back to their coordinate ranges. - [(x * 360.0 / 2**32) - 180.0, - (y * 180.0 / 2**32) - 90.0, - z - 8 - (z_offset % 3)] - end + ## + # given a location and zoom, return a short string representing it. + def encode(lon, lat, z) + code = interleave_bits(((lon + 180.0) * 2**32 / 360.0).to_i, + ((lat + 90.0) * 2**32 / 180.0).to_i) + str = "" + # add eight to the zoom level, which approximates an accuracy of + # one pixel in a tile. + ((z + 8) / 3.0).ceil.times do |i| + digit = (code >> (58 - 6 * i)) & 0x3f + str << ARRAY[digit] + end + # append characters onto the end of the string to represent + # partial zoom levels (characters themselves have a granularity + # of 3 zoom levels). + ((z + 8) % 3).times { str << "-" } - ## - # given a location and zoom, return a short string representing it. - def self.encode(lon, lat, z) - code = interleave_bits(((lon + 180.0) * 2**32 / 360.0).to_i, - ((lat + 90.0) * 2**32 / 180.0).to_i) - str = "" - # add eight to the zoom level, which approximates an accuracy of - # one pixel in a tile. - ((z + 8) / 3.0).ceil.times do |i| - digit = (code >> (58 - 6 * i)) & 0x3f - str << ARRAY[digit] + str end - # append characters onto the end of the string to represent - # partial zoom levels (characters themselves have a granularity - # of 3 zoom levels). - ((z + 8) % 3).times { str << "-" } - str - end - - private + private - ## - # interleaves the bits of two 32-bit numbers. the result is known - # as a Morton code. - def self.interleave_bits(x, y) - c = 0 - 31.downto(0) do |i| - c = (c << 1) | ((x >> i) & 1) - c = (c << 1) | ((y >> i) & 1) + ## + # interleaves the bits of two 32-bit numbers. the result is known + # as a Morton code. + def interleave_bits(x, y) + c = 0 + 31.downto(0) do |i| + c = (c << 1) | ((x >> i) & 1) + c = (c << 1) | ((y >> i) & 1) + end + c end - c end end diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 582d1ee5c..5da65c4c6 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -102,16 +102,16 @@ class ChangesetControllerTest < ActionController::TestCase basic_authorization users(:normal_user).email, "test" # Create the first user's changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_require_public_data basic_authorization users(:public_user).email, "test" # Create the first user's changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success, "Creation of changeset did not return sucess status" @@ -527,8 +527,8 @@ EOF # create a temporary changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" assert_difference "Changeset.count", 1 do put :create end @@ -1102,8 +1102,8 @@ EOF basic_authorization users(:public_user).email, "test" content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success changeset_id = @response.body.to_i @@ -1114,8 +1114,8 @@ EOF diff.root = XML::Node.new "osmChange" modify = XML::Node.new "modify" xml_old_node = old_node.to_xml_node - xml_old_node["lat"] = (2.0).to_s - xml_old_node["lon"] = (2.0).to_s + xml_old_node["lat"] = 2.0.to_s + xml_old_node["lon"] = 2.0.to_s xml_old_node["changeset"] = changeset_id.to_s modify << xml_old_node diff.root << modify @@ -1140,8 +1140,8 @@ EOF basic_authorization users(:public_user).email, "test" content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success changeset_id = @response.body.to_i @@ -1225,8 +1225,8 @@ EOF # create a temporary changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :forbidden @@ -1235,8 +1235,8 @@ EOF # create a temporary changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success changeset_id = @response.body.to_i @@ -1281,8 +1281,8 @@ EOF # create a temporary changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success changeset_id = @response.body.to_i @@ -1340,8 +1340,8 @@ OSMFILE # create a temporary changeset content "<osm><changeset>" + - "<tag k='created_by' v='osm test suite checking changesets'/>" + - "</changeset></osm>" + "<tag k='created_by' v='osm test suite checking changesets'/>" + + "</changeset></osm>" put :create assert_response :success changeset_id = @response.body.to_i @@ -2317,8 +2317,8 @@ EOF # check the result of a list def check_list_result(changesets) changesets = changesets.where("num_changes > 0") - .order(:created_at => :desc) - .limit(20) + .order(:created_at => :desc) + .limit(20) assert changesets.size <= 20 assert_select "ol.changesets", :count => [changesets.size, 1].min do @@ -2334,8 +2334,8 @@ EOF # check the result of a feed def check_feed_result(changesets) changesets = changesets.where("num_changes > 0") - .order(:created_at => :desc) - .limit(20) + .order(:created_at => :desc) + .limit(20) assert changesets.size <= 20 assert_select "feed", :count => [changesets.size, 1].min do diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb index 8952daade..5fd610e7f 100644 --- a/test/controllers/node_controller_test.rb +++ b/test/controllers/node_controller_test.rb @@ -466,8 +466,8 @@ class NodeControllerTest < ActionController::TestCase # try and put something into a string that the API might # use unquoted and therefore allow code injection... content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" + - '<tag k="#{@user.inspect}" v="0"/>' + - "</node></osm>" + '<tag k="#{@user.inspect}" v="0"/>' + + "</node></osm>" put :create assert_require_public_data "Shouldn't be able to create with non-public user" @@ -478,8 +478,8 @@ class NodeControllerTest < ActionController::TestCase # try and put something into a string that the API might # use unquoted and therefore allow code injection... content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" + - '<tag k="#{@user.inspect}" v="0"/>' + - "</node></osm>" + '<tag k="#{@user.inspect}" v="0"/>' + + "</node></osm>" put :create assert_response :success nodeid = @response.body diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index bb9286f1d..ae6c94667 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -165,8 +165,8 @@ class RelationControllerTest < ActionController::TestCase # This time try with a role attribute in the relation nid = current_nodes(:used_node_1).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member ref='#{nid}' type='node' role='some'/>" + - "<tag k='test' v='yes' /></relation></osm>" + "<member ref='#{nid}' type='node' role='some'/>" + + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for forbidden due to user assert_response :forbidden, @@ -177,7 +177,7 @@ class RelationControllerTest < ActionController::TestCase # need a role attribute to be included nid = current_nodes(:used_node_1).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" + "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for forbidden due to user assert_response :forbidden, @@ -188,9 +188,9 @@ class RelationControllerTest < ActionController::TestCase nid = current_nodes(:used_node_1).id wid = current_ways(:used_way).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member type='node' ref='#{nid}' role='some'/>" + - "<member type='way' ref='#{wid}' role='other'/>" + - "<tag k='test' v='yes' /></relation></osm>" + "<member type='node' ref='#{nid}' role='some'/>" + + "<member type='way' ref='#{wid}' role='other'/>" + + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for forbidden, due to user assert_response :forbidden, @@ -233,8 +233,8 @@ class RelationControllerTest < ActionController::TestCase # This time try with a role attribute in the relation nid = current_nodes(:used_node_1).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member ref='#{nid}' type='node' role='some'/>" + - "<tag k='test' v='yes' /></relation></osm>" + "<member ref='#{nid}' type='node' role='some'/>" + + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for success assert_response :success, @@ -265,7 +265,7 @@ class RelationControllerTest < ActionController::TestCase # need a role attribute to be included nid = current_nodes(:used_node_1).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" + "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for success assert_response :success, @@ -296,9 +296,9 @@ class RelationControllerTest < ActionController::TestCase nid = current_nodes(:used_node_1).id wid = current_ways(:used_way).id content "<osm><relation changeset='#{changeset_id}'>" + - "<member type='node' ref='#{nid}' role='some'/>" + - "<member type='way' ref='#{wid}' role='other'/>" + - "<tag k='test' v='yes' /></relation></osm>" + "<member type='node' ref='#{nid}' role='some'/>" + + "<member type='way' ref='#{wid}' role='other'/>" + + "<tag k='test' v='yes' /></relation></osm>" put :create # hope for success assert_response :success, @@ -412,8 +412,8 @@ class RelationControllerTest < ActionController::TestCase # create a relation with non-existing node as member content "<osm><relation changeset='#{changeset_id}'>" + - "<member type='node' ref='0'/><tag k='test' v='yes' />" + - "</relation></osm>" + "<member type='node' ref='0'/><tag k='test' v='yes' />" + + "</relation></osm>" put :create # expect failure assert_response :precondition_failed, @@ -432,8 +432,8 @@ class RelationControllerTest < ActionController::TestCase # create some xml that should return an error content "<osm><relation changeset='#{changeset_id}'>" + - "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" + - "<tag k='tester' v='yep'/></relation></osm>" + "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" + + "<tag k='tester' v='yep'/></relation></osm>" put :create # expect failure assert_response :bad_request @@ -963,7 +963,7 @@ OSM a_tags.each do |k, v| assert_equal v, b_tags[k], "Tags which were not altered should be the same. " + - "#{a_tags.inspect} != #{b_tags.inspect}" + "#{a_tags.inspect} != #{b_tags.inspect}" end end diff --git a/test/controllers/way_controller_test.rb b/test/controllers/way_controller_test.rb index 8226f435c..45acabece 100644 --- a/test/controllers/way_controller_test.rb +++ b/test/controllers/way_controller_test.rb @@ -121,8 +121,8 @@ class WayControllerTest < ActionController::TestCase # create a way with pre-existing nodes content "<osm><way changeset='#{changeset_id}'>" + - "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + - "<tag k='test' v='yes' /></way></osm>" + "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + + "<tag k='test' v='yes' /></way></osm>" put :create # hope for failure assert_response :forbidden, @@ -138,8 +138,8 @@ class WayControllerTest < ActionController::TestCase # create a way with pre-existing nodes content "<osm><way changeset='#{changeset_id}'>" + - "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + - "<tag k='test' v='yes' /></way></osm>" + "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + + "<tag k='test' v='yes' /></way></osm>" put :create # hope for success assert_response :success, @@ -179,7 +179,7 @@ class WayControllerTest < ActionController::TestCase # create a way with non-existing node content "<osm><way changeset='#{open_changeset_id}'>" + - "<nd ref='0'/><tag k='test' v='yes' /></way></osm>" + "<nd ref='0'/><tag k='test' v='yes' /></way></osm>" put :create # expect failure assert_response :forbidden, @@ -187,7 +187,7 @@ class WayControllerTest < ActionController::TestCase # create a way with no nodes content "<osm><way changeset='#{open_changeset_id}'>" + - "<tag k='test' v='yes' /></way></osm>" + "<tag k='test' v='yes' /></way></osm>" put :create # expect failure assert_response :forbidden, @@ -195,7 +195,7 @@ class WayControllerTest < ActionController::TestCase # create a way inside a closed changeset content "<osm><way changeset='#{closed_changeset_id}'>" + - "<nd ref='#{nid1}'/></way></osm>" + "<nd ref='#{nid1}'/></way></osm>" put :create # expect failure assert_response :forbidden, @@ -211,7 +211,7 @@ class WayControllerTest < ActionController::TestCase # create a way with non-existing node content "<osm><way changeset='#{open_changeset_id}'>" + - "<nd ref='0'/><tag k='test' v='yes' /></way></osm>" + "<nd ref='0'/><tag k='test' v='yes' /></way></osm>" put :create # expect failure assert_response :precondition_failed, @@ -220,7 +220,7 @@ class WayControllerTest < ActionController::TestCase # create a way with no nodes content "<osm><way changeset='#{open_changeset_id}'>" + - "<tag k='test' v='yes' /></way></osm>" + "<tag k='test' v='yes' /></way></osm>" put :create # expect failure assert_response :precondition_failed, @@ -229,7 +229,7 @@ class WayControllerTest < ActionController::TestCase # create a way inside a closed changeset content "<osm><way changeset='#{closed_changeset_id}'>" + - "<nd ref='#{nid1}'/></way></osm>" + "<nd ref='#{nid1}'/></way></osm>" put :create # expect failure assert_response :conflict, @@ -237,9 +237,9 @@ class WayControllerTest < ActionController::TestCase # create a way with a tag which is too long content "<osm><way changeset='#{open_changeset_id}'>" + - "<nd ref='#{nid1}'/>" + - "<tag k='foo' v='#{'x' * 256}'/>" + - "</way></osm>" + "<nd ref='#{nid1}'/>" + + "<tag k='foo' v='#{'x' * 256}'/>" + + "</way></osm>" put :create # expect failure assert_response :bad_request, diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb index aed52f77c..323147030 100644 --- a/test/integration/oauth_test.rb +++ b/test/integration/oauth_test.rb @@ -329,6 +329,7 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_not_nil token.created_at assert_nil token.authorized_at assert_nil token.invalidated_at + assert_equal options[:oauth_callback], token.callback_url assert_allowed token, client.permissions token diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index e36f85c06..2bc17f711 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -81,7 +81,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do post_via_redirect "/user/save", {}, - { "HTTP_ACCEPT_LANGUAGE" => "#{locale}" } + { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } end end diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 81290a14a..a6ae3ecce 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -5,7 +5,7 @@ class MessageTest < ActiveSupport::TestCase api_fixtures fixtures :messages - EURO = "\xe2\x82\xac" # euro symbol + EURO = "\xe2\x82\xac".freeze # euro symbol # This needs to be updated when new fixtures are added # or removed. diff --git a/test/models/note_comment_test.rb b/test/models/note_comment_test.rb index e61732904..323a827c0 100644 --- a/test/models/note_comment_test.rb +++ b/test/models/note_comment_test.rb @@ -22,8 +22,8 @@ class NoteCommentTest < ActiveSupport::TestCase end def test_body_valid - ok = ["Name", "vergrößern", "foo\x0abar", - "ルシステムにも対応します", "輕觸搖晃的遊戲"] + ok = %W(Name vergrößern foo\nbar + ルシステムにも対応します 輕觸搖晃的遊戲) bad = ["foo\x00bar", "foo\x08bar", "foo\x1fbar", "foo\x7fbar", "foo\ufffebar", "foo\uffffbar"] diff --git a/test/models/request_token_test.rb b/test/models/request_token_test.rb new file mode 100644 index 000000000..24ad419fa --- /dev/null +++ b/test/models/request_token_test.rb @@ -0,0 +1,12 @@ +require "test_helper" + +class RequestTokenTest < ActiveSupport::TestCase + api_fixtures + + def test_oob + assert_equal true, RequestToken.new.oob? + assert_equal true, RequestToken.new(:callback_url => "oob").oob? + assert_equal true, RequestToken.new(:callback_url => "OOB").oob? + assert_equal false, RequestToken.new(:callback_url => "http://test.host/").oob? + end +end diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb index 524ec637a..32d98c9eb 100644 --- a/test/models/trace_test.rb +++ b/test/models/trace_test.rb @@ -26,30 +26,30 @@ class TraceTest < ActiveSupport::TestCase def test_visible check_query(Trace.visible, [ - :public_trace_file, :anon_trace_file, :trackable_trace_file, - :identifiable_trace_file, :zipped_trace_file, :tar_trace_file, - :tar_gzip_trace_file, :tar_bzip_trace_file, :pending_trace_file - ]) + :public_trace_file, :anon_trace_file, :trackable_trace_file, + :identifiable_trace_file, :zipped_trace_file, :tar_trace_file, + :tar_gzip_trace_file, :tar_bzip_trace_file, :pending_trace_file + ]) end def test_visible_to check_query(Trace.visible_to(1), [ - :public_trace_file, :identifiable_trace_file, :pending_trace_file - ]) + :public_trace_file, :identifiable_trace_file, :pending_trace_file + ]) check_query(Trace.visible_to(2), [ - :public_trace_file, :anon_trace_file, :trackable_trace_file, - :identifiable_trace_file, :pending_trace_file - ]) + :public_trace_file, :anon_trace_file, :trackable_trace_file, + :identifiable_trace_file, :pending_trace_file + ]) check_query(Trace.visible_to(3), [ - :public_trace_file, :identifiable_trace_file, :pending_trace_file - ]) + :public_trace_file, :identifiable_trace_file, :pending_trace_file + ]) end def test_visible_to_all check_query(Trace.visible_to_all, [ - :public_trace_file, :identifiable_trace_file, - :deleted_trace_file, :pending_trace_file - ]) + :public_trace_file, :identifiable_trace_file, + :deleted_trace_file, :pending_trace_file + ]) end def test_tagged diff --git a/test/models/user_preference_test.rb b/test/models/user_preference_test.rb index ee2dbf2d8..d3400bc9d 100644 --- a/test/models/user_preference_test.rb +++ b/test/models/user_preference_test.rb @@ -20,7 +20,7 @@ class UserPreferenceTest < ActiveSupport::TestCase new_up.k = up.k new_up.v = "some other value" assert_not_equal new_up.v, up.v - assert_raise (ActiveRecord::RecordNotUnique) { new_up.save } + assert_raise(ActiveRecord::RecordNotUnique) { new_up.save } end def test_check_valid_length diff --git a/test/models/way_test.rb b/test/models/way_test.rb index 807079f09..6c8b0f80b 100644 --- a/test/models/way_test.rb +++ b/test/models/way_test.rb @@ -29,7 +29,7 @@ class WayTest < ActiveSupport::TestCase way = Way.find(current_ways(:visible_way).id) assert way.valid? # it already has 1 node - 1.upto((MAX_NUMBER_OF_WAY_NODES) / 2) do + 1.upto(MAX_NUMBER_OF_WAY_NODES / 2) do way.add_nd_num(current_nodes(:used_node_1).id) way.add_nd_num(current_nodes(:used_node_2).id) end