Added custom serializer example #924

RemusRD · 2019-01-03T20:11:43Z

Hi!, first PR here, hope it all works, just tell me if anything has to be fixed, thanks!

artembilan

Please, find some feedback from me in code lines.

In general this is great, although looks a little complicated.
Maybe just because I don't know Kotlin well...

Thank you very much so far!

artembilan · 2019-01-03T20:13:42Z

samples/sample-04/.gitignore

@@ -0,0 +1,29 @@
+# Created by .ignore support plugin (hsz.mobi)


Please, copy/paste content from one existing .ignore file, e.g. : https://github.com/spring-projects/spring-kafka/blob/master/samples/sample-01/.gitignore

artembilan · 2019-01-03T20:14:23Z

samples/sample-04/README.adoc

+== Sample 4
+
+This sample uses a custom kafka serializer and deserializer in order to consume and send events using the jdk8 LocalDatetime
+and Kotlin data classes.


Please, follow One sentence per line rule: https://asciidoctor.org/docs/asciidoc-recommended-practices/#one-sentence-per-line

artembilan · 2019-01-03T20:16:20Z

samples/sample-04/build.gradle

+        classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:${kotlinVersion}")
+        classpath("org.jetbrains.kotlin:kotlin-allopen:${kotlinVersion}")
+        classpath("org.jetbrains.kotlin:kotlin-noarg:${kotlinVersion}")
+        classpath("io.spring.gradle:dependency-management-plugin:1.0.2.RELEASE")


Looks like there is a newer version of this plugin: https://github.com/spring-gradle-plugins/dependency-management-plugin/releases

artembilan · 2019-01-03T20:16:58Z

samples/sample-04/build.gradle

+    ext {
+        kotlinVersion = "1.3.10"
+        springBootVersion = "2.1.1.RELEASE"
+        arrow_version = "0.8.1"


What is this about?
Doesn't look like this property is used somewhere in the buildscript definition

artembilan · 2019-01-03T20:18:02Z

samples/sample-04/build.gradle

+apply plugin: "kotlin-jpa"
+
+
+group "com.remusrd"


That's wrong group if you contribute into this project.
See all other samples here on the matter.

artembilan · 2019-01-03T20:26:19Z

samples/sample-04/src/main/kotlin/com/remusrd/notesample/configuration/KafkaConfig.kt

+import org.springframework.context.annotation.Bean
+import org.springframework.context.annotation.Configuration
+import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder
+import org.springframework.kafka.core.*


No asterisk imports, please.

artembilan · 2019-01-03T20:26:52Z

samples/sample-04/src/main/kotlin/com/remusrd/notesample/configuration/KafkaConfig.kt

+
+
+    @Bean
+    fun kafkaTemplate(): KafkaTemplate<Any, Any> {


If we rely on Spring Boot, I don't see reason to declare this bean.

This was because IntelliJ says there are no beans of type KafkaTemplate, I guess it is a bug of some type

artembilan · 2019-01-03T20:28:24Z

samples/sample-04/src/main/kotlin/com/remusrd/notesample/configuration/KafkaConfig.kt

+
+
+    @Bean
+    fun defaultKafkaConsumerFactory(): ConsumerFactory<Any, Any> {


Why do we need to override auto-configured beans?
This one and the next - defaultKafkaProducerFactory().

What should I do instead?, creating my own beans without replacing the default ones?

artembilan · 2019-01-03T20:31:36Z

samples/sample-04/src/main/kotlin/com/remusrd/notesample/domain/annotation/NoArg.kt

+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+annotation class NoArg


Why do we need this annotation and the next Open one?
Aren't there something already explicit in Kotlin world?
Like I see those in the Gradle config:

apply plugin: "kotlin-allopen" apply plugin: "kotlin-noarg"

artembilan · 2019-01-03T20:33:25Z

samples/sample-04/src/main/resources/application.yml

+spring.kafka.bootstrap-servers: localhost:9092
+spring.kafka.consumer.group-id: noteGroup
+spring.kafka.consumer.auto-offset-reset: earliest
+spring.kafka.consumer.properties.spring.json.trusted.packages: com.remusrd.notesample.domain.event


Can you reformat this file to really true YAML style?

Thanks a lot for the feedback, will fix it in the next days!

artembilan

Please, consider the next review round.

Thanks

artembilan · 2019-01-08T19:50:11Z

samples/sample-04/build.gradle

+    implementation "org.springframework.boot:spring-boot-starter-web"
+    implementation "org.springframework.boot:spring-boot-starter-data-jpa"
+    implementation "org.springframework.kafka:spring-kafka"
+    implementation "com.fasterxml.jackson.module:jackson-module-kotlin:2.9.7"


We don't need an explicit version for this dependency as well. It is managed by the jackson-bom: https://github.com/FasterXML/jackson-bom/blob/master/pom.xml#L258

artembilan · 2019-01-08T19:50:56Z

samples/sample-04/build.gradle

+
+    // BBDD
+    implementation "mysql:mysql-connector-java:8.0.13"
+    implementation "com.h2database:h2:1.4.197"


These two are managed by Spring Boot: we don't need their versions.

artembilan · 2019-01-08T19:53:54Z

samples/sample-04/src/main/kotlin/com/example/controller/NoteController.kt

+import com.example.domain.Note
+import com.example.service.NoteService
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.web.bind.annotation.*


Please, revise all the classes for avoiding asterisk imports.

artembilan · 2019-01-08T19:54:59Z

samples/sample-04/src/main/kotlin/com/example/domain/event/NoteEvent.kt

+import com.example.domain.annotation.Open
+
+@NoArg
+@Open


Why do we need these annotations?
Why the mentioned plugins not enough?

artembilan · 2019-01-08T19:55:29Z

samples/sample-04/src/main/kotlin/com/example/sampleApp.kt

+import org.springframework.boot.autoconfigure.SpringBootApplication
+
+@SpringBootApplication
+class NoteSampleApplication


Why file name doesn't match class name?

artembilan · 2019-01-08T19:56:43Z

samples/sample-04/src/main/resources/application.yml

+      properties:
+        spring:
+          json:
+            trusted:packages: com.remusrd.notesample.domain.event


What is this package?
Doesn't look like legitimate package name in the public project.

artembilan · 2019-01-08T19:58:11Z

samples/sample-04/build.gradle

+    annotation("com.remusrd.notesample.domain.annotation.NoArg")
+}
+allOpen{
+    annotation("com.remusrd.notesample.domain.annotation.Open")


I feel like we can just remove these two declarations.
First of all - wrong package.
Secondly if we are here, then project even works without these two annotations.

I'm confused, so everybody else is going to be too. 🤷‍♂️

rahilsh · 2020-08-22T18:48:44Z

samples/sample-04/README.adoc

+
+Docker containers needed are in `samples/sample-04/docker`.
+
+You can do POST's and GET's to the `localhost:8080/notes endpoint.`


You can do POST's and GET's to the localhost:8080/notes endpoint.

garyrussell

Thanks for the contribution, but I concur with @artembilan - the samples here are meant to be small and compact snippets to show how to perform simple tasks, rather than fully functional applications. Configuring a custom deserializer is a rather trivial task and a user shouldn't have to plough through all this code to find out how to do it.

It probably belongs in some external samples repo.

garyrussell · 2020-08-24T21:30:59Z

samples/sample-04/src/main/kotlin/com/example/configuration/KafkaConfig.kt

+        val jsonDeserializer = JsonDeserializer<Any>(objectMapper)
+        jsonDeserializer.configure(kafkaProperties.buildConsumerProperties(), false)
+        val kafkaConsumerFactory = DefaultKafkaConsumerFactory<Any, Any>(
+                kafkaProperties.buildConsumerProperties(),


Unfortunately, the Boot team does not consider KafkaProperties a public API and we would be conflicting with their position if we put code like this in an official Spring repo, promoting its use. They asked us to remove all such references from the documentation.

artembilan · 2024-01-17T18:24:34Z

I'm closing this one as not fully Kafka related: too much Kotlin, too much JPA.
The sample should be as simple as possible to give a reader a chance to learn something.
Plus there is no reporter reaction for our review for a long time.

Thanks for understanding.

Added custom serializer example

01ae368

artembilan requested changes Jan 3, 2019

View reviewed changes

Remus Richard Dumitrache added 9 commits January 5, 2019 18:58

fixed gitignore, refactored application.yml

dd2f143

removed dumb bean, updated gradle-wrapper version

d72a4ec

Removed default property

0d28587

changed groupId of the project

0ffcf04

refactored directory names +1

0815e19

removed some idea files

64ad620

gradle springbootversion removed

76947c5

moved arrow dependency scope

d291fe6

copyright notice and spring plugin dependency version

e0bd4d5

artembilan requested changes Jan 8, 2019

View reviewed changes

rahilsh reviewed Aug 22, 2020

View reviewed changes

garyrussell reviewed Aug 24, 2020

View reviewed changes

garyrussell added the status: waiting for reporter label Aug 24, 2020

artembilan closed this Jan 17, 2024

artembilan added status: invalid and removed status: waiting for reporter labels Jan 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added custom serializer example #924

Added custom serializer example #924

RemusRD commented Jan 3, 2019

artembilan left a comment

artembilan Jan 3, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

RemusRD Jan 5, 2019

artembilan Jan 3, 2019

RemusRD Jan 5, 2019

artembilan Jan 3, 2019

artembilan Jan 3, 2019

RemusRD Jan 3, 2019

artembilan left a comment

artembilan Jan 8, 2019

artembilan Jan 8, 2019

artembilan Jan 8, 2019

artembilan Jan 8, 2019

artembilan Jan 8, 2019

artembilan Jan 8, 2019

artembilan Jan 8, 2019 •

edited

Loading

rahilsh Aug 22, 2020

garyrussell left a comment

garyrussell Aug 24, 2020

artembilan commented Jan 17, 2024

		@@ -0,0 +1,29 @@
		# Created by .ignore support plugin (hsz.mobi)



		@Bean
		fun defaultKafkaConsumerFactory(): ConsumerFactory<Any, Any> {


		Docker containers needed are in `samples/sample-04/docker`.

		You can do POST's and GET's to the `localhost:8080/notes endpoint.`

Added custom serializer example #924

Added custom serializer example #924

Conversation

RemusRD commented Jan 3, 2019

artembilan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artembilan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artembilan Jan 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyrussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artembilan commented Jan 17, 2024

artembilan Jan 8, 2019 •

edited

Loading