From 85339b4a7a5475865ebc64e8cddb471039e4b9b0 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Wed, 13 Dec 2023 15:39:21 +0100 Subject: [PATCH 1/6] fix: IDP-2192 ONBOARDING_KO If Drools Not Ready (#236) --- .../OnboardingContextHolderService.java | 3 + .../OnboardingContextHolderServiceImpl.java | 34 +++++- .../evaluate/RuleEngineServiceImpl.java | 19 ++++ .../utils/OnboardingConstants.java | 1 + ...eBuilderConsumerConfigIntegrationTest.java | 1 + .../BeneficiaryRule2DroolsRuleImplTest.java | 15 ++- ...nboardingContextHolderServiceImplTest.java | 2 + .../evaluate/RuleEngineServiceImplTest.java | 101 +++++++++++++++--- 8 files changed, 154 insertions(+), 22 deletions(-) diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderService.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderService.java index ebe5e2af..302e9e1f 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderService.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderService.java @@ -4,6 +4,8 @@ import org.kie.api.KieBase; import reactor.core.publisher.Mono; +import java.util.Set; + /** * This component will retrieve the beneficiaries' rules kieContainer and the PDND token associated to the input initiative id @@ -11,6 +13,7 @@ * */ public interface OnboardingContextHolderService { KieBase getBeneficiaryRulesKieBase(); + Set getBeneficiaryRulesKieInitiativeIds(); void setBeneficiaryRulesKieBase(KieBase kieBase); Mono getInitiativeConfig(String initiativeId); diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java index 26d53dec..e9b8aa60 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java @@ -5,6 +5,7 @@ import it.gov.pagopa.admissibility.model.InitiativeConfig; import it.gov.pagopa.admissibility.service.build.KieContainerBuilderService; import lombok.extern.slf4j.Slf4j; +import org.drools.core.definitions.rule.impl.RuleImpl; import org.kie.api.KieBase; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -25,10 +26,13 @@ import reactor.util.retry.Retry; import java.util.Arrays; +import java.util.Collections; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.function.Function; +import java.util.stream.Collectors; @Service @Slf4j @@ -45,6 +49,7 @@ public class OnboardingContextHolderServiceImpl extends ReadinessStateHealthIndi private final boolean preLoadContainer; private KieBase kieBase; + private Set kieInitiatives = Collections.emptySet(); private byte[] kieBaseSerialized; private boolean contextReady=false; @@ -89,8 +94,7 @@ public OnboardingContextHolderReadyEvent(Object source) { //region kieContainer holder @Override public void setBeneficiaryRulesKieBase(KieBase newKieBase) { - preLoadKieBase(newKieBase); - this.kieBase = newKieBase; + acceptNewKieBase(newKieBase); if (isRedisCacheEnabled) { kieBaseSerialized = SerializationUtils.serialize(newKieBase); @@ -102,6 +106,23 @@ public void setBeneficiaryRulesKieBase(KieBase newKieBase) { } } + private void acceptNewKieBase(KieBase newKieBase) { + preLoadKieBase(newKieBase); + this.kieBase = newKieBase; + this.kieInitiatives = readKieInitiatives(newKieBase); + } + + private Set readKieInitiatives(KieBase kieBase) { + if (kieBase == null) { + return Collections.emptySet(); + } else { + return kieBase.getKiePackages().stream() + .flatMap(p -> p.getRules().stream()) + .map(r -> ((RuleImpl) r).getAgendaGroup()) + .collect(Collectors.toSet()); + } + } + private void preLoadKieBase(KieBase kieBase) { if (preLoadContainer) { kieContainerBuilderService.preLoadKieBase(kieBase); @@ -113,6 +134,11 @@ public KieBase getBeneficiaryRulesKieBase() { return kieBase; } + @Override + public Set getBeneficiaryRulesKieInitiativeIds() { + return kieInitiatives; + } + @Scheduled(initialDelayString = "${app.beneficiary-rule.cache.refresh-ms-rate}", fixedRateString = "${app.beneficiary-rule.cache.refresh-ms-rate}") public void refreshKieContainer() { refreshKieContainer(x -> log.trace("Refreshed KieContainer"), Retry.max(3), Mono::error); @@ -129,9 +155,7 @@ public void refreshKieContainer( this.kieBaseSerialized = c; try { KieBase newKieBase = org.apache.commons.lang3.SerializationUtils.deserialize(c); - preLoadKieBase(newKieBase); - - this.kieBase = newKieBase; + acceptNewKieBase(newKieBase); } catch (Exception e) { log.warn("[BENEFICIARY_RULE_BUILDER] Cached KieContainer cannot be executed! refreshing it!"); return null; diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java index 6ad9a15e..499e91d4 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java @@ -3,11 +3,13 @@ import it.gov.pagopa.admissibility.dto.onboarding.EvaluationDTO; import it.gov.pagopa.admissibility.dto.onboarding.OnboardingDTO; import it.gov.pagopa.admissibility.dto.onboarding.OnboardingDroolsDTO; +import it.gov.pagopa.admissibility.dto.onboarding.OnboardingRejectionReason; import it.gov.pagopa.admissibility.mapper.Onboarding2EvaluationMapper; import it.gov.pagopa.admissibility.mapper.Onboarding2OnboardingDroolsMapper; import it.gov.pagopa.admissibility.model.InitiativeConfig; import it.gov.pagopa.admissibility.service.CriteriaCodeService; import it.gov.pagopa.admissibility.service.onboarding.OnboardingContextHolderService; +import it.gov.pagopa.admissibility.utils.OnboardingConstants; import it.gov.pagopa.common.reactive.utils.PerformanceLogger; import lombok.extern.slf4j.Slf4j; import org.drools.core.command.runtime.rule.AgendaGroupSetFocusCommand; @@ -15,6 +17,7 @@ import org.kie.api.runtime.StatelessKieSession; import org.kie.internal.command.CommandFactory; import org.springframework.stereotype.Service; +import org.springframework.util.CollectionUtils; import java.util.Arrays; import java.util.List; @@ -52,10 +55,26 @@ public EvaluationDTO applyRules(OnboardingDTO onboardingRequest, InitiativeConfi long before = System.currentTimeMillis(); statelessKieSession.execute(CommandFactory.newBatchExecution(cmds)); + checkIfContainerWasReady(req, initiative); + PerformanceLogger.logTiming("ONBOARDING_RULE_ENGINE", before, "resulted into rejections %s".formatted(req.getOnboardingRejectionReasons())); log.trace("[ONBOARDING_REQUEST] [RULE_ENGINE] Send message prepared: {}", req); return onboarding2EvaluationMapper.apply(req, initiative, req.getOnboardingRejectionReasons()); } + + private void checkIfContainerWasReady(OnboardingDroolsDTO req, InitiativeConfig initiative) { + if (req.getOnboardingRejectionReasons().isEmpty() && // there is not rejection reason + !CollectionUtils.isEmpty(initiative.getAutomatedCriteria()) && // the drools container is supposed to be involved + !onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds() // the initiative was not inside the container drools + .contains(initiative.getInitiativeId()) + ) { + req.getOnboardingRejectionReasons().add(new OnboardingRejectionReason( + OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR, + OnboardingConstants.REJECTION_REASON_RULE_ENGINE_NOT_READY, + null, null, null + )); + } + } } diff --git a/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java b/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java index 02e0fb88..062d69c4 100644 --- a/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java +++ b/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java @@ -20,6 +20,7 @@ private OnboardingConstants(){} public static final String REJECTION_REASON_FAMILY_KO = "FAMILY_FAIL"; public static final String REJECTION_REASON_RESIDENCE_KO = "RESIDENCE_FAIL"; public static final String REJECTION_REASON_BIRTHDATE_KO = "BIRTHDATE_FAIL"; + public static final String REJECTION_REASON_RULE_ENGINE_NOT_READY = "RULE_ENGINE_NOT_READY"; public static final String REJECTION_REASON_GENERIC_ERROR = "GENERIC_ERROR"; /** Rejection reason for the second family member due to the family not meeting the requirements of the initiative */ diff --git a/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/BeneficiaryRuleBuilderConsumerConfigIntegrationTest.java b/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/BeneficiaryRuleBuilderConsumerConfigIntegrationTest.java index d6f35fd6..30e2daa4 100644 --- a/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/BeneficiaryRuleBuilderConsumerConfigIntegrationTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/BeneficiaryRuleBuilderConsumerConfigIntegrationTest.java @@ -75,6 +75,7 @@ void testBeneficiaryRuleBuilding() { long timeEnd = System.currentTimeMillis(); Assertions.assertEquals(validRules, countSaved[0]); + Assertions.assertEquals(validRules, onboardingContextHolderServiceSpy.getBeneficiaryRulesKieInitiativeIds().size()); Assertions.assertEquals(expectedRules[0], ruleBuiltSize); checkInitiativeCounters(validRules); diff --git a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java index c735d979..95f9fc65 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; /* ****************** @@ -144,6 +145,7 @@ private void checkResult(DroolsRule result, Initiative2BuildDTO dto) { @Test void testExecutions() { testExecution(Collections.emptyList()); + testExecution(List.of("NOTREADY")); testExecution(List.of("ISEE")); testExecution(List.of("BIRTHDATE")); testExecution(List.of("ISEE", "BIRTHDATE")); @@ -153,6 +155,7 @@ void testExecution(List failingCode) { //given boolean expectedIseeFail = failingCode.contains("ISEE"); boolean expectedBirthDateFail = failingCode.contains("BIRTHDATE"); + boolean expectedNotReady = failingCode.equals(List.of("NOTREADY")); Initiative2BuildDTO initiative = buildInitiative(); @@ -175,6 +178,10 @@ void testExecution(List failingCode) { OnboardingContextHolderService onboardingContextHolderService = Mockito.mock(OnboardingContextHolderService.class); Mockito.when(onboardingContextHolderService.getBeneficiaryRulesKieBase()).thenReturn(buildKieBase(rule)); + Mockito.when(onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds()) + .thenReturn(expectedNotReady + ? Collections.emptySet() + : Set.of(initiative.getInitiativeId())); RuleEngineService ruleEngineService = new RuleEngineServiceImpl(onboardingContextHolderService, new Onboarding2EvaluationMapper(), criteriaCodeServiceMock, new Onboarding2OnboardingDroolsMapper()); @@ -208,7 +215,13 @@ void testExecution(List failingCode) { .authorityLabel("Agenzia per l'Italia Digitale") .build()); } - expectedEvaluationResult.setStatus(expectedEvaluationResult.getOnboardingRejectionReasons().size() == 0 ? OnboardingEvaluationStatus.ONBOARDING_OK : OnboardingEvaluationStatus.ONBOARDING_KO); + if (expectedNotReady) { + expectedEvaluationResult.getOnboardingRejectionReasons().add(OnboardingRejectionReason.builder() + .type(OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR) + .code("RULE_ENGINE_NOT_READY") + .build()); + } + expectedEvaluationResult.setStatus(expectedEvaluationResult.getOnboardingRejectionReasons().isEmpty() ? OnboardingEvaluationStatus.ONBOARDING_OK : OnboardingEvaluationStatus.ONBOARDING_KO); Assertions.assertEquals(expectedEvaluationResult, evaluationResult); } diff --git a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImplTest.java index 364a249d..a44d7dc5 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImplTest.java @@ -30,6 +30,7 @@ import java.math.BigDecimal; import java.time.LocalDate; import java.util.ArrayList; +import java.util.Collections; import java.util.List; @ExtendWith(MockitoExtension.class) @@ -78,6 +79,7 @@ void getKieContainer(boolean isRedisCacheEnabled) { //Then Assertions.assertNotNull(result); + Assertions.assertEquals(Collections.emptySet(), onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds()); if (!isRedisCacheEnabled) { Assertions.assertSame(expectedKieBase, result); } diff --git a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java index fd5d3230..fd45329c 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java @@ -1,59 +1,76 @@ package it.gov.pagopa.admissibility.service.onboarding.evaluate; +import it.gov.pagopa.admissibility.connector.repository.DroolsRuleRepository; import it.gov.pagopa.admissibility.drools.transformer.extra_filter.ExtraFilter2DroolsTransformerFacadeImplTest; import it.gov.pagopa.admissibility.dto.onboarding.*; +import it.gov.pagopa.admissibility.dto.rule.AutomatedCriteriaDTO; import it.gov.pagopa.admissibility.enums.OnboardingEvaluationStatus; import it.gov.pagopa.admissibility.mapper.Onboarding2EvaluationMapper; import it.gov.pagopa.admissibility.mapper.Onboarding2OnboardingDroolsMapper; import it.gov.pagopa.admissibility.model.DroolsRule; import it.gov.pagopa.admissibility.model.InitiativeConfig; -import it.gov.pagopa.admissibility.connector.repository.DroolsRuleRepository; import it.gov.pagopa.admissibility.service.CriteriaCodeService; import it.gov.pagopa.admissibility.service.build.KieContainerBuilderServiceImpl; import it.gov.pagopa.admissibility.service.build.KieContainerBuilderServiceImplTest; import it.gov.pagopa.admissibility.service.onboarding.OnboardingContextHolderService; -import it.gov.pagopa.admissibility.service.onboarding.OnboardingContextHolderServiceImpl; +import it.gov.pagopa.admissibility.utils.OnboardingConstants; import lombok.extern.slf4j.Slf4j; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.*; import org.junit.jupiter.api.extension.ExtendWith; import org.kie.api.KieBase; +import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import reactor.core.publisher.Flux; import java.time.LocalDateTime; import java.util.Collections; +import java.util.List; @ExtendWith(MockitoExtension.class) @Slf4j class RuleEngineServiceImplTest { + public static final String INITIATIVEID = "INITIATIVEID"; + @Mock private OnboardingContextHolderService onboardingContextHolderServiceMock; + @Mock private CriteriaCodeService criteriaCodeServiceMock; + + private final Onboarding2EvaluationMapper onboarding2EvaluationMapper = new Onboarding2EvaluationMapper(); + private final Onboarding2OnboardingDroolsMapper onboarding2OnboardingDroolsMapper = new Onboarding2OnboardingDroolsMapper(); + + private RuleEngineService ruleEngineService; + @BeforeAll public static void configDroolsLogLevel() { KieContainerBuilderServiceImplTest.configDroolsLogs(); } + @BeforeEach + void init(){ + ruleEngineService = new RuleEngineServiceImpl(onboardingContextHolderServiceMock, onboarding2EvaluationMapper, criteriaCodeServiceMock, onboarding2OnboardingDroolsMapper); + } + + @AfterEach + void verifyNotMoreInvocations(){ + Mockito.verifyNoMoreInteractions( + onboardingContextHolderServiceMock, + criteriaCodeServiceMock); + } + @Test void applyRules() { // Given - OnboardingContextHolderService onboardingContextHolderServiceMock = Mockito.mock(OnboardingContextHolderServiceImpl.class); - CriteriaCodeService criteriaCodeServiceMock = Mockito.mock(CriteriaCodeService.class); - Onboarding2EvaluationMapper onboarding2EvaluationMapper = new Onboarding2EvaluationMapper(); - Onboarding2OnboardingDroolsMapper onboarding2OnboardingDroolsMapper = new Onboarding2OnboardingDroolsMapper(); - - RuleEngineService ruleEngineService = new RuleEngineServiceImpl(onboardingContextHolderServiceMock, onboarding2EvaluationMapper, criteriaCodeServiceMock, onboarding2OnboardingDroolsMapper); - + String initiativeId = INITIATIVEID; OnboardingDTO onboardingDTO = new OnboardingDTO(); - onboardingDTO.setInitiativeId("INITIATIVEID"); + onboardingDTO.setInitiativeId(initiativeId); InitiativeConfig initiativeConfig = new InitiativeConfig(); - initiativeConfig.setInitiativeId("INITIATIVEID"); + initiativeConfig.setInitiativeId(initiativeId); initiativeConfig.setInitiativeName("INITIATIVENAME"); initiativeConfig.setOrganizationId("ORGANIZATIONID"); - Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()).thenReturn(buildContainer(onboardingDTO.getInitiativeId())); + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) + .thenReturn(buildContainer(onboardingDTO.getInitiativeId())); // When EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); @@ -61,7 +78,7 @@ void applyRules() { // Then Mockito.verify(onboardingContextHolderServiceMock).getBeneficiaryRulesKieBase(); - Assertions.assertTrue(result instanceof EvaluationCompletedDTO); + Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); Assertions.assertNotNull(result.getAdmissibilityCheckDate()); Assertions.assertFalse(result.getAdmissibilityCheckDate().isAfter(LocalDateTime.now())); Assertions.assertTrue(result.getAdmissibilityCheckDate().isAfter(LocalDateTime.now().minusMinutes(2))); @@ -79,6 +96,58 @@ void applyRules() { Assertions.assertEquals(expected, result); } + @Test + void testNotRules_notKieBasedInitiatives(){ + // Given + String initiativeId = "NOTKIEINITITATIVEID"; + OnboardingDTO onboardingDTO = new OnboardingDTO(); + onboardingDTO.setInitiativeId(initiativeId); + + InitiativeConfig initiativeConfig = new InitiativeConfig(); + initiativeConfig.setInitiativeId(initiativeId); + + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) + .thenReturn(buildContainer(INITIATIVEID)); + + // When + EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); + + // Then + Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); + Assertions.assertEquals(OnboardingEvaluationStatus.ONBOARDING_OK, ((EvaluationCompletedDTO)result).getStatus()); + Assertions.assertEquals(Collections.emptyList(), ((EvaluationCompletedDTO)result).getOnboardingRejectionReasons()); + } + + @Test + void testNotRules_kieBasedInitiatives(){ + // Given + String initiativeId = "KIEINITITATIVEID_NOT_IN_CONTAINER"; + OnboardingDTO onboardingDTO = new OnboardingDTO(); + onboardingDTO.setInitiativeId(initiativeId); + + InitiativeConfig initiativeConfig = new InitiativeConfig(); + initiativeConfig.setInitiativeId(initiativeId); + initiativeConfig.setAutomatedCriteria(List.of(AutomatedCriteriaDTO.builder().build())); + + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) + .thenReturn(buildContainer(INITIATIVEID)); + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieInitiativeIds()) + .thenReturn(Collections.emptySet()); + + // When + EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); + + // Then + Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); + Assertions.assertEquals(OnboardingEvaluationStatus.ONBOARDING_KO, ((EvaluationCompletedDTO)result).getStatus()); + Assertions.assertEquals(List.of(new OnboardingRejectionReason( + OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR, + OnboardingConstants.REJECTION_REASON_RULE_ENGINE_NOT_READY, + null, null, null + )), + ((EvaluationCompletedDTO)result).getOnboardingRejectionReasons()); + } + private KieBase buildContainer(String initiativeId) { DroolsRule ignoredRule = new DroolsRule(); ignoredRule.setId("IGNORED"); From 42f4d01fa80dd9bfd7d32037e6884adf1fba4671 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:46:09 +0100 Subject: [PATCH 2/6] fix: IDP-2192 kieContainer ready logic simplified (#238) --- .../evaluate/RuleEngineServiceImpl.java | 46 +++++++++---------- .../evaluate/RuleEngineServiceImplTest.java | 6 +-- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java index 499e91d4..ce1064c0 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java @@ -41,40 +41,38 @@ public RuleEngineServiceImpl(OnboardingContextHolderService onboardingContextHol public EvaluationDTO applyRules(OnboardingDTO onboardingRequest, InitiativeConfig initiative) { log.trace("[ONBOARDING_REQUEST] [RULE_ENGINE] evaluating rules of user {} into initiative {}", onboardingRequest.getUserId(), onboardingRequest.getInitiativeId()); - StatelessKieSession statelessKieSession = onboardingContextHolderService.getBeneficiaryRulesKieBase().newStatelessKieSession(); - OnboardingDroolsDTO req = onboarding2OnboardingDroolsMapper.apply(onboardingRequest); - @SuppressWarnings("unchecked") - List> cmds = Arrays.asList( - CommandFactory.newInsert(req), - CommandFactory.newInsert(criteriaCodeService), - new AgendaGroupSetFocusCommand(req.getInitiativeId()) - ); - - long before = System.currentTimeMillis(); - statelessKieSession.execute(CommandFactory.newBatchExecution(cmds)); + if(checkIfKieBaseContainerIsReady(initiative)) { + StatelessKieSession statelessKieSession = onboardingContextHolderService.getBeneficiaryRulesKieBase().newStatelessKieSession(); - checkIfContainerWasReady(req, initiative); + @SuppressWarnings("unchecked") + List> cmds = Arrays.asList( + CommandFactory.newInsert(req), + CommandFactory.newInsert(criteriaCodeService), + new AgendaGroupSetFocusCommand(req.getInitiativeId()) + ); - PerformanceLogger.logTiming("ONBOARDING_RULE_ENGINE", before, "resulted into rejections %s".formatted(req.getOnboardingRejectionReasons())); - - log.trace("[ONBOARDING_REQUEST] [RULE_ENGINE] Send message prepared: {}", req); - - return onboarding2EvaluationMapper.apply(req, initiative, req.getOnboardingRejectionReasons()); - } + long before = System.currentTimeMillis(); + statelessKieSession.execute(CommandFactory.newBatchExecution(cmds)); - private void checkIfContainerWasReady(OnboardingDroolsDTO req, InitiativeConfig initiative) { - if (req.getOnboardingRejectionReasons().isEmpty() && // there is not rejection reason - !CollectionUtils.isEmpty(initiative.getAutomatedCriteria()) && // the drools container is supposed to be involved - !onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds() // the initiative was not inside the container drools - .contains(initiative.getInitiativeId()) - ) { + PerformanceLogger.logTiming("ONBOARDING_RULE_ENGINE", before, "resulted into rejections %s".formatted(req.getOnboardingRejectionReasons())); + } else { req.getOnboardingRejectionReasons().add(new OnboardingRejectionReason( OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR, OnboardingConstants.REJECTION_REASON_RULE_ENGINE_NOT_READY, null, null, null )); } + + log.trace("[ONBOARDING_REQUEST] [RULE_ENGINE] Send message prepared: {}", req); + + return onboarding2EvaluationMapper.apply(req, initiative, req.getOnboardingRejectionReasons()); + } + + private boolean checkIfKieBaseContainerIsReady(InitiativeConfig initiative) { + return CollectionUtils.isEmpty(initiative.getAutomatedCriteria()) || // the drools container is supposed to not be involved + onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds() // the initiative is inside the container drools + .contains(initiative.getInitiativeId()); } } diff --git a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java index fd45329c..7982d120 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java @@ -70,14 +70,12 @@ void applyRules() { initiativeConfig.setOrganizationId("ORGANIZATIONID"); Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) - .thenReturn(buildContainer(onboardingDTO.getInitiativeId())); + .thenReturn(buildContainer(initiativeId)); // When EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); // Then - Mockito.verify(onboardingContextHolderServiceMock).getBeneficiaryRulesKieBase(); - Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); Assertions.assertNotNull(result.getAdmissibilityCheckDate()); Assertions.assertFalse(result.getAdmissibilityCheckDate().isAfter(LocalDateTime.now())); @@ -129,8 +127,6 @@ void testNotRules_kieBasedInitiatives(){ initiativeConfig.setInitiativeId(initiativeId); initiativeConfig.setAutomatedCriteria(List.of(AutomatedCriteriaDTO.builder().build())); - Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) - .thenReturn(buildContainer(INITIATIVEID)); Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieInitiativeIds()) .thenReturn(Collections.emptySet()); From eb20c3baff61791e9d1f0a2406a4780dfb17fd18 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Thu, 14 Dec 2023 17:53:45 +0100 Subject: [PATCH 3/6] chore: logging drools rules deleted by command (#239) * logging drools rules deleted by command * logging drools rules deleted by command * logging drools rules deleted by command --- .../repository/DroolsRuleRepository.java | 4 ++-- .../InitiativeCountersRepository.java | 4 ++-- .../admissibility/model/DroolsRule.java | 3 +++ .../build/BeneficiaryRule2DroolsRuleImpl.java | 2 ++ .../DeleteInitiativeServiceImpl.java | 15 ++++++------ .../OnboardingContextHolderServiceImpl.java | 1 + .../mongo/ReactiveMongoRepositoryExt.java | 11 +++++++++ .../mongo/ReactiveMongoRepositoryImpl.java | 24 ++++++++++++++++++- .../CommandConsumerConfigIntegrationTest.java | 4 +--- .../BeneficiaryRule2DroolsRuleImplTest.java | 2 +- ...ficiaryRuleBuilderMediatorServiceTest.java | 4 +++- .../DeleteInitiativeServiceImplTest.java | 19 ++++++++------- 12 files changed, 67 insertions(+), 26 deletions(-) create mode 100644 src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryExt.java diff --git a/src/main/java/it/gov/pagopa/admissibility/connector/repository/DroolsRuleRepository.java b/src/main/java/it/gov/pagopa/admissibility/connector/repository/DroolsRuleRepository.java index 026b7f21..c1931475 100644 --- a/src/main/java/it/gov/pagopa/admissibility/connector/repository/DroolsRuleRepository.java +++ b/src/main/java/it/gov/pagopa/admissibility/connector/repository/DroolsRuleRepository.java @@ -1,9 +1,9 @@ package it.gov.pagopa.admissibility.connector.repository; import it.gov.pagopa.admissibility.model.DroolsRule; -import org.springframework.data.mongodb.repository.ReactiveMongoRepository; +import it.gov.pagopa.common.reactive.mongo.ReactiveMongoRepositoryExt; /** * it will handle the persistence of {@link it.gov.pagopa.admissibility.model.DroolsRule} entity*/ -public interface DroolsRuleRepository extends ReactiveMongoRepository { +public interface DroolsRuleRepository extends ReactiveMongoRepositoryExt { } diff --git a/src/main/java/it/gov/pagopa/admissibility/connector/repository/InitiativeCountersRepository.java b/src/main/java/it/gov/pagopa/admissibility/connector/repository/InitiativeCountersRepository.java index a082fc1b..b214af6b 100644 --- a/src/main/java/it/gov/pagopa/admissibility/connector/repository/InitiativeCountersRepository.java +++ b/src/main/java/it/gov/pagopa/admissibility/connector/repository/InitiativeCountersRepository.java @@ -1,9 +1,9 @@ package it.gov.pagopa.admissibility.connector.repository; import it.gov.pagopa.admissibility.model.InitiativeCounters; -import org.springframework.data.mongodb.repository.ReactiveMongoRepository; +import it.gov.pagopa.common.reactive.mongo.ReactiveMongoRepositoryExt; /** * it will handle the persistence of {@link InitiativeCounters} entity*/ -public interface InitiativeCountersRepository extends ReactiveMongoRepository, InitiativeCountersReservationOpsRepository { +public interface InitiativeCountersRepository extends ReactiveMongoRepositoryExt, InitiativeCountersReservationOpsRepository { } diff --git a/src/main/java/it/gov/pagopa/admissibility/model/DroolsRule.java b/src/main/java/it/gov/pagopa/admissibility/model/DroolsRule.java index bbe44ddd..a87becfe 100644 --- a/src/main/java/it/gov/pagopa/admissibility/model/DroolsRule.java +++ b/src/main/java/it/gov/pagopa/admissibility/model/DroolsRule.java @@ -9,6 +9,8 @@ import org.springframework.data.annotation.Id; import org.springframework.data.mongodb.core.mapping.Document; +import java.time.LocalDateTime; + @Slf4j @Data @AllArgsConstructor @@ -23,4 +25,5 @@ public class DroolsRule { private String rule; private String ruleVersion; private InitiativeConfig initiativeConfig; + private LocalDateTime updateDate; } diff --git a/src/main/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImpl.java index 23e89a52..33f770fe 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImpl.java @@ -18,6 +18,7 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Flux; +import java.time.LocalDateTime; import java.util.stream.Collectors; @Service @@ -49,6 +50,7 @@ public DroolsRule apply(Initiative2BuildDTO initiative) { out.setId(initiative.getInitiativeId()); out.setName(initiative.getInitiativeName()); out.setRuleVersion("20230404"); + out.setUpdateDate(LocalDateTime.now()); out.setRule(""" package %s; diff --git a/src/main/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImpl.java index b589d41f..6e256e86 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImpl.java @@ -51,20 +51,21 @@ private Mono execAndLogTiming(String deleteFlowName, String initiativeId, Mon } private Mono deleteDroolsRule(String initiativeId) { - return droolsRuleRepository.deleteById(initiativeId) - .doOnSuccess(d -> { - log.info("[DELETE_INITIATIVE] Deleted initiative {} from collection: beneficiary_rule", initiativeId); + return droolsRuleRepository.removeById(initiativeId) + .doOnNext(d -> { + log.info("[DELETE_INITIATIVE] Deleted {} initiative {} from collection: beneficiary_rule", d.getDeletedCount(), initiativeId); auditUtilities.logDeletedDroolsRule(initiativeId); }) .then(); } private Mono deleteInitiativeCounters(String initiativeId) { - return initiativeCountersRepository.deleteById(initiativeId) - .doOnSuccess(i -> { - log.info("[DELETE_INITIATIVE] Deleted initiative {} from collection: initiative_counters", initiativeId); + return initiativeCountersRepository.removeById(initiativeId) + .doOnNext(i -> { + log.info("[DELETE_INITIATIVE] Deleted {} initiative {} from collection: initiative_counters", i.getDeletedCount(), initiativeId); auditUtilities.logDeletedInitiativeCounters(initiativeId); - }); + }) + .then(); } private Mono deleteOnboardingFamilies(String initiativeId) { diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java index e9b8aa60..4c8e3ed2 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceImpl.java @@ -110,6 +110,7 @@ private void acceptNewKieBase(KieBase newKieBase) { preLoadKieBase(newKieBase); this.kieBase = newKieBase; this.kieInitiatives = readKieInitiatives(newKieBase); + log.info("[BENEFICIARY_RULE_CONTAINER_LOAD] Rule engine rules loaded: {}", kieInitiatives); } private Set readKieInitiatives(KieBase kieBase) { diff --git a/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryExt.java b/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryExt.java new file mode 100644 index 00000000..dd1b1890 --- /dev/null +++ b/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryExt.java @@ -0,0 +1,11 @@ +package it.gov.pagopa.common.reactive.mongo; + +import com.mongodb.client.result.DeleteResult; +import org.springframework.data.mongodb.repository.ReactiveMongoRepository; +import org.springframework.data.repository.NoRepositoryBean; +import reactor.core.publisher.Mono; + +@NoRepositoryBean +public interface ReactiveMongoRepositoryExt extends ReactiveMongoRepository { + Mono removeById(ID id); +} diff --git a/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryImpl.java b/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryImpl.java index f76e851a..d9a56ee4 100644 --- a/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryImpl.java +++ b/src/main/java/it/gov/pagopa/common/reactive/mongo/ReactiveMongoRepositoryImpl.java @@ -1,16 +1,20 @@ package it.gov.pagopa.common.reactive.mongo; +import com.mongodb.client.result.DeleteResult; import lombok.NonNull; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.repository.query.MongoEntityInformation; import org.springframework.data.mongodb.repository.support.SimpleReactiveMongoRepository; +import org.springframework.util.Assert; import reactor.core.publisher.Mono; import java.io.Serializable; -public class ReactiveMongoRepositoryImpl extends SimpleReactiveMongoRepository { +import static org.springframework.data.mongodb.core.query.Criteria.where; + +public class ReactiveMongoRepositoryImpl extends SimpleReactiveMongoRepository implements ReactiveMongoRepositoryExt { private final ReactiveMongoOperations mongoOperations; private final MongoEntityInformation entityInformation; @@ -29,4 +33,22 @@ public ReactiveMongoRepositoryImpl(MongoEntityInformation entityInformatio entityInformation.getJavaType(), entityInformation.getCollectionName()).singleOrEmpty(); } + @Override + public Mono removeById(I id) { + Assert.notNull(id, "The given id must not be null"); + + return mongoOperations + .remove(getIdQuery(id), entityInformation.getJavaType(), entityInformation.getCollectionName()); + } + + @SuppressWarnings("squid:S2177") // suppressing overriding private super method + private Query getIdQuery(Object id) { + return new Query(getIdCriteria(id)); + } + + @SuppressWarnings("squid:S2177") // suppressing overriding private super method + private Criteria getIdCriteria(Object id) { + return where(entityInformation.getIdAttribute()).is(id); + } + } diff --git a/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/CommandConsumerConfigIntegrationTest.java b/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/CommandConsumerConfigIntegrationTest.java index 757db668..1759c36d 100644 --- a/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/CommandConsumerConfigIntegrationTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/connector/event/consumer/CommandConsumerConfigIntegrationTest.java @@ -70,8 +70,6 @@ void test() { checkRepositories(); checkErrorsPublished(notValidMessages, maxWaitingMs, errorUseCases); - Mockito.verify(droolsRuleRepositorySpy).findAll(); - System.out.printf(""" ************************ Time spent to send %d (%d + %d) messages (from start): %d millis @@ -188,7 +186,7 @@ protected Pattern getErrorUseCaseIdPatternMatch() { errorUseCases.add(Pair.of( () -> { Mockito.doThrow(new MongoException("Command error dummy")) - .when(droolsRuleRepositorySpy).deleteById(errorInitiativeId); + .when(droolsRuleRepositorySpy).removeById(errorInitiativeId); return commandOperationErrorString; }, errorMessage -> checkErrorMessageHeaders(errorMessage, "[ADMISSIBILITY_COMMANDS] An error occurred evaluating commands", commandOperationErrorString) diff --git a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java index 95f9fc65..49d65174 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRule2DroolsRuleImplTest.java @@ -136,7 +136,7 @@ private void checkResult(DroolsRule result, Initiative2BuildDTO dto) { .isLogoPresent(Boolean.TRUE) .beneficiaryType(InitiativeGeneralDTO.BeneficiaryTypeEnum.PF) .build()); - + expected.setUpdateDate(result.getUpdateDate()); expected.setRuleVersion("20230404"); Assertions.assertEquals(expected, result); diff --git a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRuleBuilderMediatorServiceTest.java b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRuleBuilderMediatorServiceTest.java index 4e993884..296d0136 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRuleBuilderMediatorServiceTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/build/BeneficiaryRuleBuilderMediatorServiceTest.java @@ -22,6 +22,7 @@ import reactor.core.publisher.Mono; import java.nio.charset.StandardCharsets; +import java.time.LocalDateTime; import java.util.List; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -61,7 +62,8 @@ void configureMocks() { .automatedCriteriaCodes(List.of("CODE")) .initiativeBudget(i.getGeneral().getBudget()) .beneficiaryInitiativeBudget(i.getGeneral().getBeneficiaryBudget()) - .build()); + .build(), + LocalDateTime.now()); }); Mockito.when(droolsRuleRepositoryMock.save(Mockito.any())).thenAnswer(invocation -> Mono.just(invocation.getArgument(0))); Mockito.when(kieContainerBuilderServiceMock.buildAll()).thenReturn(Mono.just(newKieBaseBuiltMock)); diff --git a/src/test/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImplTest.java index 08b77d76..e7088bee 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/commands/operations/DeleteInitiativeServiceImplTest.java @@ -1,6 +1,7 @@ package it.gov.pagopa.admissibility.service.commands.operations; import com.mongodb.MongoException; +import com.mongodb.client.result.DeleteResult; import it.gov.pagopa.admissibility.connector.repository.DroolsRuleRepository; import it.gov.pagopa.admissibility.connector.repository.InitiativeCountersRepository; import it.gov.pagopa.admissibility.connector.repository.OnboardingFamiliesRepository; @@ -43,11 +44,11 @@ void executeOK() { String initiativeId = "INITIATIVEID"; String familyid = "FAMILYID"; - Mockito.when(droolsRuleRepositoryMock.deleteById(initiativeId)) - .thenReturn(Mono.just(Mockito.mock(Void.class))); + Mockito.when(droolsRuleRepositoryMock.removeById(initiativeId)) + .thenReturn(Mono.just(Mockito.mock(DeleteResult.class))); - Mockito.when(initiativeCountersRepositoryMock.deleteById(initiativeId)) - .thenReturn(Mono.just(Mockito.mock(Void.class))); + Mockito.when(initiativeCountersRepositoryMock.removeById(initiativeId)) + .thenReturn(Mono.just(Mockito.mock(DeleteResult.class))); Family family = Family.builder() .familyId(familyid) @@ -66,8 +67,8 @@ void executeOK() { Assertions.assertNotNull(result); - Mockito.verify(droolsRuleRepositoryMock, Mockito.times(1)).deleteById(Mockito.anyString()); - Mockito.verify(initiativeCountersRepositoryMock, Mockito.times(1)).deleteById(Mockito.anyString()); + Mockito.verify(droolsRuleRepositoryMock, Mockito.times(1)).removeById(Mockito.anyString()); + Mockito.verify(initiativeCountersRepositoryMock, Mockito.times(1)).removeById(Mockito.anyString()); Mockito.verify(onboardingFamiliesRepositoryMock, Mockito.times(1)).findByInitiativeIdWithBatch(Mockito.anyString(),Mockito.anyInt()); Mockito.verify(onboardingFamiliesRepositoryMock, Mockito.times(1)).deleteById(Mockito.anyString()); @@ -76,14 +77,14 @@ void executeOK() { @Test void executeError() { String initiativeId = "INITIATIVEID"; - Mockito.when(droolsRuleRepositoryMock.deleteById(initiativeId)) + Mockito.when(droolsRuleRepositoryMock.removeById(initiativeId)) .thenThrow(new MongoException("DUMMY_EXCEPTION")); try{ deleteInitiativeService.execute(initiativeId).block(); Assertions.fail(); - }catch (Throwable t){ - Assertions.assertTrue(t instanceof MongoException); + }catch (MongoException t){ + // Do Nothing } } } \ No newline at end of file From c0e23de6d76c80451f2b8fa74364407607a04f1d Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Mon, 18 Dec 2023 18:42:18 +0100 Subject: [PATCH 4/6] feat: IDP-2090 application specific error codes (#240) * IDP-2090 application specific error codes * bump version * fixed unit tests * fixed unit tests --- pom.xml | 2 +- .../AdmissibilityErrorManagerConfig.java | 23 +++++++++++++++++++ .../AdmissibilityControllerImpl.java | 5 ++-- .../utils/OnboardingConstants.java | 9 ++++++++ .../common/web/exception/ErrorManager.java | 11 ++++++--- .../web/exception/MongoExceptionHandler.java | 12 ++++++++-- ...questRateTooLargeRetryIntegrationTest.java | 9 ++++---- .../exception/MongoExceptionHandlerTest.java | 17 ++++++-------- 8 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java diff --git a/pom.xml b/pom.xml index c3a2ebd9..2f49de58 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ it.gov.pagopa idpay-admissibility-assessor idpay-admissibility-assessor - 1.2.3 + 1.3.0 17 diff --git a/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java b/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java new file mode 100644 index 00000000..c84d654b --- /dev/null +++ b/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java @@ -0,0 +1,23 @@ +package it.gov.pagopa.admissibility.config; + +import it.gov.pagopa.admissibility.utils.OnboardingConstants; +import it.gov.pagopa.common.web.dto.ErrorDTO; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class AdmissibilityErrorManagerConfig { + + @Bean + ErrorDTO defaultErrorDTO() { + return new ErrorDTO( + OnboardingConstants.ExceptionCode.GENERIC_ERROR, + "A generic error occurred for payment" + ); + } + + @Bean + ErrorDTO tooManyRequestsErrorDTO() { + return new ErrorDTO(OnboardingConstants.ExceptionCode.TOO_MANY_REQUESTS, "Too Many Requests"); + } +} diff --git a/src/main/java/it/gov/pagopa/admissibility/controller/AdmissibilityControllerImpl.java b/src/main/java/it/gov/pagopa/admissibility/controller/AdmissibilityControllerImpl.java index 45541f15..7d7799b4 100644 --- a/src/main/java/it/gov/pagopa/admissibility/controller/AdmissibilityControllerImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/controller/AdmissibilityControllerImpl.java @@ -1,8 +1,9 @@ package it.gov.pagopa.admissibility.controller; import it.gov.pagopa.admissibility.dto.onboarding.InitiativeStatusDTO; -import it.gov.pagopa.common.web.exception.ClientExceptionNoBody; import it.gov.pagopa.admissibility.service.InitiativeStatusService; +import it.gov.pagopa.admissibility.utils.OnboardingConstants; +import it.gov.pagopa.common.web.exception.ClientExceptionWithBody; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.RestController; @@ -21,6 +22,6 @@ public AdmissibilityControllerImpl(InitiativeStatusService initiativeStatusServi @Override public Mono getInitiativeStatus(String initiativeId) { return initiativeStatusService.getInitiativeStatusAndBudgetAvailable(initiativeId) - .switchIfEmpty(Mono.error(new ClientExceptionNoBody(HttpStatus.NOT_FOUND, "Cannot find initiative having id " + initiativeId))); + .switchIfEmpty(Mono.error(new ClientExceptionWithBody(HttpStatus.NOT_FOUND, OnboardingConstants.ExceptionCode.INITIATIVE_NOT_FOUND, "The initiative with id %s does not exist".formatted(initiativeId)))); } } diff --git a/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java b/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java index 062d69c4..b7407036 100644 --- a/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java +++ b/src/main/java/it/gov/pagopa/admissibility/utils/OnboardingConstants.java @@ -37,4 +37,13 @@ private OnboardingConstants(){} //region global sequence id public static final String ANPR_E002_INVOKE = "ANPR_E002_INVOKE"; //endregion + + public static final class ExceptionCode { + private ExceptionCode(){} + + public static final String TOO_MANY_REQUESTS = "ADMISSIBILITY_TOO_MANY_REQUESTS"; + public static final String GENERIC_ERROR = "ADMISSIBILITY_GENERIC_ERROR"; + + public static final String INITIATIVE_NOT_FOUND = "ADMISSIBILITY_INITIATIVE_NOT_FOUND"; + } } diff --git a/src/main/java/it/gov/pagopa/common/web/exception/ErrorManager.java b/src/main/java/it/gov/pagopa/common/web/exception/ErrorManager.java index ac4b18c5..2438f7d8 100644 --- a/src/main/java/it/gov/pagopa/common/web/exception/ErrorManager.java +++ b/src/main/java/it/gov/pagopa/common/web/exception/ErrorManager.java @@ -5,17 +5,22 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.lang.Nullable; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.server.ServerWebExchange; +import java.util.Optional; + @RestControllerAdvice @Slf4j public class ErrorManager { - private static final ErrorDTO defaultErrorDTO; - static { - defaultErrorDTO =new ErrorDTO("Error", "Something gone wrong"); + private final ErrorDTO defaultErrorDTO; + + public ErrorManager(@Nullable ErrorDTO defaultErrorDTO) { + this.defaultErrorDTO = Optional.ofNullable(defaultErrorDTO) + .orElse(new ErrorDTO("Error", "Something gone wrong")); } @ExceptionHandler(RuntimeException.class) protected ResponseEntity handleException(RuntimeException error, ServerWebExchange exchange) { diff --git a/src/main/java/it/gov/pagopa/common/web/exception/MongoExceptionHandler.java b/src/main/java/it/gov/pagopa/common/web/exception/MongoExceptionHandler.java index 6b5c0b8f..d5906a4f 100644 --- a/src/main/java/it/gov/pagopa/common/web/exception/MongoExceptionHandler.java +++ b/src/main/java/it/gov/pagopa/common/web/exception/MongoExceptionHandler.java @@ -12,10 +12,13 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity.BodyBuilder; +import org.springframework.lang.Nullable; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.server.ServerWebExchange; +import java.util.Optional; + @RestControllerAdvice @Slf4j @Order(Ordered.HIGHEST_PRECEDENCE) @@ -23,8 +26,13 @@ public class MongoExceptionHandler { private final ErrorManager errorManager; - public MongoExceptionHandler(ErrorManager errorManager) { + private final ErrorDTO tooManyRequestsErrorDTO; + + public MongoExceptionHandler(ErrorManager errorManager, @Nullable ErrorDTO tooManyRequestsErrorDTO) { this.errorManager = errorManager; + + this.tooManyRequestsErrorDTO = Optional.ofNullable(tooManyRequestsErrorDTO) + .orElse(new ErrorDTO("TOO_MANY_REQUESTS", "Too Many Requests")); } @ExceptionHandler(DataAccessException.class) @@ -66,7 +74,7 @@ private ResponseEntity getErrorDTOResponseEntity(Exception ex, } return bodyBuilder - .body(new ErrorDTO("TOO_MANY_REQUESTS", "TOO_MANY_REQUESTS")); + .body(tooManyRequestsErrorDTO); } } \ No newline at end of file diff --git a/src/test/java/it/gov/pagopa/common/reactive/mongo/retry/MongoRequestRateTooLargeRetryIntegrationTest.java b/src/test/java/it/gov/pagopa/common/reactive/mongo/retry/MongoRequestRateTooLargeRetryIntegrationTest.java index 9b55b8e8..e5854981 100644 --- a/src/test/java/it/gov/pagopa/common/reactive/mongo/retry/MongoRequestRateTooLargeRetryIntegrationTest.java +++ b/src/test/java/it/gov/pagopa/common/reactive/mongo/retry/MongoRequestRateTooLargeRetryIntegrationTest.java @@ -63,6 +63,7 @@ @WebFluxTest class MongoRequestRateTooLargeRetryIntegrationTest { + public static final String EXPECTED_TOO_MANY_REQUESTS_ERROR = "{\"code\":\"TOO_MANY_REQUESTS\",\"message\":\"Too Many Requests\"}"; @Value("${mongo.request-rate-too-large.batch.max-retry:3}") private int maxRetry; @Value("${mongo.request-rate-too-large.batch.max-millis-elapsed:0}") @@ -177,7 +178,7 @@ void testController_MonoMethod() { .uri(uriBuilder -> uriBuilder.path("/testMono").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.TOO_MANY_REQUESTS) - .expectBody().json("{\"code\":\"TOO_MANY_REQUESTS\",\"message\":\"TOO_MANY_REQUESTS\"}"); + .expectBody().json(EXPECTED_TOO_MANY_REQUESTS_ERROR); Assertions.assertEquals(1, counter[0]); } @@ -188,7 +189,7 @@ void testController_MonoMethodRetryable() { .uri(uriBuilder -> uriBuilder.path("/testMonoRetryable").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.TOO_MANY_REQUESTS) - .expectBody().json("{\"code\":\"TOO_MANY_REQUESTS\",\"message\":\"TOO_MANY_REQUESTS\"}"); + .expectBody().json(EXPECTED_TOO_MANY_REQUESTS_ERROR); Assertions.assertEquals(API_RETRYABLE_MAX_RETRY + 1, counter[0]); } @@ -199,7 +200,7 @@ void testController_FluxMethod() { .uri(uriBuilder -> uriBuilder.path("/testFlux").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.TOO_MANY_REQUESTS) - .expectBody().json("{\"code\":\"TOO_MANY_REQUESTS\",\"message\":\"TOO_MANY_REQUESTS\"}"); + .expectBody().json(EXPECTED_TOO_MANY_REQUESTS_ERROR); Assertions.assertEquals(1, counter[0]); } @@ -210,7 +211,7 @@ void testController_FluxMethodRetryable() { .uri(uriBuilder -> uriBuilder.path("/testFluxRetryable").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.TOO_MANY_REQUESTS) - .expectBody().json("{\"code\":\"TOO_MANY_REQUESTS\",\"message\":\"TOO_MANY_REQUESTS\"}"); + .expectBody().json(EXPECTED_TOO_MANY_REQUESTS_ERROR); Assertions.assertEquals(API_RETRYABLE_MAX_RETRY + 1, counter[0]); } diff --git a/src/test/java/it/gov/pagopa/common/reactive/web/exception/MongoExceptionHandlerTest.java b/src/test/java/it/gov/pagopa/common/reactive/web/exception/MongoExceptionHandlerTest.java index 5c5076e6..db45652a 100644 --- a/src/test/java/it/gov/pagopa/common/reactive/web/exception/MongoExceptionHandlerTest.java +++ b/src/test/java/it/gov/pagopa/common/reactive/web/exception/MongoExceptionHandlerTest.java @@ -33,6 +33,9 @@ MongoExceptionHandlerTest.TestController.class, ErrorManager.class}) class MongoExceptionHandlerTest { + public static final ErrorDTO EXPECTED_DEFAULT_ERROR = new ErrorDTO("Error", "Something gone wrong"); + public static final ErrorDTO EXPECTED_TOO_MANY_REQUESTS_ERROR = new ErrorDTO("TOO_MANY_REQUESTS", "Too Many Requests"); + @Autowired private WebTestClient webTestClient; @@ -66,7 +69,6 @@ void handleUncategorizedMongoDbException() { doThrow( new UncategorizedMongoDbException(mongoQueryException.getMessage(), mongoQueryException)) .when(testControllerSpy).testEndpoint(); - ErrorDTO expectedErrorDefault = new ErrorDTO("TOO_MANY_REQUESTS","TOO_MANY_REQUESTS"); webTestClient.get() .uri(uriBuilder -> uriBuilder.path("/test").build()) @@ -75,7 +77,7 @@ void handleUncategorizedMongoDbException() { .expectHeader().exists(HttpHeaders.RETRY_AFTER) .expectHeader().valueEquals(HttpHeaders.RETRY_AFTER, "1") .expectHeader().valueEquals("Retry-After-Ms", "34") - .expectBody(ErrorDTO.class).isEqualTo(expectedErrorDefault); + .expectBody(ErrorDTO.class).isEqualTo(EXPECTED_TOO_MANY_REQUESTS_ERROR); } @Test @@ -94,7 +96,6 @@ void handleTooManyWriteDbException() { doThrow( new DataIntegrityViolationException(mongoWriteException.getMessage(), mongoWriteException)) .when(testControllerSpy).testEndpoint(); - ErrorDTO expectedErrorDefault = new ErrorDTO("TOO_MANY_REQUESTS","TOO_MANY_REQUESTS"); webTestClient.get() .uri(uriBuilder -> uriBuilder.path("/test").build()) @@ -103,7 +104,7 @@ void handleTooManyWriteDbException() { .expectHeader().exists(HttpHeaders.RETRY_AFTER) .expectHeader().valueEquals(HttpHeaders.RETRY_AFTER, "1") .expectHeader().valueEquals("Retry-After-Ms", "34") - .expectBody(ErrorDTO.class).isEqualTo(expectedErrorDefault); + .expectBody(ErrorDTO.class).isEqualTo(EXPECTED_TOO_MANY_REQUESTS_ERROR); } @Test @@ -112,13 +113,11 @@ void handleUncategorizedMongoDbExceptionNotRequestRateTooLarge() { doThrow(new UncategorizedMongoDbException("DUMMY", new Exception())) .when(testControllerSpy).testEndpoint(); - ErrorDTO expectedErrorDefault = new ErrorDTO("Error","Something gone wrong"); - webTestClient.get() .uri(uriBuilder -> uriBuilder.path("/test").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR) - .expectBody(ErrorDTO.class).isEqualTo(expectedErrorDefault); + .expectBody(ErrorDTO.class).isEqualTo(EXPECTED_DEFAULT_ERROR); } @Test @@ -126,12 +125,10 @@ void handleMongoRequestRateTooLargeRetryExpiredException() { doThrow(new MongoRequestRateTooLargeRetryExpiredException("FLOWNAME",3,3,0,100,34L,new Exception())) .when(testControllerSpy).testEndpoint(); - ErrorDTO expectedErrorDefault = new ErrorDTO("TOO_MANY_REQUESTS","TOO_MANY_REQUESTS"); - webTestClient.get() .uri(uriBuilder -> uriBuilder.path("/test").build()) .exchange() .expectStatus().isEqualTo(HttpStatus.TOO_MANY_REQUESTS) - .expectBody(ErrorDTO.class).isEqualTo(expectedErrorDefault); + .expectBody(ErrorDTO.class).isEqualTo(EXPECTED_TOO_MANY_REQUESTS_ERROR); } } \ No newline at end of file From 2f681a31e4909539d5e8c2a3629c8d810371a8c8 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:17:27 +0100 Subject: [PATCH 5/6] fix: IDP-2223 fix empty Drools Container handling (#241) --- .../AdmissibilityErrorManagerConfig.java | 2 +- .../evaluate/RuleEngineServiceImpl.java | 43 +++++++++------ ...ngContextHolderServiceIntegrationTest.java | 4 +- .../evaluate/RuleEngineServiceImplTest.java | 55 ++++++++++++++++++- 4 files changed, 81 insertions(+), 23 deletions(-) diff --git a/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java b/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java index c84d654b..ec928537 100644 --- a/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java +++ b/src/main/java/it/gov/pagopa/admissibility/config/AdmissibilityErrorManagerConfig.java @@ -12,7 +12,7 @@ public class AdmissibilityErrorManagerConfig { ErrorDTO defaultErrorDTO() { return new ErrorDTO( OnboardingConstants.ExceptionCode.GENERIC_ERROR, - "A generic error occurred for payment" + "A generic error occurred" ); } diff --git a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java index ce1064c0..a101bc07 100644 --- a/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java +++ b/src/main/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImpl.java @@ -43,26 +43,30 @@ public EvaluationDTO applyRules(OnboardingDTO onboardingRequest, InitiativeConfi OnboardingDroolsDTO req = onboarding2OnboardingDroolsMapper.apply(onboardingRequest); - if(checkIfKieBaseContainerIsReady(initiative)) { - StatelessKieSession statelessKieSession = onboardingContextHolderService.getBeneficiaryRulesKieBase().newStatelessKieSession(); + if(checkIfKieBaseShouldBeInvolved(initiative)) { + if (checkIfKieBaseContainerIsReady(initiative)) { + StatelessKieSession statelessKieSession = onboardingContextHolderService.getBeneficiaryRulesKieBase().newStatelessKieSession(); - @SuppressWarnings("unchecked") - List> cmds = Arrays.asList( - CommandFactory.newInsert(req), - CommandFactory.newInsert(criteriaCodeService), - new AgendaGroupSetFocusCommand(req.getInitiativeId()) - ); + @SuppressWarnings("unchecked") + List> cmds = Arrays.asList( + CommandFactory.newInsert(req), + CommandFactory.newInsert(criteriaCodeService), + new AgendaGroupSetFocusCommand(req.getInitiativeId()) + ); - long before = System.currentTimeMillis(); - statelessKieSession.execute(CommandFactory.newBatchExecution(cmds)); + long before = System.currentTimeMillis(); + statelessKieSession.execute(CommandFactory.newBatchExecution(cmds)); - PerformanceLogger.logTiming("ONBOARDING_RULE_ENGINE", before, "resulted into rejections %s".formatted(req.getOnboardingRejectionReasons())); + PerformanceLogger.logTiming("ONBOARDING_RULE_ENGINE", before, "resulted into rejections %s".formatted(req.getOnboardingRejectionReasons())); + } else { + req.getOnboardingRejectionReasons().add(new OnboardingRejectionReason( + OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR, + OnboardingConstants.REJECTION_REASON_RULE_ENGINE_NOT_READY, + null, null, null + )); + } } else { - req.getOnboardingRejectionReasons().add(new OnboardingRejectionReason( - OnboardingRejectionReason.OnboardingRejectionReasonType.TECHNICAL_ERROR, - OnboardingConstants.REJECTION_REASON_RULE_ENGINE_NOT_READY, - null, null, null - )); + log.info("[ONBOARDING_REQUEST][RULE_ENGINE] Selected not drools involved initiative: {}", initiative.getInitiativeId()); } log.trace("[ONBOARDING_REQUEST] [RULE_ENGINE] Send message prepared: {}", req); @@ -70,9 +74,12 @@ public EvaluationDTO applyRules(OnboardingDTO onboardingRequest, InitiativeConfi return onboarding2EvaluationMapper.apply(req, initiative, req.getOnboardingRejectionReasons()); } + private boolean checkIfKieBaseShouldBeInvolved(InitiativeConfig initiative) { + return !CollectionUtils.isEmpty(initiative.getAutomatedCriteria()); // the drools container is supposed to not be involved + } + private boolean checkIfKieBaseContainerIsReady(InitiativeConfig initiative) { - return CollectionUtils.isEmpty(initiative.getAutomatedCriteria()) || // the drools container is supposed to not be involved - onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds() // the initiative is inside the container drools + return onboardingContextHolderService.getBeneficiaryRulesKieInitiativeIds() // the initiative is inside the container drools .contains(initiative.getInitiativeId()); } } diff --git a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceIntegrationTest.java b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceIntegrationTest.java index 873df24d..99f08183 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceIntegrationTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/OnboardingContextHolderServiceIntegrationTest.java @@ -2,6 +2,7 @@ import it.gov.pagopa.admissibility.BaseIntegrationTest; import it.gov.pagopa.admissibility.dto.onboarding.*; +import it.gov.pagopa.admissibility.dto.rule.AutomatedCriteriaDTO; import it.gov.pagopa.admissibility.model.DroolsRule; import it.gov.pagopa.admissibility.model.InitiativeConfig; import it.gov.pagopa.admissibility.service.build.KieContainerBuilderService; @@ -105,7 +106,7 @@ void testKieBuildWithRedisCache() { EvaluationDTO result = executeRules(onboardingMock); Assertions.assertNotNull(result); - Assertions.assertTrue(result instanceof EvaluationCompletedDTO); + Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); EvaluationCompletedDTO resultCompleted = (EvaluationCompletedDTO) result; Assertions.assertEquals(expectedRejectionReason, resultCompleted.getOnboardingRejectionReasons()); @@ -166,6 +167,7 @@ private EvaluationDTO executeRules(OnboardingDTO onb) { .automatedCriteriaCodes(List.of("CODE1")) .organizationId("ORGANIZATION-ID") .startDate(LocalDate.MIN) + .automatedCriteria(List.of(new AutomatedCriteriaDTO())) .build(); return ruleEngineService.applyRules(onb, config); diff --git a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java index 7982d120..63f68af5 100644 --- a/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/service/onboarding/evaluate/RuleEngineServiceImplTest.java @@ -26,6 +26,7 @@ import java.time.LocalDateTime; import java.util.Collections; import java.util.List; +import java.util.Set; @ExtendWith(MockitoExtension.class) @Slf4j @@ -69,9 +70,14 @@ void applyRules() { initiativeConfig.setInitiativeName("INITIATIVENAME"); initiativeConfig.setOrganizationId("ORGANIZATIONID"); + initiativeConfig.setAutomatedCriteria(List.of(new AutomatedCriteriaDTO())); + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) .thenReturn(buildContainer(initiativeId)); + Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieInitiativeIds()) + .thenReturn(Set.of(initiativeId)); + // When EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); @@ -104,9 +110,6 @@ void testNotRules_notKieBasedInitiatives(){ InitiativeConfig initiativeConfig = new InitiativeConfig(); initiativeConfig.setInitiativeId(initiativeId); - Mockito.when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) - .thenReturn(buildContainer(INITIATIVEID)); - // When EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); @@ -159,4 +162,50 @@ private KieBase buildContainer(String initiativeId) { return new KieContainerBuilderServiceImpl(Mockito.mock(DroolsRuleRepository.class)).build(Flux.just(rule, ignoredRule)).block(); } + + @Test + void testEmptyDroolsContainer_noRules_nullCriteria() { + testEmptyDroolsContainer("package dummy;", null); + } + + @Test + void testEmptyDroolsContainer_noAgendaRules_emptyCriteria() { + testEmptyDroolsContainer(""" + package dummy; + rule "RULENAME" + when eval(true) + then return; + end + """, Collections.emptyList()); + } + + void testEmptyDroolsContainer(String rule, List automatedCriteriaDTOS) { + // Given + String initiativeId = "NOTKIEINITITATIVEID"; + + KieBase emptyContainer = new KieContainerBuilderServiceImpl(Mockito.mock(DroolsRuleRepository.class)) + .build(Flux.just(DroolsRule.builder(). + id(initiativeId) + .name("RULENAME") + .rule(rule) + .build())).block(); + Mockito.lenient() //not more invoked after fix + .when(onboardingContextHolderServiceMock.getBeneficiaryRulesKieBase()) + .thenReturn(emptyContainer); + + OnboardingDTO onboardingDTO = new OnboardingDTO(); + onboardingDTO.setInitiativeId(initiativeId); + + InitiativeConfig initiativeConfig = new InitiativeConfig(); + initiativeConfig.setInitiativeId(initiativeId); + initiativeConfig.setAutomatedCriteria(automatedCriteriaDTOS); + + // When + EvaluationDTO result = ruleEngineService.applyRules(onboardingDTO, initiativeConfig); + + // Then + Assertions.assertInstanceOf(EvaluationCompletedDTO.class, result); + Assertions.assertEquals(OnboardingEvaluationStatus.ONBOARDING_OK, ((EvaluationCompletedDTO) result).getStatus()); + Assertions.assertEquals(Collections.emptyList(), ((EvaluationCompletedDTO) result).getOnboardingRejectionReasons()); + } } From 3690c7ebb33e2b3ad0cea3bcba1f93540e75c477 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Fri, 12 Jan 2024 12:05:31 +0100 Subject: [PATCH 6/6] fix: IDP-2196 fix Cve (#243) --- .grype.yaml | 2 +- Dockerfile | 9 +- pom.xml | 93 +++++++++++-------- .../admissibility/BaseIntegrationTest.java | 4 - .../kafka/KafkaTestUtilitiesService.java | 6 +- 5 files changed, 60 insertions(+), 54 deletions(-) diff --git a/.grype.yaml b/.grype.yaml index 70db95e6..b030c206 100644 --- a/.grype.yaml +++ b/.grype.yaml @@ -1,6 +1,6 @@ ignore: # false positive match on reactor-netty packages due to a bug on grype: https://github.com/anchore/grype/issues/431 - # Actually we are using netty 4.1.100 + # Actually we are using netty 4.1.104 - vulnerability: CVE-2014-3488 # solved in netty 3.9.2 - vulnerability: CVE-2015-2156 # solved in netty 4.1.42 - vulnerability: CVE-2019-16869 # solved in netty 4.1.42 diff --git a/Dockerfile b/Dockerfile index 73c386f1..deff9032 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # # Build # -FROM maven:3.9.5-amazoncorretto-17-al2023@sha256:eeaa7ab572d931f7273fc5cf31429923f172091ae388969e11f42ec6dd817d74 AS buildtime +FROM maven:3.9.6-amazoncorretto-17-al2023@sha256:9ace9c9e506877b0e1877a7f709fa9dc7895d5fbdcc93d4170dfb3d25e2839e9 AS buildtime WORKDIR /build COPY . . @@ -11,10 +11,7 @@ RUN mvn clean package -DskipTests # # Docker RUNTIME # -FROM amazoncorretto:17.0.9-alpine3.18@sha256:df48bf2e183230040890460ddb4359a10aa6c7aad24bd88899482c52053c7e17 AS runtime - -# security fixes -RUN apk update && apk upgrade --no-cache libcrypto3 libssl3 +FROM amazoncorretto:17.0.9-alpine3.18@sha256:ed14b8c2f00dbb7b94446aa01d00583976ff0eda2577f5474035f3b4cf078dfd AS runtime RUN apk --no-cache add shadow RUN useradd --uid 10000 runner @@ -24,7 +21,7 @@ WORKDIR /app COPY --from=buildtime /build/target/*.jar /app/app.jar # The agent is enabled at runtime via JAVA_TOOL_OPTIONS. -ADD https://github.com/microsoft/ApplicationInsights-Java/releases/download/3.4.18/applicationinsights-agent-3.4.18.jar /app/applicationinsights-agent.jar +ADD https://github.com/microsoft/ApplicationInsights-Java/releases/download/3.4.19/applicationinsights-agent-3.4.19.jar /app/applicationinsights-agent.jar RUN chown -R runner:runner /app diff --git a/pom.xml b/pom.xml index 2f49de58..dc4e027d 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.springframework.boot spring-boot-starter-parent - 3.1.5 + 3.1.7 it.gov.pagopa @@ -46,6 +46,17 @@ org.springframework.boot spring-boot-starter-aop + + org.springdoc + springdoc-openapi-starter-webflux-ui + 2.3.0 + + + + + com.azure.spring + spring-cloud-azure-stream-binder-servicebus + @@ -64,13 +75,6 @@ 7.74.1.Final - - - com.azure.spring - spring-cloud-azure-stream-binder-servicebus - 5.5.0 - - jakarta.activation @@ -107,17 +111,17 @@ org.apache.commons commons-text - 1.10.0 + 1.11.0 org.codehaus.janino janino - 3.1.10 + 3.1.11 com.google.guava guava - 32.1.3-jre + 33.0.0-jre org.openapitools @@ -129,15 +133,10 @@ java-jwt 4.4.0 - - org.springdoc - springdoc-openapi-starter-webflux-ui - 2.2.0 - org.apache.commons commons-lang3 - 3.13.0 + 3.14.0 @@ -160,11 +159,12 @@ org.springframework.boot spring-boot-starter-test test - - - org.junit.jupiter - junit-jupiter-engine - test + + + junit + junit + + org.junit.platform @@ -179,7 +179,6 @@ org.springframework.cloud spring-cloud-contract-wiremock - 4.0.4 test @@ -192,11 +191,17 @@ javafaker 1.0.2 test + + + org.yaml + snakeyaml + + de.flapdoodle.embed de.flapdoodle.embed.mongo.spring30x - 4.9.3 + 4.11.0 test @@ -232,8 +237,16 @@ io.netty netty-bom - - 4.1.100.Final + + 4.1.104.Final + pom + import + + + + com.azure.spring + spring-cloud-azure-dependencies + 5.8.0 pom import @@ -241,18 +254,6 @@ - - org.springframework.boot - spring-boot-maven-plugin - ${project.parent.version} - - - - build-info - - - - org.apache.maven.plugins maven-compiler-plugin @@ -264,14 +265,27 @@ org.apache.maven.plugins maven-surefire-plugin + 3.2.3 @{argLine} --add-opens java.base/java.lang=ALL-UNNAMED + + org.springframework.boot + spring-boot-maven-plugin + ${project.parent.version} + + + + build-info + + + + io.github.git-commit-id git-commit-id-maven-plugin - 5.0.0 + 7.0.0 get-the-git-infos @@ -298,6 +312,7 @@ + org.openapitools openapi-generator-maven-plugin diff --git a/src/test/java/it/gov/pagopa/admissibility/BaseIntegrationTest.java b/src/test/java/it/gov/pagopa/admissibility/BaseIntegrationTest.java index c9eebca8..79ddb95d 100644 --- a/src/test/java/it/gov/pagopa/admissibility/BaseIntegrationTest.java +++ b/src/test/java/it/gov/pagopa/admissibility/BaseIntegrationTest.java @@ -60,9 +60,6 @@ }, controlledShutdown = true) @TestPropertySource( properties = { - // even if enabled into application.yml, spring test will not load it https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.testing.spring-boot-applications.jmx -// "spring.jmx.enabled=true", - //region common feature disabled "app.beneficiary-rule.cache.refresh-ms-rate:600000", "logging.level.it.gov.pagopa.common.kafka.service.ErrorNotifierServiceImpl=WARN", @@ -75,7 +72,6 @@ "logging.level.state.change.logger=WARN", "spring.cloud.stream.kafka.binder.configuration.security.protocol=PLAINTEXT", "spring.kafka.bootstrap-servers=${spring.embedded.kafka.brokers}", - "spring.cloud.stream.kafka.binder.zkNodes=${spring.embedded.zookeeper.connect}", "spring.cloud.stream.binders.kafka-beneficiary-rule-builder.environment.spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}", "spring.cloud.stream.binders.kafka-onboarding-outcome.environment.spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}", "spring.cloud.stream.binders.kafka-ranking-request.environment.spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}", diff --git a/src/test/java/it/gov/pagopa/common/kafka/KafkaTestUtilitiesService.java b/src/test/java/it/gov/pagopa/common/kafka/KafkaTestUtilitiesService.java index 3f05def8..7167b68e 100644 --- a/src/test/java/it/gov/pagopa/common/kafka/KafkaTestUtilitiesService.java +++ b/src/test/java/it/gov/pagopa/common/kafka/KafkaTestUtilitiesService.java @@ -65,8 +65,6 @@ public class KafkaTestUtilitiesService { private String applicationName; @Value("${spring.kafka.bootstrap-servers}") private String bootstrapServers; - @Value("${spring.cloud.stream.kafka.binder.zkNodes}") - private String zkNodes; @Autowired private ObjectMapper objectMapper; @@ -111,7 +109,7 @@ void clearTopics() { /** It will return usefull URLs related to embedded kafka */ public String getKafkaUrls() { - return "bootstrapServers: %s, zkNodes: %s".formatted(bootstrapServers, zkNodes); + return "bootstrapServers: %s".formatted(bootstrapServers); } //region consume messages @@ -320,7 +318,7 @@ public void assertCommitOrder(String flowName, int totalSendMessages) { } //endregion -//region error topic + //region error topic public void checkErrorsPublished(String topicErrors, Pattern errorUseCaseIdPatternMatch, int expectedErrorMessagesNumber, long maxWaitingMs, List, java.util.function.Consumer>>> errorUseCases) { final List> errors = consumeMessages(topicErrors, expectedErrorMessagesNumber, maxWaitingMs); for (final ConsumerRecord record : errors) {