From: Jim Hahn Date: Tue, 15 Jun 2021 18:33:56 +0000 (-0400) Subject: Replace method parameters with filter parameters X-Git-Tag: 2.5.0~38 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=policy%2Fmodels.git;a=commitdiff_plain;h=27ac6bce15318a074d4fb53606571eb30e46bf07 Replace method parameters with filter parameters Added more methods taking filter parameters instead of individual method parameters. Removed the old methods. Added genOrderClause(). Issue-ID: POLICY-3094 Change-Id: Ie9d8c13a8d4a7f81e284f07fce3d96a35918a114 Signed-off-by: Jim Hahn --- diff --git a/models-dao/src/main/java/org/onap/policy/models/dao/PfFilter.java b/models-dao/src/main/java/org/onap/policy/models/dao/PfFilter.java index e00e1d6bf..7afe243a7 100644 --- a/models-dao/src/main/java/org/onap/policy/models/dao/PfFilter.java +++ b/models-dao/src/main/java/org/onap/policy/models/dao/PfFilter.java @@ -26,9 +26,9 @@ import java.util.Map; import javax.persistence.TypedQuery; import lombok.Data; import org.onap.policy.models.base.PfConcept; + /** * This abstract class is used as a base for the filter implementations. - * */ @Data @@ -48,37 +48,48 @@ public abstract class PfFilter { * Generates the "WHERE" (and "ORDER BY") clause for a JPA query. */ public String genWhereClause(PfFilterParametersIntfc parameters) { - var filterQueryString = new StringBuilder(WHERE); + if (parameters == null) { + return ""; + } + + var builder = new ClauseBuilder(WHERE, AND); + if (parameters.getFilterMap() != null) { for (String key : parameters.getFilterMap().keySet()) { - filterQueryString.append(getKeyPrefix() + key + "= :" + key + AND); + builder.addCondition(getKeyPrefix(), key, "= :", key); } } if (parameters.getName() != null) { - filterQueryString.append(getNameFilter() + AND); + builder.addCondition(getNameFilter()); } if (parameters.getStartTime() != null) { - if (parameters.getEndTime() != null) { - filterQueryString.append(getTimeStampStartFilter()); - filterQueryString.append(AND); - filterQueryString.append(getTimeStampEndFilter()); - } else { - filterQueryString.append(getTimeStampStartFilter()); - } - } else { - if (parameters.getEndTime() != null) { - filterQueryString.append(getTimeStampEndFilter()); - } else { - filterQueryString.delete(filterQueryString.length() - AND.length(), filterQueryString.length()); - } + builder.addCondition(getTimeStampStartFilter()); + } + + if (parameters.getEndTime() != null) { + builder.addCondition(getTimeStampEndFilter()); + } + + return builder.toString(); + } + + /** + * Generates the "ORDER BY" clause for a JPA query. + */ + public String genOrderClause(PfFilterParametersIntfc parameters) { + if (parameters == null) { + return ""; } + var builder = new ClauseBuilder(ORDER, ", "); + if (parameters.getRecordNum() > 0) { - filterQueryString.append(ORDER + getTimeStampFilter() + parameters.getSortOrder()); + builder.addCondition(getTimeStampFilter(), parameters.getSortOrder()); } - return filterQueryString.toString(); + + return builder.toString(); } /** @@ -86,6 +97,9 @@ public abstract class PfFilter { * @param query query to populate */ public void setParams(TypedQuery query, PfFilterParametersIntfc parameters) { + if (parameters == null) { + return; + } if (parameters.getFilterMap() != null) { for (Map.Entry entry : parameters.getFilterMap().entrySet()) { @@ -96,19 +110,39 @@ public abstract class PfFilter { query.setParameter(this.getNameParameter(), parameters.getName()); } if (parameters.getStartTime() != null) { - if (parameters.getEndTime() != null) { - query.setParameter("startTime", Timestamp.from(parameters.getStartTime())); - query.setParameter("endTime", Timestamp.from(parameters.getEndTime())); - } else { - query.setParameter("startTime", Timestamp.from(parameters.getStartTime())); - } - } else { - if (parameters.getEndTime() != null) { - query.setParameter("endTime", Timestamp.from(parameters.getEndTime())); - } + query.setParameter("startTime", Timestamp.from(parameters.getStartTime())); + } + if (parameters.getEndTime() != null) { + query.setParameter("endTime", Timestamp.from(parameters.getEndTime())); } if (parameters.getRecordNum() > 0) { query.setMaxResults(parameters.getRecordNum()); } } + + private static class ClauseBuilder { + private final StringBuilder builder = new StringBuilder(); + private final String separator; + + private String currentSeparator; + + public ClauseBuilder(String clause, String separator) { + this.separator = separator; + this.currentSeparator = clause; + } + + @Override + public String toString() { + return builder.toString(); + } + + public void addCondition(String...condition) { + builder.append(currentSeparator); + currentSeparator = separator; + + for (String text: condition) { + builder.append(text); + } + } + } } diff --git a/models-dao/src/main/java/org/onap/policy/models/dao/impl/DefaultPfDao.java b/models-dao/src/main/java/org/onap/policy/models/dao/impl/DefaultPfDao.java index 95a5bf3fa..16680d800 100644 --- a/models-dao/src/main/java/org/onap/policy/models/dao/impl/DefaultPfDao.java +++ b/models-dao/src/main/java/org/onap/policy/models/dao/impl/DefaultPfDao.java @@ -393,7 +393,9 @@ public class DefaultPfDao implements PfDao { try { PfFilter filter = new PfFilterFactory().createFilter(someClass); - String filterQueryString = SELECT_FROM_TABLE + filter.genWhereClause(filterParams); + String filterQueryString = SELECT_FROM_TABLE + + filter.genWhereClause(filterParams) + + filter.genOrderClause(filterParams); TypedQuery query = mg.createQuery(setQueryTable(filterQueryString, someClass), someClass); filter.setParams(query, filterParams); diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java index 5321e8597..8782a6928 100644 --- a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java +++ b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java @@ -35,6 +35,7 @@ import org.onap.policy.models.pdp.concepts.PdpGroupFilter; import org.onap.policy.models.pdp.concepts.PdpPolicyStatus; import org.onap.policy.models.pdp.concepts.PdpStatistics; import org.onap.policy.models.pdp.concepts.PdpSubGroup; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifierOptVersion; import org.onap.policy.models.tosca.authorative.concepts.ToscaEntityFilter; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; @@ -332,18 +333,11 @@ public interface PolicyModelsProvider extends AutoCloseable { /** * Get filtered PdpStatistics. * - * @param name the pdpInstance name for the PDP statistics to get - * @param pdpGroupName pdpGroupName to filter statistics - * @param pdpSubGroup pdpSubGroupType name to filter statistics - * @param startTimeStamp startTimeStamp to filter statistics - * @param endTimeStamp endTimeStamp to filter statistics - * @param sortOrder sortOrder to query database - * @param getRecordNum Total query count from database + * @param filterParams filter parameters * @return the PDP statistics found * @throws PfModelException on errors getting policies */ - public List getFilteredPdpStatistics(String name, @NonNull String pdpGroupName, String pdpSubGroup, - Instant startTimeStamp, Instant endTimeStamp, String sortOrder, int getRecordNum) throws PfModelException; + public List getFilteredPdpStatistics(PdpFilterParameters filterParams) throws PfModelException; /** * Creates PDP statistics. @@ -421,8 +415,7 @@ public interface PolicyModelsProvider extends AutoCloseable { /** * Collect the audit records. * @param auditFilter filter for search - * @param numRecords max number of records to be collected * @return list of {@link PolicyAudit} or empty if none or not match with filter */ - public List getAuditRecords(AuditFilter auditFilter, @NonNull Integer numRecords); + public List getAuditRecords(AuditFilter auditFilter); } diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java index 3bba1518a..95300e62d 100644 --- a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java +++ b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java @@ -38,6 +38,7 @@ import org.onap.policy.models.pdp.concepts.PdpGroupFilter; import org.onap.policy.models.pdp.concepts.PdpPolicyStatus; import org.onap.policy.models.pdp.concepts.PdpStatistics; import org.onap.policy.models.pdp.concepts.PdpSubGroup; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.pdp.persistence.provider.PdpProvider; import org.onap.policy.models.pdp.persistence.provider.PdpStatisticsProvider; import org.onap.policy.models.provider.PolicyModelsProvider; @@ -256,13 +257,9 @@ public class DatabasePolicyModelsProviderImpl extends AbstractModelsProvider imp } @Override - public List getFilteredPdpStatistics(final String name, @NonNull final String pdpGroupName, - final String pdpSubGroup, final Instant startTimeStamp, - final Instant endTimeStamp, final String sortOrder, - final int getRecordNum) throws PfModelException { + public List getFilteredPdpStatistics(PdpFilterParameters filterParams) throws PfModelException { assertInitialized(); - return new PdpStatisticsProvider().getFilteredPdpStatistics(getPfDao(), name, pdpGroupName, pdpSubGroup, - startTimeStamp, endTimeStamp, sortOrder, getRecordNum); + return new PdpStatisticsProvider().getFilteredPdpStatistics(getPfDao(), filterParams); } @Override @@ -319,13 +316,9 @@ public class DatabasePolicyModelsProviderImpl extends AbstractModelsProvider imp } @Override - public List getAuditRecords(AuditFilter auditFilter, @NonNull Integer numRecords) { + public List getAuditRecords(AuditFilter auditFilter) { assertInitialized(); - if (auditFilter == null || auditFilter.isEmpty()) { - return new PolicyAuditProvider().getAuditRecords(getPfDao(), numRecords); - } else { - return new PolicyAuditProvider().getAuditRecords(getPfDao(), auditFilter, numRecords); - } + return new PolicyAuditProvider().getAuditRecords(getPfDao(), auditFilter); } /** diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderImpl.java b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderImpl.java index dbeccfaeb..e705ae3ed 100644 --- a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderImpl.java +++ b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderImpl.java @@ -40,6 +40,7 @@ import org.onap.policy.models.pdp.concepts.PdpGroupFilter; import org.onap.policy.models.pdp.concepts.PdpPolicyStatus; import org.onap.policy.models.pdp.concepts.PdpStatistics; import org.onap.policy.models.pdp.concepts.PdpSubGroup; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.provider.PolicyModelsProviderParameters; import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifierOptVersion; @@ -218,8 +219,7 @@ public class DummyPolicyModelsProviderImpl implements PolicyModelsProvider { } @Override - public List getFilteredPdpStatistics(String name, String pdpGroupName, String pdpSubGroup, - Instant startTimeStamp, Instant endTimeStamp, String sortOrder, int getRecordNum) { + public List getFilteredPdpStatistics(PdpFilterParameters filterParams) throws PfModelException { // Not implemented return new ArrayList<>(); } @@ -275,7 +275,7 @@ public class DummyPolicyModelsProviderImpl implements PolicyModelsProvider { } @Override - public List getAuditRecords(AuditFilter auditFilter, @NonNull Integer numRecords) { + public List getAuditRecords(AuditFilter auditFilter) { // Not implemented return new ArrayList<>(); } diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java index 4356e4b1d..de7b28a60 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java @@ -46,6 +46,7 @@ import org.onap.policy.models.pdp.concepts.PdpStatistics; import org.onap.policy.models.pdp.concepts.PdpSubGroup; import org.onap.policy.models.pdp.enums.PdpHealthStatus; import org.onap.policy.models.pdp.enums.PdpState; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.provider.PolicyModelsProviderFactory; import org.onap.policy.models.provider.PolicyModelsProviderParameters; @@ -277,10 +278,6 @@ public class DatabasePolicyModelsProviderTest { databaseProvider.deletePdpGroup(null); }).hasMessageMatching(NAME_IS_NULL); - assertThatThrownBy(() -> { - databaseProvider.getFilteredPdpStatistics(NAME, null, "sub", TIMESTAMP, TIMESTAMP, ORDER, 0); - }).hasMessageMatching(GROUP_IS_NULL); - assertThatThrownBy(() -> { databaseProvider.createPdpStatistics(null); }).hasMessageMatching("^pdpStatisticsList is marked .*on.*ull but is null$"); @@ -426,34 +423,41 @@ public class DatabasePolicyModelsProviderTest { databaseProvider.createPdpStatistics(makePdpStatisticsList()); assertEquals(NAME, databaseProvider.getPdpStatistics(null, null).get(0).getPdpInstanceId()); - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(null, GROUP, null, - null, null, ORDER, 0).get(0).getPdpInstanceId()); - assertEquals(0, databaseProvider.getFilteredPdpStatistics(null, GROUP, null, - Instant.now(), null, ORDER, 0).size()); - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(null, GROUP, null, - null, TIMESTAMP, ORDER, 0).get(0).getPdpInstanceId()); - assertEquals(0, - databaseProvider.getFilteredPdpStatistics(null, GROUP, null, Instant.now(), - Instant.now(), ORDER, 0).size()); - - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, null, null, - null, ORDER, 0).get(0).getPdpInstanceId()); - assertEquals(0, - databaseProvider.getFilteredPdpStatistics(NAME, GROUP, null, Instant.now(), Instant.now(), - ORDER, 0).size()); - - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, "type", - null, null, ORDER, 0).get(0).getPdpInstanceId()); - - assertEquals(0, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, "type", - Instant.now(), Instant.now(), ORDER, 0).size()); - - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, "type", - null, null, ORDER, 1).get(0).getPdpInstanceId()); - assertEquals(NAME, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, "type", - null, null, ORDER, 5).get(0).getPdpInstanceId()); - assertEquals(0, databaseProvider.getFilteredPdpStatistics(NAME, GROUP, "type", - Instant.now(), Instant.now(), ORDER, 5).size()); + assertEquals(NAME, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().group(GROUP).build()).get(0).getPdpInstanceId()); + assertEquals(0, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().group(GROUP).startTime(Instant.now()).build()).size()); + assertEquals(NAME, databaseProvider + .getFilteredPdpStatistics(PdpFilterParameters.builder().group(GROUP).endTime(TIMESTAMP).build()) + .get(0).getPdpInstanceId()); + assertEquals(0, databaseProvider.getFilteredPdpStatistics(PdpFilterParameters.builder().group(GROUP) + .startTime(Instant.now()).endTime(Instant.now()).build()).size()); + + assertEquals(NAME, databaseProvider + .getFilteredPdpStatistics(PdpFilterParameters.builder().name(NAME).group(GROUP).build()).get(0) + .getPdpInstanceId()); + assertEquals(0, databaseProvider.getFilteredPdpStatistics(PdpFilterParameters.builder().name(NAME).group(GROUP) + .startTime(Instant.now()).endTime(Instant.now()).build()).size()); + + assertEquals(NAME, + databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name(NAME).group(GROUP).subGroup("type").build()) + .get(0).getPdpInstanceId()); + + assertEquals(0, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name(NAME).group(GROUP).subGroup("type") + .startTime(Instant.now()).endTime(Instant.now()).build()).size()); + + assertEquals(NAME, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name(NAME).group(GROUP).subGroup("type") + .sortOrder(ORDER).recordNum(1).build()).get(0).getPdpInstanceId()); + assertEquals(NAME, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name(NAME).group(GROUP).subGroup("type") + .sortOrder(ORDER).recordNum(5).build()).get(0).getPdpInstanceId()); + assertEquals(0, databaseProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name(NAME).group(GROUP).subGroup("type") + .startTime(Instant.now()).endTime(Instant.now()) + .sortOrder(ORDER).recordNum(5).build()).size()); assertEquals(NAME, databaseProvider.deletePdpStatistics(NAME, null).get(0).getPdpInstanceId()); assertEquals(0, databaseProvider.getPdpStatistics(null, null).size()); @@ -549,22 +553,16 @@ public class DatabasePolicyModelsProviderTest { databaseProvider = new PolicyModelsProviderFactory().createPolicyModelsProvider(parameters); databaseProvider.createAuditRecords(List.of(audit)); - List createdAudits = databaseProvider.getAuditRecords(null, 10); + List createdAudits = databaseProvider.getAuditRecords(AuditFilter.builder().recordNum(10).build()); assertThat(createdAudits).hasSize(1); - createdAudits = databaseProvider.getAuditRecords(AuditFilter.builder().build(), 10); - assertThat(createdAudits).hasSize(1); - - List emptyList = - databaseProvider.getAuditRecords(AuditFilter.builder().action(AuditAction.UNDEPLOYMENT).build(), 10); + List emptyList = databaseProvider + .getAuditRecords(AuditFilter.builder().action(AuditAction.UNDEPLOYMENT).recordNum(10).build()); assertThat(emptyList).isEmpty(); assertThatThrownBy(() -> databaseProvider.createAuditRecords(null)) .hasMessageContaining("audits is marked non-null but is null"); - assertThatThrownBy(() -> databaseProvider.getAuditRecords(null, null)) - .hasMessageContaining("numRecords is marked non-null but is null"); - databaseProvider.close(); } diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java index 34cdbff0f..224ef0e85 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java @@ -39,6 +39,7 @@ import org.onap.policy.models.pdp.concepts.PdpGroupFilter; import org.onap.policy.models.pdp.concepts.PdpPolicyStatus; import org.onap.policy.models.pdp.concepts.PdpStatistics; import org.onap.policy.models.pdp.concepts.PdpSubGroup; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifierOptVersion; import org.onap.policy.models.tosca.authorative.concepts.ToscaEntityFilter; @@ -210,10 +211,9 @@ public class DummyBadProviderImpl implements PolicyModelsProvider { } @Override - public List getFilteredPdpStatistics(String name, String pdpGroupName, String pdpSubGroup, - Instant startTimeStamp, Instant endTimeStamp, String sortOrder, int getRecordNum) { + public List getFilteredPdpStatistics(PdpFilterParameters filterParams) throws PfModelException { // Not implemented - return new ArrayList<>(); + return null; } @Override @@ -273,7 +273,7 @@ public class DummyBadProviderImpl implements PolicyModelsProvider { } @Override - public List getAuditRecords(AuditFilter auditFilter, @NonNull Integer numRecords) { + public List getAuditRecords(AuditFilter auditFilter) { // Not implemented return null; } diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderTest.java index de320724a..d7c69bbb2 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyPolicyModelsProviderTest.java @@ -34,9 +34,11 @@ import java.time.Instant; import java.util.ArrayList; import org.junit.Test; import org.onap.policy.models.base.PfModelException; +import org.onap.policy.models.pap.persistence.provider.PolicyAuditProvider.AuditFilter; import org.onap.policy.models.pdp.concepts.Pdp; import org.onap.policy.models.pdp.concepts.PdpGroupFilter; import org.onap.policy.models.pdp.concepts.PdpSubGroup; +import org.onap.policy.models.pdp.persistence.provider.PdpFilterParameters; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.provider.PolicyModelsProviderFactory; import org.onap.policy.models.provider.PolicyModelsProviderParameters; @@ -114,8 +116,9 @@ public class DummyPolicyModelsProviderTest { assertTrue(dummyProvider.getPdpStatistics("name", null).isEmpty()); assertTrue( - dummyProvider.getFilteredPdpStatistics("name", null, null, - Instant.now(), Instant.now(), null, 0).isEmpty()); + dummyProvider.getFilteredPdpStatistics( + PdpFilterParameters.builder().name("name") + .startTime(Instant.now()).endTime(Instant.now()).build()).isEmpty()); assertTrue(dummyProvider.createPdpStatistics(null).isEmpty()); assertTrue(dummyProvider.updatePdpStatistics(null).isEmpty()); assertTrue(dummyProvider.deletePdpStatistics(null, Instant.now()).isEmpty()); @@ -127,7 +130,7 @@ public class DummyPolicyModelsProviderTest { assertThatCode(() -> dummyProvider.cudPolicyStatus(null, null, null)).doesNotThrowAnyException(); assertThatCode(() -> dummyProvider.createAuditRecords(null)).doesNotThrowAnyException(); - assertThat(dummyProvider.getAuditRecords(null, 10)).isEmpty(); + assertThat(dummyProvider.getAuditRecords(AuditFilter.builder().recordNum(10).build())).isEmpty(); } @Test