[ARVADOS] created: 1.3.0-2865-g7f45a8a81
Git user
git at public.arvados.org
Wed Aug 5 13:39:31 UTC 2020
at 7f45a8a8128471a162068a1552f66801a24ec5a8 (commit)
commit 7f45a8a8128471a162068a1552f66801a24ec5a8
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 96cdb90ff3d93b341fbe4cf3bbbd9bad41868a36
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 67ae90485817d5914c003c53cdcc2284a0dfcb5f
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 769ed08c1625486728fa009a939721adff30af8f
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 e79fd8a6408d9149d305c8df11122bedcfc9fd80
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 16f8cd798..74b820f88 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 92e9cd9aab499584306307438c4ce67356b3f80d
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 43a0af119a257dcf0a2b1035050a90bff1d2c5bd
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 a1ee1246eb9a06194da0019164e6d959ed3221d1
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 da3cafd0723843f822904477bb982e8af6b6f127
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 62224017b309804a1bf0b12079cfcafa4fb1b68d
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Fri Jul 24 17:12:27 2020 -0300
16470: Fixes the last deprecation warnings.
It turned out that .lock!() is an ActiveRecord method, and I was confusing it
with Container::lock and ContainerRequest::lock.
Both lock! and with_lock methods need the record to be persisted or reloaded
explicitly.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 77536eee4..16b497321 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -123,10 +123,10 @@ class ContainerRequest < ArvadosModel
while true
# get container lock first, then lock current container request
# (same order as Container#handle_completed). Locking always
- # reloads the Container and ContainerRequest records.
+ # requires explicit Container and ContainerRequest records reload.
c = Container.find_by_uuid(container_uuid)
- c.lock! if !c.nil?
- self.lock!
+ c.reload.lock! if !c.nil?
+ self.reload.lock!
if !c.nil? && container_uuid != c.uuid
# After locking, we've noticed a race, the container_uuid is
@@ -263,7 +263,7 @@ class ContainerRequest < ArvadosModel
if state_changed? and state == Committed and container_uuid.nil?
while true
c = Container.resolve(self)
- c.lock!
+ c.reload.lock!
if c.state == Container::Cancelled
# Lost a race, we have a lock on the container but the
# container was cancelled in a different request, restart
commit 7b90251f9b4e9f45eb35589885dd215267b845f6
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 74426fdd8e03ee62382023ed222edd4837757010
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
commit 618cd9cd4692300c106f3660c1a6288b5158871d
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Tue Jul 21 14:21:49 2020 -0300
16470: Adds an explicit reload before every pending with_lock call.
Previous versions of rails did an implicit reload when calling with_lock,
but from 5.2 calling with_lock with unpersisted changes will raise an
exception.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 041f55947..b2324a571 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -29,7 +29,7 @@ class Arvados::V1::ContainersController < ApplicationController
end
def update
- @object.with_lock do
+ @object.reload.with_lock do
super
end
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index adfbf6042..16f8cd798 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -339,7 +339,7 @@ class Container < ArvadosModel
end
def lock
- self.with_lock do
+ self.reload.with_lock do
if self.state != Queued
raise LockFailedError.new("cannot lock when #{self.state}")
end
@@ -357,7 +357,7 @@ class Container < ArvadosModel
end
def unlock
- self.with_lock do
+ self.reload.with_lock do
if self.state != Locked
raise InvalidStateTransitionError.new("cannot unlock when #{self.state}")
end
@@ -654,7 +654,7 @@ class Container < ArvadosModel
# This container is finished so finalize any associated container requests
# that are associated with this container.
if saved_change_to_state? and self.final?
- # These get wiped out by with_lock (which reloads the record),
+ # These get wiped out (with_lock requires to explicitly reload the record),
# so record them now in case we need to schedule a retry.
prev_secret_mounts = secret_mounts_before_last_save
prev_runtime_token = runtime_token_before_last_save
@@ -665,7 +665,7 @@ class Container < ArvadosModel
# transaction finishes. This ensure that concurrent container
# requests that try to reuse this container are finalized (on
# Complete) or don't reuse it (on Cancelled).
- self.with_lock do
+ self.reload.with_lock do
act_as_system_user do
if self.state == Cancelled
retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid)
@@ -690,7 +690,7 @@ class Container < ArvadosModel
}
c = Container.create! c_attrs
retryable_requests.each do |cr|
- cr.with_lock do
+ cr.reload.with_lock do
leave_modified_by_user_alone do
# Use row locking because this increments container_count
cr.container_uuid = c.uuid
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 37e5f455d..31179f066 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -136,7 +136,7 @@ class Job < ArvadosModel
end
def lock locked_by_uuid
- with_lock do
+ reload.with_lock do
unless self.state == Queued and self.is_locked_by_uuid.nil?
raise AlreadyLockedError
end
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index addea8306..dbfdae95a 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -515,7 +515,7 @@ class CollectionTest < ActiveSupport::TestCase
c2.description = 'foo collection'
c1.save!
assert_equal 1, c2.version
- # with_lock forces a reload, so this shouldn't produce an unique violation error
+ # with_lock requires a reload, so this shouldn't produce an unique violation error
c2.save!
assert_equal 3, c2.version
assert_equal 'foo collection', c2.description
commit d30553c54fde373f5ae4092ee915fb71354631bd
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Tue Jul 21 11:18:24 2020 -0300
16470: Removes cleanup code on test. All threads share a db connection.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index a1c8ff8a9..016a0e4eb 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -378,19 +378,6 @@ class LogTest < ActiveSupport::TestCase
sleep 0.1
end
assert_operator remaining_audit_logs.count, :<, initial_log_count
- ensure
- # The test framework rolls back our transactions, but that
- # doesn't undo the deletes we did from separate threads.
- ActiveRecord::Base.connection.exec_query 'ROLLBACK'
- Thread.new do
- begin
- dc = DatabaseController.new
- dc.define_singleton_method :render do |*args| end
- dc.reset
- ensure
- ActiveRecord::Base.connection.close
- end
- end.join
end
end
end
commit 73ce25d0e256e01f451ab8788175bbfe8a3e39b0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Tue Jul 21 10:22:41 2020 -0300
16470: Removes unnecessary gem that may also cause issues.
The gem doesn't seem to be used anywhere and may not be needed
anymore.
See: https://github.com/rails/rails/issues/28197#issuecomment-291120834
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 fddb64a28..844b789af 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -9,7 +9,6 @@ gem 'responders', '~> 2.0'
group :test, :development do
gem 'factory_bot_rails'
- gem 'database_cleaner'
# As of now (2019-03-27) There's an open issue about incompatibilities with
# newer versions of this gem: https://github.com/rails/rails-perftest/issues/38
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index a2b787e37..90306bb58 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -92,7 +92,6 @@ GEM
net-ssh-gateway (>= 1.1.0)
concurrent-ruby (1.1.6)
crass (1.0.6)
- database_cleaner (1.7.0)
erubi (1.9.0)
execjs (2.7.0)
extlib (0.9.16)
@@ -283,7 +282,6 @@ DEPENDENCIES
andand
arvados!
byebug
- database_cleaner
factory_bot_rails
httpclient
jquery-rails
@@ -317,4 +315,4 @@ DEPENDENCIES
uglifier (~> 2.0)
BUNDLED WITH
- 1.16.6
+ 1.17.3
commit 77a228e9e20aa0e9b804f5ae39f37b6687230767
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Jul 20 11:40:38 2020 -0300
16470: Fixes reload() API to match the overridden function.
reload()'s documentation states: "...in addition to the in-place modification
the method returns self for convenience."
https://api.rubyonrails.org/v5.1.7/classes/ActiveRecord/Persistence.html#method-i-reload
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 01a31adb9..80ea0c0b7 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -137,6 +137,7 @@ class ArvadosModel < ApplicationRecord
def reload(*args)
super
log_start_state
+ self
end
def self.create raw_params={}, *args
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 996981dba..8f724e1f5 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -262,8 +262,7 @@ class Collection < ArvadosModel
# Put aside the changes because with_lock requires an explicit record reload
changes = self.changes
snapshot = nil
- reload
- with_lock do
+ reload.with_lock do
# Copy the original state to save it as old version
if should_preserve_version
snapshot = self.dup
commit 28e65cc5137a4e0f0a50b7c221bc1715524ac958
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Jul 20 11:36:06 2020 -0300
16470: Avoids DB connection closing when running tests on missing places.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/lib/audit_logs.rb b/services/api/lib/audit_logs.rb
index 886c88738..2b5e3b8ab 100644
--- a/services/api/lib/audit_logs.rb
+++ b/services/api/lib/audit_logs.rb
@@ -62,7 +62,12 @@ module AuditLogs
rescue => e
Rails.logger.error "#{e.class}: #{e}\n#{e.backtrace.join("\n\t")}"
ensure
- ActiveRecord::Base.connection.close
+ # Rails 5.1+ makes test threads share a database connection, so we can't
+ # close a connection shared with other threads.
+ # https://github.com/rails/rails/commit/deba47799ff905f778e0c98a015789a1327d5087
+ if Rails.env != "test"
+ ActiveRecord::Base.connection.close
+ end
end
end
end
diff --git a/services/api/lib/sweep_trashed_objects.rb b/services/api/lib/sweep_trashed_objects.rb
index 8613c749c..c09896567 100644
--- a/services/api/lib/sweep_trashed_objects.rb
+++ b/services/api/lib/sweep_trashed_objects.rb
@@ -69,7 +69,12 @@ module SweepTrashedObjects
rescue => e
Rails.logger.error "#{e.class}: #{e}\n#{e.backtrace.join("\n\t")}"
ensure
- ActiveRecord::Base.connection.close
+ # Rails 5.1+ makes test threads share a database connection, so we can't
+ # close a connection shared with other threads.
+ # https://github.com/rails/rails/commit/deba47799ff905f778e0c98a015789a1327d5087
+ if Rails.env != "test"
+ ActiveRecord::Base.connection.close
+ end
end
end
end
commit abe743a549cbba49ac8743e513d2c49d97371eec
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Jul 20 11:35:00 2020 -0300
16470: Fixes internal API usage.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/lib/update_priority.rb b/services/api/lib/update_priority.rb
index 3a65c71fe..6c17f1bd0 100644
--- a/services/api/lib/update_priority.rb
+++ b/services/api/lib/update_priority.rb
@@ -33,7 +33,7 @@ module UpdatePriority
# priority==0 but should be >0:
act_as_system_user do
Container.
- joins("JOIN container_requests ON container_requests.container_uuid=containers.uuid AND container_requests.state=#{Container.sanitize(ContainerRequest::Committed)} AND container_requests.priority>0").
+ joins("JOIN container_requests ON container_requests.container_uuid=containers.uuid AND container_requests.state=#{ActiveRecord::Base.connection.quote(ContainerRequest::Committed)} AND container_requests.priority>0").
where('containers.state IN (?) AND containers.priority=0 AND container_requests.uuid IS NOT NULL',
[Container::Queued, Container::Locked, Container::Running]).
map(&:update_priority!)
commit 01190b55cafa04ca0b73bf49fcb30f048fb934bf
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Jul 15 11:14:09 2020 -0300
16470: Fixes tests. Avoids closing a DB connection when using threads on tests.
From Rails 5.1, test threads share a database connection, so UpdatePriority
won't try to close DB connections when running from tests.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/lib/update_priority.rb b/services/api/lib/update_priority.rb
index c688ac008..3a65c71fe 100644
--- a/services/api/lib/update_priority.rb
+++ b/services/api/lib/update_priority.rb
@@ -55,7 +55,12 @@ module UpdatePriority
rescue => e
Rails.logger.error "#{e.class}: #{e}\n#{e.backtrace.join("\n\t")}"
ensure
- ActiveRecord::Base.connection.close
+ # Rails 5.1+ makes test threads share a database connection, so we can't
+ # close a connection shared with other threads.
+ # https://github.com/rails/rails/commit/deba47799ff905f778e0c98a015789a1327d5087
+ if Rails.env != "test"
+ ActiveRecord::Base.connection.close
+ end
end
end
end
diff --git a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
index ce1d447f1..0fbc7625c 100644
--- a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
@@ -50,8 +50,7 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
refute_empty expect_rvz
authorize_with :active
get :index,
- params: {:format => :json},
- headers: auth(:active)
+ params: {:format => :json}
assert_response :success
json_response['items'].each do |svc|
url = "#{svc['service_ssl_flag'] ? 'https' : 'http'}://#{svc['service_host']}:#{svc['service_port']}/"
commit b1b062e241839ede51223f90f8e12d8222414df8
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Jul 6 19:07:29 2020 -0300
16470: Fixes deprecation warnings on unit tests.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 6057c4d26..a4d49c35c 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -325,6 +325,7 @@ class ApiClientAuthorization < ArvadosModel
end
def log_update
- super unless (changed - UNLOGGED_CHANGES).empty?
+
+ super unless (saved_changes.keys - UNLOGGED_CHANGES).empty?
end
end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index caac5611e..996981dba 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -259,9 +259,10 @@ class Collection < ArvadosModel
should_preserve_version = should_preserve_version? # Time sensitive, cache value
return(yield) unless (should_preserve_version || syncable_updates.any?)
- # Put aside the changes because with_lock forces a record reload
+ # Put aside the changes because with_lock requires an explicit record reload
changes = self.changes
snapshot = nil
+ reload
with_lock do
# Copy the original state to save it as old version
if should_preserve_version
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 912a801a6..adfbf6042 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -138,7 +138,7 @@ class Container < ArvadosModel
end
def propagate_priority
- return true unless priority_changed?
+ return true unless saved_change_to_priority?
act_as_system_user do
# Update the priority of child container requests to match new
# priority of the parent container (ignoring requests with no
@@ -556,7 +556,7 @@ class Container < ArvadosModel
# If self.final?, this update is superfluous: the final log/output
# update will be done when handle_completed calls finalize! on
# each requesting CR.
- return if self.final? || !self.log_changed?
+ return if self.final? || !saved_change_to_log?
leave_modified_by_user_alone do
ContainerRequest.where(container_uuid: self.uuid).each do |cr|
cr.update_collections(container: self, collections: ['log'])
@@ -653,11 +653,11 @@ class Container < ArvadosModel
def handle_completed
# This container is finished so finalize any associated container requests
# that are associated with this container.
- if self.state_changed? and self.final?
+ if saved_change_to_state? and self.final?
# These get wiped out by with_lock (which reloads the record),
# so record them now in case we need to schedule a retry.
- prev_secret_mounts = self.secret_mounts_was
- prev_runtime_token = self.runtime_token_was
+ prev_secret_mounts = secret_mounts_before_last_save
+ prev_runtime_token = runtime_token_before_last_save
# Need to take a lock on the container to ensure that any
# concurrent container requests that might try to reuse this
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index b30b8cc1d..77536eee4 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -472,10 +472,10 @@ class ContainerRequest < ArvadosModel
end
def update_priority
- return unless state_changed? || priority_changed? || container_uuid_changed?
+ return unless saved_change_to_state? || saved_change_to_priority? || saved_change_to_container_uuid?
act_as_system_user do
Container.
- where('uuid in (?)', [self.container_uuid_was, self.container_uuid].compact).
+ where('uuid in (?)', [container_uuid_before_last_save, self.container_uuid].compact).
map(&:update_priority!)
end
end
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 02c6a242f..7e015f356 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -57,7 +57,7 @@ class Group < ArvadosModel
end
def update_trash
- if trash_at_changed? or owner_uuid_changed?
+ if saved_change_to_trash_at? or saved_change_to_owner_uuid?
# The group was added or removed from the trash.
#
# Strategy:
@@ -97,7 +97,7 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
end
def after_ownership_change
- if owner_uuid_changed?
+ if saved_change_to_owner_uuid?
update_permissions self.owner_uuid, self.uuid, CAN_MANAGE_PERM
end
end
diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index d200bb801..c8b463696 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -168,7 +168,7 @@ class Node < ArvadosModel
end
def dns_server_update
- if ip_address_changed? && ip_address
+ if saved_change_to_ip_address? && ip_address
Node.where('id != ? and ip_address = ?',
id, ip_address).each do |stale_node|
# One or more(!) stale node records have the same IP address
@@ -178,10 +178,10 @@ class Node < ArvadosModel
stale_node.update_attributes!(ip_address: nil)
end
end
- if hostname_was && hostname_changed?
- self.class.dns_server_update(hostname_was, UNUSED_NODE_IP)
+ if hostname_before_last_save && saved_change_to_hostname?
+ self.class.dns_server_update(hostname_before_last_save, UNUSED_NODE_IP)
end
- if hostname && (hostname_changed? || ip_address_changed?)
+ if hostname && (saved_change_to_hostname? || saved_change_to_ip_address?)
self.class.dns_server_update(hostname, ip_address || UNUSED_NODE_IP)
end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 64facaa98..e1fd53e3d 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -23,32 +23,32 @@ class User < ArvadosModel
validate :must_unsetup_to_deactivate
before_update :prevent_privilege_escalation
before_update :prevent_inactive_admin
- before_update :verify_repositories_empty, :if => Proc.new { |user|
- user.username.nil? and user.username_changed?
+ before_update :verify_repositories_empty, :if => Proc.new {
+ username.nil? and username_changed?
}
before_update :setup_on_activate
before_create :check_auto_admin
- before_create :set_initial_username, :if => Proc.new { |user|
- user.username.nil? and user.email
+ before_create :set_initial_username, :if => Proc.new {
+ username.nil? and email
}
after_create :after_ownership_change
after_create :setup_on_activate
after_create :add_system_group_permission_link
- after_create :auto_setup_new_user, :if => Proc.new { |user|
+ after_create :auto_setup_new_user, :if => Proc.new {
Rails.configuration.Users.AutoSetupNewUsers and
- (user.uuid != system_user_uuid) and
- (user.uuid != anonymous_user_uuid)
+ (uuid != system_user_uuid) and
+ (uuid != anonymous_user_uuid)
}
after_create :send_admin_notifications
before_update :before_ownership_change
after_update :after_ownership_change
after_update :send_profile_created_notification
- after_update :sync_repository_names, :if => Proc.new { |user|
- (user.uuid != system_user_uuid) and
- user.username_changed? and
- (not user.username_was.nil?)
+ after_update :sync_repository_names, :if => Proc.new {
+ (uuid != system_user_uuid) and
+ saved_change_to_username? and
+ (not username_before_last_save.nil?)
}
before_destroy :clear_permissions
after_destroy :remove_self_from_permissions
@@ -151,7 +151,7 @@ SELECT 1 FROM #{PERMISSION_VIEW}
end
def after_ownership_change
- if owner_uuid_changed?
+ if saved_change_to_owner_uuid?
update_permissions self.owner_uuid, self.uuid, CAN_MANAGE_PERM
end
end
@@ -743,7 +743,7 @@ 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? || is_active_changed?)
+ if is_active && (new_record? || saved_change_to_is_active?)
setup
end
end
@@ -766,8 +766,8 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
# Send notification if the user saved profile for the first time
def send_profile_created_notification
- if self.prefs_changed?
- if self.prefs_was.andand.empty? || !self.prefs_was.andand['profile']
+ if saved_change_to_prefs?
+ if prefs_before_last_save.andand.empty? || !prefs_before_last_save.andand['profile']
profile_notification_address = Rails.configuration.Users.UserProfileNotificationAddress
ProfileNotifier.profile_created(self, profile_notification_address).deliver_now if profile_notification_address and !profile_notification_address.empty?
end
@@ -782,7 +782,7 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
end
def sync_repository_names
- old_name_re = /^#{Regexp.escape(username_was)}\//
+ old_name_re = /^#{Regexp.escape(username_before_last_save)}\//
name_sub = "#{username}/"
repositories.find_each do |repo|
repo.name = repo.name.sub(old_name_re, name_sub)
commit 46dbe751057150bf47a688ad94500429d45182f6
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Jul 6 16:44:19 2020 -0300
16470: Updates Rails API to 5.1.7
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 18797d69c..fddb64a28 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -4,7 +4,7 @@
source 'https://rubygems.org'
-gem 'rails', '~> 5.0.0'
+gem 'rails', '5.1.7'
gem 'responders', '~> 2.0'
group :test, :development do
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 127a09ee2..a2b787e37 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -22,39 +22,39 @@ GIT
GEM
remote: https://rubygems.org/
specs:
- actioncable (5.0.7.2)
- actionpack (= 5.0.7.2)
- nio4r (>= 1.2, < 3.0)
+ actioncable (5.1.7)
+ actionpack (= 5.1.7)
+ nio4r (~> 2.0)
websocket-driver (~> 0.6.1)
- actionmailer (5.0.7.2)
- actionpack (= 5.0.7.2)
- actionview (= 5.0.7.2)
- activejob (= 5.0.7.2)
+ actionmailer (5.1.7)
+ actionpack (= 5.1.7)
+ actionview (= 5.1.7)
+ activejob (= 5.1.7)
mail (~> 2.5, >= 2.5.4)
rails-dom-testing (~> 2.0)
- actionpack (5.0.7.2)
- actionview (= 5.0.7.2)
- activesupport (= 5.0.7.2)
+ actionpack (5.1.7)
+ actionview (= 5.1.7)
+ activesupport (= 5.1.7)
rack (~> 2.0)
- rack-test (~> 0.6.3)
+ rack-test (>= 0.6.3)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.0.2)
- actionview (5.0.7.2)
- activesupport (= 5.0.7.2)
+ actionview (5.1.7)
+ activesupport (= 5.1.7)
builder (~> 3.1)
- erubis (~> 2.7.0)
+ erubi (~> 1.4)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.0.3)
- activejob (5.0.7.2)
- activesupport (= 5.0.7.2)
+ activejob (5.1.7)
+ activesupport (= 5.1.7)
globalid (>= 0.3.6)
- activemodel (5.0.7.2)
- activesupport (= 5.0.7.2)
- activerecord (5.0.7.2)
- activemodel (= 5.0.7.2)
- activesupport (= 5.0.7.2)
- arel (~> 7.0)
- activesupport (5.0.7.2)
+ 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)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
@@ -66,9 +66,9 @@ GEM
addressable (2.7.0)
public_suffix (>= 2.0.2, < 5.0)
andand (1.3.3)
- arel (7.1.4)
- arvados-google-api-client (0.8.7.3)
- activesupport (>= 3.2, < 5.1)
+ arel (8.0.0)
+ arvados-google-api-client (0.8.7.4)
+ activesupport (>= 3.2, < 5.3)
addressable (~> 2.3)
autoparse (~> 0.3)
extlib (~> 0.9)
@@ -82,7 +82,7 @@ GEM
addressable (>= 2.3.1)
extlib (>= 0.9.15)
multi_json (>= 1.0.0)
- builder (3.2.3)
+ builder (3.2.4)
byebug (11.0.1)
capistrano (2.15.9)
highline
@@ -90,10 +90,10 @@ GEM
net-sftp (>= 2.0.0)
net-ssh (>= 2.0.14)
net-ssh-gateway (>= 1.1.0)
- concurrent-ruby (1.1.5)
- crass (1.0.4)
+ concurrent-ruby (1.1.6)
+ crass (1.0.6)
database_cleaner (1.7.0)
- erubis (2.7.0)
+ erubi (1.9.0)
execjs (2.7.0)
extlib (0.9.16)
factory_bot (5.0.2)
@@ -133,15 +133,15 @@ GEM
railties (>= 4)
request_store (~> 1.0)
logstash-event (1.2.02)
- loofah (2.2.3)
+ loofah (2.6.0)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.1)
mini_mime (>= 0.1.1)
memoist (0.16.2)
metaclass (0.0.4)
- method_source (0.9.2)
- mini_mime (1.0.1)
+ method_source (1.0.0)
+ mini_mime (1.0.2)
mini_portile2 (2.4.0)
minitest (5.10.3)
mocha (1.8.0)
@@ -156,8 +156,8 @@ GEM
net-ssh (5.2.0)
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
- nio4r (2.3.1)
- nokogiri (1.10.8)
+ nio4r (2.5.2)
+ nokogiri (1.10.10)
mini_portile2 (~> 2.4.0)
oauth2 (1.4.1)
faraday (>= 0.8, < 0.16.0)
@@ -181,19 +181,19 @@ GEM
power_assert (1.1.4)
public_suffix (4.0.3)
rack (2.2.3)
- rack-test (0.6.3)
- rack (>= 1.0)
- rails (5.0.7.2)
- actioncable (= 5.0.7.2)
- actionmailer (= 5.0.7.2)
- actionpack (= 5.0.7.2)
- actionview (= 5.0.7.2)
- activejob (= 5.0.7.2)
- activemodel (= 5.0.7.2)
- activerecord (= 5.0.7.2)
- activesupport (= 5.0.7.2)
+ 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)
bundler (>= 1.3.0)
- railties (= 5.0.7.2)
+ railties (= 5.1.7)
sprockets-rails (>= 2.0.0)
rails-controller-testing (1.0.4)
actionpack (>= 5.0.1.x)
@@ -202,14 +202,14 @@ GEM
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
nokogiri (>= 1.6)
- rails-html-sanitizer (1.0.4)
- loofah (~> 2.2, >= 2.2.2)
+ rails-html-sanitizer (1.3.0)
+ loofah (~> 2.3)
rails-observers (0.1.5)
activemodel (>= 4.0)
rails-perftest (0.0.7)
- railties (5.0.7.2)
- actionpack (= 5.0.7.2)
- activesupport (= 5.0.7.2)
+ railties (5.1.7)
+ actionpack (= 5.1.7)
+ activesupport (= 5.1.7)
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
@@ -263,10 +263,10 @@ GEM
therubyracer (0.12.3)
libv8 (~> 3.16.14.15)
ref
- thor (0.20.3)
+ thor (1.0.1)
thread_safe (0.3.6)
tilt (2.0.8)
- tzinfo (1.2.6)
+ tzinfo (1.2.7)
thread_safe (~> 0.1)
uglifier (2.7.2)
execjs (>= 0.3.0)
@@ -298,7 +298,7 @@ DEPENDENCIES
optimist
passenger
pg (~> 1.0)
- rails (~> 5.0.0)
+ rails (= 5.1.7)
rails-controller-testing
rails-observers
rails-perftest
commit bf60fd73bbbe5c75cdcfe9d57a85b68dc7916555
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Jul 1 18:42:41 2020 -0300
16470: Bumps dependency on activesupport for rails upgrade.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/cli/arvados-cli.gemspec b/sdk/cli/arvados-cli.gemspec
index 88a5ceece..f60adf538 100644
--- a/sdk/cli/arvados-cli.gemspec
+++ b/sdk/cli/arvados-cli.gemspec
@@ -31,7 +31,7 @@ Gem::Specification.new do |s|
s.summary = "Arvados CLI tools"
s.description = "Arvados command line tools, git commit #{git_hash}"
s.authors = ["Arvados Authors"]
- s.email = 'gem-dev at curoverse.com'
+ s.email = 'gem-dev at arvados.org'
#s.bindir = '.'
s.licenses = ['Apache-2.0']
s.files = ["bin/arv", "bin/arv-tag", "LICENSE-2.0.txt"]
@@ -42,7 +42,7 @@ Gem::Specification.new do |s|
# Our google-api-client dependency used to be < 0.9, but that could be
# satisfied by the buggy 0.9.pre*. https://dev.arvados.org/issues/9213
s.add_runtime_dependency 'arvados-google-api-client', '~> 0.6', '>= 0.6.3', '<0.8.9'
- s.add_runtime_dependency 'activesupport', '>= 3.2.13', '< 5.1'
+ s.add_runtime_dependency 'activesupport', '>= 3.2.13', '< 5.3'
s.add_runtime_dependency 'json', '>= 1.7.7', '<3'
s.add_runtime_dependency 'optimist', '~> 3.0'
s.add_runtime_dependency 'andand', '~> 1.3', '>= 1.3.3'
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list