Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MissingKotlinParameterException should have a reference to KCallable #548

Closed
dirkbolte opened this issue Feb 9, 2022 · 12 comments
Closed

Comments

@dirkbolte
Copy link

dirkbolte commented Feb 9, 2022

Use case
I want to convert a parameter error to a similar response as it would have been thrown when validation would have run and using the error messages of javax.validation.constraints. To be specific:

I want to write a data class like:

data class NotNullExample(
  @field:NotNull(message = "{my.translated.message}")
  val notNullProperty: String
)

When making the variable nullable, I would get the message translated. With the definition above, it never gets to validation and throws a MissingKotlinParameterException instead. I can catch an process this exception, thus accessing the parameter within the exception. Unfortunately, the parameter type's annotation list is empty (probably because the annotation is associated to the field) - which is required for all other validations as well. And KParameter does not provide access to the callable (although KParameterImpl has it).

So what I want to achieve is:

  1. get the callable that fails
  2. to get the return type
  3. to get the field the parameter is for
  4. to get the field annotation
  5. to get the annotation message
  6. to resolve the translated message

Describe the solution you'd like

If I can get

class MissingKotlinParameterException(
  val callable: KCallable, // <-- new
  val parameter: KParameter,
  processor: JsonParser? = null,
  msg: String
) : MismatchedInputException(processor, msg)

I can already proceed.

Describe alternatives you've considered

As a workaround, I will try to fetch the callable of Parameter via reflection. Getting the actual type might not work with other construction types and callable should be available already.

@k163377
Copy link
Contributor

k163377 commented Mar 14, 2023

There didn't seem to be a good reason to implement this in jackson-module-kotlin.
Personally, I think that for such use case, a class should be created for bean validation with adjusted nullability, etc.

As a side note, MissingKotlinParameterException is deprecated due to the problems summarized in #617.

@dirkbolte-jb10x
Copy link

Not sure I understand you correctly. Do you suggest to create a class just for validation purposes with nullable members?

@k163377
Copy link
Contributor

k163377 commented Mar 14, 2023

Do you suggest to create a class just for validation purposes with nullable members?

Yes, it is.

There is no good reason for jackson-module-kotlin to consider BeanValidation.
They are independent.
Any effort to use BeanValidation should basically be done by the user.
The conflict with the more important issue #627 is another reason why it is difficult to incorporate this change.

@dirkbolte
Copy link
Author

(sry, different account)

I completely understand that parsing and bean validation are separate concerns and I didn't ask for bringing the validation concern into this module.
Making the suggested user efforts for BeanValidation easier by exposing the KCallable Making development of bean validation easier by exposing the KCallable would be very beneficial in this.

I would be happy to help developing a patch on the (whether it would be on the deprecated approach or the new one) if the overall idea is considered ok.

@k163377
Copy link
Contributor

k163377 commented Mar 15, 2023

I have considered it, but still find it difficult to accept this change.

As pointed out in #572, Exception needs to be Serializable.
This assumption requires a very complex and difficult-to-maintain implementation.

In combination with the aforementioned points, the likelihood of incorporating this change as jackson-module-kotlin is small, so this issue is closed.

@k163377 k163377 closed this as completed Mar 15, 2023
@cowtowncoder
Copy link
Member

One possibility might be to have additional value marked as transient, if serialization of that particular thing is not needed -- it is only needed during processing on server. That would retain serializability of exception.

@dirkbolte
Copy link
Author

Would work for my scenario. If that would be acceptable, in can try to create a proposal.

@cowtowncoder
Copy link
Member

I'll let @k163377 comment; perhaps there are bigger problems wrt feasibility. But I think proposal is fine.

If you do propose a PR, it should have a test to ensure JDK serializability (there are existing tests for other Kotlin module entities).

@k163377
Copy link
Contributor

k163377 commented Mar 17, 2023

Considering only the use case described, I don't think this method is appropriate.

First, BeanValidation validates against multiple properties simultaneously, while MissingKotlinParameterException can only handle one parameter.
There are also problems with factory functions, secondary constructors, and other cases where arguments and properties do not match.

I haven't looked into how to do this, but wouldn't it be appropriate to use the parameter constraints in this case?
https://jakarta.ee/specifications/bean-validation/3.0/jakarta-ean-validation-spec-3.0.html#constraintdeclarationvalidationprocess-methodlevelconstraints-parameterconstraints

@cowtowncoder
Copy link
Member

Reading through this again, I actually concur with @k163377. Since validation is a general concern, if integrating validation it'd probably make more sense to use exceptions from jackson-databind (or ones defined by additional validation module). But not Kotlin-specific pieces -- I think validation typically would give logical names or properties, paths, and not underlying runtime entitie.

@dirkbolte
Copy link
Author

I'm thinking about how such a module would integrate with existing solutions. Thinking of Quarkus and Spring, validation is usually coming afterwards. It might be a nice addition that kicks off all validations on the spot during parsing (and thus maybe also remove the error source of missed @Valid annotations).

@cowtowncoder
Copy link
Member

Yeah one challenge is just that validation generally operates on POJOs, from top to bottom, which sort of expects full databinding results. So works fine as post-processing after ObjectMapper.readValue() call, but much more difficult to integrate at any other point -- unless one re-implements the validator logic altogether (tbh I don't know how flexible implementations are; maybe they could do things incrementally?).

Adding a hook to do something immediately after readValue() would probably be acceptable, fwtw (maybe ValidationProvider), although not sure how much value there would be for such a thing (over caller just calling validation after getting value from readValue()).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants