[ARVADOS] updated: 8d9b12f2a87ffd7183d3a36ca32ee1c7e701a0e2
Git user
git at public.curoverse.com
Mon Mar 27 14:12:56 EDT 2017
Summary of changes:
doc/_config.yml | 1 +
.../install-compute-node.html.textile.liquid | 2 -
.../install-dispatch.html.textile.liquid | 15 ++
doc/install/install-api-server.html.textile.liquid | 86 ++--------
doc/install/install-ws.html.textile.liquid | 12 +-
docker/migrate-docker19/dnd.sh | 14 +-
docker/migrate-docker19/migrate.sh | 39 +++--
sdk/cli/test/test_arv-collection-create.rb | 1 +
sdk/go/arvadosclient/arvadosclient_test.go | 7 +-
sdk/python/arvados/api.py | 3 +-
sdk/python/arvados/cache.py | 71 +++++++++
sdk/python/arvados/commands/migrate19.py | 34 ++--
sdk/python/setup.py | 2 +-
sdk/python/tests/run_test_server.py | 19 ++-
sdk/python/tests/test_cache.py | 82 ++++++++++
sdk/ruby/lib/arvados.rb | 23 +--
sdk/ruby/lib/arvados/keep.rb | 6 +-
services/api/Gemfile.lock | 4 +-
services/api/Rakefile | 10 ++
.../app/controllers/arvados/v1/users_controller.rb | 2 +-
.../api/app/controllers/database_controller.rb | 4 +-
.../api/app/models/api_client_authorization.rb | 8 +-
services/api/app/models/arvados_model.rb | 82 +++++-----
services/api/app/models/log.rb | 4 +-
services/api/config/application.default.yml | 18 +++
services/api/config/application.rb | 22 ++-
.../api/config/initializers/noop_deep_munge.rb | 1 +
services/api/config/initializers/time_format.rb | 2 +
services/api/lib/audit_logs.rb | 65 ++++++++
services/api/lib/crunch_dispatch.rb | 11 +-
services/api/lib/load_param.rb | 2 +-
services/api/test/fixtures/logs.yml | 4 +
.../api_client_authorizations_scopes_test.rb | 2 +
.../api/test/integration/database_reset_test.rb | 2 -
services/api/test/test_helper.rb | 15 +-
services/api/test/unit/crunch_dispatch_test.rb | 8 +-
services/api/test/unit/fail_jobs_test.rb | 8 +-
services/api/test/unit/log_test.rb | 78 +++++++++
services/arv-git-httpd/arvados-git-httpd.service | 5 +
.../crunch-dispatch-slurm.service | 5 +
services/crunch-run/crunchrun.go | 176 ++++++++++++++++++---
services/crunch-run/crunchrun_test.go | 76 ++++++++-
.../dockercleaner/arvados-docker-cleaner.service | 4 +
services/keep-balance/keep-balance.service | 4 +
services/keep-web/keep-web.service | 5 +
services/keepproxy/keepproxy.service | 5 +
services/keepstore/keepstore.service | 5 +
.../arvnodeman/computenode/dispatch/__init__.py | 4 -
services/nodemanager/arvnodeman/daemon.py | 31 ++--
services/nodemanager/tests/test_daemon.py | 21 ---
services/ws/arvados-ws.service | 5 +
51 files changed, 851 insertions(+), 264 deletions(-)
create mode 100644 sdk/python/arvados/cache.py
create mode 100644 sdk/python/tests/test_cache.py
create mode 100644 services/api/lib/audit_logs.rb
discards 5bc1b840c803ff725c480e21380b3dac4856df6d (commit)
discards 94068b9bbf6fd1ccc779d67f58dbdc16ab0fe209 (commit)
discards 772511f3f6a7e35704bfd351abfb76d3bba191d5 (commit)
discards bb17c578233a311a0c80e21e649ed87b5c07ef94 (commit)
discards b4951aa7a4fa795e18337f337fc249508d38fc98 (commit)
discards 5c304fbf1bfc270e8b60ab9014db30cb6b25c7e4 (commit)
discards 8053d3b0648a74f32fd25d8e404fa8260053e741 (commit)
discards 4f81fec4a9355988f5b758b25528457b9693c8d8 (commit)
discards b9dcb648165b158f5721500caed649a49715cd14 (commit)
discards 54d7045bc22a2e4314745ea732a3922ea1a40e97 (commit)
discards 9cdf7ca4ffb19dc33f37fc754cb1b73dcd29de11 (commit)
discards 6c49531f860d30ef50d08846c6869aef71dba0cd (commit)
discards 3a022c4f733ed7325106a03cd9718c1274e9ed8e (commit)
via 8d9b12f2a87ffd7183d3a36ca32ee1c7e701a0e2 (commit)
via d42ec212e992a83f8e7fc48b59fb3daf58a62787 (commit)
via c066a2e6d064a270638baea8f8b0d106f5903e0f (commit)
via c36272a5f83ba70980160c4cb205bdfb8a1c660b (commit)
via 045bace65c2395b6efe9f3d8c93bec74196f58e1 (commit)
via 7c6852e1b3675b3c1de7e9792373333cb752d40d (commit)
via dfd8c4bbd6f126b90d436e9d242cd30e15e70d2e (commit)
via de4df7f80c531ab16e59ea36671a8efa9e6ff33d (commit)
via 126dd750f48654cb3b1a4e53c5b7d337003e112f (commit)
via 30dbddd3b311653652fa731dbff950aba0712301 (commit)
via ab314b9ea3618c71556a6a5f6dd7c769beaf2737 (commit)
via 390af6a13f7c8974329aecc2f23fbfa81f8e298b (commit)
via 9090c60b28de593b8bb2ce606a9ab35b62b57608 (commit)
via 84ad215752fde4291070143411a945fa7a94241c (commit)
via c9a361f7fd3b1cf7f4959e9b0292d0f495d82771 (commit)
via 05d453ec38b10a022ea6db77867957e7115b9b35 (commit)
via 09dcf71e59907c2eaf4b94918c63da07193481a4 (commit)
via 099a8c62fcb0905855ddf243a3deddc7398c3c10 (commit)
via 13c47e13840caa688711a30a80a35637714e034a (commit)
via ce4970b9546b586c8caa8416ea13dab8c4866d59 (commit)
via d31988b6da2d779a8f3ba4ba3c40760f9729c59f (commit)
via 727c1e475afcb1a8fda1f9b689bb3e941ae93cec (commit)
via ae8aaa4c55762222c837fcce8e9ad6800ff8b128 (commit)
via b8000c3cb38b77c5c429e0fd591a43f5eeee64d1 (commit)
via 2e32ef1657b439c0398e66930c3d17437032fb1a (commit)
via 8ada36c931712304c4b2c70bdcbc316b1ad2c4e2 (commit)
via 4600343d1bff7ac4f7b9f08486541444c31af8b6 (commit)
via fd7767148061d21b77bd97a8856191d6c9bbc077 (commit)
via 6722b420effcab24dffd9b47fa277f8830bd4cca (commit)
via a8378b8deaa2bbf9d2c154d9d9bb072538c288cc (commit)
via 2aef6ca08d80c0fd25d74ddb9ab52cf535a33d3e (commit)
via 5aeebd0cdbabb1cfc815b21b26fb87622b05f6fe (commit)
via e8cc0d73309236d6efc243371969f83808e42d30 (commit)
via 1667f9860de21d29bbe32bb827db29eca62d9aeb (commit)
via c56743e301b49163a56482c13e49a01c9a0fd7dc (commit)
via ae970cb115251915c0a8e1052b23acdd2ab70fee (commit)
via a67faeeb159323d35d2a3229c7b5d014dc175767 (commit)
via 82ff0337a99b7aaed626a624633b8c068dc5e142 (commit)
via 950ae9635334cd1ca6a2738b185f6481cc3d771f (commit)
via 83203f5c739ee0b0199e76babccb60e832a0de8e (commit)
via 7772dca4096e64b1033fb9f42e604699ce07f782 (commit)
via 514cd364c3cb27b633c1368cd06d6a54927c98a8 (commit)
via 366780077def317818759e999263577d8a9f5064 (commit)
via e47fe8663303df51081a77646e061b11aa892df1 (commit)
via f0a34ecb9b41b7d4e35575803ba3dc283f369724 (commit)
via 8d2dd8003b6e865033e372b5db76fc2244378964 (commit)
via dc6c3fccb583ae98eee808addb526c45ebdbf2c6 (commit)
via bc8a4a8863f147e4c0b95d0aa3e2f3e549637ee4 (commit)
via 262d1e44ddaf3f014423a4acf96a3a6200279820 (commit)
via a54e88868ac259443e2cd8d5f6fddb4b8154acb9 (commit)
via 432e71aea50074b1674e96fb1a03cf512952ab75 (commit)
via 6b2005a2fa3e7c040f8db222010fa870f87336bf (commit)
via 5976c751048100e813edd638632927794df072f2 (commit)
via b50e323ec94e9c7355c6f1c5b93f488540d08ec3 (commit)
via 49510014c88646ef4320b137240966ba8f6d5108 (commit)
via 5689f1de6e1a263237303b73013dcebb4ad77e1f (commit)
via 659b49449d313cdb218a46ca36ac124674a4d00b (commit)
via 2fd606b328508babad9af6c0a30c159568b525c2 (commit)
via 2741b54c38ed1e32cc9f0129614a00d84f51bca8 (commit)
via 8b278cdd80f8969a954a2c789281f8f63195e894 (commit)
via 077878d94771c25c25edfe01a98a523898916d9e (commit)
via 05c97120decfeeb9ba2faa20561ee1f37c8293ba (commit)
via bbe8547e183a66fd551adffe59fbd4f5c146677c (commit)
via 37e995fb7423b2b353c599e2a1b00bda7c29ee6f (commit)
via 79e53c0eed77396cb37f60b48be0c60fe7e0ab89 (commit)
via b77893f2a8ae755f22615054f2c267d990995e1c (commit)
via f31475dfeb37c0e4d6b5244cba3bbd06e323b8e8 (commit)
via b30e81ee46c196476a03bc244fbc4419573ba34e (commit)
via 9f34ff6f6a6ea1d384528a093bad1c72bd2cea89 (commit)
via 386faadf691e444b71d6c96e7c00792d9a0ba2c7 (commit)
via 8513e042b0033599146546bd3a2ad903c67c9ff5 (commit)
via 1230d8a106c5c62edcbb9fcf6d1b94585e5596b2 (commit)
via 273a233818ae39e843fab0276f9e381da6645d28 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (5bc1b840c803ff725c480e21380b3dac4856df6d)
\
N -- N -- N (8d9b12f2a87ffd7183d3a36ca32ee1c7e701a0e2)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 8d9b12f2a87ffd7183d3a36ca32ee1c7e701a0e2
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Mar 27 14:12:40 2017 -0400
7709: Fix Ruby warnings.
diff --git a/sdk/ruby/lib/arvados/keep.rb b/sdk/ruby/lib/arvados/keep.rb
index 489eeee..00f4f36 100644
--- a/sdk/ruby/lib/arvados/keep.rb
+++ b/sdk/ruby/lib/arvados/keep.rb
@@ -35,7 +35,7 @@ module Keep
def self.parse(tok)
begin
Locator.parse!(tok)
- rescue ArgumentError => e
+ rescue ArgumentError
nil
end
end
@@ -112,7 +112,7 @@ module Keep
stream_name = nil
block_tokens = []
file_tokens = []
- line.scan /\S+/ do |token|
+ line.scan(/\S+/) do |token|
if stream_name.nil?
stream_name = unescape token
elsif file_tokens.empty? and Locator.valid? token
@@ -152,7 +152,7 @@ module Keep
@text.each_line do |line|
stream_name = nil
in_file_tokens = false
- line.scan /\S+/ do |token|
+ line.scan(/\S+/) do |token|
if stream_name.nil?
stream_name = unescape token
elsif in_file_tokens or not Locator.valid? token
commit d42ec212e992a83f8e7fc48b59fb3daf58a62787
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Mar 27 13:51:04 2017 -0400
7709: Fix warning while suppressing warnings.
diff --git a/sdk/ruby/lib/arvados.rb b/sdk/ruby/lib/arvados.rb
index 7a3f4b4..8b04065 100644
--- a/sdk/ruby/lib/arvados.rb
+++ b/sdk/ruby/lib/arvados.rb
@@ -11,18 +11,7 @@ ActiveSupport::Inflector.inflections do |inflect|
inflect.irregular 'human', 'humans'
end
-module Kernel
- def suppress_warnings
- original_verbosity = $VERBOSE
- $VERBOSE = nil
- result = yield
- $VERBOSE = original_verbosity
- return result
- end
-end
-
class Arvados
-
class TransactionFailedError < StandardError
end
@@ -262,4 +251,16 @@ class Arvados
@attributes = j
end
end
+
+ protected
+
+ def suppress_warnings
+ original_verbosity = $VERBOSE
+ begin
+ $VERBOSE = nil
+ yield
+ ensure
+ $VERBOSE = original_verbosity
+ end
+ end
end
commit c066a2e6d064a270638baea8f8b0d106f5903e0f
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Mar 27 13:28:54 2017 -0400
7709: Suppress Ruby warnings while loading gems.
diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index 4acde0f..0d6aa1d 100644
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -3,11 +3,25 @@ require File.expand_path('../boot', __FILE__)
require 'rails/all'
require 'digest'
+module Kernel
+ def suppress_warnings
+ verbose_orig = $VERBOSE
+ begin
+ $VERBOSE = nil
+ yield
+ ensure
+ $VERBOSE = verbose_orig
+ end
+ end
+end
+
if defined?(Bundler)
- # If you precompile assets before deploying to production, use this line
- Bundler.require(*Rails.groups(:assets => %w(development test)))
- # If you want your assets lazily compiled in production, use this line
- # Bundler.require(:default, :assets, Rails.env)
+ suppress_warnings do
+ # If you precompile assets before deploying to production, use this line
+ Bundler.require(*Rails.groups(:assets => %w(development test)))
+ # If you want your assets lazily compiled in production, use this line
+ # Bundler.require(:default, :assets, Rails.env)
+ end
end
module Server
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 0919985..6d8fa9f 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -1,8 +1,14 @@
ENV["RAILS_ENV"] = "test"
unless ENV["NO_COVERAGE_TEST"]
begin
- require 'simplecov'
- require 'simplecov-rcov'
+ verbose_orig = $VERBOSE
+ begin
+ $VERBOSE = nil
+ require 'simplecov'
+ require 'simplecov-rcov'
+ ensure
+ $VERBOSE = verbose_orig
+ end
class SimpleCov::Formatter::MergedFormatter
def format(result)
SimpleCov::Formatter::HTMLFormatter.new.format(result)
commit c36272a5f83ba70980160c4cb205bdfb8a1c660b
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Mar 27 13:19:29 2017 -0400
7709: Fix some ruby warnings.
diff --git a/services/api/config/initializers/noop_deep_munge.rb b/services/api/config/initializers/noop_deep_munge.rb
index abfa7e5..b1f84e9 100644
--- a/services/api/config/initializers/noop_deep_munge.rb
+++ b/services/api/config/initializers/noop_deep_munge.rb
@@ -2,6 +2,7 @@ module ActionDispatch
class Request < Rack::Request
# This Rails method messes with valid JSON, for example turning the empty
# array [] into 'nil'. We don't want that, so turn it into a no-op.
+ remove_method :deep_munge
def deep_munge(hash)
hash
end
diff --git a/services/api/config/initializers/time_format.rb b/services/api/config/initializers/time_format.rb
index ee7c91f..b2a2448 100644
--- a/services/api/config/initializers/time_format.rb
+++ b/services/api/config/initializers/time_format.rb
@@ -1,10 +1,12 @@
class ActiveSupport::TimeWithZone
+ remove_method :as_json
def as_json *args
strftime "%Y-%m-%dT%H:%M:%S.%NZ"
end
end
class Time
+ remove_method :as_json
def as_json *args
strftime "%Y-%m-%dT%H:%M:%S.%NZ"
end
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 3e1e8b5..e3de05f 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -153,7 +153,7 @@ module LoadParam
# Any ordering columns must be selected when doing select,
# otherwise it is an SQL error, so filter out invaliding orderings.
@orders.select! { |o|
- col, dir = o.split
+ col, _ = o.split
# match select column against order array entry
@select.select { |s| col == "#{table_name}.#{s}" }.any?
}
commit 045bace65c2395b6efe9f3d8c93bec74196f58e1
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 23 16:07:36 2017 -0400
7709: Fix state leaking between test suites.
diff --git a/services/api/test/integration/database_reset_test.rb b/services/api/test/integration/database_reset_test.rb
index 029e37c..a9b64b3 100644
--- a/services/api/test/integration/database_reset_test.rb
+++ b/services/api/test/integration/database_reset_test.rb
@@ -1,8 +1,6 @@
require 'test_helper'
class DatabaseResetTest < ActionDispatch::IntegrationTest
- self.use_transactional_fixtures = false
-
slow_test "reset fails when Rails.env != 'test'" do
rails_env_was = Rails.env
begin
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 40292c2..0919985 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -50,10 +50,6 @@ class ActiveSupport::TestCase
include ArvadosTestSupport
include CurrentApiClient
- setup do
- Rails.logger.warn "\n\n#{'=' * 70}\n#{self.class}\##{method_name}\n#{'-' * 70}\n\n"
- end
-
teardown do
Thread.current[:api_client_ip_address] = nil
Thread.current[:api_client_authorization] = nil
@@ -61,6 +57,7 @@ class ActiveSupport::TestCase
Thread.current[:api_client] = nil
Thread.current[:user] = nil
restore_configuration
+ User.invalidate_permissions_cache
end
def assert_not_allowed
commit 7c6852e1b3675b3c1de7e9792373333cb752d40d
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Mar 22 10:50:13 2017 -0400
7709: Accommodate Rails3 or 4 error message.
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 8e32efe..54591d3 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -2,13 +2,14 @@ package arvadosclient
import (
"fmt"
- "git.curoverse.com/arvados.git/sdk/go/arvadostest"
- . "gopkg.in/check.v1"
"net"
"net/http"
"os"
"testing"
"time"
+
+ "git.curoverse.com/arvados.git/sdk/go/arvadostest"
+ . "gopkg.in/check.v1"
)
// Gocheck boilerplate
@@ -168,7 +169,7 @@ func (s *ServerRequiredSuite) TestErrorResponse(c *C) {
Dict{"log": Dict{"bogus_attr": "foo"}},
&getback)
c.Assert(err, ErrorMatches, "arvados API server error: .*")
- c.Assert(err, ErrorMatches, ".*unknown attribute: bogus_attr.*")
+ c.Assert(err, ErrorMatches, ".*unknown attribute(: | ')bogus_attr.*")
c.Assert(err, FitsTypeOf, APIServerError{})
c.Assert(err.(APIServerError).HttpStatusCode, Equals, 422)
}
commit dfd8c4bbd6f126b90d436e9d242cd30e15e70d2e
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Mar 22 10:49:29 2017 -0400
7709: A scope that permits GET permits HEAD too.
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index f7985a9..b0c2f31 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -61,7 +61,13 @@ class ApiClientAuthorization < ArvadosModel
end
def scopes_allow_request?(request)
- scopes_allow? [request.request_method, request.path].join(' ')
+ method = request.request_method
+ if method == 'HEAD'
+ (scopes_allow?(['HEAD', request.path].join(' ')) ||
+ scopes_allow?(['GET', request.path].join(' ')))
+ else
+ scopes_allow?([method, request.path].join(' '))
+ end
end
def logged_attributes
diff --git a/services/api/test/integration/api_client_authorizations_scopes_test.rb b/services/api/test/integration/api_client_authorizations_scopes_test.rb
index 17c7516..a1a20d4 100644
--- a/services/api/test/integration/api_client_authorizations_scopes_test.rb
+++ b/services/api/test/integration/api_client_authorizations_scopes_test.rb
@@ -29,6 +29,8 @@ class ApiTokensScopeTest < ActionDispatch::IntegrationTest
assert_response 403
get(v1_url('specimens', specimens(:owned_by_active_user).uuid), *get_args)
assert_response :success
+ head(v1_url('specimens', specimens(:owned_by_active_user).uuid), *get_args)
+ assert_response :success
get(v1_url('specimens', specimens(:owned_by_spectator).uuid), *get_args)
assert_includes(403..404, @response.status)
end
commit de4df7f80c531ab16e59ea36671a8efa9e6ff33d
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Mar 22 02:20:58 2017 -0400
7709: Fix full-text index check
PostgreSQL query from http://stackoverflow.com/a/6777904
(In Rails 4.2, ActiveRecord::Base.connection.indexes() no longer
returns enough detail for this check.)
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 67da717..d83f583 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -158,23 +158,46 @@ class ArvadosModelTest < ActiveSupport::TestCase
end
test "full text search index exists on models" do
+ indexes = {}
+ conn = ActiveRecord::Base.connection
+ conn.exec_query("SELECT i.relname as indname,
+ i.relowner as indowner,
+ idx.indrelid::regclass::text as table,
+ am.amname as indam,
+ idx.indkey,
+ ARRAY(
+ SELECT pg_get_indexdef(idx.indexrelid, k + 1, true)
+ FROM generate_subscripts(idx.indkey, 1) as k
+ ORDER BY k
+ ) as keys,
+ idx.indexprs IS NOT NULL as indexprs,
+ idx.indpred IS NOT NULL as indpred
+ FROM pg_index as idx
+ JOIN pg_class as i
+ ON i.oid = idx.indexrelid
+ JOIN pg_am as am
+ ON i.relam = am.oid
+ JOIN pg_namespace as ns
+ ON ns.oid = i.relnamespace
+ AND ns.nspname = ANY(current_schemas(false))").each do |idx|
+ if idx['keys'].match(/to_tsvector/)
+ indexes[idx['table']] ||= []
+ indexes[idx['table']] << idx
+ end
+ end
fts_tables = ["collections", "container_requests", "groups", "jobs",
"pipeline_instances", "pipeline_templates", "workflows"]
fts_tables.each do |table|
table_class = table.classify.constantize
if table_class.respond_to?('full_text_searchable_columns')
- fts_index_columns = table_class.full_text_searchable_columns
- index_columns = nil
- indexes = ActiveRecord::Base.connection.indexes(table)
- fts_index_by_columns = indexes.select do |index|
- if index.columns.first.match(/to_tsvector/)
- index_columns = index.columns.first.scan(/\((?<columns>[A-Za-z_]+)\,/).flatten!
- index_columns.sort == fts_index_columns.sort
- else
- false
+ expect = table_class.full_text_searchable_columns
+ ok = false
+ indexes[table].andand.each do |idx|
+ if expect == idx['keys'].scan(/COALESCE\(([A-Za-z_]+)/).flatten
+ ok = true
end
end
- assert !fts_index_by_columns.empty?, "#{table} has no FTS index with columns #{fts_index_columns}. Instead found FTS index with columns #{index_columns}"
+ assert ok, "#{table} has no full-text index\nexpect: #{expect.inspect}\nfound: #{indexes[table].inspect}"
end
end
end
commit 126dd750f48654cb3b1a4e53c5b7d337003e112f
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 22:56:37 2017 -0400
7709: Serialized fields reject wrong types with useful error messages.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index a5aa6a4..5fdb5f3 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -79,8 +79,17 @@ class ArvadosModel < ActiveRecord::Base
# The following permit! is necessary even with
# "ActionController::Parameters.permit_all_parameters = true",
# because permit_all does not permit nested attributes.
- if has_nonstring_keys?(raw_params)
- raise ArgumentError.new("Parameters cannot have non-string keys")
+ if raw_params
+ serialized_attributes.each do |colname, coder|
+ param = raw_params[colname.to_sym]
+ if param.nil?
+ # ok
+ elsif !param.is_a?(coder.object_class)
+ raise ArgumentError.new("#{colname} parameter must be #{coder.object_class}, not #{param.class}")
+ elsif has_nonstring_keys?(param)
+ raise ArgumentError.new("#{colname} parameter cannot have non-string hash keys")
+ end
+ end
end
ActionController::Parameters.new(raw_params).permit!
end
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 246f2d9..9241465 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -181,23 +181,30 @@ class JobTest < ActiveSupport::TestCase
{runtime_constraints: []},
{tasks_summary: ""},
{tasks_summary: []},
- {script_version: "no/branch/could/ever/possibly/have/this/name"},
].each do |invalid_attrs|
test "validation failures set error messages: #{invalid_attrs.to_json}" do
# Ensure valid_attrs doesn't produce errors -- otherwise we will
# not know whether errors reported below are actually caused by
# invalid_attrs.
- Job.create! job_attrs
+ Job.new(job_attrs).save!
- job = Job.new(job_attrs(invalid_attrs))
- assert_raises(ActiveRecord::RecordInvalid, ArgumentError, RuntimeError,
- "save! did not raise the expected exception") do
- job.save!
+ err = assert_raises(ArgumentError) do
+ Job.new(job_attrs(invalid_attrs)).save!
end
- assert_not_empty job.errors, "validation failure did not provide errors"
+ assert_match /parameters|constraints|summary/, err.message
end
end
+ test "invalid script_version" do
+ invalid = {
+ script_version: "no/branch/could/ever/possibly/have/this/name",
+ }
+ err = assert_raises(ActiveRecord::RecordInvalid) do
+ Job.new(job_attrs(invalid)).save!
+ end
+ assert_match /Script version .* does not resolve to a commit/, err.message
+ end
+
[
# Each test case is of the following format
# Array of parameters where each parameter is of the format:
commit 30dbddd3b311653652fa731dbff950aba0712301
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 16:13:56 2017 -0400
7709: Remove services/api/log/
diff --git a/services/api/.gitignore b/services/api/.gitignore
index 29eb939..da4c39d 100644
--- a/services/api/.gitignore
+++ b/services/api/.gitignore
@@ -2,8 +2,7 @@
/db/*.sqlite3
# Ignore all logfiles and tempfiles.
-/log/*.log
-/log/*.log.gz
+/log
/tmp
# Sensitive files and local configuration
diff --git a/services/api/log/.gitkeep b/services/api/log/.gitkeep
deleted file mode 100644
index e69de29..0000000
commit ab314b9ea3618c71556a6a5f6dd7c769beaf2737
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Mar 19 17:19:32 2017 -0400
7709: Remove passenger/puma websocket tests.
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 44807ec..a5c9de0 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -477,6 +477,6 @@ test:
workbench_address: https://localhost:3001/
git_repositories_dir: <%= Rails.root.join 'tmp', 'git', 'test' %>
git_internal_dir: <%= Rails.root.join 'tmp', 'internal.git' %>
- websocket_address: <% if ENV['ARVADOS_TEST_EXPERIMENTAL_WS'] %>"wss://0.0.0.0:<%= ENV['ARVADOS_TEST_WSS_PORT'] %>/websocket"<% else %>false<% end %>
+ websocket_address: "wss://0.0.0.0:<%= ENV['ARVADOS_TEST_WSS_PORT'] %>/websocket"
trash_sweep_interval: -1
docker_image_formats: ["v1"]
diff --git a/services/api/test/integration/websocket_test.rb b/services/api/test/integration/websocket_test.rb
deleted file mode 100644
index 549bbc6..0000000
--- a/services/api/test/integration/websocket_test.rb
+++ /dev/null
@@ -1,742 +0,0 @@
-require 'database_cleaner'
-require 'safe_json'
-require 'test_helper'
-
-DatabaseCleaner.strategy = :deletion
-
-class WebsocketTest < ActionDispatch::IntegrationTest
- self.use_transactional_fixtures = false
-
- setup do
- DatabaseCleaner.start
- end
-
- teardown do
- DatabaseCleaner.clean
- end
-
- def self.startup
- s = TCPServer.new('0.0.0.0', 0)
- @@port = s.addr[1]
- s.close
- @@pidfile = "tmp/pids/passenger.#{@@port}.pid"
- DatabaseCleaner.start
- Dir.chdir(Rails.root) do |apidir|
- # Only passenger seems to be able to run the websockets server
- # successfully.
- _system('passenger', 'start', '-d',
- "-p#{@@port}",
- "--log-file", "/dev/stderr",
- "--pid-file", @@pidfile)
- timeout = Time.now.tv_sec + 10
- begin
- sleep 0.2
- begin
- server_pid = IO.read(@@pidfile).to_i
- good_pid = (server_pid > 0) and (Process.kill(0, pid) rescue false)
- rescue Errno::ENOENT
- good_pid = false
- end
- end while (not good_pid) and (Time.now.tv_sec < timeout)
- if not good_pid
- raise RuntimeError, "could not find API server Rails pid"
- end
- STDERR.puts "Started websocket server on port #{@@port} with pid #{server_pid}"
- end
- end
-
- def self.shutdown
- Dir.chdir(Rails.root) do
- _system('passenger', 'stop', "-p#{@@port}",
- "--pid-file", @@pidfile)
- end
- # DatabaseCleaner leaves the database empty. Prefer to leave it full.
- dc = DatabaseController.new
- dc.define_singleton_method :render do |*args| end
- dc.reset
- end
-
- def self._system(*cmd)
- Bundler.with_clean_env do
- env = {
- 'ARVADOS_WEBSOCKETS' => 'ws-only',
- 'RAILS_ENV' => 'test',
- }
- if not system(env, *cmd)
- raise RuntimeError, "Command exited #{$?}: #{cmd.inspect}"
- end
- end
- end
-
- def ws_helper(token: nil, timeout: 8)
- opened = false
- close_status = nil
- too_long = false
-
- EM.run do
- if token
- ws = Faye::WebSocket::Client.new("ws://localhost:#{@@port}/websocket?api_token=#{api_client_authorizations(token).api_token}")
- else
- ws = Faye::WebSocket::Client.new("ws://localhost:#{@@port}/websocket")
- end
-
- ws.on :open do |event|
- opened = true
- if timeout
- EM::Timer.new(timeout) do
- too_long = true if close_status.nil?
- EM.stop_event_loop
- end
- end
- end
-
- ws.on :error do |event|
- STDERR.puts "websocket client error: #{event.inspect}"
- end
-
- ws.on :close do |event|
- close_status = [:close, event.code, event.reason]
- EM.stop_event_loop
- end
-
- yield ws
- end
-
- assert opened, "Should have opened web socket"
- assert (not too_long), "Test took too long"
- assert_equal 1000, close_status[1], "Connection closed unexpectedly (check log for errors)"
- end
-
- test "connect with no token" do
- status = nil
-
- ws_helper do |ws|
- ws.on :message do |event|
- d = SafeJSON.load event.data
- status = d["status"]
- ws.close
- end
- end
-
- assert_equal 401, status
- end
-
- test "connect, subscribe and get response" do
- status = nil
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- status = d["status"]
- ws.close
- end
- end
-
- assert_equal 200, status
- end
-
- def subscribe_test
- state = 1
- spec = nil
- ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- spec = Specimen.create
- state = 2
- when 2
- ev_uuid = d["object_uuid"]
- ws.close
- end
- end
-
- end
-
- assert_not_nil spec
- assert_equal spec.uuid, ev_uuid
- end
-
- test "connect, subscribe, get event" do
- subscribe_test()
- end
-
- test "connect, subscribe, get two events" do
- state = 1
- spec = nil
- human = nil
- spec_ev_uuid = nil
- human_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- spec = Specimen.create
- human = Human.create
- state = 2
- when 2
- spec_ev_uuid = d["object_uuid"]
- state = 3
- when 3
- human_ev_uuid = d["object_uuid"]
- state = 4
- ws.close
- when 4
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil spec
- assert_not_nil human
- assert_equal spec.uuid, spec_ev_uuid
- assert_equal human.uuid, human_ev_uuid
- end
-
- test "connect, subscribe, filter events" do
- state = 1
- human = nil
- human_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe', filters: [['object_uuid', 'is_a', 'arvados#human']]}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- Specimen.create
- human = Human.create
- state = 2
- when 2
- human_ev_uuid = d["object_uuid"]
- state = 3
- ws.close
- when 3
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil human
- assert_equal human.uuid, human_ev_uuid
- end
-
-
- test "connect, subscribe, multiple filters" do
- state = 1
- spec = nil
- human = nil
- spec_ev_uuid = nil
- human_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe', filters: [['object_uuid', 'is_a', 'arvados#human']]}.to_json)
- ws.send ({method: 'subscribe', filters: [['object_uuid', 'is_a', 'arvados#specimen']]}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- state = 2
- when 2
- assert_equal 200, d["status"]
- spec = Specimen.create
- Trait.create # not part of filters, should not be received
- human = Human.create
- state = 3
- when 3
- spec_ev_uuid = d["object_uuid"]
- state = 4
- when 4
- human_ev_uuid = d["object_uuid"]
- state = 5
- ws.close
- when 5
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil spec
- assert_not_nil human
- assert_equal spec.uuid, spec_ev_uuid
- assert_equal human.uuid, human_ev_uuid
- end
-
-
- test "connect, subscribe, compound filter" do
- state = 1
- t1 = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe', filters: [['object_uuid', 'is_a', 'arvados#trait'], ['event_type', '=', 'update']]}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- t1 = Trait.create("name" => "foo")
- t1.name = "bar"
- t1.save!
- state = 2
- when 2
- assert_equal 'update', d['event_type']
- state = 3
- ws.close
- when 3
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_equal 3, state
- assert_not_nil t1
- end
-
- test "connect, subscribe, ask events starting at seq num" do
- state = 1
-
- authorize_with :active
-
- lastid = logs(:admin_changes_specimen).id
- l1 = nil
- l2 = nil
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe', last_log_id: lastid}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- state = 2
- when 2
- l1 = d["object_uuid"]
- assert_not_nil l1, "Unexpected message: #{d}"
- state = 3
- when 3
- l2 = d["object_uuid"]
- assert_not_nil l2, "Unexpected message: #{d}"
- state = 4
- ws.close
- when 4
- assert false, "Should not get any more events"
- end
- end
- end
-
- expect_next_logs = Log.where('id > ?', lastid).order('id asc')
- assert_equal expect_next_logs[0].object_uuid, l1
- assert_equal expect_next_logs[1].object_uuid, l2
- end
-
- slow_test "connect, subscribe, get event, unsubscribe" do
- state = 1
- spec = nil
- spec_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active, timeout: false) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- EM::Timer.new 3 do
- # Set a time limit on the test because after unsubscribing the server
- # still has to process the next event (and then hopefully correctly
- # decides not to send it because we unsubscribed.)
- ws.close
- end
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- spec = Specimen.create
- state = 2
- when 2
- spec_ev_uuid = d["object_uuid"]
- ws.send ({method: 'unsubscribe'}.to_json)
-
- EM::Timer.new 1 do
- Specimen.create
- end
-
- state = 3
- when 3
- assert_equal 200, d["status"]
- state = 4
- when 4
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil spec
- assert_equal spec.uuid, spec_ev_uuid
- end
-
- slow_test "connect, subscribe, get event, unsubscribe with filter" do
- state = 1
- spec = nil
- spec_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active, timeout: false) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe', filters: [['object_uuid', 'is_a', 'arvados#human']]}.to_json)
- EM::Timer.new 6 do
- # Set a time limit on the test because after unsubscribing the server
- # still has to process the next event (and then hopefully correctly
- # decides not to send it because we unsubscribed.)
- ws.close
- end
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- spec = Human.create
- state = 2
- when 2
- spec_ev_uuid = d["object_uuid"]
- ws.send ({method: 'unsubscribe', filters: [['object_uuid', 'is_a', 'arvados#human']]}.to_json)
-
- EM::Timer.new 1 do
- Human.create
- end
-
- state = 3
- when 3
- assert_equal 200, d["status"]
- state = 4
- when 4
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil spec
- assert_equal spec.uuid, spec_ev_uuid
- end
-
-
- slow_test "connect, subscribe, get event, try to unsubscribe with bogus filter" do
- state = 1
- spec = nil
- spec_ev_uuid = nil
- human = nil
- human_ev_uuid = nil
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- spec = Specimen.create
- state = 2
- when 2
- spec_ev_uuid = d["object_uuid"]
- ws.send ({method: 'unsubscribe', filters: [['foo', 'bar', 'baz']]}.to_json)
-
- EM::Timer.new 1 do
- human = Human.create
- end
-
- state = 3
- when 3
- assert_equal 404, d["status"]
- state = 4
- when 4
- human_ev_uuid = d["object_uuid"]
- state = 5
- ws.close
- when 5
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_not_nil spec
- assert_not_nil human
- assert_equal spec.uuid, spec_ev_uuid
- assert_equal human.uuid, human_ev_uuid
- end
-
- slow_test "connected, not subscribed, no event" do
- authorize_with :active
-
- ws_helper(token: :active, timeout: false) do |ws|
- ws.on :open do |event|
- EM::Timer.new 1 do
- Specimen.create
- end
-
- EM::Timer.new 3 do
- ws.close
- end
- end
-
- ws.on :message do |event|
- assert false, "Should not get any messages, message was #{event.data}"
- end
- end
- end
-
- slow_test "connected, not authorized to see event" do
- state = 1
-
- authorize_with :admin
-
- ws_helper(token: :active, timeout: false) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
-
- EM::Timer.new 3 do
- ws.close
- end
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- Specimen.create
- state = 2
- when 2
- assert false, "Should not get any messages, message was #{event.data}"
- end
- end
-
- end
-
- end
-
- test "connect, try bogus method" do
- status = nil
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({method: 'frobnabble'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- status = d["status"]
- ws.close
- end
- end
-
- assert_equal 400, status
- end
-
- test "connect, missing method" do
- status = nil
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send ({fizzbuzz: 'frobnabble'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- status = d["status"]
- ws.close
- end
- end
-
- assert_equal 400, status
- end
-
- test "connect, send malformed request" do
- status = nil
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- ws.send '<XML4EVER></XML4EVER>'
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- status = d["status"]
- ws.close
- end
- end
-
- assert_equal 400, status
- end
-
-
- test "connect, try subscribe too many filters" do
- state = 1
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- (1..17).each do |i|
- ws.send ({method: 'subscribe', filters: [['object_uuid', '=', i]]}.to_json)
- end
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when (1..Rails.configuration.websocket_max_filters)
- assert_equal 200, d["status"]
- state += 1
- when (Rails.configuration.websocket_max_filters+1)
- assert_equal 403, d["status"]
- ws.close
- end
- end
-
- end
-
- assert_equal Rails.configuration.websocket_max_filters+1, state
-
- end
-
- slow_test "connect, subscribe, lots of events" do
- state = 1
- event_count = 0
- log_start = Log.order(:id).last.id
-
- authorize_with :active
-
- ws_helper(token: :active, timeout: false) do |ws|
- EM::Timer.new 45 do
- # Needs a longer timeout than the default
- ws.close
- end
-
- ws.on :open do |event|
- ws.send ({method: 'subscribe'}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- ActiveRecord::Base.transaction do
- (1..202).each do
- Specimen.create
- end
- end
- state = 2
- when 2
- event_count += 1
- assert_equal d['id'], event_count+log_start
- if event_count == 202
- ws.close
- end
- end
- end
-
- end
-
- assert_equal 202, event_count
- end
-
-
- test "connect, subscribe with invalid filter" do
- state = 1
-
- authorize_with :active
-
- ws_helper(token: :active) do |ws|
- ws.on :open do |event|
- # test that #6451 is fixed (invalid filter crashes websockets)
- ws.send ({method: 'subscribe', filters: [['object_blarg', 'is_a', 'arvados#human']]}.to_json)
- end
-
- ws.on :message do |event|
- d = SafeJSON.load event.data
- case state
- when 1
- assert_equal 200, d["status"]
- Specimen.create
- Human.create
- state = 2
- when 2
- assert_equal 500, d["status"]
- state = 3
- ws.close
- when 3
- assert false, "Should not get any more events"
- end
- end
-
- end
-
- assert_equal 3, state
-
- # Try connecting again, ensure that websockets server is still running and
- # didn't crash per #6451
- subscribe_test()
-
- end
-
-
-end
commit 390af6a13f7c8974329aecc2f23fbfa81f8e298b
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Mar 19 02:46:00 2017 -0400
7709: Allow null for empty serialized fields.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 23bb71a..a5aa6a4 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -541,8 +541,15 @@ class ArvadosModel < ActiveRecord::Base
end
def self.where_serialized(colname, value)
- sorted = deep_sort_hash(value)
- where("#{colname.to_s} IN (?)", [sorted.to_yaml, SafeJSON.dump(sorted)])
+ if value.empty?
+ # rails4 stores as null, rails3 stored as serialized [] or {}
+ sql = "#{colname.to_s} is null or #{colname.to_s} IN (?)"
+ sorted = value
+ else
+ sql = "#{colname.to_s} IN (?)"
+ sorted = deep_sort_hash(value)
+ end
+ where(sql, [sorted.to_yaml, SafeJSON.dump(sorted)])
end
Serializer = {
diff --git a/services/api/db/migrate/20170319063406_serialized_columns_accept_null.rb b/services/api/db/migrate/20170319063406_serialized_columns_accept_null.rb
new file mode 100644
index 0000000..564586e
--- /dev/null
+++ b/services/api/db/migrate/20170319063406_serialized_columns_accept_null.rb
@@ -0,0 +1,5 @@
+class SerializedColumnsAcceptNull < ActiveRecord::Migration
+ def change
+ change_column :api_client_authorizations, :scopes, :text, null: true, default: '["all"]'
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 87b6575..015db6f 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -44,9 +44,7 @@ CREATE TABLE api_client_authorizations (
created_at timestamp without time zone NOT NULL,
updated_at timestamp without time zone NOT NULL,
default_owner_uuid character varying(255),
- scopes text DEFAULT '---
-- all
-'::text NOT NULL,
+ scopes text DEFAULT '["all"]'::text,
uuid character varying(255) NOT NULL
);
@@ -2744,4 +2742,7 @@ INSERT INTO schema_migrations (version) VALUES ('20170105160302');
INSERT INTO schema_migrations (version) VALUES ('20170216170823');
-INSERT INTO schema_migrations (version) VALUES ('20170301225558');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20170301225558');
+
+INSERT INTO schema_migrations (version) VALUES ('20170319063406');
+
commit 9090c60b28de593b8bb2ce606a9ab35b62b57608
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Mar 19 02:00:04 2017 -0400
7709: Upgrade to rails4, fix some of the compatibility issues.
diff --git a/services/api/Gemfile b/services/api/Gemfile
index ab42bf7..3e1d942 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -1,9 +1,8 @@
source 'https://rubygems.org'
-gem 'rails', '~> 3.2'
-
-# Bundle edge Rails instead:
-# gem 'rails', :git => 'git://github.com/rails/rails.git'
+gem 'rails', '~> 4.0'
+gem 'responders', '~> 2.0'
+gem 'protected_attributes'
group :test, :development do
gem 'factory_girl_rails'
@@ -21,39 +20,18 @@ end
# pg is the only supported database driver.
gem 'pg'
-# Start using multi_json once we are on Rails 3.2;
-# Rails 3.1 has a dependency on multi_json < 1.3.0 but we need version 1.3.4 to
-# fix bug https://github.com/collectiveidea/json_spec/issues/27
gem 'multi_json'
gem 'oj'
gem 'oj_mimic_json'
-# Gems used only for assets and not required
-# in production environments by default.
-group :assets do
- gem 'sass-rails', '~> 3.2'
- gem 'coffee-rails', '~> 3.2'
-
- # See https://github.com/sstephenson/execjs#readme for more supported runtimes
- gem 'therubyracer'
-
- gem 'uglifier', '~> 2.0'
-end
+# for building assets
+gem 'sass-rails', '~> 4.0'
+gem 'coffee-rails', '~> 4.0'
+gem 'therubyracer'
+gem 'uglifier', '~> 2.0'
gem 'jquery-rails'
-# To use ActiveModel has_secure_password
-# gem 'bcrypt-ruby', '~> 3.0.0'
-
-# Use unicorn as the web server
-# gem 'unicorn'
-
-# Deploy with Capistrano
-# gem 'capistrano'
-
-# To use debugger
-# gem 'ruby-debug'
-
gem 'rvm-capistrano', :group => :test
gem 'acts_as_api'
@@ -70,14 +48,11 @@ gem 'test_after_commit', :group => :test
gem 'trollop'
gem 'faye-websocket'
-gem 'themes_for_rails'
+gem 'themes_for_rails', git: 'https://github.com/curoverse/themes_for_rails'
gem 'arvados', '>= 0.1.20150615153458'
gem 'arvados-cli', '>= 0.1.20161017193526'
-# pg_power lets us use partial indexes in schema.rb in Rails 3
-gem 'pg_power'
-
gem 'puma', '~> 2.0'
gem 'sshkey'
gem 'safe_yaml'
@@ -85,6 +60,6 @@ gem 'lograge'
gem 'logstash-event'
# Install any plugin gems
-Dir.glob(File.join(File.dirname(__FILE__), 'lib', '**', "Gemfile")) do |gemfile|
- eval(IO.read(gemfile), binding)
+Dir.glob(File.join(File.dirname(__FILE__), 'lib', '**', "Gemfile")) do |f|
+ eval(IO.read(f), binding)
end
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 6217744..3c5a476 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -1,33 +1,48 @@
+GIT
+ remote: https://github.com/curoverse/themes_for_rails
+ revision: 61154877047d2346890bda0b7be5827cf51a6a76
+ specs:
+ themes_for_rails (0.5.1)
+ rails (>= 3.0.0)
+
GEM
remote: https://rubygems.org/
specs:
- actionmailer (3.2.22.5)
- actionpack (= 3.2.22.5)
- mail (~> 2.5.4)
- actionpack (3.2.22.5)
- activemodel (= 3.2.22.5)
- activesupport (= 3.2.22.5)
- builder (~> 3.0.0)
+ actionmailer (4.2.5.2)
+ actionpack (= 4.2.5.2)
+ actionview (= 4.2.5.2)
+ activejob (= 4.2.5.2)
+ mail (~> 2.5, >= 2.5.4)
+ rails-dom-testing (~> 1.0, >= 1.0.5)
+ actionpack (4.2.5.2)
+ actionview (= 4.2.5.2)
+ activesupport (= 4.2.5.2)
+ rack (~> 1.6)
+ rack-test (~> 0.6.2)
+ rails-dom-testing (~> 1.0, >= 1.0.5)
+ rails-html-sanitizer (~> 1.0, >= 1.0.2)
+ actionview (4.2.5.2)
+ activesupport (= 4.2.5.2)
+ builder (~> 3.1)
erubis (~> 2.7.0)
- journey (~> 1.0.4)
- rack (~> 1.4.5)
- rack-cache (~> 1.2)
- rack-test (~> 0.6.1)
- sprockets (~> 2.2.1)
- activemodel (3.2.22.5)
- activesupport (= 3.2.22.5)
- builder (~> 3.0.0)
- activerecord (3.2.22.5)
- activemodel (= 3.2.22.5)
- activesupport (= 3.2.22.5)
- arel (~> 3.0.2)
- tzinfo (~> 0.3.29)
- activeresource (3.2.22.5)
- activemodel (= 3.2.22.5)
- activesupport (= 3.2.22.5)
- activesupport (3.2.22.5)
- i18n (~> 0.6, >= 0.6.4)
- multi_json (~> 1.0)
+ rails-dom-testing (~> 1.0, >= 1.0.5)
+ rails-html-sanitizer (~> 1.0, >= 1.0.2)
+ activejob (4.2.5.2)
+ activesupport (= 4.2.5.2)
+ globalid (>= 0.3.0)
+ activemodel (4.2.5.2)
+ activesupport (= 4.2.5.2)
+ builder (~> 3.1)
+ activerecord (4.2.5.2)
+ activemodel (= 4.2.5.2)
+ activesupport (= 4.2.5.2)
+ arel (~> 6.0)
+ activesupport (4.2.5.2)
+ i18n (~> 0.7)
+ json (~> 1.7, >= 1.7.7)
+ minitest (~> 5.1)
+ thread_safe (~> 0.3, >= 0.3.4)
+ tzinfo (~> 1.1)
acts_as_api (1.0.0)
activemodel (>= 3.0.0)
activesupport (>= 3.0.0)
@@ -35,7 +50,7 @@ GEM
addressable (2.5.0)
public_suffix (~> 2.0, >= 2.0.2)
andand (1.3.3)
- arel (3.0.3)
+ arel (6.0.4)
arvados (0.1.20170215224121)
activesupport (>= 3, < 4.2.6)
andand (~> 1.3, >= 1.3.3)
@@ -56,16 +71,16 @@ GEM
addressable (>= 2.3.1)
extlib (>= 0.9.15)
multi_json (>= 1.0.0)
- builder (3.0.4)
+ builder (3.2.3)
capistrano (2.15.9)
highline
net-scp (>= 1.0.0)
net-sftp (>= 2.0.0)
net-ssh (>= 2.0.14)
net-ssh-gateway (>= 1.1.0)
- coffee-rails (3.2.2)
+ coffee-rails (4.2.1)
coffee-script (>= 2.2.0)
- railties (~> 3.2.0)
+ railties (>= 4.0.0, < 5.2.x)
coffee-script (2.4.1)
coffee-script-source
execjs
@@ -86,6 +101,8 @@ GEM
faye-websocket (0.10.7)
eventmachine (>= 0.12.0)
websocket-driver (>= 0.5.1)
+ globalid (0.3.7)
+ activesupport (>= 4.1.0)
google-api-client (0.8.7)
activesupport (>= 3.2, < 5.0)
addressable (~> 2.3)
@@ -109,9 +126,9 @@ GEM
highline (1.7.8)
hike (1.2.3)
i18n (0.8.1)
- journey (1.0.4)
- jquery-rails (3.1.4)
- railties (>= 3.0, < 5.0)
+ jquery-rails (4.2.2)
+ rails-dom-testing (>= 1, < 3)
+ railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
json (1.8.6)
jwt (1.5.6)
@@ -122,17 +139,22 @@ GEM
logging (2.2.0)
little-plugger (~> 1.1)
multi_json (~> 1.10)
- lograge (0.3.6)
- actionpack (>= 3)
- activesupport (>= 3)
- railties (>= 3)
+ lograge (0.4.1)
+ actionpack (>= 4, < 5.1)
+ activesupport (>= 4, < 5.1)
+ railties (>= 4, < 5.1)
logstash-event (1.2.02)
- mail (2.5.4)
- mime-types (~> 1.16)
- treetop (~> 1.4.8)
+ loofah (2.0.3)
+ nokogiri (>= 1.5.9)
+ mail (2.6.4)
+ mime-types (>= 1.16, < 4)
memoist (0.15.0)
metaclass (0.0.4)
- mime-types (1.25.1)
+ mime-types (3.1)
+ mime-types-data (~> 3.2015)
+ mime-types-data (3.2016.0521)
+ mini_portile2 (2.1.0)
+ minitest (5.10.1)
mocha (1.2.1)
metaclass (~> 0.0.1)
multi_json (1.12.1)
@@ -145,6 +167,8 @@ GEM
net-ssh (4.1.0)
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
+ nokogiri (1.7.1)
+ mini_portile2 (~> 2.1.0)
oauth2 (1.3.1)
faraday (>= 0.8, < 0.12)
jwt (~> 1.0)
@@ -153,9 +177,9 @@ GEM
rack (>= 1.2, < 3)
oj (2.18.5)
oj_mimic_json (1.0.1)
- omniauth (1.4.2)
- hashie (>= 1.2, < 4)
- rack (>= 1.0, < 3)
+ omniauth (1.6.1)
+ hashie (>= 3.4.6, < 3.6.0)
+ rack (>= 1.6.2, < 3)
omniauth-oauth2 (1.4.0)
oauth2 (~> 1.0)
omniauth (~> 1.2)
@@ -164,49 +188,53 @@ GEM
rack
rake (>= 0.8.1)
pg (0.20.0)
- pg_power (1.6.4)
- pg
- rails (~> 3.1)
- polyglot (0.3.5)
power_assert (1.0.1)
+ protected_attributes (1.1.3)
+ activemodel (>= 4.0.1, < 5.0)
public_suffix (2.0.5)
puma (2.16.0)
- rack (1.4.7)
- rack-cache (1.7.0)
- rack (>= 0.4)
- rack-ssl (1.3.4)
- rack
+ rack (1.6.5)
rack-test (0.6.3)
rack (>= 1.0)
- rails (3.2.22.5)
- actionmailer (= 3.2.22.5)
- actionpack (= 3.2.22.5)
- activerecord (= 3.2.22.5)
- activeresource (= 3.2.22.5)
- activesupport (= 3.2.22.5)
- bundler (~> 1.0)
- railties (= 3.2.22.5)
- railties (3.2.22.5)
- actionpack (= 3.2.22.5)
- activesupport (= 3.2.22.5)
- rack-ssl (~> 1.3.2)
+ rails (4.2.5.2)
+ actionmailer (= 4.2.5.2)
+ actionpack (= 4.2.5.2)
+ actionview (= 4.2.5.2)
+ activejob (= 4.2.5.2)
+ activemodel (= 4.2.5.2)
+ activerecord (= 4.2.5.2)
+ activesupport (= 4.2.5.2)
+ bundler (>= 1.3.0, < 2.0)
+ railties (= 4.2.5.2)
+ sprockets-rails
+ rails-deprecated_sanitizer (1.0.3)
+ activesupport (>= 4.2.0.alpha)
+ rails-dom-testing (1.0.8)
+ activesupport (>= 4.2.0.beta, < 5.0)
+ nokogiri (~> 1.6)
+ rails-deprecated_sanitizer (>= 1.0.1)
+ rails-html-sanitizer (1.0.3)
+ loofah (~> 2.0)
+ railties (4.2.5.2)
+ actionpack (= 4.2.5.2)
+ activesupport (= 4.2.5.2)
rake (>= 0.8.7)
- rdoc (~> 3.4)
- thor (>= 0.14.6, < 2.0)
+ thor (>= 0.18.1, < 2.0)
rake (12.0.0)
- rdoc (3.12.2)
- json (~> 1.4)
ref (2.0.0)
+ responders (2.3.0)
+ railties (>= 4.2.0, < 5.1)
retriable (1.4.1)
ruby-prof (0.16.2)
rvm-capistrano (1.5.6)
capistrano (~> 2.15.4)
safe_yaml (1.0.4)
- sass (3.4.23)
- sass-rails (3.2.6)
- railties (~> 3.2.0)
- sass (>= 3.1.10)
- tilt (~> 1.3)
+ sass (3.2.19)
+ sass-rails (4.0.5)
+ railties (>= 4.0.0, < 5.0)
+ sass (~> 3.2.2)
+ sprockets (~> 2.8, < 3.0)
+ sprockets-rails (~> 2.0)
signet (0.7.3)
addressable (~> 2.3)
faraday (~> 0.9)
@@ -218,28 +246,29 @@ GEM
simplecov-html (0.7.1)
simplecov-rcov (0.2.3)
simplecov (>= 0.4.1)
- sprockets (2.2.3)
+ sprockets (2.12.4)
hike (~> 1.2)
multi_json (~> 1.0)
rack (~> 1.0)
tilt (~> 1.1, != 1.3.0)
+ sprockets-rails (2.3.3)
+ actionpack (>= 3.0)
+ activesupport (>= 3.0)
+ sprockets (>= 2.8, < 4.0)
sshkey (1.9.0)
test-unit (3.2.3)
power_assert
test_after_commit (1.1.0)
activerecord (>= 3.2)
- themes_for_rails (0.5.1)
- rails (>= 3.0.0)
therubyracer (0.12.3)
libv8 (~> 3.16.14.15)
ref
thor (0.19.4)
+ thread_safe (0.3.6)
tilt (1.4.1)
- treetop (1.4.15)
- polyglot
- polyglot (>= 0.3.1)
trollop (2.1.2)
- tzinfo (0.3.52)
+ tzinfo (1.2.2)
+ thread_safe (~> 0.1)
uglifier (2.7.2)
execjs (>= 0.3.0)
json (>= 1.8.0)
@@ -255,7 +284,7 @@ DEPENDENCIES
andand
arvados (>= 0.1.20150615153458)
arvados-cli (>= 0.1.20161017193526)
- coffee-rails (~> 3.2)
+ coffee-rails (~> 4.0)
database_cleaner
factory_girl_rails
faye-websocket
@@ -270,19 +299,20 @@ DEPENDENCIES
omniauth-oauth2 (~> 1.1)
passenger
pg
- pg_power
+ protected_attributes
puma (~> 2.0)
- rails (~> 3.2)
+ rails (~> 4.0)
+ responders (~> 2.0)
ruby-prof
rvm-capistrano
safe_yaml
- sass-rails (~> 3.2)
+ sass-rails (~> 4.0)
simplecov (~> 0.7.1)
simplecov-rcov
sshkey
test-unit (~> 3.0)
test_after_commit
- themes_for_rails
+ themes_for_rails!
therubyracer
trollop
uglifier (~> 2.0)
diff --git a/services/api/Rakefile b/services/api/Rakefile
index 925e6c2..67e4202 100644
--- a/services/api/Rakefile
+++ b/services/api/Rakefile
@@ -4,12 +4,6 @@
require File.expand_path('../config/application', __FILE__)
-begin
- ok = PgPower
-rescue
- abort "Hm, pg_power is missing. Make sure you use 'bundle exec rake ...'"
-end
-
Server::Application.load_tasks
namespace :test do
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 71fb365..6ba334f 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -18,8 +18,8 @@ end
require 'load_param'
class ApplicationController < ActionController::Base
- include CurrentApiClient
include ThemesForRails::ActionController
+ include CurrentApiClient
include LoadParam
include DbCurrentTime
@@ -47,7 +47,7 @@ class ApplicationController < ActionController::Base
before_filter(:render_404_if_no_object,
except: [:index, :create] + ERROR_ACTIONS)
- theme :select_theme
+ theme Rails.configuration.arvados_theme
attr_writer :resource_attrs
@@ -488,7 +488,7 @@ class ApplicationController < ActionController::Base
def remote_ip
# Caveat: this is highly dependent on the proxy setup. YMMV.
- if request.headers.has_key?('HTTP_X_REAL_IP') then
+ if request.headers.key?('HTTP_X_REAL_IP') then
# We're behind a reverse proxy
@remote_ip = request.headers['HTTP_X_REAL_IP']
else
@@ -562,8 +562,4 @@ class ApplicationController < ActionController::Base
end
super(*opts)
end
-
- def select_theme
- return Rails.configuration.arvados_theme
- end
end
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index db5e7bd..7a1f699 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -123,7 +123,7 @@ class Arvados::V1::UsersController < ApplicationController
# setup succeeded. send email to user
if params[:send_notification_email] == true || params[:send_notification_email] == 'true'
- UserNotifier.account_is_setup(@object).deliver
+ UserNotifier.account_is_setup(@object).deliver_now
end
send_json kind: "arvados#HashList", items: @response.as_api_response(nil)
diff --git a/services/api/app/controllers/database_controller.rb b/services/api/app/controllers/database_controller.rb
index 6699f73..458766d 100644
--- a/services/api/app/controllers/database_controller.rb
+++ b/services/api/app/controllers/database_controller.rb
@@ -51,8 +51,8 @@ class DatabaseController < ApplicationController
# create_fixtures() is a no-op for cached fixture sets, so
# uncache them all.
- ActiveRecord::Fixtures.reset_cache
- ActiveRecord::Fixtures.
+ ActiveRecord::FixtureSet.reset_cache
+ ActiveRecord::FixtureSet.
create_fixtures(Rails.root.join('test', 'fixtures'), fixturesets)
# Dump cache of permissions etc.
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 8bb27a7..c550704 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -127,7 +127,8 @@ class UserSessionsController < ApplicationController
# Stub: automatically register all new API clients
api_client_url_prefix = callback_url.match(%r{^.*?://[^/]+})[0] + '/'
act_as_system_user do
- @api_client = ApiClient.find_or_create_by_url_prefix api_client_url_prefix
+ @api_client = ApiClient.
+ find_or_create_by(url_prefix: api_client_url_prefix)
end
api_client_auth = ApiClientAuthorization.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index b77ba1c..23bb71a 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -9,10 +9,6 @@ class ArvadosModel < ActiveRecord::Base
include DbCurrentTime
extend RecordFilters
- attr_protected :created_at
- attr_protected :modified_by_user_uuid
- attr_protected :modified_by_client_uuid
- attr_protected :modified_at
after_initialize :log_start_state
before_save :ensure_permission_to_save
before_save :ensure_owner_uuid_is_permitted
@@ -27,17 +23,16 @@ class ArvadosModel < ActiveRecord::Base
after_find :convert_serialized_symbols_to_strings
before_validation :normalize_collection_uuids
before_validation :set_default_owner
- validate :ensure_serialized_attribute_type
validate :ensure_valid_uuids
# Note: This only returns permission links. It does not account for
# permissions obtained via user.is_admin or
# user.uuid==object.owner_uuid.
has_many(:permissions,
+ ->{where(link_class: 'permission')},
foreign_key: :head_uuid,
class_name: 'Link',
- primary_key: :uuid,
- conditions: "link_class = 'permission'")
+ primary_key: :uuid)
class PermissionDeniedError < StandardError
def http_status
@@ -77,6 +72,31 @@ class ArvadosModel < ActiveRecord::Base
"#{current_api_base}/#{self.class.to_s.pluralize.underscore}/#{self.uuid}"
end
+ def self.permit_attribute_params raw_params
+ # strong_parameters does not provide security: permissions are
+ # implemented with before_save hooks.
+ #
+ # The following permit! is necessary even with
+ # "ActionController::Parameters.permit_all_parameters = true",
+ # because permit_all does not permit nested attributes.
+ if has_nonstring_keys?(raw_params)
+ raise ArgumentError.new("Parameters cannot have non-string keys")
+ end
+ ActionController::Parameters.new(raw_params).permit!
+ end
+
+ def initialize raw_params={}, *args
+ super(self.class.permit_attribute_params(raw_params), *args)
+ end
+
+ def self.create raw_params={}, *args
+ super(permit_attribute_params(raw_params), *args)
+ end
+
+ def update_attributes raw_params={}, *args
+ super(self.class.permit_attribute_params(raw_params), *args)
+ end
+
def self.selectable_attributes(template=:user)
# Return an array of attribute name strings that can be selected
# in the given template.
@@ -463,6 +483,7 @@ class ArvadosModel < ActiveRecord::Base
def update_modified_by_fields
current_time = db_current_time
+ self.created_at = created_at_was || current_time
self.updated_at = current_time
self.owner_uuid ||= current_default_owner if self.respond_to? :owner_uuid=
self.modified_at = current_time
@@ -471,6 +492,19 @@ class ArvadosModel < ActiveRecord::Base
true
end
+ def self.has_nonstring_keys? x
+ if x.is_a? Hash
+ x.each do |k,v|
+ return true if !(k.is_a?(String) || k.is_a?(Symbol)) || has_nonstring_keys?(v)
+ end
+ elsif x.is_a? Array
+ x.each do |v|
+ return true if has_nonstring_keys?(v)
+ end
+ end
+ false
+ end
+
def self.has_symbols? x
if x.is_a? Hash
x.each do |k,v|
@@ -517,25 +551,18 @@ class ArvadosModel < ActiveRecord::Base
}
def self.serialize(colname, type)
- super(colname, Serializer[type])
+ coder = Serializer[type]
+ @serialized_attributes ||= {}
+ @serialized_attributes[colname.to_s] = coder
+ super(colname, coder)
end
- def ensure_serialized_attribute_type
- # Specifying a type in the "serialize" declaration causes rails to
- # raise an exception if a different data type is retrieved from
- # the database during load(). The validation preventing such
- # crash-inducing records from being inserted in the database in
- # the first place seems to have been left as an exercise to the
- # developer.
- self.class.serialized_attributes.each do |colname, attr|
- if attr.object_class
- if self.attributes[colname].class != attr.object_class
- self.errors.add colname.to_sym, "must be a #{attr.object_class.to_s}, not a #{self.attributes[colname].class.to_s}"
- elsif self.class.has_symbols? attributes[colname]
- self.errors.add colname.to_sym, "must not contain symbols: #{attributes[colname].inspect}"
- end
- end
- end
+ def self.serialized_attributes
+ @serialized_attributes ||= {}
+ end
+
+ def serialized_attributes
+ self.class.serialized_attributes
end
def convert_serialized_symbols_to_strings
@@ -548,8 +575,8 @@ class ArvadosModel < ActiveRecord::Base
self.class.serialized_attributes.each do |colname, attr|
if self.class.has_symbols? attributes[colname]
attributes[colname] = self.class.recursive_stringify attributes[colname]
- self.send(colname + '=',
- self.class.recursive_stringify(attributes[colname]))
+ send(colname + '=',
+ self.class.recursive_stringify(attributes[colname]))
end
end
end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 6d1a0d5..bae024b 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -24,7 +24,7 @@ class Collection < ArvadosModel
before_save :set_file_names
# Query only untrashed collections by default.
- default_scope where("is_trashed = false")
+ default_scope { where("is_trashed = false") }
api_accessible :user, extend: :common do |t|
t.add :name
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 649a6f8..d38ea59 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -20,11 +20,6 @@ class Link < ArvadosModel
t.add :properties
end
- def properties
- @properties ||= Hash.new
- super
- end
-
def head_kind
if k = ArvadosModel::resource_class_for_uuid(head_uuid)
k.kind
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index eedf06a..4498425 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -102,6 +102,6 @@ class Log < ArvadosModel
end
def send_notify
- connection.execute "NOTIFY logs, '#{self.id}'"
+ ActiveRecord::Base.connection.execute "NOTIFY logs, '#{self.id}'"
end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 78ec7be..742db4c 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -10,7 +10,7 @@ class User < ArvadosModel
has_many :api_client_authorizations
validates(:username,
format: {
- with: /^[A-Za-z][A-Za-z0-9]*$/,
+ with: /\A[A-Za-z][A-Za-z0-9]*\z/,
message: "must begin with a letter and contain only alphanumerics",
},
uniqueness: true,
@@ -475,9 +475,9 @@ class User < ArvadosModel
# Send admin notifications
def send_admin_notifications
- AdminNotifier.new_user(self).deliver
+ AdminNotifier.new_user(self).deliver_now
if not self.is_active then
- AdminNotifier.new_inactive_user(self).deliver
+ AdminNotifier.new_inactive_user(self).deliver_now
end
end
@@ -502,7 +502,7 @@ class User < ArvadosModel
if self.prefs_changed?
if self.prefs_was.andand.empty? || !self.prefs_was.andand['profile']
profile_notification_address = Rails.configuration.user_profile_notification_address
- ProfileNotifier.profile_created(self, profile_notification_address).deliver if profile_notification_address
+ ProfileNotifier.profile_created(self, profile_notification_address).deliver_now if profile_notification_address
end
end
end
diff --git a/services/api/app/models/virtual_machine.rb b/services/api/app/models/virtual_machine.rb
index 094591e..6fbbddf 100644
--- a/services/api/app/models/virtual_machine.rb
+++ b/services/api/app/models/virtual_machine.rb
@@ -3,7 +3,11 @@ class VirtualMachine < ArvadosModel
include KindAndEtag
include CommonApiTemplate
- has_many :login_permissions, :foreign_key => :head_uuid, :class_name => 'Link', :primary_key => :uuid, :conditions => "link_class = 'permission' and name = 'can_login'"
+ has_many(:login_permissions,
+ -> { where("link_class = 'permission' and name = 'can_login'") },
+ foreign_key: :head_uuid,
+ class_name: 'Link',
+ primary_key: :uuid)
api_accessible :user, extend: :common do |t|
t.add :hostname
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 5241cb4..44807ec 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -441,7 +441,6 @@ development:
action_mailer.perform_deliveries: false
active_support.deprecation: :log
action_dispatch.best_standards_support: :builtin
- active_record.mass_assignment_sanitizer: :strict
active_record.auto_explain_threshold_in_seconds: 0.5
assets.compress: false
assets.debug: true
@@ -451,7 +450,7 @@ production:
cache_classes: true
consider_all_requests_local: false
action_controller.perform_caching: true
- serve_static_assets: false
+ serve_static_files: false
assets.compress: true
assets.compile: false
assets.digest: true
@@ -459,7 +458,7 @@ production:
test:
force_ssl: false
cache_classes: true
- serve_static_assets: true
+ serve_static_files: true
static_cache_control: public, max-age=3600
whiny_nils: true
consider_all_requests_local: true
@@ -468,7 +467,6 @@ test:
action_controller.allow_forgery_protection: false
action_mailer.delivery_method: :test
active_support.deprecation: :stderr
- active_record.mass_assignment_sanitizer: :strict
uuid_prefix: zzzzz
sso_app_id: arvados-server
sso_app_secret: <%= rand(2**512).to_s(36) %>
diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index f3f6424..4acde0f 100644
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -34,6 +34,13 @@ module Server
# Configure sensitive parameters which will be filtered from the log file.
config.filter_parameters += [:password]
+ # Load entire application at startup.
+ config.eager_load = true
+
+ config.active_record.raise_in_transactional_callbacks = true
+
+ config.active_support.test_order = :sorted
+
I18n.enforce_available_locales = false
# Before using the filesystem backend for Rails.cache, check
diff --git a/services/api/config/environments/development.rb.example b/services/api/config/environments/development.rb.example
index b6c4c92..449d05a 100644
--- a/services/api/config/environments/development.rb.example
+++ b/services/api/config/environments/development.rb.example
@@ -23,9 +23,6 @@ Server::Application.configure do
# Only use best-standards-support built into browsers
config.action_dispatch.best_standards_support = :builtin
- # Raise exception on mass assignment protection for Active Record models
- config.active_record.mass_assignment_sanitizer = :strict
-
# Log the query plan for queries taking more than this (works
# with SQLite, MySQL, and PostgreSQL)
config.active_record.auto_explain_threshold_in_seconds = 0.5
diff --git a/services/api/config/environments/production.rb.example b/services/api/config/environments/production.rb.example
index c1092d3..2b91d3d 100644
--- a/services/api/config/environments/production.rb.example
+++ b/services/api/config/environments/production.rb.example
@@ -9,7 +9,7 @@ Server::Application.configure do
config.action_controller.perform_caching = true
# Disable Rails's static asset server (Apache or nginx will already do this)
- config.serve_static_assets = false
+ config.serve_static_files = false
# Compress JavaScripts and CSS
config.assets.compress = true
diff --git a/services/api/config/environments/test.rb.example b/services/api/config/environments/test.rb.example
index 5baf09d..f21a6d4 100644
--- a/services/api/config/environments/test.rb.example
+++ b/services/api/config/environments/test.rb.example
@@ -8,7 +8,7 @@ Server::Application.configure do
config.cache_classes = true
# Configure static asset server for tests with Cache-Control for performance
- config.serve_static_assets = true
+ config.serve_static_files = true
config.static_cache_control = "public, max-age=3600"
# Log error messages when you accidentally call methods on nil
@@ -37,9 +37,6 @@ Server::Application.configure do
# Print deprecation notices to the stderr
config.active_support.deprecation = :stderr
- # Raise exception on mass assignment protection for Active Record models
- config.active_record.mass_assignment_sanitizer = :strict
-
# No need for SSL while testing
config.force_ssl = false
diff --git a/services/api/config/initializers/load_config.rb b/services/api/config/initializers/load_config.rb
index fd3c977..787469c 100644
--- a/services/api/config/initializers/load_config.rb
+++ b/services/api/config/initializers/load_config.rb
@@ -69,4 +69,5 @@ config/application.yml:
EOS
end
+ config.secret_key_base = config.secret_token
end
diff --git a/services/api/config/initializers/permit_all_parameters.rb b/services/api/config/initializers/permit_all_parameters.rb
new file mode 100644
index 0000000..051e13b
--- /dev/null
+++ b/services/api/config/initializers/permit_all_parameters.rb
@@ -0,0 +1 @@
+ActionController::Parameters.permit_all_parameters = true
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index 9cb53fe..77e5372 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -1,15 +1,13 @@
Server::Application.routes.draw do
themes_for_rails
- # See http://guides.rubyonrails.org/routing.html
-
# OPTIONS requests are not allowed at routes that use cookies.
['/auth/*a', '/login', '/logout'].each do |nono|
- match nono, :to => 'user_sessions#cross_origin_forbidden', :via => 'OPTIONS'
+ match nono, to: 'user_sessions#cross_origin_forbidden', via: 'OPTIONS'
end
# OPTIONS at discovery and API paths get an empty response with CORS headers.
- match '/discovery/v1/*a', :to => 'static#empty', :via => 'OPTIONS'
- match '/arvados/v1/*a', :to => 'static#empty', :via => 'OPTIONS'
+ match '/discovery/v1/*a', to: 'static#empty', via: 'OPTIONS'
+ match '/arvados/v1/*a', to: 'static#empty', via: 'OPTIONS'
namespace :arvados do
namespace :v1 do
@@ -79,7 +77,7 @@ Server::Application.routes.draw do
get 'logins', on: :member
get 'get_all_logins', on: :collection
end
- get '/permissions/:uuid', :to => 'links#get_permissions'
+ get '/permissions/:uuid', to: 'links#get_permissions'
end
end
@@ -88,22 +86,22 @@ Server::Application.routes.draw do
end
# omniauth
- match '/auth/:provider/callback', :to => 'user_sessions#create'
- match '/auth/failure', :to => 'user_sessions#failure'
+ match '/auth/:provider/callback', to: 'user_sessions#create', via: [:get, :post]
+ match '/auth/failure', to: 'user_sessions#failure', via: [:get, :post]
# not handled by omniauth provider -> 403 with no CORS headers.
- get '/auth/*a', :to => 'user_sessions#cross_origin_forbidden'
+ get '/auth/*a', to: 'user_sessions#cross_origin_forbidden'
# Custom logout
- match '/login', :to => 'user_sessions#login'
- match '/logout', :to => 'user_sessions#logout'
+ match '/login', to: 'user_sessions#login', via: [:get, :post]
+ match '/logout', to: 'user_sessions#logout', via: [:get, :post]
- match '/discovery/v1/apis/arvados/v1/rest', :to => 'arvados/v1/schema#index'
+ match '/discovery/v1/apis/arvados/v1/rest', to: 'arvados/v1/schema#index', via: [:get, :post]
- match '/static/login_failure', :to => 'static#login_failure', :as => :login_failure
+ match '/static/login_failure', to: 'static#login_failure', as: :login_failure, via: [:get, :post]
# Send unroutable requests to an arbitrary controller
# (ends up at ApplicationController#render_not_found)
- match '*a', :to => 'static#render_not_found'
+ match '*a', to: 'static#render_not_found', via: [:get, :post, :put, :patch, :delete, :options]
- root :to => 'static#home'
+ root to: 'static#home'
end
diff --git a/services/api/lib/can_be_an_owner.rb b/services/api/lib/can_be_an_owner.rb
index 16a8783..75a6350 100644
--- a/services/api/lib/can_be_an_owner.rb
+++ b/services/api/lib/can_be_an_owner.rb
@@ -14,7 +14,7 @@ module CanBeAnOwner
base.has_many(t.to_sym,
foreign_key: :owner_uuid,
primary_key: :uuid,
- dependent: :restrict)
+ dependent: :restrict_with_exception)
end
# We need custom protection for changing an owner's primary
# key. (Apart from this restriction, admins are allowed to change
diff --git a/services/api/lib/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb
index 84100c2..da67a32 100755
--- a/services/api/lib/create_superuser_token.rb
+++ b/services/api/lib/create_superuser_token.rb
@@ -26,7 +26,9 @@ module CreateSuperUserToken
# need to create a token
if !api_client_auth
# Get (or create) trusted api client
- apiClient = ApiClient.find_or_create_by_url_prefix_and_is_trusted("ssh://root@localhost/", true)
+ apiClient = ApiClient.
+ find_or_create_by(url_prefix: "ssh://root@localhost/",
+ is_trusted: true)
# Check if there is an unexpired superuser token corresponding to this api client
api_client_auth =
diff --git a/services/api/lib/has_uuid.rb b/services/api/lib/has_uuid.rb
index 36c0879..74d09e9 100644
--- a/services/api/lib/has_uuid.rb
+++ b/services/api/lib/has_uuid.rb
@@ -7,8 +7,18 @@ module HasUuid
base.validate :validate_uuid
base.before_create :assign_uuid
base.before_destroy :destroy_permission_links
- base.has_many :links_via_head, class_name: 'Link', foreign_key: :head_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :destroy
- base.has_many :links_via_tail, class_name: 'Link', foreign_key: :tail_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :destroy
+ base.has_many(:links_via_head,
+ -> { where("not (link_class = 'permission')") },
+ class_name: 'Link',
+ foreign_key: :head_uuid,
+ primary_key: :uuid,
+ dependent: :destroy)
+ base.has_many(:links_via_tail,
+ -> { where("not (link_class = 'permission')") },
+ class_name: 'Link',
+ foreign_key: :tail_uuid,
+ primary_key: :uuid,
+ dependent: :destroy)
end
module ClassMethods
diff --git a/services/api/lib/serializers.rb b/services/api/lib/serializers.rb
index 41379f3..e412f63 100644
--- a/services/api/lib/serializers.rb
+++ b/services/api/lib/serializers.rb
@@ -1,7 +1,13 @@
require 'safe_json'
class Serializer
+ class TypeMismatch < ArgumentError
+ end
+
def self.dump(val)
+ if !val.is_a?(object_class)
+ raise TypeMismatch.new("cannot serialize #{val.class} as #{object_class}")
+ end
SafeJSON.dump(val)
end
diff --git a/services/api/lib/tasks/delete_old_container_logs.rake b/services/api/lib/tasks/delete_old_container_logs.rake
index 3421fb8..33fe47d 100644
--- a/services/api/lib/tasks/delete_old_container_logs.rake
+++ b/services/api/lib/tasks/delete_old_container_logs.rake
@@ -7,7 +7,7 @@ namespace :db do
desc "Remove old container log entries from the logs table"
task delete_old_container_logs: :environment do
- delete_sql = "DELETE FROM logs WHERE id in (SELECT logs.id FROM logs JOIN containers ON logs.object_uuid = containers.uuid WHERE event_type IN ('stdout', 'stderr', 'arv-mount', 'crunch-run', 'crunchstat') AND containers.log IS NOT NULL AND containers.finished_at < '#{Rails.configuration.clean_container_log_rows_after.ago}')"
+ delete_sql = "DELETE FROM logs WHERE id in (SELECT logs.id FROM logs JOIN containers ON logs.object_uuid = containers.uuid WHERE event_type IN ('stdout', 'stderr', 'arv-mount', 'crunch-run', 'crunchstat') AND containers.log IS NOT NULL AND clock_timestamp() - containers.finished_at > interval '#{Rails.configuration.clean_container_log_rows_after} seconds')"
ActiveRecord::Base.connection.execute(delete_sql)
end
diff --git a/services/api/lib/tasks/delete_old_job_logs.rake b/services/api/lib/tasks/delete_old_job_logs.rake
index 18a5f02..dec5f72 100644
--- a/services/api/lib/tasks/delete_old_job_logs.rake
+++ b/services/api/lib/tasks/delete_old_job_logs.rake
@@ -5,7 +5,7 @@
namespace :db do
desc "Remove old job stderr entries from the logs table"
task delete_old_job_logs: :environment do
- delete_sql = "DELETE FROM logs WHERE id in (SELECT logs.id FROM logs JOIN jobs ON logs.object_uuid = jobs.uuid WHERE event_type = 'stderr' AND jobs.log IS NOT NULL AND jobs.finished_at < '#{Rails.configuration.clean_job_log_rows_after.ago}')"
+ delete_sql = "DELETE FROM logs WHERE id in (SELECT logs.id FROM logs JOIN jobs ON logs.object_uuid = jobs.uuid WHERE event_type = 'stderr' AND jobs.log IS NOT NULL AND clock_timestamp() - jobs.finished_at > interval '#{Rails.configuration.clean_job_log_rows_after} seconds')"
ActiveRecord::Base.connection.execute(delete_sql)
end
diff --git a/services/api/lib/whitelist_update.rb b/services/api/lib/whitelist_update.rb
index 8fccd0f..039d983 100644
--- a/services/api/lib/whitelist_update.rb
+++ b/services/api/lib/whitelist_update.rb
@@ -1,12 +1,23 @@
module WhitelistUpdate
def check_update_whitelist permitted_fields
attribute_names.each do |field|
- if not permitted_fields.include? field.to_sym and self.send((field.to_s + "_changed?").to_sym)
- errors.add field, "cannot be modified in this state"
+ if !permitted_fields.include?(field.to_sym) && really_changed(field)
+ errors.add field, "cannot be modified in this state (#{send(field+"_was").inspect}, #{send(field).inspect})"
end
end
end
+ def really_changed(attr)
+ return false if !send(attr+"_changed?")
+ old = send(attr+"_was")
+ new = send(attr)
+ if (old.nil? || old == [] || old == {}) && (new.nil? || new == [] || new == {})
+ false
+ else
+ old != new
+ end
+ end
+
def validate_state_change
if self.state_changed?
unless state_transitions[self.state_was].andand.include? self.state
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 2391fc1..a31ad8a 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -283,12 +283,12 @@ EOS
assert_response :success
assert_not_nil assigns(:object)
resp = assigns(:object)
- assert_equal foo_collection[:portable_data_hash], resp['portable_data_hash']
- assert_signed_manifest resp['manifest_text']
+ assert_equal foo_collection[:portable_data_hash], resp[:portable_data_hash]
+ assert_signed_manifest resp[:manifest_text]
# The manifest in the response will have had permission hints added.
# Remove any permission hints in the response before comparing it to the source.
- stripped_manifest = resp['manifest_text'].gsub(/\+A[A-Za-z0-9 at _-]+/, '')
+ stripped_manifest = resp[:manifest_text].gsub(/\+A[A-Za-z0-9 at _-]+/, '')
assert_equal foo_collection[:manifest_text], stripped_manifest
end
end
diff --git a/services/api/test/functional/database_controller_test.rb b/services/api/test/functional/database_controller_test.rb
index 4bda0d0..324a7ff 100644
--- a/services/api/test/functional/database_controller_test.rb
+++ b/services/api/test/functional/database_controller_test.rb
@@ -15,7 +15,7 @@ class DatabaseControllerTest < ActionController::TestCase
begin
Rails.env = 'production'
Rails.application.reload_routes!
- assert_raises ActionController::RoutingError do
+ assert_raises ActionController::UrlGenerationError do
post :reset
end
ensure
diff --git a/services/api/test/integration/api_client_authorizations_scopes_test.rb b/services/api/test/integration/api_client_authorizations_scopes_test.rb
index beef195..17c7516 100644
--- a/services/api/test/integration/api_client_authorizations_scopes_test.rb
+++ b/services/api/test/integration/api_client_authorizations_scopes_test.rb
@@ -4,11 +4,11 @@
require 'test_helper'
-class Arvados::V1::ApiTokensScopeTest < ActionController::IntegrationTest
+class ApiTokensScopeTest < ActionDispatch::IntegrationTest
fixtures :all
def v1_url(*parts)
- (['arvados', 'v1'] + parts).join('/')
+ (['', 'arvados', 'v1'] + parts).join('/')
end
test "user list token can only list users" do
diff --git a/services/api/test/integration/crunch_dispatch_test.rb b/services/api/test/integration/crunch_dispatch_test.rb
index a6f937b..552fd37 100644
--- a/services/api/test/integration/crunch_dispatch_test.rb
+++ b/services/api/test/integration/crunch_dispatch_test.rb
@@ -1,7 +1,7 @@
require 'test_helper'
require 'helpers/git_test_helper'
-class CrunchDispatchTest < ActionDispatch::IntegrationTest
+class CrunchDispatchIntegrationTest < ActionDispatch::IntegrationTest
include GitTestHelper
fixtures :all
diff --git a/services/api/test/integration/errors_test.rb b/services/api/test/integration/errors_test.rb
index 984f81f..1bd17dc 100644
--- a/services/api/test/integration/errors_test.rb
+++ b/services/api/test/integration/errors_test.rb
@@ -19,7 +19,7 @@ class ErrorsTest < ActionDispatch::IntegrationTest
# Generally, new routes should appear under /arvados/v1/. If
# they appear elsewhere, that might have been caused by default
# rails generator behavior that we don't want.
- assert_match(/^\/(|\*a|arvados\/v1\/.*|auth\/.*|login|logout|database\/reset|discovery\/.*|static\/.*|themes\/.*)(\(\.:format\))?$/,
+ assert_match(/^\/(|\*a|arvados\/v1\/.*|auth\/.*|login|logout|database\/reset|discovery\/.*|static\/.*|themes\/.*|assets)(\(\.:format\))?$/,
route.path.spec.to_s,
"Unexpected new route: #{route.path.spec}")
end
diff --git a/services/api/test/integration/pipeline_test.rb b/services/api/test/integration/pipeline_test.rb
index a550246..6a264bb 100644
--- a/services/api/test/integration/pipeline_test.rb
+++ b/services/api/test/integration/pipeline_test.rb
@@ -1,6 +1,6 @@
require 'test_helper'
-class PipelineTest < ActionDispatch::IntegrationTest
+class PipelineIntegrationTest < ActionDispatch::IntegrationTest
# These tests simulate the workflow of arv-run-pipeline-instance
# and other pipeline-running code.
diff --git a/services/api/test/integration/reader_tokens_test.rb b/services/api/test/integration/reader_tokens_test.rb
index 6ed8461..23dd42f 100644
--- a/services/api/test/integration/reader_tokens_test.rb
+++ b/services/api/test/integration/reader_tokens_test.rb
@@ -1,6 +1,6 @@
require 'test_helper'
-class Arvados::V1::ReaderTokensTest < ActionController::IntegrationTest
+class ReaderTokensTest < ActionDispatch::IntegrationTest
fixtures :all
def spectator_specimen
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 86bc239..40292c2 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -23,6 +23,7 @@ end
require File.expand_path('../../config/environment', __FILE__)
require 'rails/test_help'
require 'mocha'
+require 'mocha/mini_test'
module ArvadosTestSupport
def json_response
@@ -122,8 +123,6 @@ class ActiveSupport::TestCase
def self.slow_test(name, &block)
define_method(name, block) unless skip_slow_tests?
end
-
- alias_method :skip, :omit
end
class ActionController::TestCase
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 6765814..67da717 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -53,15 +53,25 @@ class ArvadosModelTest < ActiveSupport::TestCase
{'a' => {'foo' => {:bar => 'baz'}}},
{'a' => {'foo' => {'bar' => :baz}}},
{'a' => {'foo' => ['bar', :baz]}},
+ ].each do |x|
+ test "prevent symbol keys in serialized db columns: #{x.inspect}" do
+ set_user_from_auth :active
+ link = Link.create!(link_class: 'test',
+ properties: x)
+ raw = ActiveRecord::Base.connection.
+ select_value("select properties from links where uuid='#{link.uuid}'")
+ refute_match(/:[fb]/, raw)
+ end
+ end
+
+ [ {['foo'] => 'bar'},
+ {'a' => {['foo', :foo] => 'bar'}},
+ {'a' => {{'foo' => 'bar'} => 'bar'}},
{'a' => {['foo', :foo] => ['bar', 'baz']}},
].each do |x|
- test "refuse symbol keys in serialized attribute: #{x.inspect}" do
- set_user_from_auth :admin_trustedclient
- assert_nothing_raised do
- Link.create!(link_class: 'test',
- properties: {})
- end
- assert_raises ActiveRecord::RecordInvalid do
+ test "refuse non-string keys in serialized db columns: #{x.inspect}" do
+ set_user_from_auth :active
+ assert_raises(ArgumentError) do
Link.create!(link_class: 'test',
properties: x)
end
@@ -81,10 +91,11 @@ class ArvadosModelTest < ActiveSupport::TestCase
test "No HashWithIndifferentAccess in database" do
set_user_from_auth :admin_trustedclient
- assert_raises ActiveRecord::RecordInvalid do
- Link.create!(link_class: 'test',
- properties: {'foo' => 'bar'}.with_indifferent_access)
- end
+ link = Link.create!(link_class: 'test',
+ properties: {'foo' => 'bar'}.with_indifferent_access)
+ raw = ActiveRecord::Base.connection.
+ select_value("select properties from links where uuid='#{link.uuid}'")
+ assert_equal '{"foo":"bar"}', raw
end
test "store long string" do
diff --git a/services/api/test/unit/create_superuser_token_test.rb b/services/api/test/unit/create_superuser_token_test.rb
index 122ae51..45446ec 100644
--- a/services/api/test/unit/create_superuser_token_test.rb
+++ b/services/api/test/unit/create_superuser_token_test.rb
@@ -90,7 +90,7 @@ class CreateSuperUserTokenTest < ActiveSupport::TestCase
active_user_token = api_client_authorizations("admin_vm").api_token
ApiClientAuthorization.
where(user_id: system_user.id).
- update_all(scopes: SafeJSON.dump(["GET /"]))
+ update_all(scopes: ["GET /"])
fixture_tokens = ApiClientAuthorization.all.collect(&:api_token)
new_token = create_superuser_token
refute_includes(fixture_tokens, new_token)
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 5677776..246f2d9 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -176,7 +176,7 @@ class JobTest < ActiveSupport::TestCase
[
{script_parameters: ""},
{script_parameters: []},
- {script_parameters: {symbols: :are_not_allowed_here}},
+ {script_parameters: {["foo"] => ["bar"]}},
{runtime_constraints: ""},
{runtime_constraints: []},
{tasks_summary: ""},
@@ -189,8 +189,8 @@ class JobTest < ActiveSupport::TestCase
# invalid_attrs.
Job.create! job_attrs
- job = Job.create job_attrs(invalid_attrs)
- assert_raises(ActiveRecord::RecordInvalid, ArgumentError,
+ job = Job.new(job_attrs(invalid_attrs))
+ assert_raises(ActiveRecord::RecordInvalid, ArgumentError, RuntimeError,
"save! did not raise the expected exception") do
job.save!
end
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 3bd6ed4..742deda 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -162,8 +162,8 @@ class UserTest < ActiveSupport::TestCase
if auto_admin_first_user_config
# This test requires no admin users exist (except for the system user)
users(:admin).delete
- @all_users = User.where("uuid not like '%-000000000000000'").where(:is_admin => true).find(:all)
- assert_equal 0, @all_users.size, "No admin users should exist (except for the system user)"
+ @all_users = User.where("uuid not like '%-000000000000000'").where(:is_admin => true)
+ assert_equal 0, @all_users.count, "No admin users should exist (except for the system user)"
end
Rails.configuration.auto_admin_first_user = auto_admin_first_user_config
@@ -285,7 +285,7 @@ class UserTest < ActiveSupport::TestCase
end
test "find user method checks" do
- User.find(:all).each do |user|
+ User.all.each do |user|
assert_not_nil user.uuid, "non-null uuid expected for " + user.full_name
end
@@ -313,14 +313,14 @@ class UserTest < ActiveSupport::TestCase
test "create new user" do
set_user_from_auth :admin
- @all_users = User.find(:all)
+ @all_users = User.all.to_a
user = User.new
user.first_name = "first_name_for_newly_created_user"
user.save
# verify there is one extra user in the db now
- assert_equal @all_users.size+1, User.find(:all).size
+ assert_equal @all_users.size+1, User.all.count
user = User.find(user.id) # get the user back
assert_equal(user.first_name, 'first_name_for_newly_created_user')
@@ -422,7 +422,7 @@ class UserTest < ActiveSupport::TestCase
@active_user.delete
found_deleted_user = false
- User.find(:all).each do |user|
+ User.all.each do |user|
if user.uuid == active_user_uuid
found_deleted_user = true
break
commit 84ad215752fde4291070143411a945fa7a94241c
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Mar 18 21:37:00 2017 -0400
7709: Update bundle
diff --git a/services/api/Gemfile b/services/api/Gemfile
index 39f217f..ab42bf7 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -26,6 +26,7 @@ gem 'pg'
# fix bug https://github.com/collectiveidea/json_spec/issues/27
gem 'multi_json'
gem 'oj'
+gem 'oj_mimic_json'
# Gems used only for assets and not required
# in production environments by default.
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 9c9c4ae..6217744 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -28,22 +28,23 @@ GEM
activesupport (3.2.22.5)
i18n (~> 0.6, >= 0.6.4)
multi_json (~> 1.0)
- acts_as_api (0.4.3)
+ acts_as_api (1.0.0)
activemodel (>= 3.0.0)
activesupport (>= 3.0.0)
rack (>= 1.1.0)
- addressable (2.4.0)
+ addressable (2.5.0)
+ public_suffix (~> 2.0, >= 2.0.2)
andand (1.3.3)
arel (3.0.3)
- arvados (0.1.20160513152536)
+ arvados (0.1.20170215224121)
activesupport (>= 3, < 4.2.6)
andand (~> 1.3, >= 1.3.3)
google-api-client (>= 0.7, < 0.8.9)
i18n (~> 0)
json (~> 1.7, >= 1.7.7)
jwt (>= 0.1.5, < 2)
- arvados-cli (0.1.20161017193526)
- activesupport (~> 3.2, >= 3.2.13)
+ arvados-cli (0.1.20170322173355)
+ activesupport (>= 3.2.13, < 5)
andand (~> 1.3, >= 1.3.3)
arvados (~> 0.1, >= 0.1.20150128223554)
curb (~> 0.8)
@@ -68,21 +69,21 @@ GEM
coffee-script (2.4.1)
coffee-script-source
execjs
- coffee-script-source (1.10.0)
+ coffee-script-source (1.12.2)
curb (0.9.3)
database_cleaner (1.5.3)
erubis (2.7.0)
- eventmachine (1.2.0.1)
+ eventmachine (1.2.3)
execjs (2.7.0)
extlib (0.9.16)
- factory_girl (4.7.0)
+ factory_girl (4.8.0)
activesupport (>= 3.0.0)
- factory_girl_rails (4.7.0)
- factory_girl (~> 4.7.0)
+ factory_girl_rails (4.8.0)
+ factory_girl (~> 4.8.0)
railties (>= 3.0.0)
- faraday (0.9.2)
+ faraday (0.11.0)
multipart-post (>= 1.2, < 3)
- faye-websocket (0.10.4)
+ faye-websocket (0.10.7)
eventmachine (>= 0.12.0)
websocket-driver (>= 0.5.1)
google-api-client (0.8.7)
@@ -104,21 +105,21 @@ GEM
multi_json (~> 1.11)
os (~> 0.9)
signet (~> 0.7)
- hashie (3.4.6)
+ hashie (3.5.5)
highline (1.7.8)
hike (1.2.3)
- i18n (0.7.0)
+ i18n (0.8.1)
journey (1.0.4)
jquery-rails (3.1.4)
railties (>= 3.0, < 5.0)
thor (>= 0.14, < 2.0)
- json (1.8.3)
+ json (1.8.6)
jwt (1.5.6)
launchy (2.4.3)
addressable (~> 2.3)
- libv8 (3.16.14.15)
+ libv8 (3.16.14.19)
little-plugger (1.1.4)
- logging (2.1.0)
+ logging (2.2.0)
little-plugger (~> 1.1)
multi_json (~> 1.10)
lograge (0.3.6)
@@ -132,44 +133,46 @@ GEM
memoist (0.15.0)
metaclass (0.0.4)
mime-types (1.25.1)
- mocha (1.2.0)
+ mocha (1.2.1)
metaclass (~> 0.0.1)
multi_json (1.12.1)
- multi_xml (0.5.5)
+ multi_xml (0.6.0)
multipart-post (2.0.0)
net-scp (1.2.1)
net-ssh (>= 2.6.5)
net-sftp (2.1.2)
net-ssh (>= 2.6.5)
- net-ssh (3.2.0)
- net-ssh-gateway (1.2.0)
- net-ssh (>= 2.6.5)
- oauth2 (1.2.0)
- faraday (>= 0.8, < 0.10)
+ net-ssh (4.1.0)
+ net-ssh-gateway (2.0.0)
+ net-ssh (>= 4.0.0)
+ oauth2 (1.3.1)
+ faraday (>= 0.8, < 0.12)
jwt (~> 1.0)
multi_json (~> 1.3)
multi_xml (~> 0.5)
rack (>= 1.2, < 3)
- oj (2.15.0)
- omniauth (1.3.1)
+ oj (2.18.5)
+ oj_mimic_json (1.0.1)
+ omniauth (1.4.2)
hashie (>= 1.2, < 4)
rack (>= 1.0, < 3)
omniauth-oauth2 (1.4.0)
oauth2 (~> 1.0)
omniauth (~> 1.2)
os (0.9.6)
- passenger (5.0.30)
+ passenger (5.1.2)
rack
rake (>= 0.8.1)
- pg (0.19.0)
+ pg (0.20.0)
pg_power (1.6.4)
pg
rails (~> 3.1)
polyglot (0.3.5)
- power_assert (0.3.1)
+ power_assert (1.0.1)
+ public_suffix (2.0.5)
puma (2.16.0)
rack (1.4.7)
- rack-cache (1.6.1)
+ rack-cache (1.7.0)
rack (>= 0.4)
rack-ssl (1.3.4)
rack
@@ -190,7 +193,7 @@ GEM
rake (>= 0.8.7)
rdoc (~> 3.4)
thor (>= 0.14.6, < 2.0)
- rake (11.3.0)
+ rake (12.0.0)
rdoc (3.12.2)
json (~> 1.4)
ref (2.0.0)
@@ -199,7 +202,7 @@ GEM
rvm-capistrano (1.5.6)
capistrano (~> 2.15.4)
safe_yaml (1.0.4)
- sass (3.4.22)
+ sass (3.4.23)
sass-rails (3.2.6)
railties (~> 3.2.0)
sass (>= 3.1.10)
@@ -220,27 +223,27 @@ GEM
multi_json (~> 1.0)
rack (~> 1.0)
tilt (~> 1.1, != 1.3.0)
- sshkey (1.8.0)
- test-unit (3.2.1)
+ sshkey (1.9.0)
+ test-unit (3.2.3)
power_assert
test_after_commit (1.1.0)
activerecord (>= 3.2)
themes_for_rails (0.5.1)
rails (>= 3.0.0)
- therubyracer (0.12.2)
- libv8 (~> 3.16.14.0)
+ therubyracer (0.12.3)
+ libv8 (~> 3.16.14.15)
ref
- thor (0.19.1)
+ thor (0.19.4)
tilt (1.4.1)
treetop (1.4.15)
polyglot
polyglot (>= 0.3.1)
trollop (2.1.2)
- tzinfo (0.3.51)
+ tzinfo (0.3.52)
uglifier (2.7.2)
execjs (>= 0.3.0)
json (>= 1.8.0)
- websocket-driver (0.6.4)
+ websocket-driver (0.6.5)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.2)
@@ -262,6 +265,7 @@ DEPENDENCIES
mocha
multi_json
oj
+ oj_mimic_json
omniauth (~> 1.1)
omniauth-oauth2 (~> 1.1)
passenger
@@ -284,4 +288,4 @@ DEPENDENCIES
uglifier (~> 2.0)
BUNDLED WITH
- 1.13.6
+ 1.14.3
commit c9a361f7fd3b1cf7f4959e9b0292d0f495d82771
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 11:59:40 2017 -0400
7709: De-duplicate "ensure unique name" implementations.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 2072520..71fb365 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -51,8 +51,6 @@ class ApplicationController < ActionController::Base
attr_writer :resource_attrs
- MAX_UNIQUE_NAME_ATTEMPTS = 10
-
begin
rescue_from(Exception,
ArvadosModel::PermissionDeniedError,
@@ -99,50 +97,12 @@ class ApplicationController < ActionController::Base
def create
@object = model_class.new resource_attrs
- if @object.respond_to? :name and params[:ensure_unique_name]
- # Record the original name. See below.
- name_stem = @object.name
- retries = MAX_UNIQUE_NAME_ATTEMPTS
+ if @object.respond_to?(:name) && params[:ensure_unique_name]
+ @object.save_with_unique_name!
else
- retries = 0
- end
-
- begin
@object.save!
- rescue ActiveRecord::RecordNotUnique => rn
- raise unless retries > 0
- retries -= 1
-
- # Dig into the error to determine if it is specifically calling out a
- # (owner_uuid, name) uniqueness violation. In this specific case, and
- # the client requested a unique name with ensure_unique_name==true,
- # update the name field and try to save again. Loop as necessary to
- # discover a unique name. It is necessary to handle name choosing at
- # this level (as opposed to the client) to ensure that record creation
- # never fails due to a race condition.
- raise unless rn.original_exception.is_a? PG::UniqueViolation
-
- # Unfortunately ActiveRecord doesn't abstract out any of the
- # necessary information to figure out if this the error is actually
- # the specific case where we want to apply the ensure_unique_name
- # behavior, so the following code is specialized to Postgres.
- err = rn.original_exception
- detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
- raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
-
- @object.uuid = nil
-
- new_name = "#{name_stem} (#{db_current_time.utc.iso8601(3)})"
- if new_name == @object.name
- # If the database is fast enough to do two attempts in the
- # same millisecond, we need to wait to ensure we try a
- # different timestamp on each attempt.
- sleep 0.002
- new_name = "#{name_stem} (#{db_current_time.utc.iso8601(3)})"
- end
- @object.name = new_name
- retry
end
+
show
end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 0419dad..b77ba1c 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -243,6 +243,57 @@ class ArvadosModel < ActiveRecord::Base
permission_link_classes: ['permission', 'resources'])
end
+ def save_with_unique_name!
+ uuid_was = uuid
+ name_was = name
+ max_retries = 2
+ transaction do
+ conn = ActiveRecord::Base.connection
+ conn.exec_query 'SAVEPOINT save_with_unique_name'
+ begin
+ save!
+ rescue ActiveRecord::RecordNotUnique => rn
+ raise if max_retries == 0
+ max_retries -= 1
+
+ conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name'
+
+ # Dig into the error to determine if it is specifically calling out a
+ # (owner_uuid, name) uniqueness violation. In this specific case, and
+ # the client requested a unique name with ensure_unique_name==true,
+ # update the name field and try to save again. Loop as necessary to
+ # discover a unique name. It is necessary to handle name choosing at
+ # this level (as opposed to the client) to ensure that record creation
+ # never fails due to a race condition.
+ err = rn.original_exception
+ raise unless err.is_a?(PG::UniqueViolation)
+
+ # Unfortunately ActiveRecord doesn't abstract out any of the
+ # necessary information to figure out if this the error is actually
+ # the specific case where we want to apply the ensure_unique_name
+ # behavior, so the following code is specialized to Postgres.
+ detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
+ raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
+
+ new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})"
+ if new_name == name
+ # If the database is fast enough to do two attempts in the
+ # same millisecond, we need to wait to ensure we try a
+ # different timestamp on each attempt.
+ sleep 0.002
+ new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})"
+ end
+
+ self[:name] = new_name
+ self[:uuid] = nil if uuid_was.nil? && !uuid.nil?
+ conn.exec_query 'SAVEPOINT save_with_unique_name'
+ retry
+ ensure
+ conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name'
+ end
+ end
+ end
+
def logged_attributes
attributes.except(*Rails.configuration.unlogged_attributes)
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index a3cc9c1..9420ef3 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -82,15 +82,102 @@ class Container < ArvadosModel
end
end
+ # Create a new container (or find an existing one) to satisfy the
+ # given container request.
+ def self.resolve(req)
+ c_attrs = {
+ command: req.command,
+ cwd: req.cwd,
+ environment: req.environment,
+ output_path: req.output_path,
+ container_image: resolve_container_image(req.container_image),
+ mounts: resolve_mounts(req.mounts),
+ runtime_constraints: resolve_runtime_constraints(req.runtime_constraints),
+ scheduling_parameters: req.scheduling_parameters,
+ }
+ act_as_system_user do
+ if req.use_existing && (reusable = find_reusable(c_attrs))
+ reusable
+ else
+ Container.create!(c_attrs)
+ end
+ end
+ end
+
+ # Return a runtime_constraints hash that complies with requested but
+ # is suitable for saving in a container record, i.e., has specific
+ # values instead of ranges.
+ #
+ # Doing this as a step separate from other resolutions, like "git
+ # revision range to commit hash", makes sense only when there is no
+ # opportunity to reuse an existing container (e.g., container reuse
+ # is not implemented yet, or we have already found that no existing
+ # containers are suitable).
+ def self.resolve_runtime_constraints(runtime_constraints)
+ rc = {}
+ defaults = {
+ 'keep_cache_ram' =>
+ Rails.configuration.container_default_keep_cache_ram,
+ }
+ defaults.merge(runtime_constraints).each do |k, v|
+ if v.is_a? Array
+ rc[k] = v[0]
+ else
+ rc[k] = v
+ end
+ end
+ rc
+ end
+
+ # Return a mounts hash suitable for a Container, i.e., with every
+ # readonly collection UUID resolved to a PDH.
+ def self.resolve_mounts(mounts)
+ c_mounts = {}
+ mounts.each do |k, mount|
+ mount = mount.dup
+ c_mounts[k] = mount
+ if mount['kind'] != 'collection'
+ next
+ end
+ if (uuid = mount.delete 'uuid')
+ c = Collection.
+ readable_by(current_user).
+ where(uuid: uuid).
+ select(:portable_data_hash).
+ first
+ if !c
+ raise ArvadosModel::UnresolvableContainerError.new "cannot mount collection #{uuid.inspect}: not found"
+ end
+ if mount['portable_data_hash'].nil?
+ # PDH not supplied by client
+ mount['portable_data_hash'] = c.portable_data_hash
+ elsif mount['portable_data_hash'] != c.portable_data_hash
+ # UUID and PDH supplied by client, but they don't agree
+ raise ArgumentError.new "cannot mount collection #{uuid.inspect}: current portable_data_hash #{c.portable_data_hash.inspect} does not match #{c['portable_data_hash'].inspect} in request"
+ end
+ end
+ end
+ return c_mounts
+ end
+
+ # Return a container_image PDH suitable for a Container.
+ def self.resolve_container_image(container_image)
+ coll = Collection.for_latest_docker_image(container_image)
+ if !coll
+ raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
+ end
+ coll.portable_data_hash
+ end
+
def self.find_reusable(attrs)
candidates = Container.
where_serialized(:command, attrs[:command]).
where('cwd = ?', attrs[:cwd]).
where_serialized(:environment, attrs[:environment]).
where('output_path = ?', attrs[:output_path]).
- where('container_image = ?', attrs[:container_image]).
- where_serialized(:mounts, attrs[:mounts]).
- where_serialized(:runtime_constraints, attrs[:runtime_constraints])
+ where('container_image = ?', resolve_container_image(attrs[:container_image])).
+ where_serialized(:mounts, resolve_mounts(attrs[:mounts])).
+ where_serialized(:runtime_constraints, resolve_runtime_constraints(attrs[:runtime_constraints]))
# Check for Completed candidates whose output and log are both readable.
select_readable_pdh = Collection.
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 6cb9fd8..694c174 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -99,33 +99,16 @@ class ContainerRequest < ArvadosModel
manifest = Collection.unscoped do
Collection.where(portable_data_hash: pdh).first.manifest_text
end
- begin
- coll = Collection.create!(owner_uuid: owner_uuid,
- manifest_text: manifest,
- portable_data_hash: pdh,
- name: coll_name,
- properties: {
- 'type' => out_type,
- 'container_request' => uuid,
- })
- rescue ActiveRecord::RecordNotUnique => rn
- # In case this is executed as part of a transaction: When a Postgres exception happens,
- # the following statements on the same transaction become invalid, so a rollback is
- # needed. One example are Unit Tests, every test is enclosed inside a transaction so
- # that the database can be reverted before every new test starts.
- # See: http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Exception+handling+and+rolling+back
- ActiveRecord::Base.connection.execute 'ROLLBACK'
- raise unless out_type == 'output' and self.output_name
- # Postgres specific unique name check. See ApplicationController#create for
- # a detailed explanation.
- raise unless rn.original_exception.is_a? PG::UniqueViolation
- err = rn.original_exception
- detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
- raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
- # Output collection name collision detected: append a timestamp.
- coll_name = "#{self.output_name} #{Time.now.getgm.strftime('%FT%TZ')}"
- retry
- end
+
+ coll = Collection.new(owner_uuid: owner_uuid,
+ manifest_text: manifest,
+ portable_data_hash: pdh,
+ name: coll_name,
+ properties: {
+ 'type' => out_type,
+ 'container_request' => uuid,
+ })
+ coll.save_with_unique_name!
if out_type == 'output'
out_coll = coll.uuid
else
@@ -151,96 +134,6 @@ class ContainerRequest < ArvadosModel
self.scheduling_parameters ||= {}
end
- # Create a new container (or find an existing one) to satisfy this
- # request.
- def resolve
- c_mounts = mounts_for_container
- c_runtime_constraints = {
- 'keep_cache_ram' =>
- Rails.configuration.container_default_keep_cache_ram,
- }.merge(runtime_constraints_for_container)
- c_container_image = container_image_for_container
- c = act_as_system_user do
- c_attrs = {command: self.command,
- cwd: self.cwd,
- environment: self.environment,
- output_path: self.output_path,
- container_image: c_container_image,
- mounts: c_mounts,
- runtime_constraints: c_runtime_constraints}
-
- reusable = self.use_existing ? Container.find_reusable(c_attrs) : nil
- if not reusable.nil?
- reusable
- else
- c_attrs[:scheduling_parameters] = self.scheduling_parameters
- Container.create!(c_attrs)
- end
- end
- self.container_uuid = c.uuid
- end
-
- # Return a runtime_constraints hash that complies with
- # self.runtime_constraints but is suitable for saving in a container
- # record, i.e., has specific values instead of ranges.
- #
- # Doing this as a step separate from other resolutions, like "git
- # revision range to commit hash", makes sense only when there is no
- # opportunity to reuse an existing container (e.g., container reuse
- # is not implemented yet, or we have already found that no existing
- # containers are suitable).
- def runtime_constraints_for_container
- rc = {}
- runtime_constraints.each do |k, v|
- if v.is_a? Array
- rc[k] = v[0]
- else
- rc[k] = v
- end
- end
- rc
- end
-
- # Return a mounts hash suitable for a Container, i.e., with every
- # readonly collection UUID resolved to a PDH.
- def mounts_for_container
- c_mounts = {}
- mounts.each do |k, mount|
- mount = mount.dup
- c_mounts[k] = mount
- if mount['kind'] != 'collection'
- next
- end
- if (uuid = mount.delete 'uuid')
- c = Collection.
- readable_by(current_user).
- where(uuid: uuid).
- select(:portable_data_hash).
- first
- if !c
- raise ArvadosModel::UnresolvableContainerError.new "cannot mount collection #{uuid.inspect}: not found"
- end
- if mount['portable_data_hash'].nil?
- # PDH not supplied by client
- mount['portable_data_hash'] = c.portable_data_hash
- elsif mount['portable_data_hash'] != c.portable_data_hash
- # UUID and PDH supplied by client, but they don't agree
- raise ArgumentError.new "cannot mount collection #{uuid.inspect}: current portable_data_hash #{c.portable_data_hash.inspect} does not match #{c['portable_data_hash'].inspect} in request"
- end
- end
- end
- return c_mounts
- end
-
- # Return a container_image PDH suitable for a Container.
- def container_image_for_container
- coll = Collection.for_latest_docker_image(container_image)
- if !coll
- raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
- end
- coll.portable_data_hash
- end
-
def set_container
if (container_uuid_changed? and
not current_user.andand.is_admin and
@@ -249,7 +142,7 @@ class ContainerRequest < ArvadosModel
return false
end
if state_changed? and state == Committed and container_uuid.nil?
- resolve
+ self.container_uuid = Container.resolve(self).uuid
end
if self.container_uuid != self.container_uuid_was
if self.container_count_changed?
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index df3a2c3..b268ce4 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -312,8 +312,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
lambda { |resolved| resolved["ram"] == 1234234234 }],
].each do |rc, okfunc|
test "resolve runtime constraint range #{rc} to values" do
- cr = ContainerRequest.new(runtime_constraints: rc)
- resolved = cr.send :runtime_constraints_for_container
+ resolved = Container.resolve_runtime_constraints(rc)
assert(okfunc.call(resolved),
"container runtime_constraints was #{resolved.inspect}")
end
@@ -345,10 +344,9 @@ class ContainerRequestTest < ActiveSupport::TestCase
].each do |mounts, okfunc|
test "resolve mounts #{mounts.inspect} to values" do
set_user_from_auth :active
- cr = ContainerRequest.new(mounts: mounts)
- resolved = cr.send :mounts_for_container
+ resolved = Container.resolve_mounts(mounts)
assert(okfunc.call(resolved),
- "mounts_for_container returned #{resolved.inspect}")
+ "Container.resolve_mounts returned #{resolved.inspect}")
end
end
@@ -361,9 +359,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
"path" => "/foo",
},
}
- cr = ContainerRequest.new(mounts: m)
assert_raises(ArvadosModel::UnresolvableContainerError) do
- cr.send :mounts_for_container
+ Container.resolve_mounts(m)
end
end
@@ -377,9 +374,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
"path" => "/foo",
},
}
- cr = ContainerRequest.new(mounts: m)
assert_raises(ArgumentError) do
- cr.send :mounts_for_container
+ Container.resolve_mounts(m)
end
end
@@ -387,21 +383,19 @@ class ContainerRequestTest < ActiveSupport::TestCase
'arvados/apitestfixture',
'd8309758b8fe2c81034ffc8a10c36460b77db7bc5e7b448c4e5b684f9d95a678',
].each do |tag|
- test "container_image_for_container(#{tag.inspect})" do
+ test "Container.resolve_container_image(#{tag.inspect})" do
set_user_from_auth :active
- cr = ContainerRequest.new(container_image: tag)
- resolved = cr.send :container_image_for_container
+ resolved = Container.resolve_container_image(tag)
assert_equal resolved, collections(:docker_image).portable_data_hash
end
end
- test "container_image_for_container(pdh)" do
+ test "Container.resolve_container_image(pdh)" do
set_user_from_auth :active
[[:docker_image, 'v1'], [:docker_image_1_12, 'v2']].each do |coll, ver|
Rails.configuration.docker_image_formats = [ver]
pdh = collections(coll).portable_data_hash
- cr = ContainerRequest.new(container_image: pdh)
- resolved = cr.send :container_image_for_container
+ resolved = Container.resolve_container_image(pdh)
assert_equal resolved, pdh
end
end
@@ -412,9 +406,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
].each do |img|
test "container_image_for_container(#{img.inspect}) => 422" do
set_user_from_auth :active
- cr = ContainerRequest.new(container_image: img)
assert_raises(ArvadosModel::UnresolvableContainerError) do
- cr.send :container_image_for_container
+ Container.resolve_container_image(img)
end
end
end
@@ -428,12 +421,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
set_user_from_auth :active
cr = create_minimal_req!(command: ["true", "1"],
container_image: collections(:docker_image).portable_data_hash)
- assert_equal(cr.send(:container_image_for_container),
+ assert_equal(Container.resolve_container_image(cr.container_image),
collections(:docker_image_1_12).portable_data_hash)
cr = create_minimal_req!(command: ["true", "2"],
container_image: links(:docker_image_collection_tag).name)
- assert_equal(cr.send(:container_image_for_container),
+ assert_equal(Container.resolve_container_image(cr.container_image),
collections(:docker_image_1_12).portable_data_hash)
end
@@ -447,12 +440,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
set_user_from_auth :active
cr = create_minimal_req!(command: ["true", "1"],
container_image: collections(:docker_image).portable_data_hash)
- assert_equal(cr.send(:container_image_for_container),
+ assert_equal(Container.resolve_container_image(cr.container_image),
collections(:docker_image).portable_data_hash)
cr = create_minimal_req!(command: ["true", "2"],
container_image: links(:docker_image_collection_tag).name)
- assert_equal(cr.send(:container_image_for_container),
+ assert_equal(Container.resolve_container_image(cr.container_image),
collections(:docker_image).portable_data_hash)
end
@@ -465,7 +458,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr = create_minimal_req!(command: ["true", "1"],
container_image: collections(:docker_image_1_12).portable_data_hash)
assert_raises(ArvadosModel::UnresolvableContainerError) do
- cr.send(:container_image_for_container)
+ Container.resolve_container_image(cr.container_image)
end
end
@@ -477,12 +470,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr = create_minimal_req!(command: ["true", "1"],
container_image: collections(:docker_image).portable_data_hash)
assert_raises(ArvadosModel::UnresolvableContainerError) do
- cr.send(:container_image_for_container)
+ Container.resolve_container_image(cr.container_image)
end
cr = create_minimal_req!(command: ["true", "2"],
container_image: links(:docker_image_collection_tag).name)
assert_raises(ArvadosModel::UnresolvableContainerError) do
- cr.send(:container_image_for_container)
+ Container.resolve_container_image(cr.container_image)
end
end
@@ -609,7 +602,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
"It shouldn't exist more than one collection with the same owner and name '${output_name}'"
assert output_coll.name.include?(output_name),
"New name should include original name"
- assert_match /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z/, output_coll.name,
+ assert_match /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z/, output_coll.name,
"New name should include ISO8601 date"
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 50f2ec5..52d2aa6 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -157,21 +157,21 @@ class ContainerTest < ActiveSupport::TestCase
log: 'ea10d51bcf88862dbcc36eb292017dfd+45',
}
- set_user_from_auth :dispatch1
-
- c_output1 = Container.create common_attrs
- c_output2 = Container.create common_attrs
- assert_not_equal c_output1.uuid, c_output2.uuid
-
cr = ContainerRequest.new common_attrs
+ cr.use_existing = false
cr.state = ContainerRequest::Committed
- cr.container_uuid = c_output1.uuid
cr.save!
+ c_output1 = Container.where(uuid: cr.container_uuid).first
cr = ContainerRequest.new common_attrs
+ cr.use_existing = false
cr.state = ContainerRequest::Committed
- cr.container_uuid = c_output2.uuid
cr.save!
+ c_output2 = Container.where(uuid: cr.container_uuid).first
+
+ assert_not_equal c_output1.uuid, c_output2.uuid
+
+ set_user_from_auth :dispatch1
out1 = '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'
log1 = collections(:real_log_collection).portable_data_hash
@@ -184,9 +184,8 @@ class ContainerTest < ActiveSupport::TestCase
c_output2.update_attributes!({state: Container::Running})
c_output2.update_attributes!(completed_attrs.merge({log: log1, output: out2}))
- reused = Container.find_reusable(common_attrs)
- assert_not_nil reused
- assert_equal reused.uuid, c_output1.uuid
+ reused = Container.resolve(ContainerRequest.new(common_attrs))
+ assert_equal c_output1.uuid, reused.uuid
end
test "find_reusable method should select running container by start date" do
commit 05d453ec38b10a022ea6db77867957e7115b9b35
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 12:49:06 2017 -0400
7709: Do not set job_readable when it won't be used.
diff --git a/services/api/app/controllers/arvados/v1/nodes_controller.rb b/services/api/app/controllers/arvados/v1/nodes_controller.rb
index 5e2404e..023d2ff 100644
--- a/services/api/app/controllers/arvados/v1/nodes_controller.rb
+++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb
@@ -46,10 +46,12 @@ class Arvados::V1::NodesController < ApplicationController
@objects = model_class.where('last_ping_at >= ?', db_current_time - 1.hours)
end
super
- job_uuids = @objects.map { |n| n[:job_uuid] }.compact
- assoc_jobs = readable_job_uuids(job_uuids)
- @objects.each do |node|
- node.job_readable = assoc_jobs.include?(node[:job_uuid])
+ if @select.nil? or @select.include? 'job_uuid'
+ job_uuids = @objects.map { |n| n[:job_uuid] }.compact
+ assoc_jobs = readable_job_uuids(job_uuids)
+ @objects = @objects.each do |node|
+ node.job_readable = assoc_jobs.include?(node[:job_uuid])
+ end
end
end
commit 09dcf71e59907c2eaf4b94918c63da07193481a4
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 11:56:17 2017 -0400
7709: Leave container_request alone when applying default keep_cache_ram value.
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 87c3ebe..6cb9fd8 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -155,7 +155,10 @@ class ContainerRequest < ArvadosModel
# request.
def resolve
c_mounts = mounts_for_container
- c_runtime_constraints = runtime_constraints_for_container
+ c_runtime_constraints = {
+ 'keep_cache_ram' =>
+ Rails.configuration.container_default_keep_cache_ram,
+ }.merge(runtime_constraints_for_container)
c_container_image = container_image_for_container
c = act_as_system_user do
c_attrs = {command: self.command,
@@ -261,20 +264,17 @@ class ContainerRequest < ArvadosModel
def validate_runtime_constraints
case self.state
when Committed
- ['vcpus', 'ram'].each do |k|
- if not (runtime_constraints.include? k and
- runtime_constraints[k].is_a? Integer and
- runtime_constraints[k] > 0)
- errors.add :runtime_constraints, "#{k} must be a positive integer"
+ [['vcpus', true],
+ ['ram', true],
+ ['keep_cache_ram', false]].each do |k, required|
+ if !required && !runtime_constraints.include?(k)
+ next
+ end
+ v = runtime_constraints[k]
+ unless (v.is_a?(Integer) && v > 0)
+ errors.add(:runtime_constraints,
+ "[#{k}]=#{v.inspect} must be a positive integer")
end
- end
-
- if runtime_constraints.include? 'keep_cache_ram' and
- (!runtime_constraints['keep_cache_ram'].is_a?(Integer) or
- runtime_constraints['keep_cache_ram'] <= 0)
- errors.add :runtime_constraints, "keep_cache_ram must be a positive integer"
- elsif !runtime_constraints.include? 'keep_cache_ram'
- runtime_constraints['keep_cache_ram'] = Rails.configuration.container_default_keep_cache_ram
end
end
end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index af1d4b2..df3a2c3 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -123,6 +123,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr.reload
+ assert_equal({"vcpus" => 2, "ram" => 30}, cr.runtime_constraints)
+
assert_not_nil cr.container_uuid
c = Container.find_by_uuid cr.container_uuid
assert_not_nil c
@@ -502,8 +504,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
command: ["echo", "hello"],
output_path: "test",
runtime_constraints: {"vcpus" => 4,
- "ram" => 12000000000,
- "keep_cache_ram" => 268435456},
+ "ram" => 12000000000},
mounts: {"test" => {"kind" => "json"}}}
set_user_from_auth :active
cr1 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Committed,
@@ -641,34 +642,6 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
[
- [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => 100}, ContainerRequest::Committed, 100],
- [{"vcpus" => 1, "ram" => 123}, ContainerRequest::Uncommitted],
- [{"vcpus" => 1, "ram" => 123}, ContainerRequest::Committed],
- [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => -1}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
- [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => '123'}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
- ].each do |rc, state, expected|
- test "create container request with #{rc} in state #{state} and verify keep_cache_ram #{expected}" do
- common_attrs = {cwd: "test",
- priority: 1,
- command: ["echo", "hello"],
- output_path: "test",
- runtime_constraints: rc,
- mounts: {"test" => {"kind" => "json"}}}
- set_user_from_auth :active
-
- if expected == ActiveRecord::RecordInvalid
- assert_raises(ActiveRecord::RecordInvalid) do
- create_minimal_req!(common_attrs.merge({state: state}))
- end
- else
- cr = create_minimal_req!(common_attrs.merge({state: state}))
- expected = Rails.configuration.container_default_keep_cache_ram if state == ContainerRequest::Committed and expected.nil?
- assert_equal expected, cr.runtime_constraints['keep_cache_ram']
- end
- end
- end
-
- [
[{"partitions" => ["fastcpu","vfastcpu", 100]}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
[{"partitions" => ["fastcpu","vfastcpu", 100]}, ContainerRequest::Uncommitted],
[{"partitions" => "fastcpu"}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 5a19f05..50f2ec5 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -11,14 +11,22 @@ class ContainerTest < ActiveSupport::TestCase
runtime_constraints: {"vcpus" => 1, "ram" => 1},
}
- REUSABLE_COMMON_ATTRS = {container_image: "9ae44d5792468c58bcf85ce7353c7027+124",
- cwd: "test",
- command: ["echo", "hello"],
- output_path: "test",
- runtime_constraints: {"vcpus" => 4,
- "ram" => 12000000000},
- mounts: {"test" => {"kind" => "json"}},
- environment: {"var" => 'val'}}
+ REUSABLE_COMMON_ATTRS = {
+ container_image: "9ae44d5792468c58bcf85ce7353c7027+124",
+ cwd: "test",
+ command: ["echo", "hello"],
+ output_path: "test",
+ runtime_constraints: {
+ "ram" => 12000000000,
+ "vcpus" => 4,
+ },
+ mounts: {
+ "test" => {"kind" => "json"},
+ },
+ environment: {
+ "var" => "val",
+ },
+ }
def minimal_new attrs={}
cr = ContainerRequest.new DEFAULT_ATTRS.merge(attrs)
@@ -86,7 +94,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container serialized hash attributes sorted before save" do
env = {"C" => 3, "B" => 2, "A" => 1}
m = {"F" => {"kind" => 3}, "E" => {"kind" => 2}, "D" => {"kind" => 1}}
- rc = {"vcpus" => 1, "ram" => 1}
+ rc = {"vcpus" => 1, "ram" => 1, "keep_cache_ram" => 1}
c, _ = minimal_new(environment: env, mounts: m, runtime_constraints: rc)
assert_equal c.environment.to_json, Container.deep_sort_hash(env).to_json
assert_equal c.mounts.to_json, Container.deep_sort_hash(m).to_json
commit 099a8c62fcb0905855ddf243a3deddc7398c3c10
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 21 12:01:33 2017 -0400
7709: Fix tests (count existing links before adding new ones).
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 579b8cc..98643a9 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -6,7 +6,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
include UsersTestHelper
setup do
- @all_links_at_start = Link.all
+ @initial_link_count = Link.count
@vm_uuid = virtual_machines(:testvm).uuid
end
@@ -107,7 +107,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_nil created['identity_url'], 'expected no identity_url'
# arvados#user, repo link and link add user to 'All users' group
- verify_num_links @all_links_at_start, 4
+ verify_links_added 4
verify_link response_items, 'arvados#user', true, 'permission', 'can_login',
created['uuid'], created['email'], 'arvados#user', false, 'User'
@@ -269,7 +269,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_equal response_object['email'], 'foo at example.com', 'expected given email'
# four extra links; system_group, login, group and repo perms
- verify_num_links @all_links_at_start, 4
+ verify_links_added 4
end
test "setup user with fake vm and expect error" do
@@ -306,7 +306,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_equal response_object['email'], 'foo at example.com', 'expected given email'
# five extra links; system_group, login, group, vm, repo
- verify_num_links @all_links_at_start, 5
+ verify_links_added 5
end
test "setup user with valid email, no vm and no repo as input" do
@@ -324,7 +324,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_equal response_object['email'], 'foo at example.com', 'expected given email'
# three extra links; system_group, login, and group
- verify_num_links @all_links_at_start, 3
+ verify_links_added 3
verify_link response_items, 'arvados#user', true, 'permission', 'can_login',
response_object['uuid'], response_object['email'], 'arvados#user', false, 'User'
@@ -361,7 +361,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
'expecting first name'
# five extra links; system_group, login, group, repo and vm
- verify_num_links @all_links_at_start, 5
+ verify_links_added 5
end
test "setup user with an existing user email and check different object is created" do
@@ -384,7 +384,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
'expected different uuid after create operation'
assert_equal inactive_user['email'], response_object['email'], 'expected given email'
# system_group, openid, group, and repo. No vm link.
- verify_num_links @all_links_at_start, 4
+ verify_links_added 4
end
test "setup user with openid prefix" do
@@ -412,7 +412,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
# verify links
# four new links: system_group, arvados#user, repo, and 'All users' group.
- verify_num_links @all_links_at_start, 4
+ verify_links_added 4
verify_link response_items, 'arvados#user', true, 'permission', 'can_login',
created['uuid'], created['email'], 'arvados#user', false, 'User'
@@ -472,7 +472,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
# five new links: system_group, arvados#user, repo, vm and 'All
# users' group link
- verify_num_links @all_links_at_start, 5
+ verify_links_added 5
verify_link response_items, 'arvados#user', true, 'permission', 'can_login',
created['uuid'], created['email'], 'arvados#user', false, 'User'
@@ -841,9 +841,9 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
"admin's filtered index did not return inactive user")
end
- def verify_num_links (original_links, expected_additional_links)
- assert_equal expected_additional_links, Link.all.size-original_links.size,
- "Expected #{expected_additional_links.inspect} more links"
+ def verify_links_added more
+ assert_equal @initial_link_count+more, Link.count,
+ "Started with #{@initial_link_count} links, expected #{more} more"
end
def find_obj_in_resp (response_items, object_type, head_kind=nil)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list