Skip to content

Commit

Permalink
fix: published filter has hits when unpublished concept has earlier p…
Browse files Browse the repository at this point in the history
…ublished version
  • Loading branch information
NilsOveTen committed Nov 27, 2024
1 parent f78f6da commit 048e659
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 61 deletions.
2 changes: 1 addition & 1 deletion docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ services:
- discovery.type=single-node
- xpack.security.enabled=true
- ELASTIC_PASSWORD=elasticpwd
- ES_JAVA_OPTS=-Xms2G -Xmx2G
- ES_JAVA_OPTS=-Xms512m -Xmx512m
13 changes: 11 additions & 2 deletions src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,21 @@ class ElasticUpdater(
currentConceptRepository.deleteAll()
} catch (_: Exception) { }

conceptRepository.findAll<BegrepDBO>()
val groupedByOriginalId = conceptRepository.findAll<BegrepDBO>()

Check warning on line 28 in src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt#L28

Added line #L28 was not covered by tests
.groupBy { concept -> concept.originaltBegrep }

val idsOfHighestPublishedVersion: Map<String, String?> = groupedByOriginalId.mapValues {
it.value
.filter { concept -> concept.erPublisert }
.maxByOrNull { concept -> concept.versjonsnr }

Check warning on line 34 in src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt#L31-L34

Added lines #L31 - L34 were not covered by tests
?.id
}

groupedByOriginalId

Check warning on line 38 in src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt#L38

Added line #L38 was not covered by tests
.mapNotNull { pair -> pair.value.maxByOrNull { concept -> concept.versjonsnr } }
.forEach {
logger.debug("reindexing ${it.id}, ${it.ansvarligVirksomhet.id}")
currentConceptRepository.save(CurrentConcept(it))
currentConceptRepository.save(CurrentConcept(it, idsOfHighestPublishedVersion[it.originaltBegrep]))

Check warning on line 42 in src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/no/fdk/concept_catalog/elastic/ElasticUpdater.kt#L42

Added line #L42 was not covered by tests
}

logger.info("finished reindexing elastic")
Expand Down
31 changes: 17 additions & 14 deletions src/main/kotlin/no/fdk/concept_catalog/model/CurrentConcept.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ data class CurrentConcept(
val begrepsRelasjon: List<BegrepsRelasjon>?,
val internBegrepsRelasjon: List<BegrepsRelasjon>?,
val interneFelt: Map<String, InterntFelt>?,
val internErstattesAv: List<String>?
val internErstattesAv: List<String>?,
val sistPublisertId: String?
) {
constructor(dbo: BegrepDBO) : this(
constructor(dbo: BegrepDBO, latestPublishedId: String?) : this(
dbo.id, dbo.originaltBegrep, dbo.versjonsnr, dbo.revisjonAv,
dbo.status, dbo.statusURI, dbo.erPublisert, dbo.publiseringsTidspunkt,
dbo.anbefaltTerm, dbo.tillattTerm, dbo.frarådetTerm, dbo.definisjon,
Expand All @@ -68,19 +69,21 @@ data class CurrentConcept(
dbo.gyldigFom, dbo.gyldigTom, dbo.endringslogelement, dbo.opprettet,
dbo.opprettetAv, dbo.seOgså, dbo.internSeOgså, dbo.erstattesAv, dbo.assignedUser,
dbo.abbreviatedLabel, dbo.begrepsRelasjon, dbo.internBegrepsRelasjon, dbo.interneFelt,
dbo.internErstattesAv
dbo.internErstattesAv, latestPublishedId
)

fun toDBO(): BegrepDBO =
BegrepDBO(
idOfThisVersion, originaltBegrep, versjonsnr, revisjonAv,
status, statusURI, erPublisert, publiseringsTidspunkt,
anbefaltTerm, tillattTerm, frarådetTerm, definisjon,
definisjonForAllmennheten, definisjonForSpesialister,
merknad, merkelapp, ansvarligVirksomhet, eksempel,
fagområde, fagområdeKoder, omfang, kontaktpunkt,
gyldigFom, gyldigTom, endringslogelement, opprettet,
opprettetAv, seOgså, internSeOgså, erstattesAv, assignedUser,
abbreviatedLabel, begrepsRelasjon, internBegrepsRelasjon, interneFelt, internErstattesAv
fun toDTO(): Begrep =
Begrep(
id = idOfThisVersion, originaltBegrep = originaltBegrep, versjonsnr = versjonsnr, revisjonAv = revisjonAv,
status = status, statusURI = statusURI, erPublisert = erPublisert, publiseringsTidspunkt = publiseringsTidspunkt,
anbefaltTerm = anbefaltTerm, tillattTerm = tillattTerm, frarådetTerm = frarådetTerm, definisjon = definisjon,
definisjonForAllmennheten = definisjonForAllmennheten, definisjonForSpesialister = definisjonForSpesialister,
merknad = merknad, merkelapp = merkelapp, ansvarligVirksomhet = ansvarligVirksomhet, eksempel = eksempel,
fagområde = fagområde, fagområdeKoder = fagområdeKoder, omfang = omfang, kontaktpunkt = kontaktpunkt,
gyldigFom = gyldigFom, gyldigTom = gyldigTom, endringslogelement = endringslogelement, opprettet = opprettet,
opprettetAv = opprettetAv, seOgså = seOgså, internSeOgså = internSeOgså, erstattesAv = erstattesAv,
assignedUser = assignedUser, abbreviatedLabel = abbreviatedLabel, begrepsRelasjon = begrepsRelasjon,
internBegrepsRelasjon = internBegrepsRelasjon, interneFelt = interneFelt, internErstattesAv = internErstattesAv,
sistPublisertId = sistPublisertId
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,16 @@ fun SearchFilters.asQueryFilters(orgNumber: String): List<Query> {

if (published != null) {
queryFilters.add(Query.of { queryBuilder ->
queryBuilder.term { termBuilder ->
termBuilder.field("erPublisert").value(FieldValue.of(published.value))
queryBuilder.bool { boolBuilder ->
if (published.value) {
boolBuilder.must { mustBuilder ->
mustBuilder.exists { existsBuilder -> existsBuilder.field("sistPublisertId") }
}
} else {
boolBuilder.mustNot { mustNotBuilder ->
mustNotBuilder.exists { existsBuilder -> existsBuilder.field("sistPublisertId") }
}
}
}
})
}
Expand Down
34 changes: 22 additions & 12 deletions src/main/kotlin/no/fdk/concept_catalog/service/ConceptService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,25 @@ class ConceptService(
private val mapper: ObjectMapper
) {

fun deleteConcept(concept: BegrepDBO) {
if (concept.id == concept.originaltBegrep) {
currentConceptRepository.delete(CurrentConcept(concept))
private fun updateCurrentConceptForOriginalId(originalId: String) {
val allVersions = conceptRepository.getByOriginaltBegrep(originalId)
val newCurrent = allVersions.maxByOrNull { it.versjonsnr }

if (newCurrent == null && currentConceptRepository.existsById(originalId)) {
currentConceptRepository.deleteById(originalId)
} else if (newCurrent != null) {
val latestPublishedId = allVersions.filter { it.erPublisert }
.maxByOrNull { it.versjonsnr }
?.id
currentConceptRepository.save(CurrentConcept(newCurrent, latestPublishedId))
}
}

fun deleteConcept(concept: BegrepDBO) {
conceptRepository.delete(concept)
.also { logger.debug("deleted concept ${concept.id}") }

updateCurrentConceptForOriginalId(concept.originaltBegrep)
}

fun getConceptById(id: String): Begrep? =
Expand Down Expand Up @@ -209,10 +222,9 @@ class ConceptService(
): List<Begrep> {
val locations = conceptsAndOperations.map { historyService.updateHistory(it.key, it.value, user, jwt) }
try {
conceptsAndOperations.keys
.map { CurrentConcept(it) }
.run { currentConceptRepository.saveAll(this) }
return conceptRepository.saveAll(conceptsAndOperations.keys).map { it.withHighestVersionDTO() }
return conceptRepository.saveAll(conceptsAndOperations.keys)
.onEach { updateCurrentConceptForOriginalId(it.originaltBegrep) }
.map { it.withHighestVersionDTO() }
} catch (ex: Exception) {
logger.error("save failed, removing history update", ex)
locations.filterNotNull().forEach { historyService.removeHistoryUpdate(it, jwt) }
Expand Down Expand Up @@ -249,15 +261,15 @@ class ConceptService(
conceptRepository.getByOriginaltBegrep(originaltBegrep)
.filter { it.erPublisert }
.maxByOrNull { concept -> concept.versjonsnr }
?.let { it.toDTO(it.versjonsnr, it.id, findIdOfUnpublishedRevision(it)) }
?.let { it.toDTO(null, null, it.id) }
}

fun getLastPublishedForOrganization(orgNr: String): List<Begrep> =
conceptRepository.getBegrepByAnsvarligVirksomhetId(orgNr)
.filter { it.erPublisert }
.sortedByDescending {concept -> concept.versjonsnr }
.distinctBy {concept -> concept.originaltBegrep }
.map { it.toDTO(it.versjonsnr, it.id, findIdOfUnpublishedRevision(it)) }
.map { it.toDTO(null, null, it.id) }

fun getLatestVersion(originalId: String): BegrepDBO? =
conceptRepository.getByOriginaltBegrep(originalId)
Expand All @@ -266,9 +278,8 @@ class ConceptService(
fun searchConcepts(orgNumber: String, search: SearchOperation): Paginated {
val hits = conceptSearchService.searchCurrentConcepts(orgNumber, search)
return hits.map { it.content }
.map { it.toDBO() }
.map { it.withHighestVersionDTO() }
.toList()
.map { it.toDTO() }
.asPaginatedWrapDTO(hits.totalHits, search.pagination)
}

Expand Down Expand Up @@ -348,7 +359,6 @@ class ConceptService(

conceptPublisher.send(concept.ansvarligVirksomhet.id)

currentConceptRepository.save(CurrentConcept(published))
return conceptRepository.save(published)
.also { updateRelationsToNonInternal(it) }
.withHighestVersionDTO()
Expand Down
50 changes: 25 additions & 25 deletions src/test/kotlin/no/fdk/concept_catalog/contract/SearchConcepts.kt
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_2.fromSearch()), result.hits)

}

Expand All @@ -95,7 +95,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch()), result.hits)

}

Expand All @@ -117,7 +117,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch()), result.hits)
}

@Test
Expand All @@ -139,7 +139,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_0), result.hits)
assertEquals(listOf(BEGREP_0.fromSearch()), result.hits)
}

@Test
Expand All @@ -161,7 +161,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_0), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_0.fromSearch()), result.hits)
}

@Test
Expand All @@ -185,10 +185,10 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), unPublishedResponse["status"])

val unPublished: Paginated = mapper.readValue(unPublishedResponse["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_2), unPublished.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_2.fromSearch()), unPublished.hits)

val published: Paginated = mapper.readValue(publishedResponse["body"] as String)
assertEquals(listOf(BEGREP_0), published.hits)
assertEquals(listOf(BEGREP_0.fromSearch()), published.hits)
}

@Test
Expand Down Expand Up @@ -222,10 +222,10 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), withSubjectFagomr3Response["status"])

val withSubjectFagomr1: Paginated = mapper.readValue(withSubjectFagomr1Response["body"] as String)
assertEquals(listOf(BEGREP_5), withSubjectFagomr1.hits)
assertEquals(listOf(BEGREP_5.fromSearch()), withSubjectFagomr1.hits)

val withSubjectFagomr3: Paginated = mapper.readValue(withSubjectFagomr3Response["body"] as String)
assertEquals(listOf(BEGREP_4, BEGREP_5), withSubjectFagomr3.hits)
assertEquals(listOf(BEGREP_4.fromSearch(), BEGREP_5.fromSearch()), withSubjectFagomr3.hits)
}

@Test
Expand Down Expand Up @@ -259,7 +259,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), withoutInternalFieldsResponse["status"])

val withInternalFields: Paginated = mapper.readValue(withInternalFieldsResponse["body"] as String)
assertEquals(listOf(BEGREP_4), withInternalFields.hits)
assertEquals(listOf(BEGREP_4.fromSearch()), withInternalFields.hits)

val withoutInternalFields: Paginated = mapper.readValue(withoutInternalFieldsResponse["body"] as String)
assertEquals(emptyList(), withoutInternalFields.hits)
Expand Down Expand Up @@ -296,7 +296,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), withoutLabelResponse["status"])

val withInternalFields: Paginated = mapper.readValue(withLabelResponse["body"] as String)
assertEquals(listOf(BEGREP_0), withInternalFields.hits)
assertEquals(listOf(BEGREP_0.fromSearch()), withInternalFields.hits)

val withoutInternalFields: Paginated = mapper.readValue(withoutLabelResponse["body"] as String)
assertEquals(emptyList(), withoutInternalFields.hits)
Expand Down Expand Up @@ -327,7 +327,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_2.fromSearch()), result.hits)
}

@Test
Expand All @@ -344,7 +344,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), titleResponse["status"])

val titleResult: Paginated = mapper.readValue(titleResponse["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_2), titleResult.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_2.fromSearch()), titleResult.hits)

val descriptionResponse = authorizedRequest(
"/begreper/search?orgNummer=123456789",
Expand Down Expand Up @@ -373,7 +373,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), statusResponse["status"])

val statusResult: Paginated = mapper.readValue(statusResponse["body"] as String)
assertEquals(listOf(BEGREP_2), statusResult.hits)
assertEquals(listOf(BEGREP_2.fromSearch()), statusResult.hits)
}

@Test
Expand All @@ -391,7 +391,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), response["status"])

val titleResult: Paginated = mapper.readValue(response["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_0, BEGREP_2), titleResult.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_0.fromSearch(), BEGREP_2.fromSearch()), titleResult.hits)
}

@Test
Expand All @@ -408,7 +408,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_2.fromSearch()), result.hits)

}

Expand All @@ -423,7 +423,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch()), result.hits)

}

Expand All @@ -441,7 +441,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_0, BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_0.fromSearch(), BEGREP_2.fromSearch()), result.hits)
}

@Test
Expand All @@ -462,7 +462,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_2.fromSearch()), result.hits)
}

@Test
Expand All @@ -476,7 +476,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_0), result.hits)
assertEquals(listOf(BEGREP_0.fromSearch()), result.hits)
}

@Test
Expand Down Expand Up @@ -508,7 +508,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_0, BEGREP_2), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_0.fromSearch(), BEGREP_2.fromSearch()), result.hits)
}

@Test
Expand Down Expand Up @@ -549,8 +549,8 @@ class SearchConcepts : ApiTestContext() {

val result0: Paginated = mapper.readValue(rsp0["body"] as String)
val result1: Paginated = mapper.readValue(rsp1["body"] as String)
assertEquals(listOf(BEGREP_1, BEGREP_0), result0.hits)
assertEquals(listOf(BEGREP_2), result1.hits)
assertEquals(listOf(BEGREP_1.fromSearch(), BEGREP_0.fromSearch()), result0.hits)
assertEquals(listOf(BEGREP_2.fromSearch()), result1.hits)
}
}

Expand All @@ -569,7 +569,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_2, BEGREP_0, BEGREP_1), result.hits)
assertEquals(listOf(BEGREP_2.fromSearch(), BEGREP_0.fromSearch(), BEGREP_1.fromSearch()), result.hits)
}

@Test
Expand Down Expand Up @@ -631,7 +631,7 @@ class SearchConcepts : ApiTestContext() {
assertEquals(HttpStatus.OK.value(), rsp["status"])

val result: Paginated = mapper.readValue(rsp["body"] as String)
assertEquals(listOf(BEGREP_1), result.hits)
assertEquals(listOf(BEGREP_1.fromSearch()), result.hits)
}

@Test
Expand Down
Loading

0 comments on commit 048e659

Please sign in to comment.