convert middleware jackson to kotlin #359
Conversation
brianPlummer
assigned cybo42, goldstei, wdziemia, candidcold and ramonaharrison and unassigned cybo42, goldstei, wdziemia, candidcold and ramonaharrison
brianPlummer
requested review from
cybo42,
goldstei,
tiwiz,
wdziemia and
ramonaharrison
|
this new pr should address all the comments in https://github.com/NYTimes/Store/pull/350/files . |
| /** | ||
| * An implementation of [BufferedSourceAdapter] that uses [ObjectMapper] to convert Java values to JSON. | ||
| */ | ||
| class JacksonBufferedSourceAdapter<Parsed> @Inject |
There was a problem hiding this comment.
The Java class was also containing a logic that would propagate a thrown exception! Is that deleted in purpose on the Kotlin conversion?
There was a problem hiding this comment.
Yes! @tiwiz mentioned failing fast, not sure 100% if that is the desired direction. @digitalbuddha
There was a problem hiding this comment.
that would be a breaking change imo, leave the same as before pls
| private val parsedType: JavaType | ||
|
|
||
| constructor(jsonFactory: JsonFactory, type: Type) { | ||
| objectMapper = ObjectMapper(jsonFactory).registerModule(KotlinModule()) |
There was a problem hiding this comment.
Maybe add this. here as well, to match the @Inject annotated constructor as well?
|
|
||
| @Inject | ||
| constructor(objectMapper: ObjectMapper, type: Type) { | ||
| this.objectMapper = objectMapper |
There was a problem hiding this comment.
Either remove this. from here or add it to the other constructors as well :)
| @Throws(ParserException::class) | ||
| override fun apply(@NonNull bufferedSource: BufferedSource): Parsed { | ||
| val inputStream = bufferedSource.inputStream() | ||
| try { |
There was a problem hiding this comment.
Can be lift to return try { ... }
|
|
||
| @Inject | ||
| constructor(objectMapper: ObjectMapper, type: Type) { | ||
| this.objectMapper = objectMapper |
There was a problem hiding this comment.
Same as before, either remove this. or add it on the other constructor as well.
| */ | ||
| @Experimental | ||
| fun <Parsed> createObjectToSourceTransformer(objectMapper: ObjectMapper): ObjectToSourceTransformer<Parsed> { | ||
| return ObjectToSourceTransformer(JacksonBufferedSourceAdapter(objectMapper)) |
There was a problem hiding this comment.
Can be lift to assignment!
No description provided.