From 28c44ce1fc3531d069557eb555b6e83da2ba5cf8 Mon Sep 17 00:00:00 2001
From: Simone Gotti <simone.gotti@gmail.com>
Date: Sat, 6 Jul 2019 14:40:31 +0200
Subject: [PATCH] configstore: update/fix remotesource rename

* Override the provided remotesource id with the current one (it could not be
provided or provided with a different id but the remotesource ref is the way to
get the current remote source).

* When changing remotesource name check that a remote source with the new name
does not already exist.
---
 .../configstore/action/remotesource.go        | 20 +++++-
 .../services/configstore/configstore_test.go  | 67 +++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/internal/services/configstore/action/remotesource.go b/internal/services/configstore/action/remotesource.go
index 1474ddd..8976e60 100644
--- a/internal/services/configstore/action/remotesource.go
+++ b/internal/services/configstore/action/remotesource.go
@@ -125,13 +125,15 @@ func (h *ActionHandler) UpdateRemoteSource(ctx context.Context, req *UpdateRemot
 		return nil, err
 	}
 
+	var curRemoteSource *types.RemoteSource
 	var cgt *datamanager.ChangeGroupsUpdateToken
 
 	// must do all the checks in a single transaction to avoid concurrent changes
 	err := h.readDB.Do(func(tx *db.Tx) error {
 		var err error
-		// check duplicate remoteSource name
-		curRemoteSource, err := h.readDB.GetRemoteSourceByName(tx, req.RemoteSourceRef)
+
+		// check remotesource exists
+		curRemoteSource, err = h.readDB.GetRemoteSourceByName(tx, req.RemoteSourceRef)
 		if err != nil {
 			return err
 		}
@@ -139,6 +141,20 @@ func (h *ActionHandler) UpdateRemoteSource(ctx context.Context, req *UpdateRemot
 			return util.NewErrBadRequest(errors.Errorf("remotesource with ref %q doesn't exist", req.RemoteSourceRef))
 		}
 
+		if curRemoteSource.Name != req.RemoteSource.Name {
+			// check duplicate remoteSource name
+			u, err := h.readDB.GetRemoteSourceByName(tx, req.RemoteSource.Name)
+			if err != nil {
+				return err
+			}
+			if u != nil {
+				return util.NewErrBadRequest(errors.Errorf("remotesource %q already exists", u.Name))
+			}
+		}
+
+		// set/override ID that must be kept from the current remote source
+		req.RemoteSource.ID = curRemoteSource.ID
+
 		// changegroup is the remotesource id and also name since we could change the
 		// name so concurrently updating on the new name
 		cgNames := []string{util.EncodeSha256Hex("remotesourcename-" + req.RemoteSource.Name), util.EncodeSha256Hex("remotesourceid-" + req.RemoteSource.ID)}
diff --git a/internal/services/configstore/configstore_test.go b/internal/services/configstore/configstore_test.go
index ac03164..80fa885 100644
--- a/internal/services/configstore/configstore_test.go
+++ b/internal/services/configstore/configstore_test.go
@@ -829,6 +829,73 @@ func TestRemoteSource(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "test update remote source keeping same name",
+			f: func(ctx context.Context, t *testing.T, cs *Configstore) {
+				rs01 := &types.RemoteSource{
+					Name:               "rs01",
+					APIURL:             "https://api.example.com",
+					Type:               types.RemoteSourceTypeGitea,
+					AuthType:           types.RemoteSourceAuthTypeOauth2,
+					Oauth2ClientID:     "clientid",
+					Oauth2ClientSecret: "clientsecret",
+				}
+				rs01, err := cs.ah.CreateRemoteSource(ctx, rs01)
+				if err != nil {
+					t.Fatalf("unexpected err: %v", err)
+				}
+
+				rs01.APIURL = "https://api01.example.com"
+				req := &action.UpdateRemoteSourceRequest{
+					RemoteSourceRef: "rs01",
+					RemoteSource:    rs01,
+				}
+				_, err = cs.ah.UpdateRemoteSource(ctx, req)
+				if err != nil {
+					t.Fatalf("unexpected err: %v", err)
+				}
+			},
+		},
+		{
+			name: "test rename remote source to an already existing name",
+			f: func(ctx context.Context, t *testing.T, cs *Configstore) {
+				rs01 := &types.RemoteSource{
+					Name:               "rs01",
+					APIURL:             "https://api.example.com",
+					Type:               types.RemoteSourceTypeGitea,
+					AuthType:           types.RemoteSourceAuthTypeOauth2,
+					Oauth2ClientID:     "clientid",
+					Oauth2ClientSecret: "clientsecret",
+				}
+				rs01, err := cs.ah.CreateRemoteSource(ctx, rs01)
+				if err != nil {
+					t.Fatalf("unexpected err: %v", err)
+				}
+
+				rs02 := &types.RemoteSource{
+					Name:               "rs02",
+					APIURL:             "https://api.example.com",
+					Type:               types.RemoteSourceTypeGitea,
+					AuthType:           types.RemoteSourceAuthTypeOauth2,
+					Oauth2ClientID:     "clientid",
+					Oauth2ClientSecret: "clientsecret",
+				}
+				if _, err = cs.ah.CreateRemoteSource(ctx, rs02); err != nil {
+					t.Fatalf("unexpected err: %v", err)
+				}
+
+				expectedError := util.NewErrBadRequest(fmt.Errorf(`remotesource "rs02" already exists`))
+				rs01.Name = "rs02"
+				req := &action.UpdateRemoteSourceRequest{
+					RemoteSourceRef: "rs01",
+					RemoteSource:    rs01,
+				}
+				_, err = cs.ah.UpdateRemoteSource(ctx, req)
+				if err.Error() != expectedError.Error() {
+					t.Fatalf("expected err: %v, got err: %v", expectedError.Error(), err.Error())
+				}
+			},
+		},
 	}
 
 	for _, tt := range tests {