From f28e236bdd79d8e2b7e9c1186d92a1f7167825f6 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 13 Mar 2019 10:28:50 +0100 Subject: [PATCH 1/4] Upgrade minimum version of ruby to 2.5 This prepares the way for gems that depend on newer ruby, and also prepares for the rails 6 upgrade. --- .travis.yml | 2 +- INSTALL.md | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3b56d2b4b..dc22bb883 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: false language: ruby rvm: - - 2.3.3 + - 2.5.3 cache: bundler addons: postgresql: 9.5 diff --git a/INSTALL.md b/INSTALL.md index 8e47cb266..b1fb1eafb 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -5,7 +5,7 @@ If you want to deploy the software for your own project, then see the notes at t You can install the software directly on your machine, which is the traditional and probably best-supported approach. However, there is an alternative which may be easier: Vagrant. This installs the software into a virtual machine, which makes it easier to get a consistent development environment and may avoid installation difficulties. For Vagrant instructions, see [VAGRANT.md](VAGRANT.md). -These instructions are based on Ubuntu 16.04 LTS, which is the platform used by the OSMF servers. +These instructions are based on Ubuntu 18.04 LTS, which is the platform used by the OSMF servers. The instructions also work, with only minor amendments, for all other current Ubuntu releases, Fedora and MacOSX We don't recommend attempting to develop or deploy this software on Windows. If you need to use Windows, then try developing this software using Ubuntu in a virtual machine, or use [Vagrant](VAGRANT.md). @@ -18,8 +18,7 @@ of packages required before you can get the various gems installed. ## Minimum requirements -* Ruby 2.3 -* RubyGems 1.3.1+ +* Ruby 2.5+ * PostgreSQL 9.1+ * ImageMagick * Bundler @@ -28,12 +27,12 @@ of packages required before you can get the various gems installed. These can be installed on Ubuntu 16.04 or later with: ``` -sudo apt-get install ruby2.3 libruby2.3 ruby2.3-dev \ +sudo apt-get install ruby2.5 libruby2.5 ruby2.5-dev \ libmagickwand-dev libxml2-dev libxslt1-dev nodejs \ apache2 apache2-dev build-essential git-core \ postgresql postgresql-contrib libpq-dev postgresql-server-dev-all \ libsasl2-dev imagemagick libffi-dev -sudo gem2.3 install bundler +sudo gem2.5 install bundler ``` ### Alternative platforms From 1ca77d6ddac1910e85eb7ef4dedf6f1e885307e1 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 13 Mar 2019 10:33:33 +0100 Subject: [PATCH 2/4] Rubocop fixes for ruby 2.5 --- .rubocop.yml | 2 +- app/controllers/geocoder_controller.rb | 6 ++-- app/helpers/browse_tags_helper.rb | 2 +- app/models/language.rb | 14 ++++----- bin/yarn | 12 ++++---- lib/password_hash.rb | 4 +-- .../api/changesets_controller_test.rb | 30 ++++++++----------- test/models/message_test.rb | 24 +++++++-------- 8 files changed, 41 insertions(+), 53 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index b33f9046c..690d984f7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,7 @@ inherit_from: .rubocop_todo.yml AllCops: - TargetRubyVersion: 2.3 + TargetRubyVersion: 2.5 Rails: Enabled: true diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index b9cf8d096..1270a3274 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -17,11 +17,11 @@ class GeocoderController < ApplicationController @sources.push "osm_nominatim_reverse" @sources.push "geonames_reverse" if defined?(GEONAMES_USERNAME) elsif @params[:query] - if @params[:query] =~ /^\d{5}(-\d{4})?$/ + if /^\d{5}(-\d{4})?$/.match?(@params[:query]) @sources.push "osm_nominatim" - elsif @params[:query] =~ /^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i + elsif /^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i.match?(@params[:query]) @sources.push "osm_nominatim" - elsif @params[:query] =~ /^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i + elsif /^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i.match?(@params[:query]) @sources.push "ca_postcode" @sources.push "osm_nominatim" else diff --git a/app/helpers/browse_tags_helper.rb b/app/helpers/browse_tags_helper.rb index 7c43511c4..e25df03ed 100644 --- a/app/helpers/browse_tags_helper.rb +++ b/app/helpers/browse_tags_helper.rb @@ -120,7 +120,7 @@ module BrowseTagsHelper # # Also accepting / as a visual separator although not given in RFC 3966, # because it is used as a visual separator in OSM data in some countries. - if value =~ %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*(;\s*\+[\d\s\(\)/\.-]{6,25}\s*)*$} + if %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*(;\s*\+[\d\s\(\)/\.-]{6,25}\s*)*$}.match?(value) return value.split(";").map do |phone_number| # for display, remove leading and trailing whitespace phone_number = phone_number.strip diff --git a/app/models/language.rb b/app/models/language.rb index 67e98569b..bb1aa4bd2 100644 --- a/app/models/language.rb +++ b/app/models/language.rb @@ -15,14 +15,12 @@ class Language < ActiveRecord::Base def self.load(file) Language.transaction do YAML.safe_load(File.read(file)).each do |k, v| - begin - Language.update(k, :english_name => v["english"], :native_name => v["native"]) - rescue ActiveRecord::RecordNotFound - Language.create do |l| - l.code = k - l.english_name = v["english"] - l.native_name = v["native"] - end + Language.update(k, :english_name => v["english"], :native_name => v["native"]) + rescue ActiveRecord::RecordNotFound + Language.create do |l| + l.code = k + l.english_name = v["english"] + l.native_name = v["native"] end end end diff --git a/bin/yarn b/bin/yarn index 38f278aa0..99e5e7728 100755 --- a/bin/yarn +++ b/bin/yarn @@ -1,11 +1,9 @@ #!/usr/bin/env ruby APP_ROOT = File.expand_path("..", __dir__) Dir.chdir(APP_ROOT) do - begin - exec "yarnpkg", *ARGV - rescue Errno::ENOENT - warn "Yarn executable was not detected in the system." - warn "Download Yarn at https://yarnpkg.com/en/docs/install" - exit 1 - end + exec "yarnpkg", *ARGV +rescue Errno::ENOENT + warn "Yarn executable was not detected in the system." + warn "Download Yarn at https://yarnpkg.com/en/docs/install" + exit 1 end diff --git a/lib/password_hash.rb b/lib/password_hash.rb index c65df2c4f..5bafe6dda 100644 --- a/lib/password_hash.rb +++ b/lib/password_hash.rb @@ -18,7 +18,7 @@ module PasswordHash def self.check(hash, salt, candidate) if salt.nil? candidate = Digest::MD5.hexdigest(candidate) - elsif salt =~ /!/ + elsif /!/.match?(salt) algorithm, iterations, salt = salt.split("!") size = Base64.strict_decode64(hash).length candidate = self.hash(candidate, salt, iterations.to_i, size, algorithm) @@ -32,7 +32,7 @@ module PasswordHash def self.upgrade?(hash, salt) if salt.nil? return true - elsif salt =~ /!/ + elsif /!/.match?(salt) algorithm, iterations, salt = salt.split("!") return true if Base64.strict_decode64(salt).length != SALT_BYTE_SIZE return true if Base64.strict_decode64(hash).length != HASH_BYTE_SIZE diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 75896b202..daf062154 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -165,12 +165,10 @@ module Api # check that a changeset that doesn't exist returns an appropriate message def test_show_not_found [0, -32, 233455644, "afg", "213"].each do |id| - begin - get :show, :params => { :id => id } - assert_response :not_found, "should get a not found" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + get :show, :params => { :id => id } + assert_response :not_found, "should get a not found" + rescue ActionController::UrlGenerationError => ex + assert_match(/No route matches/, ex.to_s) end end @@ -239,23 +237,19 @@ module Api # First try to do it with no auth cs_ids.each do |id| - begin - put :close, :params => { :id => id } - assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + put :close, :params => { :id => id } + assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" + rescue ActionController::UrlGenerationError => ex + assert_match(/No route matches/, ex.to_s) end # Now try with auth basic_authorization create(:user).email, "test" cs_ids.each do |id| - begin - put :close, :params => { :id => id } - assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + put :close, :params => { :id => id } + assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" + rescue ActionController::UrlGenerationError => ex + assert_match(/No route matches/, ex.to_s) end end diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 18b75f451..9c321d3c3 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -53,20 +53,18 @@ class MessageTest < ActiveSupport::TestCase "\x82\x82", # multibyte continuations without multibyte identifier "\xe1\x82\x4a"] # three-byte identifier, contination and (incorrectly) plain ASCII invalid_sequences.each do |char| - begin - # create a message and save to the database - msg = make_message(char, 1) - # if the save throws, thats fine and the test should pass, as we're - # only testing invalid sequences anyway. - msg.save! + # create a message and save to the database + msg = make_message(char, 1) + # if the save throws, thats fine and the test should pass, as we're + # only testing invalid sequences anyway. + msg.save! - # get the saved message back and check that it is identical - i.e: - # its OK to accept invalid UTF-8 as long as we return it unmodified. - db_msg = msg.class.find(msg.id) - assert_equal char, db_msg.title, "Database silently truncated message title" - rescue ArgumentError => ex - assert_equal ex.to_s, "invalid byte sequence in UTF-8" - end + # get the saved message back and check that it is identical - i.e: + # its OK to accept invalid UTF-8 as long as we return it unmodified. + db_msg = msg.class.find(msg.id) + assert_equal char, db_msg.title, "Database silently truncated message title" + rescue ArgumentError => ex + assert_equal ex.to_s, "invalid byte sequence in UTF-8" end end From f4c761a0ed2134fcaf59514a7e1637cf8f6dc05b Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 26 Mar 2019 19:08:36 +0000 Subject: [PATCH 3/4] Target ruby 2.5.1 to match Ubuntu --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index dc22bb883..9438622cd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: false language: ruby rvm: - - 2.5.3 + - 2.5.1 cache: bundler addons: postgresql: 9.5 From d6af4450d1ea3599cdce6921138155f73e154816 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 26 Mar 2019 19:12:18 +0000 Subject: [PATCH 4/4] Prefer String#match? over butt ugly Regexp#match? --- app/controllers/geocoder_controller.rb | 6 +++--- app/helpers/browse_tags_helper.rb | 2 +- lib/password_hash.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index 1270a3274..d9b56147f 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -17,11 +17,11 @@ class GeocoderController < ApplicationController @sources.push "osm_nominatim_reverse" @sources.push "geonames_reverse" if defined?(GEONAMES_USERNAME) elsif @params[:query] - if /^\d{5}(-\d{4})?$/.match?(@params[:query]) + if @params[:query].match?(/^\d{5}(-\d{4})?$/) @sources.push "osm_nominatim" - elsif /^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i.match?(@params[:query]) + elsif @params[:query].match?(/^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i) @sources.push "osm_nominatim" - elsif /^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i.match?(@params[:query]) + elsif @params[:query].match?(/^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i) @sources.push "ca_postcode" @sources.push "osm_nominatim" else diff --git a/app/helpers/browse_tags_helper.rb b/app/helpers/browse_tags_helper.rb index e25df03ed..90e75e90a 100644 --- a/app/helpers/browse_tags_helper.rb +++ b/app/helpers/browse_tags_helper.rb @@ -120,7 +120,7 @@ module BrowseTagsHelper # # Also accepting / as a visual separator although not given in RFC 3966, # because it is used as a visual separator in OSM data in some countries. - if %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*(;\s*\+[\d\s\(\)/\.-]{6,25}\s*)*$}.match?(value) + if value.match?(%r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*(;\s*\+[\d\s\(\)/\.-]{6,25}\s*)*$}) return value.split(";").map do |phone_number| # for display, remove leading and trailing whitespace phone_number = phone_number.strip diff --git a/lib/password_hash.rb b/lib/password_hash.rb index 5bafe6dda..9f77fdc0d 100644 --- a/lib/password_hash.rb +++ b/lib/password_hash.rb @@ -18,7 +18,7 @@ module PasswordHash def self.check(hash, salt, candidate) if salt.nil? candidate = Digest::MD5.hexdigest(candidate) - elsif /!/.match?(salt) + elsif salt.match?(/!/) algorithm, iterations, salt = salt.split("!") size = Base64.strict_decode64(hash).length candidate = self.hash(candidate, salt, iterations.to_i, size, algorithm) @@ -32,7 +32,7 @@ module PasswordHash def self.upgrade?(hash, salt) if salt.nil? return true - elsif /!/.match?(salt) + elsif salt.match?(/!/) algorithm, iterations, salt = salt.split("!") return true if Base64.strict_decode64(salt).length != SALT_BYTE_SIZE return true if Base64.strict_decode64(hash).length != HASH_BYTE_SIZE