[ARVADOS] updated: 1.3.0-1876-g2ce754717
Git user
git at public.curoverse.com
Mon Nov 18 21:34:39 UTC 2019
Summary of changes:
lib/controller/federation/conn.go | 24 ++++++-------
lib/controller/federation/user_test.go | 11 +++---
lib/controller/router/router.go | 7 ++++
lib/controller/rpc/conn.go | 7 ++++
sdk/go/arvados/api.go | 6 ++++
sdk/go/arvadostest/api.go | 4 +++
.../app/controllers/arvados/v1/users_controller.rb | 25 +++++++++++--
services/api/config/routes.rb | 1 +
.../functional/arvados/v1/users_controller_test.rb | 41 ++++++++++++++++++++++
9 files changed, 106 insertions(+), 20 deletions(-)
via 2ce754717199a14e89f7a7389745f051ad0fa4ad (commit)
from b047cc92acab06d4c1f1d80173486f951b63d729 (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 2ce754717199a14e89f7a7389745f051ad0fa4ad
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Mon Nov 18 16:33:41 2019 -0500
15720: Batch user update API.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 47ae0ba9e..62a060b6c 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -350,7 +350,7 @@ func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (ar
if err != nil {
return resp, err
}
- ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+ batchUpdates := arvados.UserBatchUpdateOptions{}
for _, user := range resp.Items {
if !strings.HasPrefix(user.UUID, id) {
continue
@@ -380,19 +380,13 @@ func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (ar
}
}
}
- _, err = conn.local.UserUpdate(ctxRoot, arvados.UpdateOptions{
- UUID: user.UUID,
- Attrs: updates,
- })
- if errStatus(err) == http.StatusNotFound {
- updates["uuid"] = user.UUID
- _, err = conn.local.UserCreate(ctxRoot, arvados.CreateOptions{
- Attrs: updates,
- })
- }
+ batchUpdates[user.UUID] = updates
+ }
+ if len(batchUpdates) > 0 {
+ ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+ _, err = conn.local.UserBatchUpdate(ctxRoot, batchUpdates)
if err != nil {
- logger.WithError(err).WithField("UUID", user.UUID).Error("error updating local user record")
- return arvados.UserList{}, fmt.Errorf("error updating local user record: %s", err)
+ return arvados.UserList{}, fmt.Errorf("error updating local user records: %s", err)
}
}
return resp, nil
@@ -445,6 +439,10 @@ func (conn *Conn) UserDelete(ctx context.Context, options arvados.DeleteOptions)
return conn.chooseBackend(options.UUID).UserDelete(ctx, options)
}
+func (conn *Conn) UserBatchUpdate(ctx context.Context, options arvados.UserBatchUpdateOptions) (arvados.UserList, error) {
+ return conn.local.UserBatchUpdate(ctx, options)
+}
+
func (conn *Conn) APIClientAuthorizationCurrent(ctx context.Context, options arvados.GetOptions) (arvados.APIClientAuthorization, error) {
return conn.chooseBackend(options.UUID).APIClientAuthorizationCurrent(ctx, options)
}
diff --git a/lib/controller/federation/user_test.go b/lib/controller/federation/user_test.go
index 993be9b0b..a4b085d72 100644
--- a/lib/controller/federation/user_test.go
+++ b/lib/controller/federation/user_test.go
@@ -53,7 +53,7 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
c.Check(stub.Calls(nil), check.HasLen, 0)
} else if updateFail {
c.Logf("... err %#v", err)
- calls := stub.Calls(stub.UserUpdate)
+ calls := stub.Calls(stub.UserBatchUpdate)
if c.Check(calls, check.HasLen, 1) {
c.Logf("... stub.UserUpdate called with options: %#v", calls[0].Options)
shouldUpdate := map[string]bool{
@@ -84,8 +84,11 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
}
}
}
+ var uuid string
+ for uuid = range calls[0].Options.(arvados.UserBatchUpdateOptions) {
+ }
for k, shouldFind := range shouldUpdate {
- _, found := calls[0].Options.(arvados.UpdateOptions).Attrs[k]
+ _, found := calls[0].Options.(arvados.UserBatchUpdateOptions)[uuid][k]
c.Check(found, check.Equals, shouldFind, check.Commentf("offending attr: %s", k))
}
}
@@ -93,13 +96,13 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
updates := 0
for _, d := range spy.RequestDumps {
d := string(d)
- if strings.Contains(d, "PATCH /arvados/v1/users/zzzzz-tpzed-") {
+ if strings.Contains(d, "PATCH /arvados/v1/users/batch") {
c.Check(d, check.Matches, `(?ms).*Authorization: Bearer `+arvadostest.SystemRootToken+`.*`)
updates++
}
}
c.Check(err, check.IsNil)
- c.Check(updates, check.Equals, len(userlist.Items))
+ c.Check(updates, check.Equals, 1)
c.Logf("... response items %#v", userlist.Items)
}
}
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 250f3cb45..47082197a 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -283,6 +283,13 @@ func (rtr *router) addRoutes() {
},
},
{
+ arvados.EndpointUserBatchUpdate,
+ func() interface{} { return &arvados.UserBatchUpdateOptions{} },
+ func(ctx context.Context, opts interface{}) (interface{}, error) {
+ return rtr.fed.UserBatchUpdate(ctx, *opts.(*arvados.UserBatchUpdateOptions))
+ },
+ },
+ {
arvados.EndpointUserDelete,
func() interface{} { return &arvados.DeleteOptions{} },
func(ctx context.Context, opts interface{}) (interface{}, error) {
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 25efcfd43..66523e5ac 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -406,3 +406,10 @@ func (conn *Conn) UserSessionCreate(ctx context.Context, options UserSessionCrea
err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
return resp, err
}
+
+func (conn *Conn) UserBatchUpdate(ctx context.Context, options arvados.UserBatchUpdateOptions) (arvados.UserList, error) {
+ ep := arvados.APIEndpoint{Method: "PATCH", Path: "arvados/v1/users/batch_update"}
+ var resp arvados.UserList
+ err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+ return resp, err
+}
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index d86df9ef3..917a61f3a 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -54,6 +54,7 @@ var (
EndpointUserUnsetup = APIEndpoint{"POST", "arvados/v1/users/{uuid}/unsetup", ""}
EndpointUserUpdate = APIEndpoint{"PATCH", "arvados/v1/users/{uuid}", "user"}
EndpointUserUpdateUUID = APIEndpoint{"POST", "arvados/v1/users/{uuid}/update_uuid", ""}
+ EndpointUserBatchUpdate = APIEndpoint{"PATCH", "arvados/v1/users/batch", ""}
EndpointAPIClientAuthorizationCurrent = APIEndpoint{"GET", "arvados/v1/api_client_authorizations/current", ""}
)
@@ -119,6 +120,10 @@ type UserMergeOptions struct {
NewUserToken string `json:"new_user_token,omitempty"`
}
+type UserBatchUpdateOptions map[string]map[string]interface{}
+
+type UserBatchUpdateResponse struct{}
+
type DeleteOptions struct {
UUID string `json:"uuid"`
}
@@ -166,5 +171,6 @@ type API interface {
UserGetSystem(ctx context.Context, options GetOptions) (User, error)
UserList(ctx context.Context, options ListOptions) (UserList, error)
UserDelete(ctx context.Context, options DeleteOptions) (User, error)
+ UserBatchUpdate(context.Context, UserBatchUpdateOptions) (UserList, error)
APIClientAuthorizationCurrent(ctx context.Context, options GetOptions) (APIClientAuthorization, error)
}
diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index f18af8caa..91e3ee8ba 100644
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -169,6 +169,10 @@ func (as *APIStub) UserMerge(ctx context.Context, options arvados.UserMergeOptio
as.appendCall(as.UserMerge, ctx, options)
return arvados.User{}, as.Error
}
+func (as *APIStub) UserBatchUpdate(ctx context.Context, options arvados.UserBatchUpdateOptions) (arvados.UserList, error) {
+ as.appendCall(as.UserBatchUpdate, ctx, options)
+ return arvados.UserList{}, as.Error
+}
func (as *APIStub) APIClientAuthorizationCurrent(ctx context.Context, options arvados.GetOptions) (arvados.APIClientAuthorization, error) {
as.appendCall(as.APIClientAuthorizationCurrent, ctx, options)
return arvados.APIClientAuthorization{}, as.Error
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 2889eacee..ddf74cec6 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -4,12 +4,31 @@
class Arvados::V1::UsersController < ApplicationController
accept_attribute_as_json :prefs, Hash
+ accept_param_as_json :updates
skip_before_action :find_object_by_uuid, only:
- [:activate, :current, :system, :setup, :merge]
+ [:activate, :current, :system, :setup, :merge, :batch_update]
skip_before_action :render_404_if_no_object, only:
- [:activate, :current, :system, :setup, :merge]
- before_action :admin_required, only: [:setup, :unsetup, :update_uuid]
+ [:activate, :current, :system, :setup, :merge, :batch_update]
+ before_action :admin_required, only: [:setup, :unsetup, :update_uuid, :batch_update]
+
+ # Internal API used by controller to update local cache of user
+ # records from LoginCluster.
+ def batch_update
+ @objects = []
+ params[:updates].andand.each do |uuid, attrs|
+ begin
+ u = User.find_or_create_by(uuid: uuid)
+ rescue ActiveRecord::RecordNotUnique
+ retry
+ end
+ u.update_attributes!(attrs)
+ @objects << u
+ end
+ @offset = 0
+ @limit = -1
+ render_list
+ end
def current
if current_user
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index b54c3c5bf..8afd22192 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -83,6 +83,7 @@ Server::Application.routes.draw do
post 'unsetup', on: :member
post 'update_uuid', on: :member
post 'merge', on: :collection
+ patch 'batch_update', on: :collection
end
resources :virtual_machines do
get 'logins', on: :member
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 d5db10396..74f017a67 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -1043,6 +1043,47 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_nil(users(:project_viewer).redirect_to_user_uuid)
end
+ test "batch update fails for non-admin" do
+ authorize_with(:active)
+ patch(:batch_update, params: {updates: {}})
+ assert_response(403)
+ end
+
+ test "batch update" do
+ existinguuid = 'remot-tpzed-foobarbazwazqux'
+ newuuid = 'remot-tpzed-newnarnazwazqux'
+ act_as_system_user do
+ User.create!(uuid: existinguuid, email: 'root at existing.example.com')
+ end
+
+ authorize_with(:admin)
+ patch(:batch_update,
+ params: {
+ updates: {
+ existinguuid => {
+ 'first_name' => 'root',
+ 'email' => 'root at remot.example.com',
+ 'is_active' => true,
+ 'is_admin' => true,
+ 'prefs' => {'foo' => 'bar'},
+ },
+ newuuid => {
+ 'first_name' => 'noot',
+ 'email' => 'root at remot.example.com',
+ },
+ }})
+ assert_response(:success)
+
+ assert_equal('root', User.find_by_uuid(existinguuid).first_name)
+ assert_equal('root at remot.example.com', User.find_by_uuid(existinguuid).email)
+ assert_equal(true, User.find_by_uuid(existinguuid).is_active)
+ assert_equal(true, User.find_by_uuid(existinguuid).is_admin)
+ assert_equal({'foo' => 'bar'}, User.find_by_uuid(existinguuid).prefs)
+
+ assert_equal('noot', User.find_by_uuid(newuuid).first_name)
+ assert_equal('root at remot.example.com', User.find_by_uuid(newuuid).email)
+ end
+
NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name",
"last_name", "username"].sort
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list