[ARVADOS] created: 73c1fd44fed413866e5fdf97b5f8899c6d3c4a85
git at public.curoverse.com
git at public.curoverse.com
Fri Aug 8 15:48:43 EDT 2014
at 73c1fd44fed413866e5fdf97b5f8899c6d3c4a85 (commit)
commit 73c1fd44fed413866e5fdf97b5f8899c6d3c4a85
Author: Brett Smith <brett at curoverse.com>
Date: Fri Aug 8 15:42:26 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 17d7a2d..ca8da31 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)
@@ -259,15 +255,18 @@ class Arvados::V1::CollectionsController < ApplicationController
protected
def find_objects_for_index
+ # Omit manifest_text from index results unless expressly selected.
# If the user has selected fields that derive from manifest_text, we'll
- # need to fetch that, even though we don't return it in the results.
- orig_select = @select.andand.dup
- if @select and (@select & ["data_size", "files"]).any? and
+ # need to fetch it during search, then hide it from the results.
+ @select ||= model_class.api_accessible_attributes(:user).
+ map { |attr_spec| attr_spec.first.to_s } - ["manifest_text"]
+ render_select = @select.dup
+ if (@select & ["data_size", "files"]).any? and
(not @select.include?("manifest_text"))
@select << "manifest_text"
end
super
- @select = orig_select
+ @select = render_select
end
def find_object_by_uuid
@@ -292,4 +291,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 50dd42c..e153f3b 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 ce92123cc4ea9850f07d50ed56c15b71bd0f5777
Author: Brett Smith <brett at curoverse.com>
Date: Fri Aug 8 11:32:22 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..ba4b544 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -203,7 +203,11 @@ 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
+ # Find the real column names in @select, resolve those to fully-qualified
+ # SQL column names, and pass the resulting string to the select method.
+ @objects = @objects.select((@select & model_class.columns.map { |c| c.name.to_s })
+ .map { |s| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name s.to_s}" }
+ .join ", ") if @select
@objects = @objects.order(@orders.join ", ") if @orders.any?
@objects = @objects.limit(@limit)
@objects = @objects.offset(@offset)
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index aaf0615..17d7a2d 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -257,6 +257,19 @@ class Arvados::V1::CollectionsController < ApplicationController
end
protected
+
+ def find_objects_for_index
+ # If the user has selected fields that derive from manifest_text, we'll
+ # need to fetch that, even though we don't return it in the results.
+ orig_select = @select.andand.dup
+ if @select and (@select & ["data_size", "files"]).any? and
+ (not @select.include?("manifest_text"))
+ @select << "manifest_text"
+ end
+ super
+ @select = orig_select
+ end
+
def find_object_by_uuid
super
if !@object and !params[:uuid].match(/^[0-9a-f]+\+\d+$/)
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 caf4c362851acbe505d5219415ce177e2de8aa2f
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 3c5cb5ef46bc73162cefcbd20a4e03faebdcc0ba
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