[Phase 1] feat(property): 지도 범위 내 매물 조회 API 구현 - #10
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughImplements the complete F-1 property lookup REST API in Spring Boot. Adds domain enums and DTO records, a centralized error/response infrastructure, a JDBC DAO with dynamic SQL predicate building against a ChangesProperty API Backend
Sequence Diagram(s)sequenceDiagram
participant Client
participant PropertyController
participant PropertyServiceImpl
participant JdbcPropertyDao
participant DB as properties table
rect rgba(100, 149, 237, 0.5)
Note over Client,DB: Bounds search
Client->>PropertyController: GET /api/v1/properties?west=..&east=..&south=..&north=..
PropertyController->>PropertyServiceImpl: searchProperties(PropertySearchCriteria)
PropertyServiceImpl->>PropertyServiceImpl: criteria.validateBounds()
PropertyServiceImpl->>JdbcPropertyDao: findInBounds(criteria)
JdbcPropertyDao->>DB: SELECT ... WHERE is_active=true AND longitude/latitude in bounds [+ optional filters]
DB-->>JdbcPropertyDao: List of rows
JdbcPropertyDao-->>PropertyServiceImpl: List<PropertyRow>
PropertyServiceImpl-->>PropertyController: List<PropertySummaryResponse>
PropertyController-->>Client: ApiResponse{ data: ListResponse{items, totalCount}, message: "OK" }
end
rect rgba(144, 238, 144, 0.5)
Note over Client,DB: Property detail
Client->>PropertyController: GET /api/v1/properties/{propertyId}
PropertyController->>PropertyServiceImpl: getProperty(propertyId)
PropertyServiceImpl->>JdbcPropertyDao: findActiveById(propertyId)
JdbcPropertyDao->>DB: SELECT ... WHERE id=? AND is_active=true
alt found
DB-->>JdbcPropertyDao: row
JdbcPropertyDao-->>PropertyServiceImpl: Optional<PropertyRow> (present)
PropertyServiceImpl-->>PropertyController: PropertyDetailResponse
PropertyController-->>Client: ApiResponse{ data: PropertyDetailResponse, message: "OK" }
else not found
DB-->>JdbcPropertyDao: empty
JdbcPropertyDao-->>PropertyServiceImpl: Optional.empty()
PropertyServiceImpl->>PropertyController: throws ApiException(PROPERTY_NOT_FOUND)
PropertyController-->>Client: 404 ErrorResponse{ code: "PROPERTY_NOT_FOUND" }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 6
🧹 Nitpick comments (4)
backend/src/main/java/com/ssafy/salmanhae/common/response/ListResponse.java (1)
10-12: ⚡ Quick winSnapshot
itemsbefore computingtotalCount.Using the original mutable list reference can lead to
itemsandtotalCountdrifting if the list is modified after construction.💡 Suggested patch
public static <T> ListResponse<T> from(List<T> items) { - return new ListResponse<>(items, items.size()); + List<T> snapshot = List.copyOf(items); + return new ListResponse<>(snapshot, snapshot.size()); }🤖 Prompt for 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. In `@backend/src/main/java/com/ssafy/salmanhae/common/response/ListResponse.java` around lines 10 - 12, The from method in the ListResponse class is passing the mutable list reference directly to the constructor, which can cause totalCount to drift if the original list is modified after construction. Create a snapshot of the items list (such as using a copy constructor or list copy method like List.copyOf() or new ArrayList<>(items)) and pass this immutable snapshot to the constructor instead of the original items reference, ensuring the totalCount remains consistent with the stored items regardless of external modifications to the source list.backend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.java (1)
31-58: ⚡ Quick winConsider pagination for the list endpoint.
The bounds-based search returns all matching properties within the specified area without pagination. For dense urban areas, this could return hundreds or thousands of results, impacting client performance and bandwidth.
While not required for Phase 1, consider adding pagination parameters (page, size) in a future iteration to support incremental loading as users pan/zoom the map.
🤖 Prompt for 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. In `@backend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.java` around lines 31 - 58, Add pagination support to the searchProperties method in PropertyController by including page and size request parameters (with sensible defaults), updating the PropertySearchCriteria class to accept and store these pagination values, and modifying the propertyService.searchProperties call to use pagination when executing the query to limit results and improve performance for areas with large numbers of properties.backend/src/main/resources/application.properties (1)
2-5: 💤 Low valueConsider adding documentation for required environment variables.
The datasource configuration uses empty defaults, which will cause startup failure if environment variables are not set. While this fail-fast behavior is appropriate for production, adding a comment documenting the required variables would improve developer experience.
📝 Suggested documentation comment
spring.application.name=salmanhae +# Required environment variables: +# SUPABASE_DB_URL - PostgreSQL JDBC connection string (e.g., jdbc:postgresql://host:port/database) +# SUPABASE_DB_USERNAME - Database username +# SUPABASE_DB_PASSWORD - Database password spring.datasource.url=${SUPABASE_DB_URL:} spring.datasource.username=${SUPABASE_DB_USERNAME:} spring.datasource.password=${SUPABASE_DB_PASSWORD:}🤖 Prompt for 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. In `@backend/src/main/resources/application.properties` around lines 2 - 5, The datasource configuration properties spring.datasource.url, spring.datasource.username, and spring.datasource.password use empty defaults for environment variables SUPABASE_DB_URL, SUPABASE_DB_USERNAME, and SUPABASE_DB_PASSWORD respectively, but lack documentation. Add clear comments above these properties documenting that these environment variables are required and must be set for the application to start successfully, explaining what each variable is used for to improve developer experience.backend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyHttpIntegrationTest.java (1)
22-47: ⚡ Quick winConsider splitting into separate test methods for better isolation.
This single test method verifies three distinct scenarios: list endpoint success, detail endpoint success, and not-found error. Splitting these into separate test methods would improve test clarity and ensure that if one assertion fails, the others still run.
♻️ Suggested refactor
`@Test` - void propertyEndpointsRespondOverHttp() { + void listEndpointReturnsOkWithValidBounds() { ResponseEntity<Map> listResponse = restTemplate.getForEntity( "/api/v1/properties?west=126.93&east=126.94&south=37.46&north=37.48", Map.class ); assertThat(listResponse.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(listResponse.getBody()).containsEntry("message", "OK"); + } + `@Test` + void detailEndpointReturnsOkForExistingProperty() { ResponseEntity<Map> detailResponse = restTemplate.getForEntity( "/api/v1/properties/1", Map.class ); assertThat(detailResponse.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(detailResponse.getBody()).containsEntry("message", "OK"); + } + `@Test` + void detailEndpointReturnsNotFoundForMissingProperty() { ResponseEntity<Map> notFoundResponse = restTemplate.getForEntity( "/api/v1/properties/9999", Map.class ); assertThat(notFoundResponse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); assertThat(notFoundResponse.getBody()).containsEntry("code", "PROPERTY_NOT_FOUND"); }🤖 Prompt for 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. In `@backend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyHttpIntegrationTest.java` around lines 22 - 47, The propertyEndpointsRespondOverHttp test method is testing three distinct scenarios in a single test: list endpoint success, detail endpoint success, and not-found error. Split this into three separate test methods with descriptive names (such as testPropertyListEndpoint, testPropertyDetailEndpoint, and testPropertyDetailNotFound) where each method tests only one scenario. This improves test isolation and clarity, ensuring that if one assertion fails, the others can still execute independently.
🤖 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/common/exception/GlobalExceptionHandler.java`:
- Around line 20-24: The GlobalExceptionHandler is currently mapping
IllegalArgumentException to a 400 INVALID_REQUEST status code along with
MissingServletRequestParameterException and MethodArgumentTypeMismatchException.
This is problematic because IllegalArgumentException can be thrown from
service/DAO code for legitimate server-side errors, not just invalid client
requests. Remove IllegalArgumentException from the ExceptionHandler annotation
on the handler method that processes MissingServletRequestParameterException and
MethodArgumentTypeMismatchException to keep this handler scoped only to
request-binding validation exceptions and prevent masking of actual server
defects.
In
`@backend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.java`:
- Around line 39-42: The PropertyController accepts minDeposit, maxDeposit,
minPrice, and maxPrice request parameters but does not validate them before use.
You need to invoke the validateBounds() method on the PropertySearchCriteria
object before passing it to the service layer. Additionally, add validation to
ensure all filter values are non-negative (minDeposit, maxDeposit, minPrice,
maxPrice should all be >= 0) to prevent invalid data from being processed.
In
`@backend/src/main/java/com/ssafy/salmanhae/model/dao/property/JdbcPropertyDao.java`:
- Around line 76-77: Remove the LIMIT 500 clause from the SQL query in the list
query method of JdbcPropertyDao. In the sql.append() call that currently appends
" ORDER BY id ASC LIMIT 500", remove the " LIMIT 500" portion so it only appends
" ORDER BY id ASC". This will allow the query to return all active properties
within the specified bounds without silent truncation, maintaining the contract
of the list endpoint.
In
`@backend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertySearchCriteria.java`:
- Around line 37-42: The PropertySearchCriteria class currently validates that
minDeposit is not greater than maxDeposit and minPrice is not greater than
maxPrice, but it does not validate that these values are non-negative. Add
validation checks after the existing comparison validations to ensure that
minDeposit, maxDeposit, minPrice, and maxPrice are all non-negative values
(i.e., >= 0). Throw an ApiException with ErrorCode.INVALID_REQUEST if any of
these fields contain a negative value, similar to the existing ordering
validation logic.
In
`@backend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyControllerTest.java`:
- Around line 39-54: The test method
searchPropertiesAppliesTransactionAndPropertyFilters validates price filtering
but does not test the deposit filtering functionality (minDeposit and maxDeposit
parameters) that was implemented in the controller and criteria layer. Add new
test methods that perform API requests with minDeposit and maxDeposit
parameters, similar to how the existing test exercises minPrice and maxPrice.
Include assertions to verify that the deposit range filters correctly return the
expected properties with matching deposit amounts.
- Around line 56-66: The test searchPropertiesRejectsInvalidBounds currently
only validates that invalid longitude bounds (west > east) are rejected, but
does not cover the case where latitude bounds are invalid (south > north). Add a
new test method similar to searchPropertiesRejectsInvalidBounds that performs a
GET request to the /api/v1/properties endpoint with valid longitude parameters
but with south greater than north (for example, south="37.48" and north="37.46")
to ensure the API rejects this invalid latitude bounds scenario and returns a
400 Bad Request response with the appropriate error code.
---
Nitpick comments:
In `@backend/src/main/java/com/ssafy/salmanhae/common/response/ListResponse.java`:
- Around line 10-12: The from method in the ListResponse class is passing the
mutable list reference directly to the constructor, which can cause totalCount
to drift if the original list is modified after construction. Create a snapshot
of the items list (such as using a copy constructor or list copy method like
List.copyOf() or new ArrayList<>(items)) and pass this immutable snapshot to the
constructor instead of the original items reference, ensuring the totalCount
remains consistent with the stored items regardless of external modifications to
the source list.
In
`@backend/src/main/java/com/ssafy/salmanhae/controller/property/PropertyController.java`:
- Around line 31-58: Add pagination support to the searchProperties method in
PropertyController by including page and size request parameters (with sensible
defaults), updating the PropertySearchCriteria class to accept and store these
pagination values, and modifying the propertyService.searchProperties call to
use pagination when executing the query to limit results and improve performance
for areas with large numbers of properties.
In `@backend/src/main/resources/application.properties`:
- Around line 2-5: The datasource configuration properties
spring.datasource.url, spring.datasource.username, and
spring.datasource.password use empty defaults for environment variables
SUPABASE_DB_URL, SUPABASE_DB_USERNAME, and SUPABASE_DB_PASSWORD respectively,
but lack documentation. Add clear comments above these properties documenting
that these environment variables are required and must be set for the
application to start successfully, explaining what each variable is used for to
improve developer experience.
In
`@backend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyHttpIntegrationTest.java`:
- Around line 22-47: The propertyEndpointsRespondOverHttp test method is testing
three distinct scenarios in a single test: list endpoint success, detail
endpoint success, and not-found error. Split this into three separate test
methods with descriptive names (such as testPropertyListEndpoint,
testPropertyDetailEndpoint, and testPropertyDetailNotFound) where each method
tests only one scenario. This improves test isolation and clarity, ensuring that
if one assertion fails, the others can still execute independently.
🪄 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: 5868a9a0-0a06-4a09-889a-92c492e3456f
📒 Files selected for processing (27)
backend/pom.xmlbackend/src/main/java/com/ssafy/salmanhae/common/exception/ApiException.javabackend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.javabackend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorResponse.javabackend/src/main/java/com/ssafy/salmanhae/common/exception/GlobalExceptionHandler.javabackend/src/main/java/com/ssafy/salmanhae/common/response/ApiResponse.javabackend/src/main/java/com/ssafy/salmanhae/common/response/ListResponse.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/PropertyDetailResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertyRow.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertySearchCriteria.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertySummaryResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/PropertyType.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/property/TransactionType.javabackend/src/main/java/com/ssafy/salmanhae/service/property/PropertyService.javabackend/src/main/java/com/ssafy/salmanhae/service/property/PropertyServiceImpl.javabackend/src/main/resources/application.propertiesbackend/src/test/java/com/ssafy/salmanhae/SalmanhaeApplicationTests.javabackend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyControllerTest.javabackend/src/test/java/com/ssafy/salmanhae/controller/property/PropertyHttpIntegrationTest.javabackend/src/test/resources/application-test.propertiesbackend/src/test/resources/data.sqlbackend/src/test/resources/schema.sqlphases/property-api-be/phase1-property-api-be.mdphases/property-api-be/phase1.status.json
HOKAGO-MEMORIES
changed the title
HOKAGO-MEMORIES
changed the title
HOKAGO-MEMORIES
changed the title
This was referenced Jun 18, 2026
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 #9
테스트
비고
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation