Ignore: 🐛🔧 Modification and refactoring of the presigned URL creation and conversion logic

[psychology50]

작업 이유

• 🐛 The presigned URL generator is invalid #178

작업 사항

1️⃣ Swagger 업그레이드 및 쿼리 수정

• 문자열에 의존하던 type을 열거 타입으로 수정했습니다.
• chatroomId 뿐 아니라, chatId와 feedId가 optional query param으로 추가되었습니다.

2️⃣ Controller 계층에서 유효성 검사 로직 추가

/**
 * {@link kr.co.pennyway.api.apis.storage.dto.PresignedUrlDto.Req}의 유효성 검사를 담당하는 Validator
 */
@Slf4j
public class PresignedUrlDtoReqValidator implements Validator {
    @Override
    public boolean supports(@NonNull Class<?> clazz) {
        return PresignedUrlDto.Req.class.isAssignableFrom(clazz);
    }

    @Override
    public void validate(@NonNull Object target, @NonNull Errors errors) {
        PresignedUrlDto.Req req = (PresignedUrlDto.Req) target;

        if (ObjectKeyType.CHATROOM_PROFILE.equals(req.type()) && req.chatroomId() == null) {
            errors.rejectValue("chatroomId", "MISSING_CHATROOM_PARAMETER", "채팅방 이미지를 위해 채팅방 ID는 필수입니다.");
        }
        if (ObjectKeyType.CHAT_PROFILE.equals(req.type()) && req.chatroomId() == null) {
            errors.rejectValue("chatroomId", "MISSING_CHAT_PROFILE_PARAMETER", "채팅 프로필 이미지를 위해 채팅방 ID는 필수입니다.");
        }
        if (ObjectKeyType.CHAT.equals(req.type())) {
            if (req.chatroomId() == null) {
                errors.rejectValue("chatroomId", "MISSING_CHAT_PARAMETER", "채팅 이미지를 위해 채팅방 ID는 필수입니다.");
            }
            if (req.chatId() == null) {
                errors.rejectValue("chatId", "MISSING_CHAT_PARAMETER", "채팅 이미지를 위해 채팅 ID는 필수입니다.");
            }
        }
        if (ObjectKeyType.FEED.equals(req.type()) && req.feedId() == null) {
            errors.rejectValue("feedId", "MISSING_FEED_PARAMETER", "피드 이미지를 위해 피드 ID는 필수입니다.");
        }
    }
}

public class StorageController implements StorageApi {
    private final StorageUseCase storageUseCase;

    @InitBinder
    protected void initBinder(WebDataBinder binder) {
        binder.addValidators(new PresignedUrlDtoReqValidator());
    }

    @Override
    @GetMapping("/presigned-url")
    @PreAuthorize("isAuthenticated()")
    public ResponseEntity<?> getPresignedUrl(@Validated PresignedUrlDto.Req request, BindingResult bindingResult, @AuthenticationPrincipal SecurityUserDetails user) {
        if (bindingResult.hasErrors()) {
            throw new CustomValidationException(bindingResult);
        }

        ...
    }
}

• chatId, chatroomId, feedId를 클라이언트가 전달해줘야 하는 type인 경우, 확실하게 예외를 반환하도록 수정했습니다.
• 생성자에서 예외 처리를 하면, BeanResolveException인가? 뭐, 이상한 예외가 감싸지는 바람에 위와 같이 추가 validation 체크 로직을 반영했습니다.

3️⃣ 추상 팩토리 패턴 도입

굳이 따지자면, Simple Factory Pattern에 가까운..

public interface PresignedUrlProperty {
    String imageId();

    String timestamp();

    String ext();

    ObjectKeyType type();

    /**
     * Presigned URL 생성을 위한 변수들을 반환한다.
     * key는 각 변수명을 lowerCamelCase로 변환한 것이다.
     */
    Map<String, String> variables();
}

우선, 각 Type이 공통적으로 갖는 필드들을 필수적으로 포함하도록 강제하는 interface를 사용합니다.
Profile, Chat, ChatRoom 등의 UrlProperty는 해당 인터페이스를 구현하고, 필요한 필드를 추가하면 됩니다.

PresignedUrlPropertyFactory factory = PresignedUrlPropertyFactory.create(request.ext(), request.type())
                .userId(userId)
                .chatroomId(request.chatroomId())
                .chatId(request.chatId())
                .feedId(request.feedId())
                .build();

이러한 PresignedUrlProperty는 PresignedUrlPropertyFactory를 사용하여 생성 가능합니다.

4️⃣ presigned url 생성 로직 책임 재분배

public final class UrlGenerator {
    ...

    /**
     * S3에 임시 업로드할 파일의 URL을 생성한다.
     *
     * @param property {@link PresignedUrlProperty}: Presigned URL 생성을 위한 Property
     * @return Presigned URL
     */
    public static String createDeleteUrl(PresignedUrlProperty property) {
        return applyTemplate(property.type().getDeleteTemplate(), property.variables());
    }

    /**
     * 임시 경로에서 실제 경로로 파일을 이동시키기 위한 URL을 생성한다.
     *
     * @param type      {@link ObjectKeyType}
     * @param deleteUrl 임시 경로의 URL
     * @return Presigned URL
     */
    public static String convertDeleteToOriginUrl(ObjectKeyType type, String deleteUrl) {
        Map<String, String> variables = extractVariables(type, deleteUrl);
        return applyTemplate(type.getOriginTemplate(), variables);
    }

    ...
}

• 기존에 무분별하게 흩어져있던 Url 생성 로직들을 한 곳으로 모으고, UrlGenrator가 이름 그대로의 역할을 수행할 수 있도록 수정했습니다.
• 관리해야 할 파일이 훨씬 줄었으며, 잘못된 인터페이스 구현 사례를 제거하고, enum 클래스가 파싱 로직을 수행하는 등 책임과 관심사 분리 실패 로직을 모두 올바르게 처리할 수 있게 되었습니다.

5️⃣ 테스트 케이스 개선

public class PresignedUrlPropertyFactoryTest {
    ...

    @ParameterizedTest
    @EnumSource(ObjectKeyType.class)
    @DisplayName("모든 타입에 대해 PresignedUrlProperty를 생성한다.")
    void createPropertyAllTypes(ObjectKeyType type) {
        PresignedUrlPropertyFactory factory = createValidFactory(type);
        PresignedUrlProperty property = factory.getProperty();

        assertNotNull(property);
        assertEquals(type, property.type());
        assertNotNull(property.imageId());
        assertNotNull(property.timestamp());
        assertEquals("jpg", property.ext());
    }

    ...
}

public class UrlGeneratorTest {
    ...

    @ParameterizedTest
    @EnumSource(ObjectKeyType.class)
    @DisplayName("모든 타입에 대해 임시 저장 URL을 원본 URL로 변환한다.")
    void convertDeleteToOriginUrlAllTypes(ObjectKeyType type) {
        PresignedUrlProperty property = createDummyProperty(type);
        String deleteUrl = UrlGenerator.createDeleteUrl(property);
        String originUrl = UrlGenerator.convertDeleteToOriginUrl(type, deleteUrl);

        String expectedOriginUrl = createExpectedOriginUrl(type, property);
        assertEquals(expectedOriginUrl, originUrl, "For type: " + type);
    }

    ...
}

똑같은 문제가 재발하지 않음을 보장하기 위해 테스트 케이스를 보강했습니다.
특히 PropertyFactory와 UrlGenrator에 대한 테스트를 꼼꼼하게 수행하여, 정상 동작을 보장하고 있습니다.

리뷰어가 중점적으로 확인해야 하는 부분

• 없음

발견한 이슈

• 없음