3rd party class customization

[Verdent]

Signed-off-by: David Kral [email protected]

[rmannibucau]

Still think we shouldnt have 2 models in the API and since we cant drop annotations as of today we should stick to it instead of redoing the whole hierarchy.
Technically it does not limit any case.

I also took a few cases where the classes were in a jar I can't modify (so can't use annotations) and seems it always ends up with adapters or (de)serializers just because it is easier to understand, follow code.
So overall I'm really interested in getting this feature but I think more than ever it should be minimalistic in terms of API and more than anything not redefine the full model we already have differently.

Any hope it gets back on "real" decorators land (which are annotations in IT in most languages)?

[rmannibucau]

SPI is not worse than before so ok on the loading.
On the surface points I think a single one is better.
On a design on the overall PR I confirm I think we should limit it to the introspector api and not do it a seconf time there, a third later on the introspector (required anyway to make the spec integrable by 3rd parties like mp).
So think we should postpone such a direction until a minimal introspector is there and review if relevant or not (which should be IMHO).

[m0mus]

IMO, *Descriptor is a better name for *Decorator classes.

[Verdent]

Sure, I have no problem with changing the name to Descriptor :-)

[rmannibucau]

if we drop the two entry points from this class we can make this whole PR an appendix - same as proposing the introspector/annotation based solution - of the spec while there is not agreement on the final outcome.
Can enable to have API proposals and decide later (bean validation went that way in its time - for method validation).
Overall blocking point is to have something unified versus something duplicated for 3rd party libraries and I don't see us choosing before next release since both parties seems to either priviledge the interoperability and simplicity or the application API first (being said the introspector API enables to join the API point later if needed whereas decorator does not and keeps two concurrent models in the spec forever).

Wdyt? (trying to not block but not pollute the API too early without enabling 3rd party cases)

[m0mus]

What exactly do you mean by "drop the two entry points from this class". I am sorry, but I don't understand what you are trying to say. Please explain.

[rmannibucau]

Not make it explicit in the API (ie use yasson.decorators property without a constant nor a wither.

[m0mus]

@rmannibucau If you strongly disagree with this approach I would recommend you submitting your vision as a PR so we can compare them and vote for the best approach.

[lukasj]

Likely unrelated but... javadoc says throws NullPointerException if providerName is {@code null}. but the code does if (providerName == null) { throw new IllegalArgumentException();} (the fix belongs to different PR...)

[rmannibucau]

@m0mus I'm a bit out of time this month but long story short as discussed on slack:

1. you do not address Introspection API #189 with such a proposal whereas addressing it will make this feature trivial to do
2. I dislike - and it is the same kind of feedback I got when asked for a review on that proposal - the fact we fully duplicate the model we manipulate - will make it impossible for end user to find back their stuff whereas technically there is no justification at all since all can be done with annotation (including reflection less runtime)
3. we put an API before having a backend
4. it is not 3rd party libraries friendly OOTB (wither/dsl are not and require a lot of glue code for nothing IMHO, fluent setters are a bit better but do not solve previous points)

So i'd start by solving 1, put an API enabling to provide an introspector manipulating the annotations instances we already have and voilà, we are 3rd party friendly and solved 2 others issues without making the API way more complex nor duplicating our own model completely :). We can revisit a more fluent API later - as CDI 1->2 did - if needed but not sure it is and hopefully javase will help us with some common GenericLiteral impl in the jre itself.

[m0mus]

1. you do not address [Introspection API #189](https://github.com/eclipse-ee4j/jsonb-api/issues/189) with such a proposal whereas addressing it will make this feature trivial to do

You are right. We have an API for adding customizations at runtime, but we don't have an API to read them. We'll address it.
Yes, we don't. We'll address it

2. I dislike - and it is the same kind of feedback I got when asked for a review on that proposal - the fact we fully duplicate the model we manipulate - will make it impossible for end user to find back their stuff whereas technically there is no justification at all since all can be done with annotation (including reflection less runtime)

I don't agree here. The goal is to have two ways of adding customizations: at compile time using annotations (it's already there) and at runtime (this is what this PR is about). I don't agree that the model (I think you are talking about the customizations model) is duplicated. The model stays the same the way it's been defined in the earlier versions of the spec. We just adding a runtime API to access it. If you are talking about well-defined API vs generic API, I am not a big fan of too generic APIs. They are too wide and allow doing things going out of scope.

3. we put an API before having a backend

We need to prove implementability of API provideing at least one compatible implementation. Yasson folks are working on it. API work is in progress and we can change it as many times as we want before the release.

4. it is not 3rd party libraries friendly OOTB (wither/dsl are not and require a lot of glue code for nothing IMHO, fluent setters are a bit better but do not solve previous points)

I am not sure I understand what you mean here. I think that this API is quite user friendly and can be used with 3-rd party objects without any problems. If you don't think so, please provide a use case.

[rmannibucau]

I don't agree here. The goal is to have two ways of adding customizations: at compile time using annotations (it's already there) and at runtime (this is what this PR is about). I don't agree that the model (I think you are talking about the customizations model) is duplicated. The model stays the same the way it's been defined in the earlier versions of the spec. We just adding a runtime API to access it. If you are talking about well-defined API vs generic API, I am not a big fan of too generic APIs. They are too wide and allow doing things going out of scope.

It is not a generic API since it is a very JSON-B specific (@Jsonb<something>), it is about two API to do the exact same thing. Customization API is 100% a way to bind markers on a model, we already have all markers so no need to duplicate them all and it is exactly what this PR does in current flavor.
For any integrator it requires to handle 2 models instead of one which is useless technically and makes the model understanding harder until there is an unified view which will be the introspector which should manipulate a single model which means the annotation - at least in current form of the PR and hopefully not a 3rd model in all cases.

We need to prove implementability of API provideing at least one compatible implementation. Yasson folks are working on it. API work is in progress and we can change it as many times as we want before the release.

To be honest Johnzon supports that since a lot of time - before JSON-B - using a kind of custom introspector injection in its model parsing and it is likely what enables the most flexibility for integrators (originally it was used a lot to bind jackson model on johnzon-mapper one). With current customization form it is a lot of work - you redefine the whole model - whereas you just want to go through the model and map what is needed visiting it in 100% of case for this use case. So here again I think we miss the right level to define the API.

I am not sure I understand what you mean here. I think that this API is quite user friendly and can be used with 3-rd party objects without any problems. If you don't think so, please provide a use case.

As explained on slack and several other channel this API covers the end user application external customization case which is probably 5% of the use case of such an API.
Integrators are likely the most common case and even for end applications you often just want to "map" custom annotations (stereotypes like is one use case for example).
So long story short, as friendly this API can be, it requires a lot of glue code to be used and does not tackle the real challenge of customizations, this is why I think we should go back to basic and maybe move this PR as a yasson specific thing until we have the instrospector properly setup without making the spec boilerplated with a lot of interfaces and API for no real end user gain (it is really a pivot moment because JSON-B is quite acceptable in current form but I hope it does not become another JPA which is way too fat to stay mainstream and a default choice to be very concrete).

[jansupol]

I am not sure the optional makes sense here. If empty, the spec still needs to define whether it is nillable or not when not set.

[Verdent]

Optional is here because the value didn't have to be set at all. Simply returned boolean is not enough in this case since we need 3 states. True, false and not set

[rmannibucau]

Tt is always true or false for the runtime.
If you pass empty() you immediately do orElse(true) so at the end it is true or false.
This means you want the user to be able to explicit it or not so means the method is called or not I think, so maybe the API design is reversed compared to the goal and lead to this misleading signature?

[Verdent]

That is not true. If user didn't call nillable method, it is equivalent to the situation where annotation @JsonbNillable has not been applied on the field/method and we have to behave correctly according to that. True/False is just not enough here.

[rmannibucau]

@Verdent when @JsonbNillable is not on an attribute the attribute is nillable=true so true/false are strictly sufficient. Now I fully understand the intent of your API which is to let the user say "I don't care, use the (configured or not) default" but then the API is not designed for that case, it is reversed as mentionned.

[rmannibucau]

this is true but if the user did he probably knows too since he has to use the jsonbconfig anyway with that solution so he can wire the default properly without any issue.
It is less misleading to say that it is nillable or not rather than saying it is nillable or not or maybe (still I understand you dont want an enum {TRUE, FALSE, DEFAULT} but optional means something else semantically).

[Verdent]

What user did? :-) User can set only true or false via builder and not Optional. I don't understand what is misleading here since it just says: User either set true/false by himself or didn't set anything at all since the method has not been called. In that case value from config will be used by implementation. There is simply no misleading part here.

[rmannibucau]

it is never misleading for the people writing it, this is why reviews are not that bad ;)

javadoc says Return if the component can be nillable in the given {@link Scope}., signature is not compliant to that and in any case you are nillable or not right, whatever you use global config or not so not sure the point to make it indirect for the caller is what I meant.

[Verdent]

eh... are you trying to tell me... that this whole discussion here was just about incorrect wording? OK, I will change that.

[rmannibucau]

hehe, no no, that even the comment is closer to the expectation.
That said I will not fight on these picky details now since I'd like the overall design to be more relevant for end users first so will stick to the main thread.

[m0mus]

@rmannibucau

For any integrator it requires to handle 2 models instead of one which is useless technically and makes the model understanding harder until there is an unified view which will be the introspector which should manipulate a single model which means the annotation - at least in current form of the PR and hopefully not a 3rd model in all cases.

Who is integrator? Is it a person/team who works on a compatible implementation? Or it's a user integrating some 3-rd party code into his application?

[rmannibucau]

@m0mus integrator somebody doing the integration between 2 layers (3rd party, custom application schema/design/contract API etc...). It is never a JSON-B provider in what I mentionned. Maybe in simpler terms: a JSON-B API consumer. Hope it clarifies it.

[Verdent]

Fine, just means this feature will be postponed too.

You mean this PR?

As mentionned this means you do release R "feature F", release R+r "feature F was garbage, use feature F'", I want to avoid that.

I am not gonna be deprecating the runtime customization I am designing now. This is not something temporal. Visitor is not here to replace it. Just another way.

As mentionned it covers a single customization case: final app one, it is very few of the use cases (libraries, global mapping changes etc.). So even if it is functional for this particular use case i'm more than happy to wait for a more global and simple solution and avoid a specific, single case, and verbose one. Jsonb is big enough as of today IMHO, no need to make it a big spec just by duplicating things IMHO.

I simply don't agree with these arguments. I am still trying to make compromise and have proposed visitor with my customization objects, but to do that properly, we just have to wait. Using your "simple" annotation based solution is just unintuitive way of programming, since it simply doesn't help user in any way. That's not something regular programmer would want to do IMHO.

[rmannibucau]

Just another way.

This got proven a very bad way to do API. Lead to misleading and ambiguous experience to end users.

That's not something regular programmer would want to do IMHO.

Thought that when cdi 1.0 went out but I was wrong. It is exactly as your proposal, just a doc thing but industrializable without custom wrappers for 3rd parties so just even at that level it superseeds a fluent builder API.

[Verdent]

This got proven a very bad way to do API. Lead to misleading and ambiguous experience to end users.

What other suggestions do you have? I am really trying to make both worlds work together here, but I will simply not build my proposal on top of the literals or incorrect API. The reason for introducing visitor later is just because I want to make it right in terms of the API. I think it is time for you to fully introduce your desired solution in details, so we can review it.

Thought that when cdi 1.0 went out but I was wrong. It is exactly as your proposal, just a doc thing but industrializable without custom wrappers for 3rd parties so just even at that level it superseeds a fluent builder API.

CDI uses that approach and it is completely fine in terms of what they are doing, but it is something, what I do not believe, belong to the JSONB code and usage style.

[rmannibucau]

@Verdent i sent one proposal - based on johnzon - on our slack chan, but we can also reuse introspector kind of api for the high level idea, both work but miss the jsonb context - this is where visitor came from (think adapter, deserializer, which must be provided to the customizer before it contributes some customization for ex). So overall api can be something like:

public interface JsonbModelCustomizer {
  default void onType(TypeContext context) {}
  default void onProperty(PropertyContext context) {} // field and method merged view respecting visibility
}

Base context would be an inner interface:

public interface Context {
  <T extends Annotation> T getAnnotation(Class<T> a);
}

Type and property flavors will just expose, resp., the underlying getType()+skipProperty(String)+register(Annotation...)+registerProperty(Annotation...) and Optional+Optional as getters.

Nothing more needed, handles all types customizations including lists/maps, keep focused on our current model and finally is build time friendly since no reflection is required at runtime (even if build will stay simpler and way faster using it ;)).
Also makes the API simpler to integrate with 3rd party since API surface is smaller.

[Verdent]

Well... it's not that simple as you are still claiming it to be. Alright lets do some tests here.

public class Pojo {

     private String valueOne;

    //getters and setters are also present
}

Show me how the usage of this API would look like, when I want to have a different name for serialization and for deserialization and for example @JsonbNillable to be also used on this class.

[rmannibucau]

public class MyJsonbModelCustomizer implements JsonbModelCustomizer {
  @Override
  public void onType(TypeContext context) { context.register(new JsonbNillable.Literal(); }
}

About the serial/deserial names, JSON-B does not support it until you use a custom deserializer/serializer (I hear you can put a setter and getter not asymmetric but not 100% sure it happens often enough) so at the end you ctx.registerProperty(new JsonbProperty.Literal("serialName"))registerProperty(new JsonbProperty.Literal("deserialName"));; and be it.
If you really want to handle that case - I'm not convinced it is needed but fine if a big majority think it is - we just split onProperty i onSerializationProperty and onDeserializationProperty, does not dramatically change the proposal IMHO. I assume the default impl can call onProperty too if we want to stick to a simpler API?

[Verdent]

1. you are registering the names as a property?
2. This is not about how often something happens. This is about possibility to do the same customization in runtime as you can do in compile time.
3. This does not target only one class, but all of them. The same for properties since if we have this example, it will simply add the same annotations everywhere.

public class Pojo {
     private String valueOne;
     private String valueTwo;

    //getters and setters are also present
}

So you are really trying to tell me, this is simpler, faster to work with and easier to use then my proposal? It gets messy and overly complicated with just a few properties. In fact 2 are enough to have unnecessary complexity of ifs or switch statements.

public class MyJsonbModelCustomizer implements JsonbModelCustomizer {
    
    @Override
    public void onType(TypeContext context) {
        context.register(new JsonbNillable.Literal();
    }

    @Override
    public void onSerializationProperty(PropertyContext context) {
        context.registerProperty(new JsonbProperty.Literal("serialName"));
    }

    @Override
    public void onDeserializationProperty(PropertyContext context) {
        context.registerProperty(new JsonbProperty.Literal("deserialName"));
    }

}

[rmannibucau]

1. yes but I assume what you want is not a customizer but an actual ModelProvider which would also need a Supplier<T> and Consumer<T> to get/set the value of a virtual field (but it is another issue for me, just mentionning it cause the topic kind of change with exchanges)
2. I fully disagree on the last part since until now you claim you can't do it without your proposal but it is not true technically at all so I'm not sure how to refine the topic/comment there 🤔
3. not sure why, context.getAnnotation(JsonbProperty.class )gives your the propery name, getField/getmethod the accessor so this is not true once again :s.

From my experience it is clearly easier to integrate with 3rd party libraries and classes than an explicit highly verbose API + you still didn't solve the model duplication.

[Verdent]

yes but I assume what you want is not a customizer but an actual ModelProvider which would also need a Supplier and Consumer to get/set the value of a virtual field (but it is another issue for me, just mentionning it cause the topic kind of change with exchanges)

Yes, if you are registering property, it would require more then just one annotation.

I fully disagree on the last part since until now you claim you can't do it without your proposal but it is not true technically at all so I'm not sure how to refine the topic/comment there 🤔

I am not claiming you cannot do it without my solution. I am claiming my solution is easier to use.

not sure why, context.getAnnotation(JsonbProperty.class )gives your the propery name, getField/getmethod the accessor so this is not true once again :s.

This is simply not true. Now try to demonstrate how it would look like for 3 or more different properties where you want to have different customization (for example names) for serialization and deserialization. And also try to make some mechanism which would execute your proposed solution only for a specific class and not for all. Because now it is basically widely used for each type.

From my experience it is clearly easier to integrate with 3rd party libraries and classes than an explicit highly verbose API + you still didn't solve the model duplication.

I don't believe there is any duplication, that means I do not see anything to fix. And once again I am not against visitor in general, I can see its reasoning. I am against the literals in our APIs and having visitor only without the easy way to do stuff. Visitor would be there as more of a hardcore part if user really needs that.

[rmannibucau]

Yes, if you are registering property, it would require more then just one annotation

If it is a virtual one it requires just the both functions (to be bidir), if we stick to customizations just annotations as in the example (but can be JsonbNumberFormat.Literal and others too indeed)

I am not claiming you cannot do it without my solution. I am claiming my solution is easier to use.

In one very particular case yes, in all others I don't think so. Plus, it duplicates the model which is a big issue for me.

This is simply not true. Now try to demonstrate how it would look like for 3 or more different properties where you want to have different customization (for example names) for serialization and deserialization. And also try to make some mechanism which would execute your proposed solution only for a specific class and not for all. Because now it is basically widely used for each type.

Well it is as you can see:

public class MyJsonbModelCustomizer implements JsonbModelCustomizer {
    @Override
    public void onProperty(PropertyContext context) { // impl ensures this request is fulfilled and falls back on member.getName() if needed
        context.registerProperty(new JsonbProperty.Literal(context.getAnnotation(JsonbProperty.class).value().equals("foo") ? "f" : "bar"));
    }
}

On your last point: I don't want to make a lot of convenient helper functions, this always lead to a noisy API which is more bothering and does not encourage people to learn proper ways to solve their issues.
Once again, I would first solve the common needs when we manage to find an agreement (schema extraction enablement, customizations) then see if we need a more friendly DSL or not - can even be an optional part of the spec but ont a requirement IMHO.

[Verdent]

In one very particular case yes, in all others I don't think so. Plus, it duplicates the model which is a big issue for me.

That it duplicates something is just your opinion. From my point of view it is perfectly fine to have it like this. It is easier to use in most of the needed cases. And for those, it is not, visitor will be presented to be able to use. As I mentioned before.... a bigger hammer to do the stuff.

public class MyJsonbModelCustomizer implements JsonbModelCustomizer {
@OverRide
public void onProperty(PropertyContext context) { // impl ensures this request is fulfilled and falls back on member.getName() if needed
context.registerProperty(new JsonbProperty.Literal(context.getAnnotation(JsonbProperty.class).value().equals("foo") ? "f" : "bar"));
}
}

Eh... I am not sure what to say here. I have several issues here. This still does not address multiple properties or specific class. Another is that JsonbProperty annotation doesn't have to be there and most likely will not be and that means it cannot be done simply like that. I know you are trying to put everything on a single line just to look it is simpler and less chatty. But I would prefer to do it correctly and with all cases covered.

Once again, I would first solve the common needs

That's exactly what my proposal is about. Visitor is for more extra needs.

I would be more then happy to come up with some sort of compromise. I have already introduced several of those, but literals are simply no go for me. I will go over that line.

[rmannibucau]

Visitor is for more extra needs.

You didnt get it. Your solution solves end user customization, period.
My proposal solves this + 3rd party customizations + schema extraction with a single API and no duplication of the model.
This is the common needs for me, not only your case which is not that common in regards of others IMHO.

Another is that JsonbProperty annotation doesn't have to be there and most likely will not be and that means it cannot be done simply like that.

It is as explained in the comment. Give a try if you doubt.

[Verdent]

only your case which is not that common in regards of others IMHO.

others -> you
Common usecase is something common -> name change, number format, nillable, transient. That's common. And no we are not duplicating any model so there is no issue there. Sure I am covering customization only in terms of the base PR since that's what this is about. Introspection or advanced customization (visitor) will be also supported, but I have suggested to postpone it due to obvious reason I have described before.

It is as explained in the comment. Give a try if you doubt.

I did. So you think that this method should basically just fail with NullPointerException since otherwise I am not sure how the impl will even know to fall back.

[m0mus]

It definitely makes sense to use reflectionless Object Model API because we are targeting to Jakarta Core profile which is going to be build time processing friendly.

[m0mus]

Folks, this conversation becomes never ending. Both parties have strong opinion and no one wants to step back. It's getting close to October deadline and we may not have enough time to deal with the implementation and TCKs.

I see two options:

1. Merge the customization API as it was proposed without a visitor and an introspector. We'll add both in the next version of API when Object Model API is somehow (we don't know how) extracted from CDI.
2. Postpone the customization API to the next version of JSONB. We are still willing to add it to Yasson. So, users will be able to play with it and provide some feedback before it gets merged to the spec.

I would like all project committers to vote. I'll close the ballot in a week at Sep 23rd 13:00 CEST.

@njr-11 @Verdent @edbratt @jansupol @jimma @lukasj @otaviojava @pdudits @rmannibucau @tomas-langer @ivanjunckes please vote in the comments.

I hope I didn't forget anyone.

[otaviojava]

Optional of boolean seems weird, if we want a third

[Verdent]

That was designed exactly to have the third option here :-) It should correspond to whether it has been set to true/false explicitly or wasn't set at all, in which case @JsonbTransient of the property should be reflected (if present) I was trying to avoid using true/false/null

[otaviojava]

The same issue here about optional of boolean

[otaviojava]

@m0mus I like MVPs, so I vote to merge as it is and then move to the next step.

[rmannibucau]

@m0mus without surprises, I think this misses the bbig opportunity to have an unified model and unified API solving > 3 issues (missing features) so I'd keep it as an extension not belonging to the spec requirements (= agree it can be a DSL end users like on top of the actual spec API but nothing more) so I'd be to not merge it.

[lukasj]

I'm for merging this.

[Verdent]

I am a bit puzzled here. I am for merging it, but I want to do it right. There is possible problem I can see and that is the moment, when we decide to add that visitor and introspector functionality, I have mentioned before. I would like to have these two features build on top of this customization PR and the problem might be, that we will need to change something in this customization design, but if it is already released and it is breaking change, we will not be able to. So I might vote for postponing and merge when visitor and introspector do have all the required parts ready and are fully designed. Does this thought make sense? or you think it will be fine to merge it now, since I do have the design of the visitor and introspector more or less ready and we might not need later to change the customization at all?

[jansupol]

The open question for Jakarta EE 11 is the possible replacement of Java reflection API with a metamodel for compile-time (annotation) processing. Such a thing would be required by multiple specs that would want to focus on speed (thank you Quarkus), starting with CDI lite, up to JSON-B. Introducing an API that handles the annotations directly can cause issues for EE 11 and the new API could have been immediately deprecated in EE 11. We do not know for certain what requirements this compile-time processing brings, and hence I agree with David to postpone this very API change until we have a more complete idea of the requirements. Then we will be able to design the API for introspection, too.

[rmannibucau]

I guess we can fully ignore reflectionless because:

1. It cant hit jakarta (platform) alone (only for sub parts of the specs and to reuse the only example - as of today, CDI - it is a full new spec from scratch with no link with current version
2. Technically you dont need anything new for cdi, jsonb, .... the jdk already provides all we need (indeed build time constraints are high enough to not be adopted by most users but technically we know how to do)
3. Build time always had been out of scope of EE, not sure it is sane to go that path - indeed it is a way to say it ;)

So long story short, the big risk there is to create an api not reaching enough consumer coverage and superseeded in a next version we already envision even if we dont have the api yet.

Side note: we dont have to await an ee version to release a major jsonb version, we can be faster if we reach a compromise. EE just gets the current last version in last release plan "sprints".

[jansupol]

@rmannibucau This issue is likely not to right place to discuss reflectionless. But it is a big topic right now for EE 11. I completely agree that it can be ignored, and the benefit not being used. I agree that JDK provides everything needed so we do not need anything new from CDI or whomever. We may not benefit from what is provided by other projects and we can reinvent the wheel. But I am not sure it is the right way to go. We may see where the EE world is heading soon; I expect a wide discussion once EE10 is dealt with.

Anyway, I understand that you agree with postponing the PR.

[pdudits]

I like the feature and can see the where it could be useful.

The javadoc should however consistently describe the semantics of the methods, and we shouldn't introduce any types that have no purpose for the user of the API.

[pdudits]

Since this is the entry point to the feature, I miss more explanation about the effect of using a type customization in the Javadoc:

• Does it fully override information provided via annotations?
• How does it cooperate or interfere with adapters and serializers defined for the type?

An example would be nice as well

[pdudits]

The documentation for most of the getters in the package is confusing.

The pattern "Return X of the decorated type" raises obvious question -- what is "decorated type"? If it stands for what getType() then either javadoc for getType() should explicitly mention decorated type, or documentation should use "type which is customization bound to" instead of "decorated type".

"Return X, otherwise empty" is also unclear -- What does empty return mean? That decorated type has no defined property order strategy? Or that it has no customized property strategy, and therefore default (i. e. annotation-based if it exists on underlying type) behavior will be active? I assume latter therefore the wording should rather follow pattern "Return customized X if defined, otherwise empty"

[pdudits]

In my opinion the type hierarchy of the customizations does not serve any purpose for the user and makes API harder to navigate. I'd drop this entire hierarchy and limit public API just to the interfaces that should be used directly by the user.

I can see how this can be convenient for implementor, but implementor is free to build its implementation class hierarchy instead.

[Verdent]

Hmm yes I kind of agree, but the whole hierarchy has been made this way to prevent having method duplication :-) Or do you agree with having it and just hide it as package private and expose just the end interfaces?

[pdudits]

You got me reading Java Language Specification with this question, and it doesn't directly list the consequences. I believe it's better to inline the interfaces.

[pdudits]

ParamCustomization as it seems is not really a customization, it is a parameter of CreatorCustomization.
Should it follow the same naming pattern, and should it be top level interface?

I'd propose name CreatorParameter instead, moving it into CreatorCustomization would be also an option to explore.

[pdudits]

I recommend using more specific name, so this doesn't creep into autocomplete for e. g. jakarta.inject.Scope.
CustomizationScope would be more fitting.

[Verdent]

Yes, this seems reasonable to me. Original thought was to avoid having long enum name

[pdudits]

ignoreNillable above says "Ignore nillable customization", while this says "Ignore deserializer".

What is the semantic of these ignore methods? Resetting the customization, or overriding default beaviour, e. g. ignoring deserializer when one is defined with an annotation?

[Verdent]

The whole concept of the ignore methods should be in place to ignore specific annotation on the component, other customizations (and also inherited ones) which apply to the component should be applied. I am still playing a bit with an idea of having just a single ignore(Class<? extends Annotation>) method. I am still not sure if that is not more clear. What do you think about this common ignore method? Would it be better? Or would you prefer to have separate methods for each customization to ignore.

Anyway yes, javadoc of this version is not very clear. I do have it fixed locally, but not yet pushed :-)

[pdudits]

The reason I asked was because one can also do somePropertyCustomization.toBuilder().ignoreDeserializer(). What is the effect of this call if somePropertyCustomization defined a serializer?

The customization of every attribute is from domain Default|None|Custom(value), where Default represents no customization, so whatever spec+class annotation prescribe, None represents spec while ignoring class annotation, and finally Custom is customized value from customizer. Usually one starts at Default and with ignore method can set processing to None, with set(x) method can set processing to Custom(x). But with toBuilder you can start in any of the states and you need transition path to any other state. The common ignore method will only solve part of the problem, while introducing the binding to annotations and it is clear where it would go from there ;)

Clean model of this would be to have every customization attribute represented as type similar to Optional<T>, where it is either Customization.ignored(), Customization.annotated() or Customization<T>.of(T value). It is possible to have single getter/setter pair for every attribute and cover all of the possible states.

Additionally a convenience setter can be added, where non-null value represents of(value) and null represents annotated().

Another approach is to have provider to return Builders with full mapping, but that turns it into introspection API.

[pdudits]

This also shows weakness of toBuilder() methods. It cannot be really reused for different property names, in case I have common customization to apply to more fields. Either names should also be modifiable in builders or method toBuilder(name) should also exist.

One final alternative is to drop toBuilder() methods, and rely on TypeCustomizationBuilder.property(String,Consumer) for sharing settings. Then everything starts in annotated state.

[Verdent]

@pdudits Thank you for your feedback. I definitely agree that javadoc should be improved and I will do that :-) It contains a lot of inconsistencies and unclear formulations, which I am working on getting rid of.