1

For my file (csv) upload endpoint, I check the file type using a method in my CsvHelper class:

private static String[] TYPES =  {"text/csv", "application/vnd.ms-excel"};

public static boolean hasCsvFormat(MultipartFile file) {
    return Arrays.stream(TYPES).anyMatch(file.getContentType()::equals);
}

And call it from service as shown below:

public void create(MultipartFile file) throws Exception {
    if (!CsvHelper.hasCsvFormat(file)) {
        throw new NotValidFormatException(...);
    }
    // ...
}

I created a custom exception for this called NotValidFormatException using @ControllerAdvice, but I am not sure if it is the most proper way for this.

My questions:

1. Should I create custom exception or custom validator as mentioned on this? But as I have not a model field, I need to use @Valid on the request and not sure if I can use that approach to verify file type (by calling my hasCsvFormat() method.

2. I used this approach for creating custom exception handling. If I wanted to use that approach for this scenario, should I create a separate class (e.g. NotValidFormatException) like NoSuchElementFoundException on that example? Or should I include a new exception method to the GlobalExceptionHandler class as a common exception type?

1 Answers1

0

Based on your requirement, option number 2 is more suitable from my perspective.

You can create separate class for NotValidFormatException , annotate with ResponseStatus of your need & extend it with RuntimeException.

Now, your GlobalExceptionHandler is general purpose handler which perform actions once specific exception is thrown.

So you have to create method which should annotate with @ExceptionHandler(NotValidFormatException.class) in GlobalExceptionHandler .

Benefits of having separate class for NotValidFormatException is you can customize error message, can perform more operation within methods of that class.

If your requirement is minimal(like logging & returning response etc.) , then I would suggest to have only single ExceptionHandler method in GlobalExceptionHandler which will handle most of exceptions i.e. logging & returning response.

Ashish Patil
  • 4,428
  • 1
  • 15
  • 36
  • Perfect explanations! I am just wondering these issues, could you please clarify me about them? >>> –  Jul 18 '22 at 06:48
  • **1.** In this scene, when following second approach, I handle exceptions using `GlobalExceptionHandler` and if I want to customize any of them I add a separate exception class e.g. `NoSuchElementFoundException` and a handle method corresponding for this class e.g. `handleNoSuchElementFoundException`. IS that true? –  Jul 18 '22 at 06:50
  • **2.** For validation, I think the first approach is better, but not sure why people prefer the second one as I have seen for similar situations before. Of course it is also meaningful to throw `NotValidFormatException`, but for validation e.g. "valid e-mail", I would prefer validation instead of throving "not valid email exception". Am I wrong? Is it just related to preference or convention? –  Jul 18 '22 at 06:55
  • **3.** What about using [this](https://gist.github.com/susimsek/03b6a4d695b864dfe95d1b31959b3e53) approach? I also tried to use it, but cannot get custom message, instead I get general message. Is that approach also good for validating? And what is the problem with that approach? –  Jul 18 '22 at 07:18
  • @byte, your point 1 is correct. Regarding validation, your validation will occur at service layer & it has nothing to do with how you throw your exception & handle. – Ashish Patil Jul 18 '22 at 08:03
  • Thanks for reply, regarding to 1 and 3 I could not understand. Any clarification pls? –  Jul 18 '22 at 08:47
  • By the way, I cannot vote up as I have not enough repo :( Any vote up pls? –  Jul 18 '22 at 08:48
  • For point 1: think of `GlobalExceptionHandler` as just utility which has few handler methods for different kinds of exceptions. Your different kind of exceptions are in their own separate class . For example you defined `NoSuchElementFoundException ` in its own separate class (NoSuchElementFoundException.java etc.). – Ashish Patil Jul 18 '22 at 09:18
  • But its handler method (i.e. method which has `@ExceptionHandler(NoSuchElementFoundException.class)` annotation will be part of `GlobalExceptionHandler`. Your `NoSuchElementFoundException ` will have type of HTTP error code it will return any default custom message it should throw when this exception occurs. – Ashish Patil Jul 18 '22 at 09:18
  • Your `GlobalExceptionHandler`'s handler method will just act when this exception occurs. When I say it `just act` it means this method can wrap this exception in separate response entity & return , printing logger about this exception etc. This can vary based on your requirement. Same thing happens if you introduce new exception type, put it in separate class, define handler in `GlobalExceptionHandler`. – Ashish Patil Jul 18 '22 at 09:18
  • For point 3 : I am not sure what you mean by getting general message, as this code just sends HTTP status for `ConstraintViolationException` & `HTTP status + "Please select a file"` for `MultipartException` type. So if you need custom exception , for example whatever you throws from your service, these handler methods has to be modify & read exceptions `getMessage()` first, for example `exception.getMessage()` will gives you message which you have provided while throwing exception . – Ashish Patil Jul 18 '22 at 09:26
  • So, in order to validate if the file has a valid type, should we use exception handler instead of custom validation? Why? –  Jul 18 '22 at 15:25
  • @byte, exception handler will get invoked only when your `controlleradvice` throws error. Your custom validation method (which should be part of your your service class , should throw exception ; lets say your custom validation method should throw `NotValidFormatException`. So you still need both custom validation logic & exception handler logic. – Ashish Patil Jul 18 '22 at 16:15
  • By the way now I just had received enough repo and vote up for your helps ;) –  Jul 19 '22 at 07:51
  • Hmmm, good explanations, thanks. In this case I see that both method are ok (using valid annotation and exceptionHandler) in order to check if the file has a valid type for example and the choice is depend on the preference of the developer. Is that true? And what is the pros and cons as far as you know for these 2 approaches? Thanks in advance –  Jul 19 '22 at 07:51
  • Any reply please? –  Jul 19 '22 at 10:42
  • I didn't get you what you mean by valid annotation. – Ashish Patil Jul 20 '22 at 09:51
  • I mean using custom valid annotation e.g. @ValidImage ? –  Jul 20 '22 at 10:25
  • I am not aware of that annotation – Ashish Patil Jul 20 '22 at 10:41
  • There is not such annotation, it is custom. Just think of custom annotation please :) –  Jul 20 '22 at 10:59
  • yeah, not sure about that idea, thank you . – Ashish Patil Jul 20 '22 at 11:09