From 8dae890a7645fba17a44d84f78be03d993e22ccb Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Thu, 5 Oct 2017 19:18:38 +0100 Subject: [PATCH] Fix rubocop warnings --- .rubocop.yml | 41 +++--- .rubocop_todo.yml | 109 ++++++++------- app/controllers/amf_controller.rb | 24 ++-- app/controllers/application_controller.rb | 2 +- app/controllers/changeset_controller.rb | 40 +++--- app/controllers/geocoder_controller.rb | 10 +- app/controllers/node_controller.rb | 8 +- app/controllers/notes_controller.rb | 38 +++--- app/controllers/old_controller.rb | 2 +- app/controllers/relation_controller.rb | 6 +- app/controllers/trace_controller.rb | 2 +- app/controllers/user_controller.rb | 4 +- app/controllers/user_preference_controller.rb | 2 +- app/controllers/way_controller.rb | 8 +- app/helpers/banner_helper.rb | 2 +- app/helpers/browse_helper.rb | 16 ++- app/helpers/title_helper.rb | 4 +- app/models/changeset.rb | 6 +- app/models/node.rb | 10 +- app/models/oauth2_token.rb | 4 +- app/models/oauth2_verifier.rb | 2 +- app/models/relation.rb | 16 +-- app/models/trace.rb | 4 +- app/models/way.rb | 16 +-- config/initializers/banners.rb | 2 +- db/migrate/006_tile_nodes.rb | 12 +- db/migrate/008_remove_segments.rb | 2 +- .../020_populate_node_tags_and_remove.rb | 2 +- db/migrate/023_add_changesets.rb | 2 +- lib/bounding_box.rb | 20 +-- lib/consistency_validations.rb | 20 +-- lib/daemons/gpx_import_ctl | 2 +- lib/diff_reader.rb | 10 +- lib/not_redactable.rb | 2 +- lib/osm.rb | 4 +- lib/redactable.rb | 2 +- lib/session_persistence.rb | 2 +- test/controllers/changeset_controller_test.rb | 124 +++++++++--------- .../diary_entry_controller_test.rb | 12 +- test/controllers/geocoder_controller_test.rb | 1 - test/controllers/node_controller_test.rb | 8 +- test/controllers/old_node_controller_test.rb | 2 +- test/controllers/relation_controller_test.rb | 34 ++--- test/controllers/user_controller_test.rb | 84 ++++++------ test/controllers/way_controller_test.rb | 26 ++-- test/helpers/changeset_helper_test.rb | 4 +- test/helpers/note_helper_test.rb | 6 +- test/helpers/title_helper_test.rb | 5 +- test/helpers/user_roles_helper_test.rb | 10 +- test/integration/client_applications_test.rb | 18 +-- test/integration/user_roles_test.rb | 4 +- test/lib/bounding_box_test.rb | 2 +- test/lib/i18n_test.rb | 2 +- test/models/language_test.rb | 2 - 54 files changed, 409 insertions(+), 393 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e1f1724e0..55be8141c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -27,15 +27,34 @@ Rails: Layout/ExtraSpacing: AllowForAlignment: true -Style/BracesAroundHashParameters: - EnforcedStyle: context_dependent +Lint/PercentStringArray: + Exclude: + - 'config/initializers/secure_headers.rb' + - 'app/controllers/site_controller.rb' -Style/FileName: +Naming/FileName: Exclude: - 'script/deliver-message' - 'script/locale/reload-languages' - 'script/update-spam-blocks' +Rails/ApplicationRecord: + Enabled: false + +Rails/HasManyOrHasOneDependent: + Enabled: false + +Rails/HttpPositionalArguments: + Enabled: false + +Rails/SkipsModelValidations: + Exclude: + - 'db/migrate/*.rb' + - 'app/controllers/user_controller.rb' + +Style/BracesAroundHashParameters: + EnforcedStyle: context_dependent + Style/FormatStringToken: EnforcedStyle: template @@ -60,19 +79,3 @@ Style/StringLiterals: Style/SymbolArray: EnforcedStyle: brackets - -Rails/ApplicationRecord: - Enabled: false - -Rails/HttpPositionalArguments: - Enabled: false - -Rails/SkipsModelValidations: - Exclude: - - 'db/migrate/*.rb' - - 'app/controllers/user_controller.rb' - -Lint/PercentStringArray: - Exclude: - - 'config/initializers/secure_headers.rb' - - 'app/controllers/site_controller.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e89927c9d..f03767698 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,11 +1,19 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2016-10-20 21:45:27 +0100 using RuboCop version 0.44.1. +# on 2017-10-05 10:04:24 +0100 using RuboCop version 0.50.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. +# Offense count: 1 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. +# SupportedStyles: aligned, indented +Layout/MultilineOperationIndentation: + Exclude: + - 'lib/bounding_box.rb' + # Offense count: 34 Lint/AmbiguousOperator: Exclude: @@ -14,11 +22,11 @@ Lint/AmbiguousOperator: - 'test/lib/bounding_box_test.rb' - 'test/lib/country_test.rb' -# Offense count: 117 +# Offense count: 124 Lint/AmbiguousRegexpLiteral: Enabled: false -# Offense count: 30 +# Offense count: 32 # Configuration parameters: AllowSafeAssignment. Lint/AssignmentInCondition: Exclude: @@ -36,52 +44,62 @@ Lint/AssignmentInCondition: - 'lib/osm.rb' - 'script/deliver-message' -# Offense count: 5 +# Offense count: 4 Lint/HandleExceptions: Exclude: - 'app/controllers/amf_controller.rb' - 'app/controllers/user_controller.rb' - - 'config/initializers/session.rb' + +# Offense count: 3 +Lint/InterpolationCheck: + Exclude: + - 'test/controllers/node_controller_test.rb' + +# Offense count: 2 +Lint/RescueWithoutErrorClass: + Exclude: + - 'app/helpers/browse_helper.rb' # Offense count: 2 Lint/ShadowingOuterLocalVariable: Exclude: - 'app/views/changeset/list.atom.builder' -# Offense count: 630 +# Offense count: 666 Metrics/AbcSize: Max: 280 -# Offense count: 35 -# Configuration parameters: CountComments. +# Offense count: 41 +# Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 295 + Max: 240 # Offense count: 12 +# Configuration parameters: CountBlocks. Metrics/BlockNesting: Max: 5 -# Offense count: 62 +# Offense count: 63 # Configuration parameters: CountComments. Metrics/ClassLength: Max: 1790 -# Offense count: 69 +# Offense count: 71 Metrics/CyclomaticComplexity: Max: 20 -# Offense count: 2826 -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives. +# Offense count: 3004 +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: - Max: 1072 + Max: 1073 -# Offense count: 612 +# Offense count: 675 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 179 -# Offense count: 1 +# Offense count: 2 # Configuration parameters: CountComments. Metrics/ModuleLength: Max: 147 @@ -95,6 +113,30 @@ Metrics/ParameterLists: Metrics/PerceivedComplexity: Max: 23 +# Offense count: 5 +Naming/AccessorMethodName: + Exclude: + - 'app/controllers/application_controller.rb' + - 'app/helpers/title_helper.rb' + - 'app/models/old_way.rb' + - 'lib/osm.rb' + - 'lib/potlatch.rb' + +# Offense count: 8 +# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist. +# NamePrefix: is_, has_, have_ +# NamePrefixBlacklist: is_, has_, have_ +# NameWhitelist: is_a? +Naming/PredicateName: + Exclude: + - 'spec/**/*' + - 'app/models/changeset.rb' + - 'app/models/old_node.rb' + - 'app/models/old_relation.rb' + - 'app/models/old_way.rb' + - 'app/models/user.rb' + - 'lib/classic_pagination/pagination.rb' + # Offense count: 2 # Configuration parameters: Include. # Include: app/**/*.rb, config/**/*.rb, lib/**/*.rb @@ -122,7 +164,7 @@ Rails/NotNullColumn: - 'db/migrate/025_add_end_time_to_changesets.rb' - 'db/migrate/20120404205604_add_user_and_description_to_redaction.rb' -# Offense count: 17 +# Offense count: 20 Rails/OutputSafety: Exclude: - 'app/controllers/user_controller.rb' @@ -136,27 +178,18 @@ Rails/OutputSafety: - 'lib/rich_text.rb' - 'test/helpers/application_helper_test.rb' -# Offense count: 74 +# Offense count: 86 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: strict, flexible Rails/TimeZone: Enabled: false -# Offense count: 5 -Style/AccessorMethodName: - Exclude: - - 'app/controllers/application_controller.rb' - - 'app/helpers/title_helper.rb' - - 'app/models/old_way.rb' - - 'lib/osm.rb' - - 'lib/potlatch.rb' - # Offense count: 1 Style/AsciiComments: Exclude: - 'test/models/message_test.rb' -# Offense count: 220 +# Offense count: 219 Style/Documentation: Enabled: false @@ -182,27 +215,13 @@ Style/LineEndConcatenation: - 'test/controllers/relation_controller_test.rb' - 'test/controllers/way_controller_test.rb' -# Offense count: 71 +# Offense count: 75 # Cop supports --auto-correct. +# Configuration parameters: Strict. Style/NumericLiterals: MinDigits: 11 -# Offense count: 8 -# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist. -# NamePrefix: is_, has_, have_ -# NamePrefixBlacklist: is_, has_, have_ -# NameWhitelist: is_a? -Style/PredicateName: - Exclude: - - 'spec/**/*' - - 'app/models/changeset.rb' - - 'app/models/old_node.rb' - - 'app/models/old_relation.rb' - - 'app/models/old_way.rb' - - 'app/models/user.rb' - - 'lib/classic_pagination/pagination.rb' - -# Offense count: 97 +# Offense count: 95 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: compact, exploded diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 51db8296d..679e137d9 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -151,7 +151,7 @@ class AmfController < ApplicationController cs = Changeset.find(closeid.to_i) cs.set_closed_time_now if cs.user_id != user.id - raise OSM::APIUserChangesetMismatchError.new + raise OSM::APIUserChangesetMismatchError elsif closecomment.empty? cs.save! else @@ -229,7 +229,7 @@ class AmfController < ApplicationController begin other = YAML.safe_load(File.open(Rails.root.join("config", "potlatch", "locales", "#{lang}.yml")))[lang] loaded_lang = lang - rescue + rescue StandardError other = en end @@ -907,7 +907,7 @@ class AmfController < ApplicationController # Alternative SQL queries for getway/whichways def sql_find_ways_in_area(bbox) - sql = <<-EOF + sql = <<-SQL SELECT DISTINCT current_ways.id AS wayid,current_ways.version AS version FROM current_way_nodes INNER JOIN current_nodes ON current_nodes.id=current_way_nodes.node_id @@ -915,20 +915,20 @@ class AmfController < ApplicationController WHERE current_nodes.visible=TRUE AND current_ways.visible=TRUE AND #{OSM.sql_for_area(bbox, 'current_nodes.')} - EOF + SQL ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["wayid"].to_i, a["version"].to_i] } end def sql_find_pois_in_area(bbox) pois = [] - sql = <<-EOF + sql = <<-SQL SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.version FROM current_nodes LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id WHERE current_nodes.visible=TRUE AND cwn.id IS NULL AND #{OSM.sql_for_area(bbox, 'current_nodes.')} - EOF + SQL ActiveRecord::Base.connection.select_all(sql).each do |row| poitags = {} ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n| @@ -942,36 +942,36 @@ class AmfController < ApplicationController def sql_find_relations_in_area_and_ways(bbox, way_ids) # ** It would be more Potlatchy to get relations for nodes within ways # during 'getway', not here - sql = <<-EOF + sql = <<-SQL SELECT DISTINCT cr.id AS relid,cr.version AS version FROM current_relations cr INNER JOIN current_relation_members crm ON crm.id=cr.id INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='Node' WHERE #{OSM.sql_for_area(bbox, 'cn.')} - EOF + SQL unless way_ids.empty? - sql += <<-EOF + sql += <<-SQL UNION SELECT DISTINCT cr.id AS relid,cr.version AS version FROM current_relations cr INNER JOIN current_relation_members crm ON crm.id=cr.id WHERE crm.member_type='Way' AND crm.member_id IN (#{way_ids.join(',')}) - EOF + SQL end ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["relid"].to_i, a["version"].to_i] } end def sql_get_nodes_in_way(wayid) points = [] - sql = <<-EOF + sql = <<-SQL SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id,current_nodes.version FROM current_way_nodes,current_nodes WHERE current_way_nodes.id=#{wayid.to_i} AND current_way_nodes.node_id=current_nodes.id AND current_nodes.visible=TRUE ORDER BY sequence_id - EOF + SQL ActiveRecord::Base.connection.select_all(sql).each do |row| nodetags = {} ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n| diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 68fc4338e..0988fcdf8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -346,7 +346,7 @@ class ApplicationController < ActionController::Base # or raises a suitable error. +method+ should be a symbol, e.g: :put or :get. def assert_method(method) ok = request.send((method.to_s.downcase + "?").to_sym) - raise OSM::APIBadMethodError.new(method) unless ok + raise OSM::APIBadMethodError, method unless ok end ## diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 1c658ccbd..531ec17ee 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -310,8 +310,8 @@ class ChangesetController < ApplicationController # Add a comment to a changeset def comment # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] - raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank? + raise OSM::APIBadUserInput, "No id was given" unless params[:id] + raise OSM::APIBadUserInput, "No text was given" if params[:text].blank? # Extract the arguments id = params[:id].to_i @@ -319,7 +319,7 @@ class ChangesetController < ApplicationController # Find the changeset and check it is valid changeset = Changeset.find(id) - raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? + raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open? # Add a comment to the changeset comment = changeset.comments.create(:changeset => changeset, @@ -344,15 +344,15 @@ class ChangesetController < ApplicationController # Adds a subscriber to the changeset def subscribe # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i # Find the changeset and check it is valid changeset = Changeset.find(id) - raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? - raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(current_user.id) + raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open? + raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribers.exists?(current_user.id) # Add the subscriber changeset.subscribers << current_user @@ -365,15 +365,15 @@ class ChangesetController < ApplicationController # Removes a subscriber from the changeset def unsubscribe # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i # Find the changeset and check it is valid changeset = Changeset.find(id) - raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? - raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(current_user.id) + raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open? + raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribers.exists?(current_user.id) # Remove the subscriber changeset.subscribers.delete(current_user) @@ -386,7 +386,7 @@ class ChangesetController < ApplicationController # Sets visible flag on comment to false def hide_comment # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i @@ -405,7 +405,7 @@ class ChangesetController < ApplicationController # Sets visible flag on comment to true def unhide_comment # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i @@ -434,7 +434,7 @@ class ChangesetController < ApplicationController @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit) else # Return comments - @comments = ChangesetComment.includes(:author, :changeset).where(:visible => :true).order("created_at DESC").limit(comments_limit).preload(:changeset) + @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset) end # Render the result @@ -475,19 +475,19 @@ class ChangesetController < ApplicationController changesets else # shouldn't provide both name and UID - raise OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name + raise OSM::APIBadUserInput, "provide either the user ID or display name, but not both" if user && name # use either the name or the UID to find the user which we're selecting on. u = if name.nil? # user input checking, we don't have any UIDs < 1 - raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1 + raise OSM::APIBadUserInput, "invalid user ID" if user.to_i < 1 u = User.find(user.to_i) else u = User.find_by(:display_name => name) end # make sure we found a user - raise OSM::APINotFoundError.new if u.nil? + raise OSM::APINotFoundError if u.nil? # should be able to get changesets of public users only, or # our own changesets regardless of public-ness. @@ -514,7 +514,7 @@ class ChangesetController < ApplicationController # check that we actually have 2 elements in the array times = time.split(/,/) - raise OSM::APIBadUserInput.new("bad time range") if times.size != 2 + raise OSM::APIBadUserInput, "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) @@ -525,9 +525,9 @@ class ChangesetController < ApplicationController # 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. rescue ArgumentError => ex - raise OSM::APIBadUserInput.new(ex.message.to_s) + raise OSM::APIBadUserInput, ex.message.to_s rescue RuntimeError => ex - raise OSM::APIBadUserInput.new(ex.message.to_s) + raise OSM::APIBadUserInput, ex.message.to_s end ## @@ -563,7 +563,7 @@ class ChangesetController < ApplicationController if ids.nil? changesets elsif ids.empty? - raise OSM::APIBadUserInput.new("No changesets were given to search for") + raise OSM::APIBadUserInput, "No changesets were given to search for" else ids = ids.split(",").collect(&:to_i) changesets.where(:id => ids) @@ -584,7 +584,7 @@ class ChangesetController < ApplicationController if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 params[:limit].to_i else - raise OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000") + raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000" end else 100 diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index 82c290aa7..6110baead 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -1,5 +1,3 @@ -# coding: utf-8 - class GeocoderController < ApplicationController require "cgi" require "uri" @@ -287,7 +285,7 @@ class GeocoderController < ApplicationController end def escape_query(query) - URI.escape(query, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]", false, "N")) + CGI.escape(query) end def normalize_params @@ -322,7 +320,7 @@ class GeocoderController < ApplicationController Float(captures[0]) lat = !captures[2].casecmp("s").zero? ? captures[0].to_f : -captures[0].to_f lon = !captures[5].casecmp("w").zero? ? captures[3].to_f : -captures[3].to_f - rescue + rescue StandardError lat = !captures[0].casecmp("s").zero? ? captures[1].to_f : -captures[1].to_f lon = !captures[3].casecmp("w").zero? ? captures[4].to_f : -captures[4].to_f end @@ -334,7 +332,7 @@ class GeocoderController < ApplicationController Float(captures[0]) lat = !captures[3].casecmp("s").zero? ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60) lon = !captures[7].casecmp("w").zero? ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60) - rescue + rescue StandardError lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60) lon = !captures[4].casecmp("w").zero? ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60) end @@ -346,7 +344,7 @@ class GeocoderController < ApplicationController Float(captures[0]) lat = !captures[4].casecmp("s").zero? ? 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].casecmp("w").zero? ? 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 + rescue StandardError lat = !captures[0].casecmp("s").zero? ? 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].casecmp("w").zero? ? 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 diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 3eb127cb1..29651bceb 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -41,7 +41,7 @@ class NodeController < ApplicationController new_node = Node.from_xml(request.raw_post) unless new_node && new_node.id == node.id - raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})") + raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" end node.update_from(new_node, current_user) @@ -56,7 +56,7 @@ class NodeController < ApplicationController new_node = Node.from_xml(request.raw_post) unless new_node && new_node.id == node.id - raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})") + raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" end node.delete_with_history!(new_node, current_user) render :plain => node.version.to_s @@ -65,13 +65,13 @@ class NodeController < ApplicationController # Dump the details on many nodes whose ids are given in the "nodes" parameter. def nodes unless params["nodes"] - raise OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]") + raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" end ids = params["nodes"].split(",").collect(&:to_i) if ids.empty? - raise OSM::APIBadUserInput.new("No nodes were given to search for") + raise OSM::APIBadUserInput, "No nodes were given to search for" end doc = OSM::API.new.get_xml_doc diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index f577dc2f2..92f63e304 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -20,10 +20,10 @@ class NotesController < ApplicationController if params[:bbox] bbox = BoundingBox.from_bbox_params(params) else - raise OSM::APIBadUserInput.new("No l was given") unless params[:l] - raise OSM::APIBadUserInput.new("No r was given") unless params[:r] - raise OSM::APIBadUserInput.new("No b was given") unless params[:b] - raise OSM::APIBadUserInput.new("No t was given") unless params[:t] + raise OSM::APIBadUserInput, "No l was given" unless params[:l] + raise OSM::APIBadUserInput, "No r was given" unless params[:r] + raise OSM::APIBadUserInput, "No b was given" unless params[:b] + raise OSM::APIBadUserInput, "No t was given" unless params[:t] bbox = BoundingBox.from_lrbt_params(params) end @@ -56,9 +56,9 @@ class NotesController < ApplicationController raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip) # Check the arguments are sane - raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat] - raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon] - raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank? + raise OSM::APIBadUserInput, "No lat was given" unless params[:lat] + raise OSM::APIBadUserInput, "No lon was given" unless params[:lon] + raise OSM::APIBadUserInput, "No text was given" if params[:text].blank? # Extract the arguments lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number") @@ -69,7 +69,7 @@ class NotesController < ApplicationController Note.transaction do # Create the note @note = Note.create(:lat => lat, :lon => lon) - raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world? + raise OSM::APIBadUserInput, "The note is outside this world" unless @note.in_world? # Save the note @note.save! @@ -92,8 +92,8 @@ class NotesController < ApplicationController raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip) # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] - raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank? + raise OSM::APIBadUserInput, "No id was given" unless params[:id] + raise OSM::APIBadUserInput, "No text was given" if params[:text].blank? # Extract the arguments id = params[:id].to_i @@ -103,7 +103,7 @@ class NotesController < ApplicationController @note = Note.find(id) raise OSM::APINotFoundError unless @note raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? - raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed? + raise OSM::APINoteAlreadyClosedError, @note if @note.closed? # Add a comment to the note Note.transaction do @@ -121,7 +121,7 @@ class NotesController < ApplicationController # Close a note def close # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i @@ -131,7 +131,7 @@ class NotesController < ApplicationController @note = Note.find_by(:id => id) raise OSM::APINotFoundError unless @note raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? - raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed? + raise OSM::APINoteAlreadyClosedError, @note if @note.closed? # Close the note and add a comment Note.transaction do @@ -151,7 +151,7 @@ class NotesController < ApplicationController # Reopen a note def reopen # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i @@ -161,7 +161,7 @@ class NotesController < ApplicationController @note = Note.find_by(:id => id) raise OSM::APINotFoundError unless @note raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || current_user.moderator? - raise OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? || !@note.visible? + raise OSM::APINoteAlreadyOpenError, @note unless @note.closed? || !@note.visible? # Reopen the note and add a comment Note.transaction do @@ -206,7 +206,7 @@ class NotesController < ApplicationController # Read a note def show # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Find the note and check it is valid @note = Note.find(params[:id]) @@ -226,7 +226,7 @@ class NotesController < ApplicationController # Delete (hide) a note def destroy # Check the arguments are sane - raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput, "No id was given" unless params[:id] # Extract the arguments id = params[:id].to_i @@ -256,7 +256,7 @@ class NotesController < ApplicationController # Return a list of notes matching a given string def search # Check the arguments are sane - raise OSM::APIBadUserInput.new("No query string was given") unless params[:q] + raise OSM::APIBadUserInput, "No query string was given" unless params[:q] # Get any conditions that need to be applied @notes = closed_condition(Note.all) @@ -310,7 +310,7 @@ class NotesController < ApplicationController if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 params[:limit].to_i else - raise OSM::APIBadUserInput.new("Note limit must be between 1 and 10000") + raise OSM::APIBadUserInput, "Note limit must be between 1 and 10000" end else 100 diff --git a/app/controllers/old_controller.rb b/app/controllers/old_controller.rb index 3815f5ae0..9adf141d9 100644 --- a/app/controllers/old_controller.rb +++ b/app/controllers/old_controller.rb @@ -19,7 +19,7 @@ class OldController < ApplicationController # the .where() method used in the lookup_old_element_versions # call won't throw an error if no records are found, so we have # to do that ourselves. - raise OSM::APINotFoundError.new if @elements.empty? + raise OSM::APINotFoundError if @elements.empty? doc = OSM::API.new.get_xml_doc diff --git a/app/controllers/relation_controller.rb b/app/controllers/relation_controller.rb index 97e832049..80fd51997 100644 --- a/app/controllers/relation_controller.rb +++ b/app/controllers/relation_controller.rb @@ -36,7 +36,7 @@ class RelationController < ApplicationController new_relation = Relation.from_xml(request.raw_post) unless new_relation && new_relation.id == relation.id - raise OSM::APIBadUserInput.new("The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})") + raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})" end relation.update_from new_relation, current_user @@ -128,13 +128,13 @@ class RelationController < ApplicationController def relations unless params["relations"] - raise OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]") + raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" end ids = params["relations"].split(",").collect(&:to_i) if ids.empty? - raise OSM::APIBadUserInput.new("No relations were given to search for") + raise OSM::APIBadUserInput, "No relations were given to search for" end doc = OSM::API.new.get_xml_doc diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index 41c488e0f..e61e51e55 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -112,7 +112,7 @@ class TraceController < ApplicationController begin do_create(params[:trace][:gpx_file], params[:trace][:tagstring], params[:trace][:description], params[:trace][:visibility]) - rescue => ex + rescue StandardError => ex logger.debug ex end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index d3ed53c1b..e418e103f 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -99,7 +99,7 @@ class UserController < ApplicationController "lat" => m[2], "lon" => m[3] }.merge(editor)) end - rescue + rescue StandardError # Use default end @@ -723,7 +723,7 @@ class UserController < ApplicationController begin Notifier.email_confirm(user, user.tokens.create).deliver_now - rescue + rescue StandardError # Ignore errors sending email end else diff --git a/app/controllers/user_preference_controller.rb b/app/controllers/user_preference_controller.rb index 4b556aed0..16165513a 100644 --- a/app/controllers/user_preference_controller.rb +++ b/app/controllers/user_preference_controller.rb @@ -45,7 +45,7 @@ class UserPreferenceController < ApplicationController if preference = old_preferences.delete(pt["k"]) preference.v = pt["v"] elsif new_preferences.include?(pt["k"]) - raise OSM::APIDuplicatePreferenceError.new(pt["k"]) + raise OSM::APIDuplicatePreferenceError, pt["k"] else preference = current_user.preferences.build(:k => pt["k"], :v => pt["v"]) end diff --git a/app/controllers/way_controller.rb b/app/controllers/way_controller.rb index 8e9e00b88..e48073e10 100644 --- a/app/controllers/way_controller.rb +++ b/app/controllers/way_controller.rb @@ -36,7 +36,7 @@ class WayController < ApplicationController new_way = Way.from_xml(request.raw_post) unless new_way && new_way.id == way.id - raise OSM::APIBadUserInput.new("The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})") + raise OSM::APIBadUserInput, "The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})" end way.update_from(new_way, current_user) @@ -81,14 +81,12 @@ class WayController < ApplicationController def ways unless params["ways"] - raise OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]") + raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]" end ids = params["ways"].split(",").collect(&:to_i) - if ids.empty? - raise OSM::APIBadUserInput.new("No ways were given to search for") - end + raise OSM::APIBadUserInput, "No ways were given to search for" if ids.empty? doc = OSM::API.new.get_xml_doc diff --git a/app/helpers/banner_helper.rb b/app/helpers/banner_helper.rb index 4e888173b..661369454 100644 --- a/app/helpers/banner_helper.rb +++ b/app/helpers/banner_helper.rb @@ -4,7 +4,7 @@ module BannerHelper enddate = v[:enddate] begin parsed = enddate && Date.parse(enddate) - rescue + rescue StandardError parsed = nil end !parsed.is_a?(Date) || (parsed.is_a?(Date) && parsed.past?) diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index 998ea405f..0470d41a2 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -1,4 +1,4 @@ -require "uri" +require "cgi" module BrowseHelper def printable_name(object, version = false) @@ -114,9 +114,17 @@ module BrowseHelper # the correct page. lookup_us = lookup.tr(" ", "_") - if page = WIKI_PAGES[locale][type][lookup_us] rescue nil + if page = begin + WIKI_PAGES[locale][type][lookup_us] + rescue + nil + end url = "http://wiki.openstreetmap.org/wiki/#{page}?uselang=#{locale}" - elsif page = WIKI_PAGES["en"][type][lookup_us] rescue nil + elsif page = begin + WIKI_PAGES["en"][type][lookup_us] + rescue + nil + end url = "http://wiki.openstreetmap.org/wiki/#{page}?uselang=#{locale}" end @@ -151,7 +159,7 @@ module BrowseHelper # Must break it up to correctly build the url value = Regexp.last_match(1) section = "#" + Regexp.last_match(2) - encoded_section = "#" + URI.encode(Regexp.last_match(2).gsub(/ +/, "_"), /[^A-Za-z0-9:_]/).tr("%", ".") + encoded_section = "#" + CGI.escape(Regexp.last_match(2).gsub(/ +/, "_")).tr("%", ".") else section = "" encoded_section = "" diff --git a/app/helpers/title_helper.rb b/app/helpers/title_helper.rb index 8b1eb53b6..ebe5c3a26 100644 --- a/app/helpers/title_helper.rb +++ b/app/helpers/title_helper.rb @@ -8,10 +8,10 @@ module TitleHelper def set_title(title = nil) if title @title = TitleHelper.coder.decode(title.gsub("", "\u202a").gsub("", "\u202c")) - response.headers["X-Page-Title"] = URI.escape(t("layouts.project_name.title") + " | " + @title) + response.headers["X-Page-Title"] = ERB::Util.u(t("layouts.project_name.title") + " | " + @title) else @title = title - response.headers["X-Page-Title"] = URI.escape(t("layouts.project_name.title")) + response.headers["X-Page-Title"] = ERB::Util.u(t("layouts.project_name.title")) end end end diff --git a/app/models/changeset.rb b/app/models/changeset.rb index f41ad9955..a4daab801 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -133,7 +133,7 @@ class Changeset < ActiveRecord::Base attr_writer :tags def add_tag_keyval(k, v) - @tags = {} unless @tags + @tags ||= {} # duplicate tags are now forbidden, so we can't allow values # in the hash to be overwritten. @@ -241,10 +241,10 @@ class Changeset < ActiveRecord::Base # bounding box, only the tags of the changeset. def update_from(other, user) # ensure that only the user who opened the changeset may modify it. - raise OSM::APIUserChangesetMismatchError.new unless user.id == user_id + raise OSM::APIUserChangesetMismatchError unless user.id == user_id # can't change a closed changeset - raise OSM::APIChangesetAlreadyClosedError.new(self) unless is_open? + raise OSM::APIChangesetAlreadyClosedError, self unless is_open? # copy the other's tags self.tags = other.tags diff --git a/app/models/node.rb b/app/models/node.rb index f4367e459..edbbbc251 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -70,7 +70,7 @@ class Node < ActiveRecord::Base raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil? node.changeset_id = pt["changeset"].to_i - raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world? + raise OSM::APIBadUserInput, "The node is outside this world" unless node.in_world? # version must be present unless creating raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil? @@ -81,7 +81,7 @@ class Node < ActiveRecord::Base node.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 node cannot be zero when updating.") if node.id.zero? + raise OSM::APIBadUserInput, "ID of node cannot be zero when updating." if node.id.zero? end # We don't care about the time, as it is explicitly set on create/update/delete @@ -120,10 +120,10 @@ class Node < ActiveRecord::Base lock! check_consistency(self, new_node, user) ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id) - raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(',')}.") unless ways.empty? + raise OSM::APIPreconditionFailedError, "Node #{id} is still used by ways #{ways.collect(&:id).join(',')}." unless ways.empty? rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Node", :member_id => id }).order(:id) - raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by relations #{rels.collect(&:id).join(',')}.") unless rels.empty? + raise OSM::APIPreconditionFailedError, "Node #{id} is still used by relations #{rels.collect(&:id).join(',')}." unless rels.empty? self.changeset_id = new_node.changeset_id self.tags = {} @@ -205,7 +205,7 @@ class Node < ActiveRecord::Base attr_writer :tags def add_tag_key_val(k, v) - @tags = {} unless @tags + @tags ||= {} # duplicate tags are now forbidden, so we can't allow values # in the hash to be overwritten. diff --git a/app/models/oauth2_token.rb b/app/models/oauth2_token.rb index 1e67194fe..2c0b1f508 100644 --- a/app/models/oauth2_token.rb +++ b/app/models/oauth2_token.rb @@ -9,9 +9,9 @@ class Oauth2Token < AccessToken def to_query q = "access_token=#{token}&token_type=bearer" - q << "&state=#{URI.escape(state)}" if @state + q << "&state=#{CGI.escape(state)}" if @state q << "&expires_in=#{expires_in}" if expires_at - q << "&scope=#{URI.escape(scope)}" if scope + q << "&scope=#{CGI.escape(scope)}" if scope q end diff --git a/app/models/oauth2_verifier.rb b/app/models/oauth2_verifier.rb index a404d0c7a..1568cac99 100644 --- a/app/models/oauth2_verifier.rb +++ b/app/models/oauth2_verifier.rb @@ -21,7 +21,7 @@ class Oauth2Verifier < OauthToken def to_query q = "code=#{token}" - q << "&state=#{URI.escape(state)}" if @state + q << "&state=#{CGI.escape(state)}" if @state q end diff --git a/app/models/relation.rb b/app/models/relation.rb index d2490dbae..545336793 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -60,7 +60,7 @@ class Relation < ActiveRecord::Base relation.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 relation cannot be zero when updating.") if relation.id.zero? + raise OSM::APIBadUserInput, "ID of relation cannot be zero when updating." if relation.id.zero? end # We don't care about the timestamp nor the visibility as these are either @@ -92,7 +92,7 @@ class Relation < ActiveRecord::Base member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string. relation.add_member(member["type"].classify, member["ref"], member["role"]) end - raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil? + raise OSM::APIBadUserInput, "Some bad xml in relation" if relation.nil? relation end @@ -155,7 +155,7 @@ class Relation < ActiveRecord::Base end def add_tag_keyval(k, v) - @tags = {} unless @tags + @tags ||= {} # duplicate tags are now forbidden, so we can't allow values # in the hash to be overwritten. @@ -187,7 +187,7 @@ class Relation < ActiveRecord::Base 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) - raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil? + raise OSM::APIPreconditionFailedError, "The relation #{new_relation.id} is used in relation #{rel.relation.id}." unless rel.nil? self.changeset_id = new_relation.changeset_id self.tags = {} @@ -202,7 +202,7 @@ class Relation < ActiveRecord::Base lock! check_consistency(self, new_relation, user) unless new_relation.preconditions_ok?(members) - raise OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.") + raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." end self.changeset_id = new_relation.changeset_id self.changeset = new_relation.changeset @@ -216,7 +216,7 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) unless preconditions_ok? - raise OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.") + raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." end self.version = 0 self.visible = true @@ -253,7 +253,7 @@ class Relation < ActiveRecord::Base # and check that it is OK to use. unless element && element.visible? && element.preconditions_ok? - raise OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}") + raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" end hash[m[1]] = true end @@ -270,7 +270,7 @@ class Relation < ActiveRecord::Base old_id = id.to_i if old_id < 0 new_id = id_map[type.downcase.to_sym][old_id] - raise OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil? + raise OSM::APIBadUserInput, "Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}." if new_id.nil? [type, new_id, role] else [type, id, role] diff --git a/app/models/trace.rb b/app/models/trace.rb index 160060a6d..a2c1b79a7 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -194,8 +194,8 @@ class Trace < ActiveRecord::Base 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 id.zero? - raise OSM::APIBadUserInput.new("The id in the url (#{self.id}) is not the same as provided in the xml (#{id})") unless self.id == id + raise OSM::APIBadUserInput, "ID of trace cannot be zero when updating." if id.zero? + raise OSM::APIBadUserInput, "The id in the url (#{self.id}) is not the same as provided in the xml (#{id})" unless self.id == id end # We don't care about the time, as it is explicitly set on create/update/delete diff --git a/app/models/way.rb b/app/models/way.rb index 9586094ff..212998ccc 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -58,7 +58,7 @@ class Way < ActiveRecord::Base way.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 way cannot be zero when updating.") if way.id.zero? + raise OSM::APIBadUserInput, "ID of way cannot be zero when updating." if way.id.zero? end # We don't care about the timestamp nor the visibility as these are either @@ -138,12 +138,12 @@ class Way < ActiveRecord::Base attr_writer :tags def add_nd_num(n) - @nds = [] unless @nds + @nds ||= [] @nds << n.to_i end def add_tag_keyval(k, v) - @tags = {} unless @tags + @tags ||= {} # duplicate tags are now forbidden, so we can't allow values # in the hash to be overwritten. @@ -166,7 +166,7 @@ class Way < ActiveRecord::Base lock! check_consistency(self, new_way, user) unless new_way.preconditions_ok?(nds) - raise OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.") + raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid." end self.changeset_id = new_way.changeset_id @@ -181,7 +181,7 @@ class Way < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) unless preconditions_ok? - raise OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.") + raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." end self.version = 0 self.visible = true @@ -205,7 +205,7 @@ class Way < ActiveRecord::Base if db_nds.length < new_nds.length missing = new_nds - db_nds.collect(&:id) - raise OSM::APIPreconditionFailedError.new("Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible.") + raise OSM::APIPreconditionFailedError, "Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible." end end @@ -222,7 +222,7 @@ class Way < ActiveRecord::Base 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) - raise OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(',')}.") unless rels.empty? + raise OSM::APIPreconditionFailedError, "Way #{id} is still used by relations #{rels.collect(&:id).join(',')}." unless rels.empty? self.changeset_id = new_way.changeset_id self.changeset = new_way.changeset @@ -242,7 +242,7 @@ class Way < ActiveRecord::Base nds.map! do |node_id| if node_id < 0 new_id = id_map[:node][node_id] - raise OSM::APIBadUserInput.new("Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}") if new_id.nil? + raise OSM::APIBadUserInput, "Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}" if new_id.nil? new_id else node_id diff --git a/config/initializers/banners.rb b/config/initializers/banners.rb index 5c83c045d..8eda290c5 100644 --- a/config/initializers/banners.rb +++ b/config/initializers/banners.rb @@ -1,5 +1,5 @@ begin BANNERS = YAML.load_file(Rails.root.join("config", "banners.yml")).deep_symbolize_keys -rescue +rescue StandardError BANNERS = {}.freeze end diff --git a/db/migrate/006_tile_nodes.rb b/db/migrate/006_tile_nodes.rb index 29200d06a..1f27ec79e 100644 --- a/db/migrate/006_tile_nodes.rb +++ b/db/migrate/006_tile_nodes.rb @@ -3,33 +3,33 @@ require "migrate" class TileNodes < ActiveRecord::Migration def self.upgrade_table(from_table, to_table, model) if ENV["USE_DB_FUNCTIONS"] - execute <<-END_SQL + execute <<-SQL INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile) SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000), user_id, visible, tags, timestamp, tile_for_point(CAST(ROUND(latitude * 10000000) AS INTEGER), CAST(ROUND(longitude * 10000000) AS INTEGER)) FROM #{from_table} - END_SQL + SQL else - execute <<-END_SQL + execute <<-SQL INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile) SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000), user_id, visible, tags, timestamp, 0 FROM #{from_table} - END_SQL + SQL model.all.each(&:save!) end end def self.downgrade_table(from_table, to_table) - execute <<-END_SQL + execute <<-SQL INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp) SELECT id, latitude / 10000000, longitude / 10000000, user_id, visible, tags, timestamp FROM #{from_table} - END_SQL + SQL end def self.up diff --git a/db/migrate/008_remove_segments.rb b/db/migrate/008_remove_segments.rb index 62f618abf..fe3716152 100644 --- a/db/migrate/008_remove_segments.rb +++ b/db/migrate/008_remove_segments.rb @@ -10,7 +10,7 @@ class RemoveSegments < ActiveRecord::Migration cmd = "db/migrate/008_remove_segments_helper" src = "#{cmd}.cc" if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src) - system("c++ -O3 -Wall `mysql_config --cflags --libs` " + + system("c++ -O3 -Wall `mysql_config --cflags --libs` " \ "#{src} -o #{cmd}") || raise end diff --git a/db/migrate/020_populate_node_tags_and_remove.rb b/db/migrate/020_populate_node_tags_and_remove.rb index 0fd1dc564..e2d253d17 100644 --- a/db/migrate/020_populate_node_tags_and_remove.rb +++ b/db/migrate/020_populate_node_tags_and_remove.rb @@ -10,7 +10,7 @@ class PopulateNodeTagsAndRemove < ActiveRecord::Migration cmd = "db/migrate/020_populate_node_tags_and_remove_helper" src = "#{cmd}.c" if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src) - system("cc -O3 -Wall `mysql_config --cflags --libs` " + + system("cc -O3 -Wall `mysql_config --cflags --libs` " \ "#{src} -o #{cmd}") || raise end diff --git a/db/migrate/023_add_changesets.rb b/db/migrate/023_add_changesets.rb index 933a62cd5..8265353a7 100644 --- a/db/migrate/023_add_changesets.rb +++ b/db/migrate/023_add_changesets.rb @@ -28,7 +28,7 @@ class AddChangesets < ActiveRecord::Migration # all edits up to the API change, # 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)" + + execute "INSERT INTO changesets (id, user_id, created_at, open)" \ "SELECT id, id, creation_time, false from users;" @conv_user_tables.each do |tbl| diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index 51972efbc..f12683d3c 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -66,18 +66,14 @@ class BoundingBox def check_boundaries # check the bbox is sane if min_lon > max_lon - raise OSM::APIBadBoundingBox.new( - "The minimum longitude must be less than the maximum longitude, but it wasn't" - ) + raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't" end if min_lat > max_lat - raise OSM::APIBadBoundingBox.new( - "The minimum latitude must be less than the maximum latitude, but it wasn't" - ) + raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't" end if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT - raise OSM::APIBadBoundingBox.new("The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," + - " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}") + raise OSM::APIBadBoundingBox, "The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," \ + " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}" end self end @@ -85,8 +81,8 @@ class BoundingBox def check_size(max_area = MAX_REQUEST_AREA) # check the bbox isn't too large if area > max_area - raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + max_area.to_s + - ", and your request was too large. Either request a smaller area, or use planet.osm") + raise OSM::APIBadBoundingBox, "The maximum bbox size is " + max_area.to_s + + ", and your request was too large. Either request a smaller area, or use planet.osm" end self end @@ -171,9 +167,7 @@ class BoundingBox def from_bbox_array(bbox_array) unless bbox_array - raise OSM::APIBadUserInput.new( - "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" - ) + raise OSM::APIBadUserInput, "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. diff --git a/lib/consistency_validations.rb b/lib/consistency_validations.rb index 0d17d2830..7e9b33586 100644 --- a/lib/consistency_validations.rb +++ b/lib/consistency_validations.rb @@ -7,26 +7,26 @@ module ConsistencyValidations # This will throw an exception if there is an inconsistency def check_consistency(old, new, user) if new.id != old.id || new.id.nil? || old.id.nil? - raise OSM::APIPreconditionFailedError.new("New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}.") + raise OSM::APIPreconditionFailedError, "New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}." elsif new.version != old.version raise OSM::APIVersionMismatchError.new(new.id, new.class.to_s, new.version, old.version) elsif new.changeset.nil? - raise OSM::APIChangesetMissingError.new + raise OSM::APIChangesetMissingError elsif new.changeset.user_id != user.id - raise OSM::APIUserChangesetMismatchError.new + raise OSM::APIUserChangesetMismatchError elsif !new.changeset.is_open? - raise OSM::APIChangesetAlreadyClosedError.new(new.changeset) + raise OSM::APIChangesetAlreadyClosedError, new.changeset end end # This is similar to above, just some validations don't apply def check_create_consistency(new, user) if new.changeset.nil? - raise OSM::APIChangesetMissingError.new + raise OSM::APIChangesetMissingError elsif new.changeset.user_id != user.id - raise OSM::APIUserChangesetMismatchError.new + raise OSM::APIUserChangesetMismatchError elsif !new.changeset.is_open? - raise OSM::APIChangesetAlreadyClosedError.new(new.changeset) + raise OSM::APIChangesetAlreadyClosedError, new.changeset end end @@ -37,11 +37,11 @@ module ConsistencyValidations # check user credentials - only the user who opened a changeset # may alter it. if changeset.nil? - raise OSM::APIChangesetMissingError.new + raise OSM::APIChangesetMissingError elsif user.id != changeset.user_id - raise OSM::APIUserChangesetMismatchError.new + raise OSM::APIUserChangesetMismatchError elsif !changeset.is_open? - raise OSM::APIChangesetAlreadyClosedError.new(changeset) + raise OSM::APIChangesetAlreadyClosedError, changeset end end end diff --git a/lib/daemons/gpx_import_ctl b/lib/daemons/gpx_import_ctl index 00b3a00b3..495ce1fb1 100755 --- a/lib/daemons/gpx_import_ctl +++ b/lib/daemons/gpx_import_ctl @@ -6,7 +6,7 @@ require "erb" class Hash def with_symbols! - keys.each { |key| self[key.to_s.to_sym] = self[key] } + each_key { |key| self[key.to_s.to_sym] = self[key] } self end end diff --git a/lib/diff_reader.rb b/lib/diff_reader.rb index c2e7f1839..94c41a6d5 100644 --- a/lib/diff_reader.rb +++ b/lib/diff_reader.rb @@ -86,8 +86,8 @@ class DiffReader with_element do |model_name, _model_attributes| model = MODELS[model_name] if model.nil? - raise OSM::APIBadUserInput.new("Unexpected element type #{model_name}, " + - "expected node, way or relation.") + raise OSM::APIBadUserInput, "Unexpected element type #{model_name}, " \ + "expected node, way or relation." end # new in libxml-ruby >= 2, expand returns an element not associated # with a document. this means that there's no encoding parameter, @@ -130,7 +130,7 @@ class DiffReader # take the first element and check that it is an osmChange element @reader.read - raise OSM::APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != "osmChange" + raise OSM::APIBadUserInput, "Document element should be 'osmChange'." if @reader.name != "osmChange" result = OSM::API.new.get_xml_doc result.root.name = "diffResult" @@ -152,7 +152,7 @@ class DiffReader # check if the placeholder ID has been given before and throw # an exception if it has - we can't create the same element twice. model_sym = model.to_s.downcase.to_sym - raise OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id + raise OSM::APIBadUserInput, "Placeholder IDs must be unique for created elements." if ids[model_sym].include? placeholder_id # some elements may have placeholders for other elements in the # diff, so we must fix these before saving the element. @@ -252,7 +252,7 @@ class DiffReader else # no other actions to choose from, so it must be the users fault! - raise OSM::APIChangesetActionInvalid.new(action_name) + raise OSM::APIChangesetActionInvalid, action_name end end diff --git a/lib/not_redactable.rb b/lib/not_redactable.rb index 7fe119fea..6a5773296 100644 --- a/lib/not_redactable.rb +++ b/lib/not_redactable.rb @@ -6,6 +6,6 @@ module NotRedactable end def redact!(_r) - raise OSM::APICannotRedactError.new + raise OSM::APICannotRedactError end end diff --git a/lib/osm.rb b/lib/osm.rb index 80b68c2a9..e4835a76b 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -532,7 +532,7 @@ module OSM # Parse a float, raising a specified exception on failure def self.parse_float(str, klass, *args) Float(str) - rescue + rescue StandardError raise klass.new(*args) end @@ -553,7 +553,7 @@ module OSM tilesql = QuadTile.sql_for_area(bbox, prefix) bbox = bbox.to_scaled - "#{tilesql} AND #{prefix}latitude BETWEEN #{bbox.min_lat} AND #{bbox.max_lat} " + + "#{tilesql} AND #{prefix}latitude BETWEEN #{bbox.min_lat} AND #{bbox.max_lat} " \ "AND #{prefix}longitude BETWEEN #{bbox.min_lon} AND #{bbox.max_lon}" end diff --git a/lib/redactable.rb b/lib/redactable.rb index 6adfec72a..d827cfd74 100644 --- a/lib/redactable.rb +++ b/lib/redactable.rb @@ -13,7 +13,7 @@ module Redactable def redact!(redaction) # check that this version isn't the current version - raise OSM::APICannotRedactError.new if is_latest_version? + raise OSM::APICannotRedactError if is_latest_version? # make the change self.redaction = redaction diff --git a/lib/session_persistence.rb b/lib/session_persistence.rb index 7d42b71e7..ac0e690c2 100644 --- a/lib/session_persistence.rb +++ b/lib/session_persistence.rb @@ -55,7 +55,7 @@ module SessionPersistence if session[session_persistence_key] request.session_options[:expire_after] = session[session_persistence_key] end - rescue + rescue StandardError reset_session end end diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 9bf3a4b8e..b1aa79810 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -98,16 +98,16 @@ class ChangesetControllerTest < ActionController::TestCase def test_create basic_authorization create(:user, :data_public => false).email, "test" # Create the first user's changeset - content "" + - "" + + content "" \ + "" \ "" put :create assert_require_public_data basic_authorization create(:user).email, "test" # Create the first user's changeset - content "" + - "" + + content "" \ + "" \ "" put :create @@ -327,7 +327,7 @@ class ChangesetControllerTest < ActionController::TestCase # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -343,7 +343,7 @@ class ChangesetControllerTest < ActionController::TestCase -EOF +CHANGESET # upload it content diff @@ -357,7 +357,7 @@ EOF # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -373,7 +373,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -387,7 +387,7 @@ EOF # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -403,7 +403,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -429,7 +429,7 @@ EOF basic_authorization user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -448,7 +448,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -561,8 +561,8 @@ EOF basic_authorization create(:user).email, "test" # create a temporary changeset - content "" + - "" + + content "" \ + "" \ "" assert_difference "Changeset.count", 1 do put :create @@ -582,7 +582,7 @@ EOF # upload some widely-spaced nodes, spiralling positive and negative to cause # largest bbox over-expansion possible. - diff = < @@ -605,7 +605,7 @@ EOF -EOF +CHANGESET # upload it, which used to cause an error like "PGError: ERROR: # integer out of range" (bug #2152). but shouldn't any more. @@ -739,7 +739,7 @@ EOF basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -747,7 +747,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -770,7 +770,7 @@ EOF basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -790,7 +790,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -832,7 +832,7 @@ EOF basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -854,7 +854,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -878,7 +878,7 @@ EOF # change the location of a node multiple times, each time referencing # the last version. doesn't this depend on version numbers being # sequential? - diff = < @@ -891,7 +891,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -914,14 +914,14 @@ EOF basic_authorization changeset.user.email, "test" - diff = < -EOF +CHANGESET # upload it content diff @@ -937,13 +937,13 @@ EOF basic_authorization changeset.user.email, "test" - diff = < -EOF +CHANGESET # upload it content diff @@ -959,13 +959,13 @@ EOF basic_authorization changeset.user.email, "test" - diff = < -EOF +CHANGESET content diff post :upload, :params => { :id => changeset.id } assert_response :bad_request, "Shouldn't be able to upload a diff with the action ping" @@ -985,7 +985,7 @@ EOF basic_authorization changeset.user.email, "test" - diff = < @@ -997,7 +997,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1021,7 +1021,7 @@ EOF basic_authorization changeset.user.email, "test" - diff = < @@ -1035,7 +1035,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1056,7 +1056,7 @@ EOF basic_authorization changeset.user.email, "test" - diff = < @@ -1064,7 +1064,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1082,7 +1082,7 @@ EOF basic_authorization changeset.user.email, "test" - diff = < @@ -1096,7 +1096,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1106,7 +1106,7 @@ EOF assert_equal "Placeholder node not found for reference -4 in way -1", @response.body # the same again, but this time use an existing way - diff = < @@ -1120,7 +1120,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1139,7 +1139,7 @@ EOF basic_authorization changeset.user.email, "test" - diff = < @@ -1153,7 +1153,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1163,7 +1163,7 @@ EOF assert_equal "Placeholder Node not found for reference -4 in relation -1.", @response.body # the same again, but this time use an existing relation - diff = < @@ -1177,7 +1177,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1193,8 +1193,8 @@ EOF def test_upload_node_move basic_authorization create(:user).email, "test" - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :success @@ -1231,8 +1231,8 @@ EOF def test_upload_way_extend basic_authorization create(:user).email, "test" - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :success @@ -1280,7 +1280,7 @@ EOF # upload it content diff post :upload, :params => { :id => changeset.id } - assert_response(:success, "should be able to upload " + + assert_response(:success, "should be able to upload " \ "empty changeset: " + diff) end end @@ -1324,8 +1324,8 @@ EOF basic_authorization create(:user, :data_public => false).email, "test" # create a temporary changeset - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :forbidden @@ -1334,15 +1334,15 @@ EOF basic_authorization create(:user).email, "test" # create a temporary changeset - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :success changeset_id = @response.body.to_i # add a diff to it - diff = < @@ -1355,7 +1355,7 @@ EOF -EOF +CHANGESET # upload it content diff @@ -1380,8 +1380,8 @@ EOF basic_authorization create(:user).email, "test" # create a temporary changeset - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :success @@ -1442,15 +1442,15 @@ OSMFILE basic_authorization create(:user).email, "test" # create a temporary changeset - content "" + - "" + + content "" \ + "" \ "" put :create assert_response :success changeset_id = @response.body.to_i # add a diff to it - diff = < @@ -1470,7 +1470,7 @@ OSMFILE -EOF +CHANGESET # upload it content diff @@ -1880,7 +1880,7 @@ EOF # check that the changeset is now closed as well assert(!changeset.is_open?, - "changeset should have been auto-closed by exceeding " + + "changeset should have been auto-closed by exceeding " \ "element limit.") end diff --git a/test/controllers/diary_entry_controller_test.rb b/test/controllers/diary_entry_controller_test.rb index fcd93eb2a..f95c57d7c 100644 --- a/test/controllers/diary_entry_controller_test.rb +++ b/test/controllers/diary_entry_controller_test.rb @@ -254,7 +254,7 @@ class DiaryEntryControllerTest < ActionController::TestCase get :edit, :params => { :display_name => entry.user.display_name, :id => entry.id } assert_response :redirect - assert_redirected_to :controller => :user, :action => :login, :referer => "/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit" + assert_redirected_to :controller => :user, :action => :login, :referer => "/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit" # Verify that you get a not found error, when you pass a bogus id get :edit, @@ -284,7 +284,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_select "h1", :text => /Edit diary entry/, :count => 1 end assert_select "div#content", :count => 1 do - assert_select "form[action='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit'][method=post]", :count => 1 do + assert_select "form[action='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit'][method=post]", :count => 1 do assert_select "input#diary_entry_title[name='diary_entry[title]'][value='#{entry.title}']", :count => 1 assert_select "textarea#diary_entry_body[name='diary_entry[body]']", :text => entry.body, :count => 1 assert_select "select#diary_entry_language_code", :count => 1 @@ -329,7 +329,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_select "abbr[class='geo'][title='#{number_with_precision(new_latitude, :precision => 4)}; #{number_with_precision(new_longitude, :precision => 4)}']", :count => 1 # As we're not logged in, check that you cannot edit # print @response.body - assert_select "a[href='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1 + assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1 end # and when not logged in as the user who wrote the entry @@ -350,7 +350,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_select "abbr[class=geo][title='#{number_with_precision(new_latitude, :precision => 4)}; #{number_with_precision(new_longitude, :precision => 4)}']", :count => 1 # As we're not logged in, check that you cannot edit assert_select "li[class='hidden show_if_user_#{entry.user.id}']", :count => 1 do - assert_select "a[href='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1 + assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1 end end end @@ -430,7 +430,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_response :success assert_select ".diary-comment", :count => 1 do assert_select "#comment#{comment.id}", :count => 1 do - assert_select "a[href='/user/#{URI.encode(other_user.display_name)}']", :text => other_user.display_name, :count => 1 + assert_select "a[href='/user/#{ERB::Util.u(other_user.display_name)}']", :text => other_user.display_name, :count => 1 end assert_select ".richtext", :text => /New comment/, :count => 1 end @@ -890,7 +890,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_select "div.diary_post", entries.count entries.each do |entry| - assert_select "a[href=?]", "/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}" + assert_select "a[href=?]", "/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}" end end end diff --git a/test/controllers/geocoder_controller_test.rb b/test/controllers/geocoder_controller_test.rb index d29d77e8b..ee8991d48 100644 --- a/test/controllers/geocoder_controller_test.rb +++ b/test/controllers/geocoder_controller_test.rb @@ -1,4 +1,3 @@ -# coding: utf-8 require "test_helper" require "geocoder_controller" diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb index f2ef39dff..1fdb3f907 100644 --- a/test/controllers/node_controller_test.rb +++ b/test/controllers/node_controller_test.rb @@ -496,8 +496,8 @@ class NodeControllerTest < ActionController::TestCase # try and put something into a string that the API might # use unquoted and therefore allow code injection... - content "" + - '' + + content "" \ + '' \ "" put :create assert_require_public_data "Shouldn't be able to create with non-public user" @@ -507,8 +507,8 @@ class NodeControllerTest < ActionController::TestCase # try and put something into a string that the API might # use unquoted and therefore allow code injection... - content "" + - '' + + content "" \ + '' \ "" put :create assert_response :success diff --git a/test/controllers/old_node_controller_test.rb b/test/controllers/old_node_controller_test.rb index a05326e1e..e08fbb65e 100644 --- a/test/controllers/old_node_controller_test.rb +++ b/test/controllers/old_node_controller_test.rb @@ -137,7 +137,7 @@ class OldNodeControllerTest < ActionController::TestCase end # check all the versions - versions.keys.each do |key| + versions.each_key do |key| get :version, :params => { :id => nodeid, :version => key.to_i } assert_response :success, diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index 4b41cbc22..f90b6b58e 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -216,8 +216,8 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member # This time try with a role attribute in the relation - content "" + - "" + + content "" \ + "" \ "" put :create # hope for forbidden due to user @@ -227,7 +227,7 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - content "" + + content "" \ "" + "" put :create # hope for forbidden due to user @@ -236,9 +236,9 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a way and a node as members - content "" + - "" + - "" + + content "" \ + "" \ + "" \ "" put :create # hope for forbidden, due to user @@ -277,8 +277,8 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member # This time try with a role attribute in the relation - content "" + - "" + + content "" \ + "" \ "" put :create # hope for success @@ -308,7 +308,7 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - content "" + + content "" \ "" + "" put :create # hope for success @@ -337,9 +337,9 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a way and a node as members - content "" + - "" + - "" + + content "" \ + "" \ + "" \ "" put :create # hope for success @@ -461,8 +461,8 @@ class RelationControllerTest < ActionController::TestCase basic_authorization user.email, "test" # create a relation with non-existing node as member - content "" + - "" + + content "" \ + "" \ "" put :create # expect failure @@ -482,8 +482,8 @@ class RelationControllerTest < ActionController::TestCase basic_authorization user.email, "test" # create some xml that should return an error - content "" + - "" + + content "" \ + "" \ "" put :create # expect failure @@ -1063,7 +1063,7 @@ OSM assert_equal a_tags.keys, b_tags.keys, "Tag keys should be identical." a_tags.each do |k, v| assert_equal v, b_tags[k], - "Tags which were not altered should be the same. " + + "Tags which were not altered should be the same. " \ "#{a_tags.inspect} != #{b_tags.inspect}" end end diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb index 1561a9ebe..e149b83c7 100644 --- a/test/controllers/user_controller_test.rb +++ b/test/controllers/user_controller_test.rb @@ -778,7 +778,7 @@ class UserControllerTest < ActionController::TestCase # you are not logged in get :account, :params => { :display_name => user.display_name } assert_response :redirect - assert_redirected_to :controller => :user, :action => "login", :referer => "/user/#{URI.encode(user.display_name)}/account" + assert_redirected_to :controller => :user, :action => "login", :referer => "/user/#{ERB::Util.u(user.display_name)}/account" # Make sure that you are blocked when not logged in as the right user get :account, :params => { :display_name => user.display_name }, :session => { :user => create(:user) } @@ -791,7 +791,7 @@ class UserControllerTest < ActionController::TestCase assert_select "form#accountForm" do |form| assert_equal "post", form.attr("method").to_s assert_select "input[name='_method']", false - assert_equal "/user/#{URI.encode(user.display_name)}/account", form.attr("action").to_s + assert_equal "/user/#{ERB::Util.u(user.display_name)}/account", form.attr("action").to_s end # Updating the description should work @@ -859,7 +859,7 @@ class UserControllerTest < ActionController::TestCase # Adding external authentication should redirect to the auth provider post :account, :params => { :display_name => user.display_name, :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") }, :session => { :user => user } assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/user/#{URI.encode(user.display_name)}/account") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/user/#{ERB::Util.u(user.display_name)}/account") # Changing name to one that exists should fail new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name) @@ -941,14 +941,14 @@ class UserControllerTest < ActionController::TestCase get :view, :params => { :display_name => user.display_name } assert_response :success assert_select "div#userinformation" do - assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/traces']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 0 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0 - assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 0 + assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0 + assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 0 end # Test a user who has been blocked @@ -957,14 +957,14 @@ class UserControllerTest < ActionController::TestCase get :view, :params => { :display_name => blocked_user.display_name } assert_response :success assert_select "div#userinformation" do - assert_select "a[href^='/user/#{URI.encode(blocked_user.display_name)}/history']", 1 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/traces']", 1 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/diary']", 1 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/diary/comments']", 1 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/account']", 0 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/blocks']", 1 - assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/blocks_by']", 0 - assert_select "a[href='/blocks/new/#{URI.encode(blocked_user.display_name)}']", 0 + assert_select "a[href^='/user/#{ERB::Util.u(blocked_user.display_name)}/history']", 1 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/traces']", 1 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary']", 1 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary/comments']", 1 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/account']", 0 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks']", 1 + assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks_by']", 0 + assert_select "a[href='/blocks/new/#{ERB::Util.u(blocked_user.display_name)}']", 0 end # Test a moderator who has applied blocks @@ -973,14 +973,14 @@ class UserControllerTest < ActionController::TestCase get :view, :params => { :display_name => moderator_user.display_name } assert_response :success assert_select "div#userinformation" do - assert_select "a[href^='/user/#{URI.encode(moderator_user.display_name)}/history']", 1 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/traces']", 1 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/diary']", 1 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/diary/comments']", 1 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/account']", 0 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/blocks']", 0 - assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/blocks_by']", 1 - assert_select "a[href='/blocks/new/#{URI.encode(moderator_user.display_name)}']", 0 + assert_select "a[href^='/user/#{ERB::Util.u(moderator_user.display_name)}/history']", 1 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/traces']", 1 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary']", 1 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary/comments']", 1 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/account']", 0 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks']", 0 + assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks_by']", 1 + assert_select "a[href='/blocks/new/#{ERB::Util.u(moderator_user.display_name)}']", 0 end # Login as a normal user @@ -990,14 +990,14 @@ class UserControllerTest < ActionController::TestCase get :view, :params => { :display_name => user.display_name } assert_response :success assert_select "div#userinformation" do - assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1 + assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1 assert_select "a[href='/traces/mine']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0 - assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0 + assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 0 end # Login as a moderator @@ -1007,14 +1007,14 @@ class UserControllerTest < ActionController::TestCase get :view, :params => { :display_name => user.display_name } assert_response :success assert_select "div#userinformation" do - assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/traces']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 0 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0 - assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0 - assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 1 + assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0 + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0 + assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 1 end end diff --git a/test/controllers/way_controller_test.rb b/test/controllers/way_controller_test.rb index 415033b48..285efe269 100644 --- a/test/controllers/way_controller_test.rb +++ b/test/controllers/way_controller_test.rb @@ -128,8 +128,8 @@ class WayControllerTest < ActionController::TestCase changeset_id = private_changeset.id # create a way with pre-existing nodes - content "" + - "" + + content "" \ + "" \ "" put :create # hope for failure @@ -143,8 +143,8 @@ class WayControllerTest < ActionController::TestCase changeset_id = changeset.id # create a way with pre-existing nodes - content "" + - "" + + content "" \ + "" \ "" put :create # hope for success @@ -188,7 +188,7 @@ class WayControllerTest < ActionController::TestCase # use the first user's open changeset # create a way with non-existing node - content "" + + content "" \ "" put :create # expect failure @@ -196,7 +196,7 @@ class WayControllerTest < ActionController::TestCase "way upload with invalid node using a private user did not return 'forbidden'" # create a way with no nodes - content "" + + content "" \ "" put :create # expect failure @@ -204,7 +204,7 @@ class WayControllerTest < ActionController::TestCase "way upload with no node using a private userdid not return 'forbidden'" # create a way inside a closed changeset - content "" + + content "" \ "" put :create # expect failure @@ -216,7 +216,7 @@ class WayControllerTest < ActionController::TestCase # use the first user's open changeset # create a way with non-existing node - content "" + + content "" \ "" put :create # expect failure @@ -225,7 +225,7 @@ class WayControllerTest < ActionController::TestCase assert_equal "Precondition failed: Way requires the nodes with id in (0), which either do not exist, or are not visible.", @response.body # create a way with no nodes - content "" + + content "" \ "" put :create # expect failure @@ -234,7 +234,7 @@ class WayControllerTest < ActionController::TestCase assert_equal "Precondition failed: Cannot create way: data is invalid.", @response.body # create a way inside a closed changeset - content "" + + content "" \ "" put :create # expect failure @@ -242,9 +242,9 @@ class WayControllerTest < ActionController::TestCase "way upload to closed changeset did not return 'conflict'" # create a way with a tag which is too long - content "" + - "" + - "" + + content "" \ + "" \ + "" \ "" put :create # expect failure diff --git a/test/helpers/changeset_helper_test.rb b/test/helpers/changeset_helper_test.rb index 9c45b62dc..d932138f8 100644 --- a/test/helpers/changeset_helper_test.rb +++ b/test/helpers/changeset_helper_test.rb @@ -3,7 +3,7 @@ require "test_helper" class ChangesetHelperTest < ActionView::TestCase def test_changeset_user_link changeset = create(:changeset) - assert_equal %(#{changeset.user.display_name}), changeset_user_link(changeset) + assert_equal %(#{changeset.user.display_name}), changeset_user_link(changeset) changeset = create(:changeset, :user => create(:user, :data_public => false)) assert_equal "anonymous", changeset_user_link(changeset) @@ -20,7 +20,7 @@ class ChangesetHelperTest < ActionView::TestCase assert_match %r{^Created .* by anonymous$}, changeset_details(changeset) changeset = create(:changeset, :created_at => Time.utc(2007, 1, 1, 0, 0, 0), :closed_at => Time.utc(2007, 1, 2, 0, 0, 0)) - user_link = %(#{changeset.user.display_name}) + user_link = %(#{changeset.user.display_name}) assert_match %r{^Closed .* by #{user_link}$}, changeset_details(changeset) end diff --git a/test/helpers/note_helper_test.rb b/test/helpers/note_helper_test.rb index 5857af6bc..408236879 100644 --- a/test/helpers/note_helper_test.rb +++ b/test/helpers/note_helper_test.rb @@ -9,7 +9,7 @@ class NoteHelperTest < ActionView::TestCase user = create(:user) assert_match %r{^Created by anonymous .* ago$}, note_event("open", date, nil) - assert_match %r{^Resolved by #{user.display_name} .* ago$}, note_event("closed", date, user) + assert_match %r{^Resolved by #{user.display_name} .* ago$}, note_event("closed", date, user) end def test_note_author @@ -18,7 +18,7 @@ class NoteHelperTest < ActionView::TestCase assert_equal "", note_author(nil) assert_equal "deleted", note_author(deleted_user) - assert_equal "#{user.display_name}", note_author(user) - assert_equal "#{user.display_name}", note_author(user, :only_path => false) + assert_equal "#{user.display_name}", note_author(user) + assert_equal "#{user.display_name}", note_author(user, :only_path => false) end end diff --git a/test/helpers/title_helper_test.rb b/test/helpers/title_helper_test.rb index 7d8355430..25486f799 100644 --- a/test/helpers/title_helper_test.rb +++ b/test/helpers/title_helper_test.rb @@ -1,4 +1,3 @@ -# coding: utf-8 require "test_helper" @@ -17,11 +16,11 @@ class TitleHelperTest < ActionView::TestCase assert_equal "Test Title", @title set_title("Test & Title") - assert_equal "OpenStreetMap%20%7C%20Test%20&%20Title", response.header["X-Page-Title"] + assert_equal "OpenStreetMap%20%7C%20Test%20%26%20Title", response.header["X-Page-Title"] assert_equal "Test & Title", @title set_title("Tést & Tïtlè") - assert_equal "OpenStreetMap%20%7C%20T%C3%A9st%20&%20T%C3%AFtl%C3%A8", response.header["X-Page-Title"] + assert_equal "OpenStreetMap%20%7C%20T%C3%A9st%20%26%20T%C3%AFtl%C3%A8", response.header["X-Page-Title"] assert_equal "Tést & Tïtlè", @title end end diff --git a/test/helpers/user_roles_helper_test.rb b/test/helpers/user_roles_helper_test.rb index 3ffc9fbff..058d6abd4 100644 --- a/test/helpers/user_roles_helper_test.rb +++ b/test/helpers/user_roles_helper_test.rb @@ -18,11 +18,11 @@ class UserRolesHelperTest < ActionView::TestCase user = create(:user) icon = role_icon(user, "moderator") - assert_dom_equal %(Grant moderator access), icon + assert_dom_equal %(Grant moderator access), icon moderator_user = create(:moderator_user) icon = role_icon(moderator_user, "moderator") - assert_dom_equal %(Revoke moderator access), icon + assert_dom_equal %(Revoke moderator access), icon end def test_role_icons_normal @@ -43,14 +43,14 @@ class UserRolesHelperTest < ActionView::TestCase user = create(:user) icons = role_icons(user) - assert_dom_equal %( Grant administrator access Grant moderator access), icons + assert_dom_equal %( Grant administrator access Grant moderator access), icons moderator_user = create(:moderator_user) icons = role_icons(moderator_user) - assert_dom_equal %( Grant administrator access Revoke moderator access), icons + assert_dom_equal %( Grant administrator access Revoke moderator access), icons super_user = create(:super_user) icons = role_icons(super_user) - assert_dom_equal %( Revoke administrator access Revoke moderator access), icons + assert_dom_equal %( Revoke administrator access Revoke moderator access), icons end end diff --git a/test/integration/client_applications_test.rb b/test/integration/client_applications_test.rb index 8c5e70784..59f234c41 100644 --- a/test/integration/client_applications_test.rb +++ b/test/integration/client_applications_test.rb @@ -12,35 +12,35 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true" follow_redirect! assert_response :success - post "/login", :params => { "username" => user.email, "password" => "test", :referer => "/user/#{URI.encode(user.display_name)}" } + post "/login", :params => { "username" => user.email, "password" => "test", :referer => "/user/#{ERB::Util.u(user.display_name)}" } assert_response :redirect follow_redirect! assert_response :success assert_template "user/view" - get "/user/#{URI.encode(user.display_name)}/account" + get "/user/#{ERB::Util.u(user.display_name)}/account" assert_response :success assert_template "user/account" # check that the form to allow new client application creations exists assert_in_heading do - assert_select "ul.secondary-actions li a[href='/user/#{URI.encode(user.display_name)}/oauth_clients']" + assert_select "ul.secondary-actions li a[href='/user/#{ERB::Util.u(user.display_name)}/oauth_clients']" end # now we follow the link to the oauth client list - get "/user/#{URI.encode(user.display_name)}/oauth_clients" + get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients" assert_response :success assert_in_body do - assert_select "a[href='/user/#{URI.encode(user.display_name)}/oauth_clients/new']" + assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/oauth_clients/new']" end # now we follow the link to the new oauth client page - get "/user/#{URI.encode(user.display_name)}/oauth_clients/new" + get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients/new" assert_response :success assert_in_heading do assert_select "h1", "Register a new application" end assert_in_body do - assert_select "form[action='/user/#{URI.encode(user.display_name)}/oauth_clients']" do + assert_select "form[action='/user/#{ERB::Util.u(user.display_name)}/oauth_clients']" do [:name, :url, :callback_url, :support_url].each do |inp| assert_select "input[name=?]", "client_application[#{inp}]" end @@ -50,7 +50,7 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest end end - post "/user/#{URI.encode(user.display_name)}/oauth_clients", + post "/user/#{ERB::Util.u(user.display_name)}/oauth_clients", :params => { "client_application[name]" => "My New App", "client_application[url]" => "http://my.new.app.org/", "client_application[callback_url]" => "http://my.new.app.org/callback", @@ -62,7 +62,7 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest assert_equal "Registered the information successfully", flash[:notice] # now go back to the account page and check its listed under this user - get "/user/#{URI.encode(user.display_name)}/oauth_clients" + get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients" assert_response :success assert_template "oauth_clients/index" assert_in_body { assert_select "div>a", "My New App" } diff --git a/test/integration/user_roles_test.rb b/test/integration/user_roles_test.rb index 3f6c24248..000ea0316 100644 --- a/test/integration/user_roles_test.rb +++ b/test/integration/user_roles_test.rb @@ -32,7 +32,7 @@ class UserRolesTest < ActionDispatch::IntegrationTest assert_response :success target_user = create(:user) - post "/user/#{URI.encode(target_user.display_name)}/role/#{role}/#{action}" + post "/user/#{ERB::Util.u(target_user.display_name)}/role/#{role}/#{action}" assert_redirected_to :controller => "user", :action => "view", :display_name => target_user.display_name reset! @@ -50,7 +50,7 @@ class UserRolesTest < ActionDispatch::IntegrationTest assert_response :success target_user = create(:user) - post "/user/#{URI.encode(target_user.display_name)}/role/#{role}/#{action}" + post "/user/#{ERB::Util.u(target_user.display_name)}/role/#{role}/#{action}" assert_redirected_to :controller => "user", :action => "view", :display_name => target_user.display_name reset! diff --git a/test/lib/bounding_box_test.rb b/test/lib/bounding_box_test.rb index 53095307d..3e0c5490b 100644 --- a/test/lib/bounding_box_test.rb +++ b/test/lib/bounding_box_test.rb @@ -306,7 +306,7 @@ class BoundingBoxTest < ActiveSupport::TestCase def check_expand(bbox, array_string, margin = 0, result = nil) array = array_string.split(",").collect(&:to_f) - result = array unless result + result ||= array bbox.expand!(BoundingBox.new(array[0], array[1], array[2], array[3]), margin) check_bbox(bbox, result) end diff --git a/test/lib/i18n_test.rb b/test/lib/i18n_test.rb index 3ead036eb..689f65097 100644 --- a/test/lib/i18n_test.rb +++ b/test/lib/i18n_test.rb @@ -13,7 +13,7 @@ class I18nTest < ActiveSupport::TestCase if default_value.is_a?(Hash) variables.push("count") - default_value.each do |_subkey, subvalue| + default_value.each_value do |subvalue| subvalue.scan(/%\{(\w+)\}/) do variables.push(Regexp.last_match(1)) end diff --git a/test/models/language_test.rb b/test/models/language_test.rb index f6a6e3234..5ba558907 100644 --- a/test/models/language_test.rb +++ b/test/models/language_test.rb @@ -1,5 +1,3 @@ -# coding: utf-8 - require "test_helper" class LanguageTest < ActiveSupport::TestCase -- 2.43.2