some minor refactoring

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index e2c4130b0c1373f1de43b56c1bc45fe02b620c4a..ae3f31acbe1bba5f1410fb6a8f6ae08303b0be88 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -111,8 +111,12 @@ 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
@@ -120,22 +124,17 @@ class User < ArvadosModel
         raise "No email found in the input. Aborting user creation."
       end
 
-      if !user.save
-        raise "Save failed"
-      end
-    else
-      user = found
-    end
-
-    # Check oid_login_perm
-    oid_login_perms = Link.where(tail_uuid: user.email,
-                                head_kind: 'arvados#user',
-                                link_class: 'permission',
-                                name: 'can_login')
+      user.save!
+    
+      # 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,
@@ -143,9 +142,10 @@ 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
+        logger.info { "openid login permission: " + oid_login_perm[:uuid] }
+      else
+        oid_login_perm = oid_login_perms.first
+      end
     end
 
     # create repo, vm, and group links

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 c36461306009bfc7c1e64d45822e68f1869389e7..702209b239239c90a1f4793e5b4ca68895f52c78 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -106,8 +106,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     post :setup, {
       uuid: 'bogus_uuid',
       repo_name: 'test_repo',
-      vm_uuid: @vm_uuid,
-      openid_prefix: 'https://www.google.com/accounts/o8/id'
+      vm_uuid: @vm_uuid
     }
     response_body = JSON.parse(@response.body)
     response_errors = response_body['errors']
@@ -135,7 +134,6 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     authorize_with :admin
 
     post :setup, {
-      #uuid: 'not_an_existing_uuid_and_not_email_format',
       repo_name: 'test_repo',
       vm_uuid: @vm_uuid,
       openid_prefix: 'https://www.google.com/accounts/o8/id'
@@ -174,8 +172,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     post :setup, {
       uuid: inactive_user['uuid'],
       repo_name: 'test_repo',
-      vm_uuid: @vm_uuid,
-      openid_prefix: 'https://www.google.com/accounts/o8/id'
+      vm_uuid: @vm_uuid
     }
 
     assert_response :success
@@ -230,8 +227,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
 
     post :setup, {
       uuid: inactive_user['uuid'],
-      user: {email: 'junk_email'},
-      openid_prefix: 'https://www.google.com/accounts/o8/id'
+      user: {email: 'junk_email'}
     }
 
     assert_response :success
@@ -550,9 +546,6 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal 'test@abc.com', created['email'], 'expected input email'
 
      # verify links
-    verify_link response_items, 'oid_login_perm', true, 'permission', 'can_login',
-        created['uuid'], created['email'], 'arvados#user', false, 'User'
-
     verify_link response_items, 'group_perm', true, 'permission', 'can_read',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
@@ -580,9 +573,6 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal created['email'], 'test@abc.com', 'expected original email'
 
     # verify links
-    verify_link response_items, 'oid_login_perm', true, 'permission', 'can_login',
-        created['uuid'], created['email'], 'arvados#user', false, 'User'
-
     verify_link response_items, 'group_perm', true, 'permission', 'can_read',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 

diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index e9886af0a54f06b0b531e7dad4891015e879164b..2b6786475e13c8c8e220a4edef4aeb2c7cac9a46 100644 (file)
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -258,12 +258,6 @@ class UserTest < ActiveSupport::TestCase
     verify_user resp_user, email
     assert_equal user.uuid, resp_user[:uuid], 'expected uuid not found'
 
-    oid_login_perm = response[:oid_login_perm]
-    verify_link oid_login_perm, 'permission', 'can_login', resp_user[:email],
-        resp_user[:uuid]
-    assert_equal openid_prefix, oid_login_perm[:properties][:identity_url_prefix],
-        'expected identity_url_prefix not found for oid_login_perm'
-
     verify_link response[:group_perm], 'permission', 'can_read', 
         resp_user[:uuid], nil
 
@@ -279,12 +273,6 @@ class UserTest < ActiveSupport::TestCase
     verify_user resp_user, email
     assert_equal user.uuid, resp_user[:uuid], 'expected uuid not found'
 
-    oid_login_perm = response[:oid_login_perm]
-    verify_link oid_login_perm, 'permission', 'can_login', resp_user[:email],
-        resp_user[:uuid]
-    assert_equal openid_prefix, oid_login_perm[:properties][:identity_url_prefix],
-        'expected identity_url_prefix not found for oid_login_perm'
-
     verify_link response[:group_perm], 'permission', 'can_read', 
         resp_user[:uuid], nil