Skip to content

Kotlin template#38

Open
Toru8P wants to merge 8 commits intomasterfrom
kotlin_template
Open

Kotlin template#38
Toru8P wants to merge 8 commits intomasterfrom
kotlin_template

Conversation

@Toru8P
Copy link

@Toru8P Toru8P commented Aug 13, 2021

No description provided.

Nikita Shaposhnikov added 5 commits August 11, 2021 14:30
Moved CheckMetadata and ReceiverContext from Handler
Updated common version to 3.19.0
Updated grpc-check1 version to 3.3.0
@Toru8P Toru8P requested a review from OptimumCode August 13, 2021 09:01
@Toru8P Toru8P requested a review from OptimumCode August 13, 2021 12:14
@OptimumCode
Copy link

Could you please a template for act testing? It should be a test class that allows configuring messages in MQ and checking how the "act" acts

release_version = 3.4.1
release_version=3.5.0
grpc_act_version=3.2.0
kotlin_version=1.5.20

Choose a reason for hiding this comment

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

Could you please reduce the version of kotlin to 1.3.72 or 1.4.32 because they are more trusted

import static com.exactpro.th2.common.grpc.RequestStatus.Status.SUCCESS;
import static java.util.Objects.requireNonNull;

public class CheckMetadata {

Choose a reason for hiding this comment

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

Maybe convert to Kotlin

responseObserver: StreamObserver<PlaceMessageResponse>
) {
runCatching {
LOGGER.debug { "placeOrderFIX request: $request}" }

Choose a reason for hiding this comment

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

Please use TextFormat.shotDebug... or convert to JSON format to get a one-line value.

) {
runCatching {
LOGGER.debug { "placeOrderFIX request: $request}" }
val expectedValue = requireNotNull(request.message.fieldsMap["ClOrdID"])

Choose a reason for hiding this comment

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

Please add a lazy message to clarify exception for user

Comment on lines +139 to +142
LOGGER.debug { "placeQuoteResponseFIX request: $request" }
val expectedValue = requireNotNull(request.message.fieldsMap["RFQID"])
val settings = CallSettings("placeQuoteResponseFIX", expectedValue.simpleValue, mapOf("ExecutionReport" to "RFQID", "QuoteStatusReport" to "RFQID"))
placeMessage(settings, request, responseObserver)

Choose a reason for hiding this comment

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

This part is different for calls, but other code in this method looks like a boilerplate.
Maybe create a common method and pass a lambda to extract the expected field into it.

}

private fun placeMessage(settings: CallSettings, request: PlaceMessageRequest, responseObserver: StreamObserver<PlaceMessageResponse>) {
LOGGER.debug { "Begin place ${settings.name} ${shortDebugString(request)}" }

Choose a reason for hiding this comment

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

We already logged a similar message in every gRPC methods

status(if (status) PASSED else FAILED)
bodyData(response.toTreeTable())
messageID(response.metadata.id)
}.toProtoEvent(parentEventId.id))

Choose a reason for hiding this comment

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

Suggested change
}.toProtoEvent(parentEventId.id))
}.toProto(parentEventId))

) {
responseMessage?.let { response ->
val status: Boolean = BooleanUtils.toBoolean(
response.requiredField(REQUIRED_FIELD, SIMPLE_VALUE).simpleValue

Choose a reason for hiding this comment

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

Are you sure about the REQUIRED_FIELD fixed value for all cases?

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