[ARVADOS] updated: ee21f45e958de5ef39970981ead6416e3790cd1a

git at public.curoverse.com git at public.curoverse.com
Tue Mar 25 13:42:56 EDT 2014


Summary of changes:
 .../app/controllers/arvados/v1/users_controller.rb |   24 +++++++--
 services/api/app/models/user.rb                    |   31 +++---------
 .../functional/arvados/v1/users_controller_test.rb |   28 +++++++++--
 services/api/test/unit/user_test.rb                |   51 +------------------
 4 files changed, 54 insertions(+), 80 deletions(-)

       via  ee21f45e958de5ef39970981ead6416e3790cd1a (commit)
       via  2013479619e9aa838037262564ac4f265f786ad0 (commit)
      from  fad09fd18d6c364f358f3d7c5782b4d0360c68ab (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 ee21f45e958de5ef39970981ead6416e3790cd1a
Author: radhika chippada <radhika at radhika.curoverse>
Date:   Tue Mar 25 13:23:28 2014 -0400

    setup method in user model assumes that the user object passed in is valid and hence no longer tries to find it.

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 2c3b2df..8a42bfb 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -113,7 +113,7 @@ class Arvados::V1::UsersController < ApplicationController
             raise ArgumentError.new "Required openid_prefix parameter is missing."
           end
 
-          @object = model_class.new resource_attrs
+          @object = model_class.create! resource_attrs
         end
       end
     end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index fc7bc4f..87f7e03 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -112,29 +112,15 @@ class User < ArvadosModel
   def self.setup(user, openid_prefix, repo_name=nil, vm_uuid=nil)
     login_perm_props = {identity_url_prefix: openid_prefix}
 
-    if user.uuid
-      found = User.find_by_uuid user.uuid
-      if found
-        user = found
-      end
-    end
-
-    if !found
-      if !user.email
-        raise "No email found in the input. Aborting user creation."
-      end
-
-      user.save!
-    
-      # Check oid_login_perm
-      oid_login_perms = Link.where(tail_uuid: user.email,
+    # Check oid_login_perm
+    oid_login_perms = Link.where(tail_uuid: user.email,
                                    head_kind: 'arvados#user',
                                    link_class: 'permission',
                                    name: 'can_login')
 
-      if !oid_login_perms.any?
-        # create openid login permission
-        oid_login_perm = Link.create(link_class: 'permission',
+    if !oid_login_perms.any?
+      # create openid login permission
+      oid_login_perm = Link.create(link_class: 'permission',
                                    name: 'can_login',
                                    tail_kind: 'email',
                                    tail_uuid: user.email,
@@ -142,10 +128,9 @@ class User < ArvadosModel
                                    head_uuid: user.uuid,
                                    properties: login_perm_props
                                   )
-        logger.info { "openid login permission: " + oid_login_perm[:uuid] }
-      else
-        oid_login_perm = oid_login_perms.first
-      end
+      logger.info { "openid login permission: " + oid_login_perm[:uuid] }
+    else
+      oid_login_perm = oid_login_perms.first
     end
 
     return [user, oid_login_perm] + user.setup_repo_vm_links(repo_name, vm_uuid)
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 81f096c..daf19a9 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -81,6 +81,26 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_not_nil created['email'], 'expected non-nil email'
     assert_nil created['identity_url'], 'expected no identity_url' 
 
+    # invoke setup again with the same data
+    post :setup, {
+      repo_name: repo_name,
+      openid_prefix: 'https://www.google.com/accounts/o8/id',
+      user: {
+        uuid: "this_is_agreeable",        
+        first_name: "in_create_test_first_name",
+        last_name: "test_last_name",
+        email: "test at abc.com"
+      }
+    }
+
+    response_items = JSON.parse(@response.body)['items']
+    created = find_obj_in_resp response_items, 'User', nil
+    assert_equal 'in_create_test_first_name', created['first_name']
+    assert_not_nil created['uuid'], 'expected non-null uuid for the new user'
+    assert_equal 'this_is_agreeable', created['uuid']
+    assert_not_nil created['email'], 'expected non-nil email'
+    assert_nil created['identity_url'], 'expected no identity_url' 
+
     # since no such vm exists, expect only three new links: 
     # arvados#user, repo link and link add user to 'All users' group
     verify_num_links @all_links_at_start, 3
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index f4b8ac5..7c65242 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -149,51 +149,7 @@ class UserTest < ActiveSupport::TestCase
         'Expected PermissionDeniedError'
   end
 
-  test "setup new user as non-admin user" do
-    Thread.current[:user] = @active_user
-
-    begin
-      user = User.new
-      user.email = 'abc at xyz.com'
-      
-      User.setup user, 'http://openid/prefix'
-    rescue ArvadosModel::PermissionDeniedError => e
-    end
-
-    assert (e.message.include? 'PermissionDeniedError'),
-        'Expected PermissionDeniedError'
-  end
-
-  test "setup new user with no email" do
-    Thread.current[:user] = @admin_user
-
-    begin
-      user = User.new
-      
-      User.setup user, 'http://openid/prefix'
-    rescue ArvadosModel::RuntimeError => e
-    end
-
-    assert (e.message.include? 'No email found'),
-        'Expected RuntimeError'
-  end
-
-  test "setup new user with email but no openid_prefix" do
-    Thread.current[:user] = @admin_user
-
-    begin
-      user = User.new
-      user.email = 'abc at xyz.com'
-      
-      User.setup user
-
-    rescue ArvadosModel::ArgumentError => e
-    end
-    assert (e.message.include? 'wrong number of arguments'),
-        'Expected ArgumentError'
-  end
-
-  test "setup new user with all input data" do
+  test "setup new user" do
     Thread.current[:user] = @admin_user
 
     email = 'abc at xyz.com'
@@ -201,6 +157,7 @@ class UserTest < ActiveSupport::TestCase
 
     user = User.new
     user.email = email
+    user.uuid = 'abcdefghijklmnop'
 
     vm = VirtualMachine.create
 
@@ -233,6 +190,7 @@ class UserTest < ActiveSupport::TestCase
 
     user = User.new
     user.email = email
+    user.uuid = 'abcdefghijklmnop'
 
     response = User.setup user, openid_prefix
 
@@ -249,9 +207,6 @@ class UserTest < ActiveSupport::TestCase
     verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
 
     # invoke setup again with repo_name
-    user = User.new
-    user.uuid = resp_user[:uuid]
-
     response = User.setup user, openid_prefix, 'test_repo'
     resp_user = find_obj_in_resp response, 'User', nil
     verify_user resp_user, email

commit 2013479619e9aa838037262564ac4f265f786ad0
Author: radhika chippada <radhika at radhika.curoverse>
Date:   Tue Mar 25 12:32:35 2014 -0400

    Ground work to remove found object logic from user model's setup method

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 3d6e61c..2c3b2df 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -90,6 +90,7 @@ class Arvados::V1::UsersController < ApplicationController
 
   # create user object and all the needed links
   def setup
+    @object = nil
     if params[:uuid]
       @object = User.find_by_uuid params[:uuid]
       if !@object
@@ -97,16 +98,29 @@ class Arvados::V1::UsersController < ApplicationController
       end
     else
       if !params[:user]
-        raise ArgumentError.new "Required uuid or email"
-      end
-      @object = model_class.new resource_attrs
-      if !params[:openid_prefix]
-        raise ArgumentError.new "Required openid_prefix parameter is missing."
+        raise ArgumentError.new "Required uuid or user"
+      else
+        if params[:user]['uuid']
+          @object = User.find_by_uuid params[:user]['uuid']
+        end
+
+        if !@object
+          if !params[:user]['email']
+            raise ArgumentError.new "Require user email"
+          end
+
+          if !params[:openid_prefix]
+            raise ArgumentError.new "Required openid_prefix parameter is missing."
+          end
+
+          @object = model_class.new resource_attrs
+        end
       end
     end
 
     @response = User.setup @object, params[:openid_prefix],
                 params[:repo_name], params[:vm_uuid]
+   
     render json: { kind: "arvados#HashList", items: @response }
   end
 
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index a487a33..81f096c 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -124,7 +124,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     response_body = JSON.parse(@response.body)
     response_errors = response_body['errors']
     assert_not_nil response_errors, 'Expected error in response'
-    assert (response_errors.first.include? 'RuntimeError: No email found'),
+    assert (response_errors.first.include? 'ArgumentError: Require user email'),
       'Expected RuntimeError'
   end
 
@@ -139,7 +139,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     response_body = JSON.parse(@response.body)
     response_errors = response_body['errors']
     assert_not_nil response_errors, 'Expected error in response'
-    assert (response_errors.first.include? 'Required uuid or email'),
+    assert (response_errors.first.include? 'Required uuid or user'),
         'Expected ArgumentError'
   end
 
@@ -155,8 +155,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     response_body = JSON.parse(@response.body)
     response_errors = response_body['errors']
     assert_not_nil response_errors, 'Expected error in response'
-    assert (response_errors.first.include? '<RuntimeError: No email found'),
-        'Expected RuntimeError'
+    assert (response_errors.first.include? '<ArgumentError: Require user email'),
+        'Expected ArgumentError'
   end
 
   test "invoke setup with existing uuid, vm and repo and verify links" do

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list