[ARVADOS] updated: 05dee1b692afbd5b424ab974e5b5f0cf566781e7
git at public.curoverse.com
git at public.curoverse.com
Mon Jun 16 10:29:04 EDT 2014
Summary of changes:
services/api/app/models/arvados_model.rb | 24 ++++++++-----
services/api/app/models/collection.rb | 61 ++++++++++++++------------------
services/api/test/fixtures/links.yml | 14 ++++++++
services/api/test/unit/job_test.rb | 17 +++++++--
4 files changed, 71 insertions(+), 45 deletions(-)
via 05dee1b692afbd5b424ab974e5b5f0cf566781e7 (commit)
via de3cefb72e49061e25b4f31afa845aae515cc4c4 (commit)
via d06e01ecf1e8463a78dbed33c243d60d3f6f3d24 (commit)
via b5a06a315323e050118cec999676d4a9aff105fa (commit)
from 51c1f780628da8e0839a127963122ebb1b97020b (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 05dee1b692afbd5b424ab974e5b5f0cf566781e7
Author: Brett Smith <brett at curoverse.com>
Date: Mon Jun 16 10:29:09 2014 -0400
2879: Improve SQL for Docker image Collection search.
* Search on Links joined with Collections, to simplify later logic.
* Limit the search to Collections readable by specified users (by
default, the current user).
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index bd2fb44..29a1f91 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -148,52 +148,52 @@ class Collection < ArvadosModel
[hash_part, size_part].compact.join '+'
end
- def self.for_latest_docker_image(search_term, search_tag=nil)
- base_search = self.joins(:links_via_head).order("links.created_at DESC")
+ def self.for_latest_docker_image(search_term, search_tag=nil, readers=nil)
+ readers ||= [Thread.current[:user]]
+ base_search = Link.
+ readable_by(*readers, table_name: "collections").
+ joins("JOIN collections ON links.head_uuid = collections.uuid").
+ order("links.created_at DESC")
+
# If the search term is a Collection locator with an associated
# Docker image hash link, return that Collection.
coll_matches = base_search.
- where(uuid: search_term, links: {link_class: 'docker_image_hash'})
- if coll = coll_matches.first
- return coll
+ where(link_class: "docker_image_hash", collections: {uuid: search_term})
+ if match = coll_matches.first
+ return find_by_uuid(match.head_uuid)
end
# Find Collections with matching Docker image repository+tag pairs.
repo_matches = base_search.
- where(links: {link_class: 'docker_image_repo+tag',
- name: "#{search_term}:#{search_tag || 'latest'}"})
+ where(link_class: "docker_image_repo+tag",
+ name: "#{search_term}:#{search_tag || 'latest'}")
# Find Collections with matching Docker image hashes, unless we're
# obviously doing a repo+tag search and already found a match that way.
- if search_tag.nil? or repo_matches.first.nil?
+ if search_tag.nil? or repo_matches.empty?
hash_matches = base_search.
- where("links.link_class = ? and links.name LIKE ?",
+ where("link_class = ? and name LIKE ?",
"docker_image_hash", "#{search_term}%")
else
hash_matches = nil
end
# Select the image that was created most recently from both repo
- # and hash matches.
- latest_image = nil
+ # and hash matches. Note that the SQL search order and fallback
+ # timestamp values are chosen so that if image timestamps are
+ # missing, we use the image with the newest link.
+ latest_image_link = nil
latest_image_timestamp = "1900-01-01T00:00:00Z"
[repo_matches, hash_matches].compact.each do |search_result|
- search_result.find_each do |coll|
- coll_latest_timestamp = "1900-01-01T00:00:00Z"
-
- coll.links_via_head.find_each do |link|
- this_link_timestamp = link.properties["image_timestamp"]
- if (this_link_timestamp.andand > coll_latest_timestamp)
- coll_latest_timestamp = this_link_timestamp
- end
- end
-
- if coll_latest_timestamp > latest_image_timestamp
- latest_image = coll
- latest_image_timestamp = coll_latest_timestamp
+ search_result.find_each do |link|
+ link_timestamp = link.properties.fetch("image_timestamp",
+ "1900-01-01T00:00:01Z")
+ if link_timestamp > latest_image_timestamp
+ latest_image_link = link
+ latest_image_timestamp = link_timestamp
end
end
end
- latest_image
+ latest_image_link.nil? ? nil : find_by_uuid(latest_image_link.head_uuid)
end
end
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index d5496a6..0655785 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -441,6 +441,20 @@ bug2931_link_with_null_head_uuid:
head_uuid: ~
properties: {}
+active_user_permission_to_docker_image_collection:
+ uuid: zzzzz-o0j2j-dp1d8395ldqw33s
+ 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_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ link_class: permission
+ name: can_read
+ head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+ properties: {}
+
docker_image_collection_hash:
uuid: zzzzz-o0j2j-dockercollhasha
owner_uuid: zzzzz-tpzed-000000000000000
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 4f3e8f3..5f53b2a 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -3,6 +3,10 @@ require 'test_helper'
class JobTest < ActiveSupport::TestCase
BAD_COLLECTION = "#{'f' * 32}+0"
+ setup do
+ set_user_from_auth :active
+ end
+
test "Job without Docker image doesn't get locator" do
job = Job.new
assert job.valid?
commit de3cefb72e49061e25b4f31afa845aae515cc4c4
Author: Brett Smith <brett at curoverse.com>
Date: Mon Jun 16 10:27:38 2014 -0400
2879: Let ArvadosModel.readable_by callers specify table name.
This makes it possible to use readable_by in queries with arbitrary
joins.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 95fd055..2df6686 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -91,6 +91,13 @@ class ArvadosModel < ActiveRecord::Base
# Get rid of troublesome nils
users_list.compact!
+ # Load optional keyword arguments, if they exist.
+ if users_list.last.is_a? Hash
+ kwargs = users_list.pop
+ else
+ kwargs = {}
+ end
+
# Check if any of the users are admin. If so, we're done.
if users_list.select { |u| u.is_admin }.empty?
@@ -101,6 +108,7 @@ class ArvadosModel < ActiveRecord::Base
collect { |uuid| sanitize(uuid) }.join(', ')
sql_conds = []
sql_params = []
+ sql_table = kwargs.fetch(:table_name, table_name)
or_object_uuid = ''
# This row is owned by a member of users_list, or owned by a group
@@ -113,25 +121,25 @@ class ArvadosModel < ActiveRecord::Base
# to this row, or to the owner of this row (see join() below).
permitted_uuids = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (#{sanitized_uuid_list}))"
- sql_conds += ["#{table_name}.owner_uuid in (?)",
- "#{table_name}.uuid in (?)",
- "#{table_name}.uuid IN #{permitted_uuids}"]
+ sql_conds += ["#{sql_table}.owner_uuid in (?)",
+ "#{sql_table}.uuid in (?)",
+ "#{sql_table}.uuid IN #{permitted_uuids}"]
sql_params += [uuid_list, user_uuids]
- if self == Link and users_list.any?
+ if sql_table == "links" and users_list.any?
# This row is a 'permission' or 'resources' link class
# The uuid for a member of users_list is referenced in either the head
# or tail of the link
- sql_conds += ["(#{table_name}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{table_name}.head_uuid IN (?) OR #{table_name}.tail_uuid IN (?)))"]
+ sql_conds += ["(#{sql_table}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{sql_table}.head_uuid IN (?) OR #{sql_table}.tail_uuid IN (?)))"]
sql_params += [user_uuids, user_uuids]
end
- if self == Log and users_list.any?
+ if sql_table == "logs" and users_list.any?
# Link head points to the object described by this row
- sql_conds += ["#{table_name}.object_uuid IN #{permitted_uuids}"]
+ sql_conds += ["#{sql_table}.object_uuid IN #{permitted_uuids}"]
# This object described by this row is owned by this user, or owned by a group readable by this user
- sql_conds += ["#{table_name}.object_owner_uuid in (?)"]
+ sql_conds += ["#{sql_table}.object_owner_uuid in (?)"]
sql_params += [uuid_list]
end
commit d06e01ecf1e8463a78dbed33c243d60d3f6f3d24
Author: Brett Smith <brett at curoverse.com>
Date: Mon Jun 16 09:17:30 2014 -0400
2879: Generalize Job docker_image_locator protection tests.
Refs #2879 - the test for MassAssignmentError is not passing for
Peter. I want to try this approach to see if a more general test is
all we need.
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 2da1e22..4f3e8f3 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -84,15 +84,22 @@ class JobTest < ActiveSupport::TestCase
end
test "can't create Job with Docker image locator" do
- assert_raises(ActiveModel::MassAssignmentSecurity::Error) do
- Job.new(docker_image_locator: BAD_COLLECTION)
+ begin
+ job = Job.new(docker_image_locator: BAD_COLLECTION)
+ rescue ActiveModel::MassAssignmentSecurity::Error
+ # Test passes - expected attribute protection
+ else
+ assert_nil job.docker_image_locator
end
end
test "can't assign Docker image locator to Job" do
job = Job.new
- assert_raises(NoMethodError) do
+ begin
Job.docker_image_locator = BAD_COLLECTION
+ rescue NoMethodError
+ # Test passes - expected attribute protection
end
+ assert_nil job.docker_image_locator
end
end
commit b5a06a315323e050118cec999676d4a9aff105fa
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 13 16:16:27 2014 -0400
2879: Revert previous commit.
This was an effort to better match Docker's behavior. But it turns
out `docker run` always looks for an image tagged 'latest' if you
don't specify, so the previous behavior is actually closer.
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 898127b..bd2fb44 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -158,17 +158,10 @@ class Collection < ArvadosModel
return coll
end
- # Find Collections with a matching Docker image repository,
- # plus the tag pair if provided.
- if search_tag.nil?
- repo_matches = base_search.
- where(links: {link_class: 'docker_image_repository',
- name: search_term})
- else
- repo_matches = base_search.
- where(links: {link_class: 'docker_image_repo+tag',
- name: "#{search_term}:#{search_tag || 'latest'}"})
- end
+ # Find Collections with matching Docker image repository+tag pairs.
+ repo_matches = base_search.
+ where(links: {link_class: 'docker_image_repo+tag',
+ name: "#{search_term}:#{search_tag || 'latest'}"})
# Find Collections with matching Docker image hashes, unless we're
# obviously doing a repo+tag search and already found a match that way.
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list