Skip to content

Upgrade to jackson3 - #76

Open
guanxuc wants to merge 1 commit into
embabel:mainfrom
guanxuc:upgrade-jackson3
Open

Upgrade to jackson3#76
guanxuc wants to merge 1 commit into
embabel:mainfrom
guanxuc:upgrade-jackson3

Conversation

@guanxuc

@guanxuc guanxuc commented Aug 2, 2026

Copy link
Copy Markdown

No description provided.

Signed-off-by: guanxu <1510424541@qq.com>

@igordayen igordayen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, @guanxuc thank you

@jimador jimador left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you. Just a couple of house keeping comments for existing clients.

import org.springframework.http.ResponseEntity
import org.springframework.web.bind.annotation.*
import org.springframework.web.multipart.MultipartFile
import tools.jackson.databind.ObjectMapper

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we verify that existing consumers can still start and use Dice JSON with Jackson 2 on the classpath? If not, this needs a compatibility period before changing the public ObjectMapper type.

class PropositionJacksonRoundTripTest {

private val mapper = jacksonObjectMapper().findAndRegisterModules()
private val mapper = jacksonObjectMapper()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a JSON fixture written by the Jackson 2 repository and load it here? This only tests a Jackson 3 round trip, so it does not cover existing persisted data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants