Skip to content

Commit f5951f1

Browse files
authored
FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements (#5417)
* FINERACT-2461: Refactor EmailReadPlatformServiceImpl to use Prepared Statements * FINERACT-2461: Add unit tests for EmailReadPlatformServiceImpl
1 parent c57f44d commit f5951f1

File tree

2 files changed

+174
-19
lines changed

2 files changed

+174
-19
lines changed

fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.sql.ResultSet;
2222
import java.sql.SQLException;
2323
import java.time.LocalDate;
24+
import java.util.ArrayList;
2425
import java.util.Collection;
2526
import java.util.List;
2627
import lombok.RequiredArgsConstructor;
@@ -105,17 +106,14 @@ public EmailData mapRow(final ResultSet rs, @SuppressWarnings("unused") final in
105106

106107
@Override
107108
public Collection<EmailData> retrieveAll() {
108-
109109
final String sql = "select " + this.emailRowMapper.schema();
110-
111110
return this.jdbcTemplate.query(sql, this.emailRowMapper); // NOSONAR
112111
}
113112

114113
@Override
115114
public EmailData retrieveOne(final Long resourceId) {
116115
try {
117116
final String sql = "select " + this.emailRowMapper.schema() + " where emo.id = ?";
118-
119117
return this.jdbcTemplate.queryForObject(sql, this.emailRowMapper, new Object[] { resourceId }); // NOSONAR
120118
} catch (final EmptyResultDataAccessException e) {
121119
throw new EmailNotFoundException(resourceId, e);
@@ -124,18 +122,12 @@ public EmailData retrieveOne(final Long resourceId) {
124122

125123
@Override
126124
public Collection<EmailData> retrieveAllPending(final SearchParameters searchParameters) {
127-
final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : "";
128-
final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum =? " + sqlPlusLimit;
129-
130-
return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.PENDING.getValue()); // NOSONAR
125+
return retrieveEmailByStatus(EmailMessageStatusType.PENDING.getValue(), searchParameters.getLimit());
131126
}
132127

133128
@Override
134129
public Collection<EmailData> retrieveAllSent(final SearchParameters searchParameters) {
135-
final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : "";
136-
final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit;
137-
138-
return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.SENT.getValue()); // NOSONAR
130+
return retrieveEmailByStatus(EmailMessageStatusType.SENT.getValue(), searchParameters.getLimit());
139131
}
140132

141133
@Override
@@ -148,18 +140,12 @@ public List<Long> retrieveExternalIdsOfAllSent(final Integer limit) {
148140

149141
@Override
150142
public Collection<EmailData> retrieveAllDelivered(final Integer limit) {
151-
final String sqlPlusLimit = (limit > 0) ? " " + sqlGenerator.limit(limit) : "";
152-
final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit;
153-
154-
return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.DELIVERED.getValue()); // NOSONAR
143+
return retrieveEmailByStatus(EmailMessageStatusType.DELIVERED.getValue(), limit);
155144
}
156145

157146
@Override
158147
public Collection<EmailData> retrieveAllFailed(final SearchParameters searchParameters) {
159-
final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " " + sqlGenerator.limit(searchParameters.getLimit()) : "";
160-
final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit;
161-
162-
return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.FAILED.getValue()); // NOSONAR
148+
return retrieveEmailByStatus(EmailMessageStatusType.FAILED.getValue(), searchParameters.getLimit());
163149
}
164150

165151
@Override
@@ -185,4 +171,16 @@ public Page<EmailData> retrieveEmailByStatus(final Integer limit, final Integer
185171
return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlBuilder.toString(),
186172
new Object[] { status, fromDateString, toDateString }, this.emailRowMapper);
187173
}
174+
175+
private Collection<EmailData> retrieveEmailByStatus(final Integer status, final Integer limit) {
176+
StringBuilder sql = new StringBuilder("select " + this.emailRowMapper.schema() + " where emo.status_enum = ?");
177+
List<Object> args = new ArrayList<>();
178+
args.add(status);
179+
180+
if (limit != null && limit > 0) {
181+
sql.append(" ").append(sqlGenerator.limit(limit));
182+
}
183+
184+
return this.jdbcTemplate.query(sql.toString(), this.emailRowMapper, args.toArray());
185+
}
188186
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.fineract.infrastructure.campaigns.email.service;
20+
21+
import static org.junit.jupiter.api.Assertions.assertTrue;
22+
import static org.mockito.ArgumentMatchers.any;
23+
import static org.mockito.Mockito.verify;
24+
import static org.mockito.Mockito.when;
25+
26+
import org.apache.fineract.infrastructure.core.service.PaginationHelper;
27+
import org.apache.fineract.infrastructure.core.service.SearchParameters;
28+
import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
29+
import org.apache.fineract.infrastructure.core.service.database.DatabaseTypeResolver;
30+
import org.junit.jupiter.api.BeforeEach;
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.api.extension.ExtendWith;
33+
import org.mockito.ArgumentCaptor;
34+
import org.mockito.Mock;
35+
import org.mockito.junit.jupiter.MockitoExtension;
36+
import org.springframework.jdbc.core.JdbcTemplate;
37+
import org.springframework.jdbc.core.RowMapper;
38+
39+
@ExtendWith(MockitoExtension.class)
40+
@SuppressWarnings("unchecked")
41+
class EmailReadPlatformServiceImplTest {
42+
43+
@Mock
44+
private JdbcTemplate jdbcTemplate;
45+
46+
@Mock
47+
private DatabaseTypeResolver databaseTypeResolver; // Mock ONLY the resolver
48+
49+
@Mock
50+
private PaginationHelper paginationHelper;
51+
52+
private EmailReadPlatformServiceImpl emailReadPlatformService;
53+
54+
@BeforeEach
55+
void setUp() {
56+
// Use the REAL DatabaseSpecificSQLGenerator, not a mock
57+
// Pass null for RoutingDataSource as it's not needed for limit()
58+
DatabaseSpecificSQLGenerator sqlGenerator = new DatabaseSpecificSQLGenerator(databaseTypeResolver, null);
59+
emailReadPlatformService = new EmailReadPlatformServiceImpl(jdbcTemplate, sqlGenerator, paginationHelper);
60+
}
61+
62+
@Test
63+
void testRetrieveAllPendingWithMySQL() {
64+
// Given
65+
SearchParameters searchParameters = SearchParameters.builder().limit(10).build();
66+
67+
// Simulate MySQL environment
68+
when(databaseTypeResolver.isMySQL()).thenReturn(true);
69+
// isPostgreSQL not checked if isMySQL is true
70+
71+
// When
72+
emailReadPlatformService.retrieveAllPending(searchParameters);
73+
74+
// Then
75+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
76+
verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class));
77+
78+
String executedSql = sqlCaptor.getValue();
79+
// Verify MySQL specific syntax (LIMIT 10)
80+
assertTrue(executedSql.contains("LIMIT 0,10"), "SQL should contain MySQL LIMIT clause: " + executedSql);
81+
}
82+
83+
@Test
84+
void testRetrieveAllSentWithPostgres() {
85+
// Given
86+
SearchParameters searchParameters = SearchParameters.builder().limit(5).build();
87+
88+
// Simulate Postgres environment
89+
when(databaseTypeResolver.isMySQL()).thenReturn(false);
90+
when(databaseTypeResolver.isPostgreSQL()).thenReturn(true);
91+
92+
// When
93+
emailReadPlatformService.retrieveAllSent(searchParameters);
94+
95+
// Then
96+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
97+
verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class));
98+
99+
String executedSql = sqlCaptor.getValue();
100+
// Verify Postgres specific syntax (LIMIT 5)
101+
assertTrue(executedSql.contains("LIMIT 5 OFFSET 0"), "SQL should contain Postgres LIMIT clause: " + executedSql);
102+
}
103+
104+
@Test
105+
void testRetrieveAllPendingWithoutLimit() {
106+
// Given - limit 0 means unlimited (getLimit() returns null)
107+
SearchParameters searchParameters = SearchParameters.builder().limit(0).build();
108+
109+
// When
110+
emailReadPlatformService.retrieveAllPending(searchParameters);
111+
112+
// Then
113+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
114+
verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class));
115+
116+
String executedSql = sqlCaptor.getValue();
117+
// Verify NO LIMIT clause
118+
assertTrue(!executedSql.contains("LIMIT"), "SQL should NOT contain LIMIT when limit is null");
119+
}
120+
121+
@Test
122+
void testRetrieveWithLimitOne() {
123+
// Given
124+
SearchParameters searchParameters = SearchParameters.builder().limit(1).build();
125+
126+
// Simulate MySQL environment
127+
when(databaseTypeResolver.isMySQL()).thenReturn(true);
128+
129+
// When
130+
emailReadPlatformService.retrieveAllPending(searchParameters);
131+
132+
// Then
133+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
134+
verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class));
135+
136+
String executedSql = sqlCaptor.getValue();
137+
// Verify MySQL specific syntax (LIMIT 1)
138+
assertTrue(executedSql.contains("LIMIT 0,1"), "SQL should contain MySQL LIMIT 1 clause");
139+
}
140+
141+
@Test
142+
void testRetrieveWithNegativeLimit() {
143+
// Given
144+
SearchParameters searchParameters = SearchParameters.builder().limit(-5).build();
145+
146+
// When
147+
emailReadPlatformService.retrieveAllPending(searchParameters);
148+
149+
// Then
150+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
151+
verify(jdbcTemplate).query(sqlCaptor.capture(), any(RowMapper.class), any(Object[].class));
152+
153+
String executedSql = sqlCaptor.getValue();
154+
// Verify NO LIMIT clause for negative values (as they are invalid for SQL limit)
155+
assertTrue(!executedSql.contains("LIMIT"), "SQL should NOT contain LIMIT when limit is negative");
156+
}
157+
}

0 commit comments

Comments
 (0)