When create is invoked with user object as input, check if object exists for the...

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index a929476a2b1b5d9cf855253d0b07059b72d64f2c..6558655e65797bd6625450cd03c0aa2b5a75c978 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -105,9 +105,8 @@ class Arvados::V1::UsersController < ApplicationController
                # If user_param is passed, lookup for user. If exists, skip create and create any missing links. 
                if params[:user_param]
                        begin 
-                               @object_found = find_user_from_user_param params[:user_param]
+                               @object_found = find_user_from_input params[:user_param], params[:user_param]
                  end
-
                        if !@object_found
                                @object = User.new              # when user_param is used, it will be used as user object
                                @object[:email] = params[:user_param]                           
@@ -115,13 +114,17 @@ class Arvados::V1::UsersController < ApplicationController
                        else
                                @object = @object_found
                        end
-               else            # need to create user for the given :user data
-                       need_to_create = true
+               else            # need to create user for the given user data
+                       @object_found = find_user_from_input @object[:uuid], @object[:email]
+                       if !@object_found
+                       need_to_create = true
+                       else
+                               @object = @object_found
+                       end
                end
 
                # if just probing, return any object found      
                if just_probe 
-                       @object[:email] = nil   
                        show
                  return
                end
@@ -137,13 +140,13 @@ class Arvados::V1::UsersController < ApplicationController
                                if [] == oid_login_perm
                                        # create openid login permission
                oid_login_perm = Link.create(link_class: 'permission',
-                                          name: 'can_login',
-                                        tail_kind: 'email',
-                                        tail_uuid: @object[:email],
-                                        head_kind: 'arvados#user',
-                                        head_uuid: @object[:uuid],
-                                        properties: login_perm_props
-                                       )
+                                                name: 'can_login',
+                                                tail_kind: 'email',
+                                          tail_uuid: @object[:email],
+                                          head_kind: 'arvados#user',
+                                          head_uuid: @object[:uuid],
+                                          properties: login_perm_props
+                                               )
                                        logger.info { "openid login permission: " + oid_login_perm[:uuid] }
                                end
          else
@@ -162,20 +165,25 @@ class Arvados::V1::UsersController < ApplicationController
        protected 
 
        # find the user from the given user parameter
-       def find_user_from_user_param(user_param)
-               found_object = User.find_by_uuid user_param
+       def find_user_from_input(user_uuid, user_email)
+               if user_uuid
+                       found_object = User.find_by_uuid user_uuid
+               end
 
                if !found_object
                        begin
-                               if !user_param.match(/\w\@\w+\.\w+/)
-                                       logger.warn ("Given user param is not valid email format: #{user_param}")
+                               if !user_email
+                                       return
+                               end
+
+                               if !user_email.match(/\w\@\w+\.\w+/)
+                                       logger.warn ("Given user param is not valid email format: #{user_email}")
                                        raise ArgumentError.new "User param is not of valid email format. Stop"
                                else
-          found_objects = User.where('email=?', user_param)  
-       
+          found_objects = User.where('email=?', user_email)  
                                        if found_objects.size > 1
-                                               logger.warn ("Found #{found_objects.size} users with email #{user_param}. Stop.")
-                                               raise ArgumentError.new "Found #{found_objects.size} users with email #{user_param}. Stop."
+                                               logger.warn ("Found #{found_objects.size} users with email #{user_email}. Stop.")
+                                               raise ArgumentError.new "Found #{found_objects.size} users with email #{user_email}. Stop."
                                        elsif found_objects.size == 1
                                                found_object = found_objects.first
                                        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 b7ad8ddd827d7de6abe51d83f833deae90a517f8..fe0c22a1d741ea21ce54f5f6491ff981448a309a 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -194,10 +194,9 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_response :success
     response_object = JSON.parse(@response.body)
     assert_nil response_object['uuid'], 'expected null uuid since no object created due to just probe'
-    assert_nil response_object['email'], 'expecting null email'
        end
 
-       test "create user twice and check links are not recreated" do
+       test "create user twice with user param and check links are not recreated" do
     authorize_with :admin
 
     post :create, {
@@ -221,6 +220,34 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
       user: {}
     }
 
+    assert_response :success
+    response_object2 = JSON.parse(@response.body)
+    assert_equal response_object['uuid'], response_object2['uuid'], 'expected same uuid as first create operation'
+    assert_equal response_object['email'], 'abc@xyz.com', 'expecting given email'
+               verify_num_links @all_links_at_start, 3         # openid, group, and repo links. no vm link
+       end
+
+       test "create user twice with user object as input and check links are not recreated" do
+    authorize_with :admin
+
+    post :create, {
+      repo_name: 'test_repo',
+      user: {
+                               email: 'abc@xyz.com'
+                       }
+    }
+
+    assert_response :success
+    response_object = JSON.parse(@response.body)
+    assert_not_nil response_object['uuid'], 'expected non-null uuid for the newly created user'
+    assert_equal response_object['email'], 'abc@xyz.com', 'expecting given email'
+               verify_num_links @all_links_at_start, 3         # openid, group, and repo links. no vm link
+
+               # create again
+         post :create, user: {
+                       email: 'abc@xyz.com'
+    }
+
     assert_response :success
     response_object2 = JSON.parse(@response.body)
     assert_equal response_object['uuid'], response_object2['uuid'], 'expected same uuid as first create operation'