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
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,21 @@ public Optional<AppVersion> resolve() {
HttpServletRequest request = servletAttributes.getRequest();
return AppVersion.parse(request.getHeader(APP_VERSION_HEADER));
}

/**
* 이번 요청이 {@code rawThreshold} 와 같거나 높은 버전의 앱에서 왔는가.
*
* <p><b>모르면 아니라고 답한다.</b> 헤더가 없거나, 읽을 수 없는 값이거나, 애초에 HTTP 요청이 아닌
* 자리에서 불렸으면 전부 구버전으로 본다. 기준값을 읽지 못했을 때도 같다. 설정 오타 하나로
* 모든 요청이 갑자기 신버전 취급을 받는 것보다, 아무도 신버전이 아닌 쪽이 되돌리기 쉽다.
*
* <p>버전으로 동작을 가르는 곳이 늘어날 때 이 판정을 각자 들고 있으면 "모르면 구버전" 이라는
* 규칙이 곳곳에서 조금씩 달라진다. 비교만 여기에 두고, <b>기준 버전과 그래서 무엇이 달라지는가는
* 각 도메인이 정한다.</b> 그래야 한 도메인의 설정이 다른 도메인의 동작을 끌고 가지 않는다.
*/
public boolean isAtLeast(String rawThreshold) {
return AppVersion.parse(rawThreshold)
.flatMap(threshold -> resolve().map(requested -> requested.isAtLeast(threshold)))
.orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ public record PracticeNotificationRegisterDto(
/**
* 주간 반복인데 요일을 하나도 고르지 않은 상태인지 확인한다.
*
* <p>예전에는 이 상태가 크론 변환에서 매일 발송으로 되돌아갔다. 사용자는 특정 요일만
* 고른 줄 알면서 매일 알림을 받았고, 요청이 잘못됐다는 신호도 없었다.
* <p>이 상태는 크론 변환에서 매일 발송으로 되돌아간다. 사용자는 특정 요일만 고른 줄 알면서
* 매일 알림을 받는다. 그래서 신버전 앱 요청은 진입부에서 400 으로 막는다. 요일을 고르라는
* 검증이 없는 구버전 앱 요청은 예전 서버와 같게 매일로 저장한다.
*/
public boolean isWeeklyWithoutWeekDays() {
return WEEKLY.equalsIgnoreCase(repeatType) && (weekDays == null || weekDays.isEmpty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public class PracticeNoteService {

private final CustomEmojiValidator customEmojiValidator;

private final PracticeNotificationWeekDayPolicy weekDayPolicy;

private PracticeNote getPracticeEntity(Long practiceId, Long userId){

PracticeNote practiceNote = practiceNoteRepository.findById(practiceId)
Expand Down Expand Up @@ -214,16 +216,26 @@ private void deletePracticeWithoutOwnerCheck(Long practiceId) {
}

/**
* 주간 반복 알림에 요일이 하나도 없으면 400 으로 거절한다.
* 주간 반복 알림에 요일이 하나도 없으면 <b>신버전 앱 요청만</b> 400 으로 거절한다.
*
* <p>이 검증이 요청 진입부에 있는 이유는 복습노트 저장이나 기존 Quartz 잡 삭제가 아예 일어나지
* 않아야 하기 때문이다. Quartz 잡 삭제는 이 트랜잭션과 함께 롤백되지 않는다.
*
* <p>스케줄러도 같은 검증을 하지만, 요청을 받은 자리에서 먼저 막아야 복습노트 저장이나
* 기존 Quartz 잡 삭제가 아예 일어나지 않는다. Quartz 잡 삭제는 이 트랜잭션과 함께
* 롤백되지 않기 때문이다.
* <p>구버전 요청은 예전처럼 통과시킨다. 요일이 빈 주간 반복은 스케줄러의 크론 변환에서
* 매일 발송으로 저장된다. 구버전 앱에는 요일을 고르라는 검증이 없어서, 여기서 막으면
* 그 사용자는 복습 세트를 영영 수정할 수 없다. 판정 기준은
* {@link PracticeNotificationWeekDayPolicy} 한 곳에 있다.
*/
private void validatePracticeNotification(PracticeNotificationRegisterDto practiceNotification) {
if (practiceNotification != null && practiceNotification.isWeeklyWithoutWeekDays()) {
if (practiceNotification == null || !practiceNotification.isWeeklyWithoutWeekDays()) {
return;
}

if (weekDayPolicy.requiresWeekDays()) {
throw new ApplicationException(PracticeNoteErrorCase.PRACTICE_NOTIFICATION_WEEK_DAYS_REQUIRED);
}

log.info("요일 없는 주간 반복 알림을 구버전 앱 요청으로 보고 매일 발송으로 저장한다");
}

private <T> List<T> nullSafe(List<T> values) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@

package com.aisip.OnO.backend.practicenote.service;

import com.aisip.OnO.backend.common.exception.ApplicationException;
import com.aisip.OnO.backend.practicenote.dto.PracticeNotificationRegisterDto;
import com.aisip.OnO.backend.practicenote.exception.PracticeNoteErrorCase;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.quartz.*;
Expand All @@ -16,8 +14,6 @@ public class PracticeNotificationScheduler {
private final Scheduler scheduler;

public void schedulePracticeNotification(Long userId, Long practiceId, String practiceTitle, PracticeNotificationRegisterDto dto) {
validateNotification(dto);

try {
JobDetail jobDetail = JobBuilder.newJob(PracticeNotificationJob.class)
.withIdentity("practice-" + practiceId, "practice-reminder")
Expand All @@ -44,10 +40,6 @@ public void schedulePracticeNotification(Long userId, Long practiceId, String pr
}

public void updateNotification(Long userId, Long practiceId, String title, PracticeNotificationRegisterDto dto) {
// 잡을 지운 뒤에 검증에 걸리면 기존 알림만 사라진다. Quartz 잡 삭제는 서비스 트랜잭션과
// 함께 롤백되지 않으므로, 지우기 전에 먼저 막는다.
validateNotification(dto);

deleteNotification(practiceId);
schedulePracticeNotification(userId, practiceId, title, dto);
}
Expand All @@ -61,28 +53,15 @@ public void deleteNotification(Long practiceId) {
}
}

/**
* 주간 반복인데 요일이 비어 있으면 거절한다.
*
* <p>예전에는 이 요청이 아래 크론 변환의 매일 폴백으로 흘러가, 사용자가 고르지도 않은
* 매일 알림이 등록됐다. 잘못된 요청이라는 신호 없이 동작만 달라지는 쪽이 더 나쁘다.
*/
private void validateNotification(PracticeNotificationRegisterDto dto) {
if (dto.isWeeklyWithoutWeekDays()) {
throw new ApplicationException(PracticeNoteErrorCase.PRACTICE_NOTIFICATION_WEEK_DAYS_REQUIRED);
}
}

private String convertDtoToCron(PracticeNotificationRegisterDto dto) {
int hour = dto.hour();
int minute = dto.minute();

if ("daily".equalsIgnoreCase(dto.repeatType())) {
// 매일 지정된 시각에 실행
return String.format("0 %d %d ? * *", minute, hour);
} else if ("weekly".equalsIgnoreCase(dto.repeatType())) {
} else if ("weekly".equalsIgnoreCase(dto.repeatType()) && !dto.isWeeklyWithoutWeekDays()) {
// 선택한 요일에만 지정된 시각에 실행 (e.g. MON,WED,FRI)
// 요일이 비어 있는 경우는 validateNotification 이 이미 걸러 냈다.
String dayString = dto.weekDays().stream()
.map(this::convertDayToQuartz)
.reduce((a, b) -> a + "," + b)
Expand All @@ -91,8 +70,12 @@ private String convertDtoToCron(PracticeNotificationRegisterDto dto) {
return String.format("0 %d %d ? * %s", minute, hour, dayString);
}

// daily/weekly 가 아닌 값(null 포함)은 지금처럼 매일로 둔다.
// 구버전 앱이 repeatType 을 비워 보내는 경우까지 여기서 막으면 기존 알림이 통째로 끊긴다.
// 매일 폴백. 여기로 오는 경우는 둘이다.
// 1) daily/weekly 가 아닌 값(null 포함). 구버전 앱이 repeatType 을 비워 보낸다.
// 2) 주간 반복인데 요일이 비어 있는 구버전 앱 요청. 신버전 요청은 진입부인
// PracticeNoteService 에서 이미 400 으로 걸러지고 여기까지 오지 않는다.
// 구버전 앱에는 요일을 고르라는 검증이 없어서, 여기서 막으면 그 사용자는
// 복습 세트를 저장할 수도 수정할 수도 없다. 예전 서버와 같게 매일로 저장한다.
return String.format("0 %d %d ? * *", minute, hour);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.aisip.OnO.backend.practicenote.service;

import com.aisip.OnO.backend.common.web.AppVersionResolver;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

/**
* 주간 반복 알림에 요일을 강제할 요청인지 정하는 한 곳.
*
* <p>요일 없는 주간 반복을 400 으로 막는 검증(#302)이 <b>구버전 앱 사용자를 가뒀다.</b>
* 구버전 앱에는 요일을 고르라는 클라이언트 검증이 없어서 "매주" 만 고른 저장이 그대로 올라오고,
* 예전 서버는 그 요청을 매일 크론으로 받아 줬다. 그래서 운영 DB 에 요일이 빈 주간 알림 행이 이미 있고,
* 그 복습 세트를 연 구버전 사용자는 제목만 바꿔도 계속 400 을 받는다. 앱을 올리기 전에는 빠져나갈 길이 없다.
*
* <p>그래서 <b>요청이 온 앱 버전으로 가른다.</b> 기준 버전 이상에서 온 요청만 막고, 그 아래는 예전처럼
* 매일 크론으로 저장한다. 프론트는 신버전에서 요일을 강제하므로 신버전 사용자는 이 조합을 만들 수 없다.
*
* <p><b>미션의 {@code LegacyAccrualPolicy} 를 그대로 쓰지 않은 이유.</b> 거기에는 자동 적립을 통째로
* 멈추는 비상 스위치({@code ono.mission.legacy-accrual.enabled})가 붙어 있다. 그 스위치를 내렸을 때
* 복습 알림 검증까지 같이 움직이면, XP 사고를 막으려고 끈 설정이 알림 저장 동작을 조용히 바꾼다.
* 공용으로 두는 것은 {@link AppVersionResolver#isAtLeast(String)} 의 버전 비교까지다.
*/
@Component
@RequiredArgsConstructor
public class PracticeNotificationWeekDayPolicy {

private final AppVersionResolver appVersionResolver;

/**
* 요일 없는 주간 반복을 거절하기 시작하는 첫 앱 버전.
*
* <p>기본값이 {@code 4.0.0} 인 근거는 <b>헤더 자체가 이 버전 라인에서 처음 붙는다</b> 는 것이다
* (AI-SIP/OnO_FRONT#214, 프론트 {@code pubspec.yaml} 이 {@code 4.0.0+70}). 헤더를 안 보내는
* 스토어 빌드는 기준값이 무엇이든 구버전으로 떨어지므로, 이 값은 <b>헤더를 보내는 앱 중
* 어디까지를 새 앱으로 볼지</b>만 가른다. 헤더를 붙인 커밋과 요일을 강제하는 클라이언트 검증은
* 같은 미출시 빌드에 들어 있다.
*
* <p>설정값으로 둔 이유는 요일 강제가 빠진 빌드가 뒤늦게 드러났을 때 배포 없이 기준을 올려
* 되돌리기 위해서다.
*/
@Value("${ono.practice-note.week-days-required-version:4.0.0}")
private String weekDaysRequiredVersion;

/**
* 이번 요청에서 주간 반복에 요일을 강제하는가.
*
* <p><b>모르면 강제하지 않는다.</b> 헤더가 없는 요청은 구버전이다. 잘못 보면 신버전 사용자가
* 요일 없는 주간 알림을 하나 저장할 뿐이지만, 반대로 틀리면 구버전 사용자는 복습 세트를
* 영영 수정할 수 없다.
*/
public boolean requiresWeekDays() {
return appVersionResolver.isAtLeast(weekDaysRequiredVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,47 @@ void emptyOnThreadWithoutInheritedContext() throws Exception {
}
}

@Test
@DisplayName("기준 버전과 같거나 높으면 참이다")
void isAtLeastWhenHeaderMeetsThreshold() {
bindRequestWithHeader("4.0.0+70");

assertThat(resolver.isAtLeast("4.0.0")).isTrue();
assertThat(resolver.isAtLeast("3.9.9")).isTrue();
}

@Test
@DisplayName("기준 버전보다 낮으면 거짓이다")
void isAtLeastFalseBelowThreshold() {
bindRequestWithHeader("3.6.0+67");

assertThat(resolver.isAtLeast("4.0.0")).isFalse();
}

@Test
@DisplayName("헤더가 없거나 읽을 수 없거나 HTTP 요청이 아니면 전부 거짓이다")
void isAtLeastFalseWhenVersionUnknown() {
// 버전으로 동작을 가르는 쪽의 규칙이 "모르면 구버전" 이라 세 경우가 같은 답이어야 한다.
bindRequestWithHeader(null);
assertThat(resolver.isAtLeast("4.0.0")).isFalse();

bindRequestWithHeader("abc");
assertThat(resolver.isAtLeast("4.0.0")).isFalse();

RequestContextHolder.resetRequestAttributes();
assertThat(resolver.isAtLeast("4.0.0")).isFalse();
}

@Test
@DisplayName("기준값을 읽을 수 없으면 아무도 신버전이 아니다")
void isAtLeastFalseWhenThresholdUnparsable() {
// 설정 오타 하나로 모든 요청이 갑자기 신버전 취급을 받으면 안 된다.
bindRequestWithHeader("9.9.9+999");

assertThat(resolver.isAtLeast("사.영.영")).isFalse();
assertThat(resolver.isAtLeast("")).isFalse();
}

private void bindRequestWithHeader(String appVersion) {
MockHttpServletRequest request = new MockHttpServletRequest();
if (appVersion != null) {
Expand Down
Loading
Loading