Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;

import in.koreatech.koin.domain.owner.dto.OwnerResponse;
import in.koreatech.koin.domain.owner.dto.OwnerUpdateRequest;
import in.koreatech.koin.domain.owner.dto.VerifyEmailRequest;
import in.koreatech.koin.global.auth.Auth;
import io.swagger.v3.oas.annotations.Operation;
Expand Down Expand Up @@ -48,4 +50,21 @@ ResponseEntity<Void> requestVerificationToRegister(
ResponseEntity<OwnerResponse> getOwner(
@Auth(permit = {OWNER}) Long userId
);

@ApiResponses(
value = {
@ApiResponse(responseCode = "200"),
@ApiResponse(responseCode = "401", content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "403", content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "404", content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "422", content = @Content(schema = @Schema(hidden = true))),
}
)
@Operation(summary = "사장님 인증용 첨부파일 등록")
@SecurityRequirement(name = "Jwt Authentication")
@PutMapping("/owner")
ResponseEntity<OwnerResponse> putOwner(
@Auth(permit = {OWNER}) Long userId,
@RequestBody @Valid OwnerUpdateRequest request
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
import static in.koreatech.koin.domain.user.model.UserType.OWNER;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;

import in.koreatech.koin.domain.owner.dto.OwnerResponse;
import in.koreatech.koin.domain.owner.dto.OwnerUpdateRequest;
import in.koreatech.koin.domain.owner.dto.VerifyEmailRequest;
import in.koreatech.koin.domain.owner.service.OwnerService;
import in.koreatech.koin.global.auth.Auth;
Expand All @@ -29,10 +32,21 @@ public ResponseEntity<Void> requestVerificationToRegister(
}

@Override
@GetMapping("/owner")
public ResponseEntity<OwnerResponse> getOwner(
@Auth(permit = {OWNER}) Long ownerId
) {
OwnerResponse ownerInfo = ownerService.getOwner(ownerId);
return ResponseEntity.ok().body(ownerInfo);
}

@Override
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

다른 controller들 override 다 빠져있는데 빼는걸로 통일 어때요

@PutMapping("/owner")
public ResponseEntity<OwnerResponse> putOwner(
@Auth(permit = {OWNER}) Long userId,
@RequestBody @Valid OwnerUpdateRequest request
) {
OwnerResponse ownerInfo = ownerService.putOwner(userId, request);
return ResponseEntity.ok().body(ownerInfo);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package in.koreatech.koin.domain.owner.dto;

import java.util.List;

import org.hibernate.validator.constraints.URL;

import com.fasterxml.jackson.databind.PropertyNamingStrategies.SnakeCaseStrategy;
import com.fasterxml.jackson.databind.annotation.JsonNaming;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Size;

@JsonNaming(SnakeCaseStrategy.class)
public record OwnerUpdateRequest(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

R

비록 API는 PUT /owner 이지만 우리가 이해하는 바로는 OwnerUpdateRequest는 전혀 아닌거같아요
객체명을 봤을 때 사장님 정보를 업데이트하는 것 처럼 느껴졌는데 들어왔더니 fileUrls만 있어서 당황스럽군요...

다른 정보도 수정할 수 있게끔 값을 추가해야할 필요가 있어보이네요
현재 클라이언트에서 PUT /owner 구현이 안되어있는걸로 알고있는데 한번 논의를 해봐야겠군요

@Valid
@Size(min = 3, max = 5, message = "이미지는 사업자등록증, 영업신고증, 통장사본을 포함하여 최소 3개 최대 5개까지 가능합니다.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

R

1개로 변경되었습니다.

Suggested change
@Size(min = 3, max = 5, message = "이미지는 사업자등록증, 영업신고증, 통장사본을 포함하여 최소 3개 최대 5개까지 가능합니다.")
@Size(min = 1, max = 5, message = "이미지는 사업자등록증, 영업신고증, 통장사본을 포함하여 최소 1개 최대 5개까지 가능합니다.")

@Schema(description = "첨부 파일 URL 목록(최소 3개 최대 5개, 코인 파일 형식이어야 함)")
List<InnerAttachmentUrlRequest> attachmentUrls
) {

@JsonNaming(SnakeCaseStrategy.class)
public record InnerAttachmentUrlRequest(
@NotBlank
@URL(protocol = "https", regexp = ".*static\\.koreatech\\.in.*", message = "코인 파일 저장 형식이 아닙니다.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

C

메시지가 살짝 빈약해보이는데 사용자가 입력한 값을 보여주는건 어때요?

Suggested change
@URL(protocol = "https", regexp = ".*static\\.koreatech\\.in.*", message = "코인 파일 저장 형식이 아닙니다.")
@URL(protocol = "https", regexp = ".*static\\.koreatech\\.in.*", message = "코인 파일 저장 형식이 아닙니다. : ${validatedValue}")

@Schema(description = "첨부 파일 URL (코인 파일 형식이어야 함)", example = "https://static.koreatech.in/1.png")
String fileUrl
) {

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package in.koreatech.koin.domain.owner.exception;

import in.koreatech.koin.global.exception.ArgumentCountException;

public class OwnerAttachmentsCountException extends ArgumentCountException {

private static final String DEFAULT_MESSAGE = "첨부파일 개수가 부족합니다.";
Comment on lines +5 to +7
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A

IllegalArgumentException 상속해도 무관해보이는데 어떤가요


public OwnerAttachmentsCountException(String message) {
super(message);
}

public static OwnerAttachmentsCountException withDetail(String detail) {
String message = String.format("%s %s", DEFAULT_MESSAGE, detail);
return new OwnerAttachmentsCountException(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ public void updateName() {
}

@Builder
private OwnerAttachment(Owner owner, String url, Boolean isDeleted, String name) {
private OwnerAttachment(Owner owner, String url, Boolean isDeleted) {
this.owner = owner;
this.url = url;
this.isDeleted = isDeleted;
this.name = name;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package in.koreatech.koin.domain.owner.repository;

import java.util.List;
import java.util.Optional;

import org.springframework.data.repository.Repository;

Expand All @@ -13,4 +14,6 @@ public interface OwnerAttachmentRepository extends Repository<OwnerAttachment, L
OwnerAttachment findById(Long id);

List<OwnerAttachment> findAllByOwnerId(Long ownerId);

Optional<OwnerAttachment> findByOwnerIdAndUrl(Long userId, String url);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
import org.springframework.transaction.annotation.Transactional;

import in.koreatech.koin.domain.owner.dto.OwnerResponse;
import in.koreatech.koin.domain.owner.dto.OwnerUpdateRequest;
import in.koreatech.koin.domain.owner.dto.VerifyEmailRequest;
import in.koreatech.koin.domain.owner.exception.OwnerAttachmentsCountException;
import in.koreatech.koin.domain.owner.model.Owner;
import in.koreatech.koin.domain.owner.model.OwnerAttachment;
import in.koreatech.koin.domain.owner.model.OwnerEmailRequestEvent;
Expand All @@ -31,6 +33,8 @@
@Transactional(readOnly = true)
public class OwnerService {

private static final int MIN_ATTACHMENT_COUNT = 3;

private final UserRepository userRepository;
private final OwnerInVerificationRepository ownerInVerificationRepository;
private final MailService mailService;
Expand Down Expand Up @@ -66,4 +70,35 @@ public OwnerResponse getOwner(Long ownerId) {
List<Shop> shops = shopRepository.findAllByOwnerId(ownerId);
return OwnerResponse.of(foundOwner, attachments, shops);
}

@Transactional
public OwnerResponse putOwner(Long userId, OwnerUpdateRequest request) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

C

putOwner라는 메소드 명에서 어떤 작업을 수행하는지 유추가 잘 안되는거같아요
updateOwner라는 네이밍도 생각나는데 어떻게 생각하나요

Owner foundOwner = ownerRepository.getById(userId);
List<OwnerAttachment> attachments = request.attachmentUrls().stream()
.map(url -> OwnerAttachment.builder()
.owner(foundOwner)
.url(url.fileUrl())
.isDeleted(false)
.build()
)
.toList();

for (OwnerAttachment attachment : attachments) {
if (ownerAttachmentRepository.findByOwnerIdAndUrl(userId, attachment.getUrl()).isPresent()) {
continue;
}
Comment on lines +86 to +89
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

R

이미 존재하는 이미지는 업데이트 안하는군요..
PUT이라는 메소드에 어울리는 행위일까요? PUT은 멱등성을 보장한다는 특징을 잘 생각해보시면 좋을거같아요

ownerAttachmentRepository.save(attachment);
}

List<OwnerAttachment> allAttachments = ownerAttachmentRepository.findAllByOwnerId(userId);
validateAttachments(allAttachments);
List<Shop> shops = shopRepository.findAllByOwnerId(userId);
return OwnerResponse.of(foundOwner, allAttachments, shops);
}

private void validateAttachments(List<OwnerAttachment> allAttachments) {
if (allAttachments.size() < MIN_ATTACHMENT_COUNT) {
throw OwnerAttachmentsCountException.withDetail("attachments size: " + allAttachments.size());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package in.koreatech.koin.global.exception;

public class ArgumentCountException extends RuntimeException {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

C

IllegalArgumentException과의 차이가 뭔가요?


public ArgumentCountException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,24 @@

import java.time.format.DateTimeParseException;

import in.koreatech.koin.global.auth.exception.AuthException;
import lombok.extern.slf4j.Slf4j;

import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

import in.koreatech.koin.global.auth.exception.AuthException;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@RestControllerAdvice
public class GlobalExceptionHandler {

@ExceptionHandler
public ResponseEntity<ErrorResponse> handleMethodArgumentNotValidException(MethodArgumentNotValidException e) {
log.warn(e.getMessage());
return ResponseEntity.badRequest().body(ErrorResponse.from(e.getMessage()));
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
.body(ErrorResponse.from("요청 파라미터가 잘못되었습니다."));
Comment on lines -21 to +22
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

R

DTO Validation에서 걸린 에러에 대해서 메시지를 출력해주는건데 이거 바꾼 이유가 있을까요?

}

@ExceptionHandler
Expand Down Expand Up @@ -55,4 +56,11 @@ public ResponseEntity<ErrorResponse> handleDateTimeParseException(DateTimeParseE
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
.body(ErrorResponse.from("잘못된 날짜 형식입니다."));
}

@ExceptionHandler
public ResponseEntity<ErrorResponse> handleArgumentCountException(ArgumentCountException e) {
log.warn(e.getMessage());
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
.body(ErrorResponse.from("요청 파라미터가 잘못되었습니다."));
}
}
106 changes: 105 additions & 1 deletion src/test/java/in/koreatech/koin/acceptance/OwnerApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ void getOwner() {
.when()
.get("/owner")
.then()
.log().all()
.statusCode(HttpStatus.OK.value())
.extract();

Expand Down Expand Up @@ -172,4 +171,109 @@ void checkOwnerEventListener() {

Mockito.verify(ownerEventListener).onOwnerEmailRequest(event);
}

@Test
@DisplayName("사장님 인증용 이미지를 업로드한다.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

R

이미지를 업로드한다는 문장이 MultiPartImage를 업로드하는 것 처럼 느껴져요
이 경우는 이미지 URL을 갱신한다가 어울리지 않나요?

void putOwner() {
// given
User user = User.builder()
.password("1234")
.nickname("주노")
.name("최준호")
.phoneNumber("010-1234-5678")
.userType(OWNER)
.gender(UserGender.MAN)
.email("test@koreatech.ac.kr")
.isAuthed(true)
.isDeleted(false)
.build();

Owner ownerRequest = Owner.builder()
.companyRegistrationNumber("123-45-67890")
.companyRegistrationCertificateImageUrl("https://test.com/test.jpg")
.grantShop(true)
.grantEvent(true)
.user(user)
.build();

Owner owner = ownerRepository.save(ownerRequest);

Shop shopRequest = Shop.builder()
.owner(owner)
.name("테스트 상점")
.internalName("테스트")
.chosung("테스트")
.phone("010-1234-5678")
.address("대전광역시 유성구 대학로 291")
.description("테스트 상점입니다.")
.delivery(true)
.deliveryPrice(3000L)
.payCard(true)
.payBank(true)
.isDeleted(false)
.isEvent(false)
.remarks("비고")
.hit(0L)
.build();

Shop shop = shopRepository.save(shopRequest);

String token = jwtProvider.createToken(owner.getUser());

// when then
ExtractableResponse<Response> response = RestAssured
.given()
.header("Authorization", "Bearer " + token)
.body("""
{
"attachment_urls": [
{
"file_url": "https://static.koreatech.in/example/example_image1.png"
},
{
"file_url": "https://static.koreatech.in/example/example_image2.png"
},
{
"file_url": "https://static.koreatech.in/example/example_image3.png"
}
]
}
""")
.contentType(ContentType.JSON)
.when()
.put("/owner")
.then().log().all()
.statusCode(HttpStatus.OK.value())
.extract();

assertSoftly(
softly -> {
softly.assertThat(response.body().jsonPath().getString("email")).isEqualTo(user.getEmail());
softly.assertThat(response.body().jsonPath().getString("name")).isEqualTo(user.getName());
softly.assertThat(response.body().jsonPath().getString("company_number"))
.isEqualTo(owner.getCompanyRegistrationNumber());

softly.assertThat(response.body().jsonPath().getList("attachments").size()).isEqualTo(3);
softly.assertThat(response.body().jsonPath().getLong("attachments[0].id")).isNotNull();
softly.assertThat(response.body().jsonPath().getString("attachments[0].file_url"))
.isEqualTo("https://static.koreatech.in/example/example_image1.png");
softly.assertThat(response.body().jsonPath().getString("attachments[0].file_name"))
.isEqualTo("example_image1.png");
softly.assertThat(response.body().jsonPath().getLong("attachments[1].id")).isNotNull();
softly.assertThat(response.body().jsonPath().getString("attachments[1].file_url"))
.isEqualTo("https://static.koreatech.in/example/example_image2.png");
softly.assertThat(response.body().jsonPath().getString("attachments[1].file_name"))
.isEqualTo("example_image2.png");
softly.assertThat(response.body().jsonPath().getLong("attachments[2].id")).isNotNull();
softly.assertThat(response.body().jsonPath().getString("attachments[2].file_url"))
.isEqualTo("https://static.koreatech.in/example/example_image3.png");
softly.assertThat(response.body().jsonPath().getString("attachments[2].file_name"))
.isEqualTo("example_image3.png");

softly.assertThat(response.body().jsonPath().getList("shops").size()).isEqualTo(1);
softly.assertThat(response.body().jsonPath().getLong("shops[0].id")).isEqualTo(shop.getId());
softly.assertThat(response.body().jsonPath().getString("shops[0].name")).isEqualTo(shop.getName());
}
);
}
}