[th2-2469] builders for parsed/raw messages and events by andrew-drobynin · Pull Request #130 · th2-net/th2-common-j

andrew-drobynin · 2021-10-06T14:50:35Z

No description provided.

src/main/java/com/exactpro/th2/common/event/EventBuilder.java

src/main/java/com/exactpro/th2/common/schema/factory/CommonFactory.java

src/main/java/com/exactpro/th2/common/event/EventBuilder.java

src/main/kotlin/com/exactpro/th2/common/message/MessageBuilder.kt

Nikita-Smirnov-Exactpro · 2021-10-10T12:39:29Z

src/main/kotlin/com/exactpro/th2/common/message/MessageBuilder.kt

+import com.google.protobuf.Timestamp
+import java.time.Instant
+
+abstract class MessageBuilder<T> {


Please try to implement type-safe-builders
https://kotlinlang.org/docs/type-safe-builders.html#full-definition-of-the-com-example-html-package

Nikita-Smirnov-Exactpro · 2021-10-10T12:40:56Z

src/main/kotlin/com/exactpro/th2/common/message/ParsedMessageBuilder.kt

+    fun addNullField(field: String): ParsedMessageBuilder {
+        this.fields[field] = Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()
+        return this
+    }
+
+    fun addSimpleField(field: String, value: String): ParsedMessageBuilder {
+        this.fields[field] = Value.newBuilder().setSimpleValue(value).build()
+        return this
+    }


We should cover four types of values:
simple, message
list of simple, list of message

By common.proto in th2-grpc-common it can be:

message Value { oneof kind { NullValue null_value = 1; string simple_value = 2; Message message_value = 3; ListValue list_value = 4; } }

I added methods for Message and ListValue.

factory.createMessage("session alias") { metadata { sequence = 3425 direction = FIRST } body { "a" to "value", "B" to message() { body { "c" to "value" } } "D" to list [ message() { ... }, message() { ... } ], "E" to list ["", "", ""] } }

list[...] doesn't seem to be much different from built-in listOf(...), I would've prefer to use built-in collection builders where possible

Also, I think we should not forget that the builder should work and be convenient in Java as well. The version above will be hard to use in Java. Also, it can be used only if we need to build a message right here. We can't imagine how we pass this builder as a parameter to another method.

We can achieve what you described above with Kotlin extension functions. I think we need to focus on the builder's API for Java

Do we combine both approaches in one builder?
For example
java:

msg.metadata() // getter .sequence(3425) // setter .direction(FIRST) // setter msg.body() // getter .put("a", "value") .put("B", factory.createMessage() .body() .put("c", "value")) ...

OptimumCode · 2021-10-18T10:47:13Z

src/main/java/com/exactpro/th2/common/event/Event.java

+    public static Event start(IEventFactory eventFactory) {
+        return eventFactory.start();
+    }


I don't think we need this method here at all. We can simply call IEventFactory.start() instead

@andreydrobynin @Nikita-Smirnov-Exactpro what do you think about that?

@OptimumOpium as I remember @Nikita-Smirnov-Exactpro asked to save back compatibility.

It is a new method. How do we save backward compatibility here?

Sorry, I meant we want to have a new method with same name as an old. I think @Nikita-Smirnov-Exactpro can clarify it correctly than me.

From my point of view, it looks a bit strange. Why do I have to write Event.start(eventFactory) instead of eventFactory.start() in my code?

OptimumCode · 2021-10-18T10:47:34Z

src/main/java/com/exactpro/th2/common/event/Event.java

+    public static Event from(Instant startTimestamp, IEventFactory eventFactory) {
+        return eventFactory.from(startTimestamp);
+    }


The same as for start method

Here also back compatibility

src/main/java/com/exactpro/th2/common/schema/message/RabbitMqTest.java

OptimumCode · 2021-10-18T10:49:35Z

src/main/kotlin/com/exactpro/th2/common/event/EventFactory.kt

+
+    override fun from(startTimestamp: Instant, endTimestamp: Instant?) = from(startTimestamp, endTimestamp, this)
+
+    override fun from(startTimestamp: Instant, endTimestamp: Instant?, eventFactory: IEventFactory) =


Why do we need this method?

@Nikita-Smirnov-Exactpro asked to add it. As I remember he wanted to have builder interface with old implementation (com.exactpro.th2.common.event.Event)

I still don't understand why do we need a method in the factory that accepts the factory as one of the parameters. How is it connected to an old implementation?

src/main/kotlin/com/exactpro/th2/common/schema/configuration/ConfigurationManager.kt

src/test/kotlin/com/exactpro/th2/common/event/TestEvent.kt

…builder

…sage"

OptimumCode · 2021-11-09T16:17:35Z

src/main/java/com/exactpro/th2/common/event/Event.java

    protected static final ThreadLocal<ObjectMapper> OBJECT_MAPPER = ThreadLocal.withInitial(() -> new ObjectMapper().setSerializationInclusion(NON_NULL));

    protected final String id = generateUUID();
+    protected String bookName;


I think we should make it final and set in the constructor

@Nikita-Smirnov-Exactpro asked to have builder methods (e.g. bookName(), endTimestamp())

I understand why we need the endTimestamp setter. But why do we need a setter for bookName? Why shouldn't we set it once in the constructor?

OptimumCode · 2021-11-09T16:18:18Z

src/main/java/com/exactpro/th2/common/event/Event.java

+    public static Event start(IEventFactory eventFactory) {
+        return eventFactory.start();
+    }


@andreydrobynin @Nikita-Smirnov-Exactpro what do you think about that?

src/main/java/com/exactpro/th2/common/event/EventUtils.java

src/main/java/com/exactpro/th2/common/message/Test.java

src/main/java/com/exactpro/th2/common/message/MessageBuilder.java

src/main/kotlin/com/exactpro/th2/common/message/BuilderExtension.kt

* Refactoring

andrew-drobynin requested a review from Nikita-Smirnov-Exactpro October 6, 2021 14:50

andrew-drobynin force-pushed the th2-2469 branch from b1a1566 to 354d0f3 Compare October 6, 2021 15:00

[th2-2469] builders for parsed/raw messages and events

3eb34a0

andrew-drobynin force-pushed the th2-2469 branch from 354d0f3 to 3eb34a0 Compare October 6, 2021 15:29

Nikita-Smirnov-Exactpro requested changes Oct 10, 2021

View reviewed changes

andrey.drobynin added 5 commits October 11, 2021 12:40

code review: back rename EventBuilder -> Event

b01c412

code review: event/message factories

e0cbdd7

code review: event constructors with EventFactory

393f1ef

code review: subsequences as list

be9dc7a

code review: support message and list value

4edafbb

andrew-drobynin force-pushed the th2-2469 branch from 4d1be62 to 4edafbb Compare October 11, 2021 14:28

Nikita-Smirnov-Exactpro requested a review from OptimumCode October 12, 2021 10:52

andrey.drobynin and others added 15 commits October 12, 2021 21:29

code review: eventFactory as field in Event

3fc7367

code review: raw/parsed builders new constructors and lateinit fields

82ee1da

[th2-2469] Corrected logging in the ConfigurationManager

23eafc2

nullable parentEventId

439b341

protocol = "" and messageType = "" by default

80059af

added missed setter for sessionAlias

744222e

moved setters to parent builder

de82519

builders with generic to avoid duplicate code

19ff75b

list of simple and messages as vararg

696709a

builder for inner messages only with fields

6804b78

ParsedInnerMessageBuilder as field in ParsedMessageBuilder

89d06a2

ParsedInnerMessageBuilder creation in factory

f9634b9

corrected javadoc

dc557a1

added interfaces for builders

eb39015

Added test with builders

bbfc803

OptimumCode reviewed Oct 18, 2021

View reviewed changes

andrey.drobynin added 2 commits October 19, 2021 17:53

Added builders from https://gitlab.exactpro.com/oleg.smirnov/message-…

d1e71cf

…builder

Uncommented code in kotlin builder extension

2a6b5ec

andrey.drobynin added 2 commits October 20, 2021 15:50

Direction as enum in common

a2fb40d

Added all fields for parsed message, renamed "message" -> "parsed mes…

b4c4eec

…sage"

andrew-drobynin force-pushed the th2-2469 branch from a1b0faf to b4c4eec Compare October 20, 2021 13:12

andrey.drobynin added 7 commits October 20, 2021 17:41

Added all fields for raw message

43b1e13

Moved old builders to "old" package, use new MessageFactory

58fd435

Moved common code for message metadata building

6377c44

Moved common code for message building

7edd785

Removed first builders approach

50b66bb

Added tests for event

11057a9

Added bookName to MessageID and EventID

610ada4

OptimumCode reviewed Nov 9, 2021

View reviewed changes

andrey.drobynin added 3 commits November 10, 2021 11:55

code review: logger lambda

3f434f6

code review: deprecated method

20113be

code review: EventFactory without CommonFactory

d116412

andrew-drobynin mentioned this pull request Nov 17, 2021

[th2-2469/th2-2567] eventId/messageId builders and pass bookName #144

Merged

Nikita-Smirnov-Exactpro added 2 commits September 3, 2022 17:31

Merge remote-tracking branch 'origin/master' into th2-2469

f494df3

[th2-2468] Added the addMessages method into MessageBodyBuilder

7c3e6c9

* Refactoring


		override fun from(startTimestamp: Instant, endTimestamp: Instant?) = from(startTimestamp, endTimestamp, this)

		override fun from(startTimestamp: Instant, endTimestamp: Instant?, eventFactory: IEventFactory) =

Conversation

andrew-drobynin commented Oct 6, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrew-drobynin Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nikita-Smirnov-Exactpro Oct 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nikita-Smirnov-Exactpro Oct 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrew-drobynin Oct 11, 2021 •

edited

Loading

Nikita-Smirnov-Exactpro Oct 12, 2021 •

edited

Loading

Nikita-Smirnov-Exactpro Oct 13, 2021 •

edited

Loading