chandrasekhar-188k commented on PR #6374:
URL: https://github.com/apache/hbase/pull/6374#issuecomment-2506429559
> > After commiting this (a little late), I realized that I have more
questions:
> > I see this annotation on ScannerModel:
> > > @JsonInclude(JsonInclude.Include.NON_NULL)
> >
> >
> > and
> > > private boolean includeStartRow = true;
> > > private boolean includeStopRow = false;
> >
> >
> > Doesn't that mean that the new attributes will get serialized regardless
of their values to JSON, and cause the deserialization in the old server to
fail ?
> > You matrix shows this as passing, but I can't see how that can happen.
Does JSON skip serializing the default values ?
>
> you are correct, when we serialize the ScannerModel the new attributes are
also included and causing the deserilization to fail at old server. when I
tested for JSON I have construcuted the JSON message manually with the required
fields, instead of Converting the ScannerModel to JSON, that's why my test was
ok. For XML and protobuff I had tested with Conversion of ScannerModel object
to the required mime type, so no problem here.
>
> For solving the issue wrt JSON, I think we can have two approaches
>
> 1. change the class level JSON include annotation to
@JsonInclude(JsonInclude.Include.NON_DEFAULT), so that it doesn't serialize the
default values. This can work with out any functional impact only if the
default values of ScannerModel and Scan Class are in sync always.. but
currently I could see the default values for some fields are different between
ScannerModel and Scan.
> 2. For the newly introduced attributes add a custom JSON include
annotation with a filter class that skips serialization of a field if the field
has default value set to it.
> ex:
>
> ```
> @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter =
MaskingFilter.class)
> private boolean includeStartRow = true;
>
> static class MaskingFilter {
> public boolean equals(Object value){
> return Boolean.TRUE.equals(value);
> }
>
> }
> ```
>
> I think the 2nd approach is better for this. I will raise an addendum PR
for this..Let me know if you have any other suggestions..
addendum PR raised for this fix, Please review
https://github.com/apache/hbase/pull/6499
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
