[ARVADOS] updated: 0ce64862c39e93591635f31a47c5b7d4aa0b9a19
git at public.curoverse.com
git at public.curoverse.com
Tue Aug 12 08:58:11 EDT 2014
Summary of changes:
doc/api/methods/collections.html.textile.liquid | 3 +
.../api/app/controllers/application_controller.rb | 12 ++-
.../arvados/v1/collections_controller.rb | 85 ++++++++++++++--------
services/api/app/models/arvados_model.rb | 22 ++++++
services/api/app/models/collection.rb | 7 +-
.../arvados/v1/collections_controller_test.rb | 47 +++++++++++-
6 files changed, 140 insertions(+), 36 deletions(-)
via 0ce64862c39e93591635f31a47c5b7d4aa0b9a19 (commit)
via bb3c3a0792e1f6be7fcb5e663ac82d5a9c9c0c13 (commit)
via 6ba24a89c7b2fa6f600910dc28af218500a463dd (commit)
via ddb56f97e792fb7bc6471a75b900ba096164d424 (commit)
via a4993e59eaefab303fb9d3b4a2cdc07d166046a4 (commit)
from d7581fb19947ff1efa3b5dcc045b24377e1fe3d6 (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 0ce64862c39e93591635f31a47c5b7d4aa0b9a19
Merge: d7581fb bb3c3a0
Author: Brett Smith <brett at curoverse.com>
Date: Tue Aug 12 08:59:23 2014 -0400
Merge branch '3412-full-collections-index'
Closes #3412, #3534.
commit bb3c3a0792e1f6be7fcb5e663ac82d5a9c9c0c13
Author: Brett Smith <brett at curoverse.com>
Date: Mon Aug 11 15:49:52 2014 -0400
3412: Make manifest_text selectable from Collections index API.
diff --git a/doc/api/methods/collections.html.textile.liquid b/doc/api/methods/collections.html.textile.liquid
index f4eabcf..8760fe8 100644
--- a/doc/api/methods/collections.html.textile.liquid
+++ b/doc/api/methods/collections.html.textile.liquid
@@ -53,6 +53,9 @@ table(table table-bordered table-condensed).
|limit|integer (default 100)|Maximum number of collections to return.|query||
|order|string|Order in which to return matching collections.|query||
|filters|array|Conditions for filtering collections.|query||
+|select|array|Data fields to return in the result list.|query|@["uuid", "manifest_text"]@|
+
+N.B.: Because adding access tokens to manifests can be computationally expensive, the @manifest_text@ field is not included in results by default. If you need it, pass a @select@ parameter that includes @manifest_text at .
h2. update
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index aaf0615..b65fa5b 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -84,17 +84,13 @@ class Arvados::V1::CollectionsController < ApplicationController
end
def show
- if current_api_client_authorization
- signing_opts = {
- key: Rails.configuration.blob_signing_key,
- api_token: current_api_client_authorization.api_token,
- ttl: Rails.configuration.blob_signing_ttl,
- }
- munge_manifest_locators(@object[:manifest_text]) do |loc|
- Blob.sign_locator(loc.to_s, signing_opts)
- end
- end
- render json: @object.as_api_response(:with_data)
+ sign_manifests(@object[:manifest_text])
+ super
+ end
+
+ def index
+ sign_manifests(*@objects.map { |c| c[:manifest_text] })
+ super
end
def collection_uuid(uuid)
@@ -257,6 +253,14 @@ class Arvados::V1::CollectionsController < ApplicationController
end
protected
+
+ def find_objects_for_index
+ # Omit manifest_text from index results unless expressly selected.
+ @select ||= model_class.api_accessible_attributes(:user).
+ map { |attr_spec| attr_spec.first.to_s } - ["manifest_text"]
+ super
+ end
+
def find_object_by_uuid
super
if !@object and !params[:uuid].match(/^[0-9a-f]+\+\d+$/)
@@ -279,4 +283,19 @@ class Arvados::V1::CollectionsController < ApplicationController
def munge_manifest_locators(manifest, &block)
self.class.munge_manifest_locators(manifest, &block)
end
+
+ def sign_manifests(*manifests)
+ if current_api_client_authorization
+ signing_opts = {
+ key: Rails.configuration.blob_signing_key,
+ api_token: current_api_client_authorization.api_token,
+ ttl: Rails.configuration.blob_signing_ttl,
+ }
+ manifests.each do |text|
+ munge_manifest_locators(text) do |loc|
+ Blob.sign_locator(loc.to_s, signing_opts)
+ end
+ end
+ end
+ end
end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index ee3b29c..ac845f5 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -6,9 +6,6 @@ class Collection < ArvadosModel
api_accessible :user, extend: :common do |t|
t.add :data_size
t.add :files
- end
-
- api_accessible :with_data, extend: :user do |t|
t.add :manifest_text
end
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 2996f1b..9c7f488 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -21,7 +21,9 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
authorize_with :active
get :index
assert_response :success
- assert_not_nil assigns(:objects)
+ assert(assigns(:objects).andand.any?, "no Collections returned in index")
+ refute(json_response["items"].any? { |c| c.has_key?("manifest_text") },
+ "basic Collections index included manifest_text")
end
test "can get non-database fields via index select" do
@@ -47,6 +49,26 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
end
end
+ test "index with manifest_text selected returns signed locators" do
+ columns = %w(uuid owner_uuid data_size files manifest_text)
+ authorize_with :active
+ get :index, select: columns
+ assert_response :success
+ assert(assigns(:objects).andand.any?,
+ "no Collections returned for index with columns selected")
+ json_response["items"].each do |coll|
+ assert_equal(columns, columns & coll.keys,
+ "Collections index did not respect selected columns")
+ loc_regexp = / [[:xdigit:]]{32}\+\d+\S+/
+ pos = 0
+ while match = loc_regexp.match(coll["manifest_text"], pos)
+ assert_match(/\+A[[:xdigit:]]+@[[:xdigit:]]{8}\b/, match.to_s,
+ "Locator in manifest_text was not signed")
+ pos = match.end(0)
+ end
+ end
+ end
+
[0,1,2].each do |limit|
test "get index with limit=#{limit}" do
authorize_with :active
commit 6ba24a89c7b2fa6f600910dc28af218500a463dd
Author: Brett Smith <brett at curoverse.com>
Date: Mon Aug 11 15:48:09 2014 -0400
3412: Make non-database fields selectable from API server.
This makes it possible to pass method-back field names, like
"data_size" and "files" from Collections, to the select parameter.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4a1a7d4..d3b5c6b 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -203,7 +203,17 @@ class ApplicationController < ActionController::Base
end
end
- @objects = @objects.select(@select.map { |s| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name s.to_s}" }.join ", ") if @select
+ if @select
+ # Map attribute names in @select to real column names, resolve
+ # those to fully-qualified SQL column names, and pass the
+ # resulting string to the select method.
+ api_column_map = model_class.attributes_required_columns
+ columns_list = @select.
+ flat_map { |attr| api_column_map[attr] }.
+ uniq.
+ map { |s| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name s}" }
+ @objects = @objects.select(columns_list.join(", "))
+ end
@objects = @objects.order(@orders.join ", ") if @orders.any?
@objects = @objects.limit(@limit)
@objects = @objects.offset(@offset)
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 1247e36..539f69d 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -68,6 +68,28 @@ class ArvadosModel < ActiveRecord::Base
self.columns.select { |col| col.name == attr.to_s }.first
end
+ def self.attributes_required_columns
+ # This method returns a hash. Each key is the name of an API attribute,
+ # and it's mapped to a list of database columns that must be fetched
+ # to generate that attribute.
+ # This implementation generates a simple map of attributes to
+ # matching column names. Subclasses can override this method
+ # to specify that method-backed API attributes need to fetch
+ # specific columns from the database.
+ all_columns = columns.map(&:name)
+ api_column_map = Hash.new { |hash, key| hash[key] = [] }
+ methods.grep(/^api_accessible_\w+$/).each do |method_name|
+ next if method_name == :api_accessible_attributes
+ send(method_name).each_pair do |api_attr_name, col_name|
+ col_name = col_name.to_s
+ if all_columns.include?(col_name)
+ api_column_map[api_attr_name.to_s] |= [col_name]
+ end
+ end
+ end
+ api_column_map
+ end
+
# Return nil if current user is not allowed to see the list of
# writers. Otherwise, return a list of user_ and group_uuids with
# write permission. (If not returning nil, current_user is always in
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 50dd42c..ee3b29c 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -12,6 +12,12 @@ class Collection < ArvadosModel
t.add :manifest_text
end
+ def self.attributes_required_columns
+ super.merge({ "data_size" => ["manifest_text"],
+ "files" => ["manifest_text"],
+ })
+ end
+
def redundancy_status
if redundancy_confirmed_as.nil?
'unconfirmed'
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 e4bbd5c..2996f1b 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -24,6 +24,29 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
assert_not_nil assigns(:objects)
end
+ test "can get non-database fields via index select" do
+ authorize_with :active
+ get(:index, filters: [["uuid", "=", collections(:foo_file).uuid]],
+ select: %w(uuid owner_uuid files))
+ assert_response :success
+ assert_equal(1, json_response["items"].andand.size,
+ "wrong number of items returned for index")
+ assert_equal([[".", "foo", 3]], json_response["items"].first["files"],
+ "wrong file list in index result")
+ end
+
+ test "can select only non-database fields for index" do
+ authorize_with :active
+ get(:index, select: %w(data_size files))
+ assert_response :success
+ assert(json_response["items"].andand.any?, "no items found in index")
+ json_response["items"].each do |coll|
+ assert_equal(coll["data_size"],
+ coll["files"].inject(0) { |size, fspec| size + fspec.last },
+ "mismatch between data size and file list")
+ end
+ end
+
[0,1,2].each do |limit|
test "get index with limit=#{limit}" do
authorize_with :active
commit ddb56f97e792fb7bc6471a75b900ba096164d424
Author: Brett Smith <brett at curoverse.com>
Date: Thu Aug 7 15:15:57 2014 -0400
3412: Refactor common manifest munging in API Collections controller.
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 921d3ba..aaf0615 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -45,16 +45,9 @@ class Arvados::V1::CollectionsController < ApplicationController
end
# Remove any permission signatures from the manifest.
- resource_attrs[:manifest_text]
- .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word|
- word.strip!
- loc = Locator.parse(word)
- if loc
- " " + loc.without_signature.to_s
- else
- " " + word
- end
- }
+ munge_manifest_locators(resource_attrs[:manifest_text]) do |loc|
+ loc.without_signature.to_s
+ end
# Save the collection with the stripped manifest.
act_as_system_user do
@@ -97,16 +90,9 @@ class Arvados::V1::CollectionsController < ApplicationController
api_token: current_api_client_authorization.api_token,
ttl: Rails.configuration.blob_signing_ttl,
}
- @object[:manifest_text]
- .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word|
- word.strip!
- loc = Locator.parse(word)
- if loc
- " " + Blob.sign_locator(word, signing_opts)
- else
- " " + word
- end
- }
+ munge_manifest_locators(@object[:manifest_text]) do |loc|
+ Blob.sign_locator(loc.to_s, signing_opts)
+ end
end
render json: @object.as_api_response(:with_data)
end
@@ -258,6 +244,18 @@ class Arvados::V1::CollectionsController < ApplicationController
render json: visited
end
+ def self.munge_manifest_locators(manifest)
+ # Given a manifest text and a block, yield each locator,
+ # and replace it with whatever the block returns.
+ manifest.andand.gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) do |word|
+ if loc = Locator.parse(word.strip)
+ " " + yield(loc)
+ else
+ " " + word
+ end
+ end
+ end
+
protected
def find_object_by_uuid
super
@@ -277,4 +275,8 @@ class Arvados::V1::CollectionsController < ApplicationController
end
end
end
+
+ def munge_manifest_locators(manifest, &block)
+ self.class.munge_manifest_locators(manifest, &block)
+ end
end
commit a4993e59eaefab303fb9d3b4a2cdc07d166046a4
Author: Brett Smith <brett at curoverse.com>
Date: Thu Aug 7 15:14:36 2014 -0400
3412: Clean API Collections controller trailing whitespace.
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index a0c64aa..921d3ba 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -149,7 +149,7 @@ class Arvados::V1::CollectionsController < ApplicationController
logger.debug "visiting #{uuid}"
- if m
+ if m
# uuid is a collection
Collection.readable_by(current_user).where(uuid: uuid).each do |c|
visited[uuid] = c.as_api_response
@@ -166,7 +166,7 @@ class Arvados::V1::CollectionsController < ApplicationController
Job.readable_by(current_user).where(log: uuid).each do |job|
generate_provenance_edges(visited, job.uuid)
end
-
+
else
# uuid is something else
rsc = ArvadosModel::resource_class_for_uuid uuid
@@ -208,7 +208,7 @@ class Arvados::V1::CollectionsController < ApplicationController
logger.debug "visiting #{uuid}"
- if m
+ if m
# uuid is a collection
Collection.readable_by(current_user).where(uuid: uuid).each do |c|
visited[uuid] = c.as_api_response
@@ -226,7 +226,7 @@ class Arvados::V1::CollectionsController < ApplicationController
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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list