Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
db/
docker/db/
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ out/
.vscode/
.idea
.env
/db
/docker/db
!/resources/db/*.sql
mongodb

Expand Down
15 changes: 1 addition & 14 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-security'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-data-elasticsearch'
implementation 'co.elastic.clients:elasticsearch-java:8.14.0'
implementation 'jakarta.json:jakarta.json-api:2.1.3'
implementation 'org.eclipse.parsson:parsson:1.1.5'
implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310'
Expand All @@ -48,6 +46,7 @@ dependencies {
implementation 'io.github.resilience4j:resilience4j-reactor:2.2.0'
implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-xml'
implementation 'io.github.cdimascio:java-dotenv:5.2.2'
implementation 'org.modelmapper:modelmapper:3.1.1'

implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.9'

Expand Down Expand Up @@ -82,15 +81,3 @@ tasks.named('test') {
clean {
delete file('src/main/generated')
}


// bootRun 자동 컴파일 설정 (핫 리로딩)
tasks.named('bootRun') {
dependsOn 'classes'

doFirst {
println "starting bootRun with auto-compile..."
}
}


50 changes: 0 additions & 50 deletions docker-compose.yml

This file was deleted.

29 changes: 0 additions & 29 deletions docker/Dockerfile

This file was deleted.

30 changes: 30 additions & 0 deletions docker/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
version: "3.8"

services:
db:
image: mysql:8.4
container_name: db
platform: linux/amd64
environment:
MYSQL_USER: artrip
MYSQL_ROOT_PASSWORD: artrip1!
MYSQL_DATABASE: artrip
MYSQL_PASSWORD: artrip1!
ports:
- "33069:3306"
volumes:
- ./db/data:/var/lib/mysql
- ./db/conf:/etc/mysql/conf.d

redis:
image: redis:7.2
container_name: redis
restart: always
ports:
- "63799:6379"
volumes:
- redis-data:/data


volumes:
redis-data:
37 changes: 0 additions & 37 deletions src/main/java/org/atdev/artrip/config/ElasticsearchConfig.java

This file was deleted.

This file was deleted.

18 changes: 18 additions & 0 deletions src/main/java/org/atdev/artrip/config/ModelMapperConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.atdev.artrip.config;

import org.modelmapper.ModelMapper;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class ModelMapperConfig {
@Bean
public ModelMapper modelMapper() {
ModelMapper modelMapper = new ModelMapper();
modelMapper.getConfiguration()
.setFieldAccessLevel(org.modelmapper.config.Configuration.AccessLevel.PRIVATE)
.setFieldMatchingEnabled(true);

return modelMapper;
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModelMapper의 필요성이 느껴진 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convter는 전에 말씀주신대로 컨버터를 작성하신분만 사용가능하기 떄문에 재사용이 어렵다고 생각되었습니다.
또한, 하나씩 모든 객체를 매핑하기 위해서 작성하는것 보다 DTO에서 entity로 변환할 수 있는 ModelMapper를 사용하는것이 생산성을 높일 수 있다고 생각했습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entity -> (service layer) dto -> (presentation layer) dto 과정에서 ModelMapper가 굳이 필요하진 않을 것 같다고는 생각합니다.

그 이유는 저희 프로젝트에서 복잡한 매핑을 맺는 부분은 없어보이고 ModelMapper 라이브러리를 사용할만한 곳이 보이지 않았기 때문인데 매핑 관계가 복잡한 테이블 예시가 있으시다면 사용해도 될 것 같습니다만 그게 아니라면 DTO 간에 변환으로만으로도 충분히 커버가 가능할 것 같은데 좋은 고민이었습니다. 같이 고민을 좀 해보시죠.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우선 지금 사용하는곳이 없어서 제외했습니다.
추후 필요하게되면 다시 추가 하겠습니다!

11 changes: 6 additions & 5 deletions src/main/java/org/atdev/artrip/controller/ExhibitController.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import io.swagger.v3.oas.annotations.Operation;
import lombok.RequiredArgsConstructor;
import org.atdev.artrip.controller.dto.response.ExhibitDetailResponse;
import org.atdev.artrip.controller.dto.response.HomeListResponse;
import org.atdev.artrip.service.ExhibitService;
import org.atdev.artrip.controller.dto.request.ExhibitFilterRequest;
import org.atdev.artrip.controller.dto.response.FilterResponse;
Expand Down Expand Up @@ -93,12 +94,12 @@ public ResponseEntity<CommonResponse<List<RegionResponse>>> getDomestic(){
home = {HomeError._HOME_INVALID_DATE_RANGE, HomeError._HOME_UNRECOGNIZED_REGION, HomeError._HOME_EXHIBIT_NOT_FOUND}
)
@PostMapping("/filter")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESTful API의 규약상 "행위"보다는 "리소스"에 초점을 맞추는 것이 좋은데 행위에 초점을 맞춘 것으로 보여요.
다른 URL Path를 고려해보는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SearchController 외 Search 관련 된것을 제외 후 exhibit 하위로 이전하였습니다.

public ResponseEntity<FilterResponse> getDomesticFilter(@RequestBody ExhibitFilterRequest dto,
@RequestParam(required = false) Long cursor,
@RequestParam(defaultValue = "20") Long size,
@AuthenticationPrincipal UserDetails userDetails) {
public ResponseEntity<FilterResponse<HomeListResponse>> getDomesticFilter(@RequestBody ExhibitFilterRequest dto,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET을 제외한 HTTP Method에서 반환 타입을 모두 낼 필요는 없을 것 같은데 이건 프론트엔드 측과 합의된 사항일까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아직 합의 된게 없습니다.
같은 코드가 너무 많아서 브렌치 새로 작성한 후 수정 진행하겠습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET을 제외한 나머지 경우에는 상태코드만 명확히 전달해줘도 코드가 충분히 감소할 것 같아요.
이건 정답이 있는 것은 아니긴한데 빠른 시일 내로 결정지었으면 하네요.

@RequestParam(required = false) Long cursor,
@RequestParam(defaultValue = "20") Long size,
@AuthenticationPrincipal UserDetails userDetails) {
Long userId = getUserId(userDetails);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserDetails에서 userId를 꺼내 쓰는 방식이 아무래도 공통적으로 사용되는 부분으로 보여지는데 재사용성을 위해서라면 별도의 어노테이션으로 달아서 현재 인증 객체를 참조하는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커스텀 어노테이션으로 만들었습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

userId만 있다면 지금처럼 쓰셔도 될 것 같고 다른 Claims의 일부를 파싱해서 써야 한다면 @LoginUser 등 다른 네이밍도 검토할 수 있을 것 같아요.

FilterResponse exhibits = homeService.getFilterExhibit(dto, size, cursor,userId);
FilterResponse<HomeListResponse> exhibits = homeService.getFilterExhibit(dto, size, cursor,userId);
Copy link

@dnwls16071 dnwls16071 Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서비스 레이어에서 DTO로 리턴해주는 부분에서 그대로 프레젠테이션 레이어로 가게 되는데 이렇게 되면 유지보수 시 영향이 있을 것 같아요.
레이어 간 분리를 해보는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네! 분리하겠습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entity -> (service layer dto -> (presentation layer) dto 이런 흐름으로 분리하는게 좋을 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

service 레이어에 사용될 dto를 result로 생성을 해서 작업을 진행했습니다 말씀 주신 방향성이 맞는지 확인부탁드려도될까요 ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 이거 관련된 내용 @wook099 님께 알려드렸어요.
한 번 여쭤보시는게 빠를 거에요~


return ResponseEntity.ok(exhibits);

Expand Down
Loading