[ARVADOS] updated: 86c192b3ae3614f69305cfd60664c9b720b84692
git at public.curoverse.com
git at public.curoverse.com
Wed Mar 19 11:33:15 EDT 2014
Summary of changes:
.../app/controllers/arvados/v1/jobs_controller.rb | 8 +++-
services/api/app/models/commit.rb | 19 +++++++--
services/api/app/models/job.rb | 2 +-
services/api/test/fixtures/links.yml | 15 +++++++
services/api/test/fixtures/repositories.yml | 2 +-
.../arvados/v1/commits_controller_test.rb | 28 +++++++++----
.../api/test/functional/arvados/v1/git_setup.rb | 10 +++++
.../arvados/v1/job_reuse_controller_test.rb | 42 +++++++++++++++++++-
8 files changed, 108 insertions(+), 18 deletions(-)
via 86c192b3ae3614f69305cfd60664c9b720b84692 (commit)
from 9c3904d27dc7d6b6aa5834ff0f5815a8b3685e99 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 86c192b3ae3614f69305cfd60664c9b720b84692
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Wed Mar 19 11:33:08 2014 -0400
Added tests for permissions to reuse jobs, tests for excluding commits.
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 062a56e..c8fcd22 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -6,11 +6,11 @@ class Arvados::V1::JobsController < ApplicationController
skip_before_filter :render_404_if_no_object, :only => :queue
def create
- puts resource_attrs
r = Commit.find_commit_range(current_user,
resource_attrs[:repository],
resource_attrs[:minimum_script_version],
- resource_attrs[:script_version])
+ resource_attrs[:script_version],
+ resource_attrs[:exclude_script_versions])
if !resource_attrs[:nondeterministic]
Job.readable_by(current_user).where(script: resource_attrs[:script],
script_version: r).
@@ -24,6 +24,10 @@ class Arvados::V1::JobsController < ApplicationController
if r
resource_attrs[:script_version] = r[0]
end
+
+ # Don't pass these on to activerecord
+ resource_attrs.delete(:minimum_script_version)
+ resource_attrs.delete(:exclude_script_versions)
super
end
diff --git a/services/api/app/models/commit.rb b/services/api/app/models/commit.rb
index 5e29c76..7979ff4 100644
--- a/services/api/app/models/commit.rb
+++ b/services/api/app/models/commit.rb
@@ -39,7 +39,7 @@ class Commit < ActiveRecord::Base
# nil
# end
- def self.find_commit_range(current_user, repository, minimum, maximum)
+ def self.find_commit_range(current_user, repository, minimum, maximum, exclude)
only_valid_chars = /[^A-Za-z0-9_-]/
if only_valid_chars.match(minimum) || only_valid_chars.match(maximum)
logger.warn "find_commit_range called with string containing invalid characters: '#{minimum}', '#{maximum}'"
@@ -90,6 +90,16 @@ class Commit < ActiveRecord::Base
# If not found, nothing else to do
next if !max_hash
+ resolved_exclude = nil
+ if exclude
+ resolved_exclude = []
+ exclude.each do |e|
+ IO.foreach("|git rev-list --max-count=1 #{e}") do |line|
+ resolved_exclude.push(line.strip)
+ end
+ end
+ end
+
if minimum
# Get the commit hash for the lower bound
min_hash = nil
@@ -103,12 +113,13 @@ class Commit < ActiveRecord::Base
# Now find all commits between them
#puts "git rev-list #{min_hash}..#{max_hash}"
IO.foreach("|git rev-list #{min_hash}..#{max_hash}") do |line|
- commits.push(line.strip)
+ hash = line.strip
+ commits.push(hash) if !resolved_exclude or !resolved_exclude.include? hash
end
- commits.push(min_hash)
+ commits.push(min_hash) if !resolved_exclude or !resolved_exclude.include? min_hash
else
- commits.push(max_hash)
+ commits.push(max_hash) if !resolved_exclude or !resolved_exclude.include? max_hash
end
end
end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 21414e2..6e3f90c 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -76,7 +76,7 @@ class Job < ArvadosModel
return true
end
if new_record? or script_version_changed?
- sha1 = Commit.find_commit_range(current_user, nil, nil, self.script_version)[0] rescue nil
+ sha1 = Commit.find_commit_range(current_user, nil, nil, self.script_version, nil)[0] rescue nil
if sha1
self.script_version = sha1
else
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 6cb8c63..25e60ad 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -222,3 +222,18 @@ barbaz_job_readable_by_spectator:
head_uuid: zzzzz-8i9sb-cjs4pklxxjykyuq
properties: {}
+foo_repository_readable_by_spectator:
+ uuid: zzzzz-o0j2j-cpy7p41hpk5xxx
+ owner_uuid: zzzzz-tpzed-000000000000000
+ created_at: 2014-01-24 20:42:26 -0800
+ modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+ modified_by_user_uuid: zzzzz-tpzed-000000000000000
+ modified_at: 2014-01-24 20:42:26 -0800
+ updated_at: 2014-01-24 20:42:26 -0800
+ tail_kind: arvados#user
+ tail_uuid: zzzzz-tpzed-l1s2piq4t4mps8r
+ link_class: permission
+ name: can_read
+ head_kind: arvados#repository
+ head_uuid: zzzzz-2x53u-382brsig8rp3666
+ properties: {}
diff --git a/services/api/test/fixtures/repositories.yml b/services/api/test/fixtures/repositories.yml
index eb91ded..ec3755d 100644
--- a/services/api/test/fixtures/repositories.yml
+++ b/services/api/test/fixtures/repositories.yml
@@ -1,4 +1,4 @@
foo:
uuid: zzzzz-2x53u-382brsig8rp3666
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
- name: foo
\ No newline at end of file
+ name: foo
diff --git a/services/api/test/functional/arvados/v1/commits_controller_test.rb b/services/api/test/functional/arvados/v1/commits_controller_test.rb
index e884b34..70e5276 100644
--- a/services/api/test/functional/arvados/v1/commits_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/commits_controller_test.rb
@@ -3,41 +3,51 @@ load 'test/functional/arvados/v1/git_setup.rb'
class Arvados::V1::CommitsControllerTest < ActionController::TestCase
fixtures :repositories, :users
-
+
+ # See git_setup.rb for the commit log for test.git.tar
include GitSetup
test "test_find_commit_range" do
authorize_with :active
# single
- a = Commit.find_commit_range(users(:active), nil, nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556')
+ a = Commit.find_commit_range(users(:active), nil, nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
assert_equal ['31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
#test "test_branch1" do
- a = Commit.find_commit_range(users(:active), nil, nil, 'master')
+ a = Commit.find_commit_range(users(:active), nil, nil, 'master', nil)
assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57'], a
#test "test_branch2" do
- a = Commit.find_commit_range(users(:active), 'foo', nil, 'b1')
+ a = Commit.find_commit_range(users(:active), 'foo', nil, 'b1', nil)
assert_equal ['1de84a854e2b440dc53bf42f8548afa4c17da332'], a
#test "test_branch3" do
- a = Commit.find_commit_range(users(:active), 'foo', nil, 'HEAD')
+ a = Commit.find_commit_range(users(:active), 'foo', nil, 'HEAD', nil)
assert_equal ['1de84a854e2b440dc53bf42f8548afa4c17da332'], a
#test "test_single_revision_repo" do
- a = Commit.find_commit_range(users(:active), "foo", nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556')
+ a = Commit.find_commit_range(users(:active), "foo", nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
assert_equal ['31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
- a = Commit.find_commit_range(users(:active), "bar", nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556')
+ a = Commit.find_commit_range(users(:active), "bar", nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
assert_equal nil, a
#test "test_multi_revision" do
- a = Commit.find_commit_range(users(:active), nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57')
+ a = Commit.find_commit_range(users(:active), nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', nil)
assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '4fe459abe02d9b365932b8f5dc419439ab4e2577', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
#test "test_tag" do
- a = Commit.find_commit_range(users(:active), nil, 'tag1', 'master')
+ a = Commit.find_commit_range(users(:active), nil, 'tag1', 'master', nil)
assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '4fe459abe02d9b365932b8f5dc419439ab4e2577'], a
+
+ #test "test_multi_revision_exclude" do
+ a = Commit.find_commit_range(users(:active), nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['4fe459abe02d9b365932b8f5dc419439ab4e2577'])
+ assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
+
+ #test "test_multi_revision_tagged_exclude" do
+ a = Commit.find_commit_range(users(:active), nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['tag1'])
+ assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
+
end
end
diff --git a/services/api/test/functional/arvados/v1/git_setup.rb b/services/api/test/functional/arvados/v1/git_setup.rb
index 242542a..46f5f70 100644
--- a/services/api/test/functional/arvados/v1/git_setup.rb
+++ b/services/api/test/functional/arvados/v1/git_setup.rb
@@ -1,6 +1,16 @@
require 'fileutils'
require 'tmpdir'
+# Commit log for test.git.tar
+# master is the main branch
+# b1 is a branch off of master
+# tag1 is a tag
+#
+# 1de84a8 * b1
+# 077ba2a * master
+# 4fe459a * tag1
+# 31ce37f * foo
+
module GitSetup
def setup
@tmpdir = Dir.mktmpdir()
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index f259144..220566b 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -2,8 +2,9 @@ require 'test_helper'
load 'test/functional/arvados/v1/git_setup.rb'
class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
- fixtures :repositories, :users, :jobs
+ fixtures :repositories, :users, :jobs, :links
+ # See git_setup.rb for the commit log for test.git.tar
include GitSetup
test "test_reuse_job" do
@@ -116,4 +117,43 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
end
+ test "test_cannot_reuse_job_no_permission" do
+ @controller = Arvados::V1::JobsController.new
+ authorize_with :spectator
+ post :create, job: {
+ script: "hash",
+ script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+ script_parameters: {
+ input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+ an_integer: '1'
+ }
+ }
+ assert_response :success
+ assert_not_nil assigns(:object)
+ new_job = JSON.parse(@response.body)
+ assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+ assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
+ end
+
+ test "test_cannot_reuse_job_excluded" do
+ @controller = Arvados::V1::JobsController.new
+ authorize_with :active
+ post :create, job: {
+ script: "hash",
+ minimum_script_version: "31ce37fe365b3dc204300a3e4c396ad333ed0556",
+ script_version: "master",
+ exclude_script_versions: ["tag1"],
+ script_parameters: {
+ input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+ an_integer: '1'
+ }
+ }
+ assert_response :success
+ assert_not_nil assigns(:object)
+ new_job = JSON.parse(@response.body)
+ assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+ assert_equal '077ba2ad3ea24a929091a9e6ce545c93199b8e57', new_job['script_version']
+ end
+
+
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list