You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by aw...@apache.org on 2020/01/14 07:37:30 UTC
[fineract] branch develop updated: more SpotBugs related / inspired
code clean up (see FINERACT-702)
This is an automated email from the ASF dual-hosted git repository.
awasum pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new 6ca1a67 more SpotBugs related / inspired code clean up (see FINERACT-702)
new fc786fa Merge pull request #684 from vorburger/spotbugs-extra
6ca1a67 is described below
commit 6ca1a678cb35cf2f05ec107be3e43b94c1c182f3
Author: Michael Vorburger <mi...@vorburger.ch>
AuthorDate: Fri Jan 10 18:46:46 2020 +0100
more SpotBugs related / inspired code clean up (see FINERACT-702)
based upon code review of changes made by @awasum in previous commit:
- fix clear contains() GC_UNRELATED_TYPES bug in Loan
- fix clear equals() EC_UNRELATED_TYPES bug in Loan
- fix more equals() & hashCode() in...
* Charge
* TrialBalance
* LoanProductBorrowerCycleVariations
* LoanProductProvisioningEntry
- fix AccountingConstants to avoid using @FindBugsSuppressWarnings
- fix SelfLoansDataValidator instead of @FindBugsSuppressWarnings
- minor clean up in build.gradle (spotbugs has replaced findbugs)
PS: This change originally included also fixing the missing hashCode()
and equals() in ShareAccountCharge, but that turned out to be non
trivial, see FINERACT-827, and is thus not yet fixed by this.
---
fineract-provider/build.gradle | 6 ---
.../accounting/common/AccountingConstants.java | 19 ++++----
.../accounting/glaccount/domain/TrialBalance.java | 16 +++----
.../domain/LoanProductProvisioningEntry.java | 53 +++++++++++++---------
.../core/boot/ApplicationExitUtil.java | 3 +-
.../core/domain/AbstractPersistableCustom.java | 18 +-------
.../organisation/monetary/domain/MoneyHelper.java | 1 +
.../fineract/portfolio/charge/domain/Charge.java | 43 ++++++++----------
.../portfolio/loanaccount/domain/Loan.java | 18 +++++---
.../loanaccount/domain/LoanTransaction.java | 2 +
.../domain/LoanProductBorrowerCycleVariations.java | 31 ++++---------
.../domain/LoanProductConfigurableAttributes.java | 2 -
.../loanaccount/data/SelfLoansDataValidator.java | 15 +-----
.../accounting/common/AccountingConstantsTest.java | 30 ++++++++++++
14 files changed, 124 insertions(+), 133 deletions(-)
diff --git a/fineract-provider/build.gradle b/fineract-provider/build.gradle
index 934017e..da005d9 100644
--- a/fineract-provider/build.gradle
+++ b/fineract-provider/build.gradle
@@ -64,7 +64,6 @@ apply plugin: 'openjpa'
apply plugin: "com.github.spotbugs"
// apply plugin: 'pmd'
-// apply plugin: 'findbugs'
dependencyManagement {
imports {
@@ -341,11 +340,6 @@ compileJava{
/*
pmd {
sourceSets = [sourceSets.main]
- ignoreFailures = true
-}
-findbugs {
- ignoreFailures = true
- sourceSets = [sourceSets.main]
}
*/
diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/common/AccountingConstants.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/common/AccountingConstants.java
index c5bc432..26cc1ad 100755
--- a/fineract-provider/src/main/java/org/apache/fineract/accounting/common/AccountingConstants.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/common/AccountingConstants.java
@@ -21,8 +21,6 @@ package org.apache.fineract.accounting.common;
import org.apache.fineract.accounting.financialactivityaccount.data.FinancialActivityData;
import org.apache.fineract.accounting.glaccount.domain.GLAccountType;
-import net.sf.ehcache.util.FindBugsSuppressWarnings;
-
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -245,7 +243,15 @@ public class AccountingConstants {
private final Integer value;
private final String code;
private final GLAccountType mappedGLAccountType;
+
private static List<FinancialActivityData> financialActivities;
+ static {
+ financialActivities = new ArrayList<>();
+ for (final FINANCIAL_ACTIVITY type : FINANCIAL_ACTIVITY.values()) {
+ FinancialActivityData financialActivityData = convertToFinancialActivityData(type);
+ financialActivities.add(financialActivityData);
+ }
+ }
private FINANCIAL_ACTIVITY(final Integer value, final String code, final GLAccountType mappedGLAccountType) {
this.value = value;
@@ -291,16 +297,7 @@ public class AccountingConstants {
return convertToFinancialActivityData(type);
}
- // TODO Fix this..
- @FindBugsSuppressWarnings("LI_LAZY_INIT_UPDATE_STATIC")
public static List<FinancialActivityData> getAllFinancialActivities() {
- if (financialActivities == null) {
- financialActivities = new ArrayList<>();
- for (final FINANCIAL_ACTIVITY type : FINANCIAL_ACTIVITY.values()) {
- FinancialActivityData financialActivityData = convertToFinancialActivityData(type);
- financialActivities.add(financialActivityData);
- }
- }
return financialActivities;
}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/glaccount/domain/TrialBalance.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/glaccount/domain/TrialBalance.java
index 9735aff..54ec151 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/accounting/glaccount/domain/TrialBalance.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/glaccount/domain/TrialBalance.java
@@ -67,7 +67,6 @@ public class TrialBalance extends AbstractPersistableCustom<Long> {
}
protected TrialBalance() {
-
}
public Long getOfficeId() {
@@ -97,16 +96,17 @@ public class TrialBalance extends AbstractPersistableCustom<Long> {
@Override
public boolean equals(Object obj) {
if (!obj.getClass().equals(getClass())) return false;
- TrialBalance trialBalance = (TrialBalance) obj;
- return trialBalance.getOfficeId().equals(this.getOfficeId())
- && trialBalance.getGlAccountId().equals(this.getGlAccountId())
- && trialBalance.getEntryDate().equals(this.getEntryDate())
- && trialBalance.getTransactionDate().equals(this.getTransactionDate());
+ TrialBalance other = (TrialBalance) obj;
+ return Objects.equals(other.officeId, officeId)
+ && Objects.equals(other.glAccountId, glAccountId)
+ && Objects.equals(other.amount, amount)
+ && Objects.equals(other.entryDate, entryDate)
+ && Objects.equals(other.transactionDate, transactionDate)
+ && Objects.equals(other.closingBalance, closingBalance);
}
@Override
public int hashCode() {
- // TODO return Objects.hash(officeId, glAccountId, amount, entryDate, transactionDate, closingBalance);
- return Objects.hash(officeId, glAccountId, entryDate, transactionDate);
+ return Objects.hash(officeId, glAccountId, amount, entryDate, transactionDate, closingBalance);
}
}
\ No newline at end of file
diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/domain/LoanProductProvisioningEntry.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/domain/LoanProductProvisioningEntry.java
index 0eb2d73..042a187 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/domain/LoanProductProvisioningEntry.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/domain/LoanProductProvisioningEntry.java
@@ -74,8 +74,8 @@ public class LoanProductProvisioningEntry extends AbstractPersistableCustom<Long
private GLAccount expenseAccount;
protected LoanProductProvisioningEntry() {
-
}
+
public LoanProductProvisioningEntry(final LoanProduct loanProduct, final Office office, final String currencyCode,
final ProvisioningCategory provisioningCategory, final Long overdueInDays, final BigDecimal reservedAmount,
final GLAccount liabilityAccount, final GLAccount expenseAccount, Long criteriaId) {
@@ -95,41 +95,50 @@ public class LoanProductProvisioningEntry extends AbstractPersistableCustom<Long
}
public BigDecimal getReservedAmount() {
- return this.reservedAmount ;
+ return this.reservedAmount;
}
public void addReservedAmount(BigDecimal value) {
- this.reservedAmount = this.reservedAmount.add(value) ;
+ this.reservedAmount = this.reservedAmount.add(value);
}
public Office getOffice() {
- return this.office ;
- }
+ return this.office;
+ }
+
+ public GLAccount getLiabilityAccount() {
+ return this.liabilityAccount;
+ }
+
+ public String getCurrencyCode() {
+ return this.currencyCode;
+ }
+
+ public GLAccount getExpenseAccount() {
+ return this.expenseAccount;
+ }
+
+ // TODO Note that this domain class does equals() & hashCode() on getId() for @JoinColumn attributes, which not all other classes do...
- public GLAccount getLiabilityAccount() {
- return this.liabilityAccount ;
- }
-
- public String getCurrencyCode() {
- return this.currencyCode ;
- }
-
- public GLAccount getExpenseAccount() {
- return this.expenseAccount ;
- }
-
@Override
public boolean equals(Object obj) {
if (!obj.getClass().equals(getClass())) return false;
LoanProductProvisioningEntry other = (LoanProductProvisioningEntry) obj;
- return Objects.equals(other.loanProduct.getId(), this.loanProduct.getId())
- && Objects.equals(other.provisioningCategory.getId(), this.provisioningCategory.getId())
+ return Objects.equals(other.entry.getId(), this.entry.getId())
+ && Objects.equals(other.criteriaId, this.criteriaId)
&& Objects.equals(other.office.getId(), this.office.getId())
- && Objects.equals(other.getCurrencyCode(), this.getCurrencyCode());
+ && Objects.equals(other.currencyCode, this.currencyCode)
+ && Objects.equals(other.loanProduct.getId(), this.loanProduct.getId())
+ && Objects.equals(other.provisioningCategory.getId(), this.provisioningCategory.getId())
+ && Objects.equals(other.overdueInDays, this.overdueInDays)
+ && Objects.equals(other.reservedAmount, this.reservedAmount)
+ && Objects.equals(other.liabilityAccount.getId(), this.liabilityAccount.getId())
+ && Objects.equals(other.expenseAccount.getId(), this.expenseAccount.getId());
}
@Override
public int hashCode() {
- // TODO equals() must match: return Objects.hash(entry, criteriaId, office, currencyCode, loanProduct, provisioningCategory, overdueInDays, reservedAmount, liabilityAccount, expenseAccount);
- return Objects.hash(loanProduct.getId(), provisioningCategory.getId(), office.getId(), getCurrencyCode());
+ // NOT return Objects.hash(entry, criteriaId, office, currencyCode, loanProduct, provisioningCategory, overdueInDays, reservedAmount, liabilityAccount, expenseAccount);
+ // to remain consistent with the implementation in equals(), also use getId() here.
+ return Objects.hash(entry.getId(), criteriaId, office.getId(), currencyCode, loanProduct.getId(), provisioningCategory.getId(), overdueInDays, reservedAmount, liabilityAccount.getId(), expenseAccount.getId());
}
}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/ApplicationExitUtil.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/ApplicationExitUtil.java
index 1e5eb07..cbb622e 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/ApplicationExitUtil.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/ApplicationExitUtil.java
@@ -22,6 +22,7 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import org.springframework.context.ConfigurableApplicationContext;
@@ -38,7 +39,7 @@ public abstract class ApplicationExitUtil {
System.out.println("\nHit Enter to quit...");
// NOTE: In Eclipse, System.console() is not available.. so:
// (@see https://bugs.eclipse.org/bugs/show_bug.cgi?id=122429)
- BufferedReader d = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset()));
+ BufferedReader d = new BufferedReader(new InputStreamReader(System.in, StandardCharsets.UTF_8));
d.readLine();
ctx.stop();
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractPersistableCustom.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractPersistableCustom.java
index a1ca55a..b0c99e5 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractPersistableCustom.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractPersistableCustom.java
@@ -35,36 +35,20 @@ public abstract class AbstractPersistableCustom<PK extends Serializable> impleme
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
- /*
- * (non-Javadoc)
- *
- * @see org.springframework.data.domain.Persistable#getId()
- */
@Override
public Long getId() {
return id;
}
- /**
- * Sets the id of the entity.
- *
- * @param id the id to set
- */
protected void setId(final Long id) {
-
this.id = id;
}
- /*
- * (non-Javadoc)
- *
- * @see org.springframework.data.domain.Persistable#isNew()
- */
@Override
public boolean isNew() {
return null == this.id;
}
- //We have removed toString(), hashCode() and equals() methods. By adding them end up issues with OpenJPA
+ // We have removed toString(), hashCode() and equals() methods here, because by adding them here, we end up with issues on OpenJPA.
}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/organisation/monetary/domain/MoneyHelper.java b/fineract-provider/src/main/java/org/apache/fineract/organisation/monetary/domain/MoneyHelper.java
index 8f3a75f..83e5f56 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/organisation/monetary/domain/MoneyHelper.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/organisation/monetary/domain/MoneyHelper.java
@@ -40,6 +40,7 @@ public class MoneyHelper {
private ConfigurationDomainService configurationDomainService;
@PostConstruct
+ // This is a hack, but fixing this is not trivial, because some @Entity domain classes use this helper
@FindBugsSuppressWarnings("ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD")
public void someFunction () {
staticConfigurationDomainService = configurationDomainService;
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/domain/Charge.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/domain/Charge.java
index ff8b538..1a79c66 100755
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/domain/Charge.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/domain/Charge.java
@@ -112,7 +112,6 @@ public class Charge extends AbstractPersistableCustom<Long> {
private TaxGroup taxGroup;
public static Charge fromJson(final JsonCommand command, final GLAccount account, final TaxGroup taxGroup) {
-
final String name = command.stringValueOfParameterNamed("name");
final BigDecimal amount = command.bigDecimalValueOfParameterNamed("amount");
final String currencyCode = command.stringValueOfParameterNamed("currencyCode");
@@ -138,7 +137,6 @@ public class Charge extends AbstractPersistableCustom<Long> {
}
protected Charge() {
- //
}
private Charge(final String name, final BigDecimal amount, final String currencyCode, final ChargeAppliesTo chargeAppliesTo,
@@ -288,7 +286,6 @@ public class Charge extends AbstractPersistableCustom<Long> {
}
public Map<String, Object> update(final JsonCommand command) {
-
final Map<String, Object> actualChanges = new LinkedHashMap<>(7);
final String localeAsInput = command.locale();
@@ -507,7 +504,6 @@ public class Charge extends AbstractPersistableCustom<Long> {
}
public ChargeData toData() {
-
final EnumOptionData chargeTimeType = ChargeEnumerations.chargeTimeType(this.chargeTimeType);
final EnumOptionData chargeAppliesTo = ChargeEnumerations.chargeAppliesTo(this.chargeAppliesTo);
final EnumOptionData chargeCalculationType = ChargeEnumerations.chargeCalculationType(this.chargeCalculation);
@@ -605,26 +601,25 @@ public class Charge extends AbstractPersistableCustom<Long> {
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Charge)) return false;
- Charge charge = (Charge) o;
- return Objects.equals(name, charge.name) &&
- Objects.equals(amount, charge.amount) &&
- Objects.equals(currencyCode, charge.currencyCode) &&
- Objects.equals(chargeAppliesTo, charge.chargeAppliesTo) &&
- Objects.equals(chargeTimeType, charge.chargeTimeType) &&
- Objects.equals(chargeCalculation, charge.chargeCalculation) &&
- Objects.equals(chargePaymentMode, charge.chargePaymentMode) &&
- Objects.equals(feeOnDay, charge.feeOnDay) &&
- Objects.equals(feeInterval, charge.feeInterval) &&
- Objects.equals(feeOnMonth, charge.feeOnMonth) &&
- Objects.equals(penalty, charge.penalty) &&
- Objects.equals(active, charge.active) &&
- Objects.equals(deleted, charge.deleted) &&
- Objects.equals(minCap, charge.minCap) &&
- Objects.equals(maxCap, charge.maxCap) &&
- Objects.equals(feeFrequency, charge.feeFrequency) &&
- Objects.equals(account, charge.account) &&
- Objects.equals(taxGroup, charge.taxGroup);
-
+ Charge other= (Charge) o;
+ return Objects.equals(name, other.name) &&
+ Objects.equals(amount, other.amount) &&
+ Objects.equals(currencyCode, other.currencyCode) &&
+ Objects.equals(chargeAppliesTo, other.chargeAppliesTo) &&
+ Objects.equals(chargeTimeType, other.chargeTimeType) &&
+ Objects.equals(chargeCalculation, other.chargeCalculation) &&
+ Objects.equals(chargePaymentMode, other.chargePaymentMode) &&
+ Objects.equals(feeOnDay, other.feeOnDay) &&
+ Objects.equals(feeInterval, other.feeInterval) &&
+ Objects.equals(feeOnMonth, other.feeOnMonth) &&
+ penalty == other.penalty &&
+ active == other.active &&
+ deleted == other.deleted &&
+ Objects.equals(minCap, other.minCap) &&
+ Objects.equals(maxCap, other.maxCap) &&
+ Objects.equals(feeFrequency, other.feeFrequency) &&
+ Objects.equals(account, other.account) &&
+ Objects.equals(taxGroup, other.taxGroup);
}
@Override
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java
index a8e386e..678885d 100755
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java
@@ -734,15 +734,12 @@ public class Loan extends AbstractPersistableCustom<Long> {
updateLoanSummaryDerivedFields();
}
- // TODO Fix this correctly...
- // See this review: https://github.com/apache/fineract/pull/670/files/0409af3903d350afe43ef4837e4d915ccbe14285#r357920211
- @FindBugsSuppressWarnings("GC_UNRELATED_TYPES")
private void removeOrModifyTransactionAssociatedWithLoanChargeIfDueAtDisbursement(final LoanCharge loanCharge) {
if (loanCharge.isDueAtDisbursement()) {
LoanTransaction transactionToRemove = null;
List<LoanTransaction> transactions = getLoanTransactions() ;
for (final LoanTransaction transaction : transactions) {
- if (transaction.isRepaymentAtDisbursement() && transaction.getLoanChargesPaid().contains(loanCharge)) {
+ if (transaction.isRepaymentAtDisbursement() && doesLoanChargePaidByContainLoanCharge(transaction.getLoanChargesPaid(), loanCharge)) {
final MonetaryCurrency currency = loanCurrency();
final Money chargeAmount = Money.of(currency, loanCharge.amount());
@@ -764,6 +761,15 @@ public class Loan extends AbstractPersistableCustom<Long> {
}
}
+ private boolean doesLoanChargePaidByContainLoanCharge(Set<LoanChargePaidBy> loanChargePaidBys, LoanCharge loanCharge) {
+ for (LoanChargePaidBy loanChargePaidBy : loanChargePaidBys) {
+ if (loanChargePaidBy.getLoanCharge().equals(loanCharge)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
public Map<String, Object> updateLoanCharge(final LoanCharge loanCharge, final JsonCommand command) {
final Map<String, Object> actualChanges = new LinkedHashMap<>(3);
@@ -2719,8 +2725,6 @@ public class Loan extends AbstractPersistableCustom<Long> {
return chargesPayment;
}
- // TODO FIX THIS
- @FindBugsSuppressWarnings("EC_UNRELATED_TYPES") // see this review: https://github.com/apache/fineract/pull/670/files/0409af3903d350afe43ef4837e4d915ccbe14285#r357920364
public Map<String, Object> undoDisbursal(final ScheduleGeneratorDTO scheduleGeneratorDTO, final List<Long> existingTransactionIds,
final List<Long> existingReversedTransactionIds, AppUser currentUser) {
@@ -2741,7 +2745,7 @@ public class Loan extends AbstractPersistableCustom<Long> {
this.actualDisbursementDate = null;
this.disbursedBy = null;
boolean isDisbursedAmountChanged =
- !this.approvedPrincipal.equals(this.loanRepaymentScheduleDetail.getPrincipal());
+ !this.approvedPrincipal.equals(this.loanRepaymentScheduleDetail.getPrincipal().getAmount());
this.loanRepaymentScheduleDetail.setPrincipal(this.approvedPrincipal);
if (this.loanProduct.isMultiDisburseLoan()) {
for (final LoanDisbursementDetails details : this.disbursementDetails) {
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java
index 938177f..8b7ad31 100755
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java
@@ -796,4 +796,6 @@ public class LoanTransaction extends AbstractPersistableCustom<Long> {
&& !(this.isDisbursement() || this.isAccrual() || this.isRepaymentAtDisbursement() || this.isNonMonetaryTransaction() || this
.isIncomePosting());
}
+
+ // TODO missing hashCode(), equals(Object obj), but probably OK as long as this is never stored in a Collection.
}
\ No newline at end of file
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductBorrowerCycleVariations.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductBorrowerCycleVariations.java
index 8530758..1070331 100755
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductBorrowerCycleVariations.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductBorrowerCycleVariations.java
@@ -56,7 +56,6 @@ public class LoanProductBorrowerCycleVariations extends AbstractPersistableCusto
private BigDecimal defaultValue;
protected LoanProductBorrowerCycleVariations() {
-
}
public LoanProductBorrowerCycleVariations(final Integer borrowerCycleNumber, final Integer paramType, final Integer valueConditionType,
@@ -83,30 +82,19 @@ public class LoanProductBorrowerCycleVariations extends AbstractPersistableCusto
@Override
public boolean equals(final Object obj) {
- final LoanProductBorrowerCycleVariations borrowerCycleVariations = (LoanProductBorrowerCycleVariations) obj;
- boolean minValequal = false;
- if (borrowerCycleVariations.minValue == null && this.minValue == null) {
- minValequal = true;
- } else if (borrowerCycleVariations.minValue != null && this.minValue != null) {
- minValequal = borrowerCycleVariations.minValue.equals(this.minValue);
- }
-
- boolean maxValequal = false;
- if (borrowerCycleVariations.maxValue == null && this.maxValue == null) {
- maxValequal = true;
- } else if (borrowerCycleVariations.maxValue != null && this.maxValue != null) {
- maxValequal = borrowerCycleVariations.maxValue.equals(this.maxValue);
- }
- if (borrowerCycleVariations.borrowerCycleNumber.equals(this.borrowerCycleNumber)
- && borrowerCycleVariations.defaultValue.equals(this.defaultValue) && minValequal && maxValequal
- && borrowerCycleVariations.valueConditionType.equals(this.valueConditionType)
- && borrowerCycleVariations.paramType.equals(this.paramType)) { return true; }
- return false;
+ final LoanProductBorrowerCycleVariations other = (LoanProductBorrowerCycleVariations) obj;
+ return Objects.equals(loanProduct, other.loanProduct)
+ && Objects.equals(borrowerCycleNumber, other.borrowerCycleNumber)
+ && Objects.equals(paramType, other.paramType)
+ && Objects.equals(valueConditionType, other.valueConditionType)
+ && Objects.equals(minValue, other.minValue)
+ && Objects.equals(maxValue, other.maxValue)
+ && Objects.equals(defaultValue, other.defaultValue);
}
@Override
public int hashCode() {
- return Objects.hash(borrowerCycleNumber, minValue, maxValue);
+ return Objects.hash(loanProduct, borrowerCycleNumber, paramType, valueConditionType, minValue, maxValue, defaultValue);
}
public void copy(final LoanProductBorrowerCycleVariations borrowerCycleVariations) {
@@ -132,5 +120,4 @@ public class LoanProductBorrowerCycleVariations extends AbstractPersistableCusto
public BigDecimal getDefaultValue() {
return this.defaultValue;
}
-
}
\ No newline at end of file
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductConfigurableAttributes.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductConfigurableAttributes.java
index 4da1efc..85f319b 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductConfigurableAttributes.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProductConfigurableAttributes.java
@@ -18,8 +18,6 @@
*/
package org.apache.fineract.portfolio.loanproduct.domain;
-import org.apache.commons.lang.builder.EqualsBuilder;
-import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.domain.AbstractPersistableCustom;
import org.apache.fineract.portfolio.loanproduct.LoanProductConstants;
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/self/loanaccount/data/SelfLoansDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/self/loanaccount/data/SelfLoansDataValidator.java
index 133e54f..2e3e6db 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/self/loanaccount/data/SelfLoansDataValidator.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/self/loanaccount/data/SelfLoansDataValidator.java
@@ -40,8 +40,6 @@ import org.springframework.stereotype.Component;
import com.google.gson.JsonElement;
-import net.sf.ehcache.util.FindBugsSuppressWarnings;
-
@Component
public class SelfLoansDataValidator {
private static final Set<String> allowedAssociationParameters = new HashSet<>(
@@ -85,9 +83,6 @@ public class SelfLoansDataValidator {
}
- // TODO fix this!
- @FindBugsSuppressWarnings("EC_UNRELATED_TYPES")
- // See review here: https://github.com/apache/fineract/pull/670
public HashMap<String, Object> validateLoanApplication(final String json){
if (StringUtils.isBlank(json)) { throw new InvalidJsonException(); }
@@ -98,7 +93,7 @@ public class SelfLoansDataValidator {
final String loanTypeParameterName = "loanType";
final String loanTypeStr = this.fromApiJsonHelper.extractStringNamed(loanTypeParameterName, element);
- baseDataValidator.reset().parameter(loanTypeParameterName).value(loanTypeStr).notNull().equals("individual");
+ baseDataValidator.reset().parameter(loanTypeParameterName).value(loanTypeStr).notNull().isOneOfTheseStringValues("individual");
final String clientIdParameterName = "clientId";
final String clientId = this.fromApiJsonHelper.extractStringNamed(clientIdParameterName, element);
@@ -110,12 +105,8 @@ public class SelfLoansDataValidator {
retAttr.put("clientId", Long.parseLong(clientId));
return retAttr;
-
}
- // TODO fix this!
- @FindBugsSuppressWarnings("EC_UNRELATED_TYPES")
- // See review here: https://github.com/apache/fineract/pull/670/files/0409af3903d350afe43ef4837e4d915ccbe14285#r357920937
public HashMap<String, Object> validateModifyLoanApplication(final String json) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors).resource("loan");
@@ -125,7 +116,7 @@ public class SelfLoansDataValidator {
final String loanTypeParameterName = "loanType";
if(this.fromApiJsonHelper.parameterExists(loanTypeParameterName, element)){
final String loanTypeStr = this.fromApiJsonHelper.extractStringNamed(loanTypeParameterName, element);
- baseDataValidator.reset().parameter(loanTypeParameterName).value(loanTypeStr).notNull().equals("individual");
+ baseDataValidator.reset().parameter(loanTypeParameterName).value(loanTypeStr).notNull().isOneOfTheseStringValues("individual");
}
final String clientIdParameterName = "clientId";
@@ -159,6 +150,4 @@ public class SelfLoansDataValidator {
unsupportedParams.add("template");
}
}
-
-
}
diff --git a/fineract-provider/src/test/java/org/apache/fineract/accounting/common/AccountingConstantsTest.java b/fineract-provider/src/test/java/org/apache/fineract/accounting/common/AccountingConstantsTest.java
new file mode 100644
index 0000000..21d81f9
--- /dev/null
+++ b/fineract-provider/src/test/java/org/apache/fineract/accounting/common/AccountingConstantsTest.java
@@ -0,0 +1,30 @@
+/**
+ * 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.accounting.common;
+
+import org.junit.Test;
+import org.springframework.util.Assert;
+
+public class AccountingConstantsTest {
+
+ @Test
+ public void testGetAllFinancialActivities() {
+ Assert.notEmpty(AccountingConstants.FINANCIAL_ACTIVITY.getAllFinancialActivities(), "static initialization of collection of all enums is broken");
+ }
+}