Skip to content

Commit d7dcf52

Browse files
dkrasnoffdkrasnoff
committed
fixup! KTL-4388: Moved out to a separate table maven artifact data.
1 parent e4ea1e5 commit d7dcf52

3 files changed

Lines changed: 82 additions & 25 deletions

File tree

‎app/src/test/kotlin/io/klibs/core/pckg/service/MavenArtifactServiceTest.kt‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@ import io.klibs.core.pckg.entity.MavenArtifactEntity
55
import io.klibs.core.pckg.dto.MavenCoordinatesDTO
66
import io.klibs.core.pckg.repository.MavenArtifactRepository
77
import org.junit.jupiter.api.Test
8+
import org.junit.jupiter.api.assertThrows
9+
import org.mockito.Mockito.doReturn
10+
import org.mockito.Mockito.verify
11+
import org.mockito.kotlin.eq
12+
import org.mockito.kotlin.whenever
813
import org.springframework.beans.factory.annotation.Autowired
14+
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean
915
import org.springframework.test.context.ActiveProfiles
1016
import kotlin.test.assertEquals
1117
import kotlin.test.assertNotNull
@@ -15,14 +21,14 @@ import kotlin.test.assertTrue
1521
class MavenArtifactServiceTest : BaseUnitWithDbLayerTest() {
1622

1723
@Autowired
18-
private lateinit var mavenArtifactService: MavenArtifactService
24+
private lateinit var uut: MavenArtifactService
1925

20-
@Autowired
26+
@MockitoSpyBean
2127
private lateinit var mavenArtifactRepository: MavenArtifactRepository
2228

2329
@Test
2430
fun `resolveOrCreateAll returns empty map for empty input`() {
25-
val result = mavenArtifactService.resolveOrCreateAll(emptySet())
31+
val result = uut.resolveOrCreateAll(emptySet())
2632

2733
assertTrue(result.isEmpty())
2834
assertEquals(0L, mavenArtifactRepository.count())
@@ -36,7 +42,7 @@ class MavenArtifactServiceTest : BaseUnitWithDbLayerTest() {
3642
MavenCoordinatesDTO("io.klibs", "beta", "1.0.0"),
3743
)
3844

39-
val resolved = mavenArtifactService.resolveOrCreateAll(coords)
45+
val resolved = uut.resolveOrCreateAll(coords)
4046

4147
assertEquals(coords, resolved.keys)
4248
val ids = resolved.values.map { requireNotNull(it.id) }
@@ -64,11 +70,46 @@ class MavenArtifactServiceTest : BaseUnitWithDbLayerTest() {
6470
MavenCoordinatesDTO("io.klibs", "alpha", "2.0.0"),
6571
)
6672

67-
val firstRun = mavenArtifactService.resolveOrCreateAll(coords)
68-
val secondRun = mavenArtifactService.resolveOrCreateAll(coords)
73+
val firstRun = uut.resolveOrCreateAll(coords)
74+
val secondRun = uut.resolveOrCreateAll(coords)
6975

7076
assertEquals(preSavedEntityId, firstRun.getValue(MavenCoordinatesDTO("io.klibs", "alpha", "1.0.0")).id)
71-
assertEquals(firstRun.mapValues { it.value.id }, secondRun.mapValues { it.value.id },)
77+
assertEquals(firstRun.mapValues { it.value.id }, secondRun.mapValues { it.value.id })
7278
assertEquals(2L, mavenArtifactRepository.count())
7379
}
80+
81+
@Test
82+
fun `resolveOrCreate recovers existing row when saveIfAbsent loses the race`() {
83+
val coords = MavenCoordinatesDTO("io.klibs", "race", "1.0.0")
84+
val concurrentlyInserted = mavenArtifactRepository.save(
85+
MavenArtifactEntity(groupId = coords.groupId, artifactId = coords.artifactId, version = coords.version)
86+
)
87+
val expectedId = requireNotNull(concurrentlyInserted.id)
88+
89+
doReturn(null, concurrentlyInserted)
90+
.whenever(mavenArtifactRepository)
91+
.findByGroupIdAndArtifactIdAndVersion(eq(coords.groupId), eq(coords.artifactId), eq(coords.version))
92+
93+
val result = uut.resolveOrCreate(coords)
94+
95+
assertEquals(expectedId, result.id)
96+
assertEquals(coords.groupId, result.groupId)
97+
assertEquals(coords.artifactId, result.artifactId)
98+
assertEquals(coords.version, result.version)
99+
verify(mavenArtifactRepository).saveIfAbsent(coords.groupId, coords.artifactId, coords.version)
100+
}
101+
102+
@Test
103+
fun `insertOrLookup throws when saveIfAbsent lost race but row cannot be re-read`() {
104+
val coords = MavenCoordinatesDTO("io.klibs", "missing", "1.0.0")
105+
doReturn(0L)
106+
.whenever(mavenArtifactRepository)
107+
.saveIfAbsent(eq(coords.groupId), eq(coords.artifactId), eq(coords.version))
108+
doReturn(null)
109+
.whenever(mavenArtifactRepository)
110+
.findByGroupIdAndArtifactIdAndVersion(eq(coords.groupId), eq(coords.artifactId), eq(coords.version))
111+
112+
val ex = assertThrows<IllegalArgumentException> { uut.resolveOrCreate(coords) }
113+
assertTrue(ex.message!!.contains("maven_artifact row is still missing after upsert"))
114+
}
74115
}

‎core/package/src/main/kotlin/io/klibs/core/pckg/repository/MavenArtifactRepository.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.klibs.core.pckg.repository
22

33
import io.klibs.core.pckg.entity.MavenArtifactEntity
4+
import org.springframework.data.jpa.repository.Modifying
45
import org.springframework.data.jpa.repository.Query
56
import org.springframework.data.repository.CrudRepository
67
import org.springframework.data.repository.query.Param
@@ -13,6 +14,20 @@ interface MavenArtifactRepository : CrudRepository<MavenArtifactEntity, Long> {
1314
version: String,
1415
): MavenArtifactEntity?
1516

17+
@Modifying
18+
@Query(
19+
value = """
20+
INSERT INTO MavenArtifactEntity (groupId, artifactId, version)
21+
VALUES (:groupId, :artifactId, :version)
22+
ON CONFLICT (groupId, artifactId, version) DO NOTHING
23+
""",
24+
)
25+
fun saveIfAbsent(
26+
@Param("groupId") groupId: String,
27+
@Param("artifactId") artifactId: String,
28+
@Param("version") version: String,
29+
): Long
30+
1631
/**
1732
* Bulk-fetches the rows whose `(groupId, artifactId, version)` triple matches any element
1833
* of [keys]. Each key must be packed as `"$groupId|$artifactId|$version"`.

‎core/package/src/main/kotlin/io/klibs/core/pckg/service/MavenArtifactService.kt‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,18 @@ import io.klibs.core.pckg.dto.MavenArtifactDTO
44
import io.klibs.core.pckg.entity.MavenArtifactEntity
55
import io.klibs.core.pckg.dto.MavenCoordinatesDTO
66
import io.klibs.core.pckg.repository.MavenArtifactRepository
7-
import org.springframework.dao.DataIntegrityViolationException
87
import org.springframework.stereotype.Service
8+
import org.springframework.transaction.annotation.Propagation
99
import org.springframework.transaction.annotation.Transactional
1010

1111
@Service
12-
@Transactional
12+
@Transactional(propagation = Propagation.REQUIRES_NEW)
1313
class MavenArtifactService(
1414
private val mavenArtifactRepository: MavenArtifactRepository,
1515
) {
1616

17+
private val log = org.slf4j.LoggerFactory.getLogger(MavenArtifactService::class.java)
18+
1719
/**
1820
* Resolves the given [coordinates] to their `maven_artifact` rows, inserting any
1921
* coordinates that are not yet present.
@@ -53,24 +55,23 @@ class MavenArtifactService(
5355
return MavenArtifactDTO.fromEntity(existing ?: insertOrLookup(coords))
5456
}
5557

58+
/**
59+
* Inserts a `maven_artifact` row for the given [coords] if it does not exist yet, then
60+
* reads and returns the row.
61+
*/
5662
private fun insertOrLookup(coords: MavenCoordinatesDTO): MavenArtifactEntity {
57-
return try {
58-
mavenArtifactRepository.save(
59-
MavenArtifactEntity(
60-
groupId = coords.groupId,
61-
artifactId = coords.artifactId,
62-
version = coords.version,
63-
)
63+
val inserted = mavenArtifactRepository.saveIfAbsent(
64+
coords.groupId, coords.artifactId, coords.version,
65+
)
66+
if (inserted == 0.toLong()) {
67+
log.debug("Lost race on ${coords.groupId}:${coords.artifactId}:${coords.version}, re-reading it")
68+
}
69+
return requireNotNull(
70+
mavenArtifactRepository.findByGroupIdAndArtifactIdAndVersion(
71+
coords.groupId, coords.artifactId, coords.version,
6472
)
65-
} catch (_: DataIntegrityViolationException) {
66-
// A concurrent transaction inserted the same GAV first — re-read it.
67-
requireNotNull(
68-
mavenArtifactRepository.findByGroupIdAndArtifactIdAndVersion(
69-
coords.groupId, coords.artifactId, coords.version,
70-
)
71-
) {
72-
"Lost race on maven_artifact insert and the row is still missing for $coords"
73-
}
73+
) {
74+
"maven_artifact row is still missing after upsert for $coords"
7475
}
7576
}
7677

0 commit comments

Comments
 (0)