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
