From: Tom Hughes Date: Mon, 22 Jan 2018 14:36:16 +0000 (+0000) Subject: Fix new rubocop warnings X-Git-Tag: live~3195 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/a83030dab7512c4b2848e777f7a7dbff456774b3?hp=-c;ds=sidebyside Fix new rubocop warnings --- a83030dab7512c4b2848e777f7a7dbff456774b3 diff --git a/.rubocop.yml b/.rubocop.yml index 55be8141c..5e7be9797 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -41,12 +41,18 @@ Naming/FileName: Rails/ApplicationRecord: Enabled: false +Rails/CreateTableWithTimestamps: + Enabled: false + Rails/HasManyOrHasOneDependent: Enabled: false Rails/HttpPositionalArguments: Enabled: false +Rails/InverseOf: + Enabled: false + Rails/SkipsModelValidations: Exclude: - 'db/migrate/*.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b4104079c..3ebd4d35f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -55,11 +55,6 @@ 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: @@ -227,3 +222,8 @@ Style/NumericLiterals: # SupportedStyles: compact, exploded Style/RaiseArgs: Enabled: false + +# Offense count: 2 +Style/RescueStandardError: + Exclude: + - 'app/helpers/browse_helper.rb' diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 4c24a1cec..9f909ea10 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -508,14 +508,10 @@ class AmfController < ApplicationController rels = [] if searchterm.to_i > 0 rel = Relation.where(:id => searchterm.to_i).first - if rel && rel.visible - rels.push([rel.id, rel.tags, rel.members, rel.version]) - end + rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel && rel.visible else RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t| - if t.relation.visible - rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) - end + rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible end end rels @@ -558,9 +554,7 @@ class AmfController < ApplicationController mid = renumberednodes[mid] if m[0] == "Node" mid = renumberedways[mid] if m[0] == "Way" end - if mid - typedmembers << [m[0], mid, m[2].delete("\000-\037\ufffe\uffff", "^\011\012\015")] - end + typedmembers << [m[0], mid, m[2].delete("\000-\037\ufffe\uffff", "^\011\012\015")] if mid end # assign new contents @@ -748,9 +742,7 @@ class AmfController < ApplicationController return [-4, "node", id] end - unless visible || node.ways.empty? - return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version - end + return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version unless visible || node.ways.empty? end # We always need a new node, based on the data that has been sent to us new_node = Node.new @@ -793,9 +785,7 @@ class AmfController < ApplicationController n = Node.where(:id => id).first if n v = n.version - unless timestamp == "" - n = OldNode.where("node_id = ? AND timestamp <= ?", id, timestamp).unredacted.order("timestamp DESC").first - end + n = OldNode.where("node_id = ? AND timestamp <= ?", id, timestamp).unredacted.order("timestamp DESC").first unless timestamp == "" end if n diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index c6cc3ba5f..f9b48cb1c 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -157,9 +157,7 @@ class ApiController < ApplicationController # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0] - unless nodes_to_fetch.empty? - nodes += Node.includes(:node_tags).find(nodes_to_fetch) - end + nodes += Node.includes(:node_tags).find(nodes_to_fetch) unless nodes_to_fetch.empty? visible_nodes = {} changeset_cache = {} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a51dad875..a24df48e0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -29,9 +29,7 @@ class ApplicationController < ActionController::Base end end elsif session[:token] - if self.current_user = User.authenticate(:token => session[:token]) - session[:user] = current_user.id - end + session[:user] = current_user.id if self.current_user = User.authenticate(:token => session[:token]) end rescue StandardError => ex logger.info("Exception authorizing user: #{ex}") @@ -381,9 +379,7 @@ class ApplicationController < ActionController::Base ## # ensure that there is a "this_user" instance variable def lookup_this_user - unless @this_user = User.active.find_by(:display_name => params[:display_name]) - render_unknown_user params[:display_name] - end + render_unknown_user params[:display_name] unless @this_user = User.active.find_by(:display_name => params[:display_name]) end ## @@ -469,9 +465,7 @@ class ApplicationController < ActionController::Base authdata = request.env["HTTP_AUTHORIZATION"].to_s.split end # only basic authentication supported - if authdata && authdata[0] == "Basic" - user, pass = Base64.decode64(authdata[1]).split(":", 2) - end + user, pass = Base64.decode64(authdata[1]).split(":", 2) if authdata && authdata[0] == "Basic" [user, pass] end diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 8fbbe1362..f294d23d5 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -7,12 +7,12 @@ class ChangesetController < ApplicationController skip_before_action :verify_authenticity_token, :except => [:list] before_action :authorize_web, :only => [:list, :feed, :comments_feed] before_action :set_locale, :only => [:list, :feed, :comments_feed] - before_action :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] + before_action :authorize, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] before_action :require_moderator, :only => [:hide_comment, :unhide_comment] - before_action :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] - before_action :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe] - before_action :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] - before_action :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed] + before_action :require_allow_write_api, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] + before_action :require_public_data, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe] + before_action :check_api_writable, :only => [:create, :update, :upload, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] + before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed] before_action(:only => [:list, :feed, :comments_feed]) { |c| c.check_database_readable(true) } around_action :api_call_handle_error, :except => [:list, :feed, :comments_feed] around_action :api_call_timeout, :except => [:list, :feed, :comments_feed, :upload] @@ -296,9 +296,7 @@ class ChangesetController < ApplicationController changesets = changesets.where(:user_id => current_user.nearby) end - if @params[:max_id] - changesets = changesets.where("changesets.id <= ?", @params[:max_id]) - end + changesets = changesets.where("changesets.id <= ?", @params[:max_id]) if @params[:max_id] @edits = changesets.order("changesets.id DESC").limit(20).preload(:user, :changeset_tags, :comments) @@ -334,9 +332,7 @@ class ChangesetController < ApplicationController # Notify current subscribers of the new comment changeset.subscribers.visible.each do |user| - if current_user != user - Notifier.changeset_comment_notification(comment, user).deliver_now - end + Notifier.changeset_comment_notification(comment, user).deliver_now if current_user != user end # Add the commenter to the subscribers if necessary diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index 9e0fd4991..6eb662bd8 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -65,9 +65,7 @@ class DiaryEntryController < ApplicationController # Notify current subscribers of the new comment @entry.subscribers.visible.each do |user| - if current_user != user - Notifier.diary_comment_notification(@diary_comment, user).deliver_now - end + Notifier.diary_comment_notification(@diary_comment, user).deliver_now if current_user != user end # Add the commenter to the subscribers if necessary diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index b9bde31ab..e9fa7f26a 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -112,9 +112,7 @@ class GeocoderController < ApplicationController maxlat = params[:maxlat] # get view box - if minlon && minlat && maxlon && maxlat - viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}" - end + viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}" if minlon && minlat && maxlon && maxlat # get objects to excude exclude = "&exclude_place_ids=#{params[:exclude]}" if params[:exclude] @@ -153,9 +151,7 @@ class GeocoderController < ApplicationController rank = (place.attributes["place_rank"].to_i + 1) / 2 prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name place.elements["extratags"].elements.each("tag") do |extratag| - if extratag.attributes["key"] == "place" - prefix_name = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name - end + prefix_name = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name if extratag.attributes["key"] == "place" end end prefix = t "geocoder.search_osm_nominatim.prefix_format", :name => prefix_name diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 29651bceb..20baf6bb4 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -40,9 +40,7 @@ class NodeController < ApplicationController node = Node.find(params[:id]) new_node = Node.from_xml(request.raw_post) - unless new_node && new_node.id == 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 + raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id node.update_from(new_node, current_user) render :plain => node.version.to_s @@ -55,24 +53,18 @@ class NodeController < ApplicationController node = Node.find(params[:id]) new_node = Node.from_xml(request.raw_post) - unless new_node && new_node.id == 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 + raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id node.delete_with_history!(new_node, current_user) render :plain => node.version.to_s end # Dump the details on many nodes whose ids are given in the "nodes" parameter. def nodes - unless params["nodes"] - raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" - end + raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"] ids = params["nodes"].split(",").collect(&:to_i) - if ids.empty? - raise OSM::APIBadUserInput, "No nodes were given to search for" - end + raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty? doc = OSM::API.new.get_xml_doc Node.find(ids).each do |node| diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 92f63e304..a51d70f90 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -350,9 +350,7 @@ class NotesController < ApplicationController comment = note.comments.create!(attributes) note.comments.map(&:author).uniq.each do |user| - if notify && user && user != current_user && user.visible? - Notifier.note_comment_notification(comment, user).deliver_now - end + Notifier.note_comment_notification(comment, user).deliver_now if notify && user && user != current_user && user.visible? end end end diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb index 734783a35..84bbcf185 100644 --- a/app/controllers/oauth_controller.rb +++ b/app/controllers/oauth_controller.rb @@ -63,9 +63,7 @@ class OauthController < ApplicationController "&oauth_token=#{@token.token}" end - unless @token.oauth10? - @redirect_url.query += "&oauth_verifier=#{@token.verifier}" - end + @redirect_url.query += "&oauth_verifier=#{@token.verifier}" unless @token.oauth10? redirect_to @redirect_url.to_s end diff --git a/app/controllers/relation_controller.rb b/app/controllers/relation_controller.rb index 25532a95c..059fb8d7e 100644 --- a/app/controllers/relation_controller.rb +++ b/app/controllers/relation_controller.rb @@ -35,9 +35,7 @@ class RelationController < ApplicationController relation = Relation.find(params[:id]) new_relation = Relation.from_xml(request.raw_post) - unless new_relation && new_relation.id == 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 + raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})" unless new_relation && new_relation.id == relation.id relation.update_from new_relation, current_user render :plain => relation.version.to_s @@ -123,15 +121,11 @@ class RelationController < ApplicationController end def relations - unless params["relations"] - raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" - end + raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"] ids = params["relations"].split(",").collect(&:to_i) - if ids.empty? - raise OSM::APIBadUserInput, "No relations were given to search for" - end + raise OSM::APIBadUserInput, "No relations were given to search for" if ids.empty? doc = OSM::API.new.get_xml_doc diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index 74dbc41f5..fa8ec5a1e 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -11,9 +11,7 @@ class SiteController < ApplicationController before_action :update_totp, :only => [:index] def index - unless STATUS == :database_readonly || STATUS == :database_offline - session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"]) - end + session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"]) unless STATUS == :database_readonly || STATUS == :database_offline end def permalink @@ -147,9 +145,7 @@ class SiteController < ApplicationController def redirect_map_params anchor = [] - if params[:lat] && params[:lon] - anchor << "map=#{params.delete(:zoom) || 5}/#{params.delete(:lat)}/#{params.delete(:lon)}" - end + anchor << "map=#{params.delete(:zoom) || 5}/#{params.delete(:lat)}/#{params.delete(:lon)}" if params[:lat] && params[:lon] if params[:layers] anchor << "layers=#{params.delete(:layers)}" @@ -157,8 +153,6 @@ class SiteController < ApplicationController anchor << "layers=N" end - if anchor.present? - redirect_to params.to_unsafe_h.merge(:anchor => anchor.join("&")) - end + redirect_to params.to_unsafe_h.merge(:anchor => anchor.join("&")) if anchor.present? end end diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index 105405ccf..a720c5fff 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -119,9 +119,7 @@ class TraceController < ApplicationController if @trace.id flash[:notice] = t "trace.create.trace_uploaded" - if current_user.traces.where(:inserted => false).count > 4 - flash[:warning] = t "trace.trace_header.traces_waiting", :count => current_user.traces.where(:inserted => false).count - end + flash[:warning] = t "trace.trace_header.traces_waiting", :count => current_user.traces.where(:inserted => false).count if current_user.traces.where(:inserted => false).count > 4 redirect_to :action => :list, :display_name => current_user.display_name end @@ -176,9 +174,7 @@ class TraceController < ApplicationController @trace.description = params[:trace][:description] @trace.tagstring = params[:trace][:tagstring] @trace.visibility = params[:trace][:visibility] - if @trace.save - redirect_to :action => "view", :display_name => current_user.display_name - end + redirect_to :action => "view", :display_name => current_user.display_name if @trace.save end end rescue ActiveRecord::RecordNotFound @@ -205,9 +201,7 @@ class TraceController < ApplicationController def georss @traces = Trace.visible_to_all.visible - if params[:display_name] - @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] }) - end + @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] }) if params[:display_name] @traces = @traces.tagged(params[:tag]) if params[:tag] @traces = @traces.order("timestamp DESC") diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 5c41a79dc..0c3ad0b05 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -45,9 +45,7 @@ class UserController < ApplicationController if current_user current_user.terms_seen = true - if current_user.save - flash[:notice] = t("user.new.terms declined", :url => t("user.new.terms declined url")).html_safe - end + flash[:notice] = t("user.new.terms declined", :url => t("user.new.terms declined url")).html_safe if current_user.save if params[:referer] redirect_to params[:referer] @@ -533,9 +531,7 @@ class UserController < ApplicationController session[:new_user].auth_provider = provider session[:new_user].auth_uid = uid - if email_verified && email == session[:new_user].email - session[:new_user].status = "active" - end + session[:new_user].status = "active" if email_verified && email == session[:new_user].email redirect_to :action => "terms" else diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a330eb5f9..adcf5c6c0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -40,9 +40,7 @@ module ApplicationHelper end def if_user(user, tag = :div, &block) - if user - content_tag(tag, capture(&block), :class => "hidden show_if_user_#{user.id}") - end + content_tag(tag, capture(&block), :class => "hidden show_if_user_#{user.id}") if user end def unless_user(user, tag = :div, &block) @@ -110,9 +108,7 @@ module ApplicationHelper if current_user data[:user] = current_user.id.to_json - unless current_user.home_lon.nil? || current_user.home_lat.nil? - data[:user_home] = { :lat => current_user.home_lat, :lon => current_user.home_lon } - end + data[:user_home] = { :lat => current_user.home_lat, :lon => current_user.home_lon } unless current_user.home_lon.nil? || current_user.home_lat.nil? end data[:location] = session[:location] if session[:location] diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index c2e974e93..b90e27f85 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -8,18 +8,14 @@ module BrowseHelper object.id end name = t "printable_name.with_id", :id => id.to_s - if version - name = t "printable_name.with_version", :id => name, :version => object.version.to_s - end + name = t "printable_name.with_version", :id => name, :version => object.version.to_s if version # don't look at object tags if redacted, so as to avoid giving # away redacted version tag information. unless object.redacted? locale = I18n.locale.to_s - while locale =~ /-[^-]+/ && !object.tags.include?("name:#{I18n.locale}") - locale = locale.sub(/-[^-]+/, "") - end + locale = locale.sub(/-[^-]+/, "") while locale =~ /-[^-]+/ && !object.tags.include?("name:#{I18n.locale}") if object.tags.include? "name:#{locale}" name = t "printable_name.with_name_html", :name => content_tag(:bdi, object.tags["name:#{locale}"].to_s), :id => content_tag(:bdi, name) diff --git a/app/models/relation.rb b/app/models/relation.rb index 157794cd6..2495830ee 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -182,9 +182,7 @@ class Relation < ActiveRecord::Base end def delete_with_history!(new_relation, user) - unless visible - raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id) - end + raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id) unless visible # need to start the transaction here, so that the database can # provide repeatable reads for the used-by checks. this means it @@ -208,9 +206,7 @@ class Relation < ActiveRecord::Base Relation.transaction do lock! check_consistency(self, new_relation, user) - unless new_relation.preconditions_ok?(members) - raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." - end + raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." unless new_relation.preconditions_ok?(members) self.changeset_id = new_relation.changeset_id self.changeset = new_relation.changeset self.tags = new_relation.tags @@ -222,9 +218,7 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) - unless preconditions_ok? - raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." - end + raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." unless preconditions_ok? self.version = 0 self.visible = true save_with_history! @@ -259,9 +253,7 @@ class Relation < ActiveRecord::Base element = model.lock("for share").find_by(:id => m[1]) # and check that it is OK to use. - unless element && element.visible? && element.preconditions_ok? - raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" - end + raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element && element.visible? && element.preconditions_ok? hash[m[1]] = true end diff --git a/app/models/user.rb b/app/models/user.rb index 7a8414ec0..9eeb98290 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -277,9 +277,7 @@ class User < ActiveRecord::Base ## # perform a spam check on a user def spam_check - if status == "active" && spam_score > SPAM_THRESHOLD - update(:status => "suspended") - end + update(:status => "suspended") if status == "active" && spam_score > SPAM_THRESHOLD end ## diff --git a/app/models/way.rb b/app/models/way.rb index 1954f744c..e5b73ceaa 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -120,14 +120,10 @@ class Way < ActiveRecord::Base way_nodes.each do |nd| if visible_nodes # if there is a list of visible nodes then use that to weed out deleted nodes - if visible_nodes[nd.node_id] - ordered_nodes[nd.sequence_id] = nd.node_id.to_s - end + ordered_nodes[nd.sequence_id] = nd.node_id.to_s if visible_nodes[nd.node_id] else # otherwise, manually go to the db to check things - if nd.node && nd.node.visible? - ordered_nodes[nd.sequence_id] = nd.node_id.to_s - end + ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node && nd.node.visible? end end @@ -184,9 +180,7 @@ class Way < ActiveRecord::Base Way.transaction do lock! check_consistency(self, new_way, user) - unless new_way.preconditions_ok?(nds) - raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid." - end + raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid." unless new_way.preconditions_ok?(nds) self.changeset_id = new_way.changeset_id self.changeset = new_way.changeset @@ -199,9 +193,7 @@ class Way < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) - unless preconditions_ok? - raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." - end + raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." unless preconditions_ok? self.version = 0 self.visible = true save_with_history! @@ -209,9 +201,7 @@ class Way < ActiveRecord::Base def preconditions_ok?(old_nodes = []) return false if nds.empty? - if nds.length > MAX_NUMBER_OF_WAY_NODES - raise OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES) - end + raise OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES) if nds.length > MAX_NUMBER_OF_WAY_NODES # check only the new nodes, for efficiency - old nodes having been checked last time and can't # be deleted when they're in-use. diff --git a/app/views/user/api_read.builder b/app/views/user/api_read.builder index 0e8bfa5ef..fe5af4bcf 100644 --- a/app/views/user/api_read.builder +++ b/app/views/user/api_read.builder @@ -10,9 +10,7 @@ xml.osm("version" => API_VERSION, "generator" => GENERATOR) do else xml.tag! "contributor-terms", :agreed => @this_user.terms_agreed.present? end - if @this_user.image.file? || @this_user.image_use_gravatar - xml.tag! "img", :href => user_image_url(@this_user, :size => 256) - end + xml.tag! "img", :href => user_image_url(@this_user, :size => 256) if @this_user.image.file? || @this_user.image_use_gravatar xml.tag! "roles" do @this_user.roles.each do |role| xml.tag! role.role diff --git a/config/application.rb b/config/application.rb index 02dd1d2b2..9ba122daf 100644 --- a/config/application.rb +++ b/config/application.rb @@ -34,9 +34,7 @@ module OpenStreetMap config.paths["app/models"].skip_eager_load! if STATUS == :database_offline # Use memcached for caching if required - if defined?(MEMCACHE_SERVERS) - config.cache_store = :mem_cache_store, MEMCACHE_SERVERS, { :namespace => "rails:cache" } - end + config.cache_store = :mem_cache_store, MEMCACHE_SERVERS, { :namespace => "rails:cache" } if defined?(MEMCACHE_SERVERS) # Use logstash for logging if required if defined?(LOGSTASH_PATH) diff --git a/config/environments/development.rb b/config/environments/development.rb index 8e7213a9d..97226480c 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -35,9 +35,7 @@ Rails.application.configure do config.active_support.deprecation = :log # Raise an error on page load if there are pending migrations. - unless STATUS == :database_offline - config.active_record.migration_error = :page_load - end + config.active_record.migration_error = :page_load unless STATUS == :database_offline # Debug mode disables concatenation and preprocessing of assets. # This option may cause significant delays in view rendering with a large diff --git a/config/environments/production.rb b/config/environments/production.rb index c4c98ad48..56bd22881 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -93,9 +93,7 @@ Rails.application.configure do end # Do not dump schema after migrations. - unless STATUS == :database_offline - config.active_record.dump_schema_after_migration = false - end + config.active_record.dump_schema_after_migration = false unless STATUS == :database_offline # Enable autoloading of dependencies. config.enable_dependency_loading = true diff --git a/config/initializers/cors.rb b/config/initializers/cors.rb index ee0b0d98f..2bd558d2f 100644 --- a/config/initializers/cors.rb +++ b/config/initializers/cors.rb @@ -7,9 +7,7 @@ module OpenStreetMap class Cors < Rack::Cors def call(env) status, headers, body = super env - if headers["Access-Control-Allow-Origin"] - headers["Cache-Control"] = "no-cache" - end + headers["Cache-Control"] = "no-cache" if headers["Access-Control-Allow-Origin"] [status, headers, body] end end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 180469bfc..7e499c0e6 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -26,9 +26,7 @@ windowslive_options = { :name => "windowslive", :scope => "wl.signin,wl.emails" github_options = { :name => "github", :scope => "user:email" } wikipedia_options = { :name => "wikipedia", :client_options => { :site => "https://meta.wikimedia.org" } } -if defined?(GOOGLE_OPENID_REALM) - google_options[:openid_realm] = GOOGLE_OPENID_REALM -end +google_options[:openid_realm] = GOOGLE_OPENID_REALM if defined?(GOOGLE_OPENID_REALM) Rails.application.config.middleware.use OmniAuth::Builder do provider :openid, openid_options diff --git a/db/migrate/021_move_to_innodb.rb b/db/migrate/021_move_to_innodb.rb index 5be81b695..3232e2741 100644 --- a/db/migrate/021_move_to_innodb.rb +++ b/db/migrate/021_move_to_innodb.rb @@ -19,9 +19,7 @@ class MoveToInnodb < ActiveRecord::Migration[5.0] # current version to something less so that we can update the version in # batches of 10000 tbl.classify.constantize.update_all(:version => -1) - while tbl.classify.constantize.where(:version => -1).count > 0 - tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) - end + tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) while tbl.classify.constantize.where(:version => -1).count > 0 # execute "UPDATE current_#{tbl} SET version = " + # "(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)" # The above update causes a MySQL error: diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index f12683d3c..1fc5dc696 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -18,23 +18,17 @@ class BoundingBox end def self.from_bbox_params(params) - if params[:bbox] && params[:bbox].count(",") == 3 - bbox_array = params[:bbox].split(",") - end + bbox_array = params[:bbox].split(",") if params[:bbox] && params[:bbox].count(",") == 3 from_bbox_array(bbox_array) end def self.from_lon_lat_params(params) - if params[:minlon] && params[:minlat] && params[:maxlon] && params[:maxlat] - bbox_array = [params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]] - end + bbox_array = [params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]] if params[:minlon] && params[:minlat] && params[:maxlon] && params[:maxlat] from_bbox_array(bbox_array) end def self.from_lrbt_params(params) - if params[:l] && params[:b] && params[:t] && params[:t] - bbox_array = [params[:l], params[:b], params[:r], params[:t]] - end + bbox_array = [params[:l], params[:b], params[:r], params[:t]] if params[:l] && params[:b] && params[:t] && params[:t] from_bbox_array(bbox_array) end @@ -65,12 +59,8 @@ class BoundingBox def check_boundaries # check the bbox is sane - if min_lon > max_lon - 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, "The minimum latitude must be less than the maximum latitude, but it wasn't" - end + raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't" if min_lon > max_lon + raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't" if min_lat > max_lat if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT raise OSM::APIBadBoundingBox, "The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," \ " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}" @@ -166,9 +156,7 @@ class BoundingBox private def from_bbox_array(bbox_array) - unless bbox_array - raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" - end + raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and # max_lat within their respective boundaries. min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min diff --git a/lib/diff_reader.rb b/lib/diff_reader.rb index 94c41a6d5..cb553bfbc 100644 --- a/lib/diff_reader.rb +++ b/lib/diff_reader.rb @@ -60,9 +60,7 @@ class DiffReader attributes = {} if @reader.has_attributes? - while @reader.move_to_next_attribute == 1 - attributes[@reader.name] = @reader.value - end + attributes[@reader.name] = @reader.value while @reader.move_to_next_attribute == 1 @reader.move_to_element end @@ -112,9 +110,7 @@ class DiffReader # such as save_ and delete_with_history. def check(model, xml, new) raise OSM::APIBadXMLError.new(model, xml) if new.nil? - unless new.changeset_id == @changeset.id - raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id) - end + raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id) unless new.changeset_id == @changeset.id end ## diff --git a/lib/session_persistence.rb b/lib/session_persistence.rb index 5e933f48a..6bd05f5ce 100644 --- a/lib/session_persistence.rb +++ b/lib/session_persistence.rb @@ -52,9 +52,7 @@ module SessionPersistence # Filter callback def persist_session - if session[session_persistence_key] - request.session_options[:expire_after] = session[session_persistence_key] - end + request.session_options[:expire_after] = session[session_persistence_key] if session[session_persistence_key] rescue StandardError reset_session end diff --git a/test/controllers/amf_controller_test.rb b/test/controllers/amf_controller_test.rb index 83ef03bbf..bd7a51884 100644 --- a/test/controllers/amf_controller_test.rb +++ b/test/controllers/amf_controller_test.rb @@ -1,8 +1,9 @@ require "test_helper" require "stringio" -include Potlatch class AmfControllerTest < ActionController::TestCase + include Potlatch + ## # test all routes which lead to this controller def test_routes @@ -730,8 +731,8 @@ class AmfControllerTest < ActionController::TestCase # This node has no tags # create a node with random lat/lon - lat = rand(100) - 50 + rand - lon = rand(100) - 50 + rand + lat = rand(-50..49) + rand + lon = rand(-50..49) + rand changeset = create(:changeset) user = changeset.user @@ -770,8 +771,8 @@ class AmfControllerTest < ActionController::TestCase # This node has some tags # create a node with random lat/lon - lat = rand(100) - 50 + rand - lon = rand(100) - 50 + rand + lat = rand(-50..49) + rand + lon = rand(-50..49) + rand amf_content "putpoi", "/2", ["#{user.email}:test", changeset.id, nil, nil, lon, lat, { "key" => "value", "ping" => "pong" }, nil] post :amf_write @@ -811,8 +812,8 @@ class AmfControllerTest < ActionController::TestCase # This node has no tags # create a node with random lat/lon - lat = rand(100) - 50 + rand - lon = rand(100) - 50 + rand + lat = rand(-50..49) + rand + lon = rand(-50..49) + rand changeset = create(:changeset) user = changeset.user @@ -847,8 +848,8 @@ class AmfControllerTest < ActionController::TestCase # This node has no tags # create a node with random lat/lon - lat = rand(100) - 50 + rand - lon = rand(100) - 50 + rand + lat = rand(-50..49) + rand + lon = rand(-50..49) + rand changeset = create(:changeset) user = changeset.user diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index a0f7960c3..5205714df 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -812,9 +812,7 @@ CHANGESET assert_equal 2, Node.find(new_node_id).tags.size, "new node should have two tags" assert_equal [new_node_id, node.id], Way.find(way.id).nds, "way nodes should match" Relation.find(relation.id).members.each do |type, id, _role| - if type == "node" - assert_equal new_node_id, id, "relation should contain new node" - end + assert_equal new_node_id, id, "relation should contain new node" if type == "node" end end diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb index 42b63bdf6..5f737f798 100644 --- a/test/controllers/node_controller_test.rb +++ b/test/controllers/node_controller_test.rb @@ -33,8 +33,8 @@ class NodeControllerTest < ActionController::TestCase changeset = create(:changeset, :user => user) # create a node with random lat/lon - lat = rand(100) - 50 + rand - lon = rand(100) - 50 + rand + lat = rand(-50..50) + rand + lon = rand(-50..50) + rand ## First try with no auth # create a minimal xml file diff --git a/test/integration/user_blocks_test.rb b/test/integration/user_blocks_test.rb index 4048d5303..938136592 100644 --- a/test/integration/user_blocks_test.rb +++ b/test/integration/user_blocks_test.rb @@ -2,7 +2,7 @@ require "test_helper" class UserBlocksTest < ActionDispatch::IntegrationTest def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) } + { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) } end def test_api_blocked diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb index dcc7fa7e9..7ec7fa730 100644 --- a/test/integration/user_terms_seen_test.rb +++ b/test/integration/user_terms_seen_test.rb @@ -74,7 +74,7 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest private def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) } + { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) } end def with_terms_seen(value) diff --git a/test/lib/i18n_test.rb b/test/lib/i18n_test.rb index 689f65097..3aa528ceb 100644 --- a/test/lib/i18n_test.rb +++ b/test/lib/i18n_test.rb @@ -24,9 +24,7 @@ class I18nTest < ActiveSupport::TestCase end end - if key =~ /^(active(model|record)\.)?errors\./ - variables.push("attribute") - end + variables.push("attribute") if key =~ /^(active(model|record)\.)?errors\./ value = I18n.t(key, :locale => locale, :fallback => true) diff --git a/test/test_helper.rb b/test/test_helper.rb index 70f69a3ae..c9ec46dcf 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,7 +82,7 @@ module ActiveSupport ## # set request headers for HTTP basic authentication def basic_authorization(user, pass) - @request.env["HTTP_AUTHORIZATION"] = format("Basic %s", Base64.encode64("#{user}:#{pass}")) + @request.env["HTTP_AUTHORIZATION"] = format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) end ##