This is an automated email from the ASF dual-hosted git repository. ptuomola pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/fineract.git
commit d3ef3b8f70292c82d7bac50582bf3441ad301ca8 Author: Joseph Makara <[email protected]> AuthorDate: Sun Apr 11 13:19:02 2021 +0300 Code review changes (FINERACT-854) --- .../service/EmailReadPlatformServiceImpl.java | 14 +++++----- .../dataqueries/api/RunreportsApiResource.java | 10 +------ .../service/BasicAuthTenantDetailsServiceJdbc.java | 13 +++++---- .../security/service/JdbcTenantDetailsService.java | 31 ++++++++-------------- .../V365__reportCategoryList-FINERACT-1306.sql | 9 ++----- 5 files changed, 27 insertions(+), 50 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 126a404..71b8332 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 @@ -130,9 +130,9 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { @Override public Collection<EmailData> retrieveAllPending(final SearchParameters searchParameters) { final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " limit 0, " + searchParameters.getLimit() : ""; - final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = " + sqlPlusLimit; + final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum =? " + sqlPlusLimit; - return this.jdbcTemplate.query(sql, this.emailRowMapper, new Object[] { EmailMessageStatusType.PENDING.getValue() }); + return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.PENDING.getValue()); } @Override @@ -140,15 +140,15 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " limit 0, " + searchParameters.getLimit() : ""; final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - return this.jdbcTemplate.query(sql, this.emailRowMapper, new Object[] { EmailMessageStatusType.SENT.getValue() }); + return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.SENT.getValue()); } @Override public List<Long> retrieveExternalIdsOfAllSent(final Integer limit) { final String sqlPlusLimit = (limit > 0) ? " limit 0, " + limit : ""; - final String sql = "select external_id from " + this.emailRowMapper.tableName() + " where status_enum = " + sqlPlusLimit; + final String sql = "select external_id from " + this.emailRowMapper.tableName() + " where status_enum =? " + sqlPlusLimit; - return this.jdbcTemplate.queryForList(sql, Long.class, new Object[] { EmailMessageStatusType.SENT.getValue() }); + return this.jdbcTemplate.queryForList(sql, Long.class, EmailMessageStatusType.SENT.getValue()); } @Override @@ -156,7 +156,7 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { final String sqlPlusLimit = (limit > 0) ? " limit 0, " + limit : ""; final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - return this.jdbcTemplate.query(sql, this.emailRowMapper, new Object[] { EmailMessageStatusType.DELIVERED.getValue() }); + return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.DELIVERED.getValue()); } @Override @@ -164,7 +164,7 @@ public class EmailReadPlatformServiceImpl implements EmailReadPlatformService { final String sqlPlusLimit = (searchParameters.getLimit() > 0) ? " limit 0, " + searchParameters.getLimit() : ""; final String sql = "select " + this.emailRowMapper.schema() + " where emo.status_enum = ?" + sqlPlusLimit; - return this.jdbcTemplate.query(sql, this.emailRowMapper, new Object[] { EmailMessageStatusType.FAILED.getValue() }); + return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.FAILED.getValue()); } @Override diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java index aa9bbb8..0f0c4c5 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java @@ -39,7 +39,6 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import org.apache.fineract.infrastructure.core.api.ApiParameterHelper; import org.apache.fineract.infrastructure.core.exception.PlatformServiceUnavailableException; -import org.apache.fineract.infrastructure.dataqueries.service.DatatableReportingProcessService; import org.apache.fineract.infrastructure.dataqueries.service.ReadReportingService; import org.apache.fineract.infrastructure.report.provider.ReportingProcessServiceProvider; import org.apache.fineract.infrastructure.report.service.ReportingProcessService; @@ -61,16 +60,13 @@ public class RunreportsApiResource { private final PlatformSecurityContext context; private final ReadReportingService readExtraDataAndReportingService; private final ReportingProcessServiceProvider reportingProcessServiceProvider; - private final DatatableReportingProcessService datatableReportingProcessService; @Autowired public RunreportsApiResource(final PlatformSecurityContext context, final ReadReportingService readExtraDataAndReportingService, - final ReportingProcessServiceProvider reportingProcessServiceProvider, - DatatableReportingProcessService aDatatableReportingProcessService) { + final ReportingProcessServiceProvider reportingProcessServiceProvider) { this.context = context; this.readExtraDataAndReportingService = readExtraDataAndReportingService; this.reportingProcessServiceProvider = reportingProcessServiceProvider; - datatableReportingProcessService = aDatatableReportingProcessService; } @GET @@ -109,10 +105,6 @@ public class RunreportsApiResource { // Pass through isSelfServiceUserReport so that ReportingProcessService implementations can use it queryParams.putSingle(IS_SELF_SERVICE_USER_REPORT_PARAMETER, Boolean.toString(isSelfServiceUserReport)); - if (parameterType) { - return datatableReportingProcessService.processRequest(reportName, queryParams); - } - String reportType = this.readExtraDataAndReportingService.getReportType(reportName, isSelfServiceUserReport); ReportingProcessService reportingProcessService = this.reportingProcessServiceProvider.findReportingProcessService(reportType); if (reportingProcessService == null) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java index a5c1596..bbbff0a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java @@ -49,7 +49,7 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails private static final class TenantMapper implements RowMapper<FineractPlatformTenant> { private final boolean isReport; - private final StringBuilder sqlBuilder = new StringBuilder("SELECT t.id, ts.id as connectionId, ")// + private final StringBuilder sqlBuilder = new StringBuilder(" t.id, ts.id as connectionId , ")// .append(" t.timezone_id as timezoneId , t.name,t.identifier, ts.schema_name as schemaName, ts.schema_server as schemaServer,")// .append(" ts.schema_server_port as schemaServerPort, ts.schema_connection_parameters as schemaConnectionParameters, ts.auto_update as autoUpdate,")// .append(" ts.schema_username as schemaUsername, ts.schema_password as schemaPassword , ts.pool_initial_size as initialSize,")// @@ -60,7 +60,7 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails .append(" ts.pool_min_evictable_idle_time_millis as poolMinEvictableIdleTimeMillis,")// .append(" ts.deadlock_max_retries as maxRetriesOnDeadlock,")// .append(" ts.deadlock_max_retry_interval as maxIntervalBetweenRetries ")// - .append("FROM tenants t LEFT JOIN tenant_server_connections ts "); + .append(" from tenants t left join tenant_server_connections ts "); TenantMapper(boolean isReport) { this.isReport = isReport; @@ -68,11 +68,10 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails public String schema() { if (this.isReport) { - this.sqlBuilder.append(" ON t.report_Id = ts.id"); + this.sqlBuilder.append(" on t.report_Id = ts.id"); } else { - this.sqlBuilder.append(" ON t.oltp_Id = ts.id"); + this.sqlBuilder.append(" on t.oltp_Id = ts.id"); } - this.sqlBuilder.append(" WHERE t.identifier = ?"); return this.sqlBuilder.toString(); } @@ -139,9 +138,9 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails try { final TenantMapper rm = new TenantMapper(isReport); - final String sql = rm.schema(); + final String sql = "select " + rm.schema() + " where t.identifier = ?"; - return this.jdbcTemplate.queryForObject(sql, rm, tenantIdentifier); + return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { tenantIdentifier }); } catch (final EmptyResultDataAccessException e) { throw new InvalidTenantIdentiferException("The tenant identifier: " + tenantIdentifier + " is not valid.", e); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java index 879fec9..4e645bf 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java @@ -49,9 +49,7 @@ public class JdbcTenantDetailsService implements TenantDetailsService { private static final class TenantMapper implements RowMapper<FineractPlatformTenant> { - private final String tenantIdentifier; - - private final StringBuilder sqlBuilder = new StringBuilder("SELECT t.id, ts.id as connectionId, ")// + private final StringBuilder sqlBuilder = new StringBuilder("t.id, ts.id as connectionId , ")// .append(" t.timezone_id as timezoneId , t.name,t.identifier, ts.schema_name as schemaName, ts.schema_server as schemaServer,")// .append(" ts.schema_server_port as schemaServerPort, ts.schema_connection_parameters as schemaConnectionParameters, ts.auto_update as autoUpdate,")// .append(" ts.schema_username as schemaUsername, ts.schema_password as schemaPassword , ts.pool_initial_size as initialSize,")// @@ -62,28 +60,21 @@ public class JdbcTenantDetailsService implements TenantDetailsService { .append(" ts.pool_min_evictable_idle_time_millis as poolMinEvictableIdleTimeMillis,")// .append(" ts.deadlock_max_retries as maxRetriesOnDeadlock,")// .append(" ts.deadlock_max_retry_interval as maxIntervalBetweenRetries ")// - .append("FROM tenants t LEFT JOIN tenant_server_connections ts ON t.oltp_Id=ts.id "); - - TenantMapper(String aTenantIdentifier) { - this.tenantIdentifier = aTenantIdentifier; - } + .append(" from tenants t left join tenant_server_connections ts on t.oltp_Id=ts.id "); public String schema() { - if (tenantIdentifier != null) { - this.sqlBuilder.append(" WHERE t.identifier = ?"); - } return this.sqlBuilder.toString(); } @Override public FineractPlatformTenant mapRow(final ResultSet rs, @SuppressWarnings("unused") final int rowNum) throws SQLException { final Long id = rs.getLong("id"); - final String identifier = rs.getString("identifier"); + final String tenantIdentifier = rs.getString("identifier"); final String name = rs.getString("name"); final String timezoneId = rs.getString("timezoneId"); final FineractPlatformTenantConnection connection = getDBConnection(rs); - return new FineractPlatformTenant(id, identifier, name, timezoneId, connection); + return new FineractPlatformTenant(id, tenantIdentifier, name, timezoneId, connection); } // gets the DB connection @@ -136,22 +127,22 @@ public class JdbcTenantDetailsService implements TenantDetailsService { @Override @Cacheable(value = "tenantsById") - public FineractPlatformTenant loadTenantById(final String aTenantIdentifier) { + public FineractPlatformTenant loadTenantById(final String tenantIdentifier) { try { - final TenantMapper rm = new TenantMapper(aTenantIdentifier); - final String sql = rm.schema(); + final TenantMapper rm = new TenantMapper(); + final String sql = "select " + rm.schema() + " where t.identifier = ?"; - return this.jdbcTemplate.queryForObject(sql, rm, aTenantIdentifier); + return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { tenantIdentifier }); } catch (final EmptyResultDataAccessException e) { - throw new InvalidTenantIdentiferException("The tenant identifier: " + aTenantIdentifier + " is not valid.", e); + throw new InvalidTenantIdentiferException("The tenant identifier: " + tenantIdentifier + " is not valid.", e); } } @Override public List<FineractPlatformTenant> findAllTenants() { - final TenantMapper rm = new TenantMapper(null); - final String sql = rm.schema(); + final TenantMapper rm = new TenantMapper(); + final String sql = "select " + rm.schema(); final List<FineractPlatformTenant> fineractPlatformTenants = this.jdbcTemplate.query(sql, rm, new Object[] {}); return fineractPlatformTenants; diff --git a/fineract-provider/src/main/resources/sql/migrations/core_db/V365__reportCategoryList-FINERACT-1306.sql b/fineract-provider/src/main/resources/sql/migrations/core_db/V365__reportCategoryList-FINERACT-1306.sql index 794ab21..70ffd5d 100644 --- a/fineract-provider/src/main/resources/sql/migrations/core_db/V365__reportCategoryList-FINERACT-1306.sql +++ b/fineract-provider/src/main/resources/sql/migrations/core_db/V365__reportCategoryList-FINERACT-1306.sql @@ -18,10 +18,5 @@ -- -- two tables added: ReportCategoryList and FullReportList (FINERACT-1306) -INSERT INTO stretchy_report (report_name, report_type, report_category, report_sql, description, core_report, use_report) -SELECT 'ReportCategoryList', 'Table', NULL, NULL, NULL, 1, 1 -ON DUPLICATE key UPDATE report_name = 'ReportCategoryList'; - -INSERT INTO stretchy_report (report_name, report_type, report_category, report_sql, description, core_report, use_report) -SELECT 'FullReportList', 'Table', NULL, NULL, NULL, 1, 1 -ON DUPLICATE key UPDATE report_name = 'FullReportList'; +INSERT INTO stretchy_report (report_name, report_type, report_category, report_sql, description, core_report, use_report)VALUES ("ReportCategoryList", 'Table', '(NULL)', '(NULL)', '(NULL)', 1, 1); +INSERT INTO stretchy_report (report_name, report_type, report_category, report_sql, description, core_report, use_report)VALUES ("FullReportList", 'Table', '(NULL)', '(NULL)', '(NULL)', 1, 1);
