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 >
