FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements (#5417)

* FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements * FINERACT-2461: Add unit tests for EmailReadPlatformServiceImpl
2026-02-06 10:39:40 +01:00
parent c57f44d3f0
commit f5951f1a7b
2 changed files with 174 additions and 19 deletions
@@ -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());
+    }
 }
@@ -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");
+    }
+}