From f5951f1a7b98838599ba90065d0e5e91fb92bca9 Mon Sep 17 00:00:00 2001 From: Saifulhuq <90808980+Saifulhuq01@users.noreply.github.com> Date: Fri, 6 Feb 2026 15:09:40 +0530 Subject: [PATCH] FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements (#5417) * FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements * FINERACT-2461: Add unit tests for EmailReadPlatformServiceImpl --- .../service/EmailReadPlatformServiceImpl.java | 36 ++-- .../EmailReadPlatformServiceImplTest.java | 157 ++++++++++++++++++ 2 files changed, 174 insertions(+), 19 deletions(-) create mode 100644 fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImplTest.java diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java index f17fdf8ce..955ca561f 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java @@ -21,6 +21,7 @@ package org.apache.fineract.infrastructure.campaigns.email.service; import java.sql.ResultSet; import java.sql.SQLException; import java.time.LocalDate; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import lombok.RequiredArgsConstructor; @@ -105,9 +106,7 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { @Override public Collection retrieveAll() { - final String sql = "select " + this.emailRowMapper.schema(); - return this.jdbcTemplate.query(sql, this.emailRowMapper); // NOSONAR } @@ -115,7 +114,6 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { public EmailData retrieveOne(final Long resourceId) { try { final String sql = "select " + this.emailRowMapper.schema() + " where emo.id = ?"; - return this.jdbcTemplate.queryForObject(sql, this.emailRowMapper, new Object[] { resourceId }); // NOSONAR } catch (final EmptyResultDataAccessException e) { throw new EmailNotFoundException(resourceId, e); @@ -124,18 +122,12 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { @Override public Collection retrieveAllPending(final SearchParameters searchParameters) { - final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : ""; - final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum =? " + sqlPlusLimit; - - return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.PENDING.getValue()); // NOSONAR + return retrieveEmailByStatus(EmailMessageStatusType.PENDING.getValue(), searchParameters.getLimit()); } @Override public Collection retrieveAllSent(final SearchParameters searchParameters) { - final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : ""; - final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - - return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.SENT.getValue()); // NOSONAR + return retrieveEmailByStatus(EmailMessageStatusType.SENT.getValue(), searchParameters.getLimit()); } @Override @@ -148,18 +140,12 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { @Override public Collection retrieveAllDelivered(final Integer limit) { - final String sqlPlusLimit = (limit > 0) ? " " + sqlGenerator.limit(limit) : ""; - final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - - return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.DELIVERED.getValue()); // NOSONAR + return retrieveEmailByStatus(EmailMessageStatusType.DELIVERED.getValue(), limit); } @Override public Collection retrieveAllFailed(final SearchParameters searchParameters) { - final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : ""; - final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - - return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.FAILED.getValue()); // NOSONAR + return retrieveEmailByStatus(EmailMessageStatusType.FAILED.getValue(), searchParameters.getLimit()); } @Override @@ -185,4 +171,16 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlBuilder.toString(), new Object[] { status, fromDateString, toDateString }, this.emailRowMapper); } + + private Collection retrieveEmailByStatus(final Integer status, final Integer limit) { + StringBuilder sql = new StringBuilder("select " + this.emailRowMapper.schema() + " where emo.status_enum = ?"); + List args = new ArrayList<>(); + args.add(status); + + if (limit != null && limit > 0) { + sql.append(" ").append(sqlGenerator.limit(limit)); + } + + return this.jdbcTemplate.query(sql.toString(), this.emailRowMapper, args.toArray()); + } } diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImplTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImplTest.java new file mode 100644 index 000000000..049c9d67d --- /dev/null +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImplTest.java @@ -0,0 +1,157 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ +package org.apache.fineract.infrastructure.campaigns.email.service; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.apache.fineract.infrastructure.core.service.PaginationHelper; +import org.apache.fineract.infrastructure.core.service.SearchParameters; +import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator; +import org.apache.fineract.infrastructure.core.service.database.DatabaseTypeResolver; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowMapper; + +@ExtendWith(MockitoExtension.class) +@SuppressWarnings("unchecked") +class EmailReadPlatformServiceImplTest { + + @Mock + private JdbcTemplate jdbcTemplate; + + @Mock + private DatabaseTypeResolver databaseTypeResolver; // Mock ONLY the resolver + + @Mock + private PaginationHelper paginationHelper; + + private EmailReadPlatformServiceImpl emailReadPlatformService; + + @BeforeEach + void setUp() { + // Use the REAL DatabaseSpecificSQLGenerator, not a mock + // Pass null for RoutingDataSource as it's not needed for limit() + DatabaseSpecificSQLGenerator sqlGenerator = new DatabaseSpecificSQLGenerator(databaseTypeResolver, null); + emailReadPlatformService = new EmailReadPlatformServiceImpl(jdbcTemplate, sqlGenerator, paginationHelper); + } + + @Test + void testRetrieveAllPendingWithMySQL() { + // Given + SearchParameters searchParameters = SearchParameters.builder().limit(10).build(); + + // Simulate MySQL environment + when(databaseTypeResolver.isMySQL()).thenReturn(true); + // isPostgreSQL not checked if isMySQL is true + + // When + emailReadPlatformService.retrieveAllPending(searchParameters); + + // Then + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class)); + + String executedSql = sqlCaptor.getValue(); + // Verify MySQL specific syntax (LIMIT 10) + assertTrue(executedSql.contains("LIMIT 0,10"), "SQL should contain MySQL LIMIT clause: " + executedSql); + } + + @Test + void testRetrieveAllSentWithPostgres() { + // Given + SearchParameters searchParameters = SearchParameters.builder().limit(5).build(); + + // Simulate Postgres environment + when(databaseTypeResolver.isMySQL()).thenReturn(false); + when(databaseTypeResolver.isPostgreSQL()).thenReturn(true); + + // When + emailReadPlatformService.retrieveAllSent(searchParameters); + + // Then + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class)); + + String executedSql = sqlCaptor.getValue(); + // Verify Postgres specific syntax (LIMIT 5) + assertTrue(executedSql.contains("LIMIT 5 OFFSET 0"), "SQL should contain Postgres LIMIT clause: " + executedSql); + } + + @Test + void testRetrieveAllPendingWithoutLimit() { + // Given - limit 0 means unlimited (getLimit() returns null) + SearchParameters searchParameters = SearchParameters.builder().limit(0).build(); + + // When + emailReadPlatformService.retrieveAllPending(searchParameters); + + // Then + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class)); + + String executedSql = sqlCaptor.getValue(); + // Verify NO LIMIT clause + assertTrue(!executedSql.contains("LIMIT"), "SQL should NOT contain LIMIT when limit is null"); + } + + @Test + void testRetrieveWithLimitOne() { + // Given + SearchParameters searchParameters = SearchParameters.builder().limit(1).build(); + + // Simulate MySQL environment + when(databaseTypeResolver.isMySQL()).thenReturn(true); + + // When + emailReadPlatformService.retrieveAllPending(searchParameters); + + // Then + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class)); + + String executedSql = sqlCaptor.getValue(); + // Verify MySQL specific syntax (LIMIT 1) + assertTrue(executedSql.contains("LIMIT 0,1"), "SQL should contain MySQL LIMIT 1 clause"); + } + + @Test + void testRetrieveWithNegativeLimit() { + // Given + SearchParameters searchParameters = SearchParameters.builder().limit(-5).build(); + + // When + emailReadPlatformService.retrieveAllPending(searchParameters); + + // Then + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class)); + + String executedSql = sqlCaptor.getValue(); + // Verify NO LIMIT clause for negative values (as they are invalid for SQL limit) + assertTrue(!executedSql.contains("LIMIT"), "SQL should NOT contain LIMIT when limit is negative"); + } +}