Skip to content
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

Ignore: ✨ 사용자가 수신한 푸시 알림 리스트 최신순 조회 #134

Merged
merged 32 commits into from
Jul 17, 2024

Conversation

psychology50
Copy link
Member

@psychology50 psychology50 commented Jul 17, 2024

작업 이유

image

  • 사용자가 수신한 푸시 알림 리스트를 무한 스크롤로 조회 가능한 API 구현

작업 사항

1️⃣ Notification Entity 수정

CREATE TABLE `pennyway`.`notification` (
	`id` bigint NOT NULL AUTO_INCREMENT,
	`type` char NOT NULL,
    `announcement` char NOT NULL,
	`read_at` datetime DEFAULT NULL,
	`created_at` datetime NOT NULL,
	`updated_at` datetime NOT NULL,
	`sender` bigint DEFAULT NULL,
	`receiver` bigint NOT NULL,
	PRIMARY KEY (`id`),
	UNIQUE INDEX `id_UNIQUE` (`id` ASC) VISIBLE,
	FOREIGN KEY (`sender`) REFERENCES `pennyway`.`user` (`id`)
	ON DELETE CASCADE ON UPDATE CASCADE,
	FOREIGN KEY (`receiver`) REFERENCES `pennyway`.`user` (`id`)
	ON DELETE CASCADE ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE utf8mb4_unicode_ci;
  • 기존 iOS 측에서 딥 링크를 위한 데이터를 확인할 수 없던 문제가 존재함.
  • 따라서 송신자 sender, 수신자 receiver, 대상 pk toId 필드를 추가함.
  • 송신자의 이름을 join 없이 유추하기 위해 비정규화하여 receiver_name 필드를 추가
    • 해당 방식은 송신자의 이름이 바뀌었을 시, 문제가 발생할 우려가 존재하므로 개선될 필요가 있음.
    • 현재는 사용하지 않기 때문에 해당 문제를 이슈로 등록하고 추후 처리.
  • notification은 사용자가 자기 자신의 데이터를 불러오는 경우밖에 없으므로 receiver_name은 따로 저장하지 않음.

2️⃣ 푸시 알림 제목, 내용 포맷팅

public class Notification extends DateAuditable {
    ...

    public String createFormattedTitle() {
        if (type.equals(NoticeType.ANNOUNCEMENT)) {
            return announcement.createFormattedTitle(receiverName);
        }
        return ""; // TODO: 알림 종류가 신규로 추가될 때, 해당 로직을 구현해야 함.
    }

    public String createFormattedContent() {
        if (type.equals(NoticeType.ANNOUNCEMENT)) {
            return announcement.createFormattedContent(receiverName);
        }
        return ""; // TODO: 알림 종류가 신규로 추가될 때, 해당 로직을 구현해야 함.
    }
}
  • 이유가 없다면, Notification Entity 내의 함수를 호출하여 포맷팅된 문자열을 얻을 수 있음.
  • 현재는 공지 알림인 경우밖에 존재하지 않으므로, notice type class 내의 포매팅 메서드는 구현하지 않음.

public enum Announcement implements LegacyCommonType {
    ...

    public String createFormattedTitle(String name) {
        validateName(name);

        if (this.title.indexOf("%") == -1) {
            return this.title;
        }

        return String.format(title, name);
    }

    public String createFormattedContent(String name) {
        validateName(name);

        if (this.content.indexOf("%") == -1) {
            return this.content;
        }

        return String.format(content, name);
    }
}
  • Announcement는 제목과 내용에 수신자의 이름을 삽입하여 문자열을 반환하는 함수를 지님.
  • .contains()가 아닌 .indexOf()를 사용한 이유는 성능 개선 목적을 위함.
    • contains()의 시간복잡도는 O(N*M)
    • indexOf()의 시간복잡도는 O(N)
    • title 혹은 content에 수신자의 이름을 삽입할 필요가 있는지 확인만 하면 되므로, indexOf 메서드를 사용함.

리뷰어가 중점적으로 확인해야 하는 부분

  • 전에 언급하긴 했지만, Notification Entity가 변경된 이유에 대해 이해하였는지?
  • 포맷팅을 하는 방법이 적절하게 수행되고 있다고 생각하는지?

발견한 이슈

  • receiver name만 고려하고 비정규화를 했는데, 어차피 나중엔 사용자 프로필 데이터도 수신해야 함. 이를 개선할 수 있는 방법으로 join을 할 지, 매번 쿼리를 따로 호출할 지, 사용자 정보를 모두 캐싱해두고 빠르게 조회하는 전략을 수행할 지 고민이 필요함. 혹시나 좋은 아이디어 있다면 추천해주세용.

@psychology50 psychology50 added the enhancement New feature or request label Jul 17, 2024
@psychology50 psychology50 self-assigned this Jul 17, 2024
@psychology50 psychology50 merged commit 869be8d into dev Jul 17, 2024
1 check passed
@psychology50 psychology50 deleted the feat/PW-426-get-notifications branch July 17, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant