From e9b99fa608fc92ccc9854b4251317937ba8b814b Mon Sep 17 00:00:00 2001 From: Mateusz Filipowicz Date: Mon, 21 Mar 2022 10:25:06 +0100 Subject: [PATCH] fix: recording statistics randomly failing due to lack of ongoing DB session to join fetch stats history --- .../ambassador/analysis/AnalysisService.kt | 16 +++++++++-- .../storage/project/ProjectEntity.kt | 20 -------------- .../project/ProjectEntityRepository.kt | 2 -- .../project/ProjectStatisticsHistory.kt | 7 +++-- .../ProjectStatisticsHistoryRepository.kt | 12 ++++++--- .../storage/project/ProjectEntityTest.kt | 27 +++++++++---------- 6 files changed, 37 insertions(+), 47 deletions(-) diff --git a/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt b/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt index 6f58a58f..d1170c05 100644 --- a/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt +++ b/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt @@ -11,20 +11,27 @@ import com.roche.ambassador.model.project.Project import com.roche.ambassador.model.source.ProjectSources import com.roche.ambassador.storage.project.ProjectEntity import com.roche.ambassador.storage.project.ProjectEntityRepository +import com.roche.ambassador.storage.project.ProjectStatisticsHistory +import com.roche.ambassador.storage.project.ProjectStatisticsHistoryRepository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import org.springframework.stereotype.Service +import org.springframework.transaction.PlatformTransactionManager import org.springframework.transaction.annotation.Propagation import org.springframework.transaction.annotation.Transactional +import org.springframework.transaction.support.TransactionTemplate @Service internal class AnalysisService( private val scorecardConfiguration: ScorecardConfiguration, private val projectEntityRepository: ProjectEntityRepository, + private val projectStatisticsHistoryRepository: ProjectStatisticsHistoryRepository, private val projectSources: ProjectSources, + platformTransactionManager: PlatformTransactionManager, concurrencyProvider: ConcurrencyProvider ) { + private val transactionTemplate: TransactionTemplate = TransactionTemplate(platformTransactionManager) private val analysisScope: CoroutineScope = CoroutineScope(concurrencyProvider.getSupportingDispatcher()) private val calculator: ScorecardCalculator = ScorecardCalculator(scorecardConfiguration) @@ -61,8 +68,13 @@ internal class AnalysisService( try { entity.project = analyze(entity.project) entity.updateScore(entity.project) - entity.recordStatistics() - projectEntityRepository.save(entity) + transactionTemplate.executeWithoutResult { + val savedEntity = projectEntityRepository.save(entity) + val historyEntry = ProjectStatisticsHistory.from(savedEntity) + val entryDate = historyEntry.date.toLocalDate().atStartOfDay() + projectStatisticsHistoryRepository.deleteByProjectIdAndDateBetween(historyEntry.projectId, entryDate, entryDate.plusDays(1)) + projectStatisticsHistoryRepository.save(historyEntry) + } progressMonitor.success() } catch (e: Throwable) { log.error("Failed to analyze project '{}' (id={}).", entity.project.fullName, entity.project.id, e) diff --git a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt index 5755818e..23757830 100644 --- a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt +++ b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt @@ -2,7 +2,6 @@ package com.roche.ambassador.storage.project import com.roche.ambassador.model.project.Project import com.vladmihalcea.hibernate.type.json.JsonBinaryType -import org.hibernate.annotations.BatchSize import org.hibernate.annotations.Type import org.hibernate.annotations.TypeDef import java.time.LocalDate @@ -13,10 +12,6 @@ import javax.persistence.* @Entity @Table(name = "project") @TypeDef(name = "jsonb", typeClass = JsonBinaryType::class) -@NamedEntityGraph( - name = "Project.statsHistory", - attributeNodes = [NamedAttributeNode("statsHistory")] -) class ProjectEntity( @Id var id: Long? = null, var name: String? = null, @@ -36,15 +31,6 @@ class ProjectEntity( var lastIndexedDate: LocalDateTime = LocalDateTime.now(), @Column(name = "last_analysis_date") var lastAnalysisDate: LocalDateTime? = null, - @OneToMany( - mappedBy = "project", - cascade = [CascadeType.ALL], - fetch = FetchType.LAZY, - orphanRemoval = true - ) - @BatchSize(size = 25) - @OrderBy("date") - var statsHistory: MutableList = mutableListOf(), var source: String? = null, @Column(name = "last_indexing_id") var lastIndexingId: UUID? = null, // mapping is not needed here yet, thus not adding it @@ -64,12 +50,6 @@ class ProjectEntity( fun wasIndexedBefore(otherDate: LocalDate): Boolean = lastIndexedDate.isBefore(otherDate.atStartOfDay()) - fun recordStatistics(): ProjectStatisticsHistory { - val historyEntry = ProjectStatisticsHistory.from(this) - statsHistory.add(historyEntry) - return historyEntry - } - fun updateIndex(project: Project) { this.name = project.name this.project = project diff --git a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntityRepository.kt b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntityRepository.kt index 6e934a48..1823c4a8 100644 --- a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntityRepository.kt +++ b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntityRepository.kt @@ -2,7 +2,6 @@ package com.roche.ambassador.storage.project import com.roche.ambassador.storage.Lookup import org.hibernate.jpa.QueryHints.HINT_CACHEABLE -import org.springframework.data.jpa.repository.EntityGraph import org.springframework.data.jpa.repository.Modifying import org.springframework.data.jpa.repository.Query import org.springframework.data.jpa.repository.QueryHints @@ -23,7 +22,6 @@ interface ProjectEntityRepository : PagingAndSortingRepository fun countAllBySubscribed(subscribed: Boolean): Long diff --git a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistory.kt b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistory.kt index e0d4d2f4..29b643d1 100644 --- a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistory.kt +++ b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistory.kt @@ -12,9 +12,8 @@ import javax.persistence.* @TypeDef(name = "jsonb", typeClass = JsonBinaryType::class) class ProjectStatisticsHistory( @Id @GeneratedValue var id: UUID? = null, - @ManyToOne - @JoinColumn(name = "project_id", nullable = false, updatable = false) - var project: ProjectEntity, + @Column(name = "project_id", nullable = false, updatable = false) + var projectId: Long, @Type(type = "jsonb") @Column(name = "stats", columnDefinition = "jsonb", updatable = false) @Basic(fetch = FetchType.EAGER) @@ -27,7 +26,7 @@ class ProjectStatisticsHistory( fun from(projectEntity: ProjectEntity): ProjectStatisticsHistory { val stats = ProjectStatistics.from(projectEntity.project) return ProjectStatisticsHistory( - null, projectEntity, stats, projectEntity.lastIndexedDate + null, projectEntity.id!!, stats, projectEntity.lastIndexedDate ) } } diff --git a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistoryRepository.kt b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistoryRepository.kt index 0a998ef4..7f914b37 100644 --- a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistoryRepository.kt +++ b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistoryRepository.kt @@ -1,5 +1,6 @@ package com.roche.ambassador.storage.project +import org.springframework.data.jpa.repository.Modifying import org.springframework.data.jpa.repository.Query import org.springframework.data.repository.CrudRepository import org.springframework.data.repository.query.Param @@ -8,16 +9,19 @@ import java.util.* interface ProjectStatisticsHistoryRepository : CrudRepository { - @Query("FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id ORDER BY ph.date DESC") + @Query("FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id ORDER BY ph.date DESC") fun findByProjectId(@Param("id") id: Long): List - @Query("FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date >= :startDate ORDER BY ph.date DESC") + @Query("FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date >= :startDate ORDER BY ph.date DESC") fun findByProjectIdAndDateGreaterThanEqual(@Param("id") id: Long, @Param("startDate") startDate: LocalDateTime): List - @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date < :endDate ORDER BY ph.date DESC") + @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date < :endDate ORDER BY ph.date DESC") fun findByProjectIdAndDateLessThan(@Param("id") id: Long, @Param("endDate") endDate: LocalDateTime): List - @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date BETWEEN :startDate AND :endDate ORDER BY ph.date DESC") + @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date BETWEEN :startDate AND :endDate ORDER BY ph.date DESC") fun findByProjectIdAndDateBetween(@Param("id") id: Long, @Param("startDate") startDate: LocalDateTime, @Param("endDate") endDate: LocalDateTime): List + @Query("DELETE FROM ProjectStatisticsHistory ph WHERE ph.projectId = :projectId AND ph.date BETWEEN :startDate AND :endDate") + @Modifying + fun deleteByProjectIdAndDateBetween(@Param("projectId") projectId: Long, @Param("startDate") startDate: LocalDateTime, @Param("endDate") endDate: LocalDateTime) } \ No newline at end of file diff --git a/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt b/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt index 74797235..4da95141 100644 --- a/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt +++ b/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt @@ -4,10 +4,7 @@ import com.roche.ambassador.model.Visibility import com.roche.ambassador.model.project.Permissions import com.roche.ambassador.model.project.Project import com.roche.ambassador.model.stats.Statistics -import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.Test import java.time.LocalDate -import java.time.LocalDateTime class ProjectEntityTest { @@ -21,16 +18,16 @@ class ProjectEntityTest { Permissions(true, true) ) - @Test - fun `should create new stats history entry when recording stats`() { - val project = ProjectEntity(project = createProject(), lastIndexedDate = LocalDateTime.now(),) - - val history = project.recordStatistics() - - assertThat(project.statsHistory).hasSize(1) - .containsExactly(history) - assertThat(history) - .extracting(ProjectStatisticsHistory::date, ProjectStatisticsHistory::project) - .containsExactly(project.lastIndexedDate, project) - } +// @Test +// fun `should create new stats history entry when recording stats`() { +// val project = ProjectEntity(project = createProject(), lastIndexedDate = LocalDateTime.now(),) +// +// val history = project.recordStatistics() +// +// assertThat(project.statsHistory).hasSize(1) +// .containsExactly(history) +// assertThat(history) +// .extracting(ProjectStatisticsHistory::date, ProjectStatisticsHistory::project) +// .containsExactly(project.lastIndexedDate, project) +// } }