Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 검수할 장소 등록/ 조회 API 구현 #404

Merged
merged 25 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5833293
fix : ManagerAuthInterceptor 디코딩 수정
zillionme Oct 3, 2023
9b4f610
feat: 인터셉터에 검수중 장소 조회 path 등록
zillionme Oct 3, 2023
b446c17
feat: 검수할 장소 등록 기능 추가
zillionme Oct 3, 2023
0c13a93
feat: 검수할 장소 등록 API 구현
zillionme Oct 3, 2023
7a757de
feat: 검수할 장소 목록 조회 기능 구현
zillionme Oct 3, 2023
b9693dc
feat: 검수할 장소 목록 조회 API 구현
zillionme Oct 3, 2023
29d6004
fix: 서비스 객체 빈 등록 및 인터셉터 추가
zillionme Oct 3, 2023
efbfd65
test: TemporaryPlaceBuilder 및 TemporaryPlaceFixture 작성
zillionme Oct 3, 2023
190a739
test: ManagerAuthInterceptorTest 디코딩 문제 수정
zillionme Oct 3, 2023
85afada
feat: TemporaryPlace예외 처리 추가 및 적용
zillionme Oct 3, 2023
a80562e
test: 검수할 장소 등록 테스트 추가
zillionme Oct 3, 2023
3cd6562
test: 필요없는 테스트 삭제
zillionme Oct 3, 2023
96a2a93
test: 20미터에 이미 등록된 목적지가 있다면 예외응답 테스트
dooboocookie Oct 4, 2023
4fa26c4
refactor: 필요없는 메서드 삭제
zillionme Oct 4, 2023
421daa0
refactor: PlaceCheckService 패키지 변경 및 예외 수정
zillionme Oct 4, 2023
01ebc2c
test: 검수할 장소 목록 조회 기능 테스트
zillionme Oct 4, 2023
884e107
refactor: 콘솔 로깅 디버그 레벨로 변경
zillionme Oct 4, 2023
890d080
refactor: TemporaryPlace관련 조회 쿼리 n+1문제 리팩터링
zillionme Oct 5, 2023
2df2100
refactor: TemporaryPlaceResponse에 플레이어 정보 추가
zillionme Oct 5, 2023
0cdae62
chore: 개행 제거
zillionme Oct 5, 2023
0b65425
test: TemporaryPlaceControllerTest 수정
zillionme Oct 5, 2023
c70ae34
refactor: 서비스 메서드명 변경
zillionme Oct 5, 2023
3057dba
refactor: TemporaryPlaceResponse 필드명 변경
zillionme Oct 5, 2023
2be9c53
refactor: 검수할 장소 조회 오래된 순으로 정렬 순서 변경
zillionme Oct 5, 2023
647be5a
chore: 병합 충돌 해결
zillionme Oct 5, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class ManagerAuthInterceptor implements HandlerInterceptor {

public static final int AUTH_HEADER_INFO_SIZE = 2;

public static final String AUTH_HEADER_TYPE = "Basic ";
public static final String AUTH_HEADER_TYPE = "Basic";

@Value("${manager.id}")
private String id;
Expand All @@ -41,12 +41,12 @@ private List<String> extractHeaderInfo(final HttpServletRequest request) {
if (header == null) {
throw new AuthException(NOT_EXIST_HEADER);
}
final String decodedHeader = new String(Base64.getDecoder().decode(header));
if (!decodedHeader.startsWith(AUTH_HEADER_TYPE)) {
final String[] authHeader = header.split(" ");
if (!AUTH_HEADER_TYPE.equals(authHeader[0])) {
throw new AuthException(INVALID_HEADER);
}
final String decodedHeaderWithoutType = decodedHeader.replace(AUTH_HEADER_TYPE, "");
final String[] idAndPassword = decodedHeaderWithoutType.split(":");
final String decodedHeader = new String(Base64.getDecoder().decode(authHeader[1]));
final String[] idAndPassword = decodedHeader.split(":");
if (idAndPassword.length != AUTH_HEADER_INFO_SIZE) {
throw new AuthException(INVALID_HEADER);
}
Expand Down
16 changes: 14 additions & 2 deletions backend/src/main/java/com/now/naaga/common/config/WebConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.now.naaga.auth.presentation.argumentresolver.MemberAuthArgumentResolver;
import com.now.naaga.auth.presentation.argumentresolver.PlayerArgumentResolver;
import com.now.naaga.auth.presentation.interceptor.AuthInterceptor;
import com.now.naaga.auth.presentation.interceptor.ManagerAuthInterceptor;
import com.now.naaga.common.presentation.interceptor.RequestMatcherInterceptor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Configuration;
Expand All @@ -24,20 +25,25 @@ public class WebConfig implements WebMvcConfigurer {

private final AuthInterceptor authInterceptor;

private final ManagerAuthInterceptor managerAuthInterceptor;

@Value("${manager.origin-url}")
private String managerUriPath;

public WebConfig(final PlayerArgumentResolver playerArgumentResolver,
final MemberAuthArgumentResolver memberAuthArgumentResolver,
final AuthInterceptor authInterceptor) {
final AuthInterceptor authInterceptor,
final ManagerAuthInterceptor managerAuthInterceptor) {
this.playerArgumentResolver = playerArgumentResolver;
this.memberAuthArgumentResolver = memberAuthArgumentResolver;
this.authInterceptor = authInterceptor;
this.managerAuthInterceptor = managerAuthInterceptor;
}

@Override
public void addInterceptors(final InterceptorRegistry registry) {
registry.addInterceptor(mapAuthInterceptor());
registry.addInterceptor(mapManagerAuthInterceptor());
}

private HandlerInterceptor mapAuthInterceptor() {
Expand All @@ -51,7 +57,13 @@ private HandlerInterceptor mapAuthInterceptor() {
.excludeRequestPattern("/**/*.gif")
.excludeRequestPattern("/**/*.ico")
.excludeRequestPattern("/ranks")
.excludeRequestPattern("/**", HttpMethod.OPTIONS);
.excludeRequestPattern("/**", HttpMethod.OPTIONS)
.excludeRequestPattern("/temporary-places", HttpMethod.GET);
}

private HandlerInterceptor mapManagerAuthInterceptor() {
return new RequestMatcherInterceptor(managerAuthInterceptor)
.includeRequestPattern("/temporary-places", HttpMethod.GET);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,54 +1,35 @@
package com.now.naaga.place.application;

import static com.now.naaga.place.exception.PlaceExceptionType.NO_EXIST;

import com.now.naaga.common.domain.OrderType;
import com.now.naaga.common.infrastructure.FileManager;
import com.now.naaga.place.application.dto.CreatePlaceCommand;
import com.now.naaga.place.application.dto.FindAllPlaceCommand;
import com.now.naaga.place.application.dto.FindPlaceByIdCommand;
import com.now.naaga.place.application.dto.RecommendPlaceCommand;
import com.now.naaga.place.domain.Place;
import com.now.naaga.place.domain.PlaceCheckService;
import com.now.naaga.place.domain.PlaceRecommendService;
import com.now.naaga.place.domain.Position;
import com.now.naaga.place.domain.SortType;
import com.now.naaga.place.exception.PlaceException;
import com.now.naaga.place.persistence.repository.PlaceRepository;
import com.now.naaga.player.application.PlayerService;
import com.now.naaga.player.domain.Player;
import java.io.File;
import java.util.List;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

import java.util.List;

import static com.now.naaga.place.exception.PlaceExceptionType.NO_EXIST;

@Transactional
@Service
public class PlaceService {

private final PlaceRepository placeRepository;

private final PlayerService playerService;

private final PlaceCheckService placeCheckService;

private final PlaceRecommendService placeRecommendService;

private final FileManager<MultipartFile> fileManager;

public PlaceService(final PlaceRepository placeRepository,
final PlayerService playerService,
final PlaceCheckService placeCheckService,
final PlaceRecommendService placeRecommendService,
final FileManager<MultipartFile> fileManager) {
final PlaceRecommendService placeRecommendService) {
this.placeRepository = placeRepository;
this.playerService = playerService;
this.placeCheckService = placeCheckService;
this.placeRecommendService = placeRecommendService;
this.fileManager = fileManager;
}

@Transactional(readOnly = true)
Expand All @@ -73,23 +54,6 @@ public Place recommendPlaceByPosition(final RecommendPlaceCommand recommendPlace
}

public Place createPlace(final CreatePlaceCommand createPlaceCommand) {
final Position position = createPlaceCommand.position();
placeCheckService.checkOtherPlaceNearby(position);
final File uploadPath = fileManager.save(createPlaceCommand.imageFile());
try {
final Long playerId = createPlaceCommand.playerId();
final Player registeredPlayer = playerService.findPlayerById(playerId);
final Place place = new Place(
createPlaceCommand.name(),
createPlaceCommand.description(),
position,
fileManager.convertToUrlPath(uploadPath),
registeredPlayer);
placeRepository.save(place);
return place;
} catch (final RuntimeException exception) {
uploadPath.delete();
throw exception;
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import com.now.naaga.place.exception.PlaceException;
import com.now.naaga.place.exception.PlaceExceptionType;
import com.now.naaga.place.persistence.repository.PlaceRepository;
import java.util.List;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

@Transactional
@Service
public class PlaceCheckService {
Expand All @@ -20,7 +21,7 @@ public PlaceCheckService(final PlaceRepository placeRepository) {
@Transactional(readOnly = true)
public void checkOtherPlaceNearby(final Position position) {
List<Place> places = placeRepository.findPlaceByPositionAndDistance(position, 0.02);
if (places.size() > 0) {
if (!places.isEmpty()) {
Comment on lines -23 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋은 수정이네요👍

throw new PlaceException(PlaceExceptionType.ALREADY_EXIST_NEARBY);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public enum PlaceExceptionType implements BaseExceptionType {
HttpStatus.BAD_REQUEST,
"이미 주변에 장소가 존재합니다."
),

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
불필요한 개행은 삭제해주세용!

;

private final int errorCode;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package com.now.naaga.temporaryplace.application;

import com.now.naaga.common.infrastructure.FileManager;
import com.now.naaga.place.domain.Position;
import com.now.naaga.player.application.PlayerService;
import com.now.naaga.player.domain.Player;
import com.now.naaga.temporaryplace.application.dto.CreateTemporaryPlaceCommand;
import com.now.naaga.temporaryplace.domain.TemporaryPlace;
import com.now.naaga.temporaryplace.repository.TemporaryPlaceRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

import java.io.File;
import java.util.Comparator;
import java.util.List;

@Transactional
@Service
public class TemporaryPlaceService {

private final TemporaryPlaceRepository temporaryPlaceRepository;

private final PlayerService playerService;

private final FileManager<MultipartFile> fileManager;

public TemporaryPlaceService(final TemporaryPlaceRepository temporaryPlaceRepository,
final PlayerService playerService,
final FileManager<MultipartFile> fileManager) {
this.temporaryPlaceRepository = temporaryPlaceRepository;
this.playerService = playerService;
this.fileManager = fileManager;
}

public TemporaryPlace createTemporaryPlace(final CreateTemporaryPlaceCommand createTemporaryPlaceCommand) {
final Position position = createTemporaryPlaceCommand.position();
final File uploadPath = fileManager.save(createTemporaryPlaceCommand.imageFile());
try {
final Long playerId = createTemporaryPlaceCommand.playerId();
final Player registeredPlayer = playerService.findPlayerById(playerId);
final TemporaryPlace temporaryPlace = new TemporaryPlace(
createTemporaryPlaceCommand.name(),
createTemporaryPlaceCommand.description(),
position,
fileManager.convertToUrlPath(uploadPath),
registeredPlayer);
return temporaryPlaceRepository.save(temporaryPlace);
} catch (final RuntimeException exception) {
uploadPath.delete();
Copy link
Collaborator

Choose a reason for hiding this comment

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

P5:
이건 팀원 모두들에게 의견을 여쭙고 싶은데요.
사진을 관리하는 서비스를 둬서 해당 사진이 저장되는 것이나 트랜잭션에서 에러났을 때 삭제하는 로직 이런 것을 관리하도록 두는 것은 어떨까요?
아직 명확한 플로우가 생각이 안나서 추상적으로 적었는데, 지금 여기서 트라이 캐치로 컨트롤하는 것은 제가 쓴 코드긴 하지만 조금 빈약한 것 같아서요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

따로 빼서 이에 대한 로직을 캡슐화할 수 있을 것 같아요. 저도 추상적으로만 생각되긴 하는데, 배포 이후에 개선 방안을 생각해보면 좋을 것 같긴 하네요!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋습니다 해당 내용 이슈로 파두도록 하겠습니다

throw exception;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:

저희 아직 플레이어 장소 등록에 20미터 검증에 대한 명확한 기획 변경이 확정되지 않았습니다.
일단 사용자가 임시 장소를 등록할 때, 주변에 이미 찐 장소가 등록되어있더라면 빠른 피드백으로 에러응답을 주는 것이 맞지 않을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 아직 변경되지 않은 기획에 대해서는, 기존 기획 그대로 가는 것에 동의하는데요!

생각해보니 이번 배포 주기에 주변에 이미 찐 장소가 등록되어있더라면 빠른 피드백으로 에러응답 을 안드로이드 측에서 구현이 안되는 것으로 알고 있어요.

어떻게 하면 좋을까요? 뷰가 따로 구현되어있지 않다면 아마 사용자는 앱에서 장소 등록을 해도 무반응이 올 것 같기도...?

Copy link
Collaborator Author

@zillionme zillionme Oct 5, 2023

Choose a reason for hiding this comment

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

기획적인 문제 : 주변 장소 20m안에 다른 장소 있으면 등록 못하게 막는것을 유지할것인가
에 대해서 유저에게 빠른 피드백을 주기 위해 장소 검수를 등록하는 시점에 확인해야한다고 생각했습니다.
따라서 처음에는 검수 할 장소 등록 시점에 20m내에 다른 장소가 있으면 등록 자체를 막았는데요. 안드로이드의 뷰 구현 문제도 있고 해서 최종적으로 삭제하게 되었습니다.

이부분에 대해서는 위에 리뷰 포인트로 남겨두었는데요. 슬랙에서 얘기 나눈대로 안드 쪽에서도 아직 구현이 안된 부분이라 후일에 논의가 끝나면 다시 추가해도 될 것 같습니다


@Transactional(readOnly = true)
public List<TemporaryPlace> findAllPlace() {
final List<TemporaryPlace> temporaryPlaces = temporaryPlaceRepository.findAll();
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:

@Entity
public class TemporaryPlace extends BaseEntity {

    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Id
    private Long id;

    // ...

    @ManyToOne
    @JoinColumn(name = "player_id")
    private Player registeredPlayer;

    // ...
}
image

기본적으로 @ManyToOne은 즉시 로딩인데요.
그렇다면 이 findAll() 메서드가 호출되는 즉시 모든 player에 대한 조회를 각각 아이디로 보낼거에요!
이것이 N+1문제겠지요?

현재 뷰로 넘어가는 응답을 보면

public record TemporaryPlaceResponse(Long id,
                                     String name,
                                     CoordinateResponse coordinate,
                                     String imageUrl,
                                     String description) {
    // ...
}

등록자에 대한 정보는 안거드리므로 join은 필요가 없을 것 같아요!

그래서 제 생각에는 지연로딩으로 1차적으로 N+1 쿼리가 발생하지 않도록 하는 것이어떨까요?
추후에 이 임시 장소 정보들에서 Player 정보를 쓰게 된다면 그 떄 N+1문제가 발생할테니 그떄 해결해볼까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:

생각해보니 현재 전체 조회 API 명세에 문제가 있네요!

검수 장소에서 실제 인게임 장소로 등록되기 위해선(POST /places), 해당 장소 등록자의 playerId 가 필요해요.

때문에 TemporaryPlaceResponseregisteredPlayerId 필드가 추가되어야할 것 같습니다.

또한 위에서 루카가 잘 말씀해주셨는데, API 명세에서 playerId 가 추가되므로 이제 player 객체의 조회가 필요해져서 N+1 문제가 발생할 것 같습니다.

일단 연관관계에 대한 로딩을 지연로딩으로 바꾸고, findAll() 메서드에 Fetch Join 같은 최적화가 필요할 것 같습니다!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

API명세에 player관련 명세가 없어서 아무생각없이 코드를 짜버렸네요. 조회 쿼리를 다루면서 N+1문제에 대해서도 깊은 고민을 안했군요...ㅎㅎ
감사합니다

temporaryPlaces.sort(Comparator.comparing(TemporaryPlace::getCreatedAt));
return temporaryPlaces;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.now.naaga.temporaryplace.application.dto;

import com.now.naaga.place.domain.Position;
import com.now.naaga.player.presentation.dto.PlayerRequest;
import com.now.naaga.temporaryplace.presentation.dto.CreateTemporaryPlaceRequest;
import org.springframework.web.multipart.MultipartFile;

public record CreateTemporaryPlaceCommand(Long playerId,
String name,
String description,
Position position,
MultipartFile imageFile) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:

Suggested change
MultipartFile imageFile) {
MultipartFile imageFile) {

띄어쓰기 2번 입니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

고쳤습니다!


public static CreateTemporaryPlaceCommand of(final PlayerRequest playerRequest,
final CreateTemporaryPlaceRequest createTemporaryPlaceRequest) {
Position position = Position.of(createTemporaryPlaceRequest.latitude(), createTemporaryPlaceRequest.longitude());
return new CreateTemporaryPlaceCommand(
playerRequest.playerId(),
createTemporaryPlaceRequest.name(),
createTemporaryPlaceRequest.description(),
position,
createTemporaryPlaceRequest.imageFile());
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,47 @@
package com.now.naaga.temporaryplace.presentation;

import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import com.now.naaga.auth.presentation.annotation.Auth;
import com.now.naaga.player.presentation.dto.PlayerRequest;
import com.now.naaga.temporaryplace.application.TemporaryPlaceService;
import com.now.naaga.temporaryplace.application.dto.CreateTemporaryPlaceCommand;
import com.now.naaga.temporaryplace.domain.TemporaryPlace;
import com.now.naaga.temporaryplace.presentation.dto.CreateTemporaryPlaceRequest;
import com.now.naaga.temporaryplace.presentation.dto.TemporaryPlaceResponse;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

import java.net.URI;
import java.util.List;

@RequestMapping("/temporary-places")
@RestController
public class TemporaryPlaceController {

private final TemporaryPlaceService temporaryPlaceService;

public TemporaryPlaceController(final TemporaryPlaceService temporaryPlaceService) {
this.temporaryPlaceService = temporaryPlaceService;
}

@GetMapping
public ResponseEntity<List<TemporaryPlaceResponse>> findAllTemporaryPlace() {
final List<TemporaryPlace> temporaryPlaces = temporaryPlaceService.findAllPlace();
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:

서비스의 메서드명 일관성이 깨지는 것 같아요 ㅠㅠ

findAll() 과 같이 행위만 명시하는 방법, 혹은 findAllTemporaryPlace() 처럼 도메인의 이름을 함께 명시하는 방법 중 하나로 정해서 쭉 갔으면 좋겠습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

api 명세 얘기가 될 것 같은데요,,! 저희는 지금까지 정렬이 필요한 모든 api에 쿼리 스트링으로 조건을 달아주었습니다. 서비스 로직에서 시간 순으로 정렬을 하는 것이 보이는데 쿼리스트링으로 조건을 걸어줄 필요성에 대해서 이래 의견은 어떠신가요!?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이것도 얘기를 나눠봐야 할 것 같은데요 API명세 상으로는 아직 관련 내용이 적용되지 않아서.

  • 관리자 페이지에서 오래된 순으로 하는 게 정해져 있을 것 같아서 위와같이 코드를 짜게 되었습니다

final List<TemporaryPlaceResponse> response = TemporaryPlaceResponse.convertToResponses(temporaryPlaces);
return ResponseEntity
.status(HttpStatus.OK)
.body(response);
}

@PostMapping
public ResponseEntity<TemporaryPlaceResponse> createTemporaryPlace(@Auth final PlayerRequest playerRequest,
@ModelAttribute final CreateTemporaryPlaceRequest createTemporaryPlaceRequest) {
final CreateTemporaryPlaceCommand createTemporaryPlaceCommand = CreateTemporaryPlaceCommand.of(playerRequest, createTemporaryPlaceRequest);
final TemporaryPlace temporaryPlace = temporaryPlaceService.createTemporaryPlace(createTemporaryPlaceCommand);
final TemporaryPlaceResponse response = TemporaryPlaceResponse.from(temporaryPlace);
return ResponseEntity
.status(HttpStatus.CREATED)
.location(URI.create("/temporary-places/" + temporaryPlace.getId()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

P5:

근데 현재 이 로케이션에 대한 API는 생성이 안되어 있는데,
클라이언트에게 전달해주는 것이 맞을까요?
제 생각엔 앞으로 앱이 발전하면서 충분히 생길 수 있는 API라서 있는 것도 괜찮아 보이긴 합니다.
이레의 의견은 어떠세용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

관리자 페이지에서 임시장소 하나하나를 따로 볼 수도 있고 후에 확장성을 고려해서 위와 같이 코드를 짰숩니다~

.body(response);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.now.naaga.temporaryplace.presentation.dto;

import org.springframework.web.multipart.MultipartFile;

public record CreateTemporaryPlaceRequest(String name,
String description,
Double latitude,
Double longitude,
MultipartFile imageFile) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.now.naaga.temporaryplace.presentation.dto;

import com.now.naaga.game.presentation.dto.CoordinateResponse;
import com.now.naaga.temporaryplace.domain.TemporaryPlace;

import java.util.List;
import java.util.stream.Collectors;

public record TemporaryPlaceResponse(Long id,
Copy link
Collaborator

@kokodak kokodak Oct 4, 2023

Choose a reason for hiding this comment

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

P1:

위에 남긴 리뷰 코멘트 와 같은 맥락의 리뷰입니다!

우선 API 명세가 수정되어야할 것 같아요...! 이에 따라서 응답 DTO 에 필드 추가가 필요할 것 같습니다!!

String name,
CoordinateResponse coordinate,
String imageUrl,
String description

) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:

불필요한 개행은 삭제해주세요.


public static TemporaryPlaceResponse from(final TemporaryPlace savedTemporaryPlace) {
CoordinateResponse coordinateResponse = CoordinateResponse.of(savedTemporaryPlace.getPosition());
return new TemporaryPlaceResponse(
savedTemporaryPlace.getId(),
savedTemporaryPlace.getName(),
coordinateResponse,
savedTemporaryPlace.getImageUrl(),
savedTemporaryPlace.getDescription()
);
}

public static List<TemporaryPlaceResponse> convertToResponses(final List<TemporaryPlace> temporaryPlaces) {
return temporaryPlaces.stream()
.map(TemporaryPlaceResponse::from)
.collect(Collectors.toList());
}
}
2 changes: 1 addition & 1 deletion backend/src/main/resources/logback-spring.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
value="/home/ubuntu/logs"/>
</springProfile>

<root level="DEBUG">
<root level="INFO">
Copy link
Collaborator

Choose a reason for hiding this comment

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

P5:

저도 DEBUG 레벨 때문에 많이 불편했었는데 고쳐주셨네요!!

다만, 로깅 레벨을 바꾸는 것은 이 PR의 작업 내용과 관련이 없다고 생각해서, 이는 변경사항에서 빼주는 것도 좋을 것 같아요!

물론 변경점이 매우매우매우 작기 때문에 이정도는 괜찮긴하겠네요 ㅎㅎ

<appender-ref ref="CONSOLE"/>
</root>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class ManagerAuthInterceptorTest {
@Test
void Auth_헤더를_Base64_헤더를_디코딩해서_관리자_로그인을_처리한다() throws Exception {
// given
final String s = "Basic "+id+":"+password;
final String s = id+":"+password;
final String authHeader = new String(Base64.getEncoder().encode(s.getBytes()));
final MockHttpServletRequest request = new MockHttpServletRequest();
final MockHttpServletResponse response = new MockHttpServletResponse();
final Controller controller = Mockito.mock(Controller.class);
request.addHeader("Authorization", authHeader);
request.addHeader("Authorization","Basic "+ authHeader);

// when
final boolean expected = managerAuthInterceptor.preHandle(request, response, controller);
Expand Down
Loading
Loading