Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-actuator'

// Firebase Admin SDK
implementation 'com.google.firebase:firebase-admin:9.7.0'
implementation("com.google.firebase:firebase-admin:9.7.0") {
exclude group: "commons-logging", module: "commons-logging"
}

// Spring Security
implementation 'org.springframework.boot:spring-boot-starter-security'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
import org.springframework.scheduling.annotation.EnableScheduling;

@SpringBootApplication
@EnableScheduling
@EnableJpaAuditing
public class PinitNotificationApplication {

public static void main(String[] args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

public interface PushService {
String getVapidPublicKey();
void subscribe(Long memberId, String token);
void unsubscribe(Long memberId, String token);

void subscribe(Long memberId, String deviceId, String token);

void unsubscribe(Long memberId, String deviceId, String token);
void sendPushMessage(String token, Notification notification);

boolean isSubscribed(Long memberId, String deviceId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,48 @@

import jakarta.persistence.*;
import lombok.Getter;
import org.springframework.data.annotation.LastModifiedDate;
import org.springframework.data.jpa.domain.support.AuditingEntityListener;

import java.time.Instant;

@Entity
@Table(
uniqueConstraints = {
@UniqueConstraint(
name = "uk_token_memberId",
columnNames = {"member_id", "token"}
name = "uk_deviceId_memberId",
columnNames = {"member_id", "device_id"}
)
}
)
@EntityListeners(AuditingEntityListener.class)
@Getter
public class PushSubscription {
@Id
@GeneratedValue
private Long id;

@Column(name="token", nullable = false)
private String token;
@Column(name="member_id", nullable = false)
@Column(name = "member_id", nullable = false)
private Long memberId;
@Column(name = "device_id", nullable = false)
private String deviceId;
@Column(name = "token", nullable = false)
private String token;
@LastModifiedDate
@Column(name = "modified_at", nullable = false)
private Instant modifiedAt;
protected PushSubscription() {}
public PushSubscription(Long memberId, String token) {

public PushSubscription(Long memberId, String deviceId, String token) {
this.memberId = memberId;
this.deviceId = deviceId;
this.token = token;
}
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: PushSubscription 엔티티에 토큰을 업데이트할 수 있는 setter 메서드가 없습니다.

영향: FCM 토큰이 갱신되는 경우 기존 구독의 토큰을 업데이트할 수 없어, 잘못된 토큰으로 푸시 알림을 전송하게 됩니다.

수정 제안: 토큰을 업데이트할 수 있는 public 메서드(예: updateToken(String token))를 추가하세요.

Suggested change
}
}
public void updateToken(String token) {
this.token = token;
}

Copilot uses AI. Check for mistakes.

public void updateToken(String token) {
if (this.modifiedAt.isAfter(Instant.now())) {
return;
}
this.token = token;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;
import java.util.Optional;

public interface PushSubscriptionRepository extends JpaRepository<PushSubscription, Long> {
Optional<PushSubscription> findByMemberIdAndDeviceId(Long memberId, String deviceId);

List<PushSubscription> findAllByMemberId(Long memberId);

void deleteByToken(String token);

void deleteByMemberIdAndDeviceId(Long memberId, String deviceId);
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: @Transactional 어노테이션이 deleteByMemberIdAndDeviceId 메서드 호출을 포함하는 unsubscribe 메서드에 필요하지만, repository의 커스텀 delete 메서드는 트랜잭션 내에서 실행되어야 합니다.

영향: delete 메서드가 제대로 실행되지 않을 수 있으며, "No EntityManager with actual transaction available" 예외가 발생할 수 있습니다.

수정 제안: PushSubscriptionRepository의 deleteByMemberIdAndDeviceId 메서드에 @Transactional 어노테이션을 추가하거나, 또는 FcmService의 unsubscribe 메서드가 이미 @transactional이 있으므로 정상 작동할 것입니다. 하지만 repository 인터페이스에서 delete 메서드에 @Modifying 어노테이션 추가를 고려하세요.

Copilot uses AI. Check for mistakes.
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Optional;

@Slf4j
@Service
public class FcmService implements PushService {
Expand Down Expand Up @@ -44,20 +46,31 @@ public void sendPushMessage(String token, Notification notification) {
}
}

@Override
public boolean isSubscribed(Long memberId, String deviceId) {
return pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId).isPresent();
}
Comment on lines +49 to +52
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: 새로 추가된 기능들(isSubscribed, subscribe with deviceId, unsubscribe with deviceId)에 대한 테스트 커버리지가 없습니다. 프로젝트에는 다른 서비스 로직에 대한 테스트가 있습니다(ScheduleNotificationServiceTest 등).

영향: 코드 변경이 정상적으로 작동하는지 검증되지 않아, 프로덕션에서 문제가 발생할 수 있습니다.

수정 제안: FcmService의 새로운 메서드들에 대한 단위 테스트를 추가하세요. 특히 다음 시나리오를 테스트하세요:

  1. isSubscribed가 구독 존재 시 true, 미존재 시 false 반환
  2. subscribe 시 동일 deviceId로 재구독하는 경우
  3. unsubscribe 시 정상 삭제

Copilot uses AI. Check for mistakes.

@Override
public String getVapidPublicKey() {
return vapidPublicKey;
}

@Override
@Transactional
public void subscribe(Long memberId, String token) {
pushSubscriptionRepository.save(new PushSubscription(memberId, token));
public void subscribe(Long memberId, String deviceId, String token) {
Optional<PushSubscription> byMemberIdAndDeviceId = pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId);
if (byMemberIdAndDeviceId.isPresent()) {
PushSubscription existingSubscription = byMemberIdAndDeviceId.get();
existingSubscription.updateToken(token);
} else {
pushSubscriptionRepository.save(new PushSubscription(memberId, deviceId, token));
}
}

@Override
@Transactional
public void unsubscribe(Long memberId, String token) {
pushSubscriptionRepository.delete(new PushSubscription(memberId, token));
public void unsubscribe(Long memberId, String deviceId, String token) {
pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId);
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: unsubscribe 메서드의 token 파라미터가 실제로 사용되지 않습니다. 메서드는 memberId와 deviceId만으로 삭제를 수행합니다.

영향: API 설계가 불명확하고, 클라이언트가 불필요한 token 값을 전송해야 합니다.

수정 제안: token 파라미터를 제거하고 memberId와 deviceId만 받도록 메서드 시그니처를 변경하거나, token도 함께 검증하는 로직을 추가하세요.

Suggested change
pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId);
pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId)
.ifPresent(subscription -> {
if (token != null && token.equals(subscription.getToken())) {
pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId);
} else {
log.warn("unsubscribe 요청의 토큰이 저장된 값과 일치하지 않습니다. memberId={}, deviceId={}", memberId, deviceId);
}
});

Copilot uses AI. Check for mistakes.
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
package me.pinitnotification.interfaces.notification;

import me.pinitnotification.application.push.PushService;
import me.pinitnotification.domain.member.MemberId;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.tags.Tag;
import me.pinitnotification.application.push.PushService;
import me.pinitnotification.domain.member.MemberId;
import me.pinitnotification.interfaces.notification.dto.PushTokenRequest;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: 와일드카드 import (import org.springframework.web.bind.annotation.*;)는 여러 클래스를 임포트하여 네임스페이스를 오염시킵니다.

영향: 코드 가독성이 저하되고, 실제로 어떤 클래스들이 사용되는지 파악하기 어렵습니다.

수정 제안: 사용되는 각 클래스를 명시적으로 import하세요 (GetMapping, PostMapping, RequestBody, RequestMapping, RequestParam, RestController).

Suggested change
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

Copilot uses AI. Check for mistakes.

@RestController
@RequestMapping("/push")
Expand All @@ -39,6 +35,21 @@ public String getVapidPublicKey() {
return pushService.getVapidPublicKey();
}

@GetMapping("/subscribed")
@Operation(
summary = "푸시 구독 상태 조회",
description = "인증된 회원이 푸시 알림을 구독 중인지 여부를 반환합니다."
)
@ApiResponses({
@ApiResponse(responseCode = "200", description = "구독 상태 조회 완료",
content = @Content(mediaType = "text/plain", schema = @Schema(implementation = Boolean.class)))
})
public boolean isSubscribed(
@Parameter(hidden = true) @MemberId Long memberId,
@RequestParam String deviceId) {
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: isSubscribed API의 deviceId 파라미터에 대한 검증이 없습니다.

영향: null 또는 빈 값이 전달되면 데이터베이스 쿼리가 예상치 못한 결과를 반환할 수 있습니다.

수정 제안: @RequestParam@notblank 어노테이션을 추가하여 빈 값을 방지하세요.

Copilot uses AI. Check for mistakes.
return pushService.isSubscribed(memberId, deviceId);
}

@PostMapping("/subscribe")
@Operation(
summary = "푸시 토큰 구독 등록",
Expand All @@ -51,7 +62,7 @@ public String getVapidPublicKey() {
public void subscribe(
@Parameter(hidden = true) @MemberId Long memberId,
@RequestBody PushTokenRequest request) {
pushService.subscribe(memberId, request.token());
pushService.subscribe(memberId, request.deviceId(), request.token());
}

@PostMapping("/unsubscribe")
Expand All @@ -66,7 +77,7 @@ public void subscribe(
public void unsubscribe(
@Parameter(hidden = true) @MemberId Long memberId,
@RequestBody PushTokenRequest request) {
pushService.unsubscribe(memberId, request.token());
pushService.unsubscribe(memberId, request.deviceId(), request.token());
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

@Schema(description = "푸시 토큰 요청 바디")
public record PushTokenRequest(
@Schema(description = "사용자의 디바이스 식별자. UUID 형식으로 제공됩니다.", example = "123e4567-e89b-12d3-a456-426614174000")
String deviceId,
Comment on lines +7 to +8
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: deviceId가 null일 수 있지만 이에 대한 검증이 없습니다.

영향: null deviceId로 API 요청 시 데이터베이스 제약 조건 위반으로 예외가 발생합니다.

수정 제안: @notblank 또는 @NotNull 어노테이션을 추가하여 null 또는 빈 값을 방지하세요.

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +8
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: deviceId에 대한 입력 검증이 없습니다. 설명에는 UUID 형식이라고 명시되어 있지만, 실제로 형식을 검증하지 않습니다.

영향: 잘못된 형식의 deviceId가 저장되어 데이터 일관성이 깨질 수 있습니다.

수정 제안: UUID 형식인지 검증하는 validation 어노테이션(예: @pattern 또는 커스텀 validator)을 추가하세요.

Copilot uses AI. Check for mistakes.
@Schema(description = "클라이언트에서 발급받은 FCM 푸시 토큰", example = "fcm-token-example")
String token
) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

vapid:
keys:
public: BA7L1jqFAyxo_KZZZBkUeqC1uY6iSP1NvyNVqV-L_dmqEnHA__5cNs87WJ5QDiH8UC3hgLQu99mbbxlwZZf7J4U
public: BF8QQIULasLr94n0l0xbv43yZeNICudM5lpQN08VYn2g5VjBPU0wM98HypyRmEb-y0ARRsiZ_wcgSMIC-nq-x20
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: VAPID public key가 변경되었습니다. 이는 기존 클라이언트의 푸시 구독을 무효화시킬 수 있습니다.

영향: 기존에 구독한 사용자들이 푸시 알림을 받지 못할 수 있습니다. VAPID 키는 클라이언트의 구독과 연결되어 있기 때문입니다.

수정 제안: 이것이 의도된 변경인지 확인하세요. 의도된 변경이라면 기존 구독을 마이그레이션하거나 무효화하는 계획이 필요합니다. 실수라면 원래 키로 되돌리세요.

Suggested change
public: BF8QQIULasLr94n0l0xbv43yZeNICudM5lpQN08VYn2g5VjBPU0wM98HypyRmEb-y0ARRsiZ_wcgSMIC-nq-x20
public: ${VAPID_PUBLIC_KEY}

Copilot uses AI. Check for mistakes.
private: ${VAPID_PRIVATE_KEY}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@
import java.time.ZoneOffset;
import java.util.List;

import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제점: 와일드카드 import (import static org.mockito.Mockito.*;)는 코드 가독성을 떨어뜨리고 어떤 메서드가 사용되는지 명확하지 않습니다.

영향: 코드 유지보수성이 저하되고, IDE에서 사용하지 않는 import를 감지하기 어렵습니다.

수정 제안: 명시적으로 사용되는 메서드만 import하도록 원래대로 되돌리세요.

Copilot uses AI. Check for mistakes.

@ExtendWith(MockitoExtension.class)
class NotificationDispatchSchedulerTest {
Expand Down Expand Up @@ -49,7 +44,7 @@ void dispatchDueNotifications_sendsAndDeletesPastNotifications() {

when(notificationRepository.findAll()).thenReturn(List.of(past, future));
when(pushSubscriptionRepository.findAllByMemberId(1L))
.thenReturn(List.of(new PushSubscription(1L, "token-1"), new PushSubscription(1L, "token-2")));
.thenReturn(List.of(new PushSubscription(1L, "device-1", "token-1"), new PushSubscription(1L, "device-2", "token-2")));

scheduler.dispatchDueNotifications();

Expand Down