2024_09_02 CodeReview 신청

[gooot]

No description provided.

[yoowonyoung]

스프링시큐리티 세팅 부분

• 질문에서 구체적으로 무엇을 물어보고 싶었던걸까요? inner class 를 사용한것을 분리한것을 말한것일까요? 그렇다면 제 답변은 상관없다 에요. FailedAuthenticationEntryPoint 은 WebSecurityConfig 클래스에서만 사용되고 다른곳에서 재사용되지 않으니까 inner class로 선언하는편이 좀 더 깔끔하게 보일수 있어요

JwtToken 생성 부분

• parseBearerToken 에서, JWT 토큰이 없거나 유효하지 않는 경우가 구분이 되지 않고 있습니다. 둘다 StatusCode.NO_AUTHORIZATION 를 반환 하고 있어요. 두개가 서로 구분되면 좋을것 같아요. 401 에러와 403에러는 다르니깐요
• Exception을 왜 IOException을 사용하고 있는것일까요? 좀 더 적절한 Exception을 사용 하면 좋을것 같아요
• assert는 런타임 환경에서는 동작하지 않아요. requireNonNull같은걸 사용 해야 합니다
• 이렇게 하는 경우 매번 SecurityContext를 새로 만들지 않나요? SecurityContextHolder에 이미 선언된 SecurityContext를 사용하는 방법이 좋을것 같아요

정적 팩토리 메서드

• 이런 구조의 경우에는 사실 DTO는 껍데기로만 존재하고 내부 Request / Response가 메인인것으로 보이네요. 그럴꺼라면 ScriptAddReqeust / ScriptAddResponse로 나누어서 가져가는게 더 좋지 않을까요? 일반적으로 응답객체는 생성이 편해야 하지만, 현재 사용하는 모습을 보면 ScriptAddDto.toResponse(scriptRepository.save(scriptEntity)); 라던지 ScriptGetDto.Response.toResponse(scriptEntity); 를 보면 사용하기 편리해 보이진 않습니다. 정적 팩토리 메소드를 활용하는 방법은 맞지만 구조를 변경하면 더 좋을거 같아요

테스트 코드

• 리플렉션은 자바의 캡슐화를 꺨 수 있기 때문에 꼭 필요한 경우가 아니라면 사용을 자제 하는게 좋아요. 이와 같은 경우라면 @Value가 없다는걸 가정하면, 그 상태인데 그 클래스를 그대로 활용 하려고 하는것이 문제입니다. 그렇게 할 경우 값을 어떻게 주입 해줄 생각인것일까요? 그래서 이렇게 생각을 해보면 자연스레 생성자 or Setter에 의해서 secretKey를 주입 해야 한다는것을 알게 될텐데요. 그러면 자연스레 테스트코드에서도 생성자 혹은 Setter로 주입을 해주면 됩니다.