-
Notifications
You must be signed in to change notification settings - Fork 8
게시글 API 구현 #50
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
게시글 API 구현 #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다
사소한 부분들에 대해서 코멘트 드렸습니다.
그리고 조회수 동시성 이슈 해결 과정을 공유해주셔서 감사합니다🙇🏻♀️
추가로 지금 yml 불일치 때문에 제 로컬에서는 어플리케이션이 돌아가지 않더라고요! 깃허브 서브모듈을 사용하는 방법으로 yml 공유를 해보면 좋을 것 같습니다. 그런데 제가 지금 돌아가는 yml 을 가지고 있지 않아서요😓 세원님이 yml 보내주시면 제가 설정해보려 하는데 괜찮으신가요? yml 수정에 따라서 실행이 계속 안되는 것보다, 한번 설정하고 계속 사용하는게 더 좋을 것 같습니다.
아니면 더 좋은 방법이 있으시다면 편하게 말씀해주세요!
| accessibleCodeList.add("EUROPE"); | ||
| accessibleCodeList.add("AMERICAS"); | ||
| accessibleCodeList.add("ASIA"); | ||
| accessibleCodeList.add("FREE"); | ||
| return ResponseEntity.ok().body(accessibleCodeList); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 의도적으로 BoardCode 와 다른 값이 생길 수 있게 만드신건가요? 👀
아니라면 BoardCode 안에서 자신의 values() 를 리스트로 리턴하게 하는 함수를 만드는게 더 좋을 것 같아요! 지금 코드에서는 두 곳 중 하나에 변경이 일어날 때 다른 쪽도 변경해줘야 하는데, 이때 불일치가 생길수도 있을 것 같아서요😓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.values() 사용하여 수정하였습니다.
아직까지는 권한이 나누어져있지 않아서, 하드코딩했던 것 같습니다ㅜ
너무 안일했던 것 같네요ㅜ 꼼꼼히 봐주셔서 감사합니다!
| public ResponseEntity<?> findPostsByCodeAndCategory( | ||
| @PathVariable(value = "code") String code, | ||
| @RequestParam(value = "category", defaultValue = "전체") String category) { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 이것은 단순 궁금증 ㅎㅎ
함수 안에서 개행은 어떤 규칙으로 하시는지 궁금합니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
별도의 규칙을 따르면서 작성하지는 않았습니다.
컨트롤러에서 받는 파라미터가 많아짐에 따라서 개행하였고,
가독성을 위해서 컨트롤러의 다른 함수들도 모두 파라미터별로 개행하였습니다.
| @Query(value = """ | ||
| WITH RECURSIVE CommentTree AS ( | ||
| SELECT | ||
| id, parent_id, post_id, site_user_id, content, | ||
| created_at, updated_at, | ||
| 0 AS level, CAST(id AS CHAR(255)) AS path | ||
| FROM comment | ||
| WHERE post_id = :postId AND parent_id IS NULL | ||
| UNION ALL | ||
| SELECT | ||
| c.id, c.parent_id, c.post_id, c.site_user_id, c.content, | ||
| c.created_at, c.updated_at, | ||
| ct.level + 1, CONCAT(ct.path, '->', c.id) | ||
| FROM comment c | ||
| INNER JOIN CommentTree ct ON c.parent_id = ct.id | ||
| ) | ||
| SELECT * FROM CommentTree | ||
| ORDER BY path | ||
| """, nativeQuery = true) | ||
| List<Comment> findCommentTreeByPostId(@Param("postId") Long postId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JPQL 공부 열심히 하셨네요👍🔥
| private Boolean getIsOwner(Comment comment, String email) { | ||
| return comment.getSiteUser().getEmail().equals(email); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이것은 아주아주아주 사소한 부분이긴 하지만,.. 🥹
boolean 을 반환하는 함수의 이름은 보통 is / has / can 으로 시작하는 것이 일반적이라 조심스레 말씀드려봅니다!
https://docs.oracle.com/javase/tutorial/datetime/overview/naming.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isOwner로 함수명 수정하였습니다!
해당 문서를 기준으로 함수명 작성하면 좋을 것 같네요!
| @Bean(name = "incrViewCountScript") | ||
| public RedisScript<Long> incrViewCountLuaScript() { | ||
| Resource scriptSource = new ClassPathResource("scripts/incrViewCount.lua"); | ||
| return RedisScript.of(scriptSource, Long.class); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
레디스를 이용해서 동시성 고려를 하시다니😮👏
좋은 방법 같습니다!
| this.post.getPostImageList().remove(this); | ||
| } | ||
| this.post = post; | ||
| post.getPostImageList().add(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
양방향 싱크 맞춰주는 것👏👏
| public record PostUpdateRequest( | ||
| @Getter | ||
| String postCategory, | ||
| @Getter | ||
| String title, | ||
| @Getter | ||
| String content | ||
| ) { | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
record 에서는 getter 를 자동으로 생성해줘서 롬복 안쓰셔도 됩니다.
getter가 .{필드명}() 이렇게 생성되는 걸로 알아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getter 없애고, 말씀하신 방식으로 수정하였습니다.
|
피드백 반영하여 리팩토링 진행하였습니다. |
관련 이슈
작업 내용
게시글 관련된 API 구현하였습니다.
테스트코드 작성하였습니다.
모든 테스트 정상동작 합니다.

특이 사항
추후 확장성 고려하여 게시글에 조회수 필드 추가하였습니다.
게시판 조회수 동시성 고찰.docx
게시글 수정시에는 우선 모든 사진을 삭제하고, 새로 등록하도록 하였습니다.
리뷰 요구사항 (선택)
DTO 명명규칙을 정하면 좋을 것 같습니다.
ㅅ..살려줘..