[F3] user.Put: idempotency

services/f3/driver/user.go

@@ -5,6 +5,7 @@ package driver
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
@@ -48,7 +49,29 @@ func (o *User) IsNil() bool {
 }
 
 func (o *User) Equals(other *User) bool {
-	return (o.ID == other.ID)
+	if o.ID != other.ID {
+		return false
+	}
+	//
+	// Only compare user data if both are managed by F3 otherwise
+	// they are equal if they have the same ID. Here is an example:
+	//
+	// * mirror from F3 to Forgejo => user jane created and assigned
+	//   ID 213 & IsF3()
+	// * mirror from F3 to Forgejo => user jane username in F3 is updated
+	//   the username for user ID 213 in Forgejo is also updated
+	// * user jane sign in with OAuth from the same source as the
+	//   F3 mirror. They are promoted to IsIndividual()
+	// * mirror from F3 to Forgejo => user jane username in F3 is updated
+	//   the username for user ID 213 in Forgejo is **NOT** updated, it
+	//   no longer is managed by F3
+	//
+	if !o.IsF3() || !other.IsF3() {
+		return true
+	}
+	return (o.Name == other.Name &&
+		o.FullName == other.FullName &&
+		o.Email == other.Email)
 }
 
 func (o *User) ToFormatInterface() format.Interface {
@@ -68,6 +91,7 @@ func (o *User) ToFormat() *format.User {
 func (o *User) FromFormat(user *format.User) {
 	*o = User{
 		User: user_model.User{
+			Type:     user_model.UserTypeF3,
 			ID:       user.Index.GetID(),
 			Name:     user.UserName,
 			FullName: user.Name,
@@ -122,12 +146,14 @@ func (o *UserProvider) FromFormat(ctx context.Context, p *format.User) *User {
 }
 
 func (o *UserProvider) GetObjects(ctx context.Context, page int) []*User {
-	users, _, err := user_model.SearchUsers(&user_model.SearchUserOptions{
+	sess := db.GetEngine(ctx).In("type", user_model.UserTypeIndividual, user_model.UserTypeF3)
-		Actor:       o.g.GetDoer(),
+	if page != 0 {
-		Type:        user_model.UserTypeIndividual,
+		sess = db.SetSessionPagination(sess, &db.ListOptions{Page: page, PageSize: o.g.perPage})
-		ListOptions: db.ListOptions{Page: page, PageSize: o.g.perPage},
+	}
-	})
+	sess = sess.Select("`user`.*")
-	if err != nil {
+	users := make([]*user_model.User, 0, o.g.perPage)
+
+	if err := sess.Find(&users); err != nil {
 		panic(fmt.Errorf("error while listing users: %v", err))
 	}
 	return f3_util.ConvertMap[*user_model.User, *User](users, UserConverter)
@@ -136,15 +162,29 @@ func (o *UserProvider) GetObjects(ctx context.Context, page int) []*User {
 func (o *UserProvider) ProcessObject(ctx context.Context, user *User) {
 }
 
+func GetUserByName(ctx context.Context, name string) (*user_model.User, error) {
+	if len(name) == 0 {
+		return nil, user_model.ErrUserNotExist{Name: name}
+	}
+	u := &user_model.User{Name: name}
+	has, err := db.GetEngine(ctx).In("type", user_model.UserTypeIndividual, user_model.UserTypeF3).Get(u)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, user_model.ErrUserNotExist{Name: name}
+	}
+	return u, nil
+}
+
 func (o *UserProvider) Get(ctx context.Context, exemplar *User) *User {
-	o.g.GetLogger().Debug("%+v", exemplar)
+	o.g.GetLogger().Debug("%+v", *exemplar)
 	var user *user_model.User
 	var err error
 	if exemplar.GetID() > 0 {
 		user, err = user_model.GetUserByID(ctx, exemplar.GetID())
-		o.g.GetLogger().Debug("GetUserByID: %+v %v", user, err)
+		o.g.GetLogger().Debug("%+v %v", user, err)
 	} else if exemplar.Name != "" {
-		user, err = user_model.GetUserByName(ctx, exemplar.Name)
+		user, err = GetUserByName(ctx, exemplar.Name)
 	} else {
 		panic("GetID() == 0 and UserName == \"\"")
 	}
@@ -152,26 +192,61 @@ func (o *UserProvider) Get(ctx context.Context, exemplar *User) *User {
 		if user_model.IsErrUserNotExist(err) {
 			return &User{}
 		}
-		panic(fmt.Errorf("user %v %w", exemplar, err))
+		panic(fmt.Errorf("user %+v %w", *exemplar, err))
 	}
 	return UserConverter(user)
 }
 
 func (o *UserProvider) Put(ctx context.Context, user *User) *User {
-	overwriteDefault := &user_model.CreateUserOverwriteOptions{
+	o.g.GetLogger().Trace("begin %+v", *user)
-		IsActive: util.OptionalBoolTrue,
+	u := &user_model.User{
+		ID:   user.GetID(),
+		Type: user_model.UserTypeF3,
 	}
-	u := user_model.User{
+	//
-		Name:     user.Name,
+	// Get the user, if any
-		FullName: user.FullName,
+	//
-		Email:    user.Email,
+	var has bool
-		Passwd:   user.Passwd,
+	var err error
-	}
+	if u.ID > 0 {
-	err := user_model.CreateUser(&u, overwriteDefault)
+		has, err = db.GetEngine(ctx).Get(u)
 		if err != nil {
 			panic(err)
 		}
-	return o.Get(ctx, UserConverter(&u))
+	}
+	//
+	// Set user information
+	//
+	u.Name = user.Name
+	u.LowerName = strings.ToLower(u.Name)
+	u.FullName = user.FullName
+	u.Email = user.Email
+	if !has {
+		//
+		// The user does not exist, create it
+		//
+		o.g.GetLogger().Trace("creating %+v", *u)
+		u.ID = 0
+		u.Passwd = user.Passwd
+		overwriteDefault := &user_model.CreateUserOverwriteOptions{
+			IsActive: util.OptionalBoolTrue,
+		}
+		err := user_model.CreateUser(u, overwriteDefault)
+		if err != nil {
+			panic(err)
+		}
+	} else {
+		//
+		// The user already exists, update it
+		//
+		o.g.GetLogger().Trace("updating %+v", *u)
+		if err := user_model.UpdateUserCols(ctx, u, "name", "lower_name", "email", "full_name"); err != nil {
+			panic(err)
+		}
+	}
+	r := o.Get(ctx, UserConverter(u))
+	o.g.GetLogger().Trace("finish %+v", r.User)
+	return r
 }
 
 func (o *UserProvider) Delete(ctx context.Context, user *User) *User {

tests/integration/f3_test.go

@@ -30,7 +30,7 @@ import (
 	f3_util "lab.forgefriends.org/friendlyforgeformat/gof3/util"
 )
 
-func TestF3(t *testing.T) {
+func TestF3Mirror(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, u *url.URL) {
 		AllowLocalNetworks := setting.Migrations.AllowLocalNetworks
 		setting.F3.Enabled = true
@@ -53,6 +53,7 @@ func TestF3(t *testing.T) {
 						Directory: tmpDir,
 					},
 					Features: gof3.AllFeatures,
+					Logger:   util.ToF3Logger(nil),
 				},
 				Remap: true,
 			})
@@ -75,7 +76,7 @@ func TestF3(t *testing.T) {
 		fixture.NewCommentReaction()
 
 		//
-		// Step 2: mirror the fixture into Forgejo
+		// Step 2: mirror F3 into Forgejo
 		//
 		doer, err := user_model.GetAdminUser(context.Background())
 		assert.NoError(t, err)
@@ -181,7 +182,7 @@ func TestMaybePromoteF3User(t *testing.T) {
 	assert.Equal(t, userAfterSignIn.Email, gitlabEmail)
 }
 
-func TestF3UserMapping(t *testing.T) {
+func TestF3UserMappingExisting(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, u *url.URL) {
 		AllowLocalNetworks := setting.Migrations.AllowLocalNetworks
 		setting.F3.Enabled = true
@@ -194,7 +195,7 @@ func TestF3UserMapping(t *testing.T) {
 			setting.AppVer = AppVer
 		}()
 
-		log.Debug("Step 1: create a fixture")
+		log.Debug("Step 1: create a fixture in F3")
 		fixtureNewF3Forge := func(t f3_tests.TestingT, user *format.User, tmpDir string) *f3_forges.ForgeRoot {
 			root := f3_forges.NewForgeRoot(&f3_f3.Options{
 				Options: gof3.Options{
@@ -213,7 +214,7 @@ func TestF3UserMapping(t *testing.T) {
 		fixture.NewUser(userID)
 		//		fixture.NewProject()
 
-		log.Debug("Step 2: mirror the fixture into Forgejo")
+		log.Debug("Step 2: mirror F3 into Forgejo")
 		//
 		// OAuth2 authentication source GitLab
 		//
@@ -268,3 +269,62 @@ func TestF3UserMapping(t *testing.T) {
 		assert.Contains(t, files, fmt.Sprintf("/user/%d", gitlabUser.ID))
 	})
 }
+
+func TestF3UserMappingNew(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		AllowLocalNetworks := setting.Migrations.AllowLocalNetworks
+		setting.F3.Enabled = true
+		setting.Migrations.AllowLocalNetworks = true
+		AppVer := setting.AppVer
+		// Gitea SDK (go-sdk) need to parse the AppVer from server response, so we must set it to a valid version string.
+		setting.AppVer = "1.16.0"
+		defer func() {
+			setting.Migrations.AllowLocalNetworks = AllowLocalNetworks
+			setting.AppVer = AppVer
+		}()
+
+		log.Debug("Step 1: create a fixture in F3")
+		fixtureNewF3Forge := func(t f3_tests.TestingT, user *format.User, tmpDir string) *f3_forges.ForgeRoot {
+			root := f3_forges.NewForgeRoot(&f3_f3.Options{
+				Options: gof3.Options{
+					Configuration: gof3.Configuration{
+						Directory: tmpDir,
+					},
+					Features: gof3.AllFeatures,
+					Logger:   util.ToF3Logger(nil),
+				},
+				Remap: true,
+			})
+			return root
+		}
+		fixture := f3_forges.NewFixture(t, f3_forges.FixtureForgeFactory{Fun: fixtureNewF3Forge, AdminRequired: false})
+		userID := int64(5432)
+		fixture.NewUser(userID)
+
+		log.Debug("Step 2: mirror F3 into Forgejo")
+		doer, err := user_model.GetAdminUser(context.Background())
+		assert.NoError(t, err)
+		forgejoLocalDestination := util.ForgejoForgeRoot(gof3.AllFeatures, doer, 0)
+		options := f3_common.NewMirrorOptionsRecurse()
+		forgejoLocalDestination.Forge.Mirror(context.Background(), fixture.Forge, options)
+
+		log.Debug("Step 3: change the Name of the user in F3 and mirror to Forgejo")
+		otherusername := "otheruser"
+		fixture.UserFormat.UserName = otherusername
+		fixture.Forge.Users.Upsert(context.Background(), fixture.UserFormat)
+		forgejoLocalDestination.Forge.Mirror(context.Background(), fixture.Forge, options)
+
+		log.Debug("Step 4: mirror Forgejo into F3 using the changed name")
+		f3 := util.F3ForgeRoot(gof3.AllFeatures, t.TempDir())
+		forgejoLocalOrigin := util.ForgejoForgeRoot(gof3.AllFeatures, doer, 0)
+		forgejoLocalOriginUser := forgejoLocalOrigin.Forge.Users.GetFromFormat(context.Background(), &format.User{UserName: otherusername})
+		options = f3_common.NewMirrorOptionsRecurse(forgejoLocalOriginUser)
+		f3.Forge.Mirror(context.Background(), forgejoLocalOrigin.Forge, options)
+
+		//
+		// verify the fixture and F3 are equivalent
+		//
+		files := f3_util.Command(context.Background(), "find", f3.GetDirectory())
+		assert.Contains(t, files, fmt.Sprintf("/user/%d", forgejoLocalOriginUser.GetID()))
+	})
+}