[ARVADOS] updated: bd11c0a9ff6ce23f0992af11d7dc7aef32860e22
git at public.curoverse.com
git at public.curoverse.com
Mon Apr 14 09:31:46 EDT 2014
Summary of changes:
.../api/app/controllers/application_controller.rb | 5 +-
.../app/controllers/arvados/v1/links_controller.rb | 50 +++++++++++++-------
.../app/controllers/arvados/v1/logs_controller.rb | 32 +++++++++++++
services/api/app/models/arvados_model.rb | 17 ++----
services/api/app/models/link.rb | 1 +
services/api/app/models/log.rb | 9 +++-
.../migrate/20140325175653_remove_kind_columns.rb | 4 +-
services/api/test/fixtures/logs.yml | 3 +
.../functional/arvados/v1/links_controller_test.rb | 24 +++++++++-
.../functional/arvados/v1/logs_controller_test.rb | 27 +++++++++++
10 files changed, 138 insertions(+), 34 deletions(-)
create mode 100644 services/api/test/fixtures/logs.yml
via bd11c0a9ff6ce23f0992af11d7dc7aef32860e22 (commit)
from 4c7323ea090e7570f962d821fa24c3ab20308e1e (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 bd11c0a9ff6ce23f0992af11d7dc7aef32860e22
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Mon Apr 14 09:31:42 2014 -0400
* Refactored to remove load_kind_params filter and instead override load_where_param and load_filters_param in the links and logs controllers to add the _kind functionality.
* Removed 'rescue' clause. I think I left that in from debugging.
* Fixed :log -> :logs
* filters: ['tail_kind','=','arvados#user'] works
* Added more tests.
* Added equivalent logic and tests for logs controller for object_kind
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 9aad195..0039bb0 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -118,11 +118,12 @@ class ApplicationController < ActionController::Base
end
def load_filters_param
+ @filters ||= []
if params[:filters].is_a? Array
- @filters = params[:filters]
+ @filters += params[:filters]
elsif params[:filters].is_a? String and !params[:filters].empty?
begin
- @filters = Oj.load params[:filters]
+ @filters += Oj.load params[:filters]
raise unless @filters.is_a? Array
rescue
raise ArgumentError.new("Could not parse \"filters\" param as an array")
diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index 1b79295..1b5bf78 100644
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -1,33 +1,49 @@
class Arvados::V1::LinksController < ApplicationController
- prepend_before_filter :load_kind_params, :only => :index
-
def create
resource_attrs.delete :head_kind
resource_attrs.delete :tail_kind
super
end
- def load_kind_params
- if params[:tail_uuid]
- params[:where] = Oj.load(params[:where]) if params[:where].is_a?(String)
- @where ||= {}
- @where[:tail_uuid] = params[:tail_uuid]
- end
+ protected
- if params[:where] and params[:where].is_a? Hash
- if params[:where][:head_kind]
- params[:filters] ||= []
- params[:filters] << ['head_uuid', 'is_a', params[:where][:head_kind]]
- params[:where].delete :head_kind
+ # Overrides ApplicationController load_where_param
+ def load_where_param
+ super
+
+ # head_kind and tail_kind columns are now virtual,
+ # equivilent functionality is now provided by
+ # 'is_a', so fix up any old-style 'where' clauses.
+ if @where
+ @filters ||= []
+ if @where[:head_kind]
+ @filters << ['head_uuid', 'is_a', @where[:head_kind]]
+ @where.delete :head_kind
end
- if params[:where][:tail_kind]
- params[:filters] ||= []
- params[:filters] << ['tail_uuid', 'is_a', params[:where][:tail_kind]]
- params[:where].delete :tail_kind
+ if @where[:tail_kind]
+ @filters << ['tail_uuid', 'is_a', @where[:tail_kind]]
+ @where.delete :tail_kind
end
end
+ end
+ # Overrides ApplicationController load_filters_param
+ def load_filters_param
+ super
+
+ # head_kind and tail_kind columns are now virtual,
+ # equivilent functionality is now provided by
+ # 'is_a', so fix up any old-style 'filter' clauses.
+ @filters = @filters.map do |k|
+ if k[0] == 'head_kind' and k[1] == '='
+ ['head_uuid', 'is_a', k[2]]
+ elsif k[0] == 'tail_kind' and k[1] == '='
+ ['tail_uuid', 'is_a', k[2]]
+ else
+ k
+ end
+ end
end
end
diff --git a/services/api/app/controllers/arvados/v1/logs_controller.rb b/services/api/app/controllers/arvados/v1/logs_controller.rb
index dffe662..925eee5 100644
--- a/services/api/app/controllers/arvados/v1/logs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/logs_controller.rb
@@ -1,2 +1,34 @@
class Arvados::V1::LogsController < ApplicationController
+ # Overrides ApplicationController load_where_param
+ def load_where_param
+ super
+
+ # object_kind and column is now virtual,
+ # equivilent functionality is now provided by
+ # 'is_a', so fix up any old-style 'where' clauses.
+ if @where
+ @filters ||= []
+ if @where[:object_kind]
+ @filters << ['object_uuid', 'is_a', @where[:object_kind]]
+ @where.delete :object_kind
+ end
+ end
+ end
+
+ # Overrides ApplicationController load_filters_param
+ def load_filters_param
+ super
+
+ # object_kind and column is now virtual,
+ # equivilent functionality is now provided by
+ # 'is_a', so fix up any old-style 'filter' clauses.
+ @filters = @filters.map do |k|
+ if k[0] == 'object_kind' and k[1] == '='
+ ['object_uuid', 'is_a', k[2]]
+ else
+ k
+ end
+ end
+ end
+
end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 08bb5ea..dbb807c 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -222,18 +222,13 @@ class ArvadosModel < ActiveRecord::Base
specials = [system_user_uuid, 'd41d8cd98f00b204e9800998ecf8427e+0']
foreign_key_attributes.each do |attr|
- begin
- if new_record? or send (attr + "_changed?")
- attr_value = send attr
- r = ArvadosModel::resource_class_for_uuid attr_value if attr_value
- r = r.readable_by(current_user) if r and not skip_uuid_read_permission_check.include? attr
- if r and r.where(uuid: attr_value).count == 0 and not specials.include? attr_value
- errors.add(attr, "'#{attr_value}' not found")
- end
+ if new_record? or send (attr + "_changed?")
+ attr_value = send attr
+ r = ArvadosModel::resource_class_for_uuid attr_value if attr_value
+ r = r.readable_by(current_user) if r and not skip_uuid_read_permission_check.include? attr
+ if r and r.where(uuid: attr_value).count == 0 and not specials.include? attr_value
+ errors.add(attr, "'#{attr_value}' not found")
end
- rescue Exception => e
- bt = e.backtrace.join("\n")
- errors.add(attr, "'#{attr_value}' error '#{e}'\n#{bt}\n")
end
end
end
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index cf4ffce..26e7183 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -8,6 +8,7 @@ class Link < ArvadosModel
after_update :maybe_invalidate_permissions_cache
after_create :maybe_invalidate_permissions_cache
after_destroy :maybe_invalidate_permissions_cache
+ attr_accessor :head_kind, :tail_kind
api_accessible :user, extend: :common do |t|
t.add :tail_uuid
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 923a681..f8e337b 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -4,17 +4,24 @@ class Log < ArvadosModel
include CommonApiTemplate
serialize :properties, Hash
before_validation :set_default_event_at
- attr_accessor :object
+ attr_accessor :object, :object_kind
api_accessible :user, extend: :common do |t|
t.add :object_uuid
t.add :object, :if => :object
+ t.add :object_kind
t.add :event_at
t.add :event_type
t.add :summary
t.add :properties
end
+ def object_kind
+ if k = ArvadosModel::resource_class_for_uuid(object_uuid)
+ k.kind
+ end
+ end
+
def fill_object(thing)
self.object_uuid ||= thing.uuid
self.summary ||= "#{self.event_type} of #{thing.uuid}"
diff --git a/services/api/db/migrate/20140325175653_remove_kind_columns.rb b/services/api/db/migrate/20140325175653_remove_kind_columns.rb
index d77130d..eae2a2c 100644
--- a/services/api/db/migrate/20140325175653_remove_kind_columns.rb
+++ b/services/api/db/migrate/20140325175653_remove_kind_columns.rb
@@ -4,13 +4,13 @@ class RemoveKindColumns < ActiveRecord::Migration
def up
remove_column :links, :head_kind
remove_column :links, :tail_kind
- remove_column :log, :object_kind
+ remove_column :logs, :object_kind
end
def down
add_column :links, :head_kind, :string
add_column :links, :tail_kind, :string
- add_column :log, :object_kind, :string
+ add_column :logs, :object_kind, :string
act_as_system_user do
Link.all.each do |l|
diff --git a/services/api/test/fixtures/logs.yml b/services/api/test/fixtures/logs.yml
new file mode 100644
index 0000000..d805439
--- /dev/null
+++ b/services/api/test/fixtures/logs.yml
@@ -0,0 +1,3 @@
+log1:
+ uuid: zzzzz-xxxxx-pshmckwoma9plh7
+ object_uuid: zzzzz-tpzed-l1s2piq4t4mps8r
\ No newline at end of file
diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb
index 4c0c8dc..09dd162 100644
--- a/services/api/test/functional/arvados/v1/links_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/links_controller_test.rb
@@ -44,7 +44,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response 422
end
- test "head and tail exist" do
+ test "head and tail exist, head_kind and tail_kind are returned" do
link = {
link_class: 'test',
name: 'stuff',
@@ -153,5 +153,27 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_equal found.count, (found.select { |f| f.head_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
end
+ test "test can still use filter tail_kind" do
+ authorize_with :admin
+ get :index, {
+ filters: [ ['tail_kind', '=', 'arvados#user'] ]
+ }
+ assert_response :success
+ found = assigns(:objects)
+ assert_not_equal 0, found.count
+ assert_equal found.count, (found.select { |f| f.tail_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ end
+
+ test "test can still use filter head_kind" do
+ authorize_with :admin
+ get :index, {
+ filters: [ ['head_kind', '=', 'arvados#user'] ]
+ }
+ assert_response :success
+ found = assigns(:objects)
+ assert_not_equal 0, found.count
+ assert_equal found.count, (found.select { |f| f.head_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ end
+
end
diff --git a/services/api/test/functional/arvados/v1/logs_controller_test.rb b/services/api/test/functional/arvados/v1/logs_controller_test.rb
index 9c41099..a224e25 100644
--- a/services/api/test/functional/arvados/v1/logs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/logs_controller_test.rb
@@ -1,6 +1,8 @@
require 'test_helper'
class Arvados::V1::LogsControllerTest < ActionController::TestCase
+ fixtures :logs
+
test "non-admins can read their own logs" do
authorize_with :active
post :create, log: {summary: "test log"}
@@ -12,4 +14,29 @@ class Arvados::V1::LogsControllerTest < ActionController::TestCase
assert_equal("test log", assigns(:object).summary,
"loaded wrong log after creation")
end
+
+ test "test can still use where object_kind" do
+ authorize_with :admin
+ get :index, {
+ where: { object_kind: 'arvados#user' }
+ }
+ assert_response :success
+ found = assigns(:objects)
+ assert_not_equal 0, found.count
+ assert_equal found.count, (found.select { |f| f.object_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ l = JSON.parse(@response.body)
+ assert_equal 'arvados#user', l['items'][0]['object_kind']
+ end
+
+ test "test can still use filter object_kind" do
+ authorize_with :admin
+ get :index, {
+ filters: [ ['object_kind', '=', 'arvados#user'] ]
+ }
+ assert_response :success
+ found = assigns(:objects)
+ assert_not_equal 0, found.count
+ assert_equal found.count, (found.select { |f| f.object_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ end
+
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list