Refactor/hooks

infra/db/event.go

@@ -85,6 +85,7 @@ func updateEvent(db *gorm.DB, eventID uuid.UUID, args domain.UpsertEventArgs) (*
 	event := ConvWriteEventParamsToEvent(args)
 	event.ID = eventID
 
+	// Event Save の明示的な呼び出し
 	err := db.Session(&gorm.Session{FullSaveAssociations: true}).
 		Omit("CreatedAt").Save(&event).Error
 	return &event, err

infra/db/group.go

@@ -107,6 +107,8 @@ func createGroup(db *gorm.DB, args domain.UpsertGroupArgs) (*Group, error) {
 func updateGroup(db *gorm.DB, groupID uuid.UUID, args domain.UpsertGroupArgs) (*Group, error) {
 	group := ConvWriteGroupParamsToGroup(args)
 	group.ID = groupID
+	// FullSaveAssociations :true だと何が更新されるか不明瞭でよくない
+	// 特に、 hooks のせいで影響範囲が想定以上に大きくなりうる
 	err := db.Session(&gorm.Session{FullSaveAssociations: true}).
 		Omit("CreatedAt").Save(&group).Error
 	return &group, err

infra/db/hooks.go

@@ -7,6 +7,18 @@ import (
 	"gorm.io/gorm"
 )
 
+/*
+やりたいこと
+
+Domain/ Service で行うべき責務を分離する
+dbでDomainを取り出さないでよくする
+
+データの整合性判定を Service に切り出す
+関連情報の更新をそれぞれの update/create/delete/upsert で明示的に行う
+AfterSave
+
+*/
+
 // BeforeSave is hook
 func (e *Event) BeforeSave(_ *gorm.DB) (err error) {
 	if e.ID == uuid.Nil {
@@ -16,6 +28,7 @@ func (e *Event) BeforeSave(_ *gorm.DB) (err error) {
 		}
 	}
 
+	// event の save のときに Room を設定している
 	if e.RoomID == uuid.Nil {
 		if e.Room.Place != "" {
 			e.Room.Verified = false
@@ -29,6 +42,7 @@ func (e *Event) BeforeSave(_ *gorm.DB) (err error) {
 	}
 
 	// 時間整合性
+	// これは Service でやるべきでは？
 	Devent := ConvEventTodomainEvent(*e)
 	if !Devent.TimeConsistency() {
 		return NewValueError(ErrTimeConsistency, "timeStart", "timeEnd")
@@ -39,6 +53,8 @@ func (e *Event) BeforeSave(_ *gorm.DB) (err error) {
 // BeforeCreate is hook
 func (e *Event) BeforeCreate(tx *gorm.DB) (err error) {
 	// 時間整合性
+	// だから、これは Service でやるべきでは？
+	// イベントの情報が満たすべきことだからワンチャン Domain もある
 	r, err := getRoom(tx.Preload("Events"), e.RoomID)
 	if errors.Is(err, gorm.ErrRecordNotFound) {
 		// 該当する部屋がない場合、部屋時間整合性は調べる必要がない
@@ -49,7 +65,10 @@ func (e *Event) BeforeCreate(tx *gorm.DB) (err error) {
 		return err
 	}
 	e.Room = *r
+	// dbまで下ったのに Domain を取り出すのはあまり良くなさそう
 	Devent := ConvEventTodomainEvent(*e)
+	// これ domain の整合性判定関数なのになんでDBで確認してるんだ？
+	// 少なくとも service でやるべきでは？
 	if !Devent.RoomTimeConsistency() {
 		return NewValueError(ErrTimeConsistency, "timeStart", "timeEnd", "room")
 	}
@@ -85,12 +104,16 @@ func (e *Event) BeforeUpdate(tx *gorm.DB) (err error) {
 	return nil
 }
 
+// dbが正しく動作しているかの確認
+// これはdbの責務なのでここ or db/event で行う
+// preloadも
 func (e *Event) AfterSave(tx *gorm.DB) (err error) {
 	event, err := getEvent(tx.Preload("Admins"), e.ID)
 	if err != nil {
 		return err
 	}
 	Devent := ConvEventTodomainEvent(*event)
+	// admin validation が domain のものなのは正しいのか？
 	if !Devent.AdminsValidation() {
 		return NewValueError(ErrNoAdmins, "admins")
 	}
@@ -102,6 +125,9 @@ func (e *Event) AfterSave(tx *gorm.DB) (err error) {
 	return nil
 }
 
+//  削除するイベントの関連情報も削除する (全て 論理削除)
+//  DBの責務ではあるが、 hooks がやるべきことか？
+//  event_attendees は delete しない (deleted_at フィールドがないため物理削除になるが、行わない)
 func (e *Event) BeforeDelete(tx *gorm.DB) (err error) {
 	// delete current m2m
 	err = tx.Where("event_id = ?", e.ID).Delete(&EventTag{}).Error
@@ -117,6 +143,7 @@ func (e *Event) BeforeDelete(tx *gorm.DB) (err error) {
 }
 
 // BeforeSave is hook
+// CreateEvent のDBの責務なので event が妥当
 func (et *EventTag) BeforeSave(tx *gorm.DB) (err error) {
 	if et.EventID == uuid.Nil {
 		return NewValueError(gorm.ErrRecordNotFound, "eventID")
@@ -144,6 +171,7 @@ func (et *EventTag) BeforeSave(tx *gorm.DB) (err error) {
 	return nil
 }
 
+// 妥当
 func (et *EventTag) BeforeDelete(tx *gorm.DB) (err error) {
 	// タグのIDが空で名前が提供されている場合は、
 	// 名前に応じたタグを削除する
@@ -162,30 +190,33 @@ func (et *EventTag) BeforeDelete(tx *gorm.DB) (err error) {
 }
 
 // BeforeSave is hook
-func (r *Room) BeforeSave(_ *gorm.DB) (err error) {
-	if r.ID != uuid.Nil {
-		return nil
-	}
-	r.ID, err = uuid.NewV4()
-	if err != nil {
-		return err
-	}
+// func (r *Room) BeforeSave(_ *gorm.DB) (err error) {
+// 	if r.ID != uuid.Nil {
+// 		return nil
+// 	}
+//  // createRoomに移動
+// 	r.ID, err = uuid.NewV4()
+// 	if err != nil {
+// 		return err
+// 	}
 
-	// 時間整合性
-	Droom := ConvRoomTodomainRoom(*r)
-	if !Droom.TimeConsistency() {
-		return NewValueError(ErrTimeConsistency, "timeStart", "timeEnd")
-	}
-	return nil
-}
+// 	// 時間整合性
+// 	// Serviceに移動
+// 	Droom := ConvRoomTodomainRoom(*r)
+// 	if !Droom.TimeConsistency() {
+// 		return NewValueError(ErrTimeConsistency, "timeStart", "timeEnd")
+// 	}
+// 	return nil
+// }
+
+// func (r *Room) BeforeUpdate(tx *gorm.DB) (err error) {
+// 	err = tx.Where("room_id", r.ID).Delete(&RoomAdmin{}).Error
+// 	if err != nil {
+// 		return err
+// 	}
+// 	return nil
+// }
 
-func (r *Room) BeforeUpdate(tx *gorm.DB) (err error) {
-	err = tx.Where("room_id", r.ID).Delete(&RoomAdmin{}).Error
-	if err != nil {
-		return err
-	}
-	return nil
-}
 
 func (r *Room) AfterSave(tx *gorm.DB) (err error) {
 	room, err := getRoom(tx.Preload("Admins"), r.ID)
@@ -266,6 +297,7 @@ func (t *Tag) BeforeCreate(_ *gorm.DB) (err error) {
 	return nil
 }
 
+/*
 // BeforeCreate is hook
 func (u *User) BeforeCreate(_ *gorm.DB) (err error) {
 	if u.ID != uuid.Nil {
@@ -277,6 +309,7 @@ func (u *User) BeforeCreate(_ *gorm.DB) (err error) {
 	}
 	return nil
 }
+*/
 
 func (t *Token) BeforeSave(_ *gorm.DB) (err error) {
 	if t.AccessToken != "" {

infra/db/room.go

@@ -74,19 +74,39 @@ func (repo *gormRepository) GetAllRooms(start, end time.Time, excludeEventID uui
 
 func createRoom(db *gorm.DB, args domain.CreateRoomArgs) (*Room, error) {
 	room := ConvCreateRoomParamsToRoom(args)
-	err := db.Create(&room).Error
+	var err error
+	// IDを新規発行
+	room.ID ,err = uuid.NewV4()
+	if err != nil {
+		return nil,err
+	}
+	// 時間整合性は service で確認済み
+	err = db.Create(&room).Error
 	return &room, err
 }
 
 func updateRoom(db *gorm.DB, roomID uuid.UUID, args domain.UpdateRoomArgs) (*Room, error) {
 	room := ConvUpdateRoomParamsToRoom(args)
 	room.ID = roomID
-	err := db.Session(&gorm.Session{FullSaveAssociations: true}).
-		Omit("verified", "CreatedAt").Save(&room).Error
+	if room.ID == uuid.Nil {
+		return nil,ErrRoomUndefined
+	}
+
+	// BeforeSave, BeforeUpdate が発火
+	err := db.Where("room_id", room.ID).Delete(&RoomAdmin{}).Error
+	if err!=nil{
+		return nil,err
+	}
+	// 時間整合性は service で確認済み
+	err = db.Omit("verified", "CreatedAt").Save(&room).Error
+
+	// err := db.Session(&gorm.Session{FullSaveAssociations: true}).
+		// Omit("verified", "CreatedAt").Save(&room).Error
 	return &room, err
 }
 
 func updateRoomVerified(db *gorm.DB, roomID uuid.UUID, verified bool) error {
+	// hooksは発火しない
 	return db.Model(&Room{}).Where("id = ?", roomID).UpdateColumn("verified", verified).Error
 }
 

infra/db/user.go

@@ -118,7 +118,18 @@ func saveUser(db *gorm.DB, user *User) (*User, error) {
 	err := db.Transaction(func(tx *gorm.DB) error {
 		existingUser, err := getUser(tx.Preload("Provider").Preload("Token"), user.ID)
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return tx.Create(&user).Error
+			// BeforeCreate が呼ばれる
+			if user.ID != uuid.Nil  {
+				// 単純にIDが違う場合
+				return ErrRecordNotFound
+			} else {
+				// IDなしで新たに作成する場合
+				user.ID ,err = uuid.NewV4()
+				if err != nil {
+					return err
+				}
+				return tx.Create(&user).Error
+			}
 		}
 		if err != nil {
 			return err
@@ -127,6 +138,8 @@ func saveUser(db *gorm.DB, user *User) (*User, error) {
 		if user.IcalSecret == "" {
 			user.IcalSecret = existingUser.IcalSecret
 		}
+		// 良くない
+		// 何が更新される？
 		return tx.Session(&gorm.Session{FullSaveAssociations: true}).Updates(user).Error
 	})
 

service/errors.go

@@ -9,6 +9,15 @@ import (
 	"github.com/traPtitech/knoQ/infra/traq"
 )
 
+var (
+	ErrInvalidArgs     = errors.New("invalid args")
+	ErrTimeConsistency = errors.New("inconsistent time")
+	ErrExpression      = errors.New("invalid expression")
+	ErrRoomUndefined   = errors.New("invalid room or args")
+	ErrNoAdmins        = errors.New("no admins")
+	ErrDuplicateEntry  = errors.New("duplicate entry")
+)
+
 func handleTraQError(err error) error {
 	if errors.Is(err, traq.ErrUnAuthorized) {
 		return domain.ErrInvalidToken

service/event_impl.go

@@ -50,6 +50,27 @@ func (s *service) UpdateEvent(ctx context.Context, reqID uuid.UUID, eventID uuid
 		WriteEventParams: params,
 		CreatedBy:        reqID,
 	}
+	// RoomIDの存在を確認 RoomがなくPlaceがあれば新たに作成
+
+	// if params.RoomID == uuid.Nil {
+	// 	if params.Place == "" {
+	// 		roomParams := domain.WriteRoomParams {
+	// 			Place: params.Place,
+	// 			TimeStart: params.TimeStart,
+	// 			TimeEnd: params.TimeEnd,
+	// 			Admins: params.Admins,
+	// 		}
+	// 		// UnVerifiedを仮定
+	// 		_,err = s.CreateUnVerifiedRoom(ctx,reqID,roomParams)
+	// 		if err != nil {
+	// 			return nil,err
+	// 		}
+	// 	} else{
+	// 		return nil, ErrRoomUndefined
+	// 	}
+	// }
+
+	// Event Save を使っている
 	event, err := s.GormRepo.UpdateEvent(eventID, p)
 	if err != nil {
 		return nil, defaultErrorHandling(err)

service/room_impl.go

@@ -9,6 +9,10 @@ import (
 )
 
 func (s *service) CreateUnVerifiedRoom(ctx context.Context, reqID uuid.UUID, params domain.WriteRoomParams) (*domain.Room, error) {
+	// 時間整合性の確認
+	if(!params.TimeStart.Before(params.TimeEnd)){
+		return nil,ErrTimeConsistency
+	}
 	p := domain.CreateRoomArgs{
 		WriteRoomParams: params,
 		Verified:        false,
@@ -23,6 +27,10 @@ func (s *service) CreateVerifiedRoom(ctx context.Context, reqID uuid.UUID, param
 	if !s.IsPrivilege(ctx, reqID) {
 		return nil, domain.ErrForbidden
 	}
+	// 時間整合性の確認
+	if(!params.TimeStart.Before(params.TimeEnd)){
+		return nil,ErrTimeConsistency
+	}
 	p := domain.CreateRoomArgs{
 		WriteRoomParams: params,
 		Verified:        true,
@@ -36,12 +44,20 @@ func (s *service) UpdateRoom(ctx context.Context, reqID uuid.UUID, roomID uuid.U
 	if !s.IsRoomAdmins(ctx, reqID, roomID) {
 		return nil, domain.ErrForbidden
 	}
+	// そもそもNilだとIsRoomAdminsで弾かれるはず
+	// if roomID  == uuid.Nil {
+	// 	return nil,ErrRoomUndefined
+	// }
 
 	p := domain.UpdateRoomArgs{
 		WriteRoomParams: params,
 		CreatedBy:       reqID,
 	}
 
+	// 時間整合性の確認
+	if(!params.TimeStart.Before(params.TimeEnd)){
+		return nil,ErrTimeConsistency
+	}
 	r, err := s.GormRepo.UpdateRoom(roomID, p)
 	return r, defaultErrorHandling(err)
 }