Upgraded to ruby 2.5

Apparently in SLE 15 (and so Tumbleweed too) the default ruby will be
2.5 instead of 2.4 as we were told. This commit adds support for this
version and sets it as the default (so we anticipate the release).

This change involved a major change: devise had to be upgraded from
3.5.0 to 4.4.0. Devise was quite old and only in 4.4.0 it received some
fixes for this ruby version. Hence, this commit also includes quite some
changes to accomodate this upgrade.

Finally, I've also fixed some minor issues (e.g. the order of the errors
controller in the routes.rb file), and I've upgraded the rubocop version
so it catches the latests things from ruby 2.5 as well.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>

.rubocop.yml

@@ -1,5 +1,5 @@
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 2.5
   TargetRailsVersion: 4.2
 
   DisplayCopNames: true

.ruby-version

@@ -1 +1 @@
-2.4.2
+2.5.0

.travis.yml

@@ -2,11 +2,7 @@ language: ruby
 sudo: required
 dist: trusty
 rvm:
-  # Stable supported version
-  - 2.4.2
-
-  # Future versions
-  - ruby-head
+  - 2.5.0
 
 matrix:
   allow_failures:
@@ -56,7 +52,11 @@ script:
   # Style and security checks
   - bundle exec rubocop -V
   - bundle exec rubocop -F
-  - bundle exec brakeman
+
+  # Note: it ignores a couple of files which use ruby 2.5 syntax which brakeman
+  # does not know how to handle...
+  - bundle exec brakeman --skip-files lib/portus/background/sync.rb,lib/portus/registry_client.rb
+
   - bundle exec rake portus:annotate_and_exit
 
   # Javascript style

Dockerfile

@@ -1,4 +1,4 @@
-FROM opensuse/ruby:2.4
+FROM opensuse/ruby:2.5
 MAINTAINER SUSE Containers Team <containers@suse.com>
 
 ENV COMPOSE=1
@@ -15,7 +15,7 @@ COPY Gemfile* ./
 #   3. We then proceed to remove unneeded clutter: first we remove some packages
 #      installed with the devel_basis pattern, and finally we zypper clean -a.
 RUN zypper ref && \
-    zypper -n in --no-recommends ruby2.4-devel ruby2.4-rubygem-bundler \
+    zypper -n in --no-recommends ruby2.5-devel \
            libxml2-devel nodejs libmysqlclient-devel postgresql-devel libxslt1 && \
     zypper -n in --no-recommends -t pattern devel_basis && \
     bundle install --retry=3 && \

Gemfile

@@ -34,7 +34,7 @@ gem "webpack-rails"
 gem "rack-cors", "~> 1.0.1"
 
 # Supported DBs
-gem "mysql2", "= 0.4.7", group: :mysql
+gem "mysql2", "= 0.4.10", group: :mysql
 gem "pg", "~> 0.20.0", group: :postgres
 
 # Pinning these specific versions because that's what we have on OBS.
@@ -98,7 +98,7 @@ group :development, :test do
   gem "grape-swagger-rails"
   gem "hirb"
   gem "md2man", "~>5.1.1", require: false
-  gem "rubocop", "~> 0.51.0", require: false
+  gem "rubocop", "~> 0.52.0", require: false
   gem "web-console", "~> 2.1.3"
   gem "wirb"
   gem "wirble"

Gemfile.lock

@@ -60,7 +60,7 @@ GEM
     bootstrap-sass (3.3.5)
       autoprefixer-rails (>= 5.0.0.1)
       sass (>= 3.2.19)
-    brakeman (4.0.1)
+    brakeman (4.1.1)
     builder (3.2.3)
     byebug (9.1.0)
     capybara (2.14.4)
@@ -86,12 +86,11 @@ GEM
     debug_inspector (0.0.2)
     descendants_tracker (0.0.4)
       thread_safe (~> 0.3, >= 0.3.1)
-    devise (3.5.1)
+    devise (4.4.0)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
-      railties (>= 3.2.6, < 5)
+      railties (>= 4.1.0, < 5.2)
       responders
-      thread_safe (~> 0.1)
       warden (~> 1.2.3)
     diff-lcs (1.3)
     docile (1.1.5)
@@ -201,7 +200,7 @@ GEM
     mustermann (1.0.1)
     mustermann-grape (1.0.0)
       mustermann (~> 1.0.0)
-    mysql2 (0.4.7)
+    mysql2 (0.4.10)
     nenv (0.3.0)
     net-ldap (0.16.1)
     nokogiri (1.8.1)
@@ -240,9 +239,9 @@ GEM
     opener (0.1.0)
     orm_adapter (0.5.0)
     paint (1.0.0)
-    parallel (1.12.0)
-    parser (2.4.0.0)
-      ast (~> 2.2)
+    parallel (1.12.1)
+    parser (2.4.0.2)
+      ast (~> 2.3)
     pg (0.20.0)
     poltergeist (1.15.0)
       capybara (~> 2.1)
@@ -306,15 +305,15 @@ GEM
       activesupport (= 4.2.10)
       rake (>= 0.8.7)
       thor (>= 0.18.1, < 2.0)
-    rainbow (2.2.2)
-      rake
+    rainbow (3.0.0)
     rake (12.2.1)
     rb-fsevent (0.10.2)
     rb-inotify (0.9.10)
       ffi (>= 0.5.0, < 2)
     redcarpet (3.4.0)
-    responders (2.1.0)
-      railties (>= 4.2.0, < 5)
+    responders (2.4.0)
+      actionpack (>= 4.2.0, < 5.3)
+      railties (>= 4.2.0, < 5.3)
     rouge (1.11.1)
     rspec (3.7.0)
       rspec-core (~> 3.7.0)
@@ -337,11 +336,11 @@ GEM
       rspec-mocks (~> 3.7.0)
       rspec-support (~> 3.7.0)
     rspec-support (3.7.0)
-    rubocop (0.51.0)
+    rubocop (0.52.1)
       parallel (~> 1.10)
-      parser (>= 2.3.3.1, < 3.0)
+      parser (>= 2.4.0.2, < 3.0)
       powerpack (~> 0.1)
-      rainbow (>= 2.2.2, < 3.0)
+      rainbow (>= 2.2.2, < 4.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.0, >= 1.0.1)
     ruby-graphviz (1.2.2)
@@ -474,7 +473,7 @@ DEPENDENCIES
   md2man (~> 5.1.1)
   minitest (= 5.10.1)
   multi_json (= 1.12.1)
-  mysql2 (= 0.4.7)
+  mysql2 (= 0.4.10)
   net-ldap (~> 0.16.1)
   omniauth-github
   omniauth-gitlab
@@ -496,7 +495,7 @@ DEPENDENCIES
   redcarpet (~> 3.4.0)
   rspec-core (~> 3.7.0)
   rspec-rails
-  rubocop (~> 0.51.0)
+  rubocop (~> 0.52.0)
   sass (~> 3.4.23)
   sass-rails (~> 5.0.6)
   search_cop

app/controllers/auth/omniauth_callbacks_controller.rb

@@ -2,7 +2,10 @@
 
 class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
   skip_before_action :verify_authenticity_token
+
+  # rubocop:disable Rails/LexicallyScopedActionFilter
   before_action :check_user, except: [:failure]
+  # rubocop:enable Rails/LexicallyScopedActionFilter
 
   # GET /users/auth/:provider/callback. Providers redirect to the endpoint.
   # Callback for Google OAuth2.

app/controllers/auth/registrations_controller.rb

@@ -9,7 +9,10 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   before_action :check_signup, only: %i[new create]
   before_action :check_admin, only: %i[new create]
   before_action :configure_sign_up_params, only: [:create]
+
+  # rubocop:disable Rails/LexicallyScopedActionFilter
   before_action :authenticate_user!, only: [:disable]
+  # rubocop:enable Rails/LexicallyScopedActionFilter
 
   # Re-implemented so the template has the auxiliary variables regarding if
   # there are more users on the system or this is the first user to be created.
@@ -22,13 +25,13 @@ def create
     build_resource(sign_up_params)
 
     resource.save
+    yield resource if block_given?
     if resource.persisted?
-      session_flash(resource, :signed_up)
-      sign_up(resource_name, resource)
-      respond_with resource, location: after_sign_up_path_for(resource), float: true
+      resource_persisted_response!
     else
-      redirect_to new_user_registration_path,
-                  alert: resource.errors.full_messages
+      clean_up_passwords resource
+      set_minimum_password_length
+      redirect_to new_user_registration_path, alert: resource.errors.full_messages
     end
   end
 
@@ -41,7 +44,7 @@ def update
     success =
       if password_update?
         succ = current_user.update_with_password(user_params)
-        sign_in(current_user, bypass: true) if succ
+        bypass_sign_in(current_user) if succ
         succ
       else
         current_user.update_without_password(params.require(:user).permit(:email, :display_name))
@@ -91,13 +94,27 @@ def check_signup
   end
 
   def configure_sign_up_params
-    devise_parameter_sanitizer.for(:sign_up) << :email
-    return if User.admins.any?
-    devise_parameter_sanitizer.for(:sign_up) << :admin
+    keys = User.admins.any? ? %i[email] : %i[email admin]
+    devise_parameter_sanitizer.permit(:sign_up, keys: keys)
   end
 
   protected
 
+  # Performs the response in the case of a registration success.
+  def resource_persisted_response!
+    if resource.active_for_authentication?
+      session_flash(resource, :signed_up)
+      sign_up(resource_name, resource)
+      respond_with resource, location: after_sign_up_path_for(resource), float: true
+    else
+      # :nocov:
+      session_flash(resource, :"signed_up_but_#{resource.inactive_message}")
+      expire_data_after_sign_in!
+      respond_with resource, location: after_inactive_sign_up_path_for(resource), float: true
+      # :nocov:
+    end
+  end
+
   # Returns true if the contents of the `params` hash contains the needed keys
   # to update the password of the user.
   def password_update?

app/controllers/concerns/check_ldap.rb

@@ -7,7 +7,9 @@ module CheckLDAP
   extend ActiveSupport::Concern
 
   included do
+    # rubocop:disable Rails/LexicallyScopedActionFilter
     before_action :check_ldap, only: %i[new create]
+    # rubocop:enable Rails/LexicallyScopedActionFilter
   end
 
   # Redirect to the login page if LDAP is enabled.

app/controllers/concerns/deletable.rb

@@ -6,7 +6,9 @@ module Deletable
   extend ActiveSupport::Concern
 
   included do
+    # rubocop:disable Rails/LexicallyScopedActionFilter
     before_action :delete_enabled?, only: [:destroy]
+    # rubocop:enable Rails/LexicallyScopedActionFilter
   end
 
   # Returns true if users can delete images/tags.

app/helpers/activities_helper.rb

@@ -35,12 +35,7 @@ def activity_team(activity, nested = nil)
   # `:owner_name`). If that doesn't work either, then the given `empty_user`
   # parameter is returned as-is.
   def activity_user(activity, method, param, empty_user)
-    if activity.send(method)
-      activity.send(method).display_username
-    elsif activity.parameters[param].blank?
-      empty_user
-    else
-      activity.parameters[param]
-    end
+    return activity.send(method).display_username if activity.send(method)
+    activity.parameters[param].presence || empty_user
   end
 end

app/models/application_token.rb

@@ -55,9 +55,7 @@ def create_token(current_user:, user_id: nil, params:)
         plain_token,
         application_token.token_salt
       )
-      if application_token.save
-        application_token.create_activity!(:create, current_user)
-      end
+      application_token.create_activity!(:create, current_user) if application_token.save
 
       [application_token, plain_token]
     end

app/models/comment.rb

@@ -20,7 +20,7 @@
 class Comment < ActiveRecord::Base
   include PublicActivity::Common
   belongs_to :repository
-  belongs_to :author, class_name: "User", foreign_key: "user_id"
+  belongs_to :author, class_name: "User", foreign_key: "user_id", inverse_of: "comments"
 
   validates :body, presence: true
 

app/models/registry.rb

@@ -72,9 +72,7 @@ def self.find_from_event(event)
       logger.debug("No hostname matching #{request_hostname}, testing external_hostname")
       registry = Registry.find_by(external_hostname: request_hostname)
     end
-    if registry.nil?
-      logger.info("Ignoring event coming from unknown registry #{request_hostname}")
-    end
+    logger.info("Ignoring event coming from unknown registry #{request_hostname}") if registry.nil?
     registry
   end
 

app/models/tag.rb

@@ -33,7 +33,7 @@ class Tag < ActiveRecord::Base
   enum status: { scan_none: 0, scan_working: 1, scan_done: 2 }
 
   belongs_to :repository
-  belongs_to :author, class_name: "User", foreign_key: "user_id"
+  belongs_to :author, class_name: "User", foreign_key: "user_id", inverse_of: "tags"
 
   # We don't validate the tag, because we will fetch that from the registry,
   # and that's guaranteed to have a good format.
@@ -47,13 +47,8 @@ class Tag < ActiveRecord::Base
 
   # Returns a string containing the username of the user that pushed this tag.
   def owner
-    if author
-      author.display_username
-    elsif username.blank?
-      "someone"
-    else
-      username
-    end
+    return author.display_username if author
+    username.presence || "someone"
   end
 
   # Delete all the tags that match the given digest. Call this method if you

app/models/user.rb

@@ -70,6 +70,8 @@ class User < ActiveRecord::Base
   has_many :teams, through: :team_users
   has_many :stars, dependent: :destroy
   has_many :application_tokens, dependent: :destroy
+  has_many :tags, dependent: :nullify
+  has_many :comments, dependent: :destroy
 
   scope :not_portus, -> { where.not username: "portus" }
   scope :enabled,    -> { not_portus.where enabled: true }
@@ -96,7 +98,7 @@ def portus?
   # Returns the username to be displayed.
   def display_username
     return username unless APP_CONFIG.enabled?("display_name")
-    display_name.blank? ? username : display_name
+    display_name.presence || username
   end
 
   # This method will be called automatically once a user is created. It will
@@ -182,9 +184,7 @@ def self.search_from_query(members, query)
   # Return true if there's an application token matching it, false otherwise
   def application_token_valid?(plain_token)
     application_tokens.each do |t|
-      if t.token_hash == BCrypt::Engine.hash_secret(plain_token, t.token_salt)
-        return true
-      end
+      return true if t.token_hash == BCrypt::Engine.hash_secret(plain_token, t.token_salt)
     end
 
     false

app/models/webhook.rb

@@ -41,8 +41,8 @@ class Webhook < ActiveRecord::Base
 
   belongs_to :namespace
 
-  has_many :deliveries, class_name: "WebhookDelivery", dependent: :destroy
-  has_many :headers, class_name: "WebhookHeader", dependent: :destroy
+  has_many :deliveries, class_name: "WebhookDelivery", dependent: :destroy, inverse_of: "webhook"
+  has_many :headers, class_name: "WebhookHeader", dependent: :destroy, inverse_of: "webhook"
 
   validates :url, presence: true, url: true
 

app/models/webhook_delivery.rb

@@ -25,7 +25,7 @@
 # information regarding both the request and the response.  Webhook deliveries
 # can also be retrigged.
 class WebhookDelivery < ActiveRecord::Base
-  belongs_to :webhook
+  belongs_to :webhook, inverse_of: "deliveries"
 
   validates :uuid, uniqueness: { scope: :webhook_id }
 

app/models/webhook_header.rb

@@ -20,7 +20,7 @@
 # A WebhookHeader is a key value pair, and describes a HTTP header which is
 # to be included in a webhook request.
 class WebhookHeader < ActiveRecord::Base
-  belongs_to :webhook
+  belongs_to :webhook, inverse_of: "headers"
 
   validates :name, uniqueness: { scope: :webhook_id }
 end