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 <rmannibu...@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 <rmannibu...@gmail.com> a > écrit : > >> >> >> Le dim. 10 avr. 2022 à 17:34, Mark Struberg <strub...@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 < >>> 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 >>>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>> >>>
