You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/06/12 16:57:22 UTC
[GitHub] [fineract] percyashu opened a new pull request, #2355: FINERACT-1483: Optional value should only be accessed after calling i…
percyashu opened a new pull request, #2355:
URL: https://github.com/apache/fineract/pull/2355
[FINERACT-1483](https://issues.apache.org/jira/browse/FINERACT-1483)
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2355: FINERACT-1483: Optional value should only be accessed after calling isPresent()
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2355:
URL: https://github.com/apache/fineract/pull/2355#discussion_r895933966
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -777,11 +777,11 @@ public void createCashBasedJournalEntriesAndReversalsForSavingsCharges(final Off
}
public LoanTransaction getLoanTransactionById(final long loanTransactionId) {
- return this.loanTransactionRepository.findById(loanTransactionId).get();
+ return this.loanTransactionRepository.getById(loanTransactionId);
Review Comment:
findById and getById is fundamentally different.
The first one is returning an actual entity object if found while the other one (getById) is returning only a reference to the object.
You can imagine the 2 by comparing:
- EntityManager#find
- EntityManager#getReference
The latter one is only a lazy reference to the object and will be loaded upon access (at least for most JPA providers).
I'd say let's not mess around the references so to simply get rid of the sonar issues, let's go with `this.loanTransactionRepository.findById(loanTransactionId).orElse(null)`
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -777,11 +777,11 @@ public void createCashBasedJournalEntriesAndReversalsForSavingsCharges(final Off
}
public LoanTransaction getLoanTransactionById(final long loanTransactionId) {
- return this.loanTransactionRepository.findById(loanTransactionId).get();
+ return this.loanTransactionRepository.getById(loanTransactionId);
}
public SavingsAccountTransaction getSavingsTransactionById(final long savingsTransactionId) {
- return this.savingsAccountTransactionRepository.findById(savingsTransactionId).get();
+ return this.savingsAccountTransactionRepository.getById(savingsTransactionId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/NotificationEventListener.java:
##########
@@ -52,7 +52,7 @@ public void receive(NotificationData notificationData) {
if (notificationData.getOfficeId() != null) {
List<Long> tempUserIds = new ArrayList<>(userIds);
for (Long userId : tempUserIds) {
- AppUser appUser = appUserRepository.findById(userId).get();
+ AppUser appUser = appUserRepository.getById(userId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainServiceJpa.java:
##########
@@ -115,13 +115,13 @@ public boolean isConstraintApproachEnabledForDatatables() {
@Override
public boolean isEhcacheEnabled() {
- return this.cacheTypeRepository.findById(1L).get().isEhcacheEnabled();
+ return this.cacheTypeRepository.getById(1L).isEhcacheEnabled();
}
@Transactional
@Override
public void updateCache(final CacheType cacheType) {
- final PlatformCache cache = this.cacheTypeRepository.findById(1L).get();
+ final PlatformCache cache = this.cacheTypeRepository.getById(1L);
Review Comment:
This should be something similar to this:
```
this.cacheTypeRepository.findById(1L).ifPresent(cache -> {
cache.update(cacheType);
this.cacheTypeRepository.save(cache);
}
```
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/StandingInstructionWritePlatformServiceImpl.java:
##########
@@ -183,7 +183,7 @@ public CommandProcessingResult update(final Long id, final JsonCommand command)
@Override
public CommandProcessingResult delete(final Long id) {
- AccountTransferStandingInstruction standingInstructionsForUpdate = this.standingInstructionRepository.findById(id).get();
+ AccountTransferStandingInstruction standingInstructionsForUpdate = this.standingInstructionRepository.getById(id);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -376,14 +376,14 @@ public CommandProcessingResult deposit(final Long savingsId, final JsonCommand c
LOG.debug("Deposit account has been created: {} ", deposit);
- GroupSavingsIndividualMonitoring gsim = gsimRepository.findById(account.getGsim().getId()).get();
+ GroupSavingsIndividualMonitoring gsim = gsimRepository.getById(account.getGsim().getId());
LOG.info("parent deposit : {} ", gsim.getParentDeposit());
LOG.info("child account : {} ", savingsId);
BigDecimal currentBalance = gsim.getParentDeposit();
BigDecimal newBalance = currentBalance.add(transactionAmount);
gsim.setParentDeposit(newBalance);
gsimRepository.save(gsim);
- LOG.info("balance after making deposit : {} ", gsimRepository.findById(account.getGsim().getId()).get().getParentDeposit());
+ LOG.info("balance after making deposit : {} ", gsimRepository.getById(account.getGsim().getId()).getParentDeposit());
Review Comment:
This is anyway a bad pattern because we're making a database query just to log something.
What if the logger is not even configured to info level but above, like warn? Then the logger won't log anything but we still make the db call.
In those cases it's wise to wrap the log message into a level check like:
```
if (LOG.isInfoEnabled()) {
// do the expensive logging
}
```
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -939,7 +939,7 @@ private void checkClientOrGroupActive(final SavingsAccount account) {
public CommandProcessingResult bulkGSIMClose(final Long gsimId, final JsonCommand command) {
final Long parentSavingId = gsimId;
- GroupSavingsIndividualMonitoring parentSavings = gsimRepository.findById(parentSavingId).get();
+ GroupSavingsIndividualMonitoring parentSavings = gsimRepository.getById(parentSavingId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/creditbureau/service/CreditReportWritePlatformServiceImpl.java:
##########
@@ -146,20 +140,24 @@ public String addCreditReport(Long bureauId, File creditReport, FormDataContentD
}
- private Optional<String> getCreditBureau(Long creditBureauID) {
+ private String getCreditBureau(Long creditBureauID) {
if (creditBureauID != null) {
Optional<CreditBureau> creditBureau = this.creditBureauRepository.findById(creditBureauID);
if (creditBureau.isEmpty()) {
- return Optional.empty();
+ baseDataValidator.reset().failWithCode("creditBureau.has.not.been.Integrated");
Review Comment:
You could swap the condition and get rid of this exceptional case.
So it could be
```
if (!creditBureau.isEmpty()) {
return creditBureau.get().getName()
}
```
And the later exception throw logic will catch the error cases anyway.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainServiceJpa.java:
##########
@@ -115,13 +115,13 @@ public boolean isConstraintApproachEnabledForDatatables() {
@Override
public boolean isEhcacheEnabled() {
- return this.cacheTypeRepository.findById(1L).get().isEhcacheEnabled();
+ return this.cacheTypeRepository.getById(1L).isEhcacheEnabled();
Review Comment:
This could be like this:
`this.cacheTypeRepository.findById(1L).map(PlatformCache::isEhcacheEnabled).orElse(false)`
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -83,7 +83,7 @@ public Long notify(Collection<Long> userIds, String objectType, Long objectId, S
private List<Long> insertIntoNotificationMapper(Collection<Long> userIds, Long generatedNotificationId) {
List<Long> mappedIds = new ArrayList<>();
for (Long userId : userIds) {
- AppUser appUser = this.appUserRepository.findById(userId).get();
+ AppUser appUser = this.appUserRepository.getById(userId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -924,7 +924,7 @@ private void createDebitJournalEntryForSavings(final Office office, final String
String modifiedTransactionId = transactionId;
if (StringUtils.isNumeric(transactionId)) {
long id = Long.parseLong(transactionId);
- savingsAccountTransaction = this.savingsAccountTransactionRepository.findById(id).get();
+ savingsAccountTransaction = this.savingsAccountTransactionRepository.getById(id);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -210,11 +210,11 @@ private Collection<LoanProductProvisioningEntry> generateLoanProvisioningEntry(P
.retrieveLoanProductsProvisioningData(date);
Map<Integer, LoanProductProvisioningEntry> provisioningEntries = new HashMap<>();
for (LoanProductProvisioningEntryData data : entries) {
- LoanProduct loanProduct = this.loanProductRepository.findById(data.getProductId()).get();
+ LoanProduct loanProduct = this.loanProductRepository.getById(data.getProductId());
Office office = this.officeRepositoryWrapper.findOneWithNotFoundDetection(data.getOfficeId());
- ProvisioningCategory provisioningCategory = provisioningCategoryRepository.findById(data.getCategoryId()).get();
- GLAccount liabilityAccount = glAccountRepository.findById(data.getLiablityAccount()).get();
- GLAccount expenseAccount = glAccountRepository.findById(data.getExpenseAccount()).get();
+ ProvisioningCategory provisioningCategory = provisioningCategoryRepository.getById(data.getCategoryId());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanApplicationWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1498,7 +1498,7 @@ public CommandProcessingResult undoGLIMLoanApplicationApproval(final Long loanId
// GroupLoanIndividualMonitoringAccount
// glimAccount=glimRepository.findOne(loanId);
final Long parentLoanId = loanId;
- GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.findById(parentLoanId).get();
+ GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.getById(parentLoanId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -833,7 +833,7 @@ private void createCreditJournalEntryForSavings(final Office office, final Strin
String modifiedTransactionId = transactionId;
if (StringUtils.isNumeric(transactionId)) {
long id = Long.parseLong(transactionId);
- savingsAccountTransaction = this.savingsAccountTransactionRepository.findById(id).get();
+ savingsAccountTransaction = this.savingsAccountTransactionRepository.getById(id);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -904,7 +904,7 @@ private void createDebitJournalEntryForLoan(final Office office, final String cu
String modifiedTransactionId = transactionId;
if (StringUtils.isNumeric(transactionId)) {
long id = Long.parseLong(transactionId);
- loanTransaction = this.loanTransactionRepository.findById(id).get();
+ loanTransaction = this.loanTransactionRepository.getById(id);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -440,7 +440,7 @@ public Long transferFunds(final AccountTransferDTO accountTransferDTO) {
// if the savings account is GSIM, update its parent as well
if (toSavingsAccount.getGsim() != null) {
- GroupSavingsIndividualMonitoring gsim = gsimRepository.findById(toSavingsAccount.getGsim().getId()).get();
+ GroupSavingsIndividualMonitoring gsim = gsimRepository.getById(toSavingsAccount.getGsim().getId());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -210,11 +210,11 @@ private Collection<LoanProductProvisioningEntry> generateLoanProvisioningEntry(P
.retrieveLoanProductsProvisioningData(date);
Map<Integer, LoanProductProvisioningEntry> provisioningEntries = new HashMap<>();
for (LoanProductProvisioningEntryData data : entries) {
- LoanProduct loanProduct = this.loanProductRepository.findById(data.getProductId()).get();
+ LoanProduct loanProduct = this.loanProductRepository.getById(data.getProductId());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -341,7 +341,7 @@ private LoanLifecycleStateMachine defaultLoanLifecycleStateMachine() {
@Override
public CommandProcessingResult disburseGLIMLoan(final Long loanId, final JsonCommand command) {
final Long parentLoanId = loanId;
- GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.findById(parentLoanId).get();
+ GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.getById(parentLoanId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -854,7 +854,7 @@ private void createCreditJournalEntryForLoan(final Office office, final String c
String modifiedTransactionId = transactionId;
if (StringUtils.isNumeric(transactionId)) {
long id = Long.parseLong(transactionId);
- loanTransaction = this.loanTransactionRepository.findById(id).get();
+ loanTransaction = this.loanTransactionRepository.getById(id);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanApplicationWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1577,7 +1577,7 @@ public CommandProcessingResult rejectGLIMApplicationApproval(final Long glimId,
// GroupLoanIndividualMonitoringAccount
// glimAccount=glimRepository.findOne(loanId);
final Long parentLoanId = glimId;
- GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.findById(parentLoanId).get();
+ GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.getById(parentLoanId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanApplicationWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1338,7 +1338,7 @@ public void validateMultiDisbursementData(final JsonCommand command, LocalDate e
public CommandProcessingResult approveGLIMLoanAppication(final Long loanId, final JsonCommand command) {
final Long parentLoanId = loanId;
- GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.findById(parentLoanId).get();
+ GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.getById(parentLoanId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -938,7 +938,7 @@ public CommandProcessingResult makeGLIMLoanRepayment(final Long loanId, final Js
final Long parentLoanId = loanId;
- glimRepository.findById(parentLoanId).get();
+ glimRepository.getById(parentLoanId);
Review Comment:
I think the intention here was to actually fail the execution in case the parentLoanId is not found but with this refactoring we're losing that.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -848,7 +848,7 @@ public CommandProcessingResult undoGLIMLoanDisbursal(final Long loanId, final Js
// GroupLoanIndividualMonitoringAccount
// glimAccount=glimRepository.findOne(loanId);
final Long parentLoanId = loanId;
- GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.findById(parentLoanId).get();
+ GroupLoanIndividualMonitoringAccount parentLoan = glimRepository.getById(parentLoanId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/meeting/service/MeetingWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -164,7 +164,7 @@ private CalendarInstance getCalendarInstance(final JsonCommand command) {
/*
* If group is within a center then center entityType should be passed for retrieving CalendarInstance.
*/
- final Group group = this.groupRepository.findById(entityId).get();
+ final Group group = this.groupRepository.getById(entityId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -376,14 +376,14 @@ public CommandProcessingResult deposit(final Long savingsId, final JsonCommand c
LOG.debug("Deposit account has been created: {} ", deposit);
- GroupSavingsIndividualMonitoring gsim = gsimRepository.findById(account.getGsim().getId()).get();
+ GroupSavingsIndividualMonitoring gsim = gsimRepository.getById(account.getGsim().getId());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountAssembler.java:
##########
@@ -432,7 +432,7 @@ public SavingsAccount assembleFrom(final Client client, final Group group, final
}
accountType = AccountType.JLG;
}
- final SavingsProduct product = this.savingProductRepository.findById(productId).get();
+ final SavingsProduct product = this.savingProductRepository.getById(productId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -376,14 +376,14 @@ public CommandProcessingResult deposit(final Long savingsId, final JsonCommand c
LOG.debug("Deposit account has been created: {} ", deposit);
- GroupSavingsIndividualMonitoring gsim = gsimRepository.findById(account.getGsim().getId()).get();
+ GroupSavingsIndividualMonitoring gsim = gsimRepository.getById(account.getGsim().getId());
LOG.info("parent deposit : {} ", gsim.getParentDeposit());
LOG.info("child account : {} ", savingsId);
BigDecimal currentBalance = gsim.getParentDeposit();
BigDecimal newBalance = currentBalance.add(transactionAmount);
gsim.setParentDeposit(newBalance);
gsimRepository.save(gsim);
- LOG.info("balance after making deposit : {} ", gsimRepository.findById(account.getGsim().getId()).get().getParentDeposit());
+ LOG.info("balance after making deposit : {} ", gsimRepository.getById(account.getGsim().getId()).getParentDeposit());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -448,7 +448,7 @@ public CommandProcessingResult withdrawal(final Long savingsId, final JsonComman
transactionAmount, paymentDetail, transactionBooleanValues, backdatedTxnsAllowedTill);
if (isGsim && (withdrawal.getId() != null)) {
- GroupSavingsIndividualMonitoring gsim = gsimRepository.findById(account.getGsim().getId()).get();
+ GroupSavingsIndividualMonitoring gsim = gsimRepository.getById(account.getGsim().getId());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -225,7 +225,7 @@ public SavingsAccountWritePlatformServiceJpaRepositoryImpl(final PlatformSecurit
public CommandProcessingResult gsimActivate(final Long gsimId, final JsonCommand command) {
Long parentSavingId = gsimId;
- GroupSavingsIndividualMonitoring parentSavings = gsimRepository.findById(parentSavingId).get();
+ GroupSavingsIndividualMonitoring parentSavings = gsimRepository.getById(parentSavingId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/creditbureau/service/CreditReportWritePlatformServiceImpl.java:
##########
@@ -146,20 +140,24 @@ public String addCreditReport(Long bureauId, File creditReport, FormDataContentD
}
- private Optional<String> getCreditBureau(Long creditBureauID) {
+ private String getCreditBureau(Long creditBureauID) {
Review Comment:
Let's rename the method. It's not returning a CreditBureau object but only its name. Should be `getCreditBureauName`
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -612,7 +612,7 @@ public CommandProcessingResult undoApplicationApproval(final Long savingsId, fin
public CommandProcessingResult rejectGSIMApplication(final Long gsimId, final JsonCommand command) {
final Long parentSavingId = gsimId;
- GroupSavingsIndividualMonitoring parentSavings = gsimRepository.findById(parentSavingId).get();
+ GroupSavingsIndividualMonitoring parentSavings = gsimRepository.getById(parentSavingId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -552,7 +552,7 @@ public CommandProcessingResult approveApplication(final Long savingsId, final Js
@Override
public CommandProcessingResult undoGSIMApplicationApproval(final Long gsimId, final JsonCommand command) {
final Long parentSavingId = gsimId;
- GroupSavingsIndividualMonitoring parentSavings = gsimRepository.findById(parentSavingId).get();
+ GroupSavingsIndividualMonitoring parentSavings = gsimRepository.getById(parentSavingId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountSchedularServiceImpl.java:
##########
@@ -49,7 +49,7 @@ public ShareAccountSchedularServiceImpl(final ShareAccountDividendRepository sha
@Transactional
public void postDividend(final Long dividendDetailId, final Long savingsId) {
- ShareAccountDividendDetails shareAccountDividendDetails = this.shareAccountDividendRepository.findById(dividendDetailId).get();
+ ShareAccountDividendDetails shareAccountDividendDetails = this.shareAccountDividendRepository.getById(dividendDetailId);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -488,7 +488,7 @@ public CommandProcessingResult approveGSIMApplication(final Long gsimId, final J
// GroupLoanIndividualMonitoringAccount
// glimAccount=glimRepository.findOne(loanId);
Long parentSavingId = gsimId;
- GroupSavingsIndividualMonitoring parentSavings = gsimRepository.findById(parentSavingId).get();
+ GroupSavingsIndividualMonitoring parentSavings = gsimRepository.getById(parentSavingId);
Review Comment:
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] percyashu commented on a diff in pull request #2355: FINERACT-1483: Optional value should only be accessed after calling isPresent()
Posted by GitBox <gi...@apache.org>.
percyashu commented on code in PR #2355:
URL: https://github.com/apache/fineract/pull/2355#discussion_r896120353
##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java:
##########
@@ -777,11 +777,11 @@ public void createCashBasedJournalEntriesAndReversalsForSavingsCharges(final Off
}
public LoanTransaction getLoanTransactionById(final long loanTransactionId) {
- return this.loanTransactionRepository.findById(loanTransactionId).get();
+ return this.loanTransactionRepository.getById(loanTransactionId);
Review Comment:
Interesting, will make the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] percyashu commented on a diff in pull request #2355: FINERACT-1483: Optional value should only be accessed after calling isPresent()
Posted by GitBox <gi...@apache.org>.
percyashu commented on code in PR #2355:
URL: https://github.com/apache/fineract/pull/2355#discussion_r897131780
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -938,7 +938,7 @@ public CommandProcessingResult makeGLIMLoanRepayment(final Long loanId, final Js
final Long parentLoanId = loanId;
- glimRepository.findById(parentLoanId).get();
+ glimRepository.findById(parentLoanId).orElse(null);
Review Comment:
I think instead of introducing null checks, we can directly throw an exception as shown below
`glimRepository.findById(parentLoanId).orElseThrow(NoSuchElementException::new);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2355: FINERACT-1483: Optional value should only be accessed after calling isPresent()
Posted by GitBox <gi...@apache.org>.
galovics merged PR #2355:
URL: https://github.com/apache/fineract/pull/2355
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2355: FINERACT-1483: Optional value should only be accessed after calling isPresent()
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2355:
URL: https://github.com/apache/fineract/pull/2355#discussion_r896465426
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -938,7 +938,7 @@ public CommandProcessingResult makeGLIMLoanRepayment(final Long loanId, final Js
final Long parentLoanId = loanId;
- glimRepository.findById(parentLoanId).get();
+ glimRepository.findById(parentLoanId).orElse(null);
Review Comment:
This is not gonna fly. This rather needs to be an existence check.
If you wanna keep it that way, extract the result it to a variable and do a null-check to make sure the code fails in case the parent loan doesn't exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org