Skip to content

Commit

Permalink
Code review changes (FINERACT-854)
Browse files Browse the repository at this point in the history
  • Loading branch information
@josemakara2 @ptuomola
josemakara2 authored and ptuomola committed Apr 12, 2021
1 parent b3d0025 commit d3ef3b8
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 50 deletions.
14 changes: 7 additions & 7 deletions .../apache/fineract/infrastructure/campaigns/email/service/EmailReadPlatformServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,41 +130,41 @@ public EmailData retrieveOne(final Long resourceId) {
@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
public Collection<EmailData> retrieveAllSent(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;

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
public Collection<EmailData> retrieveAllDelivered(final Integer limit) {
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
public Collection<EmailData> retrieveAllFailed(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;

return this.jdbcTemplate.query(sql, this.emailRowMapper, new Object[] { EmailMessageStatusType.FAILED.getValue() });
return this.jdbcTemplate.query(sql, this.emailRowMapper, EmailMessageStatusType.FAILED.getValue());
}

@Override
Expand Down
10 changes: 1 addition & 9 deletions ...c/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
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;
Expand All @@ -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
Expand Down Expand Up @@ -109,10 +105,6 @@ public Response runReport(@PathParam("reportName") @Parameter(description = "rep
// 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) {
Expand Down
13 changes: 6 additions & 7 deletions ...rg/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public BasicAuthTenantDetailsServiceJdbc(@Qualifier("hikariTenantDataSource") fi
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,")//
Expand All @@ -60,19 +60,18 @@ private static final class TenantMapper implements RowMapper<FineractPlatformTen
.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;
}

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();
}

Expand Down Expand Up @@ -139,9 +138,9 @@ public FineractPlatformTenant loadTenantById(final String tenantIdentifier, fina

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);
}
Expand Down
31 changes: 11 additions & 20 deletions ...in/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ public JdbcTenantDetailsService(@Qualifier("hikariTenantDataSource") final DataS

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,")//
Expand All @@ -62,28 +60,21 @@ private static final class TenantMapper implements RowMapper<FineractPlatformTen
.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
Expand Down Expand Up @@ -136,22 +127,22 @@ private int bindValueInMinMaxRange(final int value, int min, int max) {

@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;
Expand Down
9 changes: 2 additions & 7 deletions ...ider/src/main/resources/sql/migrations/core_db/V365__reportCategoryList-FINERACT-1306.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit d3ef3b8

Please sign in to comment.