From 676194789a8b880e2416f9d3bf2484a9fc6be1bc Mon Sep 17 00:00:00 2001 From: "a.sreekumar" Date: Tue, 15 Feb 2022 12:16:30 +0000 Subject: Fix issue with GeneratedValue in PfGeneratedIdKey PfGeneratedIdKey class (which is used as a composite key in JpaPolicyAudit and JpaPdpStatistics) uses GeneratedValue in a wrong way and not according to the specification. This review fixes it. PfGeneratedIdKey class is removed, and the generatedId is directly specified in the JpaPolicyAudit and JpaPdpStatistics classes. Note: These classes are only used by PAP, so the related methods for db interaction is removed as PAP directly talks to DB using spring repository layer. Also the only end result this change brings is that the 'generatedId' alone will be used as the primary key instead of 'generatedId, name and version' together. Corresponding changes in DB Migrator: https://gerrit.onap.org/r/c/policy/docker/+/127139 PAP: https://gerrit.onap.org/r/c/policy/pap/+/127130 Change-Id: Ib4ea8b60ffe5c2480746569c0354bf474a6b7006 Issue-ID: POLICY-3897 Signed-off-by: a.sreekumar --- .../pap/persistence/concepts/JpaPolicyAudit.java | 83 ++++++-- .../persistence/provider/PolicyAuditProvider.java | 146 ------------- .../provider/PolicyAuditProviderTest.java | 234 --------------------- 3 files changed, 63 insertions(+), 400 deletions(-) delete mode 100644 models-pap/src/main/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProvider.java delete mode 100644 models-pap/src/test/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProviderTest.java (limited to 'models-pap') diff --git a/models-pap/src/main/java/org/onap/policy/models/pap/persistence/concepts/JpaPolicyAudit.java b/models-pap/src/main/java/org/onap/policy/models/pap/persistence/concepts/JpaPolicyAudit.java index e151441da..35e385637 100644 --- a/models-pap/src/main/java/org/onap/policy/models/pap/persistence/concepts/JpaPolicyAudit.java +++ b/models-pap/src/main/java/org/onap/policy/models/pap/persistence/concepts/JpaPolicyAudit.java @@ -1,7 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2021 Nordix Foundation. - * Modifications Copyright (C) 2021 Bell Canada. All rights reserved. + * Modifications Copyright (C) 2021-2022 Bell Canada. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,28 +22,36 @@ package org.onap.policy.models.pap.persistence.concepts; import java.time.Instant; +import java.util.ArrayList; import java.util.Date; import java.util.List; import javax.persistence.Column; -import javax.persistence.EmbeddedId; import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; import javax.persistence.Index; import javax.persistence.Inheritance; import javax.persistence.InheritanceType; import javax.persistence.Table; +import javax.persistence.TableGenerator; import javax.persistence.Temporal; import javax.persistence.TemporalType; import javax.validation.constraints.NotNull; import lombok.Data; import lombok.EqualsAndHashCode; +import lombok.NonNull; import org.apache.commons.lang3.builder.CompareToBuilder; +import org.onap.policy.common.parameters.BeanValidationResult; +import org.onap.policy.common.parameters.ValidationStatus; +import org.onap.policy.common.parameters.annotations.Pattern; import org.onap.policy.common.utils.validation.Assertions; import org.onap.policy.models.base.PfAuthorative; import org.onap.policy.models.base.PfConcept; -import org.onap.policy.models.base.PfGeneratedIdKey; +import org.onap.policy.models.base.PfConceptKey; import org.onap.policy.models.base.PfKey; import org.onap.policy.models.base.PfReferenceKey; -import org.onap.policy.models.base.validation.annotations.VerifyKey; +import org.onap.policy.models.base.Validated; import org.onap.policy.models.pap.concepts.PolicyAudit; import org.onap.policy.models.pap.concepts.PolicyAudit.AuditAction; import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifier; @@ -62,11 +70,24 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifier; public class JpaPolicyAudit extends PfConcept implements PfAuthorative { private static final long serialVersionUID = -2935734300607322191L; - @EmbeddedId - @Column - @NotNull - @VerifyKey(versionNotNull = true) - private PfGeneratedIdKey key; + @Id + @Column(name = "ID") + @GeneratedValue(strategy = GenerationType.TABLE, generator = "auditIdGen") + @TableGenerator( + name = "auditIdGen", + table = "sequence", + pkColumnName = "SEQ_NAME", + valueColumnName = "SEQ_COUNT", + pkColumnValue = "SEQ_GEN") + private Long generatedId; + + @Column(name = "name", length = 120) + @Pattern(regexp = PfKey.NAME_REGEXP) + private String name; + + @Column(name = "version", length = 20) + @Pattern(regexp = PfKey.VERSION_REGEXP) + private String version; @Column private String pdpGroup; @@ -90,7 +111,8 @@ public class JpaPolicyAudit extends PfConcept implements PfAuthorative getKeys() { - return getKey().getKeys(); + final List keyList = new ArrayList<>(); + keyList.add(getKey()); + return keyList; } @Override - public void clean() { - key.clean(); + public PfKey getKey() { + return new PfConceptKey(name, version); + } + @Override + public void clean() { pdpGroup = Assertions.validateStringParameter("pdpGroup", pdpGroup, PfReferenceKey.LOCAL_NAME_REGEXP); pdpType = Assertions.validateStringParameter("pdpType", pdpType, PfReferenceKey.LOCAL_NAME_REGEXP); user = Assertions.validateStringParameter("user", user, PfReferenceKey.LOCAL_NAME_REGEXP); } + + @Override + public BeanValidationResult validate(@NonNull String fieldName) { + BeanValidationResult result = super.validate(fieldName); + if (PfKey.NULL_KEY_NAME.equals(name)) { + result.addResult("name", name, ValidationStatus.INVALID, Validated.IS_NULL); + } + return result; + } } diff --git a/models-pap/src/main/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProvider.java b/models-pap/src/main/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProvider.java deleted file mode 100644 index ec759d34d..000000000 --- a/models-pap/src/main/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProvider.java +++ /dev/null @@ -1,146 +0,0 @@ -/*- - * ============LICENSE_START======================================================= - * Copyright (C) 2021 Nordix Foundation. - * Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved. - * Modifications Copyright (C) 2021 Bell Canada. All rights reserved. - * ================================================================================ - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://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 - * ============LICENSE_END========================================================= - */ - -package org.onap.policy.models.pap.persistence.provider; - -import java.time.Instant; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; -import javax.ws.rs.core.Response; -import lombok.Builder; -import lombok.Data; -import lombok.NonNull; -import org.apache.commons.lang3.StringUtils; -import org.onap.policy.common.parameters.BeanValidationResult; -import org.onap.policy.models.base.PfModelRuntimeException; -import org.onap.policy.models.dao.PfDao; -import org.onap.policy.models.dao.PfFilterParametersIntfc; -import org.onap.policy.models.pap.concepts.PolicyAudit; -import org.onap.policy.models.pap.concepts.PolicyAudit.AuditAction; -import org.onap.policy.models.pap.persistence.concepts.JpaPolicyAudit; - -/** - * Provider for Policy Audit. - * - * @author Adheli Tavares (adheli.tavares@est.tech) - * - */ -public class PolicyAuditProvider { - - private static final Integer DEFAULT_MAX_RECORDS = 100; - private static final Integer DEFAULT_MIN_RECORDS = 10; - - /** - * Create audit records. - * - * @param audits list of policy audit - */ - public void createAuditRecords(@NonNull PfDao dao, @NonNull final List audits) { - List jpaAudits = audits.stream().map(JpaPolicyAudit::new).collect(Collectors.toList()); - - var result = new BeanValidationResult("createAuditRecords", jpaAudits); - - var count = 0; - for (JpaPolicyAudit jpaAudit : jpaAudits) { - result.addResult(jpaAudit.validate(String.valueOf(count++))); - } - - if (!result.isValid()) { - throw new PfModelRuntimeException(Response.Status.BAD_REQUEST, result.getResult()); - } - - dao.createCollection(jpaAudits); - } - - /** - * Collect audit records based on filters at {@link AuditFilter}. - * - * @param auditFilter {@link AuditFilter} object with filters for search - * @return list of {@link PolicyAudit} records - */ - public List getAuditRecords(@NonNull PfDao dao, @NonNull AuditFilter auditFilter) { - if (auditFilter.getRecordNum() < 1) { - auditFilter.setRecordNum(DEFAULT_MIN_RECORDS); - } else if (auditFilter.getRecordNum() > DEFAULT_MAX_RECORDS) { - auditFilter.setRecordNum(DEFAULT_MAX_RECORDS); - } - - return dao.getFiltered(JpaPolicyAudit.class, auditFilter).stream().map(JpaPolicyAudit::toAuthorative) - .collect(Collectors.toList()); - } - - /** - * Create a filter for looking for audit records. - * name - policy name - * version - policy version - * pdpGroup - PDP group that policy might be related - * action - type of action/operation realized on policy - * fromDate - start of period in case of time interval search - */ - @Data - @Builder - public static class AuditFilter implements PfFilterParametersIntfc { - private String name; - private String version; - private AuditAction action; - private String pdpGroup; - private Instant fromDate; - private Instant toDate; - private int recordNum; - @Builder.Default - private String sortOrder = "DESC"; - - // initialized lazily, if not set via the builder - private Map filterMap; - - @Override - public Instant getStartTime() { - return fromDate; - } - - @Override - public Instant getEndTime() { - return toDate; - } - - @Override - public Map getFilterMap() { - if (filterMap != null) { - return filterMap; - } - - filterMap = new HashMap<>(); - - if (StringUtils.isNotBlank(pdpGroup)) { - filterMap.put("pdpGroup", pdpGroup); - } - - if (action != null) { - filterMap.put("action", action); - } - - return filterMap; - } - } -} diff --git a/models-pap/src/test/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProviderTest.java b/models-pap/src/test/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProviderTest.java deleted file mode 100644 index 41c9b92a6..000000000 --- a/models-pap/src/test/java/org/onap/policy/models/pap/persistence/provider/PolicyAuditProviderTest.java +++ /dev/null @@ -1,234 +0,0 @@ -/*- - * ============LICENSE_START======================================================= - * Copyright (C) 2021 Nordix Foundation. - * ================================================================================ - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://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 - * ============LICENSE_END========================================================= - */ - -package org.onap.policy.models.pap.persistence.provider; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; - -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.List; -import java.util.Properties; -import java.util.concurrent.TimeUnit; -import org.awaitility.Awaitility; -import org.eclipse.persistence.config.PersistenceUnitProperties; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.onap.policy.models.base.PfModelRuntimeException; -import org.onap.policy.models.dao.DaoParameters; -import org.onap.policy.models.dao.PfDao; -import org.onap.policy.models.dao.PfDaoFactory; -import org.onap.policy.models.dao.impl.DefaultPfDao; -import org.onap.policy.models.pap.concepts.PolicyAudit; -import org.onap.policy.models.pap.concepts.PolicyAudit.AuditAction; -import org.onap.policy.models.pap.persistence.provider.PolicyAuditProvider.AuditFilter; -import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifier; - -/** - * Class for unit testing {@link PolicyAuditProvider}. - * - * @author Adheli Tavares (adheli.tavares@est.tech) - * - */ -public class PolicyAuditProviderTest { - - private static final String FIELD_IS_NULL = "%s is marked .*ull but is null"; - private static final String GROUP_A = "groupA"; - private static final String GROUP_B = "groupB"; - private static final ToscaConceptIdentifier MY_POLICY = new ToscaConceptIdentifier("MyPolicy", "1.2.3"); - private static final ToscaConceptIdentifier MY_POLICY2 = new ToscaConceptIdentifier("MyPolicyB", "2.3.4"); - private static final Integer NUMBER_RECORDS = 10; - - private PfDao pfDao; - - /** - * Set up the DAO towards the database. - * - * @throws Exception on database errors - */ - @Before - public void setupDao() throws Exception { - final DaoParameters daoParameters = new DaoParameters(); - daoParameters.setPluginClass(DefaultPfDao.class.getName()); - - daoParameters.setPersistenceUnit("ToscaConceptTest"); - - Properties jdbcProperties = new Properties(); - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_USER, "policy"); - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_PASSWORD, "P01icY"); - - if (System.getProperty("USE-MARIADB") != null) { - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_DRIVER, "org.mariadb.jdbc.Driver"); - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_URL, "jdbc:mariadb://localhost:3306/policy"); - } else { - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_DRIVER, "org.h2.Driver"); - jdbcProperties.setProperty(PersistenceUnitProperties.JDBC_URL, "jdbc:h2:mem:PolicyAuditProviderTest"); - } - - daoParameters.setJdbcProperties(jdbcProperties); - - pfDao = new PfDaoFactory().createPfDao(daoParameters); - pfDao.init(daoParameters); - } - - @After - public void teardown() { - pfDao.close(); - } - - @Test - public void testCreatePolicyAudit() { - PolicyAuditProvider provider = new PolicyAuditProvider(); - - Instant date = Instant.now(); - provider.createAuditRecords(pfDao, generatePolicyAudits(date, GROUP_A, MY_POLICY)); - - List records = - provider.getAuditRecords(pfDao, AuditFilter.builder().recordNum(NUMBER_RECORDS).build()); - assertThat(records).hasSize(2); - - // as the start date is 10 min ahead of first record, shouldn't return any records - List emptyList = provider.getAuditRecords(pfDao, - AuditFilter.builder().fromDate(Instant.now().plusSeconds(600)).recordNum(600).build()); - assertThat(emptyList).isEmpty(); - } - - @Test - public void testCreatePolicyAuditInvalid() { - PolicyAuditProvider provider = new PolicyAuditProvider(); - - List audits = List.of(PolicyAudit.builder().pdpType("pdpType").action(AuditAction.DEPLOYMENT) - .timestamp(Instant.now()).build()); - - assertThrows(PfModelRuntimeException.class, () -> provider.createAuditRecords(pfDao, audits)); - - List records = - provider.getAuditRecords(pfDao, AuditFilter.builder().recordNum(NUMBER_RECORDS).build()); - assertThat(records).isEmpty(); - } - - @Test - public void testFilters() { - PolicyAuditProvider provider = new PolicyAuditProvider(); - - Instant date = Instant.now().truncatedTo(ChronoUnit.SECONDS); - provider.createAuditRecords(pfDao, generatePolicyAudits(date, GROUP_A, MY_POLICY)); - provider.createAuditRecords(pfDao, generatePolicyAudits(date, GROUP_B, MY_POLICY)); - provider.createAuditRecords(pfDao, generatePolicyAudits(date, GROUP_B, MY_POLICY2)); - Awaitility.await().pollDelay(3, TimeUnit.SECONDS).until(() -> { - return true; - }); - - List records = provider.getAuditRecords(pfDao, - AuditFilter.builder().fromDate(date).toDate(Instant.now()).recordNum(NUMBER_RECORDS).build()); - assertThat(records).hasSize(6); - - List recordsWithGroupB = provider.getAuditRecords(pfDao, - AuditFilter.builder().pdpGroup(GROUP_B).recordNum(NUMBER_RECORDS).build()); - assertThat(recordsWithGroupB).hasSize(4); - - List recordsWithActionDeploy = provider.getAuditRecords(pfDao, - AuditFilter.builder().action(AuditAction.DEPLOYMENT).recordNum(NUMBER_RECORDS).build()); - assertThat(recordsWithActionDeploy).hasSize(3); - - List recordsWithMyPolicy = provider.getAuditRecords(pfDao, AuditFilter.builder() - .name(MY_POLICY.getName()).version(MY_POLICY.getVersion()).recordNum(NUMBER_RECORDS).build()); - assertThat(recordsWithMyPolicy).hasSize(4); - } - - @Test - public void testLoadRecordsForLimit() { - PolicyAuditProvider provider = new PolicyAuditProvider(); - - List loadAudits = new ArrayList<>(); - - // going to create 102 records. - for (int i = 0; i <= 50; i++) { - loadAudits.addAll(generatePolicyAudits(Instant.now().plusSeconds(i), GROUP_A, MY_POLICY)); - } - - provider.createAuditRecords(pfDao, loadAudits); - - List records = - provider.getAuditRecords(pfDao, AuditFilter.builder().recordNum(NUMBER_RECORDS).build()); - assertThat(records).hasSize(10); - - // check that is being ordered - assertTrue(records.get(0).getTimestamp().isAfter(records.get(9).getTimestamp())); - assertEquals(loadAudits.get(loadAudits.size() - 1).getTimestamp(), records.get(0).getTimestamp()); - - // try to get 102 records should return 100 - records = provider.getAuditRecords(pfDao, AuditFilter.builder().recordNum(102).build()); - assertThat(records).hasSize(100); - - // try to get -1 records should return 10 - records = provider.getAuditRecords(pfDao, AuditFilter.builder().recordNum(-1).build()); - assertThat(records).hasSize(10); - } - - @Test - public void policyProviderExceptions() { - PolicyAuditProvider provider = new PolicyAuditProvider(); - - assertThatThrownBy(() -> { - provider.createAuditRecords(null, null); - }).hasMessageMatching(String.format(FIELD_IS_NULL, "dao")); - - assertThatThrownBy(() -> { - provider.createAuditRecords(pfDao, null); - }).hasMessageMatching(String.format(FIELD_IS_NULL, "audits")); - - assertThatThrownBy(() -> { - provider.getAuditRecords(null, AuditFilter.builder().build()); - }).hasMessageMatching(String.format(FIELD_IS_NULL, "dao")); - - assertThatThrownBy(() -> { - provider.getAuditRecords(pfDao, null); - }).hasMessageMatching(String.format(FIELD_IS_NULL, "auditFilter")); - } - - private List generatePolicyAudits(Instant date, String group, ToscaConceptIdentifier policy) { - // @formatter:off - PolicyAudit deploy = PolicyAudit.builder() - .pdpGroup(group) - .pdpType("pdpType") - .policy(policy) - .action(AuditAction.DEPLOYMENT) - .timestamp(date.truncatedTo(ChronoUnit.SECONDS)) - .build(); - - PolicyAudit undeploy = PolicyAudit.builder() - .pdpGroup(group) - .pdpType("pdpType") - .policy(policy) - .action(AuditAction.UNDEPLOYMENT) - .timestamp(date.plusSeconds(1).truncatedTo(ChronoUnit.SECONDS)) - .build(); - // @formatter:on - - return List.of(deploy, undeploy); - } -} -- cgit 1.2.3-korg