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 <rmannibu...@gmail.com>:
> 
> Le dim. 10 avr. 2022 à 17:16, Mark Struberg <strub...@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 <rmannibu...@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 <strub...@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 <strub...@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 <
>>>> rmannibu...@gmail.com>:
>>>>>> 
>>>>>> Le lun. 28 mars 2022 à 10:05, Mark Struberg <strub...@yahoo.de.invalid
>>>> <mailto:strub...@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 <
>>>> rmannibu...@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
>> <strub...@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 <
>>>>>>> rmannibu...@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 <
>>>>>>>>> jlmonte...@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
>>>>> 
>>>> 
>>>> 
>> 
>> 

Reply via email to