[Feat/replace enum with type entity] - Enum 을 Type Entity로 교체#67
[Feat/replace enum with type entity] - Enum 을 Type Entity로 교체#67
Conversation
|
""" WalkthroughThis change refactors the handling of egg types and their associations throughout the codebase, migrating from enum-based representations to fully normalized JPA entity models and database-driven logic. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RewardService
participant EggGrantService
participant EggTypeRepository
participant ItemRepository
User->>RewardService: claimReward(command)
RewardService->>EggGrantService: grantRandomEggToUser(user)
EggGrantService->>EggTypeRepository: findEggTypeByRequiredDistanceInMetersLessThanEqual(distance)
EggTypeRepository-->>EggGrantService: List<EggType>
EggGrantService->>ItemRepository: findByEggType(eggType)
ItemRepository-->>EggGrantService: Optional<Egg>
EggGrantService-->>RewardService: Egg
RewardService-->>User: RewardResponse(isRewarded, eggCode, eggTypeName, lovePointAmount)
sequenceDiagram
participant HatchClient
participant EggTypeRepository
participant RunimoDefinitionRepository
participant HatchContext
HatchClient->>EggTypeRepository: findEggTypeByLevelLessThan(eggLevel)
EggTypeRepository-->>HatchClient: List<EggType>
HatchClient->>RunimoDefinitionRepository: findIdInEggTypes(eggTypes)
RunimoDefinitionRepository-->>HatchClient: List<Long> (runimo IDs)
HatchClient->>HatchContext: setHatchContext(runimoIdPool)
HatchContext->>HatchClient: execute() (returns runimoId)
HatchClient->>RunimoDefinitionRepository: findById(runimoId)
RunimoDefinitionRepository-->>HatchClient: RunimoDefinition
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
src/main/java/org/runimo/runimo/item/service/EggFactory.java (1)
16-16: Consider using a named constant from a configuration class instead of a hard-coded IDWhile defining
GREETING_EGG_IDas a constant is better than using a magic number directly, hard-coded IDs can be problematic if database records change. Consider:
- Defining this constant in a central configuration class for all special egg IDs
- Using a named query or finder method that retrieves the egg by a more stable attribute like code or name
- Storing this ID in an application property for easier configuration management
src/main/java/org/runimo/runimo/user/repository/UserItemRepository.java (2)
31-34: Consider adding validation for eggCode valuesMoving from enum to string-based codes loses compile-time type safety. Consider adding validation to ensure the eggCode parameter contains valid values, perhaps through a validation service or using constants for common egg codes.
31-34: Evaluate the performance impact of the join queryThe new query joins with the Egg table, which may have performance implications compared to directly filtering by an enum value. For high-frequency calls, consider adding an index on the egg's itemCode field if not already present.
src/main/java/org/runimo/runimo/rewards/service/dto/RewardResponse.java (1)
5-5: Consider using primitive boolean instead of Boolean wrapperUsing
Booleanwrapper type rather than primitivebooleanallows for null values, which could lead to NullPointerExceptions. Unless null is a meaningful state for this field, consider using the primitive type instead.- Boolean isRewarded, + boolean isRewarded,src/test/java/org/runimo/runimo/item/service/EggEnumRandomGeneratorTest.java (1)
9-9: Class name seems inconsistent with the migration direction.While the code changes correctly reflect the move from enum to entity, the class name change to include "Enum" seems contrary to the migration direction away from enums. Consider renaming to better reflect the entity-based approach.
-class EggEnumRandomGeneratorTest { +class EggTypeRandomGeneratorTest {src/main/java/org/runimo/runimo/runimo/domain/RunimoDefinition.java (1)
33-36: Good refactoring from enum to entity relationshipThe change from what was likely an enum-based field to a proper
@OneToOneJPA relationship with theEggTypeentity is a good improvement for database normalization and flexibility.However, consider adding
FetchType.LAZYto the@OneToOneannotation to prevent unnecessary eager loading:- @OneToOne + @OneToOne(fetch = FetchType.LAZY) @JoinColumn(name = "egg_type_id", nullable = false) private EggType type;src/main/java/org/runimo/runimo/rewards/service/RewardService.java (1)
38-40: Updated reward response construction to use entity propertiesThe changes to the
RewardResponseconstruction appropriately reflect the conversion from enum-based to entity-basedEggType, including:
- Using a boolean flag to indicate if a reward was given
- Using the item code from the egg
- Using the name from the
EggTypeentity instead of the enum valueHowever, there's a potential null pointer risk if
getEggType()returns null. Consider adding a null check:- return new RewardResponse(!grantedEgg.isEmpty(), grantedEgg.getItemCode(), - grantedEgg.getEggType().getName(), + return new RewardResponse(!grantedEgg.isEmpty(), grantedEgg.getItemCode(), + grantedEgg.isEmpty() ? "EMPTY" : grantedEgg.getEggType().getName(), grantedLoveAmount);src/main/java/org/runimo/runimo/item/domain/Egg.java (1)
38-40: Consider enhancing isEmpty() implementationThe current isEmpty() method checks for object identity (this == EMPTY), which works as long as the EMPTY constant is properly managed. For more robustness, consider adding null checks or other criteria that would define an empty egg.
public boolean isEmpty() { - return this == EMPTY; + return this == EMPTY || (this.getItemCode() != null && this.getItemCode().equals(EMPTYFIELD)); }src/main/java/org/runimo/runimo/hatch/service/HatchContext.java (1)
8-18: Type change from RunimoType enum to Long ID is appropriate, but add null checksThe transition from using RunimoType enums to Long IDs aligns well with the PR objective. However, consider adding null and empty checks to prevent potential NullPointerExceptions or IndexOutOfBoundsExceptions.
private List<Long> runimoPool; private HatchRandomStrategy hatchRandomStrategy; public Long execute() { + if (runimoPool == null || runimoPool.isEmpty()) { + return null; // Or throw a custom exception + } int chosenIdx = hatchRandomStrategy.generateNumber(); return runimoPool.get(chosenIdx); } public void setHatchContext(List<Long> runimoIdPool) { + if (runimoIdPool == null) { + runimoIdPool = List.of(); // Or throw an exception + } this.runimoPool = runimoIdPool; }src/main/java/org/runimo/runimo/item/repository/EggTypeRepository.java (1)
13-15: Rename finder methods to conform to Spring‑Data naming conventions
findEggTypeByRequiredDistanceInMetersLessThanEqualreads redundantly.
The canonical form isfindByRequiredDistanceInMetersLessThanEqual(same forLevel). Aligning with the convention:- List<EggType> findEggTypeByRequiredDistanceInMetersLessThanEqual(Long totalDistanceInMeters); - List<EggType> findEggTypeByLevelLessThan(Integer level); + List<EggType> findByRequiredDistanceInMetersLessThanEqual(Long totalDistanceInMeters); + List<EggType> findByLevelLessThan(Integer level);This keeps repository APIs predictable and self‑documenting.
src/main/java/org/runimo/runimo/runimo/service/usecase/RunimoUsecaseImpl.java (1)
67-71: Variable name does not reflect its content
runimoTypeInfosactually storesRunimoTypeSimpleModellists, notRunimoTypeInfo. ConsiderrunimoTypeModelsByEggIdto avoid confusion.src/main/java/org/runimo/runimo/item/domain/EggType.java (2)
22-29: Consider adding uniqueness constraints on business‑key columns
nameand especiallycodelook like natural identifiers. To enforce data integrity at the DB level:- @Column(name = "code", nullable = false) + @Column(name = "code", nullable = false, unique = true, length = 50)Similarly add
unique = true(and perhaps a sensiblelength) toname.
35-41: Minor: reuse validation utility to reduce duplicationYou repeat “value must be ≥ 0” checks. Extracting a small
requireNonNegative(long value, String fieldName)utility keeps entity constructors concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
src/main/java/org/runimo/runimo/hatch/controller/HatchController.java(1 hunks)src/main/java/org/runimo/runimo/hatch/service/HatchClient.java(3 hunks)src/main/java/org/runimo/runimo/hatch/service/HatchContext.java(1 hunks)src/main/java/org/runimo/runimo/hatch/service/strategy/EqualRandom.java(1 hunks)src/main/java/org/runimo/runimo/hatch/service/usecase/HatchUsecaseImpl.java(1 hunks)src/main/java/org/runimo/runimo/item/domain/Egg.java(3 hunks)src/main/java/org/runimo/runimo/item/domain/EggType.java(1 hunks)src/main/java/org/runimo/runimo/item/repository/EggTypeRepository.java(1 hunks)src/main/java/org/runimo/runimo/item/repository/ItemRepository.java(1 hunks)src/main/java/org/runimo/runimo/item/service/EggFactory.java(1 hunks)src/main/java/org/runimo/runimo/rewards/service/RewardService.java(1 hunks)src/main/java/org/runimo/runimo/rewards/service/dto/RewardResponse.java(1 hunks)src/main/java/org/runimo/runimo/rewards/service/eggs/EggGrantService.java(3 hunks)src/main/java/org/runimo/runimo/runimo/domain/RunimoDefinition.java(2 hunks)src/main/java/org/runimo/runimo/runimo/repository/RunimoDefinitionRepository.java(2 hunks)src/main/java/org/runimo/runimo/runimo/service/dto/RunimoTypeSimpleModel.java(1 hunks)src/main/java/org/runimo/runimo/runimo/service/dto/response/RunimoTypeGroup.java(1 hunks)src/main/java/org/runimo/runimo/runimo/service/dto/response/RunimoTypeInfo.java(2 hunks)src/main/java/org/runimo/runimo/runimo/service/usecase/RunimoUsecaseImpl.java(3 hunks)src/main/java/org/runimo/runimo/user/repository/UserItemRepository.java(1 hunks)src/main/java/org/runimo/runimo/user/service/UserItemFinder.java(1 hunks)src/main/resources/sql/schema.sql(5 hunks)src/test/java/org/runimo/runimo/hatch/controller/HatchControllerTest.java(1 hunks)src/test/java/org/runimo/runimo/item/service/EggEnumRandomGeneratorTest.java(1 hunks)src/test/java/org/runimo/runimo/rewards/RewardTest.java(1 hunks)src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java(2 hunks)src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java(4 hunks)src/test/java/org/runimo/runimo/runimo/controller/RunimoControllerTest.java(1 hunks)src/test/resources/sql/egg_data.sql(1 hunks)src/test/resources/sql/get_my_runimo_list_test_data.sql(0 hunks)src/test/resources/sql/hatch_test_data.sql(0 hunks)src/test/resources/sql/incubating_egg_test_data.sql(1 hunks)src/test/resources/sql/main_view_data.sql(2 hunks)src/test/resources/sql/schema.sql(5 hunks)src/test/resources/sql/set_main_runimo_test_data.sql(0 hunks)src/test/resources/sql/user_item_test_data.sql(0 hunks)src/test/resources/sql/user_mypage_test_data.sql(0 hunks)
💤 Files with no reviewable changes (5)
- src/test/resources/sql/user_item_test_data.sql
- src/test/resources/sql/get_my_runimo_list_test_data.sql
- src/test/resources/sql/set_main_runimo_test_data.sql
- src/test/resources/sql/user_mypage_test_data.sql
- src/test/resources/sql/hatch_test_data.sql
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/org/runimo/runimo/item/domain/EggType.java (3)
src/main/java/org/runimo/runimo/item/domain/Egg.java (1)
Entity(16-41)src/main/java/org/runimo/runimo/item/domain/Item.java (1)
Table(17-49)src/main/java/org/runimo/runimo/common/GlobalConsts.java (1)
NoArgsConstructor(8-21)
src/main/java/org/runimo/runimo/hatch/service/HatchClient.java (1)
src/main/java/org/runimo/runimo/hatch/exception/HatchException.java (1)
HatchException(6-19)
🔇 Additional comments (47)
src/test/java/org/runimo/runimo/hatch/controller/HatchControllerTest.java (1)
69-69: LGTM: Minor formatting adjustment.This whitespace change doesn't affect functionality and appears to be a minor code style adjustment.
src/main/java/org/runimo/runimo/hatch/controller/HatchController.java (1)
10-10: LGTM: Import reordering.Import statement reordering doesn't affect functionality.
src/main/java/org/runimo/runimo/hatch/service/usecase/HatchUsecaseImpl.java (2)
7-7: LGTM: Import reordering.Import statement reordering doesn't affect functionality.
30-60:Details
✅ Verification successful
Verify implementation with new EggType entity.
The execute method appears unchanged despite the conversion from enum to entity types. Verify that
egg.getEggType()on line 37 correctly returns the new EggType entity and thatHatchClient.getRandomRunimoDefinition()properly handles it.
🏁 Script executed:
#!/bin/bash # Search for the definition of EggType and how it's handled in HatchClient echo "Checking Egg class for getEggType method:" rg -A 5 "getEggType" --glob "*/item/domain/Egg.java" echo -e "\nChecking HatchClient for getRandomRunimoDefinition method:" rg -A 10 "getRandomRunimoDefinition" --glob "*/hatch/service/HatchClient.java"Length of output: 655
🏁 Script executed:
#!/bin/bash set -e echo "Locating Egg.java files:" find . -type f -name Egg.java echo -e "\nInspecting getEggType usage in Java sources:" rg -A5 "getEggType" --glob "*.java" || true echo -e "\nLocating EggType.java files:" find . -type f -name EggType.java || echo "EggType.java not found" echo -e "\nInspecting EggType class definitions:" rg -A5 "class EggType" --glob "*.java" || true echo -e "\nLocating HatchClient.java files:" find . -type f -name HatchClient.java || echo "HatchClient.java not found" echo -e "\nInspecting getRandomRunimoDefinition usage in Java sources:" rg -A5 "getRandomRunimoDefinition" --glob "*.java" || trueLength of output: 4881
🏁 Script executed:
#!/bin/bash set -e echo "=== Egg.java (getEggType) ===" rg -A3 -B3 "getEggType" --glob "src/main/java/**/*.java" echo -e "\n=== EggType.java (fields & methods) ===" sed -n '1,200p' src/main/java/org/runimo/runimo/item/domain/EggType.java echo -e "\n=== HatchClient.java (getRandomRunimoDefinition signature) ===" rg -A5 -B3 "getRandomRunimoDefinition" --glob "src/main/java/**/*.java"Length of output: 5842
🏁 Script executed:
#!/bin/bash set -e echo "=== Displaying Egg.java (first 200 lines) ===" sed -n '1,200p' src/main/java/org/runimo/runimo/item/domain/Egg.javaLength of output: 1458
Implementation correctly uses the EggType entity
I’ve confirmed that
Egg#getEggType()(generated by Lombok’s@Getter) returns the JPA‐mapped EggType entity, andHatchClient.getRandomRunimoDefinition(EggType eggType)now accepts the entity (not an enum), uses its properties (e.g.level) to build the pool, and resolves the definition viaRunimoDefinitionRepository.No changes are needed here.
src/test/resources/sql/incubating_egg_test_data.sql (1)
21-22:Details
✅ Verification successful
LGTM: Updated column from enum to entity ID reference.
Column name change from
egg_typetoegg_type_idand value change from string to integer correctly reflects the migration from enum to entity-based approach.
🏁 Script executed:
#!/bin/bash # Verify if egg_type table is populated in test data echo "Checking for egg_type table population in SQL files:" rg -l "INSERT INTO egg_type" --glob "*/test/resources/sql/*.sql" echo -e "\nChecking egg_type table structure:" fd "egg_type.java" -iLength of output: 358
egg_type test data population confirmed
The test SQL files (schema.sqlandegg_data.sqlinsrc/test/resources/sql/) include the necessaryINSERT INTO egg_typestatements, so the newegg_type_id = 1reference will resolve correctly. LGTM.src/test/java/org/runimo/runimo/runimo/controller/RunimoControllerTest.java (2)
92-92: LGTM: Minor formatting adjustment.This whitespace change doesn't affect functionality and appears to be a minor code style adjustment.
99-102: LGTM: Added assertions for new egg unlock distance field.The added assertions correctly verify the new
egg_required_distance_in_metersfield in the response payload, ensuring that:
- The first runimo group (마당/Madang) requires 0 meters (immediately available)
- The second runimo group (숲/Forest) requires 30,000 meters (approx. 30km)
This change aligns with the PR objective of including running distance needed to unlock each runimo group.
src/test/java/org/runimo/runimo/rewards/RewardTest.java (1)
103-107: Method correctly updated to use code-based lookup instead of type-based lookupThe change from
findEggByUserIdAndEggTypetofindEggByUserIdAndEggCodeproperly aligns with the PR's objective of replacing enum types with entity types. This update ensures the test stays consistent with the domain model changes.src/main/java/org/runimo/runimo/runimo/service/dto/response/RunimoTypeGroup.java (1)
10-11: Good addition of distance requirement informationThe new
eggRequiredDistanceInMetersfield with proper Swagger documentation enhances the API response with the distance needed to unlock each Runimo group, as mentioned in the PR objectives.src/main/java/org/runimo/runimo/hatch/service/strategy/EqualRandom.java (1)
9-9: Good practice making size field finalMaking the
sizefield final is a good practice as it ensures immutability after initialization, enhancing code reliability.src/main/java/org/runimo/runimo/runimo/service/dto/response/RunimoTypeInfo.java (1)
21-25: Well-implemented static factory method for conversionThe addition of the
frommethod provides a clean way to convert between model and DTO objects, using Java streams for efficient mapping. This supports the PR objective of modifying the mapping logic.src/main/java/org/runimo/runimo/item/service/EggFactory.java (2)
20-23: Verify that GREETING_EGG_ID will consistently map to the correct eggThe change from finding by EggType to finding by ID aligns with the entity-based approach. However, ensure that ID 1L will consistently map to the greeting egg across all environments and database migrations.
Consider adding a comment in the code explaining that ID 1L corresponds to the greeting egg type to improve code maintainability.
26-30:Details
✅ Verification successful
Confirm that createEggRandomly still works with EggType as an entity
This method still uses
EggTypeobjects. Ensure thateggTypeRandomGenerator.generateRandomEggTypeanditemFinder.findEggByEggTypehave been updated to work with EggType as an entity rather than an enum.
🏁 Script executed:
#!/bin/bash # Check how EggTypeRandomGenerator and ItemFinder use EggType rg -A 3 "EggTypeRandomGenerator|generateRandomEggType" src/ rg -A 3 "findEggByEggType" src/Length of output: 3841
🏁 Script executed:
#!/bin/bash # Display ItemFinder and EggTypeRandomGenerator implementations sed -n '1,200p' src/main/java/org/runimo/runimo/item/service/ItemFinder.java echo -e "\n---\n" sed -n '1,200p' src/main/java/org/runimo/runimo/item/service/EggTypeRandomGenerator.javaLength of output: 1906
🏁 Script executed:
#!/bin/bash # Inspect EggType entity definition sed -n '1,200p' src/main/java/org/runimo/runimo/item/domain/EggType.java echo -e "\n---\n" # Inspect repository method signature for findByEggType sed -n '1,200p' src/main/java/org/runimo/runimo/item/repository/ItemRepository.javaLength of output: 2578
🏁 Script executed:
#!/bin/bash # Inspect BaseEntity to see equals/hashCode implementation sed -n '1,200p' src/main/java/org/runimo/runimo/common/BaseEntity.javaLength of output: 1029
Confirmed:
createEggRandomlyworks withEggTypeas a JPA entity
I’ve verified thatEggTypeRandomGenerator.generateRandomEggTypereturns domainEggTypeinstances (not enums) and thatItemFinder.findEggByEggTypebinds the entity in its JPA query (where e.eggType = :eggType), so the method correctly handlesEggTypeas an entity.src/main/java/org/runimo/runimo/item/repository/ItemRepository.java (1)
18-19: LGTM: Improved parameter naming consistencyGood update to make the query parameter name
:eggTypematch the method parametereggType. This improves code readability and maintains consistent naming conventions.src/main/java/org/runimo/runimo/user/repository/UserItemRepository.java (1)
31-34:Details
✅ Verification successful
Verify that all callers of the previous findByUserIdAndEggType method have been updated
The introduction of
findByUserIdAndEggCodeto replace what was likelyfindByUserIdAndEggType(using EggType enum) is a good approach for the entity conversion. However, ensure all code that previously called the removed method has been updated to use this new method.
🏁 Script executed:
#!/bin/bash # Check for any remaining references to findByUserIdAndEggType rg "findByUserIdAndEggType" src/Length of output: 32
No remaining references to
findByUserIdAndEggType
- Verified via
rg "findByUserIdAndEggType" src/—no occurrences found.All callers have been updated to use the new
findByUserIdAndEggCodemethod.src/main/java/org/runimo/runimo/rewards/service/dto/RewardResponse.java (2)
7-7: LGTM: Changed eggType from enum to StringGood change converting the
eggTypefield from EggType enum to String. This properly decouples the DTO from the domain model, which is especially important now that EggType is an entity.
4-9:Details
✅ Verification successful
Verify all constructors of RewardResponse have been updated
Adding a new field to the record requires updates to all places where RewardResponse is constructed. Ensure that all service methods that create RewardResponse objects have been updated to provide the new isRewarded field.
🏁 Script executed:
#!/bin/bash # Find all places where RewardResponse is constructed rg -A 2 -B 2 "new RewardResponse\(" src/Length of output: 681
All
RewardResponseconstructor calls include the newisRewardedfieldThe only usage of
new RewardResponse(...)is in:
src/main/java/org/runimo/runimo/rewards/service/RewardService.java
→return new RewardResponse(!grantedEgg.isEmpty(), grantedEgg.getItemCode(), grantedEgg.getEggType().getName(), grantedLoveAmount);No other constructor invocations found—no further updates required.
src/main/java/org/runimo/runimo/rewards/service/eggs/EggGrantService.java (2)
7-7: Dependencies added correctly for the new EggTypeRepository.The import and dependency injection for EggTypeRepository are correctly added to support the transition from enum-based to entity-based egg types.
Also applies to: 23-23
36-37: Repository query replaces static method call appropriately.The change from
EggType.getUnLockedEggTypesstatic method to a repository query is aligned with the migration from enums to entities. The repository method name clearly indicates its purpose.src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java (2)
187-189: Test assertions updated for new response field.The test assertions have been correctly updated to validate the new
is_rewardedfield and the change from a null value to the "EMPTY" string for the egg_type field.
263-263: New assertion validates isRewarded field.The added assertion correctly validates that when a user doesn't meet the distance requirement, the
is_rewardedfield is set to false, which aligns with the new response field requirements.src/main/java/org/runimo/runimo/user/service/UserItemFinder.java (1)
23-25: Method signature updated to use string code instead of enum type.The method has been appropriately updated to use a string egg code rather than the EggType enum, which aligns with the migration from enum to entity-based approach. The method name clearly reflects its updated purpose.
src/test/java/org/runimo/runimo/item/service/EggEnumRandomGeneratorTest.java (2)
17-30: Test data updated correctly for entity-based approach.The test now properly creates EggType entities with appropriate attributes instead of relying on enum values. This correctly supports testing the random generator with the new entity model.
32-32: Method call updated for entity-based input.The method call has been updated to work with the new entity-based approach, which is good. The test still validates that the random generator works correctly with the new EggType entities.
src/test/resources/sql/egg_data.sql (1)
2-13: Proper test data setup for the new entity modelThe SQL has been properly updated to support the new entity relationship model with:
- Truncating the new egg_type table
- Adding inserts for the egg types with appropriate attributes
- Updating the item table inserts to use foreign key references
This change aligns well with the entity-relationship refactoring.
src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java (4)
77-82: Updated test mock to use entity factory methodThe test correctly uses
EggType.of()factory method to create an EggType entity instead of using an enum constant, which aligns with the domain model changes.
86-87: Updated assertions for new response structureThe test properly verifies the new
isRewardedboolean flag and compares the egg type string name instead of an enum constant.
101-106: Consistent use of EggType factory methodThe code consistently uses the
EggType.of()factory method to create test data, which is good for maintainability.
124-126: Updated negative case assertionTest for the negative case has been correctly updated to check the
isRewardedflag and empty egg properties.src/test/resources/sql/main_view_data.sql (2)
14-18: Enum to Entity transition looks good for the item tableThe change from string-based egg types to numeric foreign key references (egg_type_id) aligns well with the PR objective of replacing enums with entity types. The item table now properly references egg type IDs 1 and 2 instead of string values.
21-30: Runimo definitions properly expanded with egg type referencesThe runimo_definition table now correctly uses egg_type_id foreign key references instead of string-based egg types. The expansion from 4 to 8 entries provides good coverage for the different egg types (4 for egg_type_id 1 and 4 for egg_type_id 2).
src/main/java/org/runimo/runimo/item/domain/Egg.java (1)
24-26:Details
✅ Verification successful
JPA OneToOne relationship properly configured with lazy loading
The one-to-one relationship with EggType is correctly set up with lazy loading, which is good for performance. However, be cautious with the lazy fetch type to avoid potential LazyInitializationExceptions if the eggType is accessed outside of a transactional context.
Run this script to check if there are potential places where eggType might be accessed outside a transaction:
🏁 Script executed:
#!/bin/bash # Search for places where eggType is accessed rg "\.getEggType\(\)" --type java src/main -C 2 # Search for @Transactional annotations rg "@Transactional" --type java src/mainLength of output: 5930
No LazyInitializationException risk detected for eggType access
All occurrences of
getEggType()are invoked inside methods annotated with@Transactional(including@Transactional(readOnly = true)) in both RewardService.java and HatchUsecaseImpl.java, ensuring the persistence context remains open for lazy loading. No further changes are required.src/main/java/org/runimo/runimo/runimo/repository/RunimoDefinitionRepository.java (1)
15-19: New query method for finding runimo IDs by egg types looks goodThe new JPQL query correctly filters runimo definitions by egg types, returning only the IDs. This method will be useful for performance when only IDs are needed.
src/main/resources/sql/schema.sql (7)
3-3: Good addition of DROP TABLE statement.Adding explicit DROP TABLE statement for the new egg_type table ensures clean schema migrations and prevents potential conflicts when rerunning scripts.
119-119: Properly normalized database structure.Replacing the string-based enum column with a foreign key reference to the new egg_type table is a good normalization practice. This improves data integrity and flexibility.
126-137: Well-structured egg_type entity table.The new egg_type table provides a robust structure with all necessary fields:
- Primary key and identification fields (id, name, code)
- Domain-specific fields (required_distance_in_meters, level)
- Audit timestamps for proper tracking
This change allows for more flexible egg type management without code changes.
182-182: Consistent foreign key implementation.The foreign key in runimo_definition properly links to the egg_type table, maintaining the relationship between these entities.
213-222: Correctly updated runimo_definition data inserts.The inserts have been properly updated to reference egg_type_id instead of the previous string-based egg type. The data maintains the same logical groupings (runimos 1-4 belong to egg type 1, and 5-8 belong to egg type 2).
224-229: Consistent item data updates with egg type references.The item data inserts have been correctly updated to use foreign key references to the egg_type table (egg_type_id) instead of string enum values, maintaining data integrity.
208-210:Details
❓ Verification inconclusive
Verify foreign key constraint implementation.
While the data structure is properly normalized, I notice that there are no explicit foreign key constraints defined between
item.egg_type_id/runimo_definition.egg_type_idandegg_type.id. This appears intentional based on the PR description mentioning "without applying a foreign key constraint", but consider whether this omission might impact data integrity.
🏁 Script executed:
#!/bin/bash # Check if foreign key constraints exist between tables mysql -e "SELECT TABLE_NAME, COLUMN_NAME, CONSTRAINT_NAME, REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE REFERENCED_TABLE_NAME = 'egg_type';" || echo "No foreign key constraints found for egg_type table"Length of output: 405
🏁 Script executed:
#!/bin/bash echo "Searching for egg_type_id in schema.sql" rg -n "egg_type_id" src/main/resources/sql/schema.sql || echo "No egg_type_id occurrences" echo "Searching for REFERENCES egg_type in schema.sql" rg -n "REFERENCES egg_type" src/main/resources/sql/schema.sql || echo "No REFERENCES egg_type occurrences" echo "Searching for foreign key declarations in schema.sql" rg -n "foreign key" src/main/resources/sql/schema.sql || echo "No foreign key declarations"Length of output: 901
No foreign key constraints found for egg_type relations
I confirmed that
egg_type_idappears in bothrunimo_definition(line 182) anditem(line 119) but there are noREFERENCES egg_typeorFOREIGN KEYdeclarations inschema.sql. Please verify that omitting these constraints is intentional and won’t compromise data integrity. If you intend to enforce referential integrity at the database level, consider adding:ALTER TABLE runimo_definition ADD CONSTRAINT fk_runimo_egg_type FOREIGN KEY (egg_type_id) REFERENCES egg_type(id); ALTER TABLE item ADD CONSTRAINT fk_item_egg_type FOREIGN KEY (egg_type_id) REFERENCES egg_type(id);src/test/resources/sql/schema.sql (1)
3-3: Consistent schema definition across environments.The test schema has been updated with identical changes to the production schema, ensuring consistency between environments. This includes:
- Adding the egg_type table with the same structure
- Updating foreign key references in item and runimo_definition tables
- Updating data inserts to use the new relation structure
This maintains proper test environment fidelity to production.
Also applies to: 119-119, 126-137, 182-182, 208-229
src/main/java/org/runimo/runimo/runimo/service/dto/RunimoTypeSimpleModel.java (3)
3-8: Appropriate use of Lombok annotations.The class now uses Lombok annotations to reduce boilerplate code while maintaining encapsulation. The protected no-args constructor ensures proper JPA compatibility while preventing direct instantiation without parameters.
9-15: Properly refactored model to use entity references.The model has been correctly updated to reference egg types by ID (eggTypeId: Long) rather than by string name, aligning with the database normalization changes. This provides more efficient and type-safe references.
17-24: Complete constructor implementation.The all-args constructor properly initializes all fields, maintaining the class's functionality after the conversion from what appears to have been a record type previously.
src/main/java/org/runimo/runimo/hatch/service/HatchClient.java (3)
10-11: Proper repository dependency injection.Added dependency on EggTypeRepository to support the new entity-based approach. This allows fetching egg types by level criteria for the hatching logic.
Also applies to: 20-20
26-26: Improved runimo selection using database-driven approach.The hatching logic has been refactored to use database IDs instead of in-memory objects:
- Now works with a list of runimo IDs (Long) instead of type objects
- Uses direct database lookups by ID, which is more efficient
- Properly handles errors with appropriate exception throwing
This approach is more scalable and maintainable than the previous enum-based implementation.
Also applies to: 36-39
43-46: Well-designed database query pattern for egg type filtering.The method now:
- Finds all egg types with levels lower than the current egg type's level
- Retrieves all runimo IDs associated with those egg types
This database-driven approach is more flexible and maintainable than the previous enum-based logic, allowing for dynamic egg type and runimo management without code changes.
src/main/java/org/runimo/runimo/runimo/repository/RunimoDefinitionRepository.java
Show resolved
Hide resolved
src/main/java/org/runimo/runimo/item/repository/EggTypeRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/org/runimo/runimo/runimo/service/usecase/RunimoUsecaseImpl.java
Show resolved
Hide resolved
src/main/java/org/runimo/runimo/runimo/service/usecase/RunimoUsecaseImpl.java
Show resolved
Hide resolved
jeeheaG
left a comment
jeeheaG
left a comment
There was a problem hiding this comment.
고생하셨습니다!
우려되는 부분 한가지만 적었어요
RunimoType enum 도 차차 탈enum 해보아요..~
2 participants
작업내역
Enum to Enitity
EggType은Egg,RunimoDefinition에 1:N 대응됩니다.@ManyToOne사용, FK 미적용기존 로직에서 Enum 제거
Enum이 활용되는 패키지는 다음과 같았습니다.
hatch,item,rewardrunimo해당 패키지에서 Enum을 사용하던 부분을
EggType또는EggType의 PK를 사용하도록 변경했습니다.러니모 그룹 리스트 반환 로직 수정
보상 응답 필드 추가
isRewarded필드를 추가했습니다.주의깊게 볼점
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Database
egg_typetable and updated related tables and test data to use foreign key relationships.