Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add constructor for custom mapping with Jackson #1250

Merged
merged 2 commits into from Aug 7, 2019
Merged

Add constructor for custom mapping with Jackson #1250

merged 2 commits into from Aug 7, 2019

Conversation

kostya05983
Copy link

@kostya05983 kostya05983 commented Jul 27, 2019

Subsystem
ktor-client:ktor-client-features:ktor-client-json:ktor-client-jackson

Motivation
Pr adds custom jackson mapper, now people can do

val client = HttpClient(HttpClientEngine) {
    install(JsonFeature) {
        serializer = JacksonSerializer(ObjectMapper())
    }
}

refs #1208
Link for docs pr https://github.com/ktorio/ktorio.github.io/pull/204
Solution
I deleted one primary constructor and added to constructor in body of class with argument of previous constructor and constructor with objectMapper parameter to init backend value.

@kostya05983 kostya05983 marked this pull request as ready for review July 27, 2019 18:25
@cy6erGn0m
Copy link
Contributor

cy6erGn0m commented Jul 29, 2019

The only question is why? What kind of mapper do you want to pass instead of the default mapper and why do you need to replace it? Please remember that missing registerKotlinModule invocation may lead to an inability to serialize/deserialize kotlin classes in some cases.

@cy6erGn0m cy6erGn0m requested a review from e5l July 29, 2019 14:25
@kostya05983
Copy link
Author

I did it to help the user in this ticket #1208. I think, maybe someone uses DI and want to pass objectMapper easy.

@kostya05983
Copy link
Author

Hello, @e5l may you review this?
Thanks

@e5l e5l changed the base branch from master to e5l/develop August 7, 2019 10:54
@e5l
Copy link
Member

e5l commented Aug 7, 2019

Hi @kostya05983, thanks for the PR.

Merged in develop.

@e5l e5l merged commit 574fa6c into ktorio:e5l/develop Aug 7, 2019
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.

None yet

3 participants