The way the DomainClassConverter acts is unclear to me. The documentation states that it can be used to have @PathParameter directly being looked up instead of manually doing it.

So the following code inside of a controller works perfectly:

@GetMapping(path = "/{entityId}")
public void get(@PathVariable("entityId") Entity myEntity)

However, when I provide an entityId that does not exist, spring responds with 500 stating that the entityId parameter has not been provided. Which is not true, it has been provided the DCC could not find an entity to that value. So the correct response should be a 404.

I came across this issue https://github.com/spring-projects/spring-framework/issues/26296 where two solutions were provided and additionally added to the documentation:

  1. Configure the PathVariable setting required=false
    • In my case, I want this to be required, so that wouldn't be an option. The only way would be to check for null and trigger a 404 response manually however that would also cause a 404 response if the entityId is not present in the request.
  2. Handle the MissingPathVariableException to return a 404
  3. In my opinion this is wrong because the absence of a required path variable should not result in a 404 but rather a 400 response since the request was malformed.

As of what I've found out, there is no way to trigger a 404 without using any of the two above solutions. The expected behavior that I imagine (for the above code) would look like this:

Request Expected Response Controller Invocation
GET / 400 Bad Request :x:
GET /not-existing 404 Not Found :x:
GET /existing 200 OK get(Entity)

Additionally for optional path variables (i.e. @PathVariable(required = false)):

Request Expected Response Controller Invocation
GET / 200 OK get(null)
GET /not-existing 404 Not Found :x:
GET /existing 200 OK get(Entity)

Since changing the behavior would probably be considered a breaking change one less breaking solution could be to allow the DCC to throw certain exceptions that would then not get interpreted as conversion failures and simply passed through (so users can override the DCC to throw EntityNotFoundException instead of it returning null)

Comment From: snicoll

@Floppy012 DomainClassConverter is part of Spring Data REST. Did you mean to report this against https://github.com/spring-projects/spring-data-rest ?

Comment From: Floppy012

@snicoll

@Floppy012 DomainClassConverter is part of Spring Data REST. Did you mean to report this against https://github.com/spring-projects/spring-data-rest ?

The mentioned behavior is caused by ~~spring-core (I believe)~~ spring-web as a result of the DCC returning null. I don't think that a change in the DCC alone would be enough.

Edit: It's caused by this line https://github.com/spring-projects/spring-framework/blob/899de4f3bfc373bdf1c0e67c7160331bd056825b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java#L143

Comment From: rstoyanchev

Thanks for the report.

The conversion mechanism in spring-core is pretty low level, and I don't think it's in a good position to make decisions about what exceptions mean. In this case a converter with a higher level concern that should also be handled externally.

It would make more sense to have an @ExceptionHandler to handle ConversionFailedException and check to see whether it's related to a Spring Data managed domain class as suggested in https://github.com/spring-projects/spring-data-commons/issues/1043#issuecomment-752399704.

You can also handle MissingPathVariableException which exposes MethodParameter and that can also be used to check if it is about a Spring Data domain class.

Comment From: Floppy012

It would make more sense to have an @ExceptionHandler to handle ConversionFailedException and check to see whether it's related to a Spring Data managed domain class as suggested in spring-projects/spring-data-commons#1043 (comment).

That would still mean I have to override the DCC to implement a null check and throw an exception. In that case, it should be documented somewhere. It still feels like a workaround just to have spring no longer return a MissingPathVariableException for a path variable that has been provided.

You can also handle MissingPathVariableException which exposes MethodParameter and that can also be used to check if it is about a Spring Data domain class.

I haven't tried that yet, but would I also be able to find out why the exception was triggered? If it comes from the DCC returning null but the path variable has been provided it should be a Bad Request. If the path variable has not been provided the MissingPathVariableException is justified.

Comment From: rstoyanchev

That would still mean I have to override the DCC to implement a null check and throw an exception.

True, yes.

I haven't tried that yet, but would I also be able to find out why the exception was triggered? If it comes from the DCC returning null but the path variable has been provided it should be a Bad Request. If the path variable has not been provided the MissingPathVariableException is justified.

MissingPathVariableException has a flag missingAfterConversion to recognize a value that is missing from a value that was present but converted to null. Generally speaking a path variable cannot be completely missing or the path would not match the mapping by number of path segments.

Comment From: quaff

@Floppy012 Spring will send 400 instead of 500 since 6.0.14, see https://github.com/spring-projects/spring-framework/issues/31382. I think it's arguable that maybe 404 is more proper than 400.

Comment From: Floppy012

@Floppy012 Spring will send 400 instead of 500 since 6.0.14, see #31382. I think it's arguable that maybe 404 is more proper than 400.

Thanks for the information.