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

[Feature] 모집 게시글 채팅방 url 조회 API 추가 #164

Merged
merged 6 commits into from
Dec 1, 2022

Conversation

username1103
Copy link
Member

@username1103 username1103 commented Dec 1, 2022

체크 리스트

  • 적절한 제목으로 수정했나요?
  • 관련된 이슈와 연결 시켰나요?
  • Target Branch를 올바르게 설정했나요?
  • Label을 알맞게 설정했나요?

작업 내역

GET /v1/group-articles/:id/chat-url 채팅방 조회 API 추가

  • 모집게시글이 존재하지 않는 경우 GroupArticleNotFoundException 반환
  • 모집완료된 게시글이 아닌 경우, NotSuccessGroupException 반환
  • 모임참가자가 아닌 경우 NotParticipantException 반환

모집게시글 작성시 작성자데이터를 모집신청정보에 추가

  • 따로 신청정보 + 1로 모임인원 정보를 제공할 수 있지만, 모임참가자 조회시 불편할 것 같아요.
    • 신청정보에서 유저 정보들 가져오고, 작성자 유저정보 가져오고 하는게 번거롭다고 느껴졌어요.
    • 그래서 작성자의 신청정보를 모집게시글 작성시 추가하도록 했습니다.

Typeorm Connection Pool Size 설정

기본적으로 mysql 서버의 최대 연결가능수는 151로 설정되어있어요. typeorm을 사용해 mysql과 통신하면 mysql2드라이버를 사용하게 돼요. mysql2에서 기본적인 poolSize는 10이므로 typeorm에서도 10으로 예상돼요. 그래서 connection pool size를 우선 50으로 잡아놨어요.

토큰 검증 및 리프레시 로직 수정

  • 기존에 엑세스토큰이 쿠키에 없으면 바로 에러를 던지도록 코드가 짜여져있었어요. 하지만 쿠키에 엑세스토큰을 담다보니 만료되면 쿠키가 사라져서 요청에 포함되지 않아요. 이로 인해 리프레시 토큰을 사용한 갱신로직이 동작하지 않았어요.

  • 엑세스토큰이 없는 경우에 리프레시 토큰을 체크하도록 수정했습니다.

문제 상황과 해결

커넥션이 이미 종료된 상태라는 에러

현재 모집게시글 조회 API에서만 아래와 같은 오류가 발생해 INTERNAL_SERVER_ERROR를 반환하고 있어요.
image

찾아본 결과, 커넥션 연결 후 오랜시간 사용하지 않다가 사용하면 생기는 문제 같아요. 아무래도 connection이 오랜시간 사용되지 않아서 디비측에서 끊어버렸는데, 이 부분에 대해 typeorm에서 핸들링이 안되고 있는게 아닐까 싶은 생각이 들어요. 확실하게는 확인해봐야할 것 같습니다.

모듈 순환 참조 에러

현재 GroupApplicationModule에서 GroupArticleModule을 import해 GroupArticleRepository를 사용하고 있어요. 이번 API 제작시에, 사용자가 모임 참가자인지 체크가 필요했어서 GroupApplicationModule이 가지고 있는 GroupApplicationRepository가 필요하게 되었어요. 그래서 GroupApplicationModuleGroupArticleModule에 import 해주었더니 모듈간 순환 참조에러가 발생하게 되었어요.

해결하기 위해서 TypeOrmDataSourceGroupArticleService에 주입해 GroupApplicationRepository를 가져와서 쿼리를 날리도록 수정해 GroupApplicationModule을 GroupArticleModule에 import할 필요없도록 하여 해결했어요. 현재는 한군데에서만 해당 레포지토리 를 사용하면 되어서 이렇게 처리했는데 다양한 곳에서 GroupApplicationModule에서 만든 GroupApplicationRepository를 사용해야 하는 경우, 어떻게 처리하는게 좋을지 고민이되었어요.

A모듈이 B모듈에 의존성을 가진다는건 B 모듈변경이 발생횄을 때, A모듈에 영향을 미친다를 의미해요. 그렇기 때문에 순환 참조하는 경우, 한쪽에서 수정하면 다른쪽에서 영향을 끼칠거고 그게 자기 자신한테도 영향을 줄 수 있어 수정이 어려워진다는 문제가 있다고 생각해요. 어떻게 해결할 수 있을지 좀 더 고민해볼 필요가 있을 것 같아요.

image

비고

  • 참고했던 링크 등 참고 사항을 적어주세요. 코드 리뷰하는 사람이 참고해야 하는 내용을 자유로운 형식으로 적을 수 있습니다.

- 모집게시글이 존재하지 않으면 GroupArticleNotFoundException 반환
- 참가자가 아니면 NotParticipantException 반환
- 모집완료된 게시글이 아니면 NotSuccessGroupException 반환
- 모집게시글이 존재하지 않는 경우 GroupArticleNotFoundException 반환
- 모집완료된 게시글이 아닌 경우, NotSuccessGroupException 반환
- 모임참가자가 아닌 경우 NotParticipantException 반환
- 엑세스토큰이 없는 경우 쿠키를 제거하고 InvalidTokenException를 반환하도록 하였음
- 하지만 클라이언트에서 쿠키가 만료되어 사라진 경우가 있어 올바른 로직이 아님
- 엑세스토큰이 없는 경우 리프레시 토큰을 확인하도록 수정
@username1103 username1103 linked an issue Dec 1, 2022 that may be closed by this pull request
2 tasks
@username1103 username1103 requested review from pythonstrup, sxungchxn and kong430 and removed request for pythonstrup December 1, 2022 03:20
@username1103 username1103 self-assigned this Dec 1, 2022
Copy link
Collaborator

@kong430 kong430 left a comment

Choose a reason for hiding this comment

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

고민 정리 잘 읽었습니다! 수고하셨어요~

Copy link
Member

@pythonstrup pythonstrup left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!

Comment on lines +49 to +54
await this.dataSource.transaction(async (em) => {
await em.getRepository(GroupArticle).save(groupArticle);
await em
.getRepository(GroupApplication)
.save(GroupApplication.create(user, groupArticle.group));
});
Copy link
Member

Choose a reason for hiding this comment

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

wow 이게 말로만 듣던 트랜잭션인가요...? 공부 좀 해봐야겠어요..

Comment on lines +155 to +161
const groupApplication = await this.dataSource
.getRepository(GroupApplication)
.findOneBy({
userId: user.id,
groupId: groupArticle.group.id,
status: GROUP_APPLICATION_STATUS.REGISTER,
});
Copy link
Member

Choose a reason for hiding this comment

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

여기서는 왜 의존성 주입받은 GroupApplicationRepository 안 쓰시고 dataSource 이용해서 하셨는지 궁금합니다!
이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pythonstrup 고민에 내용이 위 사항에 대한 내용이에요

Comment on lines +21 to +44
const { access_token, refresh_token } = request.cookies;

if (!access_token) throw new Error('엑세스 토큰이 존재하지 않습니다');
try {
if (!access_token) {
throw new Error('엑세스 토큰이 존재하지 않습니다');
}

try {
const authTokenPayload = this.jwtTokenService.verifyAuthToken(
access_token,
TokenType.ACCESS,
);
const authTokenPayload = this.jwtTokenService.verifyAuthToken(
access_token,
TokenType.ACCESS,
);

const user = await this.dataSource
.getRepository(User)
.findOneBy({ id: authTokenPayload.userId, deletedAt: null });
const user = await this.dataSource
.getRepository(User)
.findOneBy({ id: authTokenPayload.userId, deletedAt: IsNull() });

if (!user) throw new Error('유저가 존재하지 않습니다');
if (!user) {
throw new Error('유저가 존재하지 않습니다');
}

request.user = user;
request.user = user;

return true;
} catch (e) {
if (!refresh_token) throw new Error('Not Found RefreshToken');
return true;
} catch (e) {
Copy link
Member

Choose a reason for hiding this comment

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

이제 이중 try catch에서 해방 된건가요..?

} catch (e) {
response.clearCookie('access_token');
response.clearCookie('refresh_token');
throw new InvalidTokenException(e.message);
}
Copy link
Member

Choose a reason for hiding this comment

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

대신 이곳에 생겼군요... ㅋㅋㅋ

Copy link
Member Author

Choose a reason for hiding this comment

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

😭

Copy link
Member

@sxungchxn sxungchxn left a comment

Choose a reason for hiding this comment

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

고생하셨어요

@username1103 username1103 merged commit 7701618 into develop Dec 1, 2022
@username1103 username1103 deleted the feature/119-group-chat-url branch December 1, 2022 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] 모집 게시글 채팅방 url 조회 API 추가
4 participants