fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From awa...@apache.org
Subject [fineract] branch develop updated: more SpotBugs related / inspired code clean up (see FINERACT-702)
Date Tue, 14 Jan 2020 07:37:30 GMT
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 <mike@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");
+	}
+}


Mime
View raw message