Skip to content

Commit

Permalink
FINERACT-2076: SQL query optimization
Browse files Browse the repository at this point in the history
  • Loading branch information
vidakovic committed Apr 25, 2024
1 parent 039bca4 commit a022a51
Show file tree
Hide file tree
Showing 85 changed files with 1,186 additions and 1,193 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ public String retrieveProviioningEntries(@QueryParam("entryId") final Long entry
@QueryParam("productId") final Long productId, @QueryParam("categoryId") final Long categoryId,
@Context final UriInfo uriInfo) {
this.platformSecurityContext.authenticatedUser();
SearchParameters params = SearchParameters.forProvisioningEntries(entryId, officeId, productId, categoryId, offset, limit);
SearchParameters params = SearchParameters.builder().limit(limit).offset(offset).provisioningEntryId(entryId).officeId(officeId)
.productId(productId).categoryId(categoryId).build();
Page<LoanProductProvisioningEntryData> entries = this.provisioningEntriesReadPlatformService.retrieveProvisioningEntries(params);
final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters());
return this.entriesApiJsonSerializer.serialize(settings, entries, PROVISIONING_ENTRY_PARAMETERS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,33 +319,33 @@ public Page<LoanProductProvisioningEntryData> retrieveProvisioningEntries(Search
String whereClose = " where ";
List<Object> items = new ArrayList<>();

if (searchParams.isProvisioningEntryIdPassed()) {
if (searchParams.hasProvisioningEntryId()) {
sqlBuilder.append(whereClose + " entry.history_id = ?");
items.add(searchParams.getProvisioningEntryId());
whereClose = " and ";
}

if (searchParams.isOfficeIdPassed()) {
if (searchParams.hasOfficeId()) {
sqlBuilder.append(whereClose + " entry.office_id = ?");
items.add(searchParams.getOfficeId());
whereClose = " and ";
}

if (searchParams.isProductIdPassed()) {
if (searchParams.hasProductId()) {
sqlBuilder.append(whereClose + " entry.product_id = ?");
items.add(searchParams.getProductId());
whereClose = " and ";
}

if (searchParams.isCategoryIdPassed()) {
if (searchParams.hasCategoryId()) {
sqlBuilder.append(whereClose + " entry.category_id = ?");
items.add(searchParams.getCategoryId());
}
sqlBuilder.append(" order by entry.id");

if (searchParams.isLimited()) {
if (searchParams.hasLimit()) {
sqlBuilder.append(" limit ").append(searchParams.getLimit());
if (searchParams.isOffset()) {
if (searchParams.hasOffset()) {
sqlBuilder.append(" offset ").append(searchParams.getOffset());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.apache.fineract.infrastructure.core.service.Page;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
import org.apache.fineract.infrastructure.security.service.SqlValidator;
import org.apache.fineract.organisation.teller.data.CashierData;
import org.apache.fineract.organisation.teller.data.CashierTransactionData;
import org.apache.fineract.organisation.teller.data.CashierTransactionsWithSummaryData;
Expand All @@ -70,6 +71,7 @@ public class TellerApiResource {
private final DefaultToApiJsonSerializer<TellerData> jsonSerializer;
private final TellerManagementReadPlatformService readPlatformService;
private final PortfolioCommandSourceWritePlatformService commandWritePlatformService;
private final SqlValidator sqlValidator;

@GET
@Consumes({ MediaType.TEXT_HTML, MediaType.APPLICATION_JSON })
Expand Down Expand Up @@ -315,7 +317,10 @@ public String getTransactionsForCashier(@PathParam("tellerId") @Parameter(descri

final LocalDate fromDate = null;
final LocalDate toDate = null;
final SearchParameters searchParameters = SearchParameters.forPagination(offset, limit, orderBy, sortOrder);
sqlValidator.validate(orderBy);
sqlValidator.validate(sortOrder);
final SearchParameters searchParameters = SearchParameters.builder().limit(limit).offset(offset).orderBy(orderBy)
.sortOrder(sortOrder).build();
final Page<CashierTransactionData> cashierTxns = this.readPlatformService.retrieveCashierTransactions(cashierId, false, fromDate,
toDate, currencyCode, searchParameters);

Expand Down Expand Up @@ -344,7 +349,10 @@ public String getTransactionsWtihSummaryForCashier(@PathParam("tellerId") @Param
final LocalDate fromDate = null;
final LocalDate toDate = null;

final SearchParameters searchParameters = SearchParameters.forPagination(offset, limit, orderBy, sortOrder);
sqlValidator.validate(orderBy);
sqlValidator.validate(sortOrder);
final SearchParameters searchParameters = SearchParameters.builder().limit(limit).offset(offset).orderBy(orderBy)
.sortOrder(sortOrder).build();

final CashierTransactionsWithSummaryData cashierTxnWithSummary = this.readPlatformService
.retrieveCashierTransactionsWithSummary(cashierId, false, fromDate, toDate, currencyCode, searchParameters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,9 @@ public CashierTransactionDataValidator(final TellerManagementReadPlatformService
}

public void validateSettleCashAndCashOutTransactions(final Long cashierId, String currencyCode, final BigDecimal transactionAmount) {
final Integer offset = null;
final Integer limit = null;
final String orderBy = null;
final String sortOrder = null;
final LocalDate fromDate = null;
final LocalDate toDate = null;
final SearchParameters searchParameters = SearchParameters.forPagination(offset, limit, orderBy, sortOrder);
final SearchParameters searchParameters = SearchParameters.builder().build();
final CashierTransactionsWithSummaryData cashierTxnWithSummary = this.tellerManagementReadPlatformService
.retrieveCashierTransactionsWithSummary(cashierId, false, fromDate, toDate, currencyCode, searchParameters);
.retrieveCashierTransactionsWithSummary(cashierId, false, null, null, currencyCode, searchParameters);
if (MathUtil.isGreaterThan(transactionAmount, cashierTxnWithSummary.getNetCash())) {
throw new CashierInsufficientAmountException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ public class FineractProperties {

private FineractModulesProperties module;

private FineractSqlValidationProperties sqlValidation;

@Getter
@Setter
public static class FineractTenantProperties {
Expand Down Expand Up @@ -533,4 +535,38 @@ public static class FineractModulesProperties {
public static class FineractInvestorModuleProperties extends AbstractFineractModuleProperties {

}

@Getter
@Setter
public static class FineractSqlValidationProperties {

private List<FineractSqlValidationPatternProperties> patterns;
private List<FineractSqlValidationProfileProperties> profiles;
}

@Getter
@Setter
public static class FineractSqlValidationProfileProperties {

private String name;
private String description;
private List<FineractSqlValidationPatternReferenceProperties> patternRefs;
private Boolean enabled = true;
}

@Getter
@Setter
public static class FineractSqlValidationPatternReferenceProperties {

private String name;
private Integer order;
}

@Getter
@Setter
public static class FineractSqlValidationPatternProperties {

private String name;
private String pattern;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,101 +18,62 @@
*/
package org.apache.fineract.infrastructure.core.data;

import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator;

/**
* <p>
* Immutable data object representing pagination parameter values.
* </p>
*/
public final class PaginationParameters {

private final boolean paged;
private final Integer offset;
private final Integer limit;
private final String orderBy;
private final String sortOrder;

public static PaginationParameters instance(Boolean paged, Integer offset, Integer limit, String orderBy, String sortOrder) {
if (null == paged) {
paged = false;
}

final Integer maxLimitAllowed = getCheckedLimit(limit);
@Builder
@Getter
public class PaginationParameters {

return new PaginationParameters(paged, offset, maxLimitAllowed, orderBy, sortOrder);
}

private PaginationParameters(boolean paged, Integer offset, Integer limit, String orderBy, String sortOrder) {
SQLInjectionValidator.validateSQLInput(orderBy);
SQLInjectionValidator.validateSQLInput(sortOrder);

this.paged = paged;
this.offset = offset;
this.limit = limit;
this.orderBy = orderBy;
this.sortOrder = sortOrder;
}

public static Integer getCheckedLimit(final Integer limit) {
// TODO: why do we really need this class? SearchParameters seems to provide similar functionality

final Integer maxLimitAllowed = 200;
// default to max limit first off
Integer checkedLimit = maxLimitAllowed;
public static final int DEFAULT_MAX_LIMIT = 200;

if (limit != null && limit > 0) {
checkedLimit = limit;
} else if (limit != null) {
// unlimited case: limit provided and 0 or less
checkedLimit = null;
}

return checkedLimit;
}

public boolean isPaged() {
return this.paged;
}

public Integer getOffset() {
return this.offset;
}
private boolean paged;
private Integer offset;
@Getter(AccessLevel.NONE)
private Integer limit;
private String orderBy;
private String sortOrder;

public Integer getLimit() {
return this.limit;
}
if (limit == null) {
return DEFAULT_MAX_LIMIT;
}

public String getOrderBy() {
return this.orderBy;
}
if (limit > 0) {
return limit;
}

public String getSortOrder() {
return this.sortOrder;
return null; // unlimited (0 or less)
}

public boolean isOrderByRequested() {
public boolean hasOrderBy() {
return StringUtils.isNotBlank(this.orderBy);
}

public boolean isSortOrderProvided() {
public boolean hasSortOrder() {
return StringUtils.isNotBlank(this.sortOrder);
}

public boolean isLimited() {
return this.limit != null && this.limit.intValue() > 0;
public boolean hasLimit() {
return this.limit != null && this.limit > 0;
}

public boolean isOffset() {
public boolean hasOffset() {
return this.offset != null;
}

// TODO: following functions are just doing too much in one place; will disappear with type safe queries

public String orderBySql() {
final StringBuilder sql = new StringBuilder();

if (this.isOrderByRequested()) {
if (this.hasOrderBy()) {
sql.append(" order by ").append(this.getOrderBy());
if (this.isSortOrderProvided()) {
if (this.hasSortOrder()) {
sql.append(' ').append(this.getSortOrder());
}
}
Expand All @@ -121,9 +82,9 @@ public String orderBySql() {

public String limitSql() {
final StringBuilder sql = new StringBuilder();
if (this.isLimited()) {
if (this.hasLimit()) {
sql.append(" limit ").append(this.getLimit());
if (this.isOffset()) {
if (this.hasOffset()) {
sql.append(" offset ").append(this.getOffset());
}
}
Expand All @@ -132,10 +93,10 @@ public String limitSql() {

public String paginationSql() {
final StringBuilder sqlBuilder = new StringBuilder(50);
if (this.isOrderByRequested()) {
if (this.hasOrderBy()) {
sqlBuilder.append(' ').append(this.orderBySql());
}
if (this.isLimited()) {
if (this.hasLimit()) {
sqlBuilder.append(' ').append(this.limitSql());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class PaginationParametersDataValidator {
public void validateParameterValues(PaginationParameters parameters, final Set<String> supportedOrdeByValues,
final String resourceName) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
if (parameters.isOrderByRequested() && !supportedOrdeByValues.contains(parameters.getOrderBy())) {
if (parameters.hasOrderBy() && !supportedOrdeByValues.contains(parameters.getOrderBy())) {
final String defaultUserMessage = "The orderBy value '" + parameters.getOrderBy()
+ "' is not supported. The supported orderBy values are " + supportedOrdeByValues;
final ApiParameterError error = ApiParameterError.parameterError(
Expand All @@ -43,7 +43,7 @@ public void validateParameterValues(PaginationParameters parameters, final Set<S
dataValidationErrors.add(error);
}

if (parameters.isSortOrderProvided() && !sortOrderValues.contains(parameters.getSortOrder().toUpperCase())) {
if (parameters.hasSortOrder() && !sortOrderValues.contains(parameters.getSortOrder().toUpperCase())) {
final String defaultUserMessage = "The sortOrder value '" + parameters.getSortOrder()
+ "' is not supported. The supported sortOrder values are " + sortOrderValues;
final ApiParameterError error = ApiParameterError.parameterError(
Expand Down
Loading

0 comments on commit a022a51

Please sign in to comment.