Blazebit / blaze-persistence

Rich Criteria API for JPA providers
https://persistence.blazebit.com
Apache License 2.0
742 stars 90 forks source link

Generated builders not usable in Kotlin top-level functions #1683

Open david-kubecka opened 1 year ago

david-kubecka commented 1 year ago

I'm constructing EV instances via generated builders in my tests. The instances are assigned to Kotlin top-level properties (which are compiled to a Java equivalent of a class where the properties are initialized in the static block). Using builders in this fashion leads to NPE, though.

The reason is that the code in the builders accesses the static metamodel, e.g. SimpleCatView_.name. The metamodel properties are, however, not initialized, hence the NPE. I would like to discuss why it's so and whether something can be done about that (even in some special situations).

The described behaviour is quite surprising from user PoV because generated code is expected to be "final", i.e. no further boot time initialization should be necessary.

beikov commented 1 year ago

The problem here really is the fact that we need some kind of context to do "the right thing" for e.g. collection defaults etc. Builders need the static metamodel to implement some conversion. The entity view implementations need it to create the proper collection instances for default values and also to enforce some invariants in the setter method. We could theoretically try to compute that during annotation processing time, but since we can't assume that all EVs are compiled together, this might cause other issues.

I think we could work out a way to avoid referring to the static metamodel for creating collection default instances, which would make is possible to call entity view implementation constructors without context, but the checks that happen when calling setters for subview types require the whole set of EVs to be known, which can't possibly work without context. I could be persuaded to omit the check when we detect that the EVM has not been initialized, but I'd definitely want to emit a LOG warning in such a case.

david-kubecka commented 1 year ago

I can probably understand why the static metamodel is needed in those cases. Perhaps my first question would be why the metamodel is not fully initialized. At least it would help me if I understood what kind of initialization needs to be done during boot time and how is that performed. Also, what context exactly means in this situation?

beikov commented 1 year ago

The static metamodel is initialized during the bootstrap of the EVM, similar to the JPA static metamodel being initialized during the bootstrap of the EMF. The EVM is the context.

david-kubecka commented 1 year ago

Ok, so in some of my tests I don't initialize EVM at all because the code under test doesn't need it [*]. So your suggestion of omitting the checks when there's no EVM seems to make sense. I guess those checks are persistence related which exactly is irrelevant for tests that don't need EVM.

Alternatively

we can't assume that all EVs are compiled together

couldn't this ^ be optionally assumed, e.g. by configuring EVM in some way? Would it open up some other possibilities?

[*] The EVs in this case serve just the purpose of DTOs, but might be used later for serialization in different class (not under this test).

david-kubecka commented 1 year ago

Thinking about it more, the option of optionally generating a fully configured metamodel during annotation processing time seems to be the most convenient from user PoV. AFAIK it means there will be no further hassle with disabling checks etc., and the builders/implementations could then be used anywhere without any limitations as one would expect.