[Phase 2] spring analysis apis - #39
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThree new REST endpoints are added to the property backend: ChangesProperty Analysis APIs
Sequence Diagram(s)sequenceDiagram
participant Client
participant PropertyController
participant PriceAnalysisController
participant PropertyServiceImpl
participant PropertyDao
participant DB
Client->>PropertyController: GET /{propertyId}/transactions?years=3
PropertyController->>PropertyServiceImpl: getTransactions(propertyId, years)
PropertyServiceImpl->>PropertyDao: findActiveById(propertyId)
PropertyDao->>DB: SELECT from properties
DB-->>PropertyDao: PropertyRow
PropertyDao-->>PropertyServiceImpl: Optional<PropertyRow>
PropertyServiceImpl->>PropertyDao: findComparableTransactions(property, minContractYearMonth)
PropertyDao->>DB: SELECT from transaction_history (match_rank, LIMIT 20)
DB-->>PropertyDao: rows
PropertyDao-->>PropertyServiceImpl: List<PropertyTransactionResponse>
PropertyServiceImpl-->>PropertyController: list
PropertyController-->>Client: ApiResponse<ListResponse>
Client->>PropertyController: GET /{propertyId}/safety-summary?radius=500
PropertyController->>PropertyServiceImpl: getSafetySummary(propertyId, radius)
PropertyServiceImpl->>PropertyDao: findSafetySummary(propertyId, radius)
PropertyDao->>DB: SELECT from property_score_stat
DB-->>PropertyDao: Optional row
PropertyDao-->>PropertyServiceImpl: Optional<PropertySafetySummaryResponse>
PropertyServiceImpl-->>PropertyController: PropertySafetySummaryResponse (or default)
PropertyController-->>Client: ApiResponse<PropertySafetySummaryResponse>
Client->>PriceAnalysisController: GET /price-analysis?legalDongCode=...&propertyType=...&transactionType=...
PriceAnalysisController->>PropertyServiceImpl: getPriceAnalysis(legalDongCode, propertyType, transactionType)
PropertyServiceImpl->>PropertyDao: findRegionPriceStats(...)
PropertyDao->>DB: SELECT from region_price_stat
DB-->>PropertyDao: List<RegionPriceStatResponse>
PropertyServiceImpl->>PropertyDao: findBuildingPriceStats(...)
PropertyDao->>DB: SELECT from building_price_stat (LIMIT 20)
DB-->>PropertyDao: List<BuildingPriceStatResponse>
PropertyServiceImpl-->>PriceAnalysisController: PriceAnalysisResponse
PriceAnalysisController-->>Client: ApiResponse<PriceAnalysisResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/main/java/com/ssafy/salmanhae/controller/price/PriceAnalysisController.java`:
- Around line 24-29: The getPriceAnalysis method in PriceAnalysisController is
deferring validation of the legalDongCode parameter to the service layer, which
violates the project guideline that controllers should handle input validation
at the boundary. Add validation logic or validation annotations (such as
`@NotBlank` or `@NotNull`) for the legalDongCode parameter directly in the
getPriceAnalysis method to ensure invalid input is rejected at the controller
layer before delegation to the propertyService.getPriceAnalysis call.
In
`@backend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.java`:
- Around line 68-82: Add input validation constraints to the controller
parameters in both getPropertyTransactions and getPropertySafetySummary methods.
Use validation annotations such as `@NotNull` and `@Positive` (or `@Min/`@Max as
appropriate) on the propertyId `@PathVariable` parameter to ensure it is a valid
positive number, and on the years and radius `@RequestParam` parameters to enforce
appropriate numeric bounds. This ensures the controller layer owns boundary
validation rather than delegating it to the service layer, following the stated
coding guideline that controller methods should handle input validation before
delegation.
In
`@backend/src/main/java/com/ssafy/salmanhae/service/property/PropertyServiceImpl.java`:
- Around line 57-64: The getSafetySummary method accepts a dynamic radius
parameter and echoes it into the response, but the underlying
propertyDao.findSafetySummary query operates on fixed-radius database columns
(cctv_count_300m, bell_count_300m, light_count_300m, police_count_500m). Enforce
validation to only accept radius values that are actually supported by the
database (300 and 500 meters). Update the radius validation logic to reject
unsupported values with an appropriate error, ensuring the response radius value
always matches what was actually queried from the database.
In `@backend/src/test/resources/schema.sql`:
- Around line 79-88: The test schema for property_score_stat table does not
match the production migration contract and is allowing invalid states to pass
in tests. Update the property_score_stat table definition in the test DDL to
include all constraints from the production migration: add the foreign key
constraint on property_id, apply proper null/default constraints to the count
fields (cctv_count_300m, bell_count_300m, light_count_300m, police_count_500m),
and add the missing created_at timestamp column. Reference the production
migration at database/migrations/202606230003_create_property_score_stat.sql to
ensure the test schema exactly matches the production contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8fed3eb7-11df-4d40-9c06-6eb16ec1f938
📒 Files selected for processing (19)
backend/src/main/java/com/ssafy/salmanhae/controller/price/PriceAnalysisController.javabackend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.javabackend/src/main/java/com/ssafy/salmanhae/model/dao/property/JdbcPropertyDao.javabackend/src/main/java/com/ssafy/salmanhae/model/dao/property/PropertyDao.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/BuildingPriceStatResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PriceAnalysisResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertySafetySummaryResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertyTransactionResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/RegionPriceStatResponse.javabackend/src/main/java/com/ssafy/salmanhae/service/property/PropertyService.javabackend/src/main/java/com/ssafy/salmanhae/service/property/PropertyServiceImpl.javabackend/src/test/java/com/ssafy/salmanhae/controller/price/PriceAnalysisControllerTest.javabackend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyControllerTest.javabackend/src/test/resources/data.sqlbackend/src/test/resources/schema.sqldatabase/README.mddatabase/migrations/202606230003_create_property_score_stat.sqldocs/07_DOMAIN_MODEL.mddocs/08_API_SPEC.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
변경 내용
연결 이슈
closes #38
테스트
Summary by CodeRabbit
New Features
GET /api/v1/price-analysisto compare regional and building price statistics by property type and transaction type.GET /api/v1/properties/{propertyId}/transactionsto view past transactions for a property.GET /api/v1/properties/{propertyId}/safety-summaryto view safety metrics and nearby infrastructure counts.Bug Fixes
Documentation