vorburger commented on a change in pull request #1079:
URL: https://github.com/apache/fineract/pull/1079#discussion_r446302765
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/PlatformDataIntegrityException.java
##########
@@ -30,18 +33,39 @@
public PlatformDataIntegrityException(final String
globalisationMessageCode, final String defaultUserMessage,
final Object... defaultUserMessageArgs) {
+ super(findThrowableCause(defaultUserMessageArgs));
this.globalisationMessageCode = globalisationMessageCode;
this.defaultUserMessage = defaultUserMessage;
this.parameterName = null;
- this.defaultUserMessageArgs = defaultUserMessageArgs;
+ this.defaultUserMessageArgs =
filterThrowableCause(defaultUserMessageArgs);
}
public PlatformDataIntegrityException(final String
globalisationMessageCode, final String defaultUserMessage,
final String parameterName, final Object...
defaultUserMessageArgs) {
+ super(findThrowableCause(defaultUserMessageArgs));
this.globalisationMessageCode = globalisationMessageCode;
this.defaultUserMessage = defaultUserMessage;
this.parameterName = parameterName;
- this.defaultUserMessageArgs = defaultUserMessageArgs;
+ this.defaultUserMessageArgs =
filterThrowableCause(defaultUserMessageArgs);
+ }
+
+ private static Throwable findThrowableCause(Object[]
defaultUserMessageArgs) {
Review comment:
copy pasting (my) `findThrowableCause()` & `filterThrowableCause()`
seems like a shame... always try to avoid copy/paste, as much as you can. How
about introducing a new e.g. `public abstract class AbstractPlatformException
extends RuntimeException` with suitable protected constructors, and these two
methods as protected static methods?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/organisation/provisioning/exception/ProvisioningCriteriaNotFoundException.java
##########
@@ -19,10 +19,15 @@
package org.apache.fineract.organisation.provisioning.exception;
import
org.apache.fineract.infrastructure.core.exception.AbstractPlatformResourceNotFoundException;
+import org.springframework.dao.EmptyResultDataAccessException;
public class ProvisioningCriteriaNotFoundException extends
AbstractPlatformResourceNotFoundException {
public ProvisioningCriteriaNotFoundException(final Long id) {
super("error.msg.provisioning.criteria.id.invalid", "Provisioning
Criteria with identifier " + id + " does not exist", id);
}
+
+ public ProvisioningCriteriaNotFoundException(Long id,
EmptyResultDataAccessException e) {
+ super("error.msg.provisioning.criteria.id.invalid", "Provisioning
Criteria with identifier " + id + " does not exist", id);
Review comment:
you forgot actually propagating the cause here... ;-)
```suggestion
super("error.msg.provisioning.criteria.id.invalid", "Provisioning
Criteria with identifier " + id + " does not exist", id, e);
```
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java
##########
@@ -357,9 +357,8 @@ private void handleDataIntegrityIssues(final JsonCommand
command, final Throwabl
throw new
PlatformDataIntegrityException("error.msg.product.loan.duplicate.short.name",
"Loan product with short name `" + shortName + "` already
exists", "shortName", shortName);
} else if (realCause.getMessage().contains("Duplicate entry")) {
- final Object[] args = null;
throw new
PlatformDataIntegrityException("error.msg.product.loan.duplicate.charge",
- "Loan product may only have one charge of each type.`",
"charges", args);
+ "Loan product may only have one charge of each type.`",
"charges");
Review comment:
I'm not 100% sure and haven't looked much at this, but could it make
sense to:
```suggestion
"Loan product may only have one charge of each type.`",
"charges", realCause);
```
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/PlatformApiDataValidationException.java
##########
@@ -43,6 +43,30 @@ public PlatformApiDataValidationException(final String
globalisationMessageCode,
this.errors = errors;
}
+ public PlatformApiDataValidationException(final String
globalisationMessageCode, final String defaultUserMessage,
+ final List<ApiParameterError> errors, final Object...
defaultUserMessageArgs) {
Review comment:
In this case, where there is no `Object... defaultUserMessageArgs` in
the original far, you should not introduce it. Just have constructors which
take an additional last new argument like `, Throwable cause)` - makes sense?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
