Skip to content

Commit d69aaf6

Browse files
ekzm8523marcus-min
ekzm8523
and
marcus-min
authored
Feature/fix tag bug (#175)
* fix : setTags -> updateTags 버그 수정 * feature : setTags 시에 문제의 태그가 비어있을 때만 생성하도록 예외처리 생성 * refactor : 태그 Upserter 리팩토링 * refactor : ProblemTagRepository 제거 & lint --------- Co-authored-by: marcus.min <[email protected]>
1 parent 27ef61a commit d69aaf6

File tree

9 files changed

+109
-76
lines changed

9 files changed

+109
-76
lines changed

‎src/main/kotlin/io/csbroker/apiserver/common/enums/ErrorCode.kt

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ enum class ErrorCode(
1616
FORBIDDEN(403, "이 작업에 대한 권한이 없습니다."),
1717
NOT_FOUND_ENTITY(404, "대상을 찾을 수 없습니다."),
1818
USERNAME_DUPLICATED(409, "닉네임이 중복되었습니다."),
19+
TAG_DUPLICATED(409, "태그가 중복되었습니다."),
1920
EMAIL_DUPLICATED(409, "이메일이 중복되었습니다."),
2021
PROVIDER_MISS_MATCH(409, "올바르지 않은 provider입니다."),
2122
SERVER_ERROR(500, "서버에서 오류가 발생했습니다."),

‎src/main/kotlin/io/csbroker/apiserver/model/Problem.kt

+29-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package io.csbroker.apiserver.model
22

3+
import io.csbroker.apiserver.common.enums.ErrorCode
4+
import io.csbroker.apiserver.common.exception.ConditionConflictException
5+
import io.csbroker.apiserver.common.exception.EntityNotFoundException
36
import io.csbroker.apiserver.dto.problem.GradingHistoryStats
47
import io.csbroker.apiserver.dto.problem.ProblemResponseDto
58
import jakarta.persistence.CascadeType
@@ -49,7 +52,7 @@ abstract class Problem(
4952
@Column(name = "score")
5053
var score: Double,
5154

52-
@OneToMany(mappedBy = "problem", cascade = [CascadeType.ALL], fetch = FetchType.LAZY)
55+
@OneToMany(mappedBy = "problem", cascade = [CascadeType.ALL], fetch = FetchType.LAZY, orphanRemoval = true)
5356
val problemTags: MutableSet<ProblemTag> = mutableSetOf(),
5457

5558
@OneToMany(mappedBy = "problem", cascade = [CascadeType.ALL], fetch = FetchType.LAZY)
@@ -77,4 +80,29 @@ abstract class Problem(
7780
dtype,
7881
)
7982
}
83+
84+
fun addTag(tag: Tag) {
85+
val problemTag = ProblemTag(problem = this, tag = tag)
86+
if (problemTags.map { it.tag.name }.contains(tag.name)) {
87+
throw ConditionConflictException(ErrorCode.TAG_DUPLICATED, "해당 태그가 이미 존재합니다. tagName: ${tag.name}")
88+
}
89+
problemTags.add(problemTag)
90+
}
91+
92+
fun addTags(tags: List<Tag>) {
93+
val newProblemTags = tags.map { ProblemTag(problem = this, tag = it) }
94+
val existProblemTags = problemTags.map { it.tag.name }.intersect(tags.map { it.name }.toSet())
95+
if (existProblemTags.isNotEmpty()) {
96+
throw ConditionConflictException(ErrorCode.TAG_DUPLICATED, "해당 태그가 이미 존재합니다. tagName: $existProblemTags")
97+
}
98+
problemTags.addAll(newProblemTags)
99+
}
100+
101+
fun deleteTag(tag: Tag) {
102+
val problemTag = problemTags.find {
103+
it.tag.name == tag.name
104+
} ?: throw EntityNotFoundException("해당 태그가 존재하지 않습니다. tagName: ${tag.name}")
105+
106+
problemTags.remove(problemTag)
107+
}
80108
}

‎src/main/kotlin/io/csbroker/apiserver/model/Tag.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Tag(
1616
@Column(name = "tag_id")
1717
val id: Long = 0,
1818

19-
@Column(name = "tag_name", columnDefinition = "VARCHAR(30)")
19+
@Column(name = "tag_name", columnDefinition = "VARCHAR(30)", unique = true)
2020
val name: String,
2121

2222
@OneToMany(mappedBy = "tag")

‎src/main/kotlin/io/csbroker/apiserver/repository/problem/ProblemTagRepository.kt

-6
This file was deleted.

‎src/main/kotlin/io/csbroker/apiserver/service/problem/admin/AdminLongProblemServiceImpl.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class AdminLongProblemServiceImpl(
7777
}
7878

7979
longProblem.updateFromDto(updateRequestDto)
80-
tagUpserter.setTags(longProblem, updateRequestDto.tags)
80+
tagUpserter.updateTags(longProblem, updateRequestDto.tags.toMutableList())
8181
return id
8282
}
8383
}

‎src/main/kotlin/io/csbroker/apiserver/service/problem/admin/TagUpserter.kt

+12-27
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,42 @@ import io.csbroker.apiserver.common.exception.ConditionConflictException
55
import io.csbroker.apiserver.model.Problem
66
import io.csbroker.apiserver.model.ProblemTag
77
import io.csbroker.apiserver.model.Tag
8-
import io.csbroker.apiserver.repository.problem.ProblemTagRepository
98
import io.csbroker.apiserver.repository.problem.TagRepository
109
import org.springframework.stereotype.Component
1110

1211
@Component
1312
class TagUpserter(
1413
private val tagRepository: TagRepository,
15-
private val problemTagRepository: ProblemTagRepository,
1614
) {
1715
fun setTags(problem: Problem, tagNames: List<String>) {
1816
if (tagNames.isEmpty()) {
1917
throw ConditionConflictException(ErrorCode.CONDITION_NOT_FULFILLED, "태그의 개수는 1개 이상이여야합니다.")
2018
}
21-
19+
if (problem.problemTags.isNotEmpty()) {
20+
throw ConditionConflictException(ErrorCode.CONDITION_NOT_FULFILLED, "태그가 이미 존재합니다.")
21+
}
2222
val tags = tagRepository.findTagsByNameIn(tagNames)
2323

2424
checkEveryTagExist(tags, tagNames)
25-
26-
val problemTags = tags.map {
27-
ProblemTag(problem = problem, tag = it)
28-
}
29-
30-
problemTagRepository.saveAll(problemTags)
31-
problem.problemTags.addAll(problemTags)
25+
problem.addTags(tags)
3226
}
3327

3428
fun updateTags(problem: Problem, tagNames: MutableList<String>) {
35-
problem.problemTags.removeIf {
36-
if (it.tag.name !in tagNames) {
37-
problemTagRepository.delete(it)
38-
return@removeIf true
39-
}
40-
return@removeIf false
41-
}
42-
43-
tagNames.removeIf {
44-
it in problem.problemTags.map { pt ->
45-
pt.tag.name
46-
}
47-
}
29+
if (isNotChanged(problem, tagNames)) return
4830

31+
problem.problemTags.clear()
4932
val tags = tagRepository.findTagsByNameIn(tagNames)
5033
checkEveryTagExist(tags, tagNames)
5134

52-
val problemTags = tags.map {
53-
ProblemTag(problem = problem, tag = it)
54-
}
55-
35+
val problemTags = tags.map { ProblemTag(problem = problem, tag = it) }
5636
problem.problemTags.addAll(problemTags)
5737
}
5838

39+
private fun isNotChanged(
40+
problem: Problem,
41+
tagNames: MutableList<String>,
42+
) = problem.problemTags.map { it.tag.name }.toSet() == tagNames.toSet()
43+
5944
private fun checkEveryTagExist(
6045
existTags: List<Tag>,
6146
tagNames: List<String>,

‎src/test/kotlin/io/csbroker/apiserver/controller/v1/admin/problem/AdminLongProblemControllerIntegrationTest.kt

+46-13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package io.csbroker.apiserver.controller.v1.admin.problem
33
import io.csbroker.apiserver.controller.IntegrationTest
44
import io.csbroker.apiserver.dto.problem.longproblem.LongProblemUpsertRequestDto
55
import io.csbroker.apiserver.model.LongProblem
6+
import io.csbroker.apiserver.model.ProblemTag
67
import io.csbroker.apiserver.model.StandardAnswer
78
import io.csbroker.apiserver.model.Tag
89
import io.kotest.matchers.shouldBe
@@ -13,29 +14,23 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
1314
class AdminLongProblemControllerIntegrationTest : IntegrationTest() {
1415

1516
@Test
16-
fun `문제 업데이트`() {
17+
fun `문제 업데이트 - 모범 답안`() {
1718
// given
18-
val problem = save(
19-
LongProblem(
20-
title = "문제 제목",
21-
description = "문제 설명",
22-
creator = adminUser,
23-
),
24-
)
25-
save(StandardAnswer(content = "삭제될 모범 답안", longProblem = problem))
26-
save(Tag(name = "tag1"))
19+
val longProblem = LongProblem(title = "문제 제목", description = "문제 설명", creator = adminUser)
20+
save(longProblem)
21+
save(StandardAnswer(content = "삭제될 모범 답안", longProblem = longProblem))
2722
val updateRequestDto = LongProblemUpsertRequestDto(
2823
title = "Test problem",
2924
description = "This is a test problem",
30-
tags = mutableListOf("tag1"),
25+
tags = mutableListOf(),
3126
standardAnswers = listOf("업데이트될 모범 답안"),
3227
gradingStandards = mutableListOf(),
3328
)
3429

3530
// when
3631
val response = request(
3732
method = HttpMethod.PUT,
38-
url = "/api/admin/problems/long/${problem.id}",
33+
url = "/api/admin/problems/long/${longProblem.id}",
3934
isAdmin = true,
4035
body = updateRequestDto,
4136
)
@@ -45,9 +40,47 @@ class AdminLongProblemControllerIntegrationTest : IntegrationTest() {
4540
.andExpect {
4641
val standardAnswers = findAll<StandardAnswer>(
4742
"SELECT s FROM StandardAnswer s where s.longProblem.id = :id",
48-
mapOf("id" to problem.id),
43+
mapOf("id" to longProblem.id),
4944
)
5045
standardAnswers.map { it.content }.toSet() shouldBe updateRequestDto.standardAnswers.toSet()
46+
standardAnswers.size shouldBe updateRequestDto.standardAnswers.size
47+
}
48+
}
49+
50+
@Test
51+
fun `문제 업데이트 - 태그`() {
52+
// given
53+
val oldTag = save(Tag(name = "long-problem-update-tag1"))
54+
val longProblem = LongProblem(title = "문제 제목", description = "문제 설명", creator = adminUser)
55+
longProblem.addTag(oldTag)
56+
save(longProblem)
57+
val newTag1 = save(Tag(name = "${longProblem.id}-tag2"))
58+
val newTag2 = save(Tag(name = "${longProblem.id}-tag3"))
59+
val updateRequestDto = LongProblemUpsertRequestDto(
60+
title = longProblem.title,
61+
description = longProblem.description,
62+
tags = mutableListOf(newTag1.name, newTag2.name),
63+
standardAnswers = emptyList(),
64+
gradingStandards = mutableListOf(),
65+
)
66+
67+
// when
68+
val response = request(
69+
method = HttpMethod.PUT,
70+
url = "/api/admin/problems/long/${longProblem.id}",
71+
isAdmin = true,
72+
body = updateRequestDto,
73+
)
74+
75+
// then
76+
response.andExpect(status().isOk)
77+
.andExpect {
78+
val problemTags = findAll<ProblemTag>(
79+
"SELECT pt FROM ProblemTag pt JOIN FETCH pt.tag where pt.problem.id = :id",
80+
mapOf("id" to longProblem.id),
81+
)
82+
problemTags.map { it.tag.name }.toSet() shouldBe updateRequestDto.tags.toSet()
83+
problemTags.size shouldBe updateRequestDto.tags.size
5184
}
5285
}
5386
}

‎src/test/kotlin/io/csbroker/apiserver/controller/v1/admin/problem/AdminMultipleProblemIntegrationTest.kt

+5-18
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,9 @@ class AdminMultipleProblemIntegrationTest : IntegrationTest() {
2323
isMultiple = false,
2424
),
2525
)
26-
val tag = save(
27-
Tag(
28-
name = "tag1",
29-
),
30-
)
31-
save(
32-
Tag(
33-
name = "tag2",
34-
),
35-
)
36-
save(
37-
ProblemTag(
38-
problem = problem,
39-
tag = tag,
40-
),
41-
)
26+
val tag = save(Tag(name = "${problem.id}-tag1"))
27+
val newTag = save(Tag(name = "${problem.id}-tag2"))
28+
save(ProblemTag(problem = problem, tag = tag))
4229

4330
// when
4431
val response = request(
@@ -58,7 +45,7 @@ class AdminMultipleProblemIntegrationTest : IntegrationTest() {
5845
isAnswer = false,
5946
),
6047
),
61-
tags = listOf("tag2"),
48+
tags = listOf(newTag.name),
6249
score = problem.score,
6350
),
6451
)
@@ -71,7 +58,7 @@ class AdminMultipleProblemIntegrationTest : IntegrationTest() {
7158
mapOf("problemId" to problem.id),
7259
)
7360
problemTags.size shouldBe 1
74-
problemTags[0].tag.name shouldBe "tag2"
61+
problemTags[0].tag.name shouldBe newTag.name
7562
}
7663
}
7764
}

‎src/test/kotlin/io/csbroker/apiserver/service/problem/admin/TagUpserterTest.kt

+14-9
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import io.csbroker.apiserver.model.LongProblem
66
import io.csbroker.apiserver.model.ProblemTag
77
import io.csbroker.apiserver.model.Tag
88
import io.csbroker.apiserver.model.User
9-
import io.csbroker.apiserver.repository.problem.ProblemTagRepository
109
import io.csbroker.apiserver.repository.problem.TagRepository
1110
import io.mockk.clearMocks
1211
import io.mockk.every
@@ -20,8 +19,7 @@ import java.util.UUID
2019

2120
class TagUpserterTest {
2221
private val tagRepository = mockk<TagRepository>()
23-
private val problemTagRepository = mockk<ProblemTagRepository>()
24-
private val tagUpserter = TagUpserter(tagRepository, problemTagRepository)
22+
private val tagUpserter = TagUpserter(tagRepository)
2523

2624
private val user = User(
2725
id = UUID.randomUUID(),
@@ -33,7 +31,7 @@ class TagUpserterTest {
3331

3432
@BeforeEach
3533
fun setUp() {
36-
clearMocks(tagRepository, problemTagRepository)
34+
clearMocks(tagRepository)
3735
}
3836

3937
@Test
@@ -44,16 +42,12 @@ class TagUpserterTest {
4442
val tag1 = Tag(id = 1, name = "tag1")
4543
val tag2 = Tag(id = 2, name = "tag2")
4644
every { tagRepository.findTagsByNameIn(tagNames) } returns listOf(tag1, tag2)
47-
every { problemTagRepository.saveAll(any<List<ProblemTag>>()) } returns emptyList()
4845

4946
// when
5047
tagUpserter.setTags(problem, tagNames)
5148

5249
// then
53-
verify {
54-
tagRepository.findTagsByNameIn(tagNames)
55-
problemTagRepository.saveAll(any<List<ProblemTag>>())
56-
}
50+
verify { tagRepository.findTagsByNameIn(tagNames) }
5751
assertEquals(2, problem.problemTags.size)
5852
val updatedTagSet = problem.problemTags.map { it.tag.name }.toSet()
5953
assert(updatedTagSet.contains("tag1"))
@@ -82,6 +76,17 @@ class TagUpserterTest {
8276
assertThrows<ConditionConflictException> { tagUpserter.setTags(problem, tagNames) }
8377
}
8478

79+
@Test
80+
fun `이미 태그가 존재하는 문제에 태그를 생성할 시 예외가 발생한다`() {
81+
// given
82+
val problem = getLongProblem()
83+
val existTag = Tag(name = "tag1")
84+
problem.problemTags.add(ProblemTag(problem = problem, tag = existTag))
85+
86+
// when, then
87+
assertThrows<ConditionConflictException> { tagUpserter.setTags(problem, listOf("newTag!!!")) }
88+
}
89+
8590
@Test
8691
fun `존재하지 않는 태그를 업데이트하려고 하면 예외가 발생한다`() {
8792
// given

0 commit comments

Comments
 (0)