[ARVADOS] created: 1.3.0-2864-g6f9968cdf

Git user git at public.arvados.org
Fri Aug 7 14:20:21 UTC 2020


        at  6f9968cdf37dc1f7b3c3f6eecb91c484036a0d75 (commit)


commit 6f9968cdf37dc1f7b3c3f6eecb91c484036a0d75
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Aug 6 09:22:28 2020 -0300

    16470: Don't explicitly reload before locking on CR updates.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index 6bffc9a63..369294e8a 100644
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -9,14 +9,14 @@ require "rails"
 require "active_model/railtie"
 require "active_job/railtie"
 require "active_record/railtie"
-# require "active_storage/engine"
 require "action_controller/railtie"
 require "action_mailer/railtie"
 require "action_view/railtie"
-# Skip ActionCable (new in Rails 5.0) as it adds '/cable' routes that we're not using
-# require "action_cable/engine"
 require "sprockets/railtie"
 require "rails/test_unit/railtie"
+# Skipping the following:
+# * ActionCable (new in Rails 5.0) as it adds '/cable' routes that we're not using
+# * Skip ActiveStorage (new in Rails 5.1)
 
 require 'digest'
 

commit 42c80a6bf7e2ebcec4001c345b855b95a1dc0ddf
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Wed Aug 5 10:38:04 2020 -0300

    16470: Fixes unrelated typo.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index e742bbc59..2dd1d816e 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -137,7 +137,7 @@ func (h *Handler) db(ctx context.Context) (*sqlx.DB, error) {
 		db.SetMaxOpenConns(p)
 	}
 	if err := db.Ping(); err != nil {
-		ctxlog.FromContext(ctx).WithError(err).Error("postgresql connect scuceeded but ping failed")
+		ctxlog.FromContext(ctx).WithError(err).Error("postgresql connect succeeded but ping failed")
 		return nil, errDBConnection
 	}
 	h.pgdb = db

commit eddb0aa04ca644d3b2fd0289282297ed9ac8e9c0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Mon Aug 3 19:24:00 2020 -0300

    16470: Fixes collection versioning management code.
    
    Collection versioning management is implemented on an 'around_update' callback
    so some 'attribute_changed()' & 'changes()' calls changed behaviour depending
    on which side of the 'yield' were made.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 8f724e1f5..c7d5a93b5 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -303,12 +303,18 @@ class Collection < ArvadosModel
 
   def syncable_updates
     updates = {}
-    (syncable_attrs & self.changes.keys).each do |attr|
+    if self.changes.any?
+      changes = self.changes
+    else
+      # If called after save...
+      changes = self.saved_changes
+    end
+    (syncable_attrs & changes.keys).each do |attr|
       if attr == 'uuid'
         # Point old versions to current version's new UUID
-        updates['current_version_uuid'] = self.changes[attr].last
+        updates['current_version_uuid'] = changes[attr].last
       else
-        updates[attr] = self.changes[attr].last
+        updates[attr] = changes[attr].last
       end
     end
     return updates
@@ -316,7 +322,7 @@ class Collection < ArvadosModel
 
   def sync_past_versions
     updates = self.syncable_updates
-    Collection.where('current_version_uuid = ? AND uuid != ?', self.uuid_was, self.uuid_was).each do |c|
+    Collection.where('current_version_uuid = ? AND uuid != ?', self.uuid_before_last_save, self.uuid_before_last_save).each do |c|
       c.attributes = updates
       # Use a different validation context to skip the 'past_versions_cannot_be_updated'
       # validator, as on this case it is legal to update some fields.

commit d63344d18060a09a9309908cd078a4bb614c35c9
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Jul 31 16:43:40 2020 -0300

    16470: Avoids crashing when running the rake task db:create.
    
    There was a behaviour change on rake tasks that avoided the creation of the
    test database. All initializers are now called from rake tasks, so those
    initializers with code accessing the database will fail when the database
    isn't created.
    
    See: https://github.com/rails/rails/issues/32870
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/config/initializers/legacy_jobs_api.rb b/services/api/config/initializers/legacy_jobs_api.rb
index 8f3b3cb5f..2abe40566 100644
--- a/services/api/config/initializers/legacy_jobs_api.rb
+++ b/services/api/config/initializers/legacy_jobs_api.rb
@@ -8,8 +8,13 @@
 
 require 'enable_jobs_api'
 
-Server::Application.configure do
-  if ActiveRecord::Base.connection.tables.include?('jobs')
-    check_enable_legacy_jobs_api
+Rails.application.configure do
+  begin
+    if ActiveRecord::Base.connection.tables.include?('jobs')
+      check_enable_legacy_jobs_api
+    end
+  rescue ActiveRecord::NoDatabaseError
+    # Since rails 5.2, all initializers are run by rake tasks (like db:create),
+    # see: https://github.com/rails/rails/issues/32870
   end
 end

commit 6a4300f29edd25cc3fad565a6c11fe800b8207fd
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Jul 31 14:38:20 2020 -0300

    16470: File updates & additions from 'rake rails:update'
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/Gemfile b/services/api/Gemfile
index 481cf5b42..1e12d6a4c 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -22,8 +22,12 @@ group :test, :development do
   gem 'simplecov-rcov', require: false
   gem 'mocha', require: false
   gem 'byebug'
+  gem 'listen'
 end
 
+# Fast app boot times
+gem 'bootsnap', require: false
+
 gem 'pg', '~> 1.0'
 
 gem 'multi_json'
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 34863158d..427915189 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -86,6 +86,8 @@ GEM
       addressable (>= 2.3.1)
       extlib (>= 0.9.15)
       multi_json (>= 1.0.0)
+    bootsnap (1.4.7)
+      msgpack (~> 1.0)
     builder (3.2.4)
     byebug (11.0.1)
     capistrano (2.15.9)
@@ -130,6 +132,9 @@ GEM
     launchy (2.4.3)
       addressable (~> 2.3)
     libv8 (3.16.14.19)
+    listen (3.2.1)
+      rb-fsevent (~> 0.10, >= 0.10.3)
+      rb-inotify (~> 0.9, >= 0.9.10)
     lograge (0.10.0)
       actionpack (>= 4)
       activesupport (>= 4)
@@ -152,6 +157,7 @@ GEM
     minitest (5.10.3)
     mocha (1.8.0)
       metaclass (~> 0.0.1)
+    msgpack (1.3.3)
     multi_json (1.14.1)
     multi_xml (0.6.0)
     multipart-post (2.1.1)
@@ -289,10 +295,12 @@ DEPENDENCIES
   acts_as_api
   andand
   arvados!
+  bootsnap
   byebug
   factory_bot_rails
   httpclient
   jquery-rails
+  listen
   lograge
   logstash-event
   minitest (= 5.10.3)
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index c3996f0a0..2644a0657 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -360,7 +360,7 @@ class ApplicationController < ActionController::Base
     %w(created_at modified_by_client_uuid modified_by_user_uuid modified_at).each do |x|
       @attrs.delete x.to_sym
     end
-    @attrs = @attrs.symbolize_keys if @attrs.is_a? HashWithIndifferentAccess
+    @attrs = @attrs.symbolize_keys if @attrs.is_a? ActiveSupport::HashWithIndifferentAccess
     @attrs
   end
 
diff --git a/services/api/bin/bundle b/services/api/bin/bundle
index 044b5ca23..00d640cf7 100755
--- a/services/api/bin/bundle
+++ b/services/api/bin/bundle
@@ -4,5 +4,5 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__)
+ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
 load Gem.bin_path('bundler', 'bundle')
diff --git a/services/api/bin/setup b/services/api/bin/setup
index 2e4d28c58..c9142b942 100755
--- a/services/api/bin/setup
+++ b/services/api/bin/setup
@@ -4,12 +4,11 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-require 'pathname'
 require 'fileutils'
 include FileUtils
 
 # path to your application root.
-APP_ROOT = Pathname.new File.expand_path('../../', __FILE__)
+APP_ROOT = File.expand_path('..', __dir__)
 
 def system!(*args)
   system(*args) || abort("\n== Command #{args} failed ==")
diff --git a/services/api/bin/update b/services/api/bin/update
index 07a3df93e..201287ef6 100755
--- a/services/api/bin/update
+++ b/services/api/bin/update
@@ -4,12 +4,11 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-require 'pathname'
 require 'fileutils'
 include FileUtils
 
 # path to your application root.
-APP_ROOT = Pathname.new File.expand_path('../../', __FILE__)
+APP_ROOT = File.expand_path('..', __dir__)
 
 def system!(*args)
   system(*args) || abort("\n== Command #{args} failed ==")
diff --git a/services/api/bin/yarn b/services/api/bin/yarn
new file mode 100755
index 000000000..cc54a3ba5
--- /dev/null
+++ b/services/api/bin/yarn
@@ -0,0 +1,16 @@
+#!/usr/bin/env ruby
+
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+APP_ROOT = File.expand_path('..', __dir__)
+Dir.chdir(APP_ROOT) do
+  begin
+    exec "yarnpkg", *ARGV
+  rescue Errno::ENOENT
+    $stderr.puts "Yarn executable was not detected in the system."
+    $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install"
+    exit 1
+  end
+end
diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index b6174a0d8..6bffc9a63 100644
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -2,13 +2,14 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-require File.expand_path('../boot', __FILE__)
+require_relative 'boot'
 
 require "rails"
 # Pick only the frameworks we need:
 require "active_model/railtie"
 require "active_job/railtie"
 require "active_record/railtie"
+# require "active_storage/engine"
 require "action_controller/railtie"
 require "action_mailer/railtie"
 require "action_view/railtie"
diff --git a/services/api/config/boot.rb b/services/api/config/boot.rb
index 717101c2b..9605b584e 100644
--- a/services/api/config/boot.rb
+++ b/services/api/config/boot.rb
@@ -5,4 +5,5 @@
 # Set up gems listed in the Gemfile.
 ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
 
-require 'bundler/setup'
+require 'bundler/setup' # Set up gems listed in the Gemfile.
+require 'bootsnap/setup' # Speed up boot time by caching expensive operations.
\ No newline at end of file
diff --git a/services/api/config/environments/development.rb.example b/services/api/config/environments/development.rb.example
index 56a4ed6dc..f5ab77a4d 100644
--- a/services/api/config/environments/development.rb.example
+++ b/services/api/config/environments/development.rb.example
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-Server::Application.configure do
+Rails.application.configure do
   # Settings specified here will take precedence over those in config/application.rb
 
   # In the development environment your application's code is reloaded on
diff --git a/services/api/config/environments/production.rb.example b/services/api/config/environments/production.rb.example
index 6c48dcd01..c8194057c 100644
--- a/services/api/config/environments/production.rb.example
+++ b/services/api/config/environments/production.rb.example
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-Server::Application.configure do
+Rails.application.configure do
   # Settings specified here will take precedence over those in config/application.rb
 
   # Code is not reloaded between requests
diff --git a/services/api/config/environments/test.rb.example b/services/api/config/environments/test.rb.example
index 6b550587c..9cdf5d9cd 100644
--- a/services/api/config/environments/test.rb.example
+++ b/services/api/config/environments/test.rb.example
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-Server::Application.configure do
+Rails.application.configure do
   # Settings specified here will take precedence over those in config/application.rb
 
   # The test environment is used exclusively to run your application's
diff --git a/services/api/config/initializers/content_security_policy.rb b/services/api/config/initializers/content_security_policy.rb
new file mode 100644
index 000000000..853ecdeec
--- /dev/null
+++ b/services/api/config/initializers/content_security_policy.rb
@@ -0,0 +1,29 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+# Be sure to restart your server when you modify this file.
+
+# Define an application-wide content security policy
+# For further information see the following documentation
+# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy
+
+# Rails.application.config.content_security_policy do |policy|
+#   policy.default_src :self, :https
+#   policy.font_src    :self, :https, :data
+#   policy.img_src     :self, :https, :data
+#   policy.object_src  :none
+#   policy.script_src  :self, :https
+#   policy.style_src   :self, :https
+
+#   # Specify URI for violation reports
+#   # policy.report_uri "/csp-violation-report-endpoint"
+# end
+
+# If you are using UJS then enable automatic nonce generation
+# Rails.application.config.content_security_policy_nonce_generator = -> request { SecureRandom.base64(16) }
+
+# Report CSP violations to a specified URI
+# For further information see the following documentation:
+# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy-Report-Only
+# Rails.application.config.content_security_policy_report_only = true
diff --git a/services/api/config/initializers/new_framework_defaults_5_2.rb b/services/api/config/initializers/new_framework_defaults_5_2.rb
new file mode 100644
index 000000000..93a8d5240
--- /dev/null
+++ b/services/api/config/initializers/new_framework_defaults_5_2.rb
@@ -0,0 +1,42 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+# Be sure to restart your server when you modify this file.
+#
+# This file contains migration options to ease your Rails 5.2 upgrade.
+#
+# Once upgraded flip defaults one by one to migrate to the new default.
+#
+# Read the Guide for Upgrading Ruby on Rails for more info on each option.
+
+# Make Active Record use stable #cache_key alongside new #cache_version method.
+# This is needed for recyclable cache keys.
+# Rails.application.config.active_record.cache_versioning = true
+
+# Use AES-256-GCM authenticated encryption for encrypted cookies.
+# Also, embed cookie expiry in signed or encrypted cookies for increased security.
+#
+# This option is not backwards compatible with earlier Rails versions.
+# It's best enabled when your entire app is migrated and stable on 5.2.
+#
+# Existing cookies will be converted on read then written with the new scheme.
+# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true
+
+# Use AES-256-GCM authenticated encryption as default cipher for encrypting messages
+# instead of AES-256-CBC, when use_authenticated_message_encryption is set to true.
+# Rails.application.config.active_support.use_authenticated_message_encryption = true
+
+# Add default protection from forgery to ActionController::Base instead of in
+# ApplicationController.
+# Rails.application.config.action_controller.default_protect_from_forgery = true
+
+# Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and
+# 'f' after migrating old data.
+# Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true
+
+# Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header.
+# Rails.application.config.active_support.use_sha1_digests = true
+
+# Make `form_with` generate id attributes for any generated HTML tags.
+# Rails.application.config.action_view.form_with_generates_ids = true
diff --git a/services/api/config/initializers/wrap_parameters.rb b/services/api/config/initializers/wrap_parameters.rb
index 976777723..6fb978650 100644
--- a/services/api/config/initializers/wrap_parameters.rb
+++ b/services/api/config/initializers/wrap_parameters.rb
@@ -9,7 +9,7 @@
 
 # Enable parameter wrapping for JSON. You can disable this by setting :format to an empty array.
 ActiveSupport.on_load(:action_controller) do
-  wrap_parameters :format => [:json]
+  wrap_parameters format: [:json]
 end
 
 # Disable root element in JSON by default.
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index 8afd22192..697585803 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-Server::Application.routes.draw do
+Rails.application.routes.draw do
   themes_for_rails
 
   # OPTIONS requests are not allowed at routes that use cookies.
diff --git a/services/api/config/secrets.yml b/services/api/config/secrets.yml
new file mode 100644
index 000000000..62e1f27cd
--- /dev/null
+++ b/services/api/config/secrets.yml
@@ -0,0 +1,36 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+# Be sure to restart your server when you modify this file.
+
+# Your secret key is used for verifying the integrity of signed cookies.
+# If you change this key, all old signed cookies will become invalid!
+
+# Make sure the secret is at least 30 characters and all random,
+# no regular words or you'll be exposed to dictionary attacks.
+# You can use `rails secret` to generate a secure secret key.
+
+# Make sure the secrets in this file are kept private
+# if you're sharing your code publicly.
+
+# Shared secrets are available across all environments.
+
+# shared:
+#   api_key: a1B2c3D4e5F6
+
+# Environmental secrets are only available for that specific environment.
+
+development:
+  secret_key_base: 5b710df613166e048853346d14a1837593db4463b5a778a0b747346d4758a0b4fce9f136c3063f37d92def51917fd42d137f94190de2262ebf3fe25c1f16748a
+
+test:
+  secret_key_base: 52392a8314cf1d49f2a81478541578e9be2db70d2be0047492d5ce6b7c7234303e01ff8742fc4c90775fa1fbee2dc3e85d7ecb17a50c36e2b0e29943f82d0804
+
+# Do not keep production secrets in the unencrypted secrets file.
+# Instead, either read values from the environment.
+# Or, use `bin/rails secrets:setup` to configure encrypted secrets
+# and move the `production:` environment over there.
+
+production:
+  secret_key_base: <%= ENV["SECRET_KEY_BASE"] %>
diff --git a/services/api/config/storage.yml b/services/api/config/storage.yml
new file mode 100644
index 000000000..5b2c94c43
--- /dev/null
+++ b/services/api/config/storage.yml
@@ -0,0 +1,38 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+test:
+  service: Disk
+  root: <%= Rails.root.join("tmp/storage") %>
+
+local:
+  service: Disk
+  root: <%= Rails.root.join("storage") %>
+
+# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key)
+# amazon:
+#   service: S3
+#   access_key_id: <%= Rails.application.credentials.dig(:aws, :access_key_id) %>
+#   secret_access_key: <%= Rails.application.credentials.dig(:aws, :secret_access_key) %>
+#   region: us-east-1
+#   bucket: your_own_bucket
+
+# Remember not to checkin your GCS keyfile to a repository
+# google:
+#   service: GCS
+#   project: your_project
+#   credentials: <%= Rails.root.join("path/to/gcs.keyfile") %>
+#   bucket: your_own_bucket
+
+# Use rails credentials:edit to set the Azure Storage secret (as azure_storage:storage_access_key)
+# microsoft:
+#   service: AzureStorage
+#   storage_account_name: your_account_name
+#   storage_access_key: <%= Rails.application.credentials.dig(:azure_storage, :storage_access_key) %>
+#   container: your_container_name
+
+# mirror:
+#   service: Mirror
+#   primary: local
+#   mirrors: [ amazon, google, microsoft ]

commit 1cbc3c149b2ecc447d74fd3a992d0addbb30a676
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Jul 28 17:51:16 2020 -0300

    16470: Fixes deprecation warning.
    
    Explained at: https://github.com/rails/rails/pull/29619#issuecomment-392583498
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index adfbf6042..5833c2251 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -387,7 +387,7 @@ class Container < ArvadosModel
     if users_list.select { |u| u.is_admin }.any?
       return super
     end
-    Container.where(ContainerRequest.readable_by(*users_list).where("containers.uuid = container_requests.container_uuid").exists)
+    Container.where(ContainerRequest.readable_by(*users_list).where("containers.uuid = container_requests.container_uuid").arel.exists)
   end
 
   def final?

commit 96cd3b89f4dce7396bd696c4892da6e9b685fc20
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Jul 28 17:40:09 2020 -0300

    16470: Removes exception deprecated on rails 3.2
    
    Its substitute is already being used.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 83a233cd5..c3996f0a0 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -63,7 +63,6 @@ class ApplicationController < ActionController::Base
                 :with => :render_error)
     rescue_from(ActiveRecord::RecordNotFound,
                 ActionController::RoutingError,
-                ActionController::UnknownController,
                 AbstractController::ActionNotFound,
                 :with => :render_not_found)
   end

commit 4a825192c209a5af470ba1e2de60c9792cc4e13f
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Jul 28 16:54:22 2020 -0300

    16470: Upgrades rails to 5.2
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/Gemfile b/services/api/Gemfile
index 844b789af..481cf5b42 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -4,7 +4,7 @@
 
 source 'https://rubygems.org'
 
-gem 'rails', '5.1.7'
+gem 'rails', '~> 5.2.0'
 gem 'responders', '~> 2.0'
 
 group :test, :development do
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 90306bb58..34863158d 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -22,39 +22,43 @@ GIT
 GEM
   remote: https://rubygems.org/
   specs:
-    actioncable (5.1.7)
-      actionpack (= 5.1.7)
+    actioncable (5.2.4.3)
+      actionpack (= 5.2.4.3)
       nio4r (~> 2.0)
-      websocket-driver (~> 0.6.1)
-    actionmailer (5.1.7)
-      actionpack (= 5.1.7)
-      actionview (= 5.1.7)
-      activejob (= 5.1.7)
+      websocket-driver (>= 0.6.1)
+    actionmailer (5.2.4.3)
+      actionpack (= 5.2.4.3)
+      actionview (= 5.2.4.3)
+      activejob (= 5.2.4.3)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (5.1.7)
-      actionview (= 5.1.7)
-      activesupport (= 5.1.7)
-      rack (~> 2.0)
+    actionpack (5.2.4.3)
+      actionview (= 5.2.4.3)
+      activesupport (= 5.2.4.3)
+      rack (~> 2.0, >= 2.0.8)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (5.1.7)
-      activesupport (= 5.1.7)
+    actionview (5.2.4.3)
+      activesupport (= 5.2.4.3)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    activejob (5.1.7)
-      activesupport (= 5.1.7)
+    activejob (5.2.4.3)
+      activesupport (= 5.2.4.3)
       globalid (>= 0.3.6)
-    activemodel (5.1.7)
-      activesupport (= 5.1.7)
-    activerecord (5.1.7)
-      activemodel (= 5.1.7)
-      activesupport (= 5.1.7)
-      arel (~> 8.0)
-    activesupport (5.1.7)
+    activemodel (5.2.4.3)
+      activesupport (= 5.2.4.3)
+    activerecord (5.2.4.3)
+      activemodel (= 5.2.4.3)
+      activesupport (= 5.2.4.3)
+      arel (>= 9.0)
+    activestorage (5.2.4.3)
+      actionpack (= 5.2.4.3)
+      activerecord (= 5.2.4.3)
+      marcel (~> 0.3.1)
+    activesupport (5.2.4.3)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
@@ -66,7 +70,7 @@ GEM
     addressable (2.7.0)
       public_suffix (>= 2.0.2, < 5.0)
     andand (1.3.3)
-    arel (8.0.0)
+    arel (9.0.0)
     arvados-google-api-client (0.8.7.4)
       activesupport (>= 3.2, < 5.3)
       addressable (~> 2.3)
@@ -137,9 +141,12 @@ GEM
       nokogiri (>= 1.5.9)
     mail (2.7.1)
       mini_mime (>= 0.1.1)
+    marcel (0.3.3)
+      mimemagic (~> 0.3.2)
     memoist (0.16.2)
     metaclass (0.0.4)
     method_source (1.0.0)
+    mimemagic (0.3.5)
     mini_mime (1.0.2)
     mini_portile2 (2.4.0)
     minitest (5.10.3)
@@ -182,17 +189,18 @@ GEM
     rack (2.2.3)
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
-    rails (5.1.7)
-      actioncable (= 5.1.7)
-      actionmailer (= 5.1.7)
-      actionpack (= 5.1.7)
-      actionview (= 5.1.7)
-      activejob (= 5.1.7)
-      activemodel (= 5.1.7)
-      activerecord (= 5.1.7)
-      activesupport (= 5.1.7)
+    rails (5.2.4.3)
+      actioncable (= 5.2.4.3)
+      actionmailer (= 5.2.4.3)
+      actionpack (= 5.2.4.3)
+      actionview (= 5.2.4.3)
+      activejob (= 5.2.4.3)
+      activemodel (= 5.2.4.3)
+      activerecord (= 5.2.4.3)
+      activestorage (= 5.2.4.3)
+      activesupport (= 5.2.4.3)
       bundler (>= 1.3.0)
-      railties (= 5.1.7)
+      railties (= 5.2.4.3)
       sprockets-rails (>= 2.0.0)
     rails-controller-testing (1.0.4)
       actionpack (>= 5.0.1.x)
@@ -206,12 +214,12 @@ GEM
     rails-observers (0.1.5)
       activemodel (>= 4.0)
     rails-perftest (0.0.7)
-    railties (5.1.7)
-      actionpack (= 5.1.7)
-      activesupport (= 5.1.7)
+    railties (5.2.4.3)
+      actionpack (= 5.2.4.3)
+      activesupport (= 5.2.4.3)
       method_source
       rake (>= 0.8.7)
-      thor (>= 0.18.1, < 2.0)
+      thor (>= 0.19.0, < 2.0)
     rake (13.0.1)
     rb-fsevent (0.10.3)
     rb-inotify (0.9.10)
@@ -270,7 +278,7 @@ GEM
     uglifier (2.7.2)
       execjs (>= 0.3.0)
       json (>= 1.8.0)
-    websocket-driver (0.6.5)
+    websocket-driver (0.7.3)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
 
@@ -296,7 +304,7 @@ DEPENDENCIES
   optimist
   passenger
   pg (~> 1.0)
-  rails (= 5.1.7)
+  rails (~> 5.2.0)
   rails-controller-testing
   rails-observers
   rails-perftest

commit e86f10186e30c511a14cd6af4eb388eef56fbdeb
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Jul 28 16:16:29 2020 -0300

    16470: Fixes false unpersisted status when retrieving a record with audit logs.
    
    The cleanest solution I came up with is to flag the instance when it's
    retrieved from the database, and reset any changes after stashing its
    state on the log_start_state callback.
    Haven't found a way to read the serialized attributes without making
    them appear as changed, and I think it isn't possible because the
    attributes have to be unserialized before the read operation, and thus
    the dirty state machinery would assume the attribute may be modified.
    This solution isn't ideal, but I think it's acceptable as it doesn't
    make additional database requests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 80ea0c0b7..c3e1ff42a 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -16,6 +16,7 @@ class ArvadosModel < ApplicationRecord
   include DbCurrentTime
   extend RecordFilters
 
+  after_find :schedule_restoring_changes
   after_initialize :log_start_state
   before_save :ensure_permission_to_save
   before_save :ensure_owner_uuid_is_permitted
@@ -834,10 +835,24 @@ class ArvadosModel < ApplicationRecord
              Rails.configuration.AuditLogs.MaxDeleteBatch.to_i > 0)
   end
 
+  def schedule_restoring_changes
+    # This will be checked at log_start_state, to reset any (virtual) changes
+    # produced by the act of reading a serialized attribute.
+    @fresh_from_database = true
+  end
+
   def log_start_state
     if is_audit_logging_enabled?
       @old_attributes = Marshal.load(Marshal.dump(attributes))
       @old_logged_attributes = Marshal.load(Marshal.dump(logged_attributes))
+      if @fresh_from_database
+        # This instance was created from reading a database record. Attributes
+        # haven't been changed, but those serialized attributes will be reported
+        # as unpersisted, so we restore them to avoid issues with lock!() and
+        # with_lock().
+        restore_attributes
+        @fresh_from_database = nil
+      end
     end
   end
 

commit 7ef25624f7325ff4e5d9b1beaac084b40b2d00ee
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Jul 28 13:55:50 2020 -0300

    16470: Adds ArvadosModel test exposing a bug with audit logs.
    
    When audit logs are enabled, fetching objects from models with serialized
    attributes (for example: User or ContainerRequest) return an unpersisted
    instance even if reload() is called on it.
    This is a problem because from Rails 5.2, lock!() and with_lock() will raise
    an exception when called on unpersisted instances.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index c1db8c8b5..64f780713 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -295,4 +295,29 @@ class ArvadosModelTest < ActiveSupport::TestCase
     c.reload
     assert_equal({'foo' => 'bar'}, c.properties)
   end
+
+  test 'serialized attributes dirty tracking with audit log settings' do
+    Rails.configuration.AuditLogs.MaxDeleteBatch = 1000
+    set_user_from_auth :admin
+    [false, true].each do |auditlogs_enabled|
+      if auditlogs_enabled
+        Rails.configuration.AuditLogs.MaxAge = 3600
+      else
+        Rails.configuration.AuditLogs.MaxAge = 0
+      end
+      [
+        User.find_by_uuid(users(:active).uuid),
+        ContainerRequest.find_by_uuid(container_requests(:queued).uuid),
+        Container.find_by_uuid(containers(:queued).uuid),
+        PipelineInstance.find_by_uuid(pipeline_instances(:has_component_with_completed_jobs).uuid),
+        PipelineTemplate.find_by_uuid(pipeline_templates(:two_part).uuid),
+        Job.find_by_uuid(jobs(:running).uuid)
+      ].each do |obj|
+        assert_not(obj.class.serialized_attributes.empty?,
+          "#{obj.class} model doesn't have serialized attributes")
+        # obj shouldn't have changed since it's just retrieved from the database
+        assert_not(obj.changed?, "#{obj.class} model's attribute(s) appear as changed: '#{obj.changes.keys.join(',')}' with audit logs #{auditlogs_enabled ? '': 'not '}enabled.")
+      end
+    end
+  end
 end

commit 68705e426e273d4ae708bff1de55ef60910abf64
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Jul 24 11:47:16 2020 -0300

    16470: Fixes test on user model.
    
    Rails 5.1 deprecated the attr_changed? in favor of more explicit methods
    because there was ambiguity when called from an 'after' or 'before' callback.
    The test UsersTest#test_cannot_set_is_active_to_false_directly started failing
    because User.setup is called from both types of callbacks, so its internal
    checks weren't passing in some cases.
    
    Also, avoids doing unnecessary queries to get the 'All users' group.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index e1fd53e3d..778ad7d0b 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -241,11 +241,8 @@ SELECT target_uuid, perm_level
                      name: 'can_login').destroy_all
 
     # delete "All users" group read permissions for this user
-    group = Group.where(name: 'All users').select do |g|
-      g[:uuid].match(/-f+$/)
-    end.first
     Link.where(tail_uuid: self.uuid,
-                     head_uuid: group[:uuid],
+                     head_uuid: all_users_group_uuid,
                      link_class: 'permission',
                      name: 'can_read').destroy_all
 
@@ -272,10 +269,6 @@ SELECT target_uuid, perm_level
        self.is_active_was &&
        !self.is_active
 
-      group = Group.where(name: 'All users').select do |g|
-        g[:uuid].match(/-f+$/)
-      end.first
-
       # When a user is set up, they are added to the "All users"
       # group.  A user that is part of the "All users" group is
       # allowed to self-activate.
@@ -290,7 +283,7 @@ SELECT target_uuid, perm_level
       # explaining the correct way to deactivate a user.
       #
       if Link.where(tail_uuid: self.uuid,
-                    head_uuid: group[:uuid],
+                    head_uuid: all_users_group_uuid,
                     link_class: 'permission',
                     name: 'can_read').any?
         errors.add :is_active, "cannot be set to false directly, use the 'Deactivate' button on Workbench, or the 'unsetup' API call"
@@ -711,11 +704,11 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
   # add the user to the 'All users' group
   def create_user_group_link
     return (Link.where(tail_uuid: self.uuid,
-                       head_uuid: all_users_group[:uuid],
+                       head_uuid: all_users_group_uuid,
                        link_class: 'permission',
                        name: 'can_read').first or
             Link.create(tail_uuid: self.uuid,
-                        head_uuid: all_users_group[:uuid],
+                        head_uuid: all_users_group_uuid,
                         link_class: 'permission',
                         name: 'can_read'))
   end
@@ -743,7 +736,8 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
   # Automatically setup if is_active flag turns on
   def setup_on_activate
     return if [system_user_uuid, anonymous_user_uuid].include?(self.uuid)
-    if is_active && (new_record? || saved_change_to_is_active?)
+    if is_active &&
+      (new_record? || saved_change_to_is_active? || will_save_change_to_is_active?)
       setup
     end
   end

commit 273f6a48b7eb0f263b441b3c2d77a6867e842f48
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Jul 23 17:54:32 2020 -0300

    16470: Changes the way of setting the callback that sets the DB timezone.
    
    This avoids intermittent freezes when running tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/config/initializers/time_zone.rb b/services/api/config/initializers/time_zone.rb
index cedd8f3e4..26681d613 100644
--- a/services/api/config/initializers/time_zone.rb
+++ b/services/api/config/initializers/time_zone.rb
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-ActiveRecord::Base.connection.class.set_callback :checkout, :after do
+ActiveRecord::ConnectionAdapters::AbstractAdapter.set_callback :checkout, :before, ->(conn) do
   # If the database connection is in a time zone other than UTC,
   # "timestamp" values don't behave as desired.
   #
@@ -11,5 +11,5 @@ ActiveRecord::Base.connection.class.set_callback :checkout, :after do
   # before now()), but false in time zone -0100 (now() returns an
   # earlier clock time, and its time zone is dropped when comparing to
   # a "timestamp without time zone").
-  raw_connection.sync_exec("SET TIME ZONE 'UTC'")
+  conn.execute("SET TIME ZONE 'UTC'")
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list