You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by Jean-Louis Monteiro <jl...@tomitribe.com> on 2022/02/28 09:37:06 UTC
OK to release Johnzon 1.2.17?
Hi all,
Is it ok to release a 1.2.17?
I'm looking for a Jakarta EE fix -
https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
Do you guys have some additional fixes to push?
--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
+1
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Le mer. 13 avr. 2022 à 08:44, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> Will start a release soonish.
>
> LieGrue,
> strub
>
> > Am 12.04.2022 um 10:07 schrieb Mark Struberg <struberg@yahoo.de.INVALID
> >:
> >
> > took your PR and enhanced on top of it.
> >
> > Plz review!
> >
> > txs and LieGrue,
> > strub
> >
> >> Am 10.04.2022 um 20:08 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com>:
> >>
> >> Created https://github.com/apache/johnzon/pull/83 to try to share what
> I
> >> meant and try to solve it to enable the release (note I did it a bit in
> a
> >> hurry, the coming weeks are quite busy for me - so happy to get a second
> >> pair of eyes + optionally somebody running the jsonb tck)
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >> <https://rmannibucau.metawerx.net/> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>
> >>
> >> Le dim. 10 avr. 2022 à 18:42, Romain Manni-Bucau <rm...@gmail.com>
> a
> >> écrit :
> >>
> >>>
> >>>
> >>> Le dim. 10 avr. 2022 à 17:34, Mark Struberg <st...@yahoo.de.invalid>
> a
> >>> écrit :
> >>>
> >>>> Oki will re-introduce that ct.
> >>>> Should then be backward compat, isn't?
> >>>>
> >>>
> >>> If the modifier check is there back yep
> >>>
> >>>
> >>>> LieGrue,
> >>>> strub
> >>>>
> >>>>
> >>>>> Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <
> >>>> rmannibucau@gmail.com>:
> >>>>>
> >>>>> Le dim. 10 avr. 2022 à 17:16, Mark Struberg
> <st...@yahoo.de.invalid>
> >>>> a
> >>>>> écrit :
> >>>>>
> >>>>>> Actually there is not much diff. There is one ct which was only
> used in
> >>>>>> JsonbAccessMode as true. Everywhere else it was feed as false.
> >>>>>> Now we could keep this flag and feed it as default if you really
> like.
> >>>>>> That should be rather easy to do if you think it's necessary. But
> >>>> still I'd
> >>>>>> mark that 4param ct as @Deprecated. Wdyt?
> >>>>>>
> >>>>>
> >>>>> Well it is used by users writing custom access mode quite often so we
> >>>> must
> >>>>> keep it and its associated behavior as a minimum for coming release.
> >>>>> No issue to make an abstract class with more children to keep it
> >>>> simpler in
> >>>>> the code but please dont break it.
> >>>>>
> >>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
> >>>> rmannibucau@gmail.com
> >>>>>>> :
> >>>>>>>
> >>>>>>> Hi Mark,
> >>>>>>>
> >>>>>>> If I read the diff correctly we still have some breaking change in
> >>>>>>> FieldAndMethodAccessMode
> >>>>>>> constructor/behavior? Can we get it fixed?
> >>>>>>> In terms of design, jsonb access mode was originally thought as
> >>>>>> delegating
> >>>>>>> the research of the writers/readers to a 3rd party access mode and
> >>>> just
> >>>>>>> refilter on top of it with jsonb rules.
> >>>>>>> Now it is kind of mixed in the code (inheritance + composition vs
> just
> >>>>>>> composition), the reason it was not done was to avoid to expose the
> >>>>>> default
> >>>>>>> delegate access mode when a 4rd party access mode was using and mix
> >>>> them
> >>>>>> in
> >>>>>>> the jsonbaccess mode, not 100% sure if we want to keep the original
> >>>>>> design
> >>>>>>> there or not but mentionning it since it now prevent us to change
> it
> >>>>>> later.
> >>>>>>>
> >>>>>>> Overall except the missing constructor it looks ok to me, happy to
> get
> >>>>>>> thoughts on the access mode pattern too.
> >>>>>>>
> >>>>>>> Romain Manni-Bucau
> >>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>>> https://github.com/rmannibucau> |
> >>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>> <
> >>>>>>
> >>>>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg
> <st...@yahoo.de.invalid>
> >>>> a
> >>>>>>> écrit :
> >>>>>>>
> >>>>>>>> here we go. plz take a look!
> >>>>>>>>
> >>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> >>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
> >>>>>>>>
> >>>>>>>> txs and LieGrue,
> >>>>>>>> strub
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
> >>>> <struberg@yahoo.de.INVALID
> >>>>>>>>> :
> >>>>>>>>>
> >>>>>>>>> Looked at your proposed solution, but it might be tricky to get
> it
> >>>>>>>> working. The problem is that we have two modules: mapper and
> jsonb.
> >>>> And
> >>>>>> in
> >>>>>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
> >>>> defined
> >>>>>> and
> >>>>>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> >>>>>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
> >>>> actually
> >>>>>>>> did break the mapper. Sorry that I did not catch this earlier.
> >>>>>>>>>
> >>>>>>>>> Note that the FieldAndMethodAccessMode is only used if no other
> >>>>>>>> AccessMode is configured. There is also a JsonbAccessMode which
> >>>>>> delegates
> >>>>>>>> through to the accessMode of the Mapper. Normally means the
> >>>>>>>> FieldAndMethodAccessMode, but could be different.
> >>>>>>>>> We could implement the logic in JsonbAccessMode, totally ditching
> >>>> the
> >>>>>>>> accessMode coming from the Mapper. Also thought about having the
> >>>>>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding
> the
> >>>>>>>> handling as parameter over there. still not really perfect imo.
> >>>>>>>>>
> >>>>>>>>> Will try to ship a patch proposal tomorrow.
> >>>>>>>>>
> >>>>>>>>> LieGrue,
> >>>>>>>>> strub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> >>>>>>>> rmannibucau@gmail.com>:
> >>>>>>>>>>
> >>>>>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
> >>>> <struberg@yahoo.de.invalid
> >>>>>>>> <ma...@yahoo.de.invalid>> a
> >>>>>>>>>> écrit :
> >>>>>>>>>>
> >>>>>>>>>>> +1 for getting rid of java.beans.
> >>>>>>>>>>>
> >>>>>>>>>>> I've created a spec ticket for the open questions:
> >>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>>>>>>>>>
> >>>>>>>>>>> My point is: until my fix we clearly did break 4.6. And class
> >>>>>>>> hierarchies
> >>>>>>>>>>> are totally not portable right now anyway. Introducing It's
> just
> >>>>>> badly
> >>>>>>>>>>> underspecified.
> >>>>>>>>>>> So I do not see why the current implementation does make it
> worse.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Well this one is easy to answer: it changes the behavior and
> breaks
> >>>>>>>> users
> >>>>>>>>>> so it is not an option for a patch version and to be honest the
> >>>>>> proposed
> >>>>>>>>>> fix solves it for both cases so think we should tackle it this
> >>>> way, at
> >>>>>>>>>> least for now until the spec clarifies it any other way.
> >>>>>>>>>> Once again, the source of the issue is more that we lack an
> >>>>>> integration
> >>>>>>>>>> between mapper and jsonb modules than anything else and this
> >>>>>> integration
> >>>>>>>>>> had kind of been done but broke mapper so think we should fix it
> >>>> back,
> >>>>>>>> just
> >>>>>>>>>> costs us a constructor we could have avoided without this issue
> >>>> AFAIK
> >>>>>> so
> >>>>>>>>>> not a big deal, no?
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> LieGrue,
> >>>>>>>>>>> strub
> >>>>>>>>>>>
> >>>>>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> >>>>>>>> rmannibucau@gmail.com
> >>>>>>>>>>>> :
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hi,
> >>>>>>>>>>>>
> >>>>>>>>>>>> From what we discussed on this issue, the thing is that our
> jsonb
> >>>>>>>> access
> >>>>>>>>>>>> mode does not enable the visibility to see all readers/writers
> >>>> cause
> >>>>>>>>>>> there
> >>>>>>>>>>>> is some hardcoded filtering in the delegates so I think the
> >>>> trick is
> >>>>>>>> to
> >>>>>>>>>>>> simply add a toggle to the delegates access modes we use to
> not
> >>>> have
> >>>>>>>> this
> >>>>>>>>>>>> filtering and 100% rely on the visibility filtering in
> >>>> jsonbaccess
> >>>>>>>> mode
> >>>>>>>>>>> we
> >>>>>>>>>>>> already have in place.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Before the release we must also ensure getters computation
> cache
> >>>> is
> >>>>>>>>>>> cleaned
> >>>>>>>>>>>> up after the ClassMapping is created - we don't want to keep
> >>>> that in
> >>>>>>>>>>> memory
> >>>>>>>>>>>> at runtime and not sure we can clean up at another time since
> we
> >>>>>> don't
> >>>>>>>>>>> want
> >>>>>>>>>>>> a background thread for that.
> >>>>>>>>>>>> I also think it is not worth to depends on java.beans
> >>>> (java.desktop
> >>>>>>>>>>> module)
> >>>>>>>>>>>> so we would reimpl decapitalize as we already did somewhere
> else
> >>>> (it
> >>>>>>>> is
> >>>>>>>>>>> one
> >>>>>>>>>>>> liner anyway ;)).
> >>>>>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility
> must
> >>>> be
> >>>>>>>>>>> restored
> >>>>>>>>>>>> (it is part of our API and used), default should stay 1-1 but
> as
> >>>>>>>>>>> mentionned
> >>>>>>>>>>>> I'd add a new toggle (with a constructor calling "this" to
> keep
> >>>> the
> >>>>>>>>>>>> backward compat) to ignore the filtering.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Sounds like this way we solve all issues and can release.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Romain Manni-Bucau
> >>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>>>>>>>> https://github.com/rmannibucau> |
> >>>>>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>>>>>> <
> >>>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
> >>>>>> <struberg@yahoo.de.invalid
> >>>>>>>>>
> >>>>>>>>>>> a
> >>>>>>>>>>>> écrit :
> >>>>>>>>>>>>
> >>>>>>>>>>>>> To sum this up:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
> >>>> what
> >>>>>>>>>>> happens
> >>>>>>>>>>>>> if there are class hierarchies like class A extends B.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> In 3.7.1 it states that
> >>>>>>>>>>>>> For a serialization operation, if a matching public getter
> >>>> method
> >>>>>>>>>>> exists,
> >>>>>>>>>>>>> the method is called to obtain the value of the property. If
> a
> >>>>>>>> matching
> >>>>>>>>>>>>> getter method with private, protected, or defaulted to
> >>>> package-only
> >>>>>>>>>>> access
> >>>>>>>>>>>>> exists, then this field is ignored. If no matching getter
> method
> >>>>>>>> exists
> >>>>>>>>>>> and
> >>>>>>>>>>>>> the field is public, then the value is obtained directly from
> >>>> the
> >>>>>>>> field.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> This can get changed y using a @JsonbVisibility
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 4.6 Custom visibility
> >>>>>>>>>>>>> To customize scope and field access strategy as specified in
> >>>>>> section
> >>>>>>>>>>>>> 3.7.1, it is possible to specify
> >>>>>>>>>>> javax.json.bind.annotation.JsonbVisibility
> >>>>>>>>>>>>> annotation or to override default behavior globally calling
> >>>>>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
> >>>>>> custom
> >>>>>>>>>>>>> property visibility strategy.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is
> >>>> simply
> >>>>>> not
> >>>>>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when
> it
> >>>>>>>> comes to
> >>>>>>>>>>>>> class hierarchies
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>>>>> <
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> So either we say that the spec simply does not define what
> >>>> should
> >>>>>>>> happen
> >>>>>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need
> to
> >>>>>> ditch
> >>>>>>>>>>> our
> >>>>>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
> >>>>>> possible
> >>>>>>>> to
> >>>>>>>>>>>>> implement it that way.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The reason why I removed the logic from the Reader is the
> >>>>>> following:
> >>>>>>>>>>> Even
> >>>>>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
> >>>>>> property'
> >>>>>>>>>>> you
> >>>>>>>>>>>>> STILL are perfectly allowed - as per the spec - to have
> >>>> Visibility
> >>>>>>>>>>>>> strategies which do disable ALL methods and ONLY use field
> >>>> access.
> >>>>>>>>>>>>> That means the current impl is surely more correct than
> before,
> >>>> but
> >>>>>>>> not
> >>>>>>>>>>>>> yet perfect.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> LieGrue,
> >>>>>>>>>>>>> strub
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>>>>>>>>>> rmannibucau@gmail.com
> >>>>>>>>>>>>>> :
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Hi JL,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Last commit broke some stuff.
> >>>>>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few
> >>>> days,
> >>>>>>>> can it
> >>>>>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>>>>>>>>>> jlmonteiro@tomitribe.com>
> >>>>>>>>>>>>>> a écrit :
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Hi all,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Is it ok to release a 1.2.17?
> >>>>>>>>>>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Do you guys have some additional fixes to push?
> >>>>>>>>>>>>>>> --
> >>>>>>>>>>>>>>> Jean-Louis Monteiro
> >>>>>>>>>>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>>>>>>>>>> http://www.tomitribe.com
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Will start a release soonish.
LieGrue,
strub
> Am 12.04.2022 um 10:07 schrieb Mark Struberg <st...@yahoo.de.INVALID>:
>
> took your PR and enhanced on top of it.
>
> Plz review!
>
> txs and LieGrue,
> strub
>
>> Am 10.04.2022 um 20:08 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>>
>> Created https://github.com/apache/johnzon/pull/83 to try to share what I
>> meant and try to solve it to enable the release (note I did it a bit in a
>> hurry, the coming weeks are quite busy for me - so happy to get a second
>> pair of eyes + optionally somebody running the jsonb tck)
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>>
>> Le dim. 10 avr. 2022 à 18:42, Romain Manni-Bucau <rm...@gmail.com> a
>> écrit :
>>
>>>
>>>
>>> Le dim. 10 avr. 2022 à 17:34, Mark Struberg <st...@yahoo.de.invalid> a
>>> écrit :
>>>
>>>> Oki will re-introduce that ct.
>>>> Should then be backward compat, isn't?
>>>>
>>>
>>> If the modifier check is there back yep
>>>
>>>
>>>> LieGrue,
>>>> strub
>>>>
>>>>
>>>>> Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <
>>>> rmannibucau@gmail.com>:
>>>>>
>>>>> Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid>
>>>> a
>>>>> écrit :
>>>>>
>>>>>> Actually there is not much diff. There is one ct which was only used in
>>>>>> JsonbAccessMode as true. Everywhere else it was feed as false.
>>>>>> Now we could keep this flag and feed it as default if you really like.
>>>>>> That should be rather easy to do if you think it's necessary. But
>>>> still I'd
>>>>>> mark that 4param ct as @Deprecated. Wdyt?
>>>>>>
>>>>>
>>>>> Well it is used by users writing custom access mode quite often so we
>>>> must
>>>>> keep it and its associated behavior as a minimum for coming release.
>>>>> No issue to make an abstract class with more children to keep it
>>>> simpler in
>>>>> the code but please dont break it.
>>>>>
>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
>>>> rmannibucau@gmail.com
>>>>>>> :
>>>>>>>
>>>>>>> Hi Mark,
>>>>>>>
>>>>>>> If I read the diff correctly we still have some breaking change in
>>>>>>> FieldAndMethodAccessMode
>>>>>>> constructor/behavior? Can we get it fixed?
>>>>>>> In terms of design, jsonb access mode was originally thought as
>>>>>> delegating
>>>>>>> the research of the writers/readers to a 3rd party access mode and
>>>> just
>>>>>>> refilter on top of it with jsonb rules.
>>>>>>> Now it is kind of mixed in the code (inheritance + composition vs just
>>>>>>> composition), the reason it was not done was to avoid to expose the
>>>>>> default
>>>>>>> delegate access mode when a 4rd party access mode was using and mix
>>>> them
>>>>>> in
>>>>>>> the jsonbaccess mode, not 100% sure if we want to keep the original
>>>>>> design
>>>>>>> there or not but mentionning it since it now prevent us to change it
>>>>>> later.
>>>>>>>
>>>>>>> Overall except the missing constructor it looks ok to me, happy to get
>>>>>>> thoughts on the access mode pattern too.
>>>>>>>
>>>>>>> Romain Manni-Bucau
>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>> https://github.com/rmannibucau> |
>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>> <
>>>>>>
>>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
>>>> a
>>>>>>> écrit :
>>>>>>>
>>>>>>>> here we go. plz take a look!
>>>>>>>>
>>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>>>>>>>
>>>>>>>> txs and LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>
>>>>>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
>>>> <struberg@yahoo.de.INVALID
>>>>>>>>> :
>>>>>>>>>
>>>>>>>>> Looked at your proposed solution, but it might be tricky to get it
>>>>>>>> working. The problem is that we have two modules: mapper and jsonb.
>>>> And
>>>>>> in
>>>>>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
>>>> defined
>>>>>> and
>>>>>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>>>>>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
>>>> actually
>>>>>>>> did break the mapper. Sorry that I did not catch this earlier.
>>>>>>>>>
>>>>>>>>> Note that the FieldAndMethodAccessMode is only used if no other
>>>>>>>> AccessMode is configured. There is also a JsonbAccessMode which
>>>>>> delegates
>>>>>>>> through to the accessMode of the Mapper. Normally means the
>>>>>>>> FieldAndMethodAccessMode, but could be different.
>>>>>>>>> We could implement the logic in JsonbAccessMode, totally ditching
>>>> the
>>>>>>>> accessMode coming from the Mapper. Also thought about having the
>>>>>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>>>>>>>> handling as parameter over there. still not really perfect imo.
>>>>>>>>>
>>>>>>>>> Will try to ship a patch proposal tomorrow.
>>>>>>>>>
>>>>>>>>> LieGrue,
>>>>>>>>> strub
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>>>>>>>> rmannibucau@gmail.com>:
>>>>>>>>>>
>>>>>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
>>>> <struberg@yahoo.de.invalid
>>>>>>>> <ma...@yahoo.de.invalid>> a
>>>>>>>>>> écrit :
>>>>>>>>>>
>>>>>>>>>>> +1 for getting rid of java.beans.
>>>>>>>>>>>
>>>>>>>>>>> I've created a spec ticket for the open questions:
>>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>>>>>>>
>>>>>>>>>>> My point is: until my fix we clearly did break 4.6. And class
>>>>>>>> hierarchies
>>>>>>>>>>> are totally not portable right now anyway. Introducing It's just
>>>>>> badly
>>>>>>>>>>> underspecified.
>>>>>>>>>>> So I do not see why the current implementation does make it worse.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Well this one is easy to answer: it changes the behavior and breaks
>>>>>>>> users
>>>>>>>>>> so it is not an option for a patch version and to be honest the
>>>>>> proposed
>>>>>>>>>> fix solves it for both cases so think we should tackle it this
>>>> way, at
>>>>>>>>>> least for now until the spec clarifies it any other way.
>>>>>>>>>> Once again, the source of the issue is more that we lack an
>>>>>> integration
>>>>>>>>>> between mapper and jsonb modules than anything else and this
>>>>>> integration
>>>>>>>>>> had kind of been done but broke mapper so think we should fix it
>>>> back,
>>>>>>>> just
>>>>>>>>>> costs us a constructor we could have avoided without this issue
>>>> AFAIK
>>>>>> so
>>>>>>>>>> not a big deal, no?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> LieGrue,
>>>>>>>>>>> strub
>>>>>>>>>>>
>>>>>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>>>>>>>> rmannibucau@gmail.com
>>>>>>>>>>>> :
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>>>>>>>> access
>>>>>>>>>>>> mode does not enable the visibility to see all readers/writers
>>>> cause
>>>>>>>>>>> there
>>>>>>>>>>>> is some hardcoded filtering in the delegates so I think the
>>>> trick is
>>>>>>>> to
>>>>>>>>>>>> simply add a toggle to the delegates access modes we use to not
>>>> have
>>>>>>>> this
>>>>>>>>>>>> filtering and 100% rely on the visibility filtering in
>>>> jsonbaccess
>>>>>>>> mode
>>>>>>>>>>> we
>>>>>>>>>>>> already have in place.
>>>>>>>>>>>>
>>>>>>>>>>>> Before the release we must also ensure getters computation cache
>>>> is
>>>>>>>>>>> cleaned
>>>>>>>>>>>> up after the ClassMapping is created - we don't want to keep
>>>> that in
>>>>>>>>>>> memory
>>>>>>>>>>>> at runtime and not sure we can clean up at another time since we
>>>>>> don't
>>>>>>>>>>> want
>>>>>>>>>>>> a background thread for that.
>>>>>>>>>>>> I also think it is not worth to depends on java.beans
>>>> (java.desktop
>>>>>>>>>>> module)
>>>>>>>>>>>> so we would reimpl decapitalize as we already did somewhere else
>>>> (it
>>>>>>>> is
>>>>>>>>>>> one
>>>>>>>>>>>> liner anyway ;)).
>>>>>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must
>>>> be
>>>>>>>>>>> restored
>>>>>>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>>>>>>>> mentionned
>>>>>>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep
>>>> the
>>>>>>>>>>>> backward compat) to ignore the filtering.
>>>>>>>>>>>>
>>>>>>>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>>>>>>>
>>>>>>>>>>>> Romain Manni-Bucau
>>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>>>>>>> https://github.com/rmannibucau> |
>>>>>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>>>>>> <
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
>>>>>> <struberg@yahoo.de.invalid
>>>>>>>>>
>>>>>>>>>>> a
>>>>>>>>>>>> écrit :
>>>>>>>>>>>>
>>>>>>>>>>>>> To sum this up:
>>>>>>>>>>>>>
>>>>>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
>>>> what
>>>>>>>>>>> happens
>>>>>>>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>>>>>>>
>>>>>>>>>>>>> In 3.7.1 it states that
>>>>>>>>>>>>> For a serialization operation, if a matching public getter
>>>> method
>>>>>>>>>>> exists,
>>>>>>>>>>>>> the method is called to obtain the value of the property. If a
>>>>>>>> matching
>>>>>>>>>>>>> getter method with private, protected, or defaulted to
>>>> package-only
>>>>>>>>>>> access
>>>>>>>>>>>>> exists, then this field is ignored. If no matching getter method
>>>>>>>> exists
>>>>>>>>>>> and
>>>>>>>>>>>>> the field is public, then the value is obtained directly from
>>>> the
>>>>>>>> field.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>>>>>>>
>>>>>>>>>>>>> 4.6 Custom visibility
>>>>>>>>>>>>> To customize scope and field access strategy as specified in
>>>>>> section
>>>>>>>>>>>>> 3.7.1, it is possible to specify
>>>>>>>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>>>>>>>> annotation or to override default behavior globally calling
>>>>>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
>>>>>> custom
>>>>>>>>>>>>> property visibility strategy.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is
>>>> simply
>>>>>> not
>>>>>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>>>>>>>> comes to
>>>>>>>>>>>>> class hierarchies
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>>>> <
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> So either we say that the spec simply does not define what
>>>> should
>>>>>>>> happen
>>>>>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
>>>>>> ditch
>>>>>>>>>>> our
>>>>>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
>>>>>> possible
>>>>>>>> to
>>>>>>>>>>>>> implement it that way.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The reason why I removed the logic from the Reader is the
>>>>>> following:
>>>>>>>>>>> Even
>>>>>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
>>>>>> property'
>>>>>>>>>>> you
>>>>>>>>>>>>> STILL are perfectly allowed - as per the spec - to have
>>>> Visibility
>>>>>>>>>>>>> strategies which do disable ALL methods and ONLY use field
>>>> access.
>>>>>>>>>>>>> That means the current impl is surely more correct than before,
>>>> but
>>>>>>>> not
>>>>>>>>>>>>> yet perfect.
>>>>>>>>>>>>>
>>>>>>>>>>>>> LieGrue,
>>>>>>>>>>>>> strub
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>>>>>>>> rmannibucau@gmail.com
>>>>>>>>>>>>>> :
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi JL,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Last commit broke some stuff.
>>>>>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few
>>>> days,
>>>>>>>> can it
>>>>>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>>>>>>>> a écrit :
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>>>>>>>> http://www.tomitribe.com
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
took your PR and enhanced on top of it.
Plz review!
txs and LieGrue,
strub
> Am 10.04.2022 um 20:08 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Created https://github.com/apache/johnzon/pull/83 to try to share what I
> meant and try to solve it to enable the release (note I did it a bit in a
> hurry, the coming weeks are quite busy for me - so happy to get a second
> pair of eyes + optionally somebody running the jsonb tck)
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>
> Le dim. 10 avr. 2022 à 18:42, Romain Manni-Bucau <rm...@gmail.com> a
> écrit :
>
>>
>>
>> Le dim. 10 avr. 2022 à 17:34, Mark Struberg <st...@yahoo.de.invalid> a
>> écrit :
>>
>>> Oki will re-introduce that ct.
>>> Should then be backward compat, isn't?
>>>
>>
>> If the modifier check is there back yep
>>
>>
>>> LieGrue,
>>> strub
>>>
>>>
>>>> Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <
>>> rmannibucau@gmail.com>:
>>>>
>>>> Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid>
>>> a
>>>> écrit :
>>>>
>>>>> Actually there is not much diff. There is one ct which was only used in
>>>>> JsonbAccessMode as true. Everywhere else it was feed as false.
>>>>> Now we could keep this flag and feed it as default if you really like.
>>>>> That should be rather easy to do if you think it's necessary. But
>>> still I'd
>>>>> mark that 4param ct as @Deprecated. Wdyt?
>>>>>
>>>>
>>>> Well it is used by users writing custom access mode quite often so we
>>> must
>>>> keep it and its associated behavior as a minimum for coming release.
>>>> No issue to make an abstract class with more children to keep it
>>> simpler in
>>>> the code but please dont break it.
>>>>
>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
>>> rmannibucau@gmail.com
>>>>>> :
>>>>>>
>>>>>> Hi Mark,
>>>>>>
>>>>>> If I read the diff correctly we still have some breaking change in
>>>>>> FieldAndMethodAccessMode
>>>>>> constructor/behavior? Can we get it fixed?
>>>>>> In terms of design, jsonb access mode was originally thought as
>>>>> delegating
>>>>>> the research of the writers/readers to a 3rd party access mode and
>>> just
>>>>>> refilter on top of it with jsonb rules.
>>>>>> Now it is kind of mixed in the code (inheritance + composition vs just
>>>>>> composition), the reason it was not done was to avoid to expose the
>>>>> default
>>>>>> delegate access mode when a 4rd party access mode was using and mix
>>> them
>>>>> in
>>>>>> the jsonbaccess mode, not 100% sure if we want to keep the original
>>>>> design
>>>>>> there or not but mentionning it since it now prevent us to change it
>>>>> later.
>>>>>>
>>>>>> Overall except the missing constructor it looks ok to me, happy to get
>>>>>> thoughts on the access mode pattern too.
>>>>>>
>>>>>> Romain Manni-Bucau
>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>> https://github.com/rmannibucau> |
>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>> <
>>>>>
>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
>>> a
>>>>>> écrit :
>>>>>>
>>>>>>> here we go. plz take a look!
>>>>>>>
>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>>>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>>>>>>
>>>>>>> txs and LieGrue,
>>>>>>> strub
>>>>>>>
>>>>>>>
>>>>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
>>> <struberg@yahoo.de.INVALID
>>>>>>>> :
>>>>>>>>
>>>>>>>> Looked at your proposed solution, but it might be tricky to get it
>>>>>>> working. The problem is that we have two modules: mapper and jsonb.
>>> And
>>>>> in
>>>>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
>>> defined
>>>>> and
>>>>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>>>>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
>>> actually
>>>>>>> did break the mapper. Sorry that I did not catch this earlier.
>>>>>>>>
>>>>>>>> Note that the FieldAndMethodAccessMode is only used if no other
>>>>>>> AccessMode is configured. There is also a JsonbAccessMode which
>>>>> delegates
>>>>>>> through to the accessMode of the Mapper. Normally means the
>>>>>>> FieldAndMethodAccessMode, but could be different.
>>>>>>>> We could implement the logic in JsonbAccessMode, totally ditching
>>> the
>>>>>>> accessMode coming from the Mapper. Also thought about having the
>>>>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>>>>>>> handling as parameter over there. still not really perfect imo.
>>>>>>>>
>>>>>>>> Will try to ship a patch proposal tomorrow.
>>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>>>>>>> rmannibucau@gmail.com>:
>>>>>>>>>
>>>>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
>>> <struberg@yahoo.de.invalid
>>>>>>> <ma...@yahoo.de.invalid>> a
>>>>>>>>> écrit :
>>>>>>>>>
>>>>>>>>>> +1 for getting rid of java.beans.
>>>>>>>>>>
>>>>>>>>>> I've created a spec ticket for the open questions:
>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>>>>>>
>>>>>>>>>> My point is: until my fix we clearly did break 4.6. And class
>>>>>>> hierarchies
>>>>>>>>>> are totally not portable right now anyway. Introducing It's just
>>>>> badly
>>>>>>>>>> underspecified.
>>>>>>>>>> So I do not see why the current implementation does make it worse.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Well this one is easy to answer: it changes the behavior and breaks
>>>>>>> users
>>>>>>>>> so it is not an option for a patch version and to be honest the
>>>>> proposed
>>>>>>>>> fix solves it for both cases so think we should tackle it this
>>> way, at
>>>>>>>>> least for now until the spec clarifies it any other way.
>>>>>>>>> Once again, the source of the issue is more that we lack an
>>>>> integration
>>>>>>>>> between mapper and jsonb modules than anything else and this
>>>>> integration
>>>>>>>>> had kind of been done but broke mapper so think we should fix it
>>> back,
>>>>>>> just
>>>>>>>>> costs us a constructor we could have avoided without this issue
>>> AFAIK
>>>>> so
>>>>>>>>> not a big deal, no?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> LieGrue,
>>>>>>>>>> strub
>>>>>>>>>>
>>>>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>>>>>>> rmannibucau@gmail.com
>>>>>>>>>>> :
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>>>>>>> access
>>>>>>>>>>> mode does not enable the visibility to see all readers/writers
>>> cause
>>>>>>>>>> there
>>>>>>>>>>> is some hardcoded filtering in the delegates so I think the
>>> trick is
>>>>>>> to
>>>>>>>>>>> simply add a toggle to the delegates access modes we use to not
>>> have
>>>>>>> this
>>>>>>>>>>> filtering and 100% rely on the visibility filtering in
>>> jsonbaccess
>>>>>>> mode
>>>>>>>>>> we
>>>>>>>>>>> already have in place.
>>>>>>>>>>>
>>>>>>>>>>> Before the release we must also ensure getters computation cache
>>> is
>>>>>>>>>> cleaned
>>>>>>>>>>> up after the ClassMapping is created - we don't want to keep
>>> that in
>>>>>>>>>> memory
>>>>>>>>>>> at runtime and not sure we can clean up at another time since we
>>>>> don't
>>>>>>>>>> want
>>>>>>>>>>> a background thread for that.
>>>>>>>>>>> I also think it is not worth to depends on java.beans
>>> (java.desktop
>>>>>>>>>> module)
>>>>>>>>>>> so we would reimpl decapitalize as we already did somewhere else
>>> (it
>>>>>>> is
>>>>>>>>>> one
>>>>>>>>>>> liner anyway ;)).
>>>>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must
>>> be
>>>>>>>>>> restored
>>>>>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>>>>>>> mentionned
>>>>>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep
>>> the
>>>>>>>>>>> backward compat) to ignore the filtering.
>>>>>>>>>>>
>>>>>>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>>>>>>
>>>>>>>>>>> Romain Manni-Bucau
>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>>>>>> https://github.com/rmannibucau> |
>>>>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>>>>> <
>>>>>>>>>>
>>>>>>>
>>>>>
>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
>>>>> <struberg@yahoo.de.invalid
>>>>>>>>
>>>>>>>>>> a
>>>>>>>>>>> écrit :
>>>>>>>>>>>
>>>>>>>>>>>> To sum this up:
>>>>>>>>>>>>
>>>>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
>>> what
>>>>>>>>>> happens
>>>>>>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>>>>>>
>>>>>>>>>>>> In 3.7.1 it states that
>>>>>>>>>>>> For a serialization operation, if a matching public getter
>>> method
>>>>>>>>>> exists,
>>>>>>>>>>>> the method is called to obtain the value of the property. If a
>>>>>>> matching
>>>>>>>>>>>> getter method with private, protected, or defaulted to
>>> package-only
>>>>>>>>>> access
>>>>>>>>>>>> exists, then this field is ignored. If no matching getter method
>>>>>>> exists
>>>>>>>>>> and
>>>>>>>>>>>> the field is public, then the value is obtained directly from
>>> the
>>>>>>> field.
>>>>>>>>>>>>
>>>>>>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>>>>>>
>>>>>>>>>>>> 4.6 Custom visibility
>>>>>>>>>>>> To customize scope and field access strategy as specified in
>>>>> section
>>>>>>>>>>>> 3.7.1, it is possible to specify
>>>>>>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>>>>>>> annotation or to override default behavior globally calling
>>>>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
>>>>> custom
>>>>>>>>>>>> property visibility strategy.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is
>>> simply
>>>>> not
>>>>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>>>>>>> comes to
>>>>>>>>>>>> class hierarchies
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>>> <
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> So either we say that the spec simply does not define what
>>> should
>>>>>>> happen
>>>>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
>>>>> ditch
>>>>>>>>>> our
>>>>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
>>>>> possible
>>>>>>> to
>>>>>>>>>>>> implement it that way.
>>>>>>>>>>>>
>>>>>>>>>>>> The reason why I removed the logic from the Reader is the
>>>>> following:
>>>>>>>>>> Even
>>>>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
>>>>> property'
>>>>>>>>>> you
>>>>>>>>>>>> STILL are perfectly allowed - as per the spec - to have
>>> Visibility
>>>>>>>>>>>> strategies which do disable ALL methods and ONLY use field
>>> access.
>>>>>>>>>>>> That means the current impl is surely more correct than before,
>>> but
>>>>>>> not
>>>>>>>>>>>> yet perfect.
>>>>>>>>>>>>
>>>>>>>>>>>> LieGrue,
>>>>>>>>>>>> strub
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>>>>>>> rmannibucau@gmail.com
>>>>>>>>>>>>> :
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi JL,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Last commit broke some stuff.
>>>>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few
>>> days,
>>>>>>> can it
>>>>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>>>>>>> a écrit :
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>>>>>>> http://www.tomitribe.com
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Created https://github.com/apache/johnzon/pull/83 to try to share what I
meant and try to solve it to enable the release (note I did it a bit in a
hurry, the coming weeks are quite busy for me - so happy to get a second
pair of eyes + optionally somebody running the jsonb tck)
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Le dim. 10 avr. 2022 à 18:42, Romain Manni-Bucau <rm...@gmail.com> a
écrit :
>
>
> Le dim. 10 avr. 2022 à 17:34, Mark Struberg <st...@yahoo.de.invalid> a
> écrit :
>
>> Oki will re-introduce that ct.
>> Should then be backward compat, isn't?
>>
>
> If the modifier check is there back yep
>
>
>> LieGrue,
>> strub
>>
>>
>> > Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com>:
>> >
>> > Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid>
>> a
>> > écrit :
>> >
>> >> Actually there is not much diff. There is one ct which was only used in
>> >> JsonbAccessMode as true. Everywhere else it was feed as false.
>> >> Now we could keep this flag and feed it as default if you really like.
>> >> That should be rather easy to do if you think it's necessary. But
>> still I'd
>> >> mark that 4param ct as @Deprecated. Wdyt?
>> >>
>> >
>> > Well it is used by users writing custom access mode quite often so we
>> must
>> > keep it and its associated behavior as a minimum for coming release.
>> > No issue to make an abstract class with more children to keep it
>> simpler in
>> > the code but please dont break it.
>> >
>> >
>> >> LieGrue,
>> >> strub
>> >>
>> >>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com
>> >>> :
>> >>>
>> >>> Hi Mark,
>> >>>
>> >>> If I read the diff correctly we still have some breaking change in
>> >>> FieldAndMethodAccessMode
>> >>> constructor/behavior? Can we get it fixed?
>> >>> In terms of design, jsonb access mode was originally thought as
>> >> delegating
>> >>> the research of the writers/readers to a 3rd party access mode and
>> just
>> >>> refilter on top of it with jsonb rules.
>> >>> Now it is kind of mixed in the code (inheritance + composition vs just
>> >>> composition), the reason it was not done was to avoid to expose the
>> >> default
>> >>> delegate access mode when a 4rd party access mode was using and mix
>> them
>> >> in
>> >>> the jsonbaccess mode, not 100% sure if we want to keep the original
>> >> design
>> >>> there or not but mentionning it since it now prevent us to change it
>> >> later.
>> >>>
>> >>> Overall except the missing constructor it looks ok to me, happy to get
>> >>> thoughts on the access mode pattern too.
>> >>>
>> >>> Romain Manni-Bucau
>> >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> >>> <https://rmannibucau.metawerx.net/> | Old Blog
>> >>> <http://rmannibucau.wordpress.com> | Github <
>> >> https://github.com/rmannibucau> |
>> >>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> >>> <
>> >>
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>> >>>
>> >>>
>> >>>
>> >>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
>> a
>> >>> écrit :
>> >>>
>> >>>> here we go. plz take a look!
>> >>>>
>> >>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>> >>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>> >>>>
>> >>>> txs and LieGrue,
>> >>>> strub
>> >>>>
>> >>>>
>> >>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
>> <struberg@yahoo.de.INVALID
>> >>>>> :
>> >>>>>
>> >>>>> Looked at your proposed solution, but it might be tricky to get it
>> >>>> working. The problem is that we have two modules: mapper and jsonb.
>> And
>> >> in
>> >>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
>> defined
>> >> and
>> >>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>> >>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
>> actually
>> >>>> did break the mapper. Sorry that I did not catch this earlier.
>> >>>>>
>> >>>>> Note that the FieldAndMethodAccessMode is only used if no other
>> >>>> AccessMode is configured. There is also a JsonbAccessMode which
>> >> delegates
>> >>>> through to the accessMode of the Mapper. Normally means the
>> >>>> FieldAndMethodAccessMode, but could be different.
>> >>>>> We could implement the logic in JsonbAccessMode, totally ditching
>> the
>> >>>> accessMode coming from the Mapper. Also thought about having the
>> >>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>> >>>> handling as parameter over there. still not really perfect imo.
>> >>>>>
>> >>>>> Will try to ship a patch proposal tomorrow.
>> >>>>>
>> >>>>> LieGrue,
>> >>>>> strub
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>> >>>> rmannibucau@gmail.com>:
>> >>>>>>
>> >>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
>> <struberg@yahoo.de.invalid
>> >>>> <ma...@yahoo.de.invalid>> a
>> >>>>>> écrit :
>> >>>>>>
>> >>>>>>> +1 for getting rid of java.beans.
>> >>>>>>>
>> >>>>>>> I've created a spec ticket for the open questions:
>> >>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>> >>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>> >>>>>>>
>> >>>>>>> My point is: until my fix we clearly did break 4.6. And class
>> >>>> hierarchies
>> >>>>>>> are totally not portable right now anyway. Introducing It's just
>> >> badly
>> >>>>>>> underspecified.
>> >>>>>>> So I do not see why the current implementation does make it worse.
>> >>>>>>>
>> >>>>>>
>> >>>>>> Well this one is easy to answer: it changes the behavior and breaks
>> >>>> users
>> >>>>>> so it is not an option for a patch version and to be honest the
>> >> proposed
>> >>>>>> fix solves it for both cases so think we should tackle it this
>> way, at
>> >>>>>> least for now until the spec clarifies it any other way.
>> >>>>>> Once again, the source of the issue is more that we lack an
>> >> integration
>> >>>>>> between mapper and jsonb modules than anything else and this
>> >> integration
>> >>>>>> had kind of been done but broke mapper so think we should fix it
>> back,
>> >>>> just
>> >>>>>> costs us a constructor we could have avoided without this issue
>> AFAIK
>> >> so
>> >>>>>> not a big deal, no?
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> LieGrue,
>> >>>>>>> strub
>> >>>>>>>
>> >>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>> >>>> rmannibucau@gmail.com
>> >>>>>>>> :
>> >>>>>>>>
>> >>>>>>>> Hi,
>> >>>>>>>>
>> >>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>> >>>> access
>> >>>>>>>> mode does not enable the visibility to see all readers/writers
>> cause
>> >>>>>>> there
>> >>>>>>>> is some hardcoded filtering in the delegates so I think the
>> trick is
>> >>>> to
>> >>>>>>>> simply add a toggle to the delegates access modes we use to not
>> have
>> >>>> this
>> >>>>>>>> filtering and 100% rely on the visibility filtering in
>> jsonbaccess
>> >>>> mode
>> >>>>>>> we
>> >>>>>>>> already have in place.
>> >>>>>>>>
>> >>>>>>>> Before the release we must also ensure getters computation cache
>> is
>> >>>>>>> cleaned
>> >>>>>>>> up after the ClassMapping is created - we don't want to keep
>> that in
>> >>>>>>> memory
>> >>>>>>>> at runtime and not sure we can clean up at another time since we
>> >> don't
>> >>>>>>> want
>> >>>>>>>> a background thread for that.
>> >>>>>>>> I also think it is not worth to depends on java.beans
>> (java.desktop
>> >>>>>>> module)
>> >>>>>>>> so we would reimpl decapitalize as we already did somewhere else
>> (it
>> >>>> is
>> >>>>>>> one
>> >>>>>>>> liner anyway ;)).
>> >>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must
>> be
>> >>>>>>> restored
>> >>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>> >>>>>>> mentionned
>> >>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep
>> the
>> >>>>>>>> backward compat) to ignore the filtering.
>> >>>>>>>>
>> >>>>>>>> Sounds like this way we solve all issues and can release.
>> >>>>>>>>
>> >>>>>>>> Romain Manni-Bucau
>> >>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> >>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>> >>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>> >>>>>>> https://github.com/rmannibucau> |
>> >>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> >>>>>>>> <
>> >>>>>>>
>> >>>>
>> >>
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
>> >> <struberg@yahoo.de.invalid
>> >>>>>
>> >>>>>>> a
>> >>>>>>>> écrit :
>> >>>>>>>>
>> >>>>>>>>> To sum this up:
>> >>>>>>>>>
>> >>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
>> what
>> >>>>>>> happens
>> >>>>>>>>> if there are class hierarchies like class A extends B.
>> >>>>>>>>>
>> >>>>>>>>> In 3.7.1 it states that
>> >>>>>>>>> For a serialization operation, if a matching public getter
>> method
>> >>>>>>> exists,
>> >>>>>>>>> the method is called to obtain the value of the property. If a
>> >>>> matching
>> >>>>>>>>> getter method with private, protected, or defaulted to
>> package-only
>> >>>>>>> access
>> >>>>>>>>> exists, then this field is ignored. If no matching getter method
>> >>>> exists
>> >>>>>>> and
>> >>>>>>>>> the field is public, then the value is obtained directly from
>> the
>> >>>> field.
>> >>>>>>>>>
>> >>>>>>>>> This can get changed y using a @JsonbVisibility
>> >>>>>>>>>
>> >>>>>>>>> 4.6 Custom visibility
>> >>>>>>>>> To customize scope and field access strategy as specified in
>> >> section
>> >>>>>>>>> 3.7.1, it is possible to specify
>> >>>>>>> javax.json.bind.annotation.JsonbVisibility
>> >>>>>>>>> annotation or to override default behavior globally calling
>> >>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
>> >> custom
>> >>>>>>>>> property visibility strategy.
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> The problem is that that the PropertyVisibilityStrategy is
>> simply
>> >> not
>> >>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>> >>>> comes to
>> >>>>>>>>> class hierarchies
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> >>>>
>> >>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>> >>>>>>>>> <
>> >>>>>>>>>
>> >>>>>>>
>> >>>>
>> >>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> So either we say that the spec simply does not define what
>> should
>> >>>> happen
>> >>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
>> >> ditch
>> >>>>>>> our
>> >>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
>> >> possible
>> >>>> to
>> >>>>>>>>> implement it that way.
>> >>>>>>>>>
>> >>>>>>>>> The reason why I removed the logic from the Reader is the
>> >> following:
>> >>>>>>> Even
>> >>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
>> >> property'
>> >>>>>>> you
>> >>>>>>>>> STILL are perfectly allowed - as per the spec - to have
>> Visibility
>> >>>>>>>>> strategies which do disable ALL methods and ONLY use field
>> access.
>> >>>>>>>>> That means the current impl is surely more correct than before,
>> but
>> >>>> not
>> >>>>>>>>> yet perfect.
>> >>>>>>>>>
>> >>>>>>>>> LieGrue,
>> >>>>>>>>> strub
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>> >>>>>>> rmannibucau@gmail.com
>> >>>>>>>>>> :
>> >>>>>>>>>>
>> >>>>>>>>>> Hi JL,
>> >>>>>>>>>>
>> >>>>>>>>>> Last commit broke some stuff.
>> >>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few
>> days,
>> >>>> can it
>> >>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>> >>>>>>>>>>
>> >>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>> >>>>>>>>> jlmonteiro@tomitribe.com>
>> >>>>>>>>>> a écrit :
>> >>>>>>>>>>
>> >>>>>>>>>>> Hi all,
>> >>>>>>>>>>>
>> >>>>>>>>>>> Is it ok to release a 1.2.17?
>> >>>>>>>>>>> I'm looking for a Jakarta EE fix -
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> >>>>
>> >>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>> >>>>>>>>>>>
>> >>>>>>>>>>> Do you guys have some additional fixes to push?
>> >>>>>>>>>>> --
>> >>>>>>>>>>> Jean-Louis Monteiro
>> >>>>>>>>>>> http://twitter.com/jlouismonteiro
>> >>>>>>>>>>> http://www.tomitribe.com
>> >>>>>
>> >>>>
>> >>>>
>> >>
>> >>
>>
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le dim. 10 avr. 2022 à 17:34, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> Oki will re-introduce that ct.
> Should then be backward compat, isn't?
>
If the modifier check is there back yep
> LieGrue,
> strub
>
>
> > Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
> >:
> >
> > Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid>
> a
> > écrit :
> >
> >> Actually there is not much diff. There is one ct which was only used in
> >> JsonbAccessMode as true. Everywhere else it was feed as false.
> >> Now we could keep this flag and feed it as default if you really like.
> >> That should be rather easy to do if you think it's necessary. But still
> I'd
> >> mark that 4param ct as @Deprecated. Wdyt?
> >>
> >
> > Well it is used by users writing custom access mode quite often so we
> must
> > keep it and its associated behavior as a minimum for coming release.
> > No issue to make an abstract class with more children to keep it simpler
> in
> > the code but please dont break it.
> >
> >
> >> LieGrue,
> >> strub
> >>
> >>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com
> >>> :
> >>>
> >>> Hi Mark,
> >>>
> >>> If I read the diff correctly we still have some breaking change in
> >>> FieldAndMethodAccessMode
> >>> constructor/behavior? Can we get it fixed?
> >>> In terms of design, jsonb access mode was originally thought as
> >> delegating
> >>> the research of the writers/readers to a 3rd party access mode and just
> >>> refilter on top of it with jsonb rules.
> >>> Now it is kind of mixed in the code (inheritance + composition vs just
> >>> composition), the reason it was not done was to avoid to expose the
> >> default
> >>> delegate access mode when a 4rd party access mode was using and mix
> them
> >> in
> >>> the jsonbaccess mode, not 100% sure if we want to keep the original
> >> design
> >>> there or not but mentionning it since it now prevent us to change it
> >> later.
> >>>
> >>> Overall except the missing constructor it looks ok to me, happy to get
> >>> thoughts on the access mode pattern too.
> >>>
> >>> Romain Manni-Bucau
> >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>> <http://rmannibucau.wordpress.com> | Github <
> >> https://github.com/rmannibucau> |
> >>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>> <
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>
> >>>
> >>>
> >>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
> a
> >>> écrit :
> >>>
> >>>> here we go. plz take a look!
> >>>>
> >>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> >>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
> >>>>
> >>>> txs and LieGrue,
> >>>> strub
> >>>>
> >>>>
> >>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
> <struberg@yahoo.de.INVALID
> >>>>> :
> >>>>>
> >>>>> Looked at your proposed solution, but it might be tricky to get it
> >>>> working. The problem is that we have two modules: mapper and jsonb.
> And
> >> in
> >>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
> defined
> >> and
> >>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> >>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
> actually
> >>>> did break the mapper. Sorry that I did not catch this earlier.
> >>>>>
> >>>>> Note that the FieldAndMethodAccessMode is only used if no other
> >>>> AccessMode is configured. There is also a JsonbAccessMode which
> >> delegates
> >>>> through to the accessMode of the Mapper. Normally means the
> >>>> FieldAndMethodAccessMode, but could be different.
> >>>>> We could implement the logic in JsonbAccessMode, totally ditching the
> >>>> accessMode coming from the Mapper. Also thought about having the
> >>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
> >>>> handling as parameter over there. still not really perfect imo.
> >>>>>
> >>>>> Will try to ship a patch proposal tomorrow.
> >>>>>
> >>>>> LieGrue,
> >>>>> strub
> >>>>>
> >>>>>
> >>>>>
> >>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> >>>> rmannibucau@gmail.com>:
> >>>>>>
> >>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
> <struberg@yahoo.de.invalid
> >>>> <ma...@yahoo.de.invalid>> a
> >>>>>> écrit :
> >>>>>>
> >>>>>>> +1 for getting rid of java.beans.
> >>>>>>>
> >>>>>>> I've created a spec ticket for the open questions:
> >>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>>>>>
> >>>>>>> My point is: until my fix we clearly did break 4.6. And class
> >>>> hierarchies
> >>>>>>> are totally not portable right now anyway. Introducing It's just
> >> badly
> >>>>>>> underspecified.
> >>>>>>> So I do not see why the current implementation does make it worse.
> >>>>>>>
> >>>>>>
> >>>>>> Well this one is easy to answer: it changes the behavior and breaks
> >>>> users
> >>>>>> so it is not an option for a patch version and to be honest the
> >> proposed
> >>>>>> fix solves it for both cases so think we should tackle it this way,
> at
> >>>>>> least for now until the spec clarifies it any other way.
> >>>>>> Once again, the source of the issue is more that we lack an
> >> integration
> >>>>>> between mapper and jsonb modules than anything else and this
> >> integration
> >>>>>> had kind of been done but broke mapper so think we should fix it
> back,
> >>>> just
> >>>>>> costs us a constructor we could have avoided without this issue
> AFAIK
> >> so
> >>>>>> not a big deal, no?
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> LieGrue,
> >>>>>>> strub
> >>>>>>>
> >>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> >>>> rmannibucau@gmail.com
> >>>>>>>> :
> >>>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> From what we discussed on this issue, the thing is that our jsonb
> >>>> access
> >>>>>>>> mode does not enable the visibility to see all readers/writers
> cause
> >>>>>>> there
> >>>>>>>> is some hardcoded filtering in the delegates so I think the trick
> is
> >>>> to
> >>>>>>>> simply add a toggle to the delegates access modes we use to not
> have
> >>>> this
> >>>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
> >>>> mode
> >>>>>>> we
> >>>>>>>> already have in place.
> >>>>>>>>
> >>>>>>>> Before the release we must also ensure getters computation cache
> is
> >>>>>>> cleaned
> >>>>>>>> up after the ClassMapping is created - we don't want to keep that
> in
> >>>>>>> memory
> >>>>>>>> at runtime and not sure we can clean up at another time since we
> >> don't
> >>>>>>> want
> >>>>>>>> a background thread for that.
> >>>>>>>> I also think it is not worth to depends on java.beans
> (java.desktop
> >>>>>>> module)
> >>>>>>>> so we would reimpl decapitalize as we already did somewhere else
> (it
> >>>> is
> >>>>>>> one
> >>>>>>>> liner anyway ;)).
> >>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
> >>>>>>> restored
> >>>>>>>> (it is part of our API and used), default should stay 1-1 but as
> >>>>>>> mentionned
> >>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep
> the
> >>>>>>>> backward compat) to ignore the filtering.
> >>>>>>>>
> >>>>>>>> Sounds like this way we solve all issues and can release.
> >>>>>>>>
> >>>>>>>> Romain Manni-Bucau
> >>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>>>> https://github.com/rmannibucau> |
> >>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>> <
> >>>>>>>
> >>>>
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
> >> <struberg@yahoo.de.invalid
> >>>>>
> >>>>>>> a
> >>>>>>>> écrit :
> >>>>>>>>
> >>>>>>>>> To sum this up:
> >>>>>>>>>
> >>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
> what
> >>>>>>> happens
> >>>>>>>>> if there are class hierarchies like class A extends B.
> >>>>>>>>>
> >>>>>>>>> In 3.7.1 it states that
> >>>>>>>>> For a serialization operation, if a matching public getter method
> >>>>>>> exists,
> >>>>>>>>> the method is called to obtain the value of the property. If a
> >>>> matching
> >>>>>>>>> getter method with private, protected, or defaulted to
> package-only
> >>>>>>> access
> >>>>>>>>> exists, then this field is ignored. If no matching getter method
> >>>> exists
> >>>>>>> and
> >>>>>>>>> the field is public, then the value is obtained directly from the
> >>>> field.
> >>>>>>>>>
> >>>>>>>>> This can get changed y using a @JsonbVisibility
> >>>>>>>>>
> >>>>>>>>> 4.6 Custom visibility
> >>>>>>>>> To customize scope and field access strategy as specified in
> >> section
> >>>>>>>>> 3.7.1, it is possible to specify
> >>>>>>> javax.json.bind.annotation.JsonbVisibility
> >>>>>>>>> annotation or to override default behavior globally calling
> >>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
> >> custom
> >>>>>>>>> property visibility strategy.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply
> >> not
> >>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
> >>>> comes to
> >>>>>>>>> class hierarchies
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>> <
> >>>>>>>>>
> >>>>>>>
> >>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> So either we say that the spec simply does not define what should
> >>>> happen
> >>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
> >> ditch
> >>>>>>> our
> >>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
> >> possible
> >>>> to
> >>>>>>>>> implement it that way.
> >>>>>>>>>
> >>>>>>>>> The reason why I removed the logic from the Reader is the
> >> following:
> >>>>>>> Even
> >>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
> >> property'
> >>>>>>> you
> >>>>>>>>> STILL are perfectly allowed - as per the spec - to have
> Visibility
> >>>>>>>>> strategies which do disable ALL methods and ONLY use field
> access.
> >>>>>>>>> That means the current impl is surely more correct than before,
> but
> >>>> not
> >>>>>>>>> yet perfect.
> >>>>>>>>>
> >>>>>>>>> LieGrue,
> >>>>>>>>> strub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>>>>>> rmannibucau@gmail.com
> >>>>>>>>>> :
> >>>>>>>>>>
> >>>>>>>>>> Hi JL,
> >>>>>>>>>>
> >>>>>>>>>> Last commit broke some stuff.
> >>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
> >>>> can it
> >>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>>>>>
> >>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>>>>>> jlmonteiro@tomitribe.com>
> >>>>>>>>>> a écrit :
> >>>>>>>>>>
> >>>>>>>>>>> Hi all,
> >>>>>>>>>>>
> >>>>>>>>>>> Is it ok to release a 1.2.17?
> >>>>>>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>
> >>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>>>>>
> >>>>>>>>>>> Do you guys have some additional fixes to push?
> >>>>>>>>>>> --
> >>>>>>>>>>> Jean-Louis Monteiro
> >>>>>>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>>>>>> http://www.tomitribe.com
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Oki will re-introduce that ct.
Should then be backward compat, isn't?
LieGrue,
strub
> Am 10.04.2022 um 17:24 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid> a
> écrit :
>
>> Actually there is not much diff. There is one ct which was only used in
>> JsonbAccessMode as true. Everywhere else it was feed as false.
>> Now we could keep this flag and feed it as default if you really like.
>> That should be rather easy to do if you think it's necessary. But still I'd
>> mark that 4param ct as @Deprecated. Wdyt?
>>
>
> Well it is used by users writing custom access mode quite often so we must
> keep it and its associated behavior as a minimum for coming release.
> No issue to make an abstract class with more children to keep it simpler in
> the code but please dont break it.
>
>
>> LieGrue,
>> strub
>>
>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>>> :
>>>
>>> Hi Mark,
>>>
>>> If I read the diff correctly we still have some breaking change in
>>> FieldAndMethodAccessMode
>>> constructor/behavior? Can we get it fixed?
>>> In terms of design, jsonb access mode was originally thought as
>> delegating
>>> the research of the writers/readers to a 3rd party access mode and just
>>> refilter on top of it with jsonb rules.
>>> Now it is kind of mixed in the code (inheritance + composition vs just
>>> composition), the reason it was not done was to avoid to expose the
>> default
>>> delegate access mode when a 4rd party access mode was using and mix them
>> in
>>> the jsonbaccess mode, not 100% sure if we want to keep the original
>> design
>>> there or not but mentionning it since it now prevent us to change it
>> later.
>>>
>>> Overall except the missing constructor it looks ok to me, happy to get
>>> thoughts on the access mode pattern too.
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github <
>> https://github.com/rmannibucau> |
>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>> <
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>
>>>
>>>
>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid> a
>>> écrit :
>>>
>>>> here we go. plz take a look!
>>>>
>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>>>
>>>> txs and LieGrue,
>>>> strub
>>>>
>>>>
>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg <struberg@yahoo.de.INVALID
>>>>> :
>>>>>
>>>>> Looked at your proposed solution, but it might be tricky to get it
>>>> working. The problem is that we have two modules: mapper and jsonb. And
>> in
>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY defined
>> and
>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually
>>>> did break the mapper. Sorry that I did not catch this earlier.
>>>>>
>>>>> Note that the FieldAndMethodAccessMode is only used if no other
>>>> AccessMode is configured. There is also a JsonbAccessMode which
>> delegates
>>>> through to the accessMode of the Mapper. Normally means the
>>>> FieldAndMethodAccessMode, but could be different.
>>>>> We could implement the logic in JsonbAccessMode, totally ditching the
>>>> accessMode coming from the Mapper. Also thought about having the
>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>>>> handling as parameter over there. still not really perfect imo.
>>>>>
>>>>> Will try to ship a patch proposal tomorrow.
>>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>>
>>>>>
>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>>>> rmannibucau@gmail.com>:
>>>>>>
>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid
>>>> <ma...@yahoo.de.invalid>> a
>>>>>> écrit :
>>>>>>
>>>>>>> +1 for getting rid of java.beans.
>>>>>>>
>>>>>>> I've created a spec ticket for the open questions:
>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>>>
>>>>>>> My point is: until my fix we clearly did break 4.6. And class
>>>> hierarchies
>>>>>>> are totally not portable right now anyway. Introducing It's just
>> badly
>>>>>>> underspecified.
>>>>>>> So I do not see why the current implementation does make it worse.
>>>>>>>
>>>>>>
>>>>>> Well this one is easy to answer: it changes the behavior and breaks
>>>> users
>>>>>> so it is not an option for a patch version and to be honest the
>> proposed
>>>>>> fix solves it for both cases so think we should tackle it this way, at
>>>>>> least for now until the spec clarifies it any other way.
>>>>>> Once again, the source of the issue is more that we lack an
>> integration
>>>>>> between mapper and jsonb modules than anything else and this
>> integration
>>>>>> had kind of been done but broke mapper so think we should fix it back,
>>>> just
>>>>>> costs us a constructor we could have avoided without this issue AFAIK
>> so
>>>>>> not a big deal, no?
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> LieGrue,
>>>>>>> strub
>>>>>>>
>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>>>> rmannibucau@gmail.com
>>>>>>>> :
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>>>> access
>>>>>>>> mode does not enable the visibility to see all readers/writers cause
>>>>>>> there
>>>>>>>> is some hardcoded filtering in the delegates so I think the trick is
>>>> to
>>>>>>>> simply add a toggle to the delegates access modes we use to not have
>>>> this
>>>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
>>>> mode
>>>>>>> we
>>>>>>>> already have in place.
>>>>>>>>
>>>>>>>> Before the release we must also ensure getters computation cache is
>>>>>>> cleaned
>>>>>>>> up after the ClassMapping is created - we don't want to keep that in
>>>>>>> memory
>>>>>>>> at runtime and not sure we can clean up at another time since we
>> don't
>>>>>>> want
>>>>>>>> a background thread for that.
>>>>>>>> I also think it is not worth to depends on java.beans (java.desktop
>>>>>>> module)
>>>>>>>> so we would reimpl decapitalize as we already did somewhere else (it
>>>> is
>>>>>>> one
>>>>>>>> liner anyway ;)).
>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>>>>>>> restored
>>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>>>> mentionned
>>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>>>>>>> backward compat) to ignore the filtering.
>>>>>>>>
>>>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>>>
>>>>>>>> Romain Manni-Bucau
>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>>> https://github.com/rmannibucau> |
>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>> <
>>>>>>>
>>>>
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
>> <struberg@yahoo.de.invalid
>>>>>
>>>>>>> a
>>>>>>>> écrit :
>>>>>>>>
>>>>>>>>> To sum this up:
>>>>>>>>>
>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>>>>>>> happens
>>>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>>>
>>>>>>>>> In 3.7.1 it states that
>>>>>>>>> For a serialization operation, if a matching public getter method
>>>>>>> exists,
>>>>>>>>> the method is called to obtain the value of the property. If a
>>>> matching
>>>>>>>>> getter method with private, protected, or defaulted to package-only
>>>>>>> access
>>>>>>>>> exists, then this field is ignored. If no matching getter method
>>>> exists
>>>>>>> and
>>>>>>>>> the field is public, then the value is obtained directly from the
>>>> field.
>>>>>>>>>
>>>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>>>
>>>>>>>>> 4.6 Custom visibility
>>>>>>>>> To customize scope and field access strategy as specified in
>> section
>>>>>>>>> 3.7.1, it is possible to specify
>>>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>>>> annotation or to override default behavior globally calling
>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
>> custom
>>>>>>>>> property visibility strategy.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply
>> not
>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>>>> comes to
>>>>>>>>> class hierarchies
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>> <
>>>>>>>>>
>>>>>>>
>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> So either we say that the spec simply does not define what should
>>>> happen
>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
>> ditch
>>>>>>> our
>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
>> possible
>>>> to
>>>>>>>>> implement it that way.
>>>>>>>>>
>>>>>>>>> The reason why I removed the logic from the Reader is the
>> following:
>>>>>>> Even
>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
>> property'
>>>>>>> you
>>>>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>>>>>>> strategies which do disable ALL methods and ONLY use field access.
>>>>>>>>> That means the current impl is surely more correct than before, but
>>>> not
>>>>>>>>> yet perfect.
>>>>>>>>>
>>>>>>>>> LieGrue,
>>>>>>>>> strub
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>>>> rmannibucau@gmail.com
>>>>>>>>>> :
>>>>>>>>>>
>>>>>>>>>> Hi JL,
>>>>>>>>>>
>>>>>>>>>> Last commit broke some stuff.
>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
>>>> can it
>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>>>
>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>>>> a écrit :
>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>>>
>>>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>>>> --
>>>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>>>> http://www.tomitribe.com
>>>>>
>>>>
>>>>
>>
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le dim. 10 avr. 2022 à 17:16, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> Actually there is not much diff. There is one ct which was only used in
> JsonbAccessMode as true. Everywhere else it was feed as false.
> Now we could keep this flag and feed it as default if you really like.
> That should be rather easy to do if you think it's necessary. But still I'd
> mark that 4param ct as @Deprecated. Wdyt?
>
Well it is used by users writing custom access mode quite often so we must
keep it and its associated behavior as a minimum for coming release.
No issue to make an abstract class with more children to keep it simpler in
the code but please dont break it.
> LieGrue,
> strub
>
> > Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
> >:
> >
> > Hi Mark,
> >
> > If I read the diff correctly we still have some breaking change in
> > FieldAndMethodAccessMode
> > constructor/behavior? Can we get it fixed?
> > In terms of design, jsonb access mode was originally thought as
> delegating
> > the research of the writers/readers to a 3rd party access mode and just
> > refilter on top of it with jsonb rules.
> > Now it is kind of mixed in the code (inheritance + composition vs just
> > composition), the reason it was not done was to avoid to expose the
> default
> > delegate access mode when a 4rd party access mode was using and mix them
> in
> > the jsonbaccess mode, not 100% sure if we want to keep the original
> design
> > there or not but mentionning it since it now prevent us to change it
> later.
> >
> > Overall except the missing constructor it looks ok to me, happy to get
> > thoughts on the access mode pattern too.
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >
> >
> > Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid> a
> > écrit :
> >
> >> here we go. plz take a look!
> >>
> >> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> >> https://github.com/struberg/johnzon/tree/JOHNZON-364>
> >>
> >> txs and LieGrue,
> >> strub
> >>
> >>
> >>> Am 09.04.2022 um 23:14 schrieb Mark Struberg <struberg@yahoo.de.INVALID
> >>> :
> >>>
> >>> Looked at your proposed solution, but it might be tricky to get it
> >> working. The problem is that we have two modules: mapper and jsonb. And
> in
> >> JOHNZON-250 we added a logic to the mapper module which is ONLY defined
> and
> >> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> >> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually
> >> did break the mapper. Sorry that I did not catch this earlier.
> >>>
> >>> Note that the FieldAndMethodAccessMode is only used if no other
> >> AccessMode is configured. There is also a JsonbAccessMode which
> delegates
> >> through to the accessMode of the Mapper. Normally means the
> >> FieldAndMethodAccessMode, but could be different.
> >>> We could implement the logic in JsonbAccessMode, totally ditching the
> >> accessMode coming from the Mapper. Also thought about having the
> >> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
> >> handling as parameter over there. still not really perfect imo.
> >>>
> >>> Will try to ship a patch proposal tomorrow.
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>
> >>>
> >>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> >> rmannibucau@gmail.com>:
> >>>>
> >>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid
> >> <ma...@yahoo.de.invalid>> a
> >>>> écrit :
> >>>>
> >>>>> +1 for getting rid of java.beans.
> >>>>>
> >>>>> I've created a spec ticket for the open questions:
> >>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>>>
> >>>>> My point is: until my fix we clearly did break 4.6. And class
> >> hierarchies
> >>>>> are totally not portable right now anyway. Introducing It's just
> badly
> >>>>> underspecified.
> >>>>> So I do not see why the current implementation does make it worse.
> >>>>>
> >>>>
> >>>> Well this one is easy to answer: it changes the behavior and breaks
> >> users
> >>>> so it is not an option for a patch version and to be honest the
> proposed
> >>>> fix solves it for both cases so think we should tackle it this way, at
> >>>> least for now until the spec clarifies it any other way.
> >>>> Once again, the source of the issue is more that we lack an
> integration
> >>>> between mapper and jsonb modules than anything else and this
> integration
> >>>> had kind of been done but broke mapper so think we should fix it back,
> >> just
> >>>> costs us a constructor we could have avoided without this issue AFAIK
> so
> >>>> not a big deal, no?
> >>>>
> >>>>
> >>>>>
> >>>>> LieGrue,
> >>>>> strub
> >>>>>
> >>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> >> rmannibucau@gmail.com
> >>>>>> :
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> From what we discussed on this issue, the thing is that our jsonb
> >> access
> >>>>>> mode does not enable the visibility to see all readers/writers cause
> >>>>> there
> >>>>>> is some hardcoded filtering in the delegates so I think the trick is
> >> to
> >>>>>> simply add a toggle to the delegates access modes we use to not have
> >> this
> >>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
> >> mode
> >>>>> we
> >>>>>> already have in place.
> >>>>>>
> >>>>>> Before the release we must also ensure getters computation cache is
> >>>>> cleaned
> >>>>>> up after the ClassMapping is created - we don't want to keep that in
> >>>>> memory
> >>>>>> at runtime and not sure we can clean up at another time since we
> don't
> >>>>> want
> >>>>>> a background thread for that.
> >>>>>> I also think it is not worth to depends on java.beans (java.desktop
> >>>>> module)
> >>>>>> so we would reimpl decapitalize as we already did somewhere else (it
> >> is
> >>>>> one
> >>>>>> liner anyway ;)).
> >>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
> >>>>> restored
> >>>>>> (it is part of our API and used), default should stay 1-1 but as
> >>>>> mentionned
> >>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
> >>>>>> backward compat) to ignore the filtering.
> >>>>>>
> >>>>>> Sounds like this way we solve all issues and can release.
> >>>>>>
> >>>>>> Romain Manni-Bucau
> >>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>> https://github.com/rmannibucau> |
> >>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>> <
> >>>>>
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
> <struberg@yahoo.de.invalid
> >>>
> >>>>> a
> >>>>>> écrit :
> >>>>>>
> >>>>>>> To sum this up:
> >>>>>>>
> >>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
> >>>>> happens
> >>>>>>> if there are class hierarchies like class A extends B.
> >>>>>>>
> >>>>>>> In 3.7.1 it states that
> >>>>>>> For a serialization operation, if a matching public getter method
> >>>>> exists,
> >>>>>>> the method is called to obtain the value of the property. If a
> >> matching
> >>>>>>> getter method with private, protected, or defaulted to package-only
> >>>>> access
> >>>>>>> exists, then this field is ignored. If no matching getter method
> >> exists
> >>>>> and
> >>>>>>> the field is public, then the value is obtained directly from the
> >> field.
> >>>>>>>
> >>>>>>> This can get changed y using a @JsonbVisibility
> >>>>>>>
> >>>>>>> 4.6 Custom visibility
> >>>>>>> To customize scope and field access strategy as specified in
> section
> >>>>>>> 3.7.1, it is possible to specify
> >>>>> javax.json.bind.annotation.JsonbVisibility
> >>>>>>> annotation or to override default behavior globally calling
> >>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
> custom
> >>>>>>> property visibility strategy.
> >>>>>>>
> >>>>>>>
> >>>>>>> The problem is that that the PropertyVisibilityStrategy is simply
> not
> >>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
> >> comes to
> >>>>>>> class hierarchies
> >>>>>>>
> >>>>>>>
> >>>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>> <
> >>>>>>>
> >>>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>
> >>>>>>>
> >>>>>>> So either we say that the spec simply does not define what should
> >> happen
> >>>>>>> in case of hierarchic classes (totally valid imo) or we need to
> ditch
> >>>>> our
> >>>>>>> DefaultPropertyVisibility logic completely. It's simply not
> possible
> >> to
> >>>>>>> implement it that way.
> >>>>>>>
> >>>>>>> The reason why I removed the logic from the Reader is the
> following:
> >>>>> Even
> >>>>>>> if the reader says 'oh there is a getter, so let's hide the
> property'
> >>>>> you
> >>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
> >>>>>>> strategies which do disable ALL methods and ONLY use field access.
> >>>>>>> That means the current impl is surely more correct than before, but
> >> not
> >>>>>>> yet perfect.
> >>>>>>>
> >>>>>>> LieGrue,
> >>>>>>> strub
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>>>> rmannibucau@gmail.com
> >>>>>>>> :
> >>>>>>>>
> >>>>>>>> Hi JL,
> >>>>>>>>
> >>>>>>>> Last commit broke some stuff.
> >>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
> >> can it
> >>>>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>>>
> >>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>>>> jlmonteiro@tomitribe.com>
> >>>>>>>> a écrit :
> >>>>>>>>
> >>>>>>>>> Hi all,
> >>>>>>>>>
> >>>>>>>>> Is it ok to release a 1.2.17?
> >>>>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>
> >>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>>>
> >>>>>>>>> Do you guys have some additional fixes to push?
> >>>>>>>>> --
> >>>>>>>>> Jean-Louis Monteiro
> >>>>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>>>> http://www.tomitribe.com
> >>>
> >>
> >>
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le dim. 10 avr. 2022 à 17:33, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> No, because it was not spec compliant. It clearly didn't respect
> JsonbVisibility rules.
>
We all agreed on that but proposal was ;)
> LieGrue,
> strub
>
> > Am 10.04.2022 um 17:23 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
> >:
> >
> > Le dim. 10 avr. 2022 à 17:19, Mark Struberg <st...@yahoo.de.invalid>
> a
> > écrit :
> >
> >> PS: Ad JsonbAccessMode design: the problem is that you need different
> >> behaviour depending on whether @JsonbVisibility is there or not.
> >> Any custom AccessMode needs to implement the same if they want Jsonb
> >> support. Of course the design is imo not really sustainable any more,
> but
> >> cleaning that up would likely break backward compat. We could do that in
> >> the next release.
> >>
> >
> > Previous proposal with the additional flag was making it matching both
> > cases cleanly so real reason to break the api and design IMHO
> >
> >
> >> LieGrue,
> >> strub
> >>
> >>
> >>> Am 10.04.2022 um 17:15 schrieb Mark Struberg <st...@yahoo.de>:
> >>>
> >>> Actually there is not much diff. There is one ct which was only used in
> >> JsonbAccessMode as true. Everywhere else it was feed as false.
> >>> Now we could keep this flag and feed it as default if you really like.
> >> That should be rather easy to do if you think it's necessary. But still
> I'd
> >> mark that 4param ct as @Deprecated. Wdyt?
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
> >> rmannibucau@gmail.com>:
> >>>>
> >>>> Hi Mark,
> >>>>
> >>>> If I read the diff correctly we still have some breaking change in
> >>>> FieldAndMethodAccessMode
> >>>> constructor/behavior? Can we get it fixed?
> >>>> In terms of design, jsonb access mode was originally thought as
> >> delegating
> >>>> the research of the writers/readers to a 3rd party access mode and
> just
> >>>> refilter on top of it with jsonb rules.
> >>>> Now it is kind of mixed in the code (inheritance + composition vs just
> >>>> composition), the reason it was not done was to avoid to expose the
> >> default
> >>>> delegate access mode when a 4rd party access mode was using and mix
> >> them in
> >>>> the jsonbaccess mode, not 100% sure if we want to keep the original
> >> design
> >>>> there or not but mentionning it since it now prevent us to change it
> >> later.
> >>>>
> >>>> Overall except the missing constructor it looks ok to me, happy to get
> >>>> thoughts on the access mode pattern too.
> >>>>
> >>>> Romain Manni-Bucau
> >>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>> <http://rmannibucau.wordpress.com> | Github <
> >> https://github.com/rmannibucau> |
> >>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>> <
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>
> >>>>
> >>>>
> >>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <struberg@yahoo.de.invalid
> >
> >> a
> >>>> écrit :
> >>>>
> >>>>> here we go. plz take a look!
> >>>>>
> >>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> >>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
> >>>>>
> >>>>> txs and LieGrue,
> >>>>> strub
> >>>>>
> >>>>>
> >>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
> >> <struberg@yahoo.de.INVALID
> >>>>>> :
> >>>>>>
> >>>>>> Looked at your proposed solution, but it might be tricky to get it
> >>>>> working. The problem is that we have two modules: mapper and jsonb.
> >> And in
> >>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
> >> defined and
> >>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> >>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
> >> actually
> >>>>> did break the mapper. Sorry that I did not catch this earlier.
> >>>>>>
> >>>>>> Note that the FieldAndMethodAccessMode is only used if no other
> >>>>> AccessMode is configured. There is also a JsonbAccessMode which
> >> delegates
> >>>>> through to the accessMode of the Mapper. Normally means the
> >>>>> FieldAndMethodAccessMode, but could be different.
> >>>>>> We could implement the logic in JsonbAccessMode, totally ditching
> the
> >>>>> accessMode coming from the Mapper. Also thought about having the
> >>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
> >>>>> handling as parameter over there. still not really perfect imo.
> >>>>>>
> >>>>>> Will try to ship a patch proposal tomorrow.
> >>>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> >>>>> rmannibucau@gmail.com>:
> >>>>>>>
> >>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
> >> <struberg@yahoo.de.invalid
> >>>>> <ma...@yahoo.de.invalid>> a
> >>>>>>> écrit :
> >>>>>>>
> >>>>>>>> +1 for getting rid of java.beans.
> >>>>>>>>
> >>>>>>>> I've created a spec ticket for the open questions:
> >>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>>>>>>
> >>>>>>>> My point is: until my fix we clearly did break 4.6. And class
> >>>>> hierarchies
> >>>>>>>> are totally not portable right now anyway. Introducing It's just
> >> badly
> >>>>>>>> underspecified.
> >>>>>>>> So I do not see why the current implementation does make it worse.
> >>>>>>>>
> >>>>>>>
> >>>>>>> Well this one is easy to answer: it changes the behavior and breaks
> >>>>> users
> >>>>>>> so it is not an option for a patch version and to be honest the
> >> proposed
> >>>>>>> fix solves it for both cases so think we should tackle it this way,
> >> at
> >>>>>>> least for now until the spec clarifies it any other way.
> >>>>>>> Once again, the source of the issue is more that we lack an
> >> integration
> >>>>>>> between mapper and jsonb modules than anything else and this
> >> integration
> >>>>>>> had kind of been done but broke mapper so think we should fix it
> >> back,
> >>>>> just
> >>>>>>> costs us a constructor we could have avoided without this issue
> >> AFAIK so
> >>>>>>> not a big deal, no?
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> LieGrue,
> >>>>>>>> strub
> >>>>>>>>
> >>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> >>>>> rmannibucau@gmail.com
> >>>>>>>>> :
> >>>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> From what we discussed on this issue, the thing is that our jsonb
> >>>>> access
> >>>>>>>>> mode does not enable the visibility to see all readers/writers
> >> cause
> >>>>>>>> there
> >>>>>>>>> is some hardcoded filtering in the delegates so I think the trick
> >> is
> >>>>> to
> >>>>>>>>> simply add a toggle to the delegates access modes we use to not
> >> have
> >>>>> this
> >>>>>>>>> filtering and 100% rely on the visibility filtering in
> jsonbaccess
> >>>>> mode
> >>>>>>>> we
> >>>>>>>>> already have in place.
> >>>>>>>>>
> >>>>>>>>> Before the release we must also ensure getters computation cache
> is
> >>>>>>>> cleaned
> >>>>>>>>> up after the ClassMapping is created - we don't want to keep that
> >> in
> >>>>>>>> memory
> >>>>>>>>> at runtime and not sure we can clean up at another time since we
> >> don't
> >>>>>>>> want
> >>>>>>>>> a background thread for that.
> >>>>>>>>> I also think it is not worth to depends on java.beans
> (java.desktop
> >>>>>>>> module)
> >>>>>>>>> so we would reimpl decapitalize as we already did somewhere else
> >> (it
> >>>>> is
> >>>>>>>> one
> >>>>>>>>> liner anyway ;)).
> >>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must
> be
> >>>>>>>> restored
> >>>>>>>>> (it is part of our API and used), default should stay 1-1 but as
> >>>>>>>> mentionned
> >>>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep
> the
> >>>>>>>>> backward compat) to ignore the filtering.
> >>>>>>>>>
> >>>>>>>>> Sounds like this way we solve all issues and can release.
> >>>>>>>>>
> >>>>>>>>> Romain Manni-Bucau
> >>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>>>>> https://github.com/rmannibucau> |
> >>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>>> <
> >>>>>>>>
> >>>>>
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
> >> <struberg@yahoo.de.invalid
> >>>>>>
> >>>>>>>> a
> >>>>>>>>> écrit :
> >>>>>>>>>
> >>>>>>>>>> To sum this up:
> >>>>>>>>>>
> >>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define
> what
> >>>>>>>> happens
> >>>>>>>>>> if there are class hierarchies like class A extends B.
> >>>>>>>>>>
> >>>>>>>>>> In 3.7.1 it states that
> >>>>>>>>>> For a serialization operation, if a matching public getter
> method
> >>>>>>>> exists,
> >>>>>>>>>> the method is called to obtain the value of the property. If a
> >>>>> matching
> >>>>>>>>>> getter method with private, protected, or defaulted to
> >> package-only
> >>>>>>>> access
> >>>>>>>>>> exists, then this field is ignored. If no matching getter method
> >>>>> exists
> >>>>>>>> and
> >>>>>>>>>> the field is public, then the value is obtained directly from
> the
> >>>>> field.
> >>>>>>>>>>
> >>>>>>>>>> This can get changed y using a @JsonbVisibility
> >>>>>>>>>>
> >>>>>>>>>> 4.6 Custom visibility
> >>>>>>>>>> To customize scope and field access strategy as specified in
> >> section
> >>>>>>>>>> 3.7.1, it is possible to specify
> >>>>>>>> javax.json.bind.annotation.JsonbVisibility
> >>>>>>>>>> annotation or to override default behavior globally calling
> >>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
> >> custom
> >>>>>>>>>> property visibility strategy.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is
> simply
> >> not
> >>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
> >>>>> comes to
> >>>>>>>>>> class hierarchies
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>> <
> >>>>>>>>>>
> >>>>>>>>
> >>>>>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> So either we say that the spec simply does not define what
> should
> >>>>> happen
> >>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
> >> ditch
> >>>>>>>> our
> >>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
> >> possible
> >>>>> to
> >>>>>>>>>> implement it that way.
> >>>>>>>>>>
> >>>>>>>>>> The reason why I removed the logic from the Reader is the
> >> following:
> >>>>>>>> Even
> >>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
> >> property'
> >>>>>>>> you
> >>>>>>>>>> STILL are perfectly allowed - as per the spec - to have
> Visibility
> >>>>>>>>>> strategies which do disable ALL methods and ONLY use field
> access.
> >>>>>>>>>> That means the current impl is surely more correct than before,
> >> but
> >>>>> not
> >>>>>>>>>> yet perfect.
> >>>>>>>>>>
> >>>>>>>>>> LieGrue,
> >>>>>>>>>> strub
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>>>>>>> rmannibucau@gmail.com
> >>>>>>>>>>> :
> >>>>>>>>>>>
> >>>>>>>>>>> Hi JL,
> >>>>>>>>>>>
> >>>>>>>>>>> Last commit broke some stuff.
> >>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few
> days,
> >>>>> can it
> >>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>>>>>>
> >>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>>>>>>> jlmonteiro@tomitribe.com>
> >>>>>>>>>>> a écrit :
> >>>>>>>>>>>
> >>>>>>>>>>>> Hi all,
> >>>>>>>>>>>>
> >>>>>>>>>>>> Is it ok to release a 1.2.17?
> >>>>>>>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>
> >>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>>>>>>
> >>>>>>>>>>>> Do you guys have some additional fixes to push?
> >>>>>>>>>>>> --
> >>>>>>>>>>>> Jean-Louis Monteiro
> >>>>>>>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>>>>>>> http://www.tomitribe.com
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
No, because it was not spec compliant. It clearly didn't respect JsonbVisibility rules.
LieGrue,
strub
> Am 10.04.2022 um 17:23 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Le dim. 10 avr. 2022 à 17:19, Mark Struberg <st...@yahoo.de.invalid> a
> écrit :
>
>> PS: Ad JsonbAccessMode design: the problem is that you need different
>> behaviour depending on whether @JsonbVisibility is there or not.
>> Any custom AccessMode needs to implement the same if they want Jsonb
>> support. Of course the design is imo not really sustainable any more, but
>> cleaning that up would likely break backward compat. We could do that in
>> the next release.
>>
>
> Previous proposal with the additional flag was making it matching both
> cases cleanly so real reason to break the api and design IMHO
>
>
>> LieGrue,
>> strub
>>
>>
>>> Am 10.04.2022 um 17:15 schrieb Mark Struberg <st...@yahoo.de>:
>>>
>>> Actually there is not much diff. There is one ct which was only used in
>> JsonbAccessMode as true. Everywhere else it was feed as false.
>>> Now we could keep this flag and feed it as default if you really like.
>> That should be rather easy to do if you think it's necessary. But still I'd
>> mark that 4param ct as @Deprecated. Wdyt?
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com>:
>>>>
>>>> Hi Mark,
>>>>
>>>> If I read the diff correctly we still have some breaking change in
>>>> FieldAndMethodAccessMode
>>>> constructor/behavior? Can we get it fixed?
>>>> In terms of design, jsonb access mode was originally thought as
>> delegating
>>>> the research of the writers/readers to a 3rd party access mode and just
>>>> refilter on top of it with jsonb rules.
>>>> Now it is kind of mixed in the code (inheritance + composition vs just
>>>> composition), the reason it was not done was to avoid to expose the
>> default
>>>> delegate access mode when a 4rd party access mode was using and mix
>> them in
>>>> the jsonbaccess mode, not 100% sure if we want to keep the original
>> design
>>>> there or not but mentionning it since it now prevent us to change it
>> later.
>>>>
>>>> Overall except the missing constructor it looks ok to me, happy to get
>>>> thoughts on the access mode pattern too.
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github <
>> https://github.com/rmannibucau> |
>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>
>>>>
>>>>
>>>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
>> a
>>>> écrit :
>>>>
>>>>> here we go. plz take a look!
>>>>>
>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>>>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>>>>
>>>>> txs and LieGrue,
>>>>> strub
>>>>>
>>>>>
>>>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
>> <struberg@yahoo.de.INVALID
>>>>>> :
>>>>>>
>>>>>> Looked at your proposed solution, but it might be tricky to get it
>>>>> working. The problem is that we have two modules: mapper and jsonb.
>> And in
>>>>> JOHNZON-250 we added a logic to the mapper module which is ONLY
>> defined and
>>>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>>>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
>> actually
>>>>> did break the mapper. Sorry that I did not catch this earlier.
>>>>>>
>>>>>> Note that the FieldAndMethodAccessMode is only used if no other
>>>>> AccessMode is configured. There is also a JsonbAccessMode which
>> delegates
>>>>> through to the accessMode of the Mapper. Normally means the
>>>>> FieldAndMethodAccessMode, but could be different.
>>>>>> We could implement the logic in JsonbAccessMode, totally ditching the
>>>>> accessMode coming from the Mapper. Also thought about having the
>>>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>>>>> handling as parameter over there. still not really perfect imo.
>>>>>>
>>>>>> Will try to ship a patch proposal tomorrow.
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com>:
>>>>>>>
>>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
>> <struberg@yahoo.de.invalid
>>>>> <ma...@yahoo.de.invalid>> a
>>>>>>> écrit :
>>>>>>>
>>>>>>>> +1 for getting rid of java.beans.
>>>>>>>>
>>>>>>>> I've created a spec ticket for the open questions:
>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>>>>
>>>>>>>> My point is: until my fix we clearly did break 4.6. And class
>>>>> hierarchies
>>>>>>>> are totally not portable right now anyway. Introducing It's just
>> badly
>>>>>>>> underspecified.
>>>>>>>> So I do not see why the current implementation does make it worse.
>>>>>>>>
>>>>>>>
>>>>>>> Well this one is easy to answer: it changes the behavior and breaks
>>>>> users
>>>>>>> so it is not an option for a patch version and to be honest the
>> proposed
>>>>>>> fix solves it for both cases so think we should tackle it this way,
>> at
>>>>>>> least for now until the spec clarifies it any other way.
>>>>>>> Once again, the source of the issue is more that we lack an
>> integration
>>>>>>> between mapper and jsonb modules than anything else and this
>> integration
>>>>>>> had kind of been done but broke mapper so think we should fix it
>> back,
>>>>> just
>>>>>>> costs us a constructor we could have avoided without this issue
>> AFAIK so
>>>>>>> not a big deal, no?
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com
>>>>>>>>> :
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>>>>> access
>>>>>>>>> mode does not enable the visibility to see all readers/writers
>> cause
>>>>>>>> there
>>>>>>>>> is some hardcoded filtering in the delegates so I think the trick
>> is
>>>>> to
>>>>>>>>> simply add a toggle to the delegates access modes we use to not
>> have
>>>>> this
>>>>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
>>>>> mode
>>>>>>>> we
>>>>>>>>> already have in place.
>>>>>>>>>
>>>>>>>>> Before the release we must also ensure getters computation cache is
>>>>>>>> cleaned
>>>>>>>>> up after the ClassMapping is created - we don't want to keep that
>> in
>>>>>>>> memory
>>>>>>>>> at runtime and not sure we can clean up at another time since we
>> don't
>>>>>>>> want
>>>>>>>>> a background thread for that.
>>>>>>>>> I also think it is not worth to depends on java.beans (java.desktop
>>>>>>>> module)
>>>>>>>>> so we would reimpl decapitalize as we already did somewhere else
>> (it
>>>>> is
>>>>>>>> one
>>>>>>>>> liner anyway ;)).
>>>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>>>>>>>> restored
>>>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>>>>> mentionned
>>>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>>>>>>>> backward compat) to ignore the filtering.
>>>>>>>>>
>>>>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>>>>
>>>>>>>>> Romain Manni-Bucau
>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>>>> https://github.com/rmannibucau> |
>>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>>> <
>>>>>>>>
>>>>>
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
>> <struberg@yahoo.de.invalid
>>>>>>
>>>>>>>> a
>>>>>>>>> écrit :
>>>>>>>>>
>>>>>>>>>> To sum this up:
>>>>>>>>>>
>>>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>>>>>>>> happens
>>>>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>>>>
>>>>>>>>>> In 3.7.1 it states that
>>>>>>>>>> For a serialization operation, if a matching public getter method
>>>>>>>> exists,
>>>>>>>>>> the method is called to obtain the value of the property. If a
>>>>> matching
>>>>>>>>>> getter method with private, protected, or defaulted to
>> package-only
>>>>>>>> access
>>>>>>>>>> exists, then this field is ignored. If no matching getter method
>>>>> exists
>>>>>>>> and
>>>>>>>>>> the field is public, then the value is obtained directly from the
>>>>> field.
>>>>>>>>>>
>>>>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>>>>
>>>>>>>>>> 4.6 Custom visibility
>>>>>>>>>> To customize scope and field access strategy as specified in
>> section
>>>>>>>>>> 3.7.1, it is possible to specify
>>>>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>>>>> annotation or to override default behavior globally calling
>>>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
>> custom
>>>>>>>>>> property visibility strategy.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply
>> not
>>>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>>>>> comes to
>>>>>>>>>> class hierarchies
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>> <
>>>>>>>>>>
>>>>>>>>
>>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> So either we say that the spec simply does not define what should
>>>>> happen
>>>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
>> ditch
>>>>>>>> our
>>>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
>> possible
>>>>> to
>>>>>>>>>> implement it that way.
>>>>>>>>>>
>>>>>>>>>> The reason why I removed the logic from the Reader is the
>> following:
>>>>>>>> Even
>>>>>>>>>> if the reader says 'oh there is a getter, so let's hide the
>> property'
>>>>>>>> you
>>>>>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>>>>>>>> strategies which do disable ALL methods and ONLY use field access.
>>>>>>>>>> That means the current impl is surely more correct than before,
>> but
>>>>> not
>>>>>>>>>> yet perfect.
>>>>>>>>>>
>>>>>>>>>> LieGrue,
>>>>>>>>>> strub
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>>>>> rmannibucau@gmail.com
>>>>>>>>>>> :
>>>>>>>>>>>
>>>>>>>>>>> Hi JL,
>>>>>>>>>>>
>>>>>>>>>>> Last commit broke some stuff.
>>>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
>>>>> can it
>>>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>>>>
>>>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>>>>> a écrit :
>>>>>>>>>>>
>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>
>>>>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>>>>
>>>>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>>>>> --
>>>>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>>>>> http://www.tomitribe.com
>>>>>>
>>>>>
>>>>>
>>>
>>
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le dim. 10 avr. 2022 à 17:19, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> PS: Ad JsonbAccessMode design: the problem is that you need different
> behaviour depending on whether @JsonbVisibility is there or not.
> Any custom AccessMode needs to implement the same if they want Jsonb
> support. Of course the design is imo not really sustainable any more, but
> cleaning that up would likely break backward compat. We could do that in
> the next release.
>
Previous proposal with the additional flag was making it matching both
cases cleanly so real reason to break the api and design IMHO
> LieGrue,
> strub
>
>
> > Am 10.04.2022 um 17:15 schrieb Mark Struberg <st...@yahoo.de>:
> >
> > Actually there is not much diff. There is one ct which was only used in
> JsonbAccessMode as true. Everywhere else it was feed as false.
> > Now we could keep this flag and feed it as default if you really like.
> That should be rather easy to do if you think it's necessary. But still I'd
> mark that 4param ct as @Deprecated. Wdyt?
> >
> > LieGrue,
> > strub
> >
> >> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com>:
> >>
> >> Hi Mark,
> >>
> >> If I read the diff correctly we still have some breaking change in
> >> FieldAndMethodAccessMode
> >> constructor/behavior? Can we get it fixed?
> >> In terms of design, jsonb access mode was originally thought as
> delegating
> >> the research of the writers/readers to a 3rd party access mode and just
> >> refilter on top of it with jsonb rules.
> >> Now it is kind of mixed in the code (inheritance + composition vs just
> >> composition), the reason it was not done was to avoid to expose the
> default
> >> delegate access mode when a 4rd party access mode was using and mix
> them in
> >> the jsonbaccess mode, not 100% sure if we want to keep the original
> design
> >> there or not but mentionning it since it now prevent us to change it
> later.
> >>
> >> Overall except the missing constructor it looks ok to me, happy to get
> >> thoughts on the access mode pattern too.
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >> <https://rmannibucau.metawerx.net/> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>
> >>
> >> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid>
> a
> >> écrit :
> >>
> >>> here we go. plz take a look!
> >>>
> >>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> >>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
> >>>
> >>> txs and LieGrue,
> >>> strub
> >>>
> >>>
> >>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg
> <struberg@yahoo.de.INVALID
> >>>> :
> >>>>
> >>>> Looked at your proposed solution, but it might be tricky to get it
> >>> working. The problem is that we have two modules: mapper and jsonb.
> And in
> >>> JOHNZON-250 we added a logic to the mapper module which is ONLY
> defined and
> >>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> >>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This
> actually
> >>> did break the mapper. Sorry that I did not catch this earlier.
> >>>>
> >>>> Note that the FieldAndMethodAccessMode is only used if no other
> >>> AccessMode is configured. There is also a JsonbAccessMode which
> delegates
> >>> through to the accessMode of the Mapper. Normally means the
> >>> FieldAndMethodAccessMode, but could be different.
> >>>> We could implement the logic in JsonbAccessMode, totally ditching the
> >>> accessMode coming from the Mapper. Also thought about having the
> >>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
> >>> handling as parameter over there. still not really perfect imo.
> >>>>
> >>>> Will try to ship a patch proposal tomorrow.
> >>>>
> >>>> LieGrue,
> >>>> strub
> >>>>
> >>>>
> >>>>
> >>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> >>> rmannibucau@gmail.com>:
> >>>>>
> >>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg
> <struberg@yahoo.de.invalid
> >>> <ma...@yahoo.de.invalid>> a
> >>>>> écrit :
> >>>>>
> >>>>>> +1 for getting rid of java.beans.
> >>>>>>
> >>>>>> I've created a spec ticket for the open questions:
> >>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>>>>
> >>>>>> My point is: until my fix we clearly did break 4.6. And class
> >>> hierarchies
> >>>>>> are totally not portable right now anyway. Introducing It's just
> badly
> >>>>>> underspecified.
> >>>>>> So I do not see why the current implementation does make it worse.
> >>>>>>
> >>>>>
> >>>>> Well this one is easy to answer: it changes the behavior and breaks
> >>> users
> >>>>> so it is not an option for a patch version and to be honest the
> proposed
> >>>>> fix solves it for both cases so think we should tackle it this way,
> at
> >>>>> least for now until the spec clarifies it any other way.
> >>>>> Once again, the source of the issue is more that we lack an
> integration
> >>>>> between mapper and jsonb modules than anything else and this
> integration
> >>>>> had kind of been done but broke mapper so think we should fix it
> back,
> >>> just
> >>>>> costs us a constructor we could have avoided without this issue
> AFAIK so
> >>>>> not a big deal, no?
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> >>> rmannibucau@gmail.com
> >>>>>>> :
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> From what we discussed on this issue, the thing is that our jsonb
> >>> access
> >>>>>>> mode does not enable the visibility to see all readers/writers
> cause
> >>>>>> there
> >>>>>>> is some hardcoded filtering in the delegates so I think the trick
> is
> >>> to
> >>>>>>> simply add a toggle to the delegates access modes we use to not
> have
> >>> this
> >>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
> >>> mode
> >>>>>> we
> >>>>>>> already have in place.
> >>>>>>>
> >>>>>>> Before the release we must also ensure getters computation cache is
> >>>>>> cleaned
> >>>>>>> up after the ClassMapping is created - we don't want to keep that
> in
> >>>>>> memory
> >>>>>>> at runtime and not sure we can clean up at another time since we
> don't
> >>>>>> want
> >>>>>>> a background thread for that.
> >>>>>>> I also think it is not worth to depends on java.beans (java.desktop
> >>>>>> module)
> >>>>>>> so we would reimpl decapitalize as we already did somewhere else
> (it
> >>> is
> >>>>>> one
> >>>>>>> liner anyway ;)).
> >>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
> >>>>>> restored
> >>>>>>> (it is part of our API and used), default should stay 1-1 but as
> >>>>>> mentionned
> >>>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
> >>>>>>> backward compat) to ignore the filtering.
> >>>>>>>
> >>>>>>> Sounds like this way we solve all issues and can release.
> >>>>>>>
> >>>>>>> Romain Manni-Bucau
> >>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>>>>> https://github.com/rmannibucau> |
> >>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>> <
> >>>>>>
> >>>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg
> <struberg@yahoo.de.invalid
> >>>>
> >>>>>> a
> >>>>>>> écrit :
> >>>>>>>
> >>>>>>>> To sum this up:
> >>>>>>>>
> >>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
> >>>>>> happens
> >>>>>>>> if there are class hierarchies like class A extends B.
> >>>>>>>>
> >>>>>>>> In 3.7.1 it states that
> >>>>>>>> For a serialization operation, if a matching public getter method
> >>>>>> exists,
> >>>>>>>> the method is called to obtain the value of the property. If a
> >>> matching
> >>>>>>>> getter method with private, protected, or defaulted to
> package-only
> >>>>>> access
> >>>>>>>> exists, then this field is ignored. If no matching getter method
> >>> exists
> >>>>>> and
> >>>>>>>> the field is public, then the value is obtained directly from the
> >>> field.
> >>>>>>>>
> >>>>>>>> This can get changed y using a @JsonbVisibility
> >>>>>>>>
> >>>>>>>> 4.6 Custom visibility
> >>>>>>>> To customize scope and field access strategy as specified in
> section
> >>>>>>>> 3.7.1, it is possible to specify
> >>>>>> javax.json.bind.annotation.JsonbVisibility
> >>>>>>>> annotation or to override default behavior globally calling
> >>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given
> custom
> >>>>>>>> property visibility strategy.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply
> not
> >>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
> >>> comes to
> >>>>>>>> class hierarchies
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>> <
> >>>>>>>>
> >>>>>>
> >>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> So either we say that the spec simply does not define what should
> >>> happen
> >>>>>>>> in case of hierarchic classes (totally valid imo) or we need to
> ditch
> >>>>>> our
> >>>>>>>> DefaultPropertyVisibility logic completely. It's simply not
> possible
> >>> to
> >>>>>>>> implement it that way.
> >>>>>>>>
> >>>>>>>> The reason why I removed the logic from the Reader is the
> following:
> >>>>>> Even
> >>>>>>>> if the reader says 'oh there is a getter, so let's hide the
> property'
> >>>>>> you
> >>>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
> >>>>>>>> strategies which do disable ALL methods and ONLY use field access.
> >>>>>>>> That means the current impl is surely more correct than before,
> but
> >>> not
> >>>>>>>> yet perfect.
> >>>>>>>>
> >>>>>>>> LieGrue,
> >>>>>>>> strub
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>>>>> rmannibucau@gmail.com
> >>>>>>>>> :
> >>>>>>>>>
> >>>>>>>>> Hi JL,
> >>>>>>>>>
> >>>>>>>>> Last commit broke some stuff.
> >>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
> >>> can it
> >>>>>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>>>>
> >>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>>>>> jlmonteiro@tomitribe.com>
> >>>>>>>>> a écrit :
> >>>>>>>>>
> >>>>>>>>>> Hi all,
> >>>>>>>>>>
> >>>>>>>>>> Is it ok to release a 1.2.17?
> >>>>>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>>>>
> >>>>>>>>>> Do you guys have some additional fixes to push?
> >>>>>>>>>> --
> >>>>>>>>>> Jean-Louis Monteiro
> >>>>>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>>>>> http://www.tomitribe.com
> >>>>
> >>>
> >>>
> >
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
PS: Ad JsonbAccessMode design: the problem is that you need different behaviour depending on whether @JsonbVisibility is there or not.
Any custom AccessMode needs to implement the same if they want Jsonb support. Of course the design is imo not really sustainable any more, but cleaning that up would likely break backward compat. We could do that in the next release.
LieGrue,
strub
> Am 10.04.2022 um 17:15 schrieb Mark Struberg <st...@yahoo.de>:
>
> Actually there is not much diff. There is one ct which was only used in JsonbAccessMode as true. Everywhere else it was feed as false.
> Now we could keep this flag and feed it as default if you really like. That should be rather easy to do if you think it's necessary. But still I'd mark that 4param ct as @Deprecated. Wdyt?
>
> LieGrue,
> strub
>
>> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>>
>> Hi Mark,
>>
>> If I read the diff correctly we still have some breaking change in
>> FieldAndMethodAccessMode
>> constructor/behavior? Can we get it fixed?
>> In terms of design, jsonb access mode was originally thought as delegating
>> the research of the writers/readers to a 3rd party access mode and just
>> refilter on top of it with jsonb rules.
>> Now it is kind of mixed in the code (inheritance + composition vs just
>> composition), the reason it was not done was to avoid to expose the default
>> delegate access mode when a 4rd party access mode was using and mix them in
>> the jsonbaccess mode, not 100% sure if we want to keep the original design
>> there or not but mentionning it since it now prevent us to change it later.
>>
>> Overall except the missing constructor it looks ok to me, happy to get
>> thoughts on the access mode pattern too.
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>>
>> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid> a
>> écrit :
>>
>>> here we go. plz take a look!
>>>
>>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>>
>>> txs and LieGrue,
>>> strub
>>>
>>>
>>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg <struberg@yahoo.de.INVALID
>>>> :
>>>>
>>>> Looked at your proposed solution, but it might be tricky to get it
>>> working. The problem is that we have two modules: mapper and jsonb. And in
>>> JOHNZON-250 we added a logic to the mapper module which is ONLY defined and
>>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually
>>> did break the mapper. Sorry that I did not catch this earlier.
>>>>
>>>> Note that the FieldAndMethodAccessMode is only used if no other
>>> AccessMode is configured. There is also a JsonbAccessMode which delegates
>>> through to the accessMode of the Mapper. Normally means the
>>> FieldAndMethodAccessMode, but could be different.
>>>> We could implement the logic in JsonbAccessMode, totally ditching the
>>> accessMode coming from the Mapper. Also thought about having the
>>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>>> handling as parameter over there. still not really perfect imo.
>>>>
>>>> Will try to ship a patch proposal tomorrow.
>>>>
>>>> LieGrue,
>>>> strub
>>>>
>>>>
>>>>
>>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>>> rmannibucau@gmail.com>:
>>>>>
>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid
>>> <ma...@yahoo.de.invalid>> a
>>>>> écrit :
>>>>>
>>>>>> +1 for getting rid of java.beans.
>>>>>>
>>>>>> I've created a spec ticket for the open questions:
>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>>
>>>>>> My point is: until my fix we clearly did break 4.6. And class
>>> hierarchies
>>>>>> are totally not portable right now anyway. Introducing It's just badly
>>>>>> underspecified.
>>>>>> So I do not see why the current implementation does make it worse.
>>>>>>
>>>>>
>>>>> Well this one is easy to answer: it changes the behavior and breaks
>>> users
>>>>> so it is not an option for a patch version and to be honest the proposed
>>>>> fix solves it for both cases so think we should tackle it this way, at
>>>>> least for now until the spec clarifies it any other way.
>>>>> Once again, the source of the issue is more that we lack an integration
>>>>> between mapper and jsonb modules than anything else and this integration
>>>>> had kind of been done but broke mapper so think we should fix it back,
>>> just
>>>>> costs us a constructor we could have avoided without this issue AFAIK so
>>>>> not a big deal, no?
>>>>>
>>>>>
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>>> rmannibucau@gmail.com
>>>>>>> :
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> From what we discussed on this issue, the thing is that our jsonb
>>> access
>>>>>>> mode does not enable the visibility to see all readers/writers cause
>>>>>> there
>>>>>>> is some hardcoded filtering in the delegates so I think the trick is
>>> to
>>>>>>> simply add a toggle to the delegates access modes we use to not have
>>> this
>>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
>>> mode
>>>>>> we
>>>>>>> already have in place.
>>>>>>>
>>>>>>> Before the release we must also ensure getters computation cache is
>>>>>> cleaned
>>>>>>> up after the ClassMapping is created - we don't want to keep that in
>>>>>> memory
>>>>>>> at runtime and not sure we can clean up at another time since we don't
>>>>>> want
>>>>>>> a background thread for that.
>>>>>>> I also think it is not worth to depends on java.beans (java.desktop
>>>>>> module)
>>>>>>> so we would reimpl decapitalize as we already did somewhere else (it
>>> is
>>>>>> one
>>>>>>> liner anyway ;)).
>>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>>>>>> restored
>>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>>> mentionned
>>>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>>>>>> backward compat) to ignore the filtering.
>>>>>>>
>>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>>
>>>>>>> Romain Manni-Bucau
>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>>> https://github.com/rmannibucau> |
>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>> <
>>>>>>
>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg <struberg@yahoo.de.invalid
>>>>
>>>>>> a
>>>>>>> écrit :
>>>>>>>
>>>>>>>> To sum this up:
>>>>>>>>
>>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>>>>>> happens
>>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>>
>>>>>>>> In 3.7.1 it states that
>>>>>>>> For a serialization operation, if a matching public getter method
>>>>>> exists,
>>>>>>>> the method is called to obtain the value of the property. If a
>>> matching
>>>>>>>> getter method with private, protected, or defaulted to package-only
>>>>>> access
>>>>>>>> exists, then this field is ignored. If no matching getter method
>>> exists
>>>>>> and
>>>>>>>> the field is public, then the value is obtained directly from the
>>> field.
>>>>>>>>
>>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>>
>>>>>>>> 4.6 Custom visibility
>>>>>>>> To customize scope and field access strategy as specified in section
>>>>>>>> 3.7.1, it is possible to specify
>>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>>> annotation or to override default behavior globally calling
>>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
>>>>>>>> property visibility strategy.
>>>>>>>>
>>>>>>>>
>>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply not
>>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>>> comes to
>>>>>>>> class hierarchies
>>>>>>>>
>>>>>>>>
>>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>> <
>>>>>>>>
>>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>>
>>>>>>>>
>>>>>>>> So either we say that the spec simply does not define what should
>>> happen
>>>>>>>> in case of hierarchic classes (totally valid imo) or we need to ditch
>>>>>> our
>>>>>>>> DefaultPropertyVisibility logic completely. It's simply not possible
>>> to
>>>>>>>> implement it that way.
>>>>>>>>
>>>>>>>> The reason why I removed the logic from the Reader is the following:
>>>>>> Even
>>>>>>>> if the reader says 'oh there is a getter, so let's hide the property'
>>>>>> you
>>>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>>>>>> strategies which do disable ALL methods and ONLY use field access.
>>>>>>>> That means the current impl is surely more correct than before, but
>>> not
>>>>>>>> yet perfect.
>>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>>> rmannibucau@gmail.com
>>>>>>>>> :
>>>>>>>>>
>>>>>>>>> Hi JL,
>>>>>>>>>
>>>>>>>>> Last commit broke some stuff.
>>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
>>> can it
>>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>>
>>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>>> a écrit :
>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>>
>>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>>> --
>>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>>> http://www.tomitribe.com
>>>>
>>>
>>>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Actually there is not much diff. There is one ct which was only used in JsonbAccessMode as true. Everywhere else it was feed as false.
Now we could keep this flag and feed it as default if you really like. That should be rather easy to do if you think it's necessary. But still I'd mark that 4param ct as @Deprecated. Wdyt?
LieGrue,
strub
> Am 10.04.2022 um 16:24 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Hi Mark,
>
> If I read the diff correctly we still have some breaking change in
> FieldAndMethodAccessMode
> constructor/behavior? Can we get it fixed?
> In terms of design, jsonb access mode was originally thought as delegating
> the research of the writers/readers to a 3rd party access mode and just
> refilter on top of it with jsonb rules.
> Now it is kind of mixed in the code (inheritance + composition vs just
> composition), the reason it was not done was to avoid to expose the default
> delegate access mode when a 4rd party access mode was using and mix them in
> the jsonbaccess mode, not 100% sure if we want to keep the original design
> there or not but mentionning it since it now prevent us to change it later.
>
> Overall except the missing constructor it looks ok to me, happy to get
> thoughts on the access mode pattern too.
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>
> Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid> a
> écrit :
>
>> here we go. plz take a look!
>>
>> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
>> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>>
>> txs and LieGrue,
>> strub
>>
>>
>>> Am 09.04.2022 um 23:14 schrieb Mark Struberg <struberg@yahoo.de.INVALID
>>> :
>>>
>>> Looked at your proposed solution, but it might be tricky to get it
>> working. The problem is that we have two modules: mapper and jsonb. And in
>> JOHNZON-250 we added a logic to the mapper module which is ONLY defined and
>> useful in Jsonb. This is the alwaysPreferMethodVisibility in
>> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually
>> did break the mapper. Sorry that I did not catch this earlier.
>>>
>>> Note that the FieldAndMethodAccessMode is only used if no other
>> AccessMode is configured. There is also a JsonbAccessMode which delegates
>> through to the accessMode of the Mapper. Normally means the
>> FieldAndMethodAccessMode, but could be different.
>>> We could implement the logic in JsonbAccessMode, totally ditching the
>> accessMode coming from the Mapper. Also thought about having the
>> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
>> handling as parameter over there. still not really perfect imo.
>>>
>>> Will try to ship a patch proposal tomorrow.
>>>
>>> LieGrue,
>>> strub
>>>
>>>
>>>
>>>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com>:
>>>>
>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid
>> <ma...@yahoo.de.invalid>> a
>>>> écrit :
>>>>
>>>>> +1 for getting rid of java.beans.
>>>>>
>>>>> I've created a spec ticket for the open questions:
>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>>>
>>>>> My point is: until my fix we clearly did break 4.6. And class
>> hierarchies
>>>>> are totally not portable right now anyway. Introducing It's just badly
>>>>> underspecified.
>>>>> So I do not see why the current implementation does make it worse.
>>>>>
>>>>
>>>> Well this one is easy to answer: it changes the behavior and breaks
>> users
>>>> so it is not an option for a patch version and to be honest the proposed
>>>> fix solves it for both cases so think we should tackle it this way, at
>>>> least for now until the spec clarifies it any other way.
>>>> Once again, the source of the issue is more that we lack an integration
>>>> between mapper and jsonb modules than anything else and this integration
>>>> had kind of been done but broke mapper so think we should fix it back,
>> just
>>>> costs us a constructor we could have avoided without this issue AFAIK so
>>>> not a big deal, no?
>>>>
>>>>
>>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com
>>>>>> :
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> From what we discussed on this issue, the thing is that our jsonb
>> access
>>>>>> mode does not enable the visibility to see all readers/writers cause
>>>>> there
>>>>>> is some hardcoded filtering in the delegates so I think the trick is
>> to
>>>>>> simply add a toggle to the delegates access modes we use to not have
>> this
>>>>>> filtering and 100% rely on the visibility filtering in jsonbaccess
>> mode
>>>>> we
>>>>>> already have in place.
>>>>>>
>>>>>> Before the release we must also ensure getters computation cache is
>>>>> cleaned
>>>>>> up after the ClassMapping is created - we don't want to keep that in
>>>>> memory
>>>>>> at runtime and not sure we can clean up at another time since we don't
>>>>> want
>>>>>> a background thread for that.
>>>>>> I also think it is not worth to depends on java.beans (java.desktop
>>>>> module)
>>>>>> so we would reimpl decapitalize as we already did somewhere else (it
>> is
>>>>> one
>>>>>> liner anyway ;)).
>>>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>>>>> restored
>>>>>> (it is part of our API and used), default should stay 1-1 but as
>>>>> mentionned
>>>>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>>>>> backward compat) to ignore the filtering.
>>>>>>
>>>>>> Sounds like this way we solve all issues and can release.
>>>>>>
>>>>>> Romain Manni-Bucau
>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>> <http://rmannibucau.wordpress.com> | Github <
>>>>> https://github.com/rmannibucau> |
>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>> <
>>>>>
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg <struberg@yahoo.de.invalid
>>>
>>>>> a
>>>>>> écrit :
>>>>>>
>>>>>>> To sum this up:
>>>>>>>
>>>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>>>>> happens
>>>>>>> if there are class hierarchies like class A extends B.
>>>>>>>
>>>>>>> In 3.7.1 it states that
>>>>>>> For a serialization operation, if a matching public getter method
>>>>> exists,
>>>>>>> the method is called to obtain the value of the property. If a
>> matching
>>>>>>> getter method with private, protected, or defaulted to package-only
>>>>> access
>>>>>>> exists, then this field is ignored. If no matching getter method
>> exists
>>>>> and
>>>>>>> the field is public, then the value is obtained directly from the
>> field.
>>>>>>>
>>>>>>> This can get changed y using a @JsonbVisibility
>>>>>>>
>>>>>>> 4.6 Custom visibility
>>>>>>> To customize scope and field access strategy as specified in section
>>>>>>> 3.7.1, it is possible to specify
>>>>> javax.json.bind.annotation.JsonbVisibility
>>>>>>> annotation or to override default behavior globally calling
>>>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
>>>>>>> property visibility strategy.
>>>>>>>
>>>>>>>
>>>>>>> The problem is that that the PropertyVisibilityStrategy is simply not
>>>>>>> ready to implement the rules of 3.7.1 in a portable way when it
>> comes to
>>>>>>> class hierarchies
>>>>>>>
>>>>>>>
>>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>> <
>>>>>>>
>>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>>>
>>>>>>>
>>>>>>> So either we say that the spec simply does not define what should
>> happen
>>>>>>> in case of hierarchic classes (totally valid imo) or we need to ditch
>>>>> our
>>>>>>> DefaultPropertyVisibility logic completely. It's simply not possible
>> to
>>>>>>> implement it that way.
>>>>>>>
>>>>>>> The reason why I removed the logic from the Reader is the following:
>>>>> Even
>>>>>>> if the reader says 'oh there is a getter, so let's hide the property'
>>>>> you
>>>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>>>>> strategies which do disable ALL methods and ONLY use field access.
>>>>>>> That means the current impl is surely more correct than before, but
>> not
>>>>>>> yet perfect.
>>>>>>>
>>>>>>> LieGrue,
>>>>>>> strub
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com
>>>>>>>> :
>>>>>>>>
>>>>>>>> Hi JL,
>>>>>>>>
>>>>>>>> Last commit broke some stuff.
>>>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
>> can it
>>>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>>>
>>>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>>>> jlmonteiro@tomitribe.com>
>>>>>>>> a écrit :
>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> Is it ok to release a 1.2.17?
>>>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>>>
>>>>>>>>> Do you guys have some additional fixes to push?
>>>>>>>>> --
>>>>>>>>> Jean-Louis Monteiro
>>>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>>>> http://www.tomitribe.com
>>>
>>
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Mark,
If I read the diff correctly we still have some breaking change in
FieldAndMethodAccessMode
constructor/behavior? Can we get it fixed?
In terms of design, jsonb access mode was originally thought as delegating
the research of the writers/readers to a 3rd party access mode and just
refilter on top of it with jsonb rules.
Now it is kind of mixed in the code (inheritance + composition vs just
composition), the reason it was not done was to avoid to expose the default
delegate access mode when a 4rd party access mode was using and mix them in
the jsonbaccess mode, not 100% sure if we want to keep the original design
there or not but mentionning it since it now prevent us to change it later.
Overall except the missing constructor it looks ok to me, happy to get
thoughts on the access mode pattern too.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Le sam. 9 avr. 2022 à 23:49, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> here we go. plz take a look!
>
> https://github.com/struberg/johnzon/tree/JOHNZON-364 <
> https://github.com/struberg/johnzon/tree/JOHNZON-364>
>
> txs and LieGrue,
> strub
>
>
> > Am 09.04.2022 um 23:14 schrieb Mark Struberg <struberg@yahoo.de.INVALID
> >:
> >
> > Looked at your proposed solution, but it might be tricky to get it
> working. The problem is that we have two modules: mapper and jsonb. And in
> JOHNZON-250 we added a logic to the mapper module which is ONLY defined and
> useful in Jsonb. This is the alwaysPreferMethodVisibility in
> org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually
> did break the mapper. Sorry that I did not catch this earlier.
> >
> > Note that the FieldAndMethodAccessMode is only used if no other
> AccessMode is configured. There is also a JsonbAccessMode which delegates
> through to the accessMode of the Mapper. Normally means the
> FieldAndMethodAccessMode, but could be different.
> > We could implement the logic in JsonbAccessMode, totally ditching the
> accessMode coming from the Mapper. Also thought about having the
> JsonbAccessMode extending the FieldAndMethodAccessMode and adding the
> handling as parameter over there. still not really perfect imo.
> >
> > Will try to ship a patch proposal tomorrow.
> >
> > LieGrue,
> > strub
> >
> >
> >
> >> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com>:
> >>
> >> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid
> <ma...@yahoo.de.invalid>> a
> >> écrit :
> >>
> >>> +1 for getting rid of java.beans.
> >>>
> >>> I've created a spec ticket for the open questions:
> >>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> >>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
> >>>
> >>> My point is: until my fix we clearly did break 4.6. And class
> hierarchies
> >>> are totally not portable right now anyway. Introducing It's just badly
> >>> underspecified.
> >>> So I do not see why the current implementation does make it worse.
> >>>
> >>
> >> Well this one is easy to answer: it changes the behavior and breaks
> users
> >> so it is not an option for a patch version and to be honest the proposed
> >> fix solves it for both cases so think we should tackle it this way, at
> >> least for now until the spec clarifies it any other way.
> >> Once again, the source of the issue is more that we lack an integration
> >> between mapper and jsonb modules than anything else and this integration
> >> had kind of been done but broke mapper so think we should fix it back,
> just
> >> costs us a constructor we could have avoided without this issue AFAIK so
> >> not a big deal, no?
> >>
> >>
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com
> >>>> :
> >>>>
> >>>> Hi,
> >>>>
> >>>> From what we discussed on this issue, the thing is that our jsonb
> access
> >>>> mode does not enable the visibility to see all readers/writers cause
> >>> there
> >>>> is some hardcoded filtering in the delegates so I think the trick is
> to
> >>>> simply add a toggle to the delegates access modes we use to not have
> this
> >>>> filtering and 100% rely on the visibility filtering in jsonbaccess
> mode
> >>> we
> >>>> already have in place.
> >>>>
> >>>> Before the release we must also ensure getters computation cache is
> >>> cleaned
> >>>> up after the ClassMapping is created - we don't want to keep that in
> >>> memory
> >>>> at runtime and not sure we can clean up at another time since we don't
> >>> want
> >>>> a background thread for that.
> >>>> I also think it is not worth to depends on java.beans (java.desktop
> >>> module)
> >>>> so we would reimpl decapitalize as we already did somewhere else (it
> is
> >>> one
> >>>> liner anyway ;)).
> >>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
> >>> restored
> >>>> (it is part of our API and used), default should stay 1-1 but as
> >>> mentionned
> >>>> I'd add a new toggle (with a constructor calling "this" to keep the
> >>>> backward compat) to ignore the filtering.
> >>>>
> >>>> Sounds like this way we solve all issues and can release.
> >>>>
> >>>> Romain Manni-Bucau
> >>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>> <http://rmannibucau.wordpress.com> | Github <
> >>> https://github.com/rmannibucau> |
> >>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>> <
> >>>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>>>
> >>>>
> >>>>
> >>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg <struberg@yahoo.de.invalid
> >
> >>> a
> >>>> écrit :
> >>>>
> >>>>> To sum this up:
> >>>>>
> >>>>> The problem lies partly in the JSON-B spec. It doesn't define what
> >>> happens
> >>>>> if there are class hierarchies like class A extends B.
> >>>>>
> >>>>> In 3.7.1 it states that
> >>>>> For a serialization operation, if a matching public getter method
> >>> exists,
> >>>>> the method is called to obtain the value of the property. If a
> matching
> >>>>> getter method with private, protected, or defaulted to package-only
> >>> access
> >>>>> exists, then this field is ignored. If no matching getter method
> exists
> >>> and
> >>>>> the field is public, then the value is obtained directly from the
> field.
> >>>>>
> >>>>> This can get changed y using a @JsonbVisibility
> >>>>>
> >>>>> 4.6 Custom visibility
> >>>>> To customize scope and field access strategy as specified in section
> >>>>> 3.7.1, it is possible to specify
> >>> javax.json.bind.annotation.JsonbVisibility
> >>>>> annotation or to override default behavior globally calling
> >>>>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
> >>>>> property visibility strategy.
> >>>>>
> >>>>>
> >>>>> The problem is that that the PropertyVisibilityStrategy is simply not
> >>>>> ready to implement the rules of 3.7.1 in a portable way when it
> comes to
> >>>>> class hierarchies
> >>>>>
> >>>>>
> >>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>> <
> >>>>>
> >>>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>>>>
> >>>>>
> >>>>> So either we say that the spec simply does not define what should
> happen
> >>>>> in case of hierarchic classes (totally valid imo) or we need to ditch
> >>> our
> >>>>> DefaultPropertyVisibility logic completely. It's simply not possible
> to
> >>>>> implement it that way.
> >>>>>
> >>>>> The reason why I removed the logic from the Reader is the following:
> >>> Even
> >>>>> if the reader says 'oh there is a getter, so let's hide the property'
> >>> you
> >>>>> STILL are perfectly allowed - as per the spec - to have Visibility
> >>>>> strategies which do disable ALL methods and ONLY use field access.
> >>>>> That means the current impl is surely more correct than before, but
> not
> >>>>> yet perfect.
> >>>>>
> >>>>> LieGrue,
> >>>>> strub
> >>>>>
> >>>>>
> >>>>>
> >>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> >>> rmannibucau@gmail.com
> >>>>>> :
> >>>>>>
> >>>>>> Hi JL,
> >>>>>>
> >>>>>> Last commit broke some stuff.
> >>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days,
> can it
> >>>>>> wait some days? Otherwise maybe just pick this commit?
> >>>>>>
> >>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >>>>> jlmonteiro@tomitribe.com>
> >>>>>> a écrit :
> >>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Is it ok to release a 1.2.17?
> >>>>>>> I'm looking for a Jakarta EE fix -
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>>>>
> >>>>>>> Do you guys have some additional fixes to push?
> >>>>>>> --
> >>>>>>> Jean-Louis Monteiro
> >>>>>>> http://twitter.com/jlouismonteiro
> >>>>>>> http://www.tomitribe.com
> >
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
here we go. plz take a look!
https://github.com/struberg/johnzon/tree/JOHNZON-364 <https://github.com/struberg/johnzon/tree/JOHNZON-364>
txs and LieGrue,
strub
> Am 09.04.2022 um 23:14 schrieb Mark Struberg <st...@yahoo.de.INVALID>:
>
> Looked at your proposed solution, but it might be tricky to get it working. The problem is that we have two modules: mapper and jsonb. And in JOHNZON-250 we added a logic to the mapper module which is ONLY defined and useful in Jsonb. This is the alwaysPreferMethodVisibility in org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually did break the mapper. Sorry that I did not catch this earlier.
>
> Note that the FieldAndMethodAccessMode is only used if no other AccessMode is configured. There is also a JsonbAccessMode which delegates through to the accessMode of the Mapper. Normally means the FieldAndMethodAccessMode, but could be different.
> We could implement the logic in JsonbAccessMode, totally ditching the accessMode coming from the Mapper. Also thought about having the JsonbAccessMode extending the FieldAndMethodAccessMode and adding the handling as parameter over there. still not really perfect imo.
>
> Will try to ship a patch proposal tomorrow.
>
> LieGrue,
> strub
>
>
>
>> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>>
>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid <ma...@yahoo.de.invalid>> a
>> écrit :
>>
>>> +1 for getting rid of java.beans.
>>>
>>> I've created a spec ticket for the open questions:
>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>>
>>> My point is: until my fix we clearly did break 4.6. And class hierarchies
>>> are totally not portable right now anyway. Introducing It's just badly
>>> underspecified.
>>> So I do not see why the current implementation does make it worse.
>>>
>>
>> Well this one is easy to answer: it changes the behavior and breaks users
>> so it is not an option for a patch version and to be honest the proposed
>> fix solves it for both cases so think we should tackle it this way, at
>> least for now until the spec clarifies it any other way.
>> Once again, the source of the issue is more that we lack an integration
>> between mapper and jsonb modules than anything else and this integration
>> had kind of been done but broke mapper so think we should fix it back, just
>> costs us a constructor we could have avoided without this issue AFAIK so
>> not a big deal, no?
>>
>>
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>>>> :
>>>>
>>>> Hi,
>>>>
>>>> From what we discussed on this issue, the thing is that our jsonb access
>>>> mode does not enable the visibility to see all readers/writers cause
>>> there
>>>> is some hardcoded filtering in the delegates so I think the trick is to
>>>> simply add a toggle to the delegates access modes we use to not have this
>>>> filtering and 100% rely on the visibility filtering in jsonbaccess mode
>>> we
>>>> already have in place.
>>>>
>>>> Before the release we must also ensure getters computation cache is
>>> cleaned
>>>> up after the ClassMapping is created - we don't want to keep that in
>>> memory
>>>> at runtime and not sure we can clean up at another time since we don't
>>> want
>>>> a background thread for that.
>>>> I also think it is not worth to depends on java.beans (java.desktop
>>> module)
>>>> so we would reimpl decapitalize as we already did somewhere else (it is
>>> one
>>>> liner anyway ;)).
>>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>>> restored
>>>> (it is part of our API and used), default should stay 1-1 but as
>>> mentionned
>>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>>> backward compat) to ignore the filtering.
>>>>
>>>> Sounds like this way we solve all issues and can release.
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github <
>>> https://github.com/rmannibucau> |
>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <
>>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>>
>>>>
>>>>
>>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg <st...@yahoo.de.invalid>
>>> a
>>>> écrit :
>>>>
>>>>> To sum this up:
>>>>>
>>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>>> happens
>>>>> if there are class hierarchies like class A extends B.
>>>>>
>>>>> In 3.7.1 it states that
>>>>> For a serialization operation, if a matching public getter method
>>> exists,
>>>>> the method is called to obtain the value of the property. If a matching
>>>>> getter method with private, protected, or defaulted to package-only
>>> access
>>>>> exists, then this field is ignored. If no matching getter method exists
>>> and
>>>>> the field is public, then the value is obtained directly from the field.
>>>>>
>>>>> This can get changed y using a @JsonbVisibility
>>>>>
>>>>> 4.6 Custom visibility
>>>>> To customize scope and field access strategy as specified in section
>>>>> 3.7.1, it is possible to specify
>>> javax.json.bind.annotation.JsonbVisibility
>>>>> annotation or to override default behavior globally calling
>>>>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
>>>>> property visibility strategy.
>>>>>
>>>>>
>>>>> The problem is that that the PropertyVisibilityStrategy is simply not
>>>>> ready to implement the rules of 3.7.1 in a portable way when it comes to
>>>>> class hierarchies
>>>>>
>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>> <
>>>>>
>>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>>
>>>>>
>>>>> So either we say that the spec simply does not define what should happen
>>>>> in case of hierarchic classes (totally valid imo) or we need to ditch
>>> our
>>>>> DefaultPropertyVisibility logic completely. It's simply not possible to
>>>>> implement it that way.
>>>>>
>>>>> The reason why I removed the logic from the Reader is the following:
>>> Even
>>>>> if the reader says 'oh there is a getter, so let's hide the property'
>>> you
>>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>>> strategies which do disable ALL methods and ONLY use field access.
>>>>> That means the current impl is surely more correct than before, but not
>>>>> yet perfect.
>>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>>
>>>>>
>>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>>> rmannibucau@gmail.com
>>>>>> :
>>>>>>
>>>>>> Hi JL,
>>>>>>
>>>>>> Last commit broke some stuff.
>>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days, can it
>>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>>
>>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>>> jlmonteiro@tomitribe.com>
>>>>>> a écrit :
>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Is it ok to release a 1.2.17?
>>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>>
>>>>>>> Do you guys have some additional fixes to push?
>>>>>>> --
>>>>>>> Jean-Louis Monteiro
>>>>>>> http://twitter.com/jlouismonteiro
>>>>>>> http://www.tomitribe.com
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Looked at your proposed solution, but it might be tricky to get it working. The problem is that we have two modules: mapper and jsonb. And in JOHNZON-250 we added a logic to the mapper module which is ONLY defined and useful in Jsonb. This is the alwaysPreferMethodVisibility in org.apache.johnzon.mapper.access.FieldAndMethodAccessMode. This actually did break the mapper. Sorry that I did not catch this earlier.
Note that the FieldAndMethodAccessMode is only used if no other AccessMode is configured. There is also a JsonbAccessMode which delegates through to the accessMode of the Mapper. Normally means the FieldAndMethodAccessMode, but could be different.
We could implement the logic in JsonbAccessMode, totally ditching the accessMode coming from the Mapper. Also thought about having the JsonbAccessMode extending the FieldAndMethodAccessMode and adding the handling as parameter over there. still not really perfect imo.
Will try to ship a patch proposal tomorrow.
LieGrue,
strub
> Am 28.03.2022 um 10:22 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Le lun. 28 mars 2022 à 10:05, Mark Struberg <struberg@yahoo.de.invalid <ma...@yahoo.de.invalid>> a
> écrit :
>
>> +1 for getting rid of java.beans.
>>
>> I've created a spec ticket for the open questions:
>> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
>> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>>
>> My point is: until my fix we clearly did break 4.6. And class hierarchies
>> are totally not portable right now anyway. Introducing It's just badly
>> underspecified.
>> So I do not see why the current implementation does make it worse.
>>
>
> Well this one is easy to answer: it changes the behavior and breaks users
> so it is not an option for a patch version and to be honest the proposed
> fix solves it for both cases so think we should tackle it this way, at
> least for now until the spec clarifies it any other way.
> Once again, the source of the issue is more that we lack an integration
> between mapper and jsonb modules than anything else and this integration
> had kind of been done but broke mapper so think we should fix it back, just
> costs us a constructor we could have avoided without this issue AFAIK so
> not a big deal, no?
>
>
>>
>> LieGrue,
>> strub
>>
>>> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>>> :
>>>
>>> Hi,
>>>
>>> From what we discussed on this issue, the thing is that our jsonb access
>>> mode does not enable the visibility to see all readers/writers cause
>> there
>>> is some hardcoded filtering in the delegates so I think the trick is to
>>> simply add a toggle to the delegates access modes we use to not have this
>>> filtering and 100% rely on the visibility filtering in jsonbaccess mode
>> we
>>> already have in place.
>>>
>>> Before the release we must also ensure getters computation cache is
>> cleaned
>>> up after the ClassMapping is created - we don't want to keep that in
>> memory
>>> at runtime and not sure we can clean up at another time since we don't
>> want
>>> a background thread for that.
>>> I also think it is not worth to depends on java.beans (java.desktop
>> module)
>>> so we would reimpl decapitalize as we already did somewhere else (it is
>> one
>>> liner anyway ;)).
>>> Lastly, FieldAndMethodAccessMode constructor compatibility must be
>> restored
>>> (it is part of our API and used), default should stay 1-1 but as
>> mentionned
>>> I'd add a new toggle (with a constructor calling "this" to keep the
>>> backward compat) to ignore the filtering.
>>>
>>> Sounds like this way we solve all issues and can release.
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github <
>> https://github.com/rmannibucau> |
>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>> <
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>>>
>>>
>>>
>>> Le lun. 21 mars 2022 à 09:00, Mark Struberg <st...@yahoo.de.invalid>
>> a
>>> écrit :
>>>
>>>> To sum this up:
>>>>
>>>> The problem lies partly in the JSON-B spec. It doesn't define what
>> happens
>>>> if there are class hierarchies like class A extends B.
>>>>
>>>> In 3.7.1 it states that
>>>> For a serialization operation, if a matching public getter method
>> exists,
>>>> the method is called to obtain the value of the property. If a matching
>>>> getter method with private, protected, or defaulted to package-only
>> access
>>>> exists, then this field is ignored. If no matching getter method exists
>> and
>>>> the field is public, then the value is obtained directly from the field.
>>>>
>>>> This can get changed y using a @JsonbVisibility
>>>>
>>>> 4.6 Custom visibility
>>>> To customize scope and field access strategy as specified in section
>>>> 3.7.1, it is possible to specify
>> javax.json.bind.annotation.JsonbVisibility
>>>> annotation or to override default behavior globally calling
>>>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
>>>> property visibility strategy.
>>>>
>>>>
>>>> The problem is that that the PropertyVisibilityStrategy is simply not
>>>> ready to implement the rules of 3.7.1 in a portable way when it comes to
>>>> class hierarchies
>>>>
>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>> <
>>>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>>>
>>>>
>>>> So either we say that the spec simply does not define what should happen
>>>> in case of hierarchic classes (totally valid imo) or we need to ditch
>> our
>>>> DefaultPropertyVisibility logic completely. It's simply not possible to
>>>> implement it that way.
>>>>
>>>> The reason why I removed the logic from the Reader is the following:
>> Even
>>>> if the reader says 'oh there is a getter, so let's hide the property'
>> you
>>>> STILL are perfectly allowed - as per the spec - to have Visibility
>>>> strategies which do disable ALL methods and ONLY use field access.
>>>> That means the current impl is surely more correct than before, but not
>>>> yet perfect.
>>>>
>>>> LieGrue,
>>>> strub
>>>>
>>>>
>>>>
>>>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com
>>>>> :
>>>>>
>>>>> Hi JL,
>>>>>
>>>>> Last commit broke some stuff.
>>>>> Dicussed it quickly offline with Mark but Im AFK for a few days, can it
>>>>> wait some days? Otherwise maybe just pick this commit?
>>>>>
>>>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>>>> jlmonteiro@tomitribe.com>
>>>>> a écrit :
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Is it ok to release a 1.2.17?
>>>>>> I'm looking for a Jakarta EE fix -
>>>>>>
>>>>>>
>>>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>>>
>>>>>> Do you guys have some additional fixes to push?
>>>>>> --
>>>>>> Jean-Louis Monteiro
>>>>>> http://twitter.com/jlouismonteiro
>>>>>> http://www.tomitribe.com
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le lun. 28 mars 2022 à 10:05, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> +1 for getting rid of java.beans.
>
> I've created a spec ticket for the open questions:
> https://github.com/eclipse-ee4j/jsonb-api/issues/325 <
> https://github.com/eclipse-ee4j/jsonb-api/issues/325>
>
> My point is: until my fix we clearly did break 4.6. And class hierarchies
> are totally not portable right now anyway. Introducing It's just badly
> underspecified.
> So I do not see why the current implementation does make it worse.
>
Well this one is easy to answer: it changes the behavior and breaks users
so it is not an option for a patch version and to be honest the proposed
fix solves it for both cases so think we should tackle it this way, at
least for now until the spec clarifies it any other way.
Once again, the source of the issue is more that we lack an integration
between mapper and jsonb modules than anything else and this integration
had kind of been done but broke mapper so think we should fix it back, just
costs us a constructor we could have avoided without this issue AFAIK so
not a big deal, no?
>
> LieGrue,
> strub
>
> > Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
> >:
> >
> > Hi,
> >
> > From what we discussed on this issue, the thing is that our jsonb access
> > mode does not enable the visibility to see all readers/writers cause
> there
> > is some hardcoded filtering in the delegates so I think the trick is to
> > simply add a toggle to the delegates access modes we use to not have this
> > filtering and 100% rely on the visibility filtering in jsonbaccess mode
> we
> > already have in place.
> >
> > Before the release we must also ensure getters computation cache is
> cleaned
> > up after the ClassMapping is created - we don't want to keep that in
> memory
> > at runtime and not sure we can clean up at another time since we don't
> want
> > a background thread for that.
> > I also think it is not worth to depends on java.beans (java.desktop
> module)
> > so we would reimpl decapitalize as we already did somewhere else (it is
> one
> > liner anyway ;)).
> > Lastly, FieldAndMethodAccessMode constructor compatibility must be
> restored
> > (it is part of our API and used), default should stay 1-1 but as
> mentionned
> > I'd add a new toggle (with a constructor calling "this" to keep the
> > backward compat) to ignore the filtering.
> >
> > Sounds like this way we solve all issues and can release.
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >
> >
> > Le lun. 21 mars 2022 à 09:00, Mark Struberg <st...@yahoo.de.invalid>
> a
> > écrit :
> >
> >> To sum this up:
> >>
> >> The problem lies partly in the JSON-B spec. It doesn't define what
> happens
> >> if there are class hierarchies like class A extends B.
> >>
> >> In 3.7.1 it states that
> >> For a serialization operation, if a matching public getter method
> exists,
> >> the method is called to obtain the value of the property. If a matching
> >> getter method with private, protected, or defaulted to package-only
> access
> >> exists, then this field is ignored. If no matching getter method exists
> and
> >> the field is public, then the value is obtained directly from the field.
> >>
> >> This can get changed y using a @JsonbVisibility
> >>
> >> 4.6 Custom visibility
> >> To customize scope and field access strategy as specified in section
> >> 3.7.1, it is possible to specify
> javax.json.bind.annotation.JsonbVisibility
> >> annotation or to override default behavior globally calling
> >> JsonbConfig::withPropertyVisibilityStrategy method with given custom
> >> property visibility strategy.
> >>
> >>
> >> The problem is that that the PropertyVisibilityStrategy is simply not
> >> ready to implement the rules of 3.7.1 in a portable way when it comes to
> >> class hierarchies
> >>
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >> <
> >>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >>>
> >>
> >> So either we say that the spec simply does not define what should happen
> >> in case of hierarchic classes (totally valid imo) or we need to ditch
> our
> >> DefaultPropertyVisibility logic completely. It's simply not possible to
> >> implement it that way.
> >>
> >> The reason why I removed the logic from the Reader is the following:
> Even
> >> if the reader says 'oh there is a getter, so let's hide the property'
> you
> >> STILL are perfectly allowed - as per the spec - to have Visibility
> >> strategies which do disable ALL methods and ONLY use field access.
> >> That means the current impl is surely more correct than before, but not
> >> yet perfect.
> >>
> >> LieGrue,
> >> strub
> >>
> >>
> >>
> >>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com
> >>> :
> >>>
> >>> Hi JL,
> >>>
> >>> Last commit broke some stuff.
> >>> Dicussed it quickly offline with Mark but Im AFK for a few days, can it
> >>> wait some days? Otherwise maybe just pick this commit?
> >>>
> >>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> >> jlmonteiro@tomitribe.com>
> >>> a écrit :
> >>>
> >>>> Hi all,
> >>>>
> >>>> Is it ok to release a 1.2.17?
> >>>> I'm looking for a Jakarta EE fix -
> >>>>
> >>>>
> >>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>>>
> >>>> Do you guys have some additional fixes to push?
> >>>> --
> >>>> Jean-Louis Monteiro
> >>>> http://twitter.com/jlouismonteiro
> >>>> http://www.tomitribe.com
> >>>>
> >>
> >>
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
+1 for getting rid of java.beans.
I've created a spec ticket for the open questions:
https://github.com/eclipse-ee4j/jsonb-api/issues/325 <https://github.com/eclipse-ee4j/jsonb-api/issues/325>
My point is: until my fix we clearly did break 4.6. And class hierarchies are totally not portable right now anyway. Introducing It's just badly underspecified.
So I do not see why the current implementation does make it worse.
LieGrue,
strub
> Am 21.03.2022 um 09:52 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Hi,
>
> From what we discussed on this issue, the thing is that our jsonb access
> mode does not enable the visibility to see all readers/writers cause there
> is some hardcoded filtering in the delegates so I think the trick is to
> simply add a toggle to the delegates access modes we use to not have this
> filtering and 100% rely on the visibility filtering in jsonbaccess mode we
> already have in place.
>
> Before the release we must also ensure getters computation cache is cleaned
> up after the ClassMapping is created - we don't want to keep that in memory
> at runtime and not sure we can clean up at another time since we don't want
> a background thread for that.
> I also think it is not worth to depends on java.beans (java.desktop module)
> so we would reimpl decapitalize as we already did somewhere else (it is one
> liner anyway ;)).
> Lastly, FieldAndMethodAccessMode constructor compatibility must be restored
> (it is part of our API and used), default should stay 1-1 but as mentionned
> I'd add a new toggle (with a constructor calling "this" to keep the
> backward compat) to ignore the filtering.
>
> Sounds like this way we solve all issues and can release.
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>
> Le lun. 21 mars 2022 à 09:00, Mark Struberg <st...@yahoo.de.invalid> a
> écrit :
>
>> To sum this up:
>>
>> The problem lies partly in the JSON-B spec. It doesn't define what happens
>> if there are class hierarchies like class A extends B.
>>
>> In 3.7.1 it states that
>> For a serialization operation, if a matching public getter method exists,
>> the method is called to obtain the value of the property. If a matching
>> getter method with private, protected, or defaulted to package-only access
>> exists, then this field is ignored. If no matching getter method exists and
>> the field is public, then the value is obtained directly from the field.
>>
>> This can get changed y using a @JsonbVisibility
>>
>> 4.6 Custom visibility
>> To customize scope and field access strategy as specified in section
>> 3.7.1, it is possible to specify javax.json.bind.annotation.JsonbVisibility
>> annotation or to override default behavior globally calling
>> JsonbConfig::withPropertyVisibilityStrategy method with given custom
>> property visibility strategy.
>>
>>
>> The problem is that that the PropertyVisibilityStrategy is simply not
>> ready to implement the rules of 3.7.1 in a portable way when it comes to
>> class hierarchies
>>
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>> <
>> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
>>>
>>
>> So either we say that the spec simply does not define what should happen
>> in case of hierarchic classes (totally valid imo) or we need to ditch our
>> DefaultPropertyVisibility logic completely. It's simply not possible to
>> implement it that way.
>>
>> The reason why I removed the logic from the Reader is the following: Even
>> if the reader says 'oh there is a getter, so let's hide the property' you
>> STILL are perfectly allowed - as per the spec - to have Visibility
>> strategies which do disable ALL methods and ONLY use field access.
>> That means the current impl is surely more correct than before, but not
>> yet perfect.
>>
>> LieGrue,
>> strub
>>
>>
>>
>>> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>>> :
>>>
>>> Hi JL,
>>>
>>> Last commit broke some stuff.
>>> Dicussed it quickly offline with Mark but Im AFK for a few days, can it
>>> wait some days? Otherwise maybe just pick this commit?
>>>
>>> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
>> jlmonteiro@tomitribe.com>
>>> a écrit :
>>>
>>>> Hi all,
>>>>
>>>> Is it ok to release a 1.2.17?
>>>> I'm looking for a Jakarta EE fix -
>>>>
>>>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>>>
>>>> Do you guys have some additional fixes to push?
>>>> --
>>>> Jean-Louis Monteiro
>>>> http://twitter.com/jlouismonteiro
>>>> http://www.tomitribe.com
>>>>
>>
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi,
From what we discussed on this issue, the thing is that our jsonb access
mode does not enable the visibility to see all readers/writers cause there
is some hardcoded filtering in the delegates so I think the trick is to
simply add a toggle to the delegates access modes we use to not have this
filtering and 100% rely on the visibility filtering in jsonbaccess mode we
already have in place.
Before the release we must also ensure getters computation cache is cleaned
up after the ClassMapping is created - we don't want to keep that in memory
at runtime and not sure we can clean up at another time since we don't want
a background thread for that.
I also think it is not worth to depends on java.beans (java.desktop module)
so we would reimpl decapitalize as we already did somewhere else (it is one
liner anyway ;)).
Lastly, FieldAndMethodAccessMode constructor compatibility must be restored
(it is part of our API and used), default should stay 1-1 but as mentionned
I'd add a new toggle (with a constructor calling "this" to keep the
backward compat) to ignore the filtering.
Sounds like this way we solve all issues and can release.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Le lun. 21 mars 2022 à 09:00, Mark Struberg <st...@yahoo.de.invalid> a
écrit :
> To sum this up:
>
> The problem lies partly in the JSON-B spec. It doesn't define what happens
> if there are class hierarchies like class A extends B.
>
> In 3.7.1 it states that
> For a serialization operation, if a matching public getter method exists,
> the method is called to obtain the value of the property. If a matching
> getter method with private, protected, or defaulted to package-only access
> exists, then this field is ignored. If no matching getter method exists and
> the field is public, then the value is obtained directly from the field.
>
> This can get changed y using a @JsonbVisibility
>
> 4.6 Custom visibility
> To customize scope and field access strategy as specified in section
> 3.7.1, it is possible to specify javax.json.bind.annotation.JsonbVisibility
> annotation or to override default behavior globally calling
> JsonbConfig::withPropertyVisibilityStrategy method with given custom
> property visibility strategy.
>
>
> The problem is that that the PropertyVisibilityStrategy is simply not
> ready to implement the rules of 3.7.1 in a portable way when it comes to
> class hierarchies
>
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> <
> https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java
> >
>
> So either we say that the spec simply does not define what should happen
> in case of hierarchic classes (totally valid imo) or we need to ditch our
> DefaultPropertyVisibility logic completely. It's simply not possible to
> implement it that way.
>
> The reason why I removed the logic from the Reader is the following: Even
> if the reader says 'oh there is a getter, so let's hide the property' you
> STILL are perfectly allowed - as per the spec - to have Visibility
> strategies which do disable ALL methods and ONLY use field access.
> That means the current impl is surely more correct than before, but not
> yet perfect.
>
> LieGrue,
> strub
>
>
>
> > Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
> >:
> >
> > Hi JL,
> >
> > Last commit broke some stuff.
> > Dicussed it quickly offline with Mark but Im AFK for a few days, can it
> > wait some days? Otherwise maybe just pick this commit?
> >
> > Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <
> jlmonteiro@tomitribe.com>
> > a écrit :
> >
> >> Hi all,
> >>
> >> Is it ok to release a 1.2.17?
> >> I'm looking for a Jakarta EE fix -
> >>
> >>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
> >>
> >> Do you guys have some additional fixes to push?
> >> --
> >> Jean-Louis Monteiro
> >> http://twitter.com/jlouismonteiro
> >> http://www.tomitribe.com
> >>
>
>
Re: OK to release Johnzon 1.2.17?
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
To sum this up:
The problem lies partly in the JSON-B spec. It doesn't define what happens if there are class hierarchies like class A extends B.
In 3.7.1 it states that
For a serialization operation, if a matching public getter method exists, the method is called to obtain the value of the property. If a matching getter method with private, protected, or defaulted to package-only access exists, then this field is ignored. If no matching getter method exists and the field is public, then the value is obtained directly from the field.
This can get changed y using a @JsonbVisibility
4.6 Custom visibility
To customize scope and field access strategy as specified in section 3.7.1, it is possible to specify javax.json.bind.annotation.JsonbVisibility annotation or to override default behavior globally calling JsonbConfig::withPropertyVisibilityStrategy method with given custom property visibility strategy.
The problem is that that the PropertyVisibilityStrategy is simply not ready to implement the rules of 3.7.1 in a portable way when it comes to class hierarchies
https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java <https://github.com/apache/geronimo-specs/blob/trunk/geronimo-jsonb_1.0_spec/src/main/java/javax/json/bind/config/PropertyVisibilityStrategy.java>
So either we say that the spec simply does not define what should happen in case of hierarchic classes (totally valid imo) or we need to ditch our DefaultPropertyVisibility logic completely. It's simply not possible to implement it that way.
The reason why I removed the logic from the Reader is the following: Even if the reader says 'oh there is a getter, so let's hide the property' you STILL are perfectly allowed - as per the spec - to have Visibility strategies which do disable ALL methods and ONLY use field access.
That means the current impl is surely more correct than before, but not yet perfect.
LieGrue,
strub
> Am 28.02.2022 um 11:17 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Hi JL,
>
> Last commit broke some stuff.
> Dicussed it quickly offline with Mark but Im AFK for a few days, can it
> wait some days? Otherwise maybe just pick this commit?
>
> Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <jl...@tomitribe.com>
> a écrit :
>
>> Hi all,
>>
>> Is it ok to release a 1.2.17?
>> I'm looking for a Jakarta EE fix -
>>
>> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>>
>> Do you guys have some additional fixes to push?
>> --
>> Jean-Louis Monteiro
>> http://twitter.com/jlouismonteiro
>> http://www.tomitribe.com
>>
Re: OK to release Johnzon 1.2.17?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi JL,
Last commit broke some stuff.
Dicussed it quickly offline with Mark but Im AFK for a few days, can it
wait some days? Otherwise maybe just pick this commit?
Le lun. 28 févr. 2022 à 10:37, Jean-Louis Monteiro <jl...@tomitribe.com>
a écrit :
> Hi all,
>
> Is it ok to release a 1.2.17?
> I'm looking for a Jakarta EE fix -
>
> https://github.com/apache/johnzon/commit/a1c53d99c3280cbfc35b369ac4ce138e12f1d5ff
>
> Do you guys have some additional fixes to push?
> --
> Jean-Louis Monteiro
> http://twitter.com/jlouismonteiro
> http://www.tomitribe.com
>