Le mer. 12 avr. 2023 à 16:24, Markus Jung <[email protected]> a écrit :
> Hi Romain, > > > thanks for your feedback, I think the upgrade won't go seamless for most > users because of javax.* to jakarta.* migration. > I see, but we already went through this with JSONB 2. > > But if someone only uses mapper directly johnzon 2.x might actually just > be a drop in replacement for johnzon 1.x, depending on how it's used. I didnt understand, mapper is not JSONB. That said we have two cases: * mapper only usage (no jsonb) * @polymorphism from jsonb using mapper api (never said it is good but it is used) > I > agree that we should aim to make this possible, still think it might be > a bit confusing in MapperBuilder though. I added a bit of documentation > on what PolymorphismHandler is, why it exists and what setting it does. > Check a bit your PR, I wouldn't leak in mapper the jsonb design we don't fully ack. We also try to not break Mapper API (constructors). Rest and design looks ok otherwise. Side note: don't forget to close your json instance in tests (we have a rule for that if that helps), we see too much broken snippet not doing it to do it ourselves. > > > Thanks > > Markus > > On 11.04.23 21:32, Romain Manni-Bucau wrote: > > Well thing is we will NOT make it to johnzon-mapper but only in > > johnzon-jsonb so all good for me. This is a jsonb specific thing so will > be > > implemented in jsonb module. Some plumbing can be done in mapper while it > > does not break anything, does not make things slower nor surface in the > api. > > > > Both are different features and one is more relevant in several cases so > we > > should kezp both IMHO. > > > > Side note: we made the upgrade seamless so not sure what you mean. > > > > Sure we can break now but not to drop a feature IMHO - no acceptable > > migration is possible there AFAIK. > > > > Last, TCK does not require much breaking change - or if some is missed we > > will challenge and fix the spec - so not a strong point for me too to > > assume there will be any breakage (if so jakarta would be dead right? > They > > already hurt themselves enough with this kind of behavior and jsonb is > not > > a bad citizen there) > > > > Le mar. 11 avr. 2023 à 21:08, Markus Jung <[email protected]> a > écrit : > > > >> Hi all, > >> > >> I’ve made some progress on my jsonb 3 polymorphism impl and I’m now at a > >> point where I feel comfortable to open a PR if we can agree on how to > move > >> forward with johnzon-mapper’s polymorphism. > >> > >> @romain I think having two more or less unrelated sets of properties in > >> MapperBuilder for the same thing adds more confusion than what it’s > worth. > >> The code is not really that much more complex, just adds a bit more > >> flexibility I needed for hooking up jsonb polymorphism. For > johnzon-mapper > >> users that rely on a simple way to handle polymorphic types theres > >> JohnzonMapperPolymorphismHandler, though I’d like to move it’s > properties > >> out of MapperBuilder into some other Builder that can be used just like > >> PolymorphicConfig could be on the jsonb side. > >> > >> I’m with JL here - upgrading to johnzon 2.* won’t be seamless anyways, > so > >> quite honestly I’m okay with a breaking change. Just need to clearly > >> document how to migrate in some sort of changelog/johnzon 2.0 migration > >> guide. Feel like this won’t be the only breaking change that will arise > >> from jsonb 3/jsonp 2.1 tck compatibility. > >> > >> > >> Thanks > >> > >> Markus > >> > >>> On 8. Apr 2023, at 09:52, Romain Manni-Bucau <[email protected]> > >> wrote: > >>> Hi Markus, > >>> > >>> Great news! > >>> > >>> Personally I'd keep the small wiring we have right now and implement > >> jsonb3 > >>> flavor aside since both have different design and dont converge so no > >> need > >>> to make it complicated. > >>> > >>> Le ven. 7 avr. 2023 à 22:57, Jean-Louis MONTEIRO <[email protected]> > a > >>> écrit : > >>> > >>>> Hi Markus, > >>>> > >>>> I'll have a look from Monday on. Thanks for jumping into this and > >> helping > >>>> out. > >>>> > >>>> Note that if we have to break something to pass the TCK now is the > good > >>>> time. > >>>> > >>>> We are already breaking backward compatibility because of javax to > >> Jakarta > >>>> namespace. So I don't have any problem. > >>>> > >>>> Best regards > >>>> > >>>> Le ven. 7 avr. 2023, 16:46, Markus Jung <[email protected]> a > écrit > >> : > >>>>> Hi all, > >>>>> > >>>>> I’m currently working on implementing JSON-B 3 polymorphism. See > >>>>> https://github.com/jungm/johnzon/tree/jsonb3-polymorphism, still WIP > >> as > >>>> I > >>>>> need to write tests + documentation and remove johnzon-jsonb-extras. > >> But > >>>>> it’s passing all TCK polymorphism tests except for one as of right > now > >>>> (And > >>>>> that last one is probably related to locale validation in johnzon). > >>>>> > >>>>> I couldn’t really implement this on top of the current way > polymorphism > >>>>> works in johnzon-mapper because it expects that JSONs will always > only > >>>>> contain one property to describe it’s type vs the JSON-B 3 spec > having > >>>>> multiple properties to describe the type. > >>>>> As of now I have created a default polymorphism implementation that > >>>>> seamlessly mimics the way polymorphism used to work before in > >>>>> johnzon-mapper to avoid breaking changes for users, but this approach > >>>>> creates some redundancies in MapperBuilder. > >>>>> > >>>>> Ideally I’d like to completely remove this old polymorphism code also > >>>> from > >>>>> MapperBuilder and create some sort of PolymorphismBuilder that could > be > >>>>> used instead and plugged into MapperBuilder. But this will obviously > >>>> break > >>>>> the API for anyone using johonzon-mapper directly with polymorphism. > >> Any > >>>>> thoughts on this? > >>>>> > >>>>> > >>>>> Thanks > >>>>> > >>>>> Markus > >> >
