From 394959f1e9a9df44b10e2c785b53dd6c3da96e05 Mon Sep 17 00:00:00 2001 From: Vivek Singh Date: Thu, 26 Sep 2024 19:13:35 +0530 Subject: [PATCH] avniproject/avni-webapp#1317 - removed subject type join to simplify. simplified search query has lesser cost. --- Makefile | 1 + .../server/dao/SubjectSearchRepository.java | 14 ++- .../search/BaseSubjectSearchQueryBuilder.java | 9 +- .../avni/server/dao/search/SearchBuilder.java | 5 +- .../SubjectAssignmentSearchQueryBuilder.java | 4 +- .../dao/search/SubjectSearchQueryBuilder.java | 39 ++++++-- .../service/IndividualSearchService.java | 2 +- .../EncounterSearchQueryBuilderTest.java | 4 +- .../search/SubjectSearchQueryBuilderTest.java | 94 +++++++++++++------ .../domain/metadata/SubjectTypeBuilder.java | 5 + 10 files changed, 121 insertions(+), 56 deletions(-) diff --git a/Makefile b/Makefile index 0770eb4fc..0ead554e5 100644 --- a/Makefile +++ b/Makefile @@ -175,6 +175,7 @@ ci-test: open_test_results: open avni-server-api/build/reports/tests/test/index.html +open-test-results: open_test_results build-rpm: ./gradlew clean avni-server-api:buildRpm -x test --info --stacktrace diff --git a/avni-server-api/src/main/java/org/avni/server/dao/SubjectSearchRepository.java b/avni-server-api/src/main/java/org/avni/server/dao/SubjectSearchRepository.java index 306e26900..19993a203 100644 --- a/avni-server-api/src/main/java/org/avni/server/dao/SubjectSearchRepository.java +++ b/avni-server-api/src/main/java/org/avni/server/dao/SubjectSearchRepository.java @@ -2,12 +2,14 @@ import org.avni.server.dao.search.SearchBuilder; import org.avni.server.dao.search.SqlQuery; +import org.avni.server.domain.SubjectType; import org.avni.server.web.request.webapp.search.SubjectSearchRequest; import org.hibernate.query.internal.NativeQueryImpl; import org.hibernate.transform.AliasToEntityMapResultTransformer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Repository; +import org.springframework.util.StringUtils; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; @@ -20,18 +22,21 @@ @Repository public class SubjectSearchRepository extends RoleSwitchableRepository { private final Logger logger = LoggerFactory.getLogger(SubjectSearchRepository.class); + private final SubjectTypeRepository subjectTypeRepository; @PersistenceContext private EntityManager entityManager; - public SubjectSearchRepository(EntityManager entityManager) { + public SubjectSearchRepository(EntityManager entityManager, SubjectTypeRepository subjectTypeRepository) { super(entityManager); + this.subjectTypeRepository = subjectTypeRepository; } @Transactional - public List> search(SubjectSearchRequest searchRequest, SearchBuilder searchBuilder) { + public List> search(SubjectSearchRequest searchRequest, SearchBuilder searchBuilder) { + SubjectType subjectType = StringUtils.isEmpty(searchRequest.getSubjectType()) ? null : subjectTypeRepository.findByUuid(searchRequest.getSubjectType()); + SqlQuery query = searchBuilder.getSQLResultQuery(searchRequest, subjectType); try { - SqlQuery query = searchBuilder.getSQLResultQuery(searchRequest); setRoleToNone(); logger.debug("Executing query: " + query.getSql()); logger.debug("Parameters: " + query.getParameters()); @@ -49,8 +54,9 @@ public List> search(SubjectSearchRequest searchRequest, Searc @Transactional public BigInteger getTotalCount(SubjectSearchRequest searchRequest, SearchBuilder searchBuilder) { + SubjectType subjectType = StringUtils.isEmpty(searchRequest.getSubjectType()) ? null : subjectTypeRepository.findByUuid(searchRequest.getSubjectType()); + SqlQuery query = searchBuilder.getSQLCountQuery(searchRequest, subjectType); try { - SqlQuery query = searchBuilder.getSQLCountQuery(searchRequest); setRoleToNone(); Query sql = entityManager.createNativeQuery(query.getSql()); query.getParameters().forEach((name, value) -> { diff --git a/avni-server-api/src/main/java/org/avni/server/dao/search/BaseSubjectSearchQueryBuilder.java b/avni-server-api/src/main/java/org/avni/server/dao/search/BaseSubjectSearchQueryBuilder.java index cb9b88f86..2a6403a66 100644 --- a/avni-server-api/src/main/java/org/avni/server/dao/search/BaseSubjectSearchQueryBuilder.java +++ b/avni-server-api/src/main/java/org/avni/server/dao/search/BaseSubjectSearchQueryBuilder.java @@ -35,7 +35,7 @@ public class BaseSubjectSearchQueryBuilder { private static final String ADDRESS_LEVEL_JOIN = "left outer join address_level al on al.id = i.address_id"; private String orderByClause = ""; - private final Set whereClauses = new HashSet<>(); + protected final Set whereClauses = new HashSet<>(); private final Set joinClauses = new LinkedHashSet<>(); private final Map parameters = new HashMap<>(); private boolean forCount; @@ -193,13 +193,6 @@ public T withGenderFilter(List genders) { return (T) this; } - public T withSubjectTypeFilter(String subjectType) { - if (subjectType == null) return (T) this; - addParameter("subjectTypeUuid", subjectType); - whereClauses.add("st.uuid = :subjectTypeUuid"); - return (T) this; - } - public T withRegistrationDateFilter(DateRange registrationDateRange) { if (registrationDateRange == null || registrationDateRange.isEmpty()) return (T) this; diff --git a/avni-server-api/src/main/java/org/avni/server/dao/search/SearchBuilder.java b/avni-server-api/src/main/java/org/avni/server/dao/search/SearchBuilder.java index 56bfb3654..36843f298 100644 --- a/avni-server-api/src/main/java/org/avni/server/dao/search/SearchBuilder.java +++ b/avni-server-api/src/main/java/org/avni/server/dao/search/SearchBuilder.java @@ -1,9 +1,10 @@ package org.avni.server.dao.search; +import org.avni.server.domain.SubjectType; import org.avni.server.web.request.webapp.search.SubjectSearchRequest; public interface SearchBuilder { - SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest); + SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest, SubjectType subjectType); - SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest); + SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest, SubjectType subjectType); } diff --git a/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectAssignmentSearchQueryBuilder.java b/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectAssignmentSearchQueryBuilder.java index d392c3c62..1cbf1e5da 100644 --- a/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectAssignmentSearchQueryBuilder.java +++ b/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectAssignmentSearchQueryBuilder.java @@ -109,12 +109,12 @@ public SubjectAssignmentSearchQueryBuilder userGroupFilter(String groupUUID, Str } @Override - public SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest) { + public SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest, SubjectType subjectType) { return this.withSubjectSearchFilter(searchRequest).build(); } @Override - public SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest) { + public SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest, SubjectType subjectType) { return this.withSubjectSearchFilter(searchRequest).forCount().build(); } } diff --git a/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectSearchQueryBuilder.java b/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectSearchQueryBuilder.java index c1cc89646..897430f9b 100644 --- a/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectSearchQueryBuilder.java +++ b/avni-server-api/src/main/java/org/avni/server/dao/search/SubjectSearchQueryBuilder.java @@ -1,11 +1,17 @@ package org.avni.server.dao.search; +import org.avni.server.domain.SubjectType; import org.avni.server.web.request.webapp.search.DateRange; import org.avni.server.web.request.webapp.search.SubjectSearchRequest; +import org.springframework.stereotype.Component; +@Component public class SubjectSearchQueryBuilder extends BaseSubjectSearchQueryBuilder implements SearchBuilder { + public static final String SubjectTypeColumn = " st.name as \"subjectTypeName\",\n"; + public static final String HardCodedSubjectTypeColumn = " '$SubjectTypeName' as \"subjectTypeName\",\n"; + public static final String SubjectTypeJoin = " left outer join subject_type st on i.subject_type_id = st.id and st.is_voided is false\n"; - public SqlQuery build() { + public SqlQuery build(SubjectType subjectType) { String baseQuery = "select $DISTINCT i.id as \"id\",\n" + " i.first_name as \"firstName\",\n" + " i.last_name as \"lastName\",\n" + @@ -13,20 +19,28 @@ public SqlQuery build() { " cast(concat_ws(' ',i.first_name,i.middle_name,i.last_name)as text) as \"fullName\",\n" + " i.uuid as \"uuid\",\n" + " i.address_id as \"addressId\",\n" + - " st.name as \"subjectTypeName\",\n" + + "$SubjectTypeColumn" + " gender.name as \"gender\",\n" + " i.date_of_birth as \"dateOfBirth\" $CUSTOM_FIELDS\n" + "from individual i\n" + " left outer join gender on i.gender_id = gender.id\n" + - " left outer join subject_type st on i.subject_type_id = st.id and st.is_voided is false\n"; + "$SubjectTypeJoin"; + + if (subjectType == null) { + baseQuery = baseQuery.replace("$SubjectTypeColumn", SubjectTypeColumn); + baseQuery = baseQuery.replace("$SubjectTypeJoin", SubjectTypeJoin); + } else { + baseQuery = baseQuery.replace("$SubjectTypeColumn", HardCodedSubjectTypeColumn.replace("$SubjectTypeName", subjectType.getName())); + baseQuery = baseQuery.replace("$SubjectTypeJoin", ""); + } return super.buildUsingBaseQuery(baseQuery, ""); } - public SubjectSearchQueryBuilder withSubjectSearchFilter(SubjectSearchRequest request) { + public SubjectSearchQueryBuilder withSubjectSearchFilter(SubjectSearchRequest request, SubjectType subjectType) { return this .withNameFilter(request.getName()) + .withSubjectTypeFilter(subjectType) .withGenderFilter(request.getGender()) - .withSubjectTypeFilter(request.getSubjectType()) .withAgeFilter(request.getAge()) .withRegistrationDateFilter(request.getRegistrationDate()) .withEncounterDateFilter(request.getEncounterDate()) @@ -40,6 +54,13 @@ public SubjectSearchQueryBuilder withSubjectSearchFilter(SubjectSearchRequest re .withCustomFields(request.getSubjectType()); } + public SubjectSearchQueryBuilder withSubjectTypeFilter(SubjectType subjectType) { + if (subjectType == null) return this; + whereClauses.add("i.subject_type_id = :subjectTypeId"); + addParameter("subjectTypeId", subjectType.getId()); + return this; + } + public SubjectSearchQueryBuilder withEncounterDateFilter(DateRange encounterDateRange) { if (encounterDateRange == null || encounterDateRange.isEmpty()) return this; return withJoin(ENCOUNTER_JOIN, true) @@ -69,12 +90,12 @@ public SubjectSearchQueryBuilder withProgramEnrolmentDateFilter(DateRange dateRa } @Override - public SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest) { - return this.withSubjectSearchFilter(searchRequest).build(); + public SqlQuery getSQLResultQuery(SubjectSearchRequest searchRequest, SubjectType subjectType) { + return this.withSubjectSearchFilter(searchRequest, subjectType).build(subjectType); } @Override - public SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest) { - return this.withSubjectSearchFilter(searchRequest).forCount().build(); + public SqlQuery getSQLCountQuery(SubjectSearchRequest searchRequest, SubjectType subjectType) { + return this.withSubjectSearchFilter(searchRequest, subjectType).forCount().build(subjectType); } } diff --git a/avni-server-api/src/main/java/org/avni/server/service/IndividualSearchService.java b/avni-server-api/src/main/java/org/avni/server/service/IndividualSearchService.java index aa3f18bd3..58edcd2bc 100644 --- a/avni-server-api/src/main/java/org/avni/server/service/IndividualSearchService.java +++ b/avni-server-api/src/main/java/org/avni/server/service/IndividualSearchService.java @@ -40,7 +40,7 @@ private LinkedHashMap constructIndividual(List ((BigInteger) individualRecord.get("addressId")).longValue()) .collect(Collectors.toList()); - List searchSubjectEnrolledPrograms = individualIds.size() > 0 ? + List searchSubjectEnrolledPrograms = !individualIds.isEmpty() ? programEnrolmentRepository.findActiveEnrolmentsByIndividualIds(individualIds) : Collections.emptyList(); diff --git a/avni-server-api/src/test/java/org/avni/server/dao/search/EncounterSearchQueryBuilderTest.java b/avni-server-api/src/test/java/org/avni/server/dao/search/EncounterSearchQueryBuilderTest.java index 0685d04c2..6a02e8555 100644 --- a/avni-server-api/src/test/java/org/avni/server/dao/search/EncounterSearchQueryBuilderTest.java +++ b/avni-server-api/src/test/java/org/avni/server/dao/search/EncounterSearchQueryBuilderTest.java @@ -1,7 +1,5 @@ package org.avni.server.dao.search; -import org.avni.server.dao.search.EncounterSearchQueryBuilder; -import org.avni.server.dao.search.SqlQuery; import org.avni.server.domain.Organisation; import org.avni.server.domain.UserContext; import org.avni.server.framework.security.UserContextHolder; @@ -13,7 +11,7 @@ import java.util.Date; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.is; public class EncounterSearchQueryBuilderTest { diff --git a/avni-server-api/src/test/java/org/avni/server/dao/search/SubjectSearchQueryBuilderTest.java b/avni-server-api/src/test/java/org/avni/server/dao/search/SubjectSearchQueryBuilderTest.java index bf35e66e3..61a5b96eb 100644 --- a/avni-server-api/src/test/java/org/avni/server/dao/search/SubjectSearchQueryBuilderTest.java +++ b/avni-server-api/src/test/java/org/avni/server/dao/search/SubjectSearchQueryBuilderTest.java @@ -1,11 +1,15 @@ package org.avni.server.dao.search; -import org.avni.server.dao.search.SqlQuery; -import org.avni.server.dao.search.SubjectSearchQueryBuilder; -import org.junit.Test; +import org.avni.server.domain.Organisation; +import org.avni.server.domain.SubjectType; +import org.avni.server.domain.UserContext; +import org.avni.server.domain.metadata.SubjectTypeBuilder; +import org.avni.server.framework.security.UserContextHolder; import org.avni.server.web.request.webapp.search.Concept; import org.avni.server.web.request.webapp.search.DateRange; import org.avni.server.web.request.webapp.search.IntegerRange; +import org.junit.Before; +import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; @@ -13,97 +17,133 @@ import static org.assertj.core.api.Assertions.assertThat; public class SubjectSearchQueryBuilderTest { + private SubjectType subjectType; + + @Before + public void setup() { + UserContext userContext = new UserContext(); + userContext.setOrganisation(new Organisation()); + UserContextHolder.create(userContext); + subjectType = new SubjectTypeBuilder().setId(1l).setName("Individual").build(); + } + + @Test + public void withoutSubjectType() { + SqlQuery query = new SubjectSearchQueryBuilder() + .withNameFilter("name") + .build(null); + String sql = query.getSql(); + assertThat(sql).isNotEmpty(); + assertThat(sql).contains(SubjectSearchQueryBuilder.SubjectTypeColumn); + assertThat(sql).contains(SubjectSearchQueryBuilder.SubjectTypeJoin); + } @Test public void shouldBuildBaseQueryWhenRunWithoutParameters() { - SqlQuery query = new SubjectSearchQueryBuilder().build(); - System.out.println(query.getSql()); - assertThat(query.getSql()).isNotEmpty(); + SqlQuery query = new SubjectSearchQueryBuilder().build(subjectType); + String sql = query.getSql(); + assertThat(sql).isNotEmpty(); } @Test public void shouldBeAbleToSearchByName() { SqlQuery query = new SubjectSearchQueryBuilder() .withNameFilter("name") - .build(); - assertThat(query.getSql()).isNotEmpty(); + .withSubjectTypeFilter(subjectType) + .build(subjectType); + String sql = query.getSql(); + System.out.println(sql); + assertThat(sql).isNotEmpty(); + assertThat(sql).contains("i.subject_type_id = :subjectTypeId"); + assertThat(sql).contains("'Individual' as \"subjectTypeName\""); + assertThat(sql).doesNotContain(SubjectSearchQueryBuilder.SubjectTypeColumn); + assertThat(sql).doesNotContain(SubjectSearchQueryBuilder.SubjectTypeJoin); } @Test public void shouldNotAddNameFiltersForNullOrEmptyNames() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withNameFilter(" ") - .build(); + .build(subjectType); assertThat(query.getSql().contains("i.last_name ilike")).isFalse(); query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withNameFilter(null) - .build(); + .build(subjectType); assertThat(query.getSql().contains("i.last_name ilike")).isFalse(); } @Test public void shouldBreakNameStringIntoTokensInTheQuery() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withNameFilter("two tokens andAnother") - .build(); - assertThat(query.getParameters().values().contains("%two%")).isTrue(); - assertThat(query.getParameters().values().contains("%tokens%")).isTrue(); - assertThat(query.getParameters().values().contains("%andAnother%")).isTrue(); - assertThat(query.getParameters().size()).isEqualTo(5); + .build(subjectType); + assertThat(query.getParameters().containsValue("%two%")).isTrue(); + assertThat(query.getParameters().containsValue("%tokens%")).isTrue(); + assertThat(query.getParameters().containsValue("%andAnother%")).isTrue(); + assertThat(query.getParameters().size()).isEqualTo(6); } @Test public void shouldAddAgeFilter() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withAgeFilter(new IntegerRange(1, null)) - .build(); - assertThat(query.getParameters().size()).isEqualTo(3); + .build(subjectType); + assertThat(query.getParameters().size()).isEqualTo(4); } @Test public void shouldAddGenderFilter() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withGenderFilter(null) - .build(); - assertThat(query.getParameters().size()).isEqualTo(2); + .build(subjectType); + assertThat(query.getParameters().size()).isEqualTo(3); ArrayList genders = new ArrayList<>(); genders.add("firstGenderUuid"); query = new SubjectSearchQueryBuilder() .withGenderFilter(genders) - .build(); + .build(subjectType); assertThat(query.getParameters().size()).isEqualTo(3); } @Test public void shouldAddEncounterJoinWhtnAddingEncounterDateFilter() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withEncounterDateFilter(new DateRange("2021-01-01", "2022-01-01")) - .build(); - assertThat(query.getParameters().size()).isEqualTo(4); + .build(subjectType); + assertThat(query.getParameters().size()).isEqualTo(5); } @Test public void shouldNotAddTheSameJoinsMultipleTimes() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withProgramEncounterDateFilter(new DateRange("2021-01-01", "2022-01-01")) .withProgramEnrolmentDateFilter(new DateRange("2021-01-01", "2022-01-01")) - .build(); - assertThat(query.getParameters().size()).isEqualTo(6); + .build(subjectType); + assertThat(query.getParameters().size()).isEqualTo(7); } @Test public void shouldAddConditionsForConcepts() { SqlQuery query = new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) .withConceptsFilter(Arrays.asList( new Concept[]{new Concept("uuid", "registration", "CODED", Arrays.asList(new String[]{"asdf", "qwer"}), null)})) - .build(); + .build(subjectType); } @Test public void shouldMakeQueryForCount() { - SqlQuery query = new SubjectSearchQueryBuilder().forCount() - .build(); + new SubjectSearchQueryBuilder() + .withSubjectTypeFilter(subjectType) + .forCount().build(subjectType); } } diff --git a/avni-server-api/src/test/java/org/avni/server/domain/metadata/SubjectTypeBuilder.java b/avni-server-api/src/test/java/org/avni/server/domain/metadata/SubjectTypeBuilder.java index 9a0402b8b..24501e59c 100644 --- a/avni-server-api/src/test/java/org/avni/server/domain/metadata/SubjectTypeBuilder.java +++ b/avni-server-api/src/test/java/org/avni/server/domain/metadata/SubjectTypeBuilder.java @@ -31,6 +31,11 @@ public SubjectTypeBuilder setName(String name) { return this; } + public SubjectTypeBuilder setId(Long id) { + subjectType.setId(id); + return this; + } + public SubjectTypeBuilder setMandatoryFieldsForNewEntity() { String s = UUID.randomUUID().toString(); return setName(s).setUuid(s).setShouldSyncByLocation(true).setType(Subject.Individual).setActive(true);