Merge pull request #128 from sgotti/configstore_projectgroup_move

configstore: implement project group move

internal/services/configstore/action/projectgroup.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"path"
+	"strings"
 
 	"agola.io/agola/internal/datamanager"
 	"agola.io/agola/internal/db"
@@ -28,6 +29,24 @@ import (
 	errors "golang.org/x/xerrors"
 )
 
+func (h *ActionHandler) GetProjectGroup(ctx context.Context, projectGroupRef string) (*types.ProjectGroup, error) {
+	var projectGroup *types.ProjectGroup
+	err := h.readDB.Do(ctx, func(tx *db.Tx) error {
+		var err error
+		projectGroup, err = h.readDB.GetProjectGroup(tx, projectGroupRef)
+		return err
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if projectGroup == nil {
+		return nil, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef))
+	}
+
+	return projectGroup, nil
+}
+
 func (h *ActionHandler) GetProjectGroupSubgroups(ctx context.Context, projectGroupRef string) ([]*types.ProjectGroup, error) {
 	var projectGroups []*types.ProjectGroup
 	err := h.readDB.Do(ctx, func(tx *db.Tx) error {
@@ -109,6 +128,10 @@ func (h *ActionHandler) CreateProjectGroup(ctx context.Context, projectGroup *ty
 		return nil, err
 	}
 
+	if projectGroup.Parent.Type != types.ConfigTypeProjectGroup {
+		return nil, util.NewErrBadRequest(errors.Errorf("wrong project group parent type %q", projectGroup.Parent.Type))
+	}
+
 	var cgt *datamanager.ChangeGroupsUpdateToken
 
 	// must do all the checks in a single transaction to avoid concurrent changes
@@ -120,6 +143,9 @@ func (h *ActionHandler) CreateProjectGroup(ctx context.Context, projectGroup *ty
 		if parentProjectGroup == nil {
 			return util.NewErrBadRequest(errors.Errorf("project group with id %q doesn't exist", projectGroup.Parent.ID))
 		}
+		// TODO(sgotti) now we are doing a very ugly thing setting the request
+		// projectgroup parent ID that can be both an ID or a ref. Then we are fixing
+		// it to an ID here. Change the request format to avoid this.
 		projectGroup.Parent.ID = parentProjectGroup.ID
 
 		groupPath, err := h.readDB.GetProjectGroupPath(tx, parentProjectGroup)
@@ -194,14 +220,28 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje
 		if pg == nil {
 			return util.NewErrBadRequest(errors.Errorf("project group with ref %q doesn't exist", req.ProjectGroupRef))
 		}
-		// check that the project.ID matches
+		// check that the project group ID matches
 		if pg.ID != req.ProjectGroup.ID {
 			return util.NewErrBadRequest(errors.Errorf("project group with ref %q has a different id", req.ProjectGroupRef))
 		}
 
-		// check parent exists
+		if pg.Parent.Type != req.ProjectGroup.Parent.Type {
-		switch pg.Parent.Type {
+			return util.NewErrBadRequest(errors.Errorf("changing project group parent type isn't supported"))
+		}
+
+		switch req.ProjectGroup.Parent.Type {
+		case types.ConfigTypeOrg:
+			fallthrough
+		case types.ConfigTypeUser:
+			// Cannot update root project group parent
+			if pg.Parent.Type != req.ProjectGroup.Parent.Type || pg.Parent.ID != req.ProjectGroup.Parent.ID {
+				return util.NewErrBadRequest(errors.Errorf("cannot change root project group parent type or id"))
+			}
+			// if the project group is a root project group force the name to be empty
+			req.ProjectGroup.Name = ""
+
 		case types.ConfigTypeProjectGroup:
+			// check parent exists
 			group, err := h.readDB.GetProjectGroup(tx, req.ProjectGroup.Parent.ID)
 			if err != nil {
 				return err
@@ -209,30 +249,25 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje
 			if group == nil {
 				return util.NewErrBadRequest(errors.Errorf("project group with id %q doesn't exist", req.ProjectGroup.Parent.ID))
 			}
+			// TODO(sgotti) now we are doing a very ugly thing setting the request
+			// projectgroup parent ID that can be both an ID or a ref. Then we are fixing
+			// it to an ID here. Change the request format to avoid this.
+			req.ProjectGroup.Parent.ID = group.ID
 		}
 
-		// currently we don't support changing parent
+		curPGParentPath, err := h.readDB.GetPath(tx, pg.Parent.Type, pg.Parent.ID)
-		// TODO(sgotti) handle project move (changed parent project group)
-		if pg.Parent.Type != req.ProjectGroup.Parent.Type {
-			return util.NewErrBadRequest(errors.Errorf("changing project group parent isn't supported"))
-		}
-		if pg.Parent.ID != req.ProjectGroup.Parent.ID {
-			return util.NewErrBadRequest(errors.Errorf("changing project group parent isn't supported"))
-		}
-
-		// if the project group is a root project group force the name to be empty
-		if pg.Parent.Type == types.ConfigTypeOrg ||
-			pg.Parent.Type == types.ConfigTypeUser {
-			req.ProjectGroup.Name = ""
-		}
-
-		pgPath, err := h.readDB.GetProjectGroupPath(tx, pg)
 		if err != nil {
 			return err
 		}
-		pgp := path.Join(path.Dir(pgPath), req.ProjectGroup.Name)
+		curPGP := path.Join(curPGParentPath, pg.Name)
 
-		if pg.Name != req.ProjectGroup.Name {
+		pgParentPath, err := h.readDB.GetPath(tx, req.ProjectGroup.Parent.Type, req.ProjectGroup.Parent.ID)
+		if err != nil {
+			return err
+		}
+		pgp := path.Join(pgParentPath, req.ProjectGroup.Name)
+
+		if pg.Name != req.ProjectGroup.Name || pg.Parent.ID != req.ProjectGroup.Parent.ID {
 			// check duplicate project group name
 			ap, err := h.readDB.GetProjectGroupByName(tx, req.ProjectGroup.Parent.ID, req.ProjectGroup.Name)
 			if err != nil {
@@ -241,11 +276,21 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje
 			if ap != nil {
 				return util.NewErrBadRequest(errors.Errorf("project group with name %q, path %q already exists", req.ProjectGroup.Name, pgp))
 			}
+			// Cannot move inside itself or a child project group
+			if strings.HasPrefix(pgp, curPGP+"/") {
+				return util.NewErrBadRequest(errors.Errorf("cannot move project group inside itself or child project group"))
+			}
 		}
 
 		// changegroup is the project group path. Use "projectpath" prefix as it must
 		// cover both projects and projectgroups
 		cgNames := []string{util.EncodeSha256Hex("projectpath-" + pgp)}
+
+		// add new projectpath
+		if pg.Parent.ID != req.ProjectGroup.Parent.ID {
+			cgNames = append(cgNames, util.EncodeSha256Hex("projectpath-"+pgp))
+		}
+
 		cgt, err = h.readDB.GetChangeGroupsUpdateTokens(tx, cgNames)
 		if err != nil {
 			return err

internal/services/configstore/api/projectgroup.go

@@ -30,7 +30,6 @@ import (
 
 	"github.com/gorilla/mux"
 	"go.uber.org/zap"
-	errors "golang.org/x/xerrors"
 )
 
 func projectGroupResponse(ctx context.Context, readDB *readdb.ReadDB, projectGroup *types.ProjectGroup) (*csapitypes.ProjectGroup, error) {
@@ -82,11 +81,12 @@ func projectGroupsResponse(ctx context.Context, readDB *readdb.ReadDB, projectGr
 
 type ProjectGroupHandler struct {
 	log    *zap.SugaredLogger
+	ah     *action.ActionHandler
 	readDB *readdb.ReadDB
 }
 
-func NewProjectGroupHandler(logger *zap.Logger, readDB *readdb.ReadDB) *ProjectGroupHandler {
+func NewProjectGroupHandler(logger *zap.Logger, ah *action.ActionHandler, readDB *readdb.ReadDB) *ProjectGroupHandler {
-	return &ProjectGroupHandler{log: logger.Sugar(), readDB: readDB}
+	return &ProjectGroupHandler{log: logger.Sugar(), ah: ah, readDB: readDB}
 }
 
 func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -99,20 +99,9 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	var projectGroup *types.ProjectGroup
+	projectGroup, err := h.ah.GetProjectGroup(ctx, projectGroupRef)
-	err = h.readDB.Do(ctx, func(tx *db.Tx) error {
+	if httpError(w, err) {
-		var err error
-		projectGroup, err = h.readDB.GetProjectGroup(tx, projectGroupRef)
-		return err
-	})
-	if err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
-	}
-
-	if projectGroup == nil {
-		httpError(w, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef)))
 		return
 	}
 

internal/services/configstore/configstore.go

@@ -177,7 +177,7 @@ func (s *Configstore) setupDefaultRouter() http.Handler {
 	maintenanceModeHandler := api.NewMaintenanceModeHandler(logger, s.ah, s.e)
 	exportHandler := api.NewExportHandler(logger, s.ah)
 
-	projectGroupHandler := api.NewProjectGroupHandler(logger, s.readDB)
+	projectGroupHandler := api.NewProjectGroupHandler(logger, s.ah, s.readDB)
 	projectGroupSubgroupsHandler := api.NewProjectGroupSubgroupsHandler(logger, s.ah, s.readDB)
 	projectGroupProjectsHandler := api.NewProjectGroupProjectsHandler(logger, s.ah, s.readDB)
 	createProjectGroupHandler := api.NewCreateProjectGroupHandler(logger, s.ah, s.readDB)

internal/services/configstore/configstore_test.go

@@ -831,6 +831,156 @@ func TestProjectUpdate(t *testing.T) {
 	})
 }
 
+func TestProjectGroupUpdate(t *testing.T) {
+	dir, err := ioutil.TempDir("", "agola")
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	ctx := context.Background()
+
+	cs, tetcd := setupConfigstore(ctx, t, dir)
+	defer shutdownEtcd(tetcd)
+
+	t.Logf("starting cs")
+	go func() {
+		_ = cs.Run(ctx)
+	}()
+
+	// TODO(sgotti) change the sleep with a real check that all is ready
+	time.Sleep(2 * time.Second)
+
+	user, err := cs.ah.CreateUser(ctx, &action.CreateUserRequest{UserName: "user01"})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// TODO(sgotti) change the sleep with a real check that user is in readdb
+	time.Sleep(2 * time.Second)
+
+	pg01 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic}
+	pg01, err = cs.ah.CreateProjectGroup(ctx, pg01)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	pg02 := &types.ProjectGroup{Name: "pg02", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic}
+	pg02, err = cs.ah.CreateProjectGroup(ctx, pg02)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	pg03 := &types.ProjectGroup{Name: "pg03", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic}
+	pg03, err = cs.ah.CreateProjectGroup(ctx, pg03)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	pg04 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name, "pg01")}, Visibility: types.VisibilityPublic}
+	_, err = cs.ah.CreateProjectGroup(ctx, pg04)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	pg05 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name, "pg02")}, Visibility: types.VisibilityPublic}
+	pg05, err = cs.ah.CreateProjectGroup(ctx, pg05)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	t.Run("rename project group keeping same parent", func(t *testing.T) {
+		projectGroupName := "pg03"
+		pg03.Name = "newpg03"
+		_, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg03})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+	})
+	t.Run("move project to project group having project with same name", func(t *testing.T) {
+		projectGroupName := "pg01"
+		expectedErr := fmt.Sprintf("project group with name %q, path %q already exists", projectGroupName, path.Join("user", user.Name, projectGroupName))
+		pg05.Parent.ID = path.Join("user", user.Name)
+		_, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, "pg02", projectGroupName), ProjectGroup: pg05})
+		if err.Error() != expectedErr {
+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+	})
+	t.Run("move project group to project group changing name", func(t *testing.T) {
+		projectGroupName := "pg01"
+		pg05.Name = "newpg01"
+		pg05.Parent.ID = path.Join("user", user.Name)
+		_, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, "pg02", projectGroupName), ProjectGroup: pg05})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+	})
+	t.Run("move project group inside itself", func(t *testing.T) {
+		projectGroupName := "pg02"
+		expectedErr := "cannot move project group inside itself or child project group"
+		pg02.Parent.ID = path.Join("user", user.Name, "pg02")
+		_, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg02})
+		if err.Error() != expectedErr {
+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+	})
+	t.Run("move project group to child project group", func(t *testing.T) {
+		projectGroupName := "pg01"
+		expectedErr := "cannot move project group inside itself or child project group"
+		pg01.Parent.ID = path.Join("user", user.Name, "pg01", "pg01")
+		_, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg01})
+		if err.Error() != expectedErr {
+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+	})
+	t.Run("change root project group parent", func(t *testing.T) {
+
+		var rootPG *types.ProjectGroup
+		rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		rootPG.Parent.ID = path.Join("user", user.Name, "pg01")
+
+		expectedErr := "cannot change root project group parent type or id"
+		_, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG})
+		if err.Error() != expectedErr {
+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+	})
+	t.Run("change root project group name", func(t *testing.T) {
+		var rootPG *types.ProjectGroup
+		rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		rootPG.Name = "rootpgnewname"
+
+		expectedErr := "project group name for root project group must be empty"
+		_, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG})
+		if err.Error() != expectedErr {
+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+	})
+	t.Run("change root project group visibility", func(t *testing.T) {
+		var rootPG *types.ProjectGroup
+		rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		rootPG.Visibility = types.VisibilityPrivate
+
+		_, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		rootPG, err = cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		if rootPG.Visibility != types.VisibilityPrivate {
+			t.Fatalf("expected visiblity %q, got visibility: %q", types.VisibilityPublic, rootPG.Visibility)
+		}
+	})
+}
+
 func TestProjectGroupDelete(t *testing.T) {
 	dir, err := ioutil.TempDir("", "agola")
 	if err != nil {