fcmToken 등록 및 업데이트 api 구현 #74

13wjdgk · 2023-10-30T12:29:19Z

Changes 📝

fcmToken 등록 및 업데이트 api 구현

Details 🌼

기존의 fcmToken 등록 방법을 변경하였습니다
- 로그인,회원가입할 때 fcmToken을 등록하는 로직을 삭제하고, 한개의 api에서 관리할 수 있도록 하였습니다.
- 유저 아이디와 디바이스 아이디, fcmToken으로 DB에 저장합니다.
- 새로운 fcmToken이라면 새롭게 저장하고, 기존에 존재하는 fcmToken이라면 업데이트합니다.

Check List ☑️

테스트 코드를 통과했다.
merge할 브랜치의 위치를 확인했다. (main ❌)
Assignee를 지정했다.
Label을 지정했다.

Etc

2달 전에 업데이트 된 fcmToken이라면 제거하는 로직이 필요합니다

GIVEN53

fcm 토큰 등록하는 로직을 하나의 api로 통합하셨는데 이전의 구현 로직(회원가입, 로그인)에서 제거된 커밋이 보이지 않는 것 같아요 확인부탁드립니다
그리고 토큰 삭제는 이전의 로직을 그대로 사용하나요? (로그아웃 api에서 토큰을 헤더로 받아서 삭제 처리하는)

GIVEN53 · 2023-10-31T04:09:17Z

src/test/resources/schema.sql~HEAD

파일 지워주세요!

GIVEN53 · 2023-10-31T04:10:53Z

src/main/java/com/coniverse/dangjang/domain/notification/mapper/NotificationMapper.java

 	})
-	UserFcmToken toEntity(User user, String fcmToken, LocalDate createdAt);
+	UserFcmToken toEntity(User user, String fcmToken, String device_id);


P3) camel case로 변경 부탁드립니당

수정하겠습니다 !!

GIVEN53 · 2023-10-31T04:13:14Z

src/main/java/com/coniverse/dangjang/domain/notification/entity/UserFcmToken.java

-	private String fcmToken;
+public class UserFcmToken extends BaseEntity {
+	@EmbeddedId
+	private FcmId fcmId;


P3) fcmId를 getter로 노출시키지 않으려면 처리가 필요할 것 같아요

Suggested change

private FcmId fcmId;

@Getter(AccessLevel.PRIVATE)

private FcmId fcmId;

그렇네요 ! 수정하겠습니다 :)

GIVEN53 · 2023-10-31T04:21:41Z

src/main/java/com/coniverse/dangjang/domain/notification/service/NotificationService.java

 		User user = userSearchService.findUserByOauthId(oauthId);
-		userFcmTokenRepository.save(notificationMapper.toEntity(user, fcmToken, LocalDate.now()));
+		UserFcmToken userFcmToken = notificationMapper.toEntity(user, request.fcmToken(), request.deviceId());
+		Optional<UserFcmToken> exitsFcmToken = userFcmTokenRepository.findUserFcmTokenByFcmId(oauthId, request.deviceId());


exists의 오타라서 존재하는 토큰이라는 의미인지 종료된 토큰이라는 의미인지 불분명한 것 같아요 어떤 의미의 변수명인가요?

존재하는 토큰입니다 ! ㅜ 오타 수정하겠습니다 !

GIVEN53 · 2023-10-31T04:22:52Z

src/main/java/com/coniverse/dangjang/domain/notification/service/NotificationService.java

+		UserFcmToken userFcmToken = notificationMapper.toEntity(user, request.fcmToken(), request.deviceId());
+		Optional<UserFcmToken> exitsFcmToken = userFcmTokenRepository.findUserFcmTokenByFcmId(oauthId, request.deviceId());
+		if (exitsFcmToken.isEmpty()) {
+			userFcmTokenRepository.save(userFcmToken);


P3) return;을 사용해서 else문을 제거해보는 건 어떨까요??

GIVEN53 · 2023-10-31T04:26:49Z

src/main/java/com/coniverse/dangjang/domain/notification/service/NotificationService.java

 	 * @since 1.1.0
 	 */
-	public void saveFcmToken(String fcmToken, String oauthId) {
+	public void saveFcmToken(FcmTokenRequest request, String oauthId) {


P3) 기존의 fcm 토큰을 업데이트하는 로직을 먼저 실행하면 불필요한 user 조회 쿼리를 줄일 수 있을 것 같아요 그리고 두 개의 책임을 지고 있는데 리팩토링할 때 메서드를 분리해보면 어떨까 싶습니다!

if(fcmToken.isPresent()) { 토큰 업데이트(); return; } 새로운 토큰 저장();

기존에 존재하는 userFcmToken을 조회하기 위해 oauthId에 대한 검증이 필요하다고 생각했지만, oauthId는 이미 filter에서 검증된 유저임으로 조회가 필요 없을 것 같군요..! 수정하겠습니다 👍

GIVEN53 · 2023-10-31T04:31:41Z

src/main/resources/schema-dev.sql

-    `CREATED_AT` date         NOT NULL,
-    PRIMARY KEY (`FCM_TOKEN`),
+    `DEVICE_ID`  varchar(255) NOT NULL,
+    `CREATED_AT` date     NOT NULL,


BaseEntity 클래스는 LocalDateTime 타입의 필드를 제공하도록 구현해두었어요 date 타입으로 스키마를 작성하신 이유가 있을까요??

재사용을 위해 기존의 BaseEntity를 사용하긴 했지만 userFcmToken에서는 date만 필요하기 때문에, time은 불필요하다고 생각했습니다

GIVEN53 · 2023-10-31T04:34:01Z

src/main/java/com/coniverse/dangjang/domain/notification/entity/UserFcmToken.java

-public class UserFcmToken {
-	@Id
-	private String fcmToken;
+public class UserFcmToken extends BaseEntity {


Persistable 인터페이스 구현하지 않아도 save할 때 잘 작동하나요??

persistable을 통해 새로운 엔티티 여부를 판별하지 않고, select를 통해 새로운 엔티티를 판별합니다.
업데이트 시, 기존에 조회된 엔티티가 있을 경우, 더티 체킹을 통해 변경된 부분을 업데이트하기 때문에 잘 작동하고 있습니다 !

13wjdgk · 2023-10-31T11:38:53Z

fcm 토큰 등록하는 로직을 하나의 api로 통합하셨는데 이전의 구현 로직(회원가입, 로그인)에서 제거된 커밋이 보이지 않는 것 같아요 확인부탁드립니다 그리고 토큰 삭제는 이전의 로직을 그대로 사용하나요? (로그아웃 api에서 토큰을 헤더로 받아서 삭제 처리하는)

기존 커밋에서 로그인, 회원가입 Service에 fcmToken 등록 메서드를 제거하는 부분을 확인하실 수 있습니다 !
그리고 로그인, 회원가입 컨트롤러에서 fcmToken header를 제거한 커밋이 이전에 커밋 롤백하면서 반영되지 않은 것 같아 다시 수정해서 올렸습니다! 😢

또한 토큰 삭제는 이전과 동일하게 로그아웃을 통해 삭제되지만 header 대신 body로 받을 수 있게 수정하였습니다 !

coniverse-github-app · 2023-10-31T11:39:43Z

Analysis Details

0 Issues

0 Bugs
0 Vulnerabilities
0 Code Smells

Coverage and Duplications

94.44% Coverage (96.60% Estimated after merge)
0.00% Duplicated Code (0.80% Estimated after merge)

Project ID: co-niverse_dangjang-backend_AYj2jZJELehUZAlqDvRk

View in SonarQube

GIVEN53

LGTM~

13wjdgk added 3 commits October 30, 2023 19:25

feat : fcmToken 추가 api 구현

fad4160

test : fcmToken 추가 api test

3451357

merge : fcmToken 업데이트 기능 dev merge

a01ce76

13wjdgk added the feature 기능 label Oct 30, 2023

13wjdgk self-assigned this Oct 30, 2023

This comment has been minimized.

Sign in to view

13wjdgk added 2 commits October 31, 2023 07:58

refactor : fcmToken 업데이트 테스트 수정

5a91451

refactor : fcmToken 업데이트 테스트 isNotSameAs 제거

277022b

This comment has been minimized.

Sign in to view

GIVEN53 reviewed Oct 31, 2023

View reviewed changes

13wjdgk added 8 commits October 31, 2023 17:58

refactor : deviceId 카멜케이스로 변경

1ef1fbe

refactor : schema.sql~HEAD 제거

bd30fa4

refactor : saveOrUpdateFcmToken 메서드 리팩토링

f8f91ab

test : saveOrUpdateFcmToken 메서드 리팩토링에 따른 수정

c77d322

refactor : FcmId getter 접근 레벨 수정

5b88fe9

refactor : saveFcmToken,updateFcmToken 메서드 접근 제어자 private으로 변경

9458c12

refactor : 로그인, 회원가입 fcmToken 등록 제거

1aeb69c

test : 로그인,회원가입 테스트에서 fcmToken 요소 제거

0683ebf

This comment has been minimized.

Sign in to view

13wjdgk added 3 commits October 31, 2023 20:32

feat : LogoutFcmTokenRequest 생성

9cb0e0e

refactor : PostFcmTokenRequest DTO 이름 변경

dc2b5e8

refactor : 로그아웃 fcmToken header 제거 후 RequestBody로 변경

35f6998

GIVEN53 approved these changes Oct 31, 2023

View reviewed changes

GIVEN53 merged commit 6d1076a into dev Oct 31, 2023
1 check passed

GIVEN53 deleted the feat/fcmUpdate branch October 31, 2023 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fcmToken 등록 및 업데이트 api 구현 #74

fcmToken 등록 및 업데이트 api 구현 #74

13wjdgk commented Oct 30, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

GIVEN53 left a comment

GIVEN53 Oct 31, 2023

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023

GIVEN53 Oct 31, 2023

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023 •

edited

Loading

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023

GIVEN53 Oct 31, 2023

13wjdgk Oct 31, 2023

This comment has been minimized.

13wjdgk commented Oct 31, 2023

coniverse-github-app bot commented Oct 31, 2023

GIVEN53 left a comment

	private FcmId fcmId;
	@Getter(AccessLevel.PRIVATE)
	private FcmId fcmId;

fcmToken 등록 및 업데이트 api 구현 #74

fcmToken 등록 및 업데이트 api 구현 #74

Conversation

13wjdgk commented Oct 30, 2023 • edited Loading

Changes 📝

Details 🌼

Check List ☑️

Etc

This comment has been minimized.

This comment has been minimized.

GIVEN53 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

13wjdgk Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

13wjdgk commented Oct 31, 2023

coniverse-github-app bot commented Oct 31, 2023

Analysis Details

0 Issues

Coverage and Duplications

GIVEN53 left a comment

Choose a reason for hiding this comment

13wjdgk commented Oct 30, 2023 •

edited

Loading

13wjdgk Oct 31, 2023 •

edited

Loading