Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@
),
@NamedQuery(name = CaseAuditEventEntity.FIND_CREATE_EVENT, query =
FIND_BY_CASE_DATA_ID_HQL
+ " AND cae.createdDate = "
+ "(select min(caeDate.createdDate) from CaseAuditEventEntity caeDate WHERE caeDate.caseDataId = :"
+ CaseAuditEventEntity.CASE_DATA_ID + ")"
+ " ORDER BY cae.createdDate ASC"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove subquery, and sort instead of using min

),
@NamedQuery(name = CaseAuditEventEntity.FIND_BY_ID, query =
FIND_BY_ID_HQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public Optional<AuditEvent> getCreateEvent(CaseDetails caseDetails) {
final Query query = em.createNamedQuery(CaseAuditEventEntity.FIND_CREATE_EVENT);

query.setParameter(CaseAuditEventEntity.CASE_DATA_ID, Long.valueOf(caseDetails.getId()));
query.setMaxResults(1);
List<CaseAuditEventEntity> auditEvents = query.getResultList();

return (auditEvents == null || auditEvents.size() == 0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package uk.gov.hmcts.ccd.data.casedetails.search;

import jakarta.persistence.Query;

public abstract class Criterion {

protected static final String TOKEN_SEPARATOR = ".";
Expand Down Expand Up @@ -29,6 +31,10 @@ public String buildParameterId() {

public abstract String buildClauseString(String operation);

public void bindParameters(Query query) {
query.setParameter(buildParameterId(), getSoughtValue());
}

protected String makeCaseInsensitive(String in) {
return "TRIM( UPPER ( " + in + "))";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.apache.commons.lang3.StringUtils.isNotBlank;

import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -48,14 +49,13 @@ private List<Criterion> buildFromMetaData(MetaData metadata) {
result.add(new MetaDataCriterion(CaseDetailsEntity.STATE_FIELD_COL, s)));

ifPresentAndNotBlank(metadata.getCreatedDate(), cd ->
result.add(new MetaDataCriterion("date(" + CaseDetailsEntity.CREATED_DATE_FIELD_COL + ")", cd)));
result.add(buildDateRangeCriterion(CaseDetailsEntity.CREATED_DATE_FIELD_COL, cd)));

ifPresentAndNotBlank(metadata.getLastModifiedDate(), lm ->
result.add(new MetaDataCriterion("date(" + CaseDetailsEntity.LAST_MODIFIED_FIELD_COL + ")", lm)));
result.add(buildDateRangeCriterion(CaseDetailsEntity.LAST_MODIFIED_FIELD_COL, lm)));

ifPresentAndNotBlank(metadata.getLastStateModifiedDate(), lsm ->
result.add(new MetaDataCriterion(
"date(" + CaseDetailsEntity.LAST_STATE_MODIFIED_DATE_FIELD_COL + ")", lsm)));
result.add(buildDateRangeCriterion(CaseDetailsEntity.LAST_STATE_MODIFIED_DATE_FIELD_COL, lsm)));

ifPresentAndNotBlank(metadata.getSecurityClassification(), sc ->
result.add(new MetaDataCriterion(
Expand All @@ -64,6 +64,10 @@ private List<Criterion> buildFromMetaData(MetaData metadata) {
return result;
}

private Criterion buildDateRangeCriterion(String field, String soughtValue) {
return new DateRangeMetaDataCriterion(field, LocalDate.parse(soughtValue));
}

private void ifPresentAndNotBlank(Optional<String> metadata, Consumer<String> metadataConsumer) {
metadata.ifPresent(m -> {
if (isNotBlank(m)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package uk.gov.hmcts.ccd.data.casedetails.search;

import java.time.LocalDate;

import jakarta.persistence.Query;

public class DateRangeMetaDataCriterion extends MetaDataCriterion {

private static final String FROM_SUFFIX = "_from";
private static final String TO_SUFFIX = "_to";

private final LocalDate soughtDate;

public DateRangeMetaDataCriterion(String field, LocalDate soughtDate) {
super(field, soughtDate.toString());
this.soughtDate = soughtDate;
}

@Override
public String buildClauseString(String operation) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replacing date(column) = ? with a half-open timestamp range keeps the predicate sargable, so Postgres can use the existing timestamp indexes.

String parameterId = buildParameterId();
return getField() + " >= " + PARAM_PREFIX + parameterId + FROM_SUFFIX
+ " AND " + getField() + " < " + PARAM_PREFIX + parameterId + TO_SUFFIX;
}

@Override
public void bindParameters(Query query) {
String parameterId = buildParameterId();
query.setParameter(parameterId + FROM_SUFFIX, soughtDate.atStartOfDay());
query.setParameter(parameterId + TO_SUFFIX, soughtDate.plusDays(1).atStartOfDay());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ public Optional<Query> build(MetaData metadata, Map<String, String> params, bool
return Optional.empty();
}

String sortClause = sortOrderQueryBuilder.buildSortOrderClause(metadata);
String queryToFormat = isCountQuery ? MAIN_COUNT_QUERY : MAIN_QUERY;
String queryString = String.format(queryToFormat, whereClausePart, sortClause);
String queryString = buildQueryString(isCountQuery, whereClausePart, metadata);

Query query;
if (isCountQuery) {
Expand Down Expand Up @@ -133,7 +131,16 @@ private String addUserCaseStateAccessClause(MetaData metadata, Map<String, Objec
}

private void addParameters(final Query query, List<Criterion> criteria) {
criteria.forEach(criterion -> query.setParameter(criterion.buildParameterId(), criterion.getSoughtValue()));
criteria.forEach(criterion -> criterion.bindParameters(query));
}

private String buildQueryString(boolean isCountQuery, String whereClausePart, MetaData metadata) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count queries no longer build an unused sort clause, avoiding unnecessary work

if (isCountQuery) {
return String.format(MAIN_COUNT_QUERY, whereClausePart);
}

String sortClause = sortOrderQueryBuilder.buildSortOrderClause(metadata);
return String.format(MAIN_QUERY, whereClausePart, sortClause);
}

private String toClauses(final List<Criterion> criteria) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ spring.datasource.hikari.connection-timeout=${DATA_STORE_DB_CONNECTION_TIMEOUT:4
spring.datasource.hikari.idle-timeout=${DATA_STORE_DB_IDLE_TIMEOUT:300000}
spring.datasource.hikari.minimum-idle=${DATA_STORE_DB_MIN_IDLE:8}
spring.datasource.hikari.maximum-pool-size=${DATA_STORE_DB_MAX_POOL_SIZE:16}
# WARNING: disabling Open Session in View means lazy JPA associations must be loaded/mapped inside service or
# repository transactions. Accessing lazy associations during response serialization can cause LazyInitializationException.
spring.jpa.open-in-view=false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slight;y dangerous so could remove and merge in a future pr, but good best practice change

# Disable feature detection to avoid the java.sql.SQLFeatureNotSupportedException
# Method org.postgresql.jdbc.PgConnection.createClob() is not yet implemented.
spring.jpa.properties.hibernate.temp.use_jdbc_metadata_defaults = false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package uk.gov.hmcts.ccd.data.casedetails.search;

import java.time.LocalDate;

import org.junit.jupiter.api.Test;

import static org.junit.Assert.assertTrue;
Expand All @@ -25,4 +27,12 @@ public void checkMetaDataCreationTestCriteraString() {
assertTrue(subject.buildClauseString("AND").contains(META_DATA_1));
}

@Test
public void checkDateRangeMetaDataCreationTestCriteraString() {
DateRangeMetaDataCriterion subject = new DateRangeMetaDataCriterion("created_date",
LocalDate.parse("2020-09-12"));
assertTrue(subject.buildClauseString("AND").contains("created_date >= :created_date_from"));
assertTrue(subject.buildClauseString("AND").contains("created_date < :created_date_to"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ public void checkLastStateModifiedDateMetaDataTest() {

List<Criterion> result = subject.build(metaData, params);
assertEquals(3, result.size());
assertThat(result).filteredOn(m -> m.getField().equals("date(last_state_modified_date)"))
assertThat(result).filteredOn(DateRangeMetaDataCriterion.class::isInstance)
.hasSize(1)
.extracting(Criterion::getField)
.containsOnly("last_state_modified_date");
assertThat(result).filteredOn(m -> m.getField().equals("last_state_modified_date"))
.hasSize(1)
.extracting(e -> e.getSoughtValue())
.containsOnly(LAST_STATE_MODIFIED_VALUE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import com.google.common.collect.Maps;
import java.io.IOException;
import java.time.LocalDateTime;
import java.util.List;
import jakarta.persistence.EntityManager;
import jakarta.persistence.Query;
import jakarta.persistence.TypedQuery;
import org.mockito.ArgumentCaptor;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
Expand All @@ -18,11 +21,13 @@
import uk.gov.hmcts.ccd.domain.service.security.AuthorisedCaseDefinitionDataService;
import uk.gov.hmcts.ccd.infrastructure.user.UserAuthorisation;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -87,8 +92,11 @@ void shouldUserCountWhenCountIsTrue() {

classUnderTest.build(metadata, Maps.newHashMap(), true);

verify(sortOrderQueryBuilder).buildSortOrderClause(metadata);
verify(entityManager).createNativeQuery(anyString());
ArgumentCaptor<String> queryCaptor = ArgumentCaptor.forClass(String.class);
verify(sortOrderQueryBuilder, never()).buildSortOrderClause(any(MetaData.class));
verify(entityManager).createNativeQuery(queryCaptor.capture());
assertThat(queryCaptor.getValue()).startsWith("SELECT count(*)");
assertThat(queryCaptor.getValue()).doesNotContain("ORDER BY");
}

@Test
Expand Down Expand Up @@ -139,4 +147,25 @@ void shouldNotCallRoleAssignmentServiceWhenRANotEnabled() {
verify(userAuthorisation).getUserId();
verify(entityManager).createNativeQuery(anyString(), any(Class.class));
}

@Test
void shouldBindDateMetadataAsTimestampRange() {
Query query = mock(Query.class);
when(applicationParam.getEnableAttributeBasedAccessControl()).thenReturn(false);
when(authorisedCaseDefinitionDataService.getUserAuthorisedCaseStateIds(anyString(), anyString(), any()))
.thenReturn(List.of("caseStateId_1"));
when(entityManager.createNativeQuery(anyString(), any(Class.class))).thenReturn(query);

MetaData metadata = new MetaData(META_DATA_0_VALUE, META_DATA_1_VALUE);
metadata.setCreatedDate(java.util.Optional.of("2020-09-12"));

classUnderTest.build(metadata, Maps.newHashMap(), false);

ArgumentCaptor<String> queryCaptor = ArgumentCaptor.forClass(String.class);
verify(entityManager).createNativeQuery(queryCaptor.capture(), any(Class.class));
assertThat(queryCaptor.getValue()).contains("created_date >= :created_date_from");
assertThat(queryCaptor.getValue()).contains("created_date < :created_date_to");
verify(query).setParameter("created_date_from", LocalDateTime.of(2020, 9, 12, 0, 0));
verify(query).setParameter("created_date_to", LocalDateTime.of(2020, 9, 13, 0, 0));
}
}