[ARVADOS] created: 5bc1b840c803ff725c480e21380b3dac4856df6d
Git user
git at public.curoverse.com
Wed Mar 22 02:44:03 EDT 2017
at 5bc1b840c803ff725c480e21380b3dac4856df6d (commit)
commit 5bc1b840c803ff725c480e21380b3dac4856df6d
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Mar 22 02:41:06 2017 -0400
7709: Fix broken link from crunch2 to crunch1 docs.
diff --git a/doc/install/crunch2-slurm/install-compute-node.html.textile.liquid b/doc/install/crunch2-slurm/install-compute-node.html.textile.liquid
index 330cc3a..7cf1e9e 100644
--- a/doc/install/crunch2-slurm/install-compute-node.html.textile.liquid
+++ b/doc/install/crunch2-slurm/install-compute-node.html.textile.liquid
@@ -33,8 +33,8 @@ On Debian-based systems:
h2. Set up SLURM
-Install SLURM following "the same process you used to install the Crunch dispatcher":install-crunch-dispatch.html#slurm.
+Install SLURM on the compute node using the same process you used on the API server in the "previous step":install-slurm.html.
-h2. Copy configuration files from the dispatcher (API server)
+The @slurm.conf@ and @/etc/munge/munge.key@ files must be identical on all SLURM nodes. Copy the files you created on the API server in the "previous step":install-slurm.html to each compute node.
-The @slurm.conf@ and @/etc/munge/munge.key@ files need to be identical across the dispatcher and all compute nodes. Copy the files you created in the "Install the Crunch dispatcher":install-crunch-dispatch.html step to this compute node.
+Ensure the @crunch@ user exists and has the same UID and GID on the API server and all compute nodes.
commit 94068b9bbf6fd1ccc779d67f58dbdc16ab0fe209
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 772511f3f6a7e35704bfd351abfb76d3bba191d5
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 e06f9d1..f9192ee 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 bb17c578233a311a0c80e21e649ed87b5c07ef94
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 b4951aa7a4fa795e18337f337fc249508d38fc98
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 9ea7265..b5215ab 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -459,6 +459,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 5c304fbf1bfc270e8b60ab9014db30cb6b25c7e4
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 43b0f46..e06f9d1 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -539,8 +539,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 8053d3b0648a74f32fd25d8e404fa8260053e741
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 30ff3e0..cf194c9 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.3)
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 fbbf53e..2b5bf83 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/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 f37afb3..43b0f46 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.
@@ -461,6 +481,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
@@ -469,6 +490,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|
@@ -515,25 +549,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
@@ -546,8 +573,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 3207d1f..ae3f536 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -99,7 +99,7 @@ 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 cae6bbd..9ea7265 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -423,7 +423,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
@@ -433,7 +432,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
@@ -441,7 +440,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
@@ -450,7 +449,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 4f81fec4a9355988f5b758b25528457b9693c8d8
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..30ff3e0 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.20170319011245)
+ 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.3)
+ 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 b9dcb648165b158f5721500caed649a49715cd14
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..f37afb3 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -243,6 +243,55 @@ class ArvadosModel < ActiveRecord::Base
permission_link_classes: ['permission', 'resources'])
end
+ def save_with_unique_name!
+ uuid_was = uuid
+ name_was = name
+ max_retries = 2
+ 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
+
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 54d7045bc22a2e4314745ea732a3922ea1a40e97
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 9cdf7ca4ffb19dc33f37fc754cb1b73dcd29de11
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 6c49531f860d30ef50d08846c6869aef71dba0cd
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)
commit 3a022c4f733ed7325106a03cd9718c1274e9ed8e
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Mar 18 21:12:45 2017 -0400
7709: arvados-cli allows activesupport>=4
diff --git a/sdk/cli/arvados-cli.gemspec b/sdk/cli/arvados-cli.gemspec
index 0eeee57..651ebf2 100644
--- a/sdk/cli/arvados-cli.gemspec
+++ b/sdk/cli/arvados-cli.gemspec
@@ -28,7 +28,7 @@ Gem::Specification.new do |s|
# Our google-api-client dependency used to be < 0.9, but that could be
# satisfied by the buggy 0.9.pre*. https://dev.arvados.org/issues/9213
s.add_runtime_dependency 'google-api-client', '~> 0.6', '>= 0.6.3', '<0.8.9'
- s.add_runtime_dependency 'activesupport', '~> 3.2', '>= 3.2.13'
+ s.add_runtime_dependency 'activesupport', '>= 3.2.13', '< 5'
s.add_runtime_dependency 'json', '~> 1.7', '>= 1.7.7'
s.add_runtime_dependency 'trollop', '~> 2.0'
s.add_runtime_dependency 'andand', '~> 1.3', '>= 1.3.3'
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list