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 >>>>> >>>> >>>> >> >>