fix(machine): delete domain policy dependency and restructure functions (#4605)

* fix(machine): delete domain policy dependency and restructure functions

* fix(machine): delete domain policy dependency and restructure functions

* fix(machine): move check for username and name

* fix: correct unit test for machine

Co-authored-by: Livio Spring <livio.a@gmail.com>

internal/command/user_machine.go

@@ -2,108 +2,148 @@ package command
 
 import (
 	"context"
+
+	"github.com/zitadel/zitadel/internal/command/preparation"
 	"github.com/zitadel/zitadel/internal/domain"
 	caos_errs "github.com/zitadel/zitadel/internal/errors"
+	"github.com/zitadel/zitadel/internal/eventstore"
+	"github.com/zitadel/zitadel/internal/eventstore/v1/models"
 	"github.com/zitadel/zitadel/internal/repository/user"
-	"github.com/zitadel/zitadel/internal/telemetry/tracing"
 )
 
-func (c *Commands) AddMachine(ctx context.Context, orgID string, machine *domain.Machine) (*domain.Machine, error) {
-	if !machine.IsValid() {
-		return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-bm9Ds", "Errors.User.Invalid")
+type Machine struct {
+	models.ObjectRoot
+
+	Username    string
+	Name        string
+	Description string
+}
+
+func (m *Machine) content() error {
+	if m.ResourceOwner == "" {
+		return caos_errs.ThrowInvalidArgument(nil, "COMMAND-xiown2", "Errors.ResourceOwnerMissing")
 	}
-	domainPolicy, err := c.getOrgDomainPolicy(ctx, orgID)
+	if m.AggregateID == "" {
+		return caos_errs.ThrowInvalidArgument(nil, "COMMAND-p0p2mi", "Errors.User.UserIDMissing")
+	}
+	return nil
+}
+
+func (c *Commands) AddMachine(ctx context.Context, machine *Machine) (*domain.ObjectDetails, error) {
+	if machine.AggregateID == "" {
+		userID, err := c.idGenerator.Next()
+		if err != nil {
+			return nil, err
+		}
+		machine.AggregateID = userID
+	}
+	domainPolicy, err := c.getOrgDomainPolicy(ctx, machine.ResourceOwner)
 	if err != nil {
 		return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-3M9fs", "Errors.Org.DomainPolicy.NotFound")
 	}
-	userID, err := c.idGenerator.Next()
+
+	validation := prepareAddUserMachine(machine, domainPolicy)
+	cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, validation)
 	if err != nil {
 		return nil, err
 	}
-	return c.addMachineWithID(ctx, orgID, userID, machine, domainPolicy)
+	events, err := c.eventstore.Push(ctx, cmds...)
+	if err != nil {
+		return nil, err
+	}
+	return &domain.ObjectDetails{
+		Sequence:      events[len(events)-1].Sequence(),
+		EventDate:     events[len(events)-1].CreationDate(),
+		ResourceOwner: events[len(events)-1].Aggregate().ResourceOwner,
+	}, nil
 }
 
-func (c *Commands) AddMachineWithID(ctx context.Context, orgID string, userID string, machine *domain.Machine) (*domain.Machine, error) {
-	existingMachine, err := c.machineWriteModelByID(ctx, userID, orgID)
-	if err != nil {
-		return nil, err
+func prepareAddUserMachine(machine *Machine, domainPolicy *domain.DomainPolicy) preparation.Validation {
+	return func() (_ preparation.CreateCommands, err error) {
+		if err := machine.content(); err != nil {
+			return nil, err
+		}
+		if machine.Name == "" {
+			return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-bs9Ds", "Errors.User.Invalid")
+		}
+		if machine.Username == "" {
+			return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-bm9Ds", "Errors.User.Invalid")
+		}
+		return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
+			writeModel, err := getMachineWriteModelByID(ctx, filter, machine.AggregateID, machine.ResourceOwner)
+			if err != nil {
+				return nil, err
+			}
+			if isUserStateExists(writeModel.UserState) {
+				return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-k2una", "Errors.User.AlreadyExisting")
+			}
+			return []eventstore.Command{
+				user.NewMachineAddedEvent(
+					ctx,
+					UserAggregateFromWriteModel(&writeModel.WriteModel),
+					machine.Username,
+					machine.Name,
+					machine.Description,
+					domainPolicy.UserLoginMustBeDomain,
+				),
+			}, nil
+		}, nil
 	}
-	if isUserStateExists(existingMachine.UserState) {
-		return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-k2una", "Errors.User.AlreadyExisting")
-	}
-	domainPolicy, err := c.getOrgDomainPolicy(ctx, orgID)
-	if err != nil {
-		return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-3M9fs", "Errors.Org.DomainPolicy.NotFound")
-	}
-	if !domainPolicy.UserLoginMustBeDomain {
-		return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-6M0dd", "Errors.User.Invalid")
-	}
-	return c.addMachineWithID(ctx, orgID, userID, machine, domainPolicy)
 }
 
-func (c *Commands) addMachineWithID(ctx context.Context, orgID string, userID string, machine *domain.Machine, domainPolicy *domain.DomainPolicy) (*domain.Machine, error) {
-
-	machine.AggregateID = userID
-	addedMachine := NewMachineWriteModel(machine.AggregateID, orgID)
-	userAgg := UserAggregateFromWriteModel(&addedMachine.WriteModel)
-	events, err := c.eventstore.Push(ctx, user.NewMachineAddedEvent(
-		ctx,
-		userAgg,
-		machine.Username,
-		machine.Name,
-		machine.Description,
-		domainPolicy.UserLoginMustBeDomain,
-	))
+func (c *Commands) ChangeMachine(ctx context.Context, machine *Machine) (*domain.ObjectDetails, error) {
+	validation := prepareChangeUserMachine(machine)
+	cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, validation)
 	if err != nil {
 		return nil, err
 	}
-	err = AppendAndReduce(addedMachine, events...)
+	events, err := c.eventstore.Push(ctx, cmds...)
 	if err != nil {
 		return nil, err
 	}
-	return writeModelToMachine(addedMachine), nil
+	return &domain.ObjectDetails{
+		Sequence:      events[len(events)-1].Sequence(),
+		EventDate:     events[len(events)-1].CreationDate(),
+		ResourceOwner: events[len(events)-1].Aggregate().ResourceOwner,
+	}, nil
 }
 
-func (c *Commands) ChangeMachine(ctx context.Context, machine *domain.Machine) (*domain.Machine, error) {
-	existingMachine, err := c.machineWriteModelByID(ctx, machine.AggregateID, machine.ResourceOwner)
-	if err != nil {
-		return nil, err
-	}
-	if !isUserStateExists(existingMachine.UserState) {
-		return nil, caos_errs.ThrowNotFound(nil, "COMMAND-5M0od", "Errors.User.NotFound")
-	}
+func prepareChangeUserMachine(machine *Machine) preparation.Validation {
+	return func() (_ preparation.CreateCommands, err error) {
+		if err := machine.content(); err != nil {
+			return nil, err
+		}
+		return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
+			writeModel, err := getMachineWriteModelByID(ctx, filter, machine.AggregateID, machine.ResourceOwner)
+			if err != nil {
+				return nil, err
+			}
+			if !isUserStateExists(writeModel.UserState) {
+				return nil, caos_errs.ThrowNotFound(nil, "COMMAND-5M0od", "Errors.User.NotFound")
+			}
 
-	userAgg := UserAggregateFromWriteModel(&existingMachine.WriteModel)
-	changedEvent, hasChanged, err := existingMachine.NewChangedEvent(ctx, userAgg, machine.Name, machine.Description)
-	if err != nil {
-		return nil, err
-	}
-	if !hasChanged {
-		return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-2n8vs", "Errors.User.NotChanged")
-	}
+			event, hasChanged, err := writeModel.NewChangedEvent(ctx, UserAggregateFromWriteModel(&writeModel.WriteModel), machine.Name, machine.Description)
+			if err != nil {
+				return nil, err
+			}
+			if !hasChanged {
+				return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-2n8vs", "Errors.User.NotChanged")
+			}
 
-	events, err := c.eventstore.Push(ctx, changedEvent)
-	if err != nil {
-		return nil, err
+			return []eventstore.Command{
+				event,
+			}, nil
+		}, nil
 	}
-	err = AppendAndReduce(existingMachine, events...)
-	if err != nil {
-		return nil, err
-	}
-	return writeModelToMachine(existingMachine), nil
 }
 
-func (c *Commands) machineWriteModelByID(ctx context.Context, userID, resourceOwner string) (writeModel *MachineWriteModel, err error) {
-	if userID == "" {
-		return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-0Plof", "Errors.User.UserIDMissing")
-	}
-	ctx, span := tracing.NewSpan(ctx)
-	defer func() { span.EndWithError(err) }()
-
-	writeModel = NewMachineWriteModel(userID, resourceOwner)
-	err = c.eventstore.FilterToQueryReducer(ctx, writeModel)
+func getMachineWriteModelByID(ctx context.Context, filter preparation.FilterToQueryReducer, userID, resourceOwner string) (_ *MachineWriteModel, err error) {
+	writeModel := NewMachineWriteModel(userID, resourceOwner)
+	events, err := filter(ctx, writeModel.Query())
 	if err != nil {
 		return nil, err
 	}
-	return writeModel, nil
+	writeModel.AppendEvents(events...)
+	err = writeModel.Reduce()
+	return writeModel, err
 }

internal/command/user_machine_test.go

@@ -24,11 +24,10 @@ func TestCommandSide_AddMachine(t *testing.T) {
 	}
 	type args struct {
 		ctx     context.Context
-		orgID   string
-		machine *domain.Machine
+		machine *Machine
 	}
 	type res struct {
-		want *domain.Machine
+		want *domain.ObjectDetails
 		err  func(error) bool
 	}
 	tests := []struct {
@@ -38,16 +37,29 @@ func TestCommandSide_AddMachine(t *testing.T) {
 		res    res
 	}{
 		{
-			name: "user invalid, invalid argument error",
+			name: "user invalid, invalid argument error name",
 			fields: fields{
 				eventstore: eventstoreExpect(
 					t,
+					expectFilter(
+						eventFromEventPusher(
+							org.NewDomainPolicyAddedEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								true,
+								true,
+								true,
+							),
+						),
+					),
 				),
+				idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
+				ctx: context.Background(),
+				machine: &Machine{
+					ObjectRoot: models.ObjectRoot{
+						ResourceOwner: "org1",
+					},
 					Username: "username",
 				},
 			},
@@ -55,6 +67,37 @@ func TestCommandSide_AddMachine(t *testing.T) {
 				err: caos_errs.IsErrorInvalidArgument,
 			},
 		},
+		{
+			name: "user invalid, invalid argument error username",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+					expectFilter(
+						eventFromEventPusher(
+							org.NewDomainPolicyAddedEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								true,
+								true,
+								true,
+							),
+						),
+					),
+				),
+				idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
+			},
+			args: args{
+				ctx: context.Background(),
+				machine: &Machine{
+					ObjectRoot: models.ObjectRoot{
+						ResourceOwner: "org1",
+					},
+					Name: "name",
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
 		{
 			name: "org policy not found, precondition error",
 			fields: fields{
@@ -63,13 +106,16 @@ func TestCommandSide_AddMachine(t *testing.T) {
 					expectFilter(),
 					expectFilter(),
 				),
+				idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
-					Username: "username",
+				ctx: context.Background(),
+				machine: &Machine{
+					ObjectRoot: models.ObjectRoot{
+						ResourceOwner: "org1",
+					},
 					Name:     "name",
+					Username: "username",
 				},
 			},
 			res: res{
@@ -91,6 +137,7 @@ func TestCommandSide_AddMachine(t *testing.T) {
 							),
 						),
 					),
+					expectFilter(),
 					expectPush(
 						[]*repository.Event{
 							eventFromEventPusher(
@@ -109,24 +156,19 @@ func TestCommandSide_AddMachine(t *testing.T) {
 				idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
-					Username:    "username",
+				ctx: context.Background(),
+				machine: &Machine{
+					ObjectRoot: models.ObjectRoot{
+						ResourceOwner: "org1",
+					},
 					Description: "description",
 					Name:        "name",
+					Username:    "username",
 				},
 			},
 			res: res{
-				want: &domain.Machine{
-					ObjectRoot: models.ObjectRoot{
-						AggregateID:   "user1",
-						ResourceOwner: "org1",
-					},
-					Username:    "username",
-					Name:        "name",
-					Description: "description",
-					State:       domain.UserStateActive,
+				want: &domain.ObjectDetails{
+					ResourceOwner: "org1",
 				},
 			},
 		},
@@ -137,7 +179,7 @@ func TestCommandSide_AddMachine(t *testing.T) {
 				eventstore:  tt.fields.eventstore,
 				idGenerator: tt.fields.idGenerator,
 			}
-			got, err := r.AddMachine(tt.args.ctx, tt.args.orgID, tt.args.machine)
+			got, err := r.AddMachine(tt.args.ctx, tt.args.machine)
 			if tt.res.err == nil {
 				assert.NoError(t, err)
 			}
@@ -157,11 +199,10 @@ func TestCommandSide_ChangeMachine(t *testing.T) {
 	}
 	type args struct {
 		ctx     context.Context
-		orgID   string
-		machine *domain.Machine
+		machine *Machine
 	}
 	type res struct {
-		want *domain.Machine
+		want *domain.ObjectDetails
 		err  func(error) bool
 	}
 	tests := []struct {
@@ -178,9 +219,11 @@ func TestCommandSide_ChangeMachine(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
+				ctx: context.Background(),
+				machine: &Machine{
+					ObjectRoot: models.ObjectRoot{
+						ResourceOwner: "org1",
+					},
 					Username: "username",
 				},
 			},
@@ -197,14 +240,14 @@ func TestCommandSide_ChangeMachine(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
+				ctx: context.Background(),
+				machine: &Machine{
 					ObjectRoot: models.ObjectRoot{
-						AggregateID: "user1",
+						ResourceOwner: "org1",
+						AggregateID:   "user1",
 					},
-					Username: "username",
 					Name:     "name",
+					Username: "username",
 				},
 			},
 			res: res{
@@ -230,11 +273,11 @@ func TestCommandSide_ChangeMachine(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
+				ctx: context.Background(),
+				machine: &Machine{
 					ObjectRoot: models.ObjectRoot{
-						AggregateID: "user1",
+						ResourceOwner: "org1",
+						AggregateID:   "user1",
 					},
 					Name:        "name",
 					Description: "description",
@@ -270,26 +313,19 @@ func TestCommandSide_ChangeMachine(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx:   context.Background(),
-				orgID: "org1",
-				machine: &domain.Machine{
+				ctx: context.Background(),
+				machine: &Machine{
 					ObjectRoot: models.ObjectRoot{
-						AggregateID: "user1",
+						ResourceOwner: "org1",
+						AggregateID:   "user1",
 					},
-					Description: "description1",
 					Name:        "name1",
+					Description: "description1",
 				},
 			},
 			res: res{
-				want: &domain.Machine{
-					ObjectRoot: models.ObjectRoot{
-						AggregateID:   "user1",
-						ResourceOwner: "org1",
-					},
-					Username:    "username",
-					Name:        "name1",
-					Description: "description1",
-					State:       domain.UserStateActive,
+				want: &domain.ObjectDetails{
+					ResourceOwner: "org1",
 				},
 			},
 		},