This is an automated email from the ASF dual-hosted git repository.
adamsaghy pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new f5951f1a7b FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use
Prepared Statements (#5417)
f5951f1a7b is described below
commit f5951f1a7b98838599ba90065d0e5e91fb92bca9
Author: Saifulhuq <[email protected]>
AuthorDate: Fri Feb 6 15:09:40 2026 +0530
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 +++--
.../service/EmailReadPlatformServiceImplTest.java | 157 +++++++++++++++++++++
2 files changed, 174 insertions(+), 19 deletions(-)
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 f17fdf8ce7..955ca561f6 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<EmailData> 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<EmailData> 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<EmailData> 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<EmailData> 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<EmailData> 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<EmailData> retrieveEmailByStatus(final Integer status,
final Integer limit) {
+ StringBuilder sql = new StringBuilder("select " +
this.emailRowMapper.schema() + " where emo.status_enum = ?");
+ List<Object> 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 0000000000..049c9d67db
--- /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<String> 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<String> 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<String> 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<String> 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<String> 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");
+ }
+}
