Fix/make gameversion unique

[mathsuky]

User description

やったこと

• 同一のgame_id内でのバージョン名の被りを許容しないようにした。
• 既存のバージョン名で重複がある場合は+nをつけることで重複を解消するようなマイグレーションコードを書いた。
• バージョン名がかぶるようなバージョンを作ろうとした場合に400を返すようにした。

テストが落ちているところがあるのでそれを修正したいのですが詰まり気味になってしまったので，一旦DraftでPRを出させていただいて今度の進捗会の際にご相談させていただければと思います。

close #1071

---

PR Type

Bug fix, Tests, Enhancement

---

Description

• ゲームバージョン名が重複しないようにユニーク制約を追加

• 重複する既存データをリネームするマイグレーションを実装

• 重複したバージョン名のエラーハンドリングを追加

• 重複エラーに関するテストケースを追加

---

Changes walkthrough 📝

	Relevant files
Bug fix	game_version.go 重複エラーのハンドリングを追加                                                                                    src/handler/v2/game_version.go 重複したゲームバージョン名に対するエラーハンドリングを追加 +2/-0
Tests	game_version_test.go 重複エラーのテストケースを追加                                                                                    src/handler/v2/game_version_test.go 重複エラーに関するテストケースを追加 +22/-0    v2_game_version_test.go 重複エラーのテストケース修正                                                                                      src/repository/gorm2/v2_game_version_test.go 重複エラーのテストケースを修正 重複エラーの期待結果を更新 +2/-11    game_version_test.go 重複エラーのテストケース追加                                                                                      src/service/v2/game_version_test.go 重複エラーのテストケースを追加 モック関数の期待値を更新 +119/-40
Configuration changes	current.go マイグレーションバージョンの更新                                                                                  src/repository/gorm2/migrate/current.go マイグレーションバージョンをv15に更新 +3/-3
Enhancement	migrate.go 新しいマイグレーションの追加                                                                                      src/repository/gorm2/migrate/migrate.go v15マイグレーションを追加 +1/-0      v15.go v2_game_versionsテーブルのユニーク制約追加                                                        src/repository/gorm2/migrate/v15.go v2_game_versionsテーブルにユニーク制約を追加 重複データをリネームするロジックを実装 +153/-0  game_version.go 重複バージョン名のチェックを追加                                                                                  src/service/v2/game_version.go 重複したバージョン名のチェックロジックを追加 +11/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[codecov]

Codecov Report

Attention: Patch coverage is 38.59649% with 35 lines in your changes missing coverage. Please review.

Project coverage is 51.09%. Comparing base (fde824e) to head (0750ea0).
Report is 25 commits behind head on main.

Files with missing lines	Patch %	Lines
src/repository/gorm2/migrate/v15.go	31.91%	29 Missing and 3 partials ⚠️
src/service/v2/game_version.go	25.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1090      +/-   ##
==========================================
- Coverage   51.16%   51.09%   -0.07%     
==========================================
  Files         122      123       +1     
  Lines       11060    11116      +56     
==========================================
+ Hits         5659     5680      +21     
- Misses       5061     5092      +31     
- Partials      340      344       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 1071 - Fully compliant Fully compliant requirements: Prevent duplicate game version names within the same game ID. Return a 400 error when attempting to create a duplicate version. Handle existing duplicate versions by renaming them (e.g., appending +n). Not compliant requirements:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling Ensure that the new error case for ErrDuplicateGameVersion is correctly handled and does not interfere with other error cases. case errors.Is(err, service.ErrDuplicateGameVersion): return echo.NewHTTPError(http.StatusBadRequest, "duplicate game version") Migration Logic Verify that the migration logic for renaming duplicate game versions and adding the unique constraint is robust and handles edge cases. // v2_game_versionsの(GameID,Name)の組をuniqueに変更し，既存データの重複をリネームして反映する。 func v15() *gormigrate.Migration { return &gormigrate.Migration{ ID: "15", Migrate: func(tx *gorm.DB) error { var gameVersions []gameVersionTable2V15 if err := tx.Order("game_id, name, created_at"). Find(&gameVersions).Error; err != nil { return err } // 同一GameID内でnameの重複をリネーム var currentGameID uuid.UUID tmpMap := make(map[string]int) for _, gameVersion := range gameVersions { // 違うゲームになったらmapを初期化 if currentGameID != gameVersion.GameID { currentGameID = gameVersion.GameID tmpMap = make(map[string]int) } if count, exists := tmpMap[gameVersion.Name]; exists { newName := gameVersion.Name + "+" + strconv.Itoa(count) tmpMap[gameVersion.Name] = count + 1 gameVersion.Name = newName } else { tmpMap[gameVersion.Name] = 1 } if err := tx.Save(&gameVersion).Error; err != nil { return err } } // 複合ユニーク制約を追加 if err := tx.Exec( "ALTER TABLE v2_game_versions ADD CONSTRAINT unique_game_version_per_game UNIQUE (game_id, name)", ).Error; err != nil { return err } if err := tx.AutoMigrate(&gameVersionTable2V15{}); err != nil { return err } return nil }, Rollback: func(tx *gorm.DB) error { // 複合ユニーク制約を削除 if err := tx.Exec( "ALTER TABLE v2_game_versions DROP INDEX unique_game_version_per_game", ).Error; err != nil { return err } // テーブル定義をロールバック if err := tx.Migrator().DropTable(&gameVersionTable2V15{}); err != nil { return err } return nil }, } } Performance Concern The loop to check for duplicate game version names in CreateGameVersion might have performance implications for large datasets. Consider optimizing this logic. _, currentGameVersions, err := gameVersion.gameVersionRepository.GetGameVersions(ctx, gameID, 0, 0, repository.LockTypeNone) if err != nil { return fmt.Errorf("failed to get game versions: %w", err) } for _, currentGameVersion := range currentGameVersions { if currentGameVersion.GetName() == name { return service.ErrDuplicateGameVersion

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
General	Enhance error messages for duplicate game versions by including the conflicting version name ErrDuplicateGameVersionのエラーメッセージが具体的でないため、エラー内容に重複しているバージョン名を含めることで、デバッグやユーザーへのフィードバックを改善してください。 src/handler/v2/game_version.go [187] -return echo.NewHTTPError(http.StatusBadRequest, "duplicate game version") +return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("duplicate game version: %s", gameVersion.Name)) Suggestion importance[1-10]: 9 Why: Including the conflicting version name in the error message significantly improves feedback for debugging and user understanding. The suggestion is precise and directly enhances the error message's clarity.	9
	Add logging for errors encountered during GetGameVersions calls to improve debugging GetGameVersionsの呼び出しでエラーが発生した場合、エラーメッセージが適切にログに記録されていないため、エラー内容をログに記録する処理を追加してください。 src/service/v2/game_version.go [98-100] _, currentGameVersions, err := gameVersion.gameVersionRepository.GetGameVersions(ctx, gameID, 0, 0, repository.LockTypeNone) if err != nil { + log.Printf("Error fetching game versions for gameID %v: %v", gameID, err) return fmt.Errorf("failed to get game versions: %w", err) } Suggestion importance[1-10]: 8 Why: Adding logging for errors encountered during GetGameVersions calls improves traceability and debugging, especially in production environments. The suggestion is accurate and directly enhances the error handling mechanism.	8
	Log renamed duplicate game versions to ensure traceability during migration 複合ユニーク制約を追加する前に、既存データの重複をリネームする処理が適切に動作しない場合に備えて、リネーム処理の結果をログに記録するようにしてください。 src/repository/gorm2/migrate/v15.go [121-122] if err := tx.Save(&gameVersion).Error; err != nil { + log.Printf("Failed to rename duplicate game version: %v", gameVersion) return err } Suggestion importance[1-10]: 7 Why: Logging renamed duplicate game versions provides better traceability during migration, which is crucial for debugging and auditing. The suggestion is relevant and improves the robustness of the migration process.	7
	Add logging for errors encountered during AutoMigrate to improve traceability AutoMigrateの呼び出しでエラーが発生した場合、エラー内容が適切にログに記録されていないため、エラー内容をログに記録する処理を追加してください。 src/repository/gorm2/migrate/v15.go [133-134] if err := tx.AutoMigrate(&gameVersionTable2V15{}); err != nil { + log.Printf("Error during AutoMigrate for gameVersionTable2V15: %v", err) return err } Suggestion importance[1-10]: 7 Why: Logging errors during AutoMigrate enhances traceability and aids in identifying issues during schema migrations. The suggestion is valid and improves the error handling mechanism.	7

[mathsuky]

@ikura-hamu
基本的な機能と，それに伴うテストについては実装し終えましたが，codecovというものに怒られています。自分はこれを使ったことがなくあまりよくわかっていないのですが，おそらくテストが不十分だということで怒られているのだろうと解釈しています。
しかし，codecovによって指摘されているのは本筋のロジックとは関係ないように思えるエラーハンドリングとマイグレーションファイルに対するテスト不足であり，これらに対してどのような解決策を取れば良いかわからないので，教えていただきたです:pray:

[ikura-hamu]

(まだ中身見てないです)
codecovはテストがコードの何%をカバーしてるか教えてくれるやつです。ちょっと下がると怒られますが、気にしなくて大丈夫

[ikura-hamu]

実装ありがとう、そこそこ難しかったと思いますが、とてもきれいに書かれていました。

repositoryのCreateGameVersionで、ユニーク制約に違反していたらrepository.ErrDuplicatedKeyを返すようにしてください。game_genre.goのUpdateGameGenreが分かりやすいと思います。

[ikura-hamu]

ユニークインデックスを貼るところの記法をindex:idx_game_id_name,uniqueとuniqueIndex:idx_game_id_nameのどちらかで固定してほしいです。

[ikura-hamu]

AutoMigrateを実行すると、定義した構造体に合うようにスキーマやインデックスが変更されるので、ここのExecは不要です

[ikura-hamu]

DropTableしちゃうとテーブルが消えて終わってしまうので、上のユニーク制約削除だけで十分です。

[ikura-hamu]

ここでゲームのバージョンを全て取ってきてしまうと、例えば1つのゲームに10000個のバージョンがあったときにめっちゃ遅くなってしまうので、

1. gameIDと名前を指定して取得するメソッドをrepositoryの方で作って使う
2. 下の方のCreateGameVersionで返って来るエラーがrepository.ErrDuplicatedUniqueKeyかどうかを判定する

のどちらかでやってほしいです。

[ikura-hamu]

serviceのinterfaceに書かれたCreateGameVersionのところに、ErrDuplicateGameVersionを返すことがあるとコメントで書いておいてください。

[mathsuky]

レビューありがとうございました！少し遅くなってしまいましたが，ご指摘いただいた点について修正いたしましたので，際レビューお願いいたします

[ikura-hamu]

ごめん、前回のコメントの仕方が微妙だったかも。

repositoryのCreateGameVersionで、ユニーク制約に違反していたらrepository.ErrDuplicatedKeyを返すようにしてください。game_genre.goのUpdateGameGenreが分かりやすいと思います

repository/gorm2の実装でこれの修正もお願いします。

[ikura-hamu]

ここがコメントアウトなのは何か意図がありますか？なければ消しちゃってください

[mathsuky]

すみません，消し忘れです:pray:

[mathsuky]

ご指摘ありがとうございます！修正いたしましたので，ご確認お願いいたします。

[ikura-hamu]

ありがとう、お疲れ様でした