Skip to content

Commit

Permalink
fix(search): fix bug when deep paginating over legal entites and addr…
Browse files Browse the repository at this point in the history
…ess business partners

* fixes bug leading to 500 due to opensearch not supporting deep paginations
* logic now distinguishes between pagination with search parameters and without
* with search parameters put a limit to page number due to opensearch limitations
* without search parameters just paginate in database directly without opensearch
  • Loading branch information
nicoprow committed Feb 9, 2023
1 parent 5c853af commit 8d07508
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.eclipse.tractusx.bpdm.pool.component.opensearch.impl.doc.TextDoc
import org.eclipse.tractusx.bpdm.pool.component.opensearch.impl.repository.AddressDocSearchRepository
import org.eclipse.tractusx.bpdm.pool.component.opensearch.impl.repository.LegalEntityDocSearchRepository
import org.eclipse.tractusx.bpdm.pool.component.opensearch.impl.repository.TextDocSearchRepository
import org.eclipse.tractusx.bpdm.pool.config.OpenSearchConfigProperties
import org.eclipse.tractusx.bpdm.pool.dto.request.AddressPartnerSearchRequest
import org.eclipse.tractusx.bpdm.pool.dto.request.BusinessPartnerSearchRequest
import org.eclipse.tractusx.bpdm.pool.dto.request.PaginationRequest
Expand All @@ -38,12 +39,10 @@ import org.eclipse.tractusx.bpdm.pool.dto.response.LegalEntityMatchResponse
import org.eclipse.tractusx.bpdm.pool.dto.response.SuggestionResponse
import org.eclipse.tractusx.bpdm.pool.entity.AddressPartner
import org.eclipse.tractusx.bpdm.pool.entity.LegalEntity
import org.eclipse.tractusx.bpdm.pool.exception.BpdmOpenSearchUserException
import org.eclipse.tractusx.bpdm.pool.repository.AddressPartnerRepository
import org.eclipse.tractusx.bpdm.pool.repository.LegalEntityRepository
import org.eclipse.tractusx.bpdm.pool.service.AddressService
import org.eclipse.tractusx.bpdm.pool.service.BusinessPartnerFetchService
import org.eclipse.tractusx.bpdm.pool.service.toBusinessPartnerMatchDto
import org.eclipse.tractusx.bpdm.pool.service.toMatchDto
import org.eclipse.tractusx.bpdm.pool.service.*
import org.springframework.context.annotation.Primary
import org.springframework.data.domain.PageRequest
import org.springframework.stereotype.Service
Expand All @@ -62,7 +61,8 @@ class SearchServiceImpl(
val addressService: AddressService,
val textDocSearchRepository: TextDocSearchRepository,
val businessPartnerFetchService: BusinessPartnerFetchService,
val objectMapper: ObjectMapper
val objectMapper: ObjectMapper,
val openSearchConfigProperties: OpenSearchConfigProperties
) : SearchService {

private val logger = KotlinLogging.logger { }
Expand Down Expand Up @@ -159,9 +159,49 @@ class SearchServiceImpl(
private fun searchAndPreparePage(
searchRequest: BusinessPartnerSearchRequest,
paginationRequest: PaginationRequest
): PageResponse<Pair<Float, LegalEntity>> {
return if (searchRequest == BusinessPartnerSearchRequest.EmptySearchRequest) {
paginateLegalEntities(paginationRequest)
} else {
searchIndex(searchRequest, paginationRequest)
}
}

private fun searchAndPreparePage(
searchRequest: AddressPartnerSearchRequest,
paginationRequest: PaginationRequest
): PageResponse<Pair<Float, AddressPartner>> {

return if (searchRequest == AddressPartnerSearchRequest.EmptySearchRequest) {
paginateAddressPartner(paginationRequest)
} else {
searchIndex(searchRequest, paginationRequest)
}
}

private fun paginateLegalEntities(paginationRequest: PaginationRequest): PageResponse<Pair<Float, LegalEntity>> {
logger.debug { "Paginate database for legal entities" }
val legalEntityPage = legalEntityRepository.findAll(PageRequest.of(paginationRequest.page, paginationRequest.size))

return legalEntityPage.toDto(legalEntityPage.content.map { Pair(0f, it) }) // assign 0 score as no search has been conducted
}

private fun paginateAddressPartner(paginationRequest: PaginationRequest): PageResponse<Pair<Float, AddressPartner>> {
logger.debug { "Paginate database for address partners" }
val addressPage = addressPartnerRepository.findAll(PageRequest.of(paginationRequest.page, paginationRequest.size))

return addressPage.toDto(addressPage.content.map { Pair(0f, it) }) // assign 0 score as no search has been conducted
}

private fun searchIndex(
searchRequest: BusinessPartnerSearchRequest,
paginationRequest: PaginationRequest
): PageResponse<Pair<Float, LegalEntity>> {
logger.debug { "Search index for legal entities" }

if (paginationRequest.page > openSearchConfigProperties.maxPage)
throw BpdmOpenSearchUserException("When using search parameters page can't exceed ${openSearchConfigProperties.maxPage} but was ${paginationRequest.page} instead")

val searchResult = legalEntityDocSearchRepository.findBySearchRequest(
searchRequest,
PageRequest.of(paginationRequest.page, paginationRequest.size)
Expand All @@ -184,12 +224,15 @@ class SearchServiceImpl(
return PageResponse(totalHits, totalPages, paginationRequest.page, legalEntities.size, scoreLegalEntityPairs)
}

private fun searchAndPreparePage(
private fun searchIndex(
searchRequest: AddressPartnerSearchRequest,
paginationRequest: PaginationRequest
): PageResponse<Pair<Float, AddressPartner>> {
logger.debug { "Search index for addresses" }

if (paginationRequest.page > openSearchConfigProperties.maxPage)
throw BpdmOpenSearchUserException("When using search parameters page can't exceed ${openSearchConfigProperties.maxPage} but was ${paginationRequest.page} instead")

val searchResult = addressDocSearchRepository.findBySearchRequest(
searchRequest,
PageRequest.of(paginationRequest.page, paginationRequest.size)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,6 @@ class OpenSearchConfigProperties(
val scheme: String = "http",
val exportPageSize: Int = 100,
val exportSchedulerCronExpr: String = "-",
val refreshOnWrite: Boolean = false
val refreshOnWrite: Boolean = false,
val maxPage: Int = 20
)
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class AddressController(
summary = "Get page of addresses matching the search criteria",
description = "This endpoint tries to find matches among all existing business partners of type address, " +
"filtering out partners which entirely do not match and ranking the remaining partners according to the accuracy of the match. " +
"The match of a partner is better the higher its relevancy score."
"The match of a partner is better the higher its relevancy score. " +
"Note that when using search parameters the max page is \${bpdm.opensearch.max-page}."
)
@ApiResponses(
value = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class BusinessPartnerLegacyController(
summary = "Get page of business partners matching the search criteria",
description = "This endpoint tries to find matches among all existing business partners, " +
"filtering out partners which entirely do not match and ranking the remaining partners according to the accuracy of the match. " +
"The match of a partner is better the higher its relevancy score.",
"The match of a partner is better the higher its relevancy score." +
"Note that when using search parameters the max page is \${bpdm.opensearch.max-page}.",
deprecated = true
)
@ApiResponses(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class LegalEntityController(
summary = "Get page of legal entity business partners matching the search criteria",
description = "This endpoint tries to find matches among all existing business partners of type legal entity, " +
"filtering out partners which entirely do not match and ranking the remaining partners according to the accuracy of the match. " +
"The match of a partner is better the higher its relevancy score."
"The match of a partner is better the higher its relevancy score. " +
"Note that when using search parameters the max page is \${bpdm.opensearch.max-page}."
)
@ApiResponses(
value = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@ data class AddressPartnerSearchRequest constructor(
var postalDeliveryPoint: String? = null,
@field:Parameter(description = "Filter business partners by ISO 3166-1 alpha-2 country code")
var countryCode: CountryCode? = null
)
) {
companion object {
val EmptySearchRequest = AddressPartnerSearchRequest()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,8 @@ data class AddressPropertiesSearchRequest constructor(
var premise: String? = null,
@field:Parameter(description = "Filter business partners by postal delivery point full denotation")
var postalDeliveryPoint: String? = null
)
) {
companion object {
val EmptySearchRequest = AddressPropertiesSearchRequest()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,12 @@ data class BusinessPartnerSearchRequest(
val partnerProperties: LegalEntityPropertiesSearchRequest,
val addressProperties: AddressPropertiesSearchRequest,
val siteProperties: SitePropertiesSearchRequest
)
) {
companion object {
val EmptySearchRequest = BusinessPartnerSearchRequest(
LegalEntityPropertiesSearchRequest.EmptySearchRequest,
AddressPropertiesSearchRequest.EmptySearchRequest,
SitePropertiesSearchRequest.EmptySearchRequest
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,9 @@ data class LegalEntityPropertiesSearchRequest constructor(
val status: String?,
@field:Parameter(description = "Filter legal entities by classification denotation")
val classification: String?,
)
) {
companion object {
val EmptySearchRequest = LegalEntityPropertiesSearchRequest(null, null, null, null)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ import io.swagger.v3.oas.annotations.media.Schema
data class SitePropertiesSearchRequest constructor(
@field:Parameter(description = "Filter sites by name")
val siteName: String?
)
) {
companion object {
val EmptySearchRequest = SitePropertiesSearchRequest(null)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*******************************************************************************
* Copyright (c) 2021,2023 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Apache License, Version 2.0 which is available at
* https://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* SPDX-License-Identifier: Apache-2.0
******************************************************************************/

package org.eclipse.tractusx.bpdm.pool.exception

import org.springframework.http.HttpStatus
import org.springframework.web.bind.annotation.ResponseStatus

@ResponseStatus(HttpStatus.BAD_REQUEST)
class BpdmOpenSearchUserException(message: String) : RuntimeException(message) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class BusinessPartnerFetchService(
return findOrThrow(bpn).toBusinessPartnerDto()
}


/**
* Fetch a business partner by [identifierValue] (ignoring case) of [identifierType] and return as [LegalEntityPartnerResponse]
*/
Expand All @@ -62,6 +63,7 @@ class BusinessPartnerFetchService(
return findOrThrow(identifierType, identifierValue).toPoolDto()
}


@Transactional
fun findBusinessPartnerIgnoreCase(identifierType: String, identifierValue: String): BusinessPartnerResponse {
return findOrThrow(identifierType, identifierValue).toBusinessPartnerDto()
Expand Down
1 change: 1 addition & 0 deletions bpdm-pool/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ bpdm.opensearch.host=localhost
bpdm.opensearch.port=9200
bpdm.opensearch.scheme=http
bpdm.opensearch.export-page-size=100
bpdm.opensearch.max-page=20
# Special value "-" disables scheduling. See javadoc of org.springframework.scheduling.support.CronExpression.parse for format.
bpdm.opensearch.export-scheduler-cron-expr=-
bpdm.opensearch.refresh-on-write=false
Expand Down

0 comments on commit 8d07508

Please sign in to comment.