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
Merged
Show file tree
Hide file tree
Changes from 9 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 @@ -20,6 +20,12 @@ interface DepartmentDao {
@Query("SELECT * FROM departments WHERE koreanName LIKE '%' || :koreanName || '%'")
suspend fun getDepartmentsByKoreanName(koreanName: String): List<DepartmentEntity>

@Query("SELECT COUNT(*) FROM departments")
suspend fun getDepartmentsSize(): Int
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved

@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 * 를 사용해야겠지만(꽤 많은경우가 이렇긴할것같아요 ㅠ) 단순한 로직에선 지양하는게 좋을것 같네요
정보공유 감사합니다~!

suspend fun getDepartmentsBySubscribed(isSubscribed: Boolean): List<DepartmentEntity>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import com.ku_stacks.ku_ring.data.model.Department
import kotlinx.coroutines.flow.Flow

interface DepartmentRepository {
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이라고 명시할 필요가 없다고 생각했습니다.

suspend fun insertDepartment(department: Department)
suspend fun insertDepartments(departments: List<Department>)
suspend fun getAllDepartments(): List<Department>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,40 @@ class DepartmentRepositoryImpl @Inject constructor(
// null: 데이터가 업데이트되어 새 데이터를 가져와야 함
private var departments: List<Department>? = null

override suspend fun insertAllDepartmentsFromRemote() {
val departments = fetchAllDepartmentsFromRemote()
override suspend fun updateDepartmentsFromRemote() {
val departments = fetchDepartmentsFromRemote()
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
departments?.let {
departmentDao.insertDepartments(departments.map { it.toEntity() })
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를 업데이트합니다.

}
}
}

override suspend fun fetchAllDepartmentsFromRemote(): List<Department>? {
override suspend fun fetchDepartmentsFromRemote(): List<Department>? {
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
return runCatching {
departmentClient.fetchDepartmentList().data?.map { it.toDepartment() } ?: emptyList()
}.getOrNull()
}

private suspend fun updateDepartmentsName(departments: List<Department>) {
val sortedOriginalDepartments = getAllDepartments().sortedBy { it.name }
val updateTargets = departments.filter { department ->
val originalPosition =
sortedOriginalDepartments.binarySearch { it.name.compareTo(department.name) }
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
val originalDepartment = sortedOriginalDepartments[originalPosition]
return@filter originalDepartment.name != department.name || originalDepartment.koreanName != department.koreanName
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
}

withContext(ioDispatcher) {
updateTargets.forEach { (name, shortName, koreanName, _) ->
departmentDao.updateDepartment(name, shortName, koreanName)
}
}
this.departments = null
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
}

override suspend fun insertDepartment(department: Department) {
withContext(ioDispatcher) {
departmentDao.insertDepartment(department.toEntity())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class EditSubscriptionViewModel @Inject constructor(
}

viewModelScope.launch {
departmentRepository.insertAllDepartmentsFromRemote()
departmentRepository.updateDepartmentsFromRemote()
val subscribedDepartments = departmentRepository.getSubscribedDepartments()
addDepartmentsToMap(subscribedDepartments)
val notificationEnabledDepartments = repository.fetchSubscribedDepartments()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class DepartmentSubscribeViewModel @Inject constructor(

init {
viewModelScope.launch {
departmentRepository.insertAllDepartmentsFromRemote()
departmentRepository.updateDepartmentsFromRemote()
}

searchKeyword.onEach { loadFilteredDepartment(it) }
Expand Down
Original file line number Diff line number Diff line change
@@ -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에서 사용할 수 있을 것 같아 상위 패키지로 옮겼습니다.


import androidx.room.Room
import androidx.test.core.app.ApplicationProvider.getApplicationContext
Expand Down
18 changes: 18 additions & 0 deletions app/src/test/java/com/ku_stacks/ku_ring/MockUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import com.ku_stacks.ku_ring.data.api.response.NoticeResponse
import com.ku_stacks.ku_ring.data.api.response.SubscribeListResponse
import com.ku_stacks.ku_ring.data.api.response.UserListResponse
import com.ku_stacks.ku_ring.data.api.response.UserResponse
import com.ku_stacks.ku_ring.data.db.DepartmentEntity
import com.ku_stacks.ku_ring.data.db.NoticeEntity
import com.ku_stacks.ku_ring.data.db.PushEntity
import com.ku_stacks.ku_ring.data.model.Department
import com.ku_stacks.ku_ring.data.model.Notice
import org.mockito.Mockito

Expand Down Expand Up @@ -58,6 +60,13 @@ object MockUtil {
isReadOnStorage = false,
)

fun mockDepartmentEntity() = DepartmentEntity(
name = "smart_ict_convergence",
shortName = "sicte",
koreanName = "스마트ICT융합공학과",
isSubscribed = false,
)

fun mockNotice() = Notice(
postedDate = "20220203",
subject = "2022학년도 1학기 재입학 합격자 유의사항 안내",
Expand All @@ -72,6 +81,15 @@ object MockUtil {
tag = emptyList()
)

fun mockDepartment() = Department(
name = "smart_ict_convergence",
shortName = "sicte",
koreanName = "스마트ICT융합공학과",
isSubscribed = false,
isSelected = false,
isNotificationEnabled = false,
)

fun mockPushEntity() = PushEntity(
articleId = "5b4a11b",
category = "bachelor",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import com.ku_stacks.ku_ring.data.api.NoticeClient
import com.ku_stacks.ku_ring.data.db.NoticeDao
import com.ku_stacks.ku_ring.data.db.NoticeEntity
import com.ku_stacks.ku_ring.data.source.DepartmentNoticeMediator
import com.ku_stacks.ku_ring.persistence.LocalDbAbstract
import com.ku_stacks.ku_ring.LocalDbAbstract
import com.ku_stacks.ku_ring.util.PreferenceUtil
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.ku_stacks.ku_ring.persistence

import com.ku_stacks.ku_ring.LocalDbAbstract
import com.ku_stacks.ku_ring.MockUtil
import com.ku_stacks.ku_ring.data.db.DepartmentDao
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner

@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(RobolectricTestRunner::class)
class DepartmentDaoTest : LocalDbAbstract() {
private lateinit var departmentDao: DepartmentDao

@Before
fun setup() {
departmentDao = db.departmentDao()
}

@Test
fun `insertDepartment and updateDepartment test`() = runTest {
// given
val entity = MockUtil.mockDepartmentEntity()
departmentDao.insertDepartment(entity)

// when
val newShortName = "ict"
val newKoreanName = "스융공"
departmentDao.updateDepartment(
name = entity.name,
shortName = newShortName,
koreanName = newKoreanName
)

// then
val result = departmentDao.getDepartmentsByName(entity.name)
assert(result.isNotEmpty())
assertEquals(1, result.size)
assertEquals(result[0], entity.copy(shortName = newShortName, koreanName = newKoreanName))
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.ku_stacks.ku_ring.persistence

import androidx.paging.PagingSource
import com.ku_stacks.ku_ring.LocalDbAbstract
import com.ku_stacks.ku_ring.data.db.NoticeDao
import com.ku_stacks.ku_ring.data.db.NoticeEntity
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.ku_stacks.ku_ring.persistence

import com.ku_stacks.ku_ring.LocalDbAbstract
import com.ku_stacks.ku_ring.data.db.PushDao
import com.ku_stacks.ku_ring.data.db.PushEntity
import kotlinx.coroutines.runBlocking
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package com.ku_stacks.ku_ring.repository

import com.ku_stacks.ku_ring.LocalDbAbstract
import com.ku_stacks.ku_ring.MockUtil
import com.ku_stacks.ku_ring.data.api.DepartmentClient
import com.ku_stacks.ku_ring.data.api.response.DepartmentListResponse
import com.ku_stacks.ku_ring.data.api.response.DepartmentResponse
import com.ku_stacks.ku_ring.data.db.DepartmentDao
import com.ku_stacks.ku_ring.data.mapper.toDepartment
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.mockito.Mockito

@OptIn(ExperimentalCoroutinesApi::class)
class DepartmentRepositoryTest : LocalDbAbstract() {
private lateinit var departmentRepository: DepartmentRepository
private lateinit var departmentDao: DepartmentDao
private val departmentClient = Mockito.mock(DepartmentClient::class.java)

private val testDispatcher = StandardTestDispatcher()

@Before
fun setup() {
Dispatchers.setMain(testDispatcher)
departmentDao = db.departmentDao()
departmentRepository = DepartmentRepositoryImpl(
departmentDao = departmentDao,
departmentClient = departmentClient,
ioDispatcher = testDispatcher,
)
}

@After
fun tearDown() {
Dispatchers.resetMain()
}

// integrated test
@Test
fun `insert and update departments test`() = runTest {
// given
val departments = (1..10).map {
MockUtil.mockDepartment().copy(
name = it.toString(),
shortName = "dep$it",
koreanName = "학과 $it",
)
}
departmentRepository.insertDepartments(departments)
assertEquals(departments.size, departmentRepository.getAllDepartments().size)

// when
val updatedDepartmentsResponse =
departments.mapIndexed { index, (name, shortName, koreanName, _) ->
DepartmentResponse(
name = name,
shortName = if (index % 2 == 0) shortName else shortName.repeat(2),
korName = if (index % 2 == 0) koreanName else koreanName.repeat(2),
yeon-kyu marked this conversation as resolved.
Show resolved Hide resolved
)
}
Mockito.`when`(departmentClient.fetchDepartmentList())
.thenReturn(DepartmentListResponse(200, "success", updatedDepartmentsResponse))
departmentRepository.updateDepartmentsFromRemote()

// then
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.

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

}
}