-
Notifications
You must be signed in to change notification settings - Fork 0
[DP-499] 알림 생성, 알림 전송 개발 #154
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
Conversation
- 알림 수신 컨트롤러 개발 및 테스트 코드 작성
- 알림 수신 컨트롤러 개발 및 테스트 코드 작성
- 테스트 코드 돌리는데 메모리가 부족해서 늘림
# Conflicts: # src/docs/asciidoc/api/notification/notification.adoc # src/main/java/com/dreamypatisiel/devdevdev/domain/entity/Notification.java # src/main/java/com/dreamypatisiel/devdevdev/domain/repository/notification/NotificationRepository.java # src/main/java/com/dreamypatisiel/devdevdev/domain/service/notification/NotificationService.java # src/main/java/com/dreamypatisiel/devdevdev/web/controller/notification/NotificationController.java # src/main/java/com/dreamypatisiel/devdevdev/web/controller/subscription/SubscriptionController.java # src/test/java/com/dreamypatisiel/devdevdev/domain/service/notification/NotificationServiceTest.java # src/test/java/com/dreamypatisiel/devdevdev/web/controller/notification/NotificationControllerTest.java # src/test/java/com/dreamypatisiel/devdevdev/web/docs/NotificationControllerDocsTest.java
- jwt 이용하는 것에서 api-key 를 사용하는 것으로 변경
yu-so-young2
left a comment
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.
수고하셨습니다. 늦어서 죄송합니다.
src/main/java/com/dreamypatisiel/devdevdev/domain/service/notification/NotificationService.java
Outdated
Show resolved
Hide resolved
| // 구독한 기업에 대해서 새로운 글 알림이 존재하지 않은 회원 추출 | ||
| Set<Member> membersWithoutNotifications = members.stream() | ||
| .filter(member -> findSubscriptionNotifications.stream() | ||
| .noneMatch(notification -> notification.isEqualsMember(member))) |
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.
techArticleIds = [1,2,3]에 대하여 member A 가 알림이 한 개라도 생성되어 있다면 이 Set 에서 제외되는 것 같아요!
member A가 techArticleId=1 알림을 갖고 있더라도 techArticleId=2,3 으로 생성된 알림이 없다면 새로 만들어주어야 할 것 같은데, 혹시 제가 놓친 부분이 있을까요?
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.
member 에 대해서 1개의 techArticle 에 대해서 notification 테이블에 이력을 남기는 것은 맞는데, sse 알림은 1개의 company 가 여러개의 techArticle 이 발행되어도 1개의 알림만 생성하도록 정했어요.!
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.
아하 이해했습니다!! 설명 감사합니다!
| "/devdevdev/api/v1/test", | ||
| "/devdevdev/api/v1/token" | ||
| "/devdevdev/api/v1/token", | ||
| "/devdevdev/api/v1/notifications/SUBSCRIPTION" |
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.
DEV_WHITELIST_URL 화이트리스트에 /notifications/** 을 등록해두었으므로, JWT 필터를 거치지 않을텐데 DEV_JWT_FILTER_WHITELIST_URL 에도 추가하신 이유가 있을까요?! (제가 잘못 알고 있는 거라면 정정 부탁드립니다!)
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.
좋은 질문 감사합니다. 소영님!
저희는 xxJwtAuthenticationFilter 클래스를 통해서 직접 Authentication(인증) 를 설정하고 있는데요.
Authentication authenticationToken = tokenService.createAuthenticationByToken(accessToken);
SecurityContextHolder.getContext().setAuthentication(authenticationToken);DEV_WHITELIST_URL 에 등록한 url 들은 쉽게 이야기 하면 spring security 에서 해당 url 은 anonymous 를 허용한다는 의미입니다.
하지만, 그렇다고 해서 xxJwtAuthenticationFilter를 거치지 않는다는 뜻은 아닙니다.
xxJwtAuthenticationFilter는 기본적으로 모든 요청에 대해 항상 먼저 실행됩니다!
(단, shouldNotFilter() 조건에 따라 예외적으로 스킵되는 경우는 별도입니다.)
정리해볼께요!
- 인증(Authentication): "이 사람 누구야?"를 확인하는 것 (→ Authentication 객체 만들기)
- 인가(Authorization): "이 사람이 이 요청을 해도 돼?"를 판단하는 것 (→ 권한/역할 검사)
| 구분 | 설명 |
|---|---|
| DevJwtAuthenticationFilter | 요청 초반에 토큰을 파싱해서 SecurityContext에 Authentication을 세팅하는 역할 (인증 Authentication 단계) |
| SecurityFilterChain.authorizeHttpRequests() | 요청 말단에 Controller 접근 허용 여부를 판단하는 역할 (인가 Authorization 단계) |
[현재 devdevdev core의 스프링 시큐리티 흐름]
[DevJwtAuthenticationFilter] ← JWT AccessToken 검증 및 SecurityContext Authentication 세팅
↓
[SecurityExceptionFilter] ← 인증/인가 에러 잡아서 Response로 변환 (Exception to Response)
↓
[LimiterFilter] ← Rate Limiting, Dos 방어 등 요청 제한
↓
[AuthorizeHttpRequests] ← Controller 접근 허용/차단 판단 (permitAll(), hasRole() 등 체크)
해당 자원에 대해서 회원만 가능하면, DEV_WHITELIST_URL, DEV_JWT_FILTER_WHITELIST_URL에도 등록하지 않는다.
해당 자원에 대해서 모두 가능하다면, DEV_WHITELIST_URL, DEV_JWT_FILTER_WHITELIST_URL에도 등록하지 않는다.
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.
아하 제가 착각하고 있었네요 명확히 정리해주셔서 감사합니다!!
📝 DEV_WHITELIST_URL은 authorizeHttpRequests 단계에서 permitAll 처리를 해주는 거고,
JWT 필터는 그 이전에 이미 실행되기 때문에, 해당 URL에 대해 JWT 파싱 자체를 생략하려면
별도로 DEV_JWT_FILTER_WHITELIST_URL에도 추가해줘야 한다.
flowchart TD
A[Client Request] --> B[DevJwtAuthenticationFilter]
B -->|shouldNotFilter is true| C[Skip JWT Filter]
B -->|shouldNotFilter is false| D[Parse JWT and set Authentication]
C --> E[SecurityExceptionFilter]
D --> E
E --> F[LimiterFilter]
F --> G[AuthorizeHttpRequests]
G --> H{Access Allowed?}
H -->|Yes| I[Controller Reached]
H -->|No| J[Access Denied]
src/main/java/com/dreamypatisiel/devdevdev/domain/entity/enums/NotificationType.java
Outdated
Show resolved
Hide resolved
|
|
||
| List<Notification> findAllByMemberId(Long memberId); | ||
|
|
||
| List<Notification> findByMemberInAndTechArticleIdInOrderByMemberDesc(Set<Member> members, Set<Long> techArticleIds); |
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.
orderBy 조건이 들어가는 이유가 있을까요?
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.
음 큰 이유는 없던거 같아요!
쿼리 성능을 고려하면 명시적으로 order by null 을 사용하는 것도 좋아보이네요.
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.
넵 OrderByMember 는 정렬 과정에서 불필요한 부하가 생길 것 같아요. 말씀하신대로 Order By null 로 정렬을 생략하면 좋을 것 같아요!
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.
말씀 주신 내용 반영해서 아래로 코드 수정했습니다!
@Override
public List<Notification> findByMemberInAndTechArticleIdInOrderByNull(Set<Member> members, Set<Long> techArticleIds) {
return query.selectFrom(notification)
.where(notification.member.in(members)
.and(notification.techArticle.id.in(techArticleIds)))
.orderBy(stringTemplate("null").asc())
.fetch();
}
📝 작업 내용
🔗 참고할만한 자료(선택)
💬 리뷰 요구사항(선택)