[ARVADOS] updated: d10a66b66edc088d33bad52aebd7cac0a6e9ce49
git at public.curoverse.com
git at public.curoverse.com
Fri Aug 15 17:11:38 EDT 2014
Summary of changes:
.../api/app/controllers/application_controller.rb | 4 +-
.../arvados/v1/collections_controller.rb | 184 +++++++++++----------
.../controllers/arvados/v1/groups_controller.rb | 6 -
services/api/app/models/collection.rb | 29 +++-
services/api/app/models/job.rb | 1 +
services/api/app/models/locator.rb | 9 +-
services/api/app/models/pipeline_instance.rb | 1 +
.../20140811184643_collection_use_regular_uuids.rb | 1 -
.../20140815171049_add_name_description_columns.rb | 13 ++
services/api/db/structure.sql | 12 +-
services/api/lib/has_uuid.rb | 31 ++--
services/api/test/fixtures/logs.yml | 4 +-
.../arvados/v1/collections_controller_test.rb | 42 +++--
.../arvados/v1/groups_controller_test.rb | 4 +-
services/api/test/unit/arvados_model_test.rb | 16 +-
15 files changed, 210 insertions(+), 147 deletions(-)
create mode 100644 services/api/db/migrate/20140815171049_add_name_description_columns.rb
via d10a66b66edc088d33bad52aebd7cac0a6e9ce49 (commit)
from 1fc1bc4348ea3a1168f6d7bd3391f2449e30d181 (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 d10a66b66edc088d33bad52aebd7cac0a6e9ce49
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Fri Aug 15 17:11:34 2014 -0400
3036: Test fixing work in progress.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4a1a7d4..bd0fedc 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -104,7 +104,9 @@ class ApplicationController < ActionController::Base
if e.respond_to? :backtrace and e.backtrace
logger.error e.backtrace.collect { |x| x + "\n" }.join('')
end
- if @object and @object.errors and @object.errors.full_messages and not @object.errors.full_messages.empty?
+ if (@object and @object.respond_to? :errors and
+ @object.errors and @object.errors.full_messages and
+ not @object.errors.full_messages.empty?)
errors = @object.errors.full_messages
logger.error errors.inspect
else
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index a37290c..474a6ba 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -1,9 +1,8 @@
class Arvados::V1::CollectionsController < ApplicationController
def create
- owner_uuid = resource_attrs.delete(:owner_uuid) || current_user.uuid
- unless current_user.can? write: owner_uuid
- logger.warn "User #{current_user.andand.uuid} tried to set collection owner_uuid to #{owner_uuid}"
- raise ArvadosModel::PermissionDeniedError
+ if !resource_attrs[:manifest_text]
+ return send_error("'manifest_text' attribute must be specified",
+ status: :unprocessable_entity)
end
# Check permissions on the collection manifest.
@@ -54,6 +53,23 @@ class Arvados::V1::CollectionsController < ApplicationController
super
end
+ def find_object_by_uuid
+ if loc = Locator.parse(params[:id])
+ loc.strip_hints!
+ if c = Collection.readable_by(*@read_users).where({ portable_data_hash: loc.to_s }).limit(1).first
+ @object = {
+ portable_data_hash: c.portable_data_hash,
+ manifest_text: c.manifest_text,
+ files: c.files,
+ data_size: c.data_size
+ }
+ end
+ else
+ super
+ end
+ true
+ end
+
def show
if current_api_client_authorization
signing_opts = {
@@ -72,7 +88,11 @@ class Arvados::V1::CollectionsController < ApplicationController
end
}
end
- render json: @object.as_api_response(:with_data)
+ if @object.is_a? Collection
+ render json: @object.as_api_response(:with_data)
+ else
+ render json: @object
+ end
end
def script_param_edges(visited, sp)
@@ -87,48 +107,75 @@ class Arvados::V1::CollectionsController < ApplicationController
end
when String
return if sp.empty?
- m = stripped_portable_data_hash(sp)
- if m
- generate_provenance_edges(visited, m)
+ if loc = Locator.parse(sp)
+ search_edges(visited, loc.to_s, UP)
end
end
end
- def generate_provenance_edges(visited, uuid)
- m = stripped_portable_data_hash(uuid)
- uuid = m if m
+ UP = 1
+ DOWN = 2
+
+ def search_edges(visited, uuid, direction)
+ if uuid.nil? or uuid.empty? or visited[uuid]
+ return
+ end
- if not uuid or uuid.empty? or visited[uuid]
- return ""
+ if loc = Locator.parse(uuid)
+ loc.strip_hints!
+ return if visited[loc.to_s]
end
logger.debug "visiting #{uuid}"
- if m
- # uuid is a collection
- Collection.readable_by(current_user).where(portable_data_hash: uuid).each do |c|
- visited[uuid] = c.as_api_response
- visited[uuid][:files] = []
- c.files.each do |f|
- visited[uuid][:files] << f
- end
+ if loc
+ # uuid is a portable_data_hash
+ if c = Collection.readable_by(*@read_users).where(portable_data_hash: loc.to_s).limit(1).first
+ visited[loc.to_s] = {
+ portable_data_hash: c.portable_data_hash,
+ files: c.files,
+ data_size: c.data_size
+ }
end
- Job.readable_by(current_user).where(output: uuid).each do |job|
- generate_provenance_edges(visited, job.uuid)
- end
+ if direction == UP
+ # Search upstream for jobs where this locator is the output of some job
+ Job.readable_by(*@read_users).where(output: loc.to_s).each do |job|
+ search_edges(visited, job.uuid, UP)
+ end
- Job.readable_by(current_user).where(log: uuid).each do |job|
- generate_provenance_edges(visited, job.uuid)
- end
+ Job.readable_by(*@read_users).where(log: loc.to_s).each do |job|
+ search_edges(visited, job.uuid, UP)
+ end
+ elsif direction == DOWN
+ if loc.to_s == "d41d8cd98f00b204e9800998ecf8427e+0"
+ # Special case, don't follow the empty collection.
+ return
+ end
+ # Search downstream for jobs where this locator is in script_parameters
+ Job.readable_by(*@read_users).where(["jobs.script_parameters like ?", "%#{loc.to_s}%"]).each do |job|
+ search_edges(visited, job.uuid, DOWN)
+ end
+ end
else
- # uuid is something else
+ # uuid is a regular Arvados UUID
rsc = ArvadosModel::resource_class_for_uuid uuid
if rsc == Job
- Job.readable_by(current_user).where(uuid: uuid).each do |job|
+ Job.readable_by(*@read_users).where(uuid: uuid).each do |job|
visited[uuid] = job.as_api_response
- script_param_edges(visited, job.script_parameters)
+ if direction == UP
+ # Follow upstream collections referenced in the script parameters
+ script_param_edges(visited, job.script_parameters)
+ elsif direction == DOWN
+ # Follow downstream job output
+ search_edges(visited, job.output, direction)
+ end
+ end
+ elsif rsc == Collection
+ if c = Collection.readable_by(*@read_users).where(uuid: uuid).limit(1).first
+ visited[uuid] = c.as_api_response
+ search_edges(visited, c.portable_data_hash, direction)
end
elsif rsc != nil
rsc.where(uuid: uuid).each do |r|
@@ -137,75 +184,34 @@ class Arvados::V1::CollectionsController < ApplicationController
end
end
- Link.readable_by(current_user).
- where(head_uuid: uuid, link_class: "provenance").
- each do |link|
- visited[link.uuid] = link.as_api_response
- generate_provenance_edges(visited, link.tail_uuid)
+ if direction == UP
+ # Search for provenance links pointing to the current uuid
+ Link.readable_by(*@read_users).
+ where(head_uuid: uuid, link_class: "provenance").
+ each do |link|
+ visited[link.uuid] = link.as_api_response
+ search_edges(visited, link.tail_uuid, direction)
+ end
+ elsif direction == DOWN
+ # Search for provenance links emanating from the current uuid
+ Link.readable_by(current_user).
+ where(tail_uuid: uuid, link_class: "provenance").
+ each do |link|
+ visited[link.uuid] = link.as_api_response
+ search_edges(visited, link.head_uuid, direction)
+ end
end
end
def provenance
visited = {}
- generate_provenance_edges(visited, @object[:uuid])
+ search_edges(visited, @object[:uuid] || @object[:portable_data_hash], UP)
render json: visited
end
- def generate_used_by_edges(visited, uuid)
- m = stripped_portable_data_hash(uuid)
- uuid = m if m
-
- if not uuid or uuid.empty? or visited[uuid]
- return ""
- end
-
- logger.debug "visiting #{uuid}"
-
- if m
- # uuid is a collection
- Collection.readable_by(current_user).where(portable_data_hash: uuid).each do |c|
- visited[uuid] = c.as_api_response
- visited[uuid][:files] = []
- c.files.each do |f|
- visited[uuid][:files] << f
- end
- end
-
- if uuid == "d41d8cd98f00b204e9800998ecf8427e+0"
- # special case for empty collection
- return
- end
-
- Job.readable_by(current_user).where(["jobs.script_parameters like ?", "%#{uuid}%"]).each do |job|
- generate_used_by_edges(visited, job.uuid)
- end
-
- else
- # uuid is something else
- rsc = ArvadosModel::resource_class_for_uuid uuid
- if rsc == Job
- Job.readable_by(current_user).where(uuid: uuid).each do |job|
- visited[uuid] = job.as_api_response
- generate_used_by_edges(visited, job.output)
- end
- elsif rsc != nil
- rsc.where(uuid: uuid).each do |r|
- visited[uuid] = r.as_api_response
- end
- end
- end
-
- Link.readable_by(current_user).
- where(tail_uuid: uuid, link_class: "provenance").
- each do |link|
- visited[link.uuid] = link.as_api_response
- generate_used_by_edges(visited, link.head_uuid)
- end
- end
-
def used_by
visited = {}
- generate_used_by_edges(visited, @object[:uuid])
+ search_edges(visited, @object[:uuid] || @object[:portable_data_hash], DOWN)
render json: visited
end
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 17be40a..7ee4e55 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -82,12 +82,6 @@ class Arvados::V1::GroupsController < ApplicationController
cond_params = []
conds << "#{klass.table_name}.owner_uuid = ?"
cond_params << opts[:owner_uuid]
- if opts[:include_linked]
- haslink = "#{klass.table_name}.uuid IN (SELECT head_uuid FROM links WHERE link_class=#{klass.sanitize 'name'}"
- haslink += " AND links.tail_uuid=#{klass.sanitize opts[:owner_uuid]}"
- haslink += ")"
- conds << haslink
- end
if conds.any?
cond_sql = '(' + conds.join(') OR (') + ')'
@objects = @objects.where(cond_sql, *cond_params)
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 72cd0b6..f7d9fe8 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -4,7 +4,7 @@ class Collection < ArvadosModel
include CommonApiTemplate
before_validation :set_portable_data_hash
- validate :ensure_manifest_matches_hash
+ validate :ensure_hash_matches_manifest_text
api_accessible :user, extend: :common do |t|
t.add :data_size
@@ -12,6 +12,7 @@ class Collection < ArvadosModel
t.add :name
t.add :description
t.add :properties
+ t.add :portable_data_hash
end
api_accessible :with_data, extend: :user do |t|
@@ -19,17 +20,29 @@ class Collection < ArvadosModel
end
def set_portable_data_hash
- if portable_data_hash.nil? or portable_data_hash == "" or
- (manifest_text_changed? and !portable_data_hash_changed?)
- portable_data_hash = "#{Digest::MD5.hexdigest(manifest_text)}+#{manifest_text.length}"
+ if (self.portable_data_hash.nil? or (self.portable_data_hash == "") or (manifest_text_changed? and !portable_data_hash_changed?))
+ self.portable_data_hash = "#{Digest::MD5.hexdigest(manifest_text)}+#{manifest_text.length}"
+ elsif portable_data_hash_changed?
+ begin
+ loc = Locator.parse!(self.portable_data_hash)
+ loc.strip_hints!
+ self.portable_data_hash = loc.to_s
+ rescue ArgumentError => e
+ errors.add(:portable_data_hash, "#{e}")
+ return false
+ end
end
true
end
- def ensure_manifest_matches_hash
- unless Digest::MD5.hexdigest(manifest_text) == portable_data_hash
- errors.add(:portable_data_hash, "does not match hash of manifest_text")
- return false
+ def ensure_hash_matches_manifest_text
+ if manifest_text_changed? or portable_data_hash_changed?
+ computed_hash = "#{Digest::MD5.hexdigest(manifest_text)}+#{manifest_text.length}"
+ unless computed_hash == portable_data_hash
+ logger.debug "(computed) '#{computed_hash}' != '#{portable_data_hash}' (provided)"
+ errors.add(:portable_data_hash, "does not match hash of manifest_text")
+ return false
+ end
end
true
end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 039495d..97039db 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -41,6 +41,7 @@ class Job < ArvadosModel
t.add :supplied_script_version
t.add :docker_image_locator
t.add :name
+ t.add :description
end
def assert_finished
diff --git a/services/api/app/models/locator.rb b/services/api/app/models/locator.rb
index d01c9dc..39a7aaf 100644
--- a/services/api/app/models/locator.rb
+++ b/services/api/app/models/locator.rb
@@ -35,9 +35,16 @@ class Locator
# Locator.parse! returns a Locator object parsed from the string tok,
# raising an ArgumentError if tok cannot be parsed.
def self.parse!(tok)
+ if tok.nil? or tok.empty?
+ raise ArgumentError.new "locator is nil or empty"
+ end
+
m = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?(\+([[:upper:]][[:alnum:]+ at _-]*))?$/.match(tok.strip)
unless m
- raise ArgumentError.new "could not parse #{tok}"
+ raise ArgumentError.new "not a valid locator #{tok}"
+ end
+ unless m[2]
+ raise ArgumentError.new "missing size hint on #{tok}"
end
tokhash, _, toksize, _, trailer = m[1..5]
diff --git a/services/api/app/models/pipeline_instance.rb b/services/api/app/models/pipeline_instance.rb
index 354c892..f2c930c 100644
--- a/services/api/app/models/pipeline_instance.rb
+++ b/services/api/app/models/pipeline_instance.rb
@@ -17,6 +17,7 @@ class PipelineInstance < ArvadosModel
t.add :pipeline_template_uuid
t.add :pipeline_template, :if => :pipeline_template
t.add :name
+ t.add :description
t.add :components
t.add :dependencies
t.add :properties
diff --git a/services/api/db/migrate/20140811184643_collection_use_regular_uuids.rb b/services/api/db/migrate/20140811184643_collection_use_regular_uuids.rb
index e5b509c..fd9054c 100644
--- a/services/api/db/migrate/20140811184643_collection_use_regular_uuids.rb
+++ b/services/api/db/migrate/20140811184643_collection_use_regular_uuids.rb
@@ -5,7 +5,6 @@ class CollectionUseRegularUuids < ActiveRecord::Migration
add_column :collections, :properties, :text
add_column :collections, :expire_time, :date
remove_column :collections, :locator
- add_column :jobs, :name, :string
say_with_time "Step 1. Move manifest hashes into portable_data_hash field" do
ActiveRecord::Base.connection.execute("update collections set portable_data_hash=uuid, uuid=null")
diff --git a/services/api/db/migrate/20140815171049_add_name_description_columns.rb b/services/api/db/migrate/20140815171049_add_name_description_columns.rb
new file mode 100644
index 0000000..c0c8f6a
--- /dev/null
+++ b/services/api/db/migrate/20140815171049_add_name_description_columns.rb
@@ -0,0 +1,13 @@
+class AddNameDescriptionColumns < ActiveRecord::Migration
+ def up
+ add_column :jobs, :name, :string
+ add_column :jobs, :description, :text
+ add_column :pipeline_instances, :description, :text
+ end
+
+ def down
+ remove_column :jobs, :name
+ remove_column :jobs, :description
+ remove_column :pipeline_instances, :description
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index b31825e..4b32d38 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -3,7 +3,6 @@
--
SET statement_timeout = 0;
-SET lock_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SET check_function_bodies = false;
@@ -430,7 +429,9 @@ CREATE TABLE jobs (
repository character varying(255),
output_is_persistent boolean DEFAULT false NOT NULL,
supplied_script_version character varying(255),
- docker_image_locator character varying(255)
+ docker_image_locator character varying(255),
+ name character varying(255),
+ description text
);
@@ -679,7 +680,8 @@ CREATE TABLE pipeline_instances (
updated_at timestamp without time zone NOT NULL,
properties text,
state character varying(255),
- components_summary text
+ components_summary text,
+ description text
);
@@ -1981,4 +1983,6 @@ INSERT INTO schema_migrations (version) VALUES ('20140709172343');
INSERT INTO schema_migrations (version) VALUES ('20140714184006');
-INSERT INTO schema_migrations (version) VALUES ('20140811184643');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20140811184643');
+
+INSERT INTO schema_migrations (version) VALUES ('20140815171049');
\ No newline at end of file
diff --git a/services/api/lib/has_uuid.rb b/services/api/lib/has_uuid.rb
index 9f1bbf4..df175f8 100644
--- a/services/api/lib/has_uuid.rb
+++ b/services/api/lib/has_uuid.rb
@@ -31,36 +31,33 @@ module HasUuid
def validate_uuid
if self.respond_to_uuid? and self.uuid_changed?
- if self.uuid_was.nil? or self.uuid_was == ""
- if self.uuid.is_a?(String) and self.uuid.length > 0 and current_user.andand.is_admin
- if (re = self.uuid.match HasUuid::UUID_REGEX)
- if re[1] == self.class.uuid_prefix
- return true
- else
- self.errors.add(:uuid, "Matched uuid type '#{re[1]}', expected '#{self.class.uuid_prefix}'")
- return false
- end
+ if current_user.andand.is_admin and self.uuid.is_a?(String)
+ if (re = self.uuid.match HasUuid::UUID_REGEX)
+ if re[1] == self.class.uuid_prefix
+ return true
else
- self.errors.add(:uuid, "'#{self.uuid}' is not a valid Arvados UUID")
+ self.errors.add(:uuid, "Matched uuid type '#{re[1]}', expected '#{self.class.uuid_prefix}'")
return false
end
- elsif self.uuid.nil? or self.uuid == ""
- return true
else
- self.errors.add(:uuid, "Not permitted to specify uuid")
+ self.errors.add(:uuid, "'#{self.uuid}' is not a valid Arvados UUID")
return false
end
else
- self.errors.add(:uuid, "Not permitted to change uuid")
+ if self.new_record?
+ self.errors.add(:uuid, "Not permitted to specify uuid")
+ else
+ self.errors.add(:uuid, "Not permitted to change uuid")
+ end
return false
end
+ else
+ return true
end
-
- true
end
def assign_uuid
- if self.respond_to_uuid? and self.uuid.nil? or self.uuid == ""
+ if self.respond_to_uuid? and self.uuid.nil? or self.uuid.empty?
self.uuid = self.class.generate_uuid
end
true
diff --git a/services/api/test/fixtures/logs.yml b/services/api/test/fixtures/logs.yml
index d95177a..c737bb1 100644
--- a/services/api/test/fixtures/logs.yml
+++ b/services/api/test/fixtures/logs.yml
@@ -24,7 +24,7 @@ log4: # foo collection added, readable by active through link
id: 4
uuid: zzzzz-xxxxx-pshmckwoma00004
owner_uuid: zzzzz-tpzed-000000000000000 # system user
- object_uuid: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45 # foo file
+ object_uuid: 4n8aq-4zz18-znfnqtbbv4spc3w # foo file
object_owner_uuid: zzzzz-tpzed-000000000000000 # system user
event_at: <%= 4.minute.ago.to_s(:db) %>
@@ -32,6 +32,6 @@ log5: # baz collection added, readable by active and spectator through group 'al
id: 5
uuid: zzzzz-xxxxx-pshmckwoma00005
owner_uuid: zzzzz-tpzed-000000000000000 # system user
- object_uuid: ea10d51bcf88862dbcc36eb292017dfd+45 # baz file
+ object_uuid: 4n8aq-4zz18-y9vne9npefyxh8g # baz file
object_owner_uuid: zzzzz-tpzed-000000000000000 # system user
event_at: <%= 5.minute.ago.to_s(:db) %>
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 e6834e0..30559a5 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -84,7 +84,7 @@ EOS
assert_nil assigns(:objects)
get :show, {
- id: test_collection[:uuid]
+ id: test_collection[:portable_data_hash]
}
assert_response :success
assert_not_nil assigns(:object)
@@ -128,12 +128,12 @@ EOS
collection: {
owner_uuid: 'zzzzz-j7d0g-rew6elm53kancon',
manifest_text: manifest_text,
- portable_data_hash: "d30fe8ae534397864cb96c544f4cf102"
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47"
}
}
assert_response :success
resp = JSON.parse(@response.body)
- assert_equal 'zzzzz-tpzed-000000000000000', resp['owner_uuid']
+ assert_equal 'zzzzz-j7d0g-rew6elm53kancon', resp['owner_uuid']
end
test "create with owner_uuid set to group i can_manage" do
@@ -142,30 +142,44 @@ EOS
manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
post :create, {
collection: {
- owner_uuid: 'zzzzz-j7d0g-8ulrifv67tve5sx',
+ owner_uuid: groups(:system_owned_group).uuid,
manifest_text: manifest_text,
- portable_data_hash: "d30fe8ae534397864cb96c544f4cf102"
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47"
}
}
assert_response :success
resp = JSON.parse(@response.body)
- assert_equal 'zzzzz-tpzed-000000000000000', resp['owner_uuid']
+ assert_equal 'zzzzz-j7d0g-8ulrifv67tve5sx', resp['owner_uuid']
end
- test "create with owner_uuid set to group with no can_manage permission" do
+ test "create with owner_uuid fails on group with can_read permission" do
permit_unsigned_manifests
authorize_with :active
manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
post :create, {
collection: {
- owner_uuid: 'zzzzz-j7d0g-it30l961gq3t0oi',
+ owner_uuid: groups(:all_users).uuid,
manifest_text: manifest_text,
- portable_data_hash: "d30fe8ae534397864cb96c544f4cf102"
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47"
}
}
assert_response 403
end
+ test "create with owner_uuid fails on group with no permission" do
+ permit_unsigned_manifests
+ authorize_with :active
+ manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
+ post :create, {
+ collection: {
+ owner_uuid: groups(:public).uuid,
+ manifest_text: manifest_text,
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47"
+ }
+ }
+ assert_response 422
+ end
+
test "admin create with owner_uuid set to group with no permission" do
permit_unsigned_manifests
authorize_with :admin
@@ -174,7 +188,7 @@ EOS
collection: {
owner_uuid: 'zzzzz-j7d0g-it30l961gq3t0oi',
manifest_text: manifest_text,
- portable_data_hash: "d30fe8ae534397864cb96c544f4cf102"
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47"
}
}
assert_response :success
@@ -187,7 +201,7 @@ EOS
collection: <<-EOS
{
"manifest_text":". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n",\
- "portable_data_hash":"d30fe8ae534397864cb96c544f4cf102"\
+ "portable_data_hash":"d30fe8ae534397864cb96c544f4cf102+47"\
}
EOS
}
@@ -201,7 +215,7 @@ EOS
collection: <<-EOS
{
"manifest_text":". d41d8cd98f00b204e9800998ecf8427e 0:0:bar.txt\n",\
- "portable_data_hash":"d30fe8ae534397864cb96c544f4cf102"\
+ "portable_data_hash":"d30fe8ae534397864cb96c544f4cf102+47"\
}
EOS
}
@@ -261,8 +275,8 @@ EOS
where: { any: ['contains', '7f9102c395f4ffc5e3'] }
}
assert_response :success
- found = assigns(:objects).collect(&:uuid)
- assert_equal 1, found.count
+ found = assigns(:objects).collect(&:portable_data_hash)
+ assert_equal 2, found.count
assert_equal true, !!found.index('1f4b0bc7583c2a7f9102c395f4ffc5e3+45')
end
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 217809a..276037f 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -262,7 +262,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
end
test 'get group-owned objects with include_linked' do
- expected_uuid = specimens(:in_aproject_linked_from_asubproject).uuid
+ expected_uuid = specimens(:in_asubproject).uuid
authorize_with :active
get :contents, {
id: groups(:asubproject).uuid,
@@ -273,7 +273,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
uuids = json_response['items'].collect { |i| i['uuid'] }
assert_includes uuids, expected_uuid, "Did not get #{expected_uuid}"
- expected_name = links(:specimen_is_in_two_projects).name
+ expected_name = specimens(:in_asubproject).name
found_specimen_name = false
assert(json_response['links'].any?,
"Expected a non-empty array of links in response")
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index e52dfcd..e47aa3b 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -23,11 +23,23 @@ class ArvadosModelTest < ActiveSupport::TestCase
assert a.uuid.length==27, "Auto assigned uuid length is wrong."
end
+ test 'admin cannot assign uuid with wrong object type' do
+ set_user_from_auth :admin_trustedclient
+ want_uuid = Human.generate_uuid
+ a = create_with_attrs(uuid: want_uuid)
+ assert_nil a, "Admin should not be able to assign invalid uuid."
+ end
+
+ test 'admin cannot assign badly formed uuid' do
+ set_user_from_auth :admin_trustedclient
+ a = create_with_attrs(uuid: "ntoheunthaoesunhasoeuhtnsaoeunhtsth")
+ assert_nil a, "Admin should not be able to assign invalid uuid."
+ end
+
test 'admin cannot assign empty uuid' do
set_user_from_auth :admin_trustedclient
a = create_with_attrs(uuid: "")
- assert_not_equal "", a.uuid, "Admin should not assign empty uuid."
- assert a.uuid.length==27, "Auto assigned uuid length is wrong."
+ assert_nil a, "Admin cannot assign empty uuid."
end
[ {:a => 'foo'},
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list