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

null type id handling does not work with writeTypePrefix() #4407

Closed
cowtowncoder opened this issue Mar 2, 2024 · 2 comments
Closed

null type id handling does not work with writeTypePrefix() #4407

cowtowncoder opened this issue Mar 2, 2024 · 2 comments
Labels
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 2, 2024

(note: regression partly due to #3373, reported f.ex as #4354 (but not just that))

So: if custom TypeIdResolver returns null from idFromValue() or idFromValueAndType(), result is that underlying JsonGenerator.writeTypePrefix() call is omitted. This may work for some cases, but won't for others, because necessary parts of structure are missing.

Conceptually the idea would be that null for Type Id means "do not add Type Id", but doing so cannot be done by simply skipping calls to JsonGenerators writeTypePrefix() and writeTypeSuffix(): rather, calls need to be made but these methods need to be aware of possibly missing type id and output whatever else is necessary.

EDIT: another way to describe this is that if idFromValue() / idFromValueAndType() returns null, serialization occurs AS IF non-polymorphic serialization was used. For As.PROPERTY / As.EXISTING_PROPERTY, for example, output would be similar but without "type": "typeId" entry in JSON Object; and for wrappers (As.ARRAY_WRAPPER / As.OBJECT_WRAPPER) no wrapping is added.

@cowtowncoder cowtowncoder added to-evaluate Issue that has been received but not yet evaluated 2.18 and removed to-evaluate Issue that has been received but not yet evaluated labels Mar 2, 2024
@cowtowncoder
Copy link
Member Author

Thinking this through again, I realized that what can and should be done depends on mechanism:

  1. With As.PROPERTY (and As.EXISTING_PROPERTY), we CAN omit type id but if so, MUST add starting { (and closing }). This is the case I was mostly thinking
  2. With As.WRAPPER_ARRAY, we CANNOT just omit entry, something must be written as first-element of array (null?) OR fail with exception.
  3. With As.WRAPPER_OBJECT, we also CANNOT just omit entry, but we also cannot write null as property name. I guess we could write "" as key?

So (1) is fine, (2) somewhat and (3) less good.

Choice then is between failing on (2) and (3), or using placeholder.

My thinking is to go with placeholder; it is up to implementor of TypeIdResolver to either return null and get whatever we give, or avoid returning null.

cowtowncoder added a commit that referenced this issue Jun 15, 2024
@cowtowncoder
Copy link
Member Author

cowtowncoder commented Jun 15, 2024

Hmmh. Ok, actually, come to think of this... no, it should be possible to just omit Type Id write and make things work as if value was not polymorphic at all. That should work for all cases; but it cannot be done by simply avoiding calls to JsonGenerator.writeTypePrefix().

But there are two approaches, still:

  1. Do it all in databind (TypeSerializerBase)
  2. Use delegating, do it (mostly) in JsonGenerator

both approaches have their benefits.... need to think a bit more.

I think I will first try approach (1) and see how far that gets us.

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

No branches or pull requests

1 participant