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

kuring-38 [수정] 서버에서 학과 이름이 바뀌었을 때 로컬 DB에도 반영하도록 수정 #65

Merged
merged 12 commits into from
Aug 21, 2023

Conversation

mwy3055
Copy link
Member

@mwy3055 mwy3055 commented Jul 24, 2023

문제 상황

서버에서 학과 이름이 바뀌어도(예: 문화컨텐츠 -> 문화콘텐츠) 로컬 DB에서 바뀐 이름을 저장하지 않습니다.

해결 방법

서버에서 학과 데이터를 가져올 때, 로컬 DB가 비어 있지 않다면 DB에 저장된 학과 중 이름이 바뀐 학과를 업데이트하면 됩니다.

커밋 설명

코멘트로 설명하겠습니다.

if (departmentDao.getDepartmentsSize() == 0) {
departmentDao.insertDepartments(departments.toEntityList())
} else {
updateDepartmentsName(it)
Copy link
Member Author

Choose a reason for hiding this comment

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

학과 DB가 비어 있다면 서버에서 가져온 데이터를 그대로 삽입하고, 비어 있지 않다면 서버에서 가져온 데이터와 비교하여 DB를 업데이트합니다.

@@ -1,4 +1,4 @@
package com.ku_stacks.ku_ring.persistence
package com.ku_stacks.ku_ring
Copy link
Member Author

Choose a reason for hiding this comment

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

Dao 테스트 이외에도 다른 integrated test에서 사용할 수 있을 것 같아 상위 패키지로 옮겼습니다.

suspend fun insertAllDepartmentsFromRemote()
suspend fun fetchAllDepartmentsFromRemote(): List<Department>?
suspend fun updateDepartmentsFromRemote()
suspend fun fetchDepartmentsFromRemote(): List<Department>?
Copy link
Member Author

@mwy3055 mwy3055 Jul 24, 2023

Choose a reason for hiding this comment

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

현재 로직상 학과를 단순히 삽입하기만 하는 게 아니라, 상황에 따라 업데이트하기도 하므로 insertAllDepartments...라는 이름이 함수를 충분히 설명하지 못한다고 생각했습니다. 학과 DB를 서버의 데이터로 최신화한다는 의미에서 이름을 updateDepartments라고 바꿨습니다.

또, all이라는 단어는 불필요해 보입니다. 학과를 일부만 업데이트하는 경우가 없으므로 굳이 all이라고 명시할 필요가 없다고 생각했습니다.

val expectedDepartments =
updatedDepartmentsResponse.map { it.toDepartment() }.sortedBy { it.name }
val actual = departmentRepository.getAllDepartments().sortedBy { it.name }
assertEquals(expectedDepartments, actual)
Copy link
Member Author

Choose a reason for hiding this comment

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

서버에서 제공하는 학과 데이터가 바뀐 경우를 테스트하는 코드입니다.


@Query("UPDATE departments SET shortName = :shortName, koreanName = :koreanName WHERE name = :name")
suspend fun updateDepartment(name: String, shortName: String, koreanName: String)

@Query("SELECT * FROM departments WHERE isSubscribed = :isSubscribed")
Copy link
Member

Choose a reason for hiding this comment

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

DB에 관심이 많아 한가지 댓글 남겨봅니다!
SELECT 문에서 *는 지양하는 것 이 좋습니다!! 여러 이유가 있지만 다음 글이 잘 설명해주는 것 같아 링크 남겨봅니다
관련 링크 : https://hyewoncc.github.io/sql-no-wildcard/

Copy link
Member

@yeon-kyu yeon-kyu Aug 20, 2023

Choose a reason for hiding this comment

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

select *를 지양하는 이유가 7가지나 있군요! Entity를 가져와서 비즈니스로직을 처리하는곳에서는 어쩔수없이 select * 를 사용해야겠지만(꽤 많은경우가 이렇긴할것같아요 ㅠ) 단순한 로직에선 지양하는게 좋을것 같네요
정보공유 감사합니다~!

Copy link
Member

@yeon-kyu yeon-kyu left a comment

Choose a reason for hiding this comment

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

LGTM! 고생하셨습니다

@mwy3055 mwy3055 merged commit 31e5cb6 into develop Aug 21, 2023
1 check passed
@mwy3055 mwy3055 deleted the task/kuring-38 branch August 21, 2023 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants