From 609cfbfbc1d9d7ced97b248007d9aa4d545d91bf Mon Sep 17 00:00:00 2001 From: Emily Bowe Date: Fri, 14 Jun 2024 10:36:52 +0100 Subject: [PATCH 01/19] CORE-19384: Add reset after RebalanceInProgressException (#6181) When lost membership requests were investigated it seems to be a problem that occurs during rebalance in the state and event pattern. After the rebalance the commits that were not successful don't get re-processed and are instead skipped over. As part of handling this, this adds a reset of the event offset position so we try processing again from the correct place after a RebalanceInProgressException. --- .../StateAndEventSubscriptionImpl.kt | 1 + .../StateAndEventSubscriptionImplTest.kt | 16 +++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImpl.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImpl.kt index 71a3d4bc3da..d76bcb1fe5b 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImpl.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImpl.kt @@ -243,6 +243,7 @@ internal class StateAndEventSubscriptionImpl( } catch (ex: StateAndEventConsumer.RebalanceInProgressException) { log.warn ("Abandoning processing of events(keys: ${events.joinToString { it.key.toString() }}, " + "size: ${events.size}) due to rebalance", ex) + stateAndEventConsumer.resetEventOffsetPosition() return true } diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt index f468aec20bc..fb31e7b3359 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt @@ -1,10 +1,5 @@ package net.corda.messaging.subscription -import java.time.Duration -import java.util.concurrent.CompletableFuture -import java.util.concurrent.CountDownLatch -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock import net.corda.avro.serialization.CordaAvroSerializer import net.corda.lifecycle.LifecycleCoordinatorFactory import net.corda.messagebus.api.CordaTopicPartition @@ -41,6 +36,11 @@ import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import java.time.Duration +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CountDownLatch +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock class StateAndEventSubscriptionImplTest { @@ -513,8 +513,9 @@ class StateAndEventSubscriptionImplTest { verify(chunkSerializerService, times(1)).getChunkKeysToClear(any(), anyOrNull(), anyOrNull()) } + @Suppress("MaxLineLength") @Test - fun `repartition during batch processing stops the batch, does not resume consumers and does not publish outputs`() { + fun `repartition during batch processing stops the batch, does not resume consumers or publish outputs and resets event offset position`() { val (builder, producer, stateAndEventConsumer) = setupMocks(0) val records = mutableListOf>() records.add(CordaConsumerRecord(TOPIC, 1, 1, "key1", "value1", 1)) @@ -528,7 +529,7 @@ class StateAndEventSubscriptionImplTest { else -> mutableListOf() } - }.whenever(eventConsumer).poll(any()) + }.whenever(stateAndEventConsumer).pollEvents() doThrow(StateAndEventConsumer.RebalanceInProgressException("test")) .whenever(stateAndEventConsumer).resetPollInterval() @@ -558,5 +559,6 @@ class StateAndEventSubscriptionImplTest { verify(producer, never()).sendRecordOffsetsToTransaction(any(), any()) verify(producer, never()).commitTransaction() verify(chunkSerializerService, never()).getChunkKeysToClear(any(), anyOrNull(), anyOrNull()) + verify(stateAndEventConsumer, times(1)).resetEventOffsetPosition() } } From 03ef6739b5a32463b223ec8b6db14e768f3aabab Mon Sep 17 00:00:00 2001 From: Emily Bowe Date: Wed, 17 Jul 2024 11:23:24 +0100 Subject: [PATCH 02/19] CORE-20799: Change CommitFailedException classification (#6295) Change CommitFailedException classification from fatal to transient. This is required as the kafka connection tests exposed that we mark CommitFailedException as fatal and therefore do not retry. A CommitFailedException means we should abort the transaction but by classifying it as fatal we will bubble this up and we will not retry on any level. A CommitFailedException is fatal at the transaction level but not at the worker level so this should be changed. --- .../messagebus/kafka/consumer/CordaKafkaConsumerImpl.kt | 6 +++--- .../messagebus/kafka/consumer/CordaKafkaConsumerImplTest.kt | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImpl.kt b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImpl.kt index e25ddf2de79..c1daec2d7b2 100644 --- a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImpl.kt +++ b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImpl.kt @@ -70,8 +70,7 @@ class CordaKafkaConsumerImpl( ArithmeticException::class.java, FencedInstanceIdException::class.java, InconsistentGroupProtocolException::class.java, - InvalidOffsetException::class.java, - CommitFailedException::class.java + InvalidOffsetException::class.java ) val transientExceptions: Set> = setOf( TimeoutException::class.java, @@ -79,7 +78,8 @@ class CordaKafkaConsumerImpl( InterruptException::class.java, KafkaException::class.java, ConcurrentModificationException::class.java, - RebalanceInProgressException::class.java + RebalanceInProgressException::class.java, + CommitFailedException::class.java ) } diff --git a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImplTest.kt b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImplTest.kt index 17160d98864..8e9fbeec519 100644 --- a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImplTest.kt +++ b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/consumer/CordaKafkaConsumerImplTest.kt @@ -193,7 +193,7 @@ class CordaKafkaConsumerImplTest { cordaKafkaConsumer = createConsumer(consumer) doThrow(CommitFailedException()).whenever(consumer).commitSync() - assertThatExceptionOfType(CordaMessageAPIFatalException::class.java).isThrownBy { + assertThatExceptionOfType(CordaMessageAPIIntermittentException::class.java).isThrownBy { cordaKafkaConsumer.syncCommitOffsets() } verify(consumer, times(1)).commitSync() @@ -211,13 +211,13 @@ class CordaKafkaConsumerImplTest { } @Test - fun testCommitOffsetsFatal() { + fun testCommitOffsetsThrowIntermittent() { consumer = mock() cordaKafkaConsumer = createConsumer(consumer) val consumerRecord = CordaConsumerRecord(eventTopic, 1, 5L, "", "value", 0) doThrow(CommitFailedException()).whenever(consumer).commitSync(anyMap()) - assertThatExceptionOfType(CordaMessageAPIFatalException::class.java).isThrownBy { + assertThatExceptionOfType(CordaMessageAPIIntermittentException::class.java).isThrownBy { cordaKafkaConsumer.syncCommitOffsets(consumerRecord, "meta data") } verify(consumer, times(1)).commitSync(anyMap()) From 44fd98dcb8db6b69bc498b5ad894a7a2058ba7bc Mon Sep 17 00:00:00 2001 From: Emily Bowe Date: Wed, 17 Jul 2024 13:34:31 +0100 Subject: [PATCH 03/19] change error to warn (#6294) This issue appeared while running kafka connection tests which kill the kafka broker. When the connection to the kafka broker was lost, ERROR level logs related to CryptoOpsClientImpl appeared several times. This is because there was a failure in executing net.corda.data.crypto.wire.ops.rpc.queries.SupportedSchemesRpcQuery. It has been verified how this is handled in the rest worker: KeyRestResource calls listSchemes and fails at cryptoOpsClient.getSupportedSchemes. The exception handling for listSchemes will return an InternalServerException. This extends HttpApiException and the response code will be 500. As we return an error to the rest client and the system can recover, the log level for the CryptoOpsClient failing the operation should be WARN level instead. --- .../kotlin/net/corda/crypto/client/impl/CryptoOpsClientImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/crypto/crypto-client-impl/src/main/kotlin/net/corda/crypto/client/impl/CryptoOpsClientImpl.kt b/components/crypto/crypto-client-impl/src/main/kotlin/net/corda/crypto/client/impl/CryptoOpsClientImpl.kt index 2bd7fd11c97..2fdfc969d6d 100644 --- a/components/crypto/crypto-client-impl/src/main/kotlin/net/corda/crypto/client/impl/CryptoOpsClientImpl.kt +++ b/components/crypto/crypto-client-impl/src/main/kotlin/net/corda/crypto/client/impl/CryptoOpsClientImpl.kt @@ -445,7 +445,7 @@ class CryptoOpsClientImpl( } catch (e: CordaRPCAPIResponderException) { throw e.toClientException() } catch (e: Throwable) { - logger.error("Failed executing ${request::class.java.name} for tenant ${context.tenantId}", e) + logger.warn("Failed executing ${request::class.java.name} for tenant ${context.tenantId}", e) throw e } } From 3b05a6c133dec760adf146ac295f2e01e0231576 Mon Sep 17 00:00:00 2001 From: Ben Millar <44114751+ben-millar@users.noreply.github.com> Date: Tue, 23 Jul 2024 11:28:06 +0100 Subject: [PATCH 04/19] CORE-20797 Adding handling for CordaMessageAPIProducerRequiresReset exception (#6299) This change ensures that the exception `CordaMessageAPIProducerRequiresReset` is correctly handled, by re-raising it to the context which owns the `CordaProducer` and resetting the producer safely. Unit tests have also been added to verify this behaviour. --- .../messaging/mediator/MessageBusClient.kt | 56 ++++++++++++++-- .../factory/MessageBusClientFactory.kt | 17 ++--- .../messaging/publisher/CordaRPCSenderImpl.kt | 46 +++++++++++-- .../subscription/EventLogSubscriptionImpl.kt | 2 +- .../subscription/RPCSubscriptionImpl.kt | 6 +- .../mediator/MessageBusClientTest.kt | 36 +++++++++- .../EventLogSubscriptionImplTest.kt | 24 ++----- .../subscription/RPCSubscriptionImplTest.kt | 1 + .../StateAndEventSubscriptionImplTest.kt | 67 +++++++++++++------ 9 files changed, 193 insertions(+), 62 deletions(-) diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/MessageBusClient.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/MessageBusClient.kt index 8b2c43f8818..8b84dc0e4b0 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/MessageBusClient.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/MessageBusClient.kt @@ -3,6 +3,8 @@ package net.corda.messaging.mediator import net.corda.messagebus.api.producer.CordaProducer import net.corda.messagebus.api.producer.CordaProducerRecord import net.corda.messaging.api.exception.CordaMessageAPIFatalException +import net.corda.messaging.api.exception.CordaMessageAPIIntermittentException +import net.corda.messaging.api.exception.CordaMessageAPIProducerRequiresReset import net.corda.messaging.api.mediator.MediatorMessage import net.corda.messaging.api.mediator.MessagingClient import net.corda.messaging.api.mediator.MessagingClient.Companion.MSG_PROP_ENDPOINT @@ -13,16 +15,19 @@ import java.util.concurrent.CompletableFuture class MessageBusClient( override val id: String, - private val producer: CordaProducer, + private val createProducer: () -> CordaProducer, ) : MessagingClient { private companion object { private val log: Logger = LoggerFactory.getLogger(this::class.java.enclosingClass) } + var producer: CordaProducer = createProducer() + override fun send(message: MediatorMessage<*>): MediatorMessage<*> { val future = CompletableFuture() val record = message.toCordaProducerRecord() + producer.send(record) { ex -> setFutureFromResponse(ex, future, record.topic) } @@ -38,15 +43,56 @@ class MessageBusClient( future: CompletableFuture, topic: String ) { - if (exception == null) { - future.complete(Unit) + val message = "Producer clientId $id for topic $topic failed to send." + when (exception) { + null -> future.complete(Unit) + is CordaMessageAPIProducerRequiresReset -> { + logErrorAndSetFuture("$message Resetting producer.", exception, future, false) + resetProducer() + } + else -> logErrorAndSetFuture(message, exception, future, true) + } + } + + /** + * Log the [message] and [exception] and set the [future] with the appropriate exception. + */ + private fun logErrorAndSetFuture( + message: String, + exception: Exception, + future: CompletableFuture, + fatal: Boolean + ) { + if (fatal) { + log.error(message, exception) } else { - val message = "Producer clientId $id for topic $topic failed to send." log.warn(message, exception) - future.completeExceptionally(CordaMessageAPIFatalException(message, exception)) } + + future.completeExceptionally( + if (fatal) CordaMessageAPIFatalException(message, exception) + else CordaMessageAPIIntermittentException(message, exception) + ) } + /** + * Reset the producer by closing the current producer and creating a new one. + */ + private fun resetProducer() { + try { + producer.close() + } catch (ex: Exception) { + log.warn( + "Failed to close message bus messaging client [$id] safely.", ex + ) + } + + producer = createProducer() + } + + /** + * Close the producer + */ override fun close() { try { producer.close() diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/factory/MessageBusClientFactory.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/factory/MessageBusClientFactory.kt index 01ca9c87616..14a0e4f8fed 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/factory/MessageBusClientFactory.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/factory/MessageBusClientFactory.kt @@ -37,15 +37,12 @@ class MessageBusClientFactory( throwOnSerializationError = false ) - val eventProducer = cordaProducerBuilder.createProducer( - eventProducerConfig, - messageBusConfig, - config.onSerializationError - ) - - return MessageBusClient( - id, - eventProducer, - ) + return MessageBusClient(id) { + cordaProducerBuilder.createProducer( + eventProducerConfig, + messageBusConfig, + config.onSerializationError + ) + } } } \ No newline at end of file diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/publisher/CordaRPCSenderImpl.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/publisher/CordaRPCSenderImpl.kt index a9e0b28c22a..da1ace63643 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/publisher/CordaRPCSenderImpl.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/publisher/CordaRPCSenderImpl.kt @@ -22,6 +22,7 @@ import net.corda.messagebus.api.producer.CordaProducerRecord import net.corda.messagebus.api.producer.builder.CordaProducerBuilder import net.corda.messaging.api.exception.CordaMessageAPIFatalException import net.corda.messaging.api.exception.CordaMessageAPIIntermittentException +import net.corda.messaging.api.exception.CordaMessageAPIProducerRequiresReset import net.corda.messaging.api.exception.CordaRPCAPIResponderException import net.corda.messaging.api.exception.CordaRPCAPISenderException import net.corda.messaging.api.publisher.RPCSender @@ -93,8 +94,8 @@ internal class CordaRPCSenderImpl( attempts++ try { log.debug { "Creating rpc response consumer. Attempt: $attempts" } - val producerConfig = ProducerConfig(config.clientId, config.instanceId, false, ProducerRoles.RPC_SENDER) - producer = cordaProducerBuilder.createProducer(producerConfig, config.messageBusConfig) + + resetProducer() val consumerConfig = ConsumerConfig(config.group, config.clientId, ConsumerRoles.RPC_SENDER) cordaConsumerBuilder.createConsumer( @@ -249,11 +250,48 @@ internal class CordaRPCSenderImpl( try { producer?.sendRecords(listOf(record)) } catch (ex: Exception) { - future.completeExceptionally(CordaRPCAPISenderException("Failed to publish", ex)) - log.warn("Failed to publish. Exception: ${ex.message}", ex) + val message = "Failed to send request $correlationId." + + when (ex) { + is CordaMessageAPIProducerRequiresReset -> { + logErrorAndSetFuture("$message Resetting producer.", ex, future) + resetProducer() + } + else -> { + logErrorAndSetFuture(message, ex, future) + } + } } } return future } + + /** + * Log the [message] and [exception] and set the [future] with the appropriate exception. + */ + private fun logErrorAndSetFuture( + message: String, + exception: Exception, + future: CompletableFuture, + ) { + log.warn(message, exception) + future.completeExceptionally(CordaRPCAPISenderException(message, exception)) + } + + /** + * Reset the producer by closing the current producer, if it exists, and creating a new one. + */ + private fun resetProducer() { + try { + producer?.close() + } catch (ex: Exception) { + log.warn("Failed to close producer safely.", ex) + } + + producer = cordaProducerBuilder.createProducer( + ProducerConfig(config.clientId, config.instanceId, false, ProducerRoles.RPC_SENDER), + config.messageBusConfig + ) + } } diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImpl.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImpl.kt index 51ce9fdbf28..84c1bd68e63 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImpl.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImpl.kt @@ -1,6 +1,5 @@ package net.corda.messaging.subscription -import java.util.UUID import net.corda.lifecycle.LifecycleCoordinatorFactory import net.corda.lifecycle.LifecycleCoordinatorName import net.corda.lifecycle.LifecycleStatus @@ -31,6 +30,7 @@ import net.corda.metrics.CordaMetrics import net.corda.schema.Schemas.getDLQTopic import net.corda.utilities.debug import org.slf4j.LoggerFactory +import java.util.UUID /** * Implementation of an EventLogSubscription. diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/RPCSubscriptionImpl.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/RPCSubscriptionImpl.kt index eb9b412bf34..58f61df73d7 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/RPCSubscriptionImpl.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/subscription/RPCSubscriptionImpl.kt @@ -1,8 +1,5 @@ package net.corda.messaging.subscription -import java.nio.ByteBuffer -import java.time.Instant -import java.util.concurrent.CompletableFuture import net.corda.avro.serialization.CordaAvroDeserializer import net.corda.avro.serialization.CordaAvroSerializer import net.corda.data.ExceptionEnvelope @@ -32,6 +29,9 @@ import net.corda.messaging.utils.ExceptionUtils import net.corda.metrics.CordaMetrics import net.corda.utilities.debug import org.slf4j.LoggerFactory +import java.nio.ByteBuffer +import java.time.Instant +import java.util.concurrent.CompletableFuture /** * RPC subscription implementation utilizing a message bus with producer and consumer to achieve asynchronous diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/MessageBusClientTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/MessageBusClientTest.kt index 1cf81e3a2e6..056dfb6e8ce 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/MessageBusClientTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/MessageBusClientTest.kt @@ -3,6 +3,7 @@ package net.corda.messaging.mediator import net.corda.messagebus.api.producer.CordaProducer import net.corda.messagebus.api.producer.CordaProducerRecord import net.corda.messaging.api.exception.CordaMessageAPIFatalException +import net.corda.messaging.api.exception.CordaMessageAPIProducerRequiresReset import net.corda.messaging.api.mediator.MediatorMessage import net.corda.messaging.api.mediator.MessagingClient.Companion.MSG_PROP_ENDPOINT import net.corda.v5.base.exceptions.CordaRuntimeException @@ -22,6 +23,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.util.concurrent.CompletableFuture +import java.util.concurrent.atomic.AtomicInteger class MessageBusClientTest { private companion object { @@ -45,11 +47,17 @@ class MessageBusClientTest { messageProps.toHeaders(), ) + private val timesProducerCreated = AtomicInteger(0) + private fun createMockedProducer(): CordaProducer { + timesProducerCreated.getAndIncrement() + return cordaProducer + } @BeforeEach fun setup() { cordaProducer = mock() - messageBusClient = MessageBusClient("client-id", cordaProducer) + timesProducerCreated.set(0) + messageBusClient = MessageBusClient("client-id", ::createMockedProducer) } @Suppress("UNCHECKED_CAST") @@ -106,6 +114,32 @@ class MessageBusClientTest { }?.get() } + @Suppress("UNCHECKED_CAST") + @Test + fun `producer is recreated following a CordaMessageAPIProducerRequiresReset exception`() { + doAnswer { + val callback = it.getArgument(1) + callback.onCompletion(CordaMessageAPIProducerRequiresReset("test")) + }.whenever(cordaProducer).send(eq(record), any()) + + val result = messageBusClient.send(message) as MediatorMessage> + + verify(cordaProducer).send(eq(record), any()) + assertNotNull(result.payload) + + result.payload?.isCompletedExceptionally?.let { assertTrue(it) } + + result.payload?.handle { _, exception -> + assertTrue(exception is CordaMessageAPIProducerRequiresReset) + assertEquals( + "Producer clientId client-id for topic topic failed to send. Resetting Producer.", + exception.message + ) + } + + assertEquals(2, timesProducerCreated.get()) + } + @Suppress("UNCHECKED_CAST") @Test fun `send should wrap unknown exceptions`() { diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImplTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImplTest.kt index beda7eec119..80f33cc453c 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImplTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/EventLogSubscriptionImplTest.kt @@ -15,19 +15,19 @@ import net.corda.messaging.generateMockCordaConsumerRecordList import net.corda.messaging.stubs.StubEventLogProcessor import net.corda.messaging.subscription.consumer.listener.ForwardingRebalanceListener import net.corda.messaging.subscription.consumer.listener.LoggingConsumerRebalanceListener +import net.corda.test.util.waitWhile import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import org.mockito.kotlin.argumentCaptor -import net.corda.test.util.waitWhile import java.nio.ByteBuffer import java.time.Duration import java.util.concurrent.CountDownLatch @@ -268,21 +268,9 @@ class EventLogSubscriptionImplTest { @Test fun testIntermittentExceptionDuringProcessing() { - doAnswer { - if (builderInvocationCount == 0) { - builderInvocationCount++ - mockCordaConsumer - } else { - throw CordaMessageAPIFatalException("Consumer Create Fatal Error", Exception()) - } - }.whenever(cordaConsumerBuilder).createConsumer( - any(), - any(), - any(), - any(), - any(), - any() - ) + whenever(cordaProducerBuilder.createProducer(any(), any(), any())) + .thenReturn(mockCordaProducer) + .thenThrow(CordaMessageAPIFatalException("Consumer Create Fatal Error", Exception())) pollInvocationLatch = CountDownLatch(consumerPollAndProcessRetriesCount) processor = StubEventLogProcessor( @@ -313,7 +301,7 @@ class EventLogSubscriptionImplTest { any(), anyOrNull() ) - verify(cordaProducerBuilder, times(1)).createProducer(any(), any(), anyOrNull()) + verify(cordaProducerBuilder, times(2)).createProducer(any(), any(), anyOrNull()) verify(mockCordaProducer, times(consumerPollAndProcessRetriesCount + 1)).beginTransaction() verify(mockCordaProducer, times(0)).sendRecords(any()) verify(mockCordaProducer, times(0)).sendAllOffsetsToTransaction(any()) diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/RPCSubscriptionImplTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/RPCSubscriptionImplTest.kt index 183b5022529..e30042cd405 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/RPCSubscriptionImplTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/RPCSubscriptionImplTest.kt @@ -360,6 +360,7 @@ class RPCSubscriptionImplTest { waitWhile(Duration.ofSeconds(TEST_TIMEOUT_SECONDS)) { subscription.isRunning } verify(kafkaConsumer, times(2)).subscribe(config.topic) + verify(cordaProducerBuilder, times(2)).createProducer(any(), any(), anyOrNull()) assertThat(processor.incomingRecords.size).isEqualTo(1) assertFalse(firstTime) diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt index fb31e7b3359..2d7d39697c6 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/subscription/StateAndEventSubscriptionImplTest.kt @@ -11,6 +11,7 @@ import net.corda.messaging.TOPIC_PREFIX import net.corda.messaging.api.chunking.ChunkSerializerService import net.corda.messaging.api.exception.CordaMessageAPIFatalException import net.corda.messaging.api.exception.CordaMessageAPIIntermittentException +import net.corda.messaging.api.exception.CordaMessageAPIProducerRequiresReset import net.corda.messaging.api.processor.StateAndEventProcessor import net.corda.messaging.api.processor.StateAndEventProcessor.State import net.corda.messaging.api.records.Record @@ -127,16 +128,10 @@ class StateAndEventSubscriptionImplTest { fun `state and event subscription retries after intermittent exception`() { val (builder, producer, stateAndEventConsumer) = setupMocks(5) - // Note this exception can be (and probably _is_) thrown from the processor - var exceptionThrown = false - doAnswer { - if (!exceptionThrown) { - exceptionThrown = true - throw CordaMessageAPIIntermittentException("test") - } else { - producer - } - }.whenever(builder).createProducer(any(), anyOrNull()) + whenever(builder.createProducer(any(), anyOrNull())) + // Note this exception can be (and probably _is_) thrown from the processor + .thenThrow(CordaMessageAPIIntermittentException("test")) + .thenReturn(producer) val subscription = StateAndEventSubscriptionImpl( config, @@ -170,21 +165,53 @@ class StateAndEventSubscriptionImplTest { assertFalse(lifeCycleCoordinatorMockHelper.lifecycleCoordinatorThrows) } + @Test + @Timeout(TEST_TIMEOUT_SECONDS * 100) + fun `producer is recreated following a CordaMessageAPIProducerRequiresReset exception`() { + val (builder, producer, stateAndEventConsumer) = setupMocks(5) + + whenever(builder.createProducer(any(), anyOrNull())) + .thenThrow(CordaMessageAPIProducerRequiresReset("test")) + .thenReturn(producer) + + val subscription = StateAndEventSubscriptionImpl( + config, + builder, + mock(), + cordaAvroSerializer, + lifecycleCoordinatorFactory, + chunkSerializerService + ) + + subscription.start() + waitWhile(Duration.ofSeconds(TEST_TIMEOUT_SECONDS)) { subscription.isRunning } + + verify(builder, times(1)).createStateEventConsumerAndRebalanceListener( + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + verify(producer, times(1)).close() + verify(stateAndEventConsumer, times(1)).close() + verify(builder, times(2)).createProducer(any(), anyOrNull()) + + assertFalse(lifeCycleCoordinatorMockHelper.lifecycleCoordinatorThrows) + } + @Test @Timeout(TEST_TIMEOUT_SECONDS * 100) fun `state and event subscription does not retry after fatal exception`() { val (builder, producer, stateAndEventConsumer) = setupMocks(5) - // Note this exception can be (and probably _is_) thrown from the processor - var exceptionThrown = false - doAnswer { - if (!exceptionThrown) { - exceptionThrown = true - throw CordaMessageAPIFatalException("No coming back") - } else { - CompletableFuture.completedFuture(null) - } - }.whenever(stateAndEventConsumer).waitForFunctionToFinish(any(), any(), any()) + whenever(stateAndEventConsumer.waitForFunctionToFinish(any(), any(), any())) + // Note this exception can be (and probably _is_) thrown from the processor + .thenThrow(CordaMessageAPIFatalException("test")) + .thenAnswer { CompletableFuture.completedFuture(null) } val subscription = StateAndEventSubscriptionImpl( config, From 104f19efdb4121e3013bedb733ee2b80da729076 Mon Sep 17 00:00:00 2001 From: Emily Bowe Date: Fri, 26 Jul 2024 12:22:30 +0100 Subject: [PATCH 05/19] CORE-20795: Add exception handling and retry to KafkaAdmin and VirtualNodeInfoProcessor (#6296) This issue appeared while running kafka connection tests which kill the kafka broker. In a flow worker compacted subscription we got a org.apache.kafka.common.errors.TimeoutException and handled it as fatal. This is because we have no error handling in our KafkaAdmin class for the exceptions that can be thrown by KafkaFuture.get() . The KafkaAdmin client is called from the VirtualNodeInfoProcessor which also does not catch this. This PR adds retry logic to the KafkaAdmin for getTopics and in the case of exhausting these retries, error handling in VirtualNodeInfoProcessor to handle this better and prevent this exception bubbling further. --- .../read/impl/VirtualNodeInfoProcessor.kt | 8 ++- .../read/impl/VirtualNodeInfoProcessorTest.kt | 26 +++++++++ .../messagebus/kafka/admin/KafkaAdmin.kt | 38 ++++++++++++- .../messagebus/kafka/admin/KafkaAdminTest.kt | 57 +++++++++++++++++-- 4 files changed, 122 insertions(+), 7 deletions(-) diff --git a/components/virtual-node/virtual-node-info-read-service/src/main/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessor.kt b/components/virtual-node/virtual-node-info-read-service/src/main/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessor.kt index bbf0f4fe15a..497cfdc49d3 100644 --- a/components/virtual-node/virtual-node-info-read-service/src/main/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessor.kt +++ b/components/virtual-node/virtual-node-info-read-service/src/main/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessor.kt @@ -113,7 +113,13 @@ class VirtualNodeInfoProcessor(private val onStatusUpCallback: () -> Unit, priva } val currentSnapshot = virtualNodeInfoMap.getAllAsCordaObjects() - listeners.forEach { it.value.onUpdate(setOf(newRecord.key.toCorda()), currentSnapshot) } + try { + listeners.forEach { it.value.onUpdate(setOf(newRecord.key.toCorda()), currentSnapshot) } + } catch (exception: Exception) { + log.error("Virtual Node Info service could not update", exception) + onErrorCallback() + return + } } fun getAll(): List = diff --git a/components/virtual-node/virtual-node-info-read-service/src/test/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessorTest.kt b/components/virtual-node/virtual-node-info-read-service/src/test/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessorTest.kt index c0eeda368e6..495c8a67afa 100644 --- a/components/virtual-node/virtual-node-info-read-service/src/test/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessorTest.kt +++ b/components/virtual-node/virtual-node-info-read-service/src/test/kotlin/net/corda/virtualnode/read/impl/VirtualNodeInfoProcessorTest.kt @@ -4,8 +4,10 @@ import net.corda.crypto.core.SecureHashImpl import net.corda.libs.packaging.core.CpiIdentifier import net.corda.messaging.api.records.Record import net.corda.test.util.identity.createTestHoldingIdentity +import net.corda.v5.base.exceptions.CordaRuntimeException import net.corda.virtualnode.HoldingIdentity import net.corda.virtualnode.VirtualNodeInfo +import net.corda.virtualnode.read.VirtualNodeInfoListener import net.corda.virtualnode.toAvro import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Assertions.assertEquals @@ -16,6 +18,10 @@ import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertDoesNotThrow +import org.mockito.Mockito +import org.mockito.Mockito.mock +import org.mockito.kotlin.any import java.time.Instant import java.util.UUID @@ -351,4 +357,24 @@ class VirtualNodeInfoProcessorTest { processor.onSnapshot(mapOf(holdingIdentityOther.toAvro() to virtualNodeInfo.toAvro())) assertThat(onError).isTrue } + + @Test + fun `exception is caught and onError called if update fails`() { + val exceptionalListener = mock() + + var onError = false + val processor = VirtualNodeInfoProcessor({ /* don't care */ }, { onError = true }) + + processor.registerCallback(exceptionalListener) + + Mockito.`when`(exceptionalListener.onUpdate(any(), any())).thenThrow(CordaRuntimeException("")) + + val holdingIdentity = createTestHoldingIdentity("CN=Bob, O=Bob Corp, L=LDN, C=GB", "groupId") + val virtualNodeInfo = + newVirtualNodeInfo(holdingIdentity, CpiIdentifier("name", "version", secureHash)) + + assertThat(onError).isFalse + assertDoesNotThrow { processor.onNext(Record("", holdingIdentity.toAvro(), virtualNodeInfo.toAvro()), null, emptyMap()) } + assertThat(onError).isTrue + } } diff --git a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/admin/KafkaAdmin.kt b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/admin/KafkaAdmin.kt index a067dbf36f3..d1b8d47a609 100644 --- a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/admin/KafkaAdmin.kt +++ b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/admin/KafkaAdmin.kt @@ -1,13 +1,49 @@ package net.corda.messagebus.kafka.admin import net.corda.messagebus.api.admin.Admin +import net.corda.utilities.retry.Linear +import net.corda.utilities.retry.tryWithBackoff +import net.corda.v5.base.exceptions.CordaRuntimeException import org.apache.kafka.clients.admin.AdminClient +import org.slf4j.LoggerFactory +import java.util.concurrent.ExecutionException class KafkaAdmin(private val adminClient: AdminClient) : Admin { + + companion object { + private val logger = LoggerFactory.getLogger(this::class.java.enclosingClass) + } + override fun getTopics(): Set { - return adminClient.listTopics().names().get() + return tryWithBackoff( + logger = logger, + maxRetries = 3, + maxTimeMillis = 3000, + backoffStrategy = Linear(200), + shouldRetry = { _, _, throwable -> throwable.isRecoverable() }, + onRetryAttempt = { attempt, delayMillis, throwable -> + logger.warn("Attempt $attempt failed with \"${throwable.message}\", will try again after $delayMillis milliseconds") + }, + onRetryExhaustion = { attempts, elapsedMillis, throwable -> + val errorMessage = + "Execution failed with \"${throwable.message}\" after retrying $attempts times for $elapsedMillis milliseconds." + logger.warn(errorMessage) + CordaRuntimeException(errorMessage, throwable) + }, + { + adminClient.listTopics().names().get() + } + ) } + private fun Throwable.isRecoverable(): Boolean { + return when (this) { + is ExecutionException -> true + else -> false + } + } + + override fun close() { adminClient.close() } diff --git a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/admin/KafkaAdminTest.kt b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/admin/KafkaAdminTest.kt index 2a59a414be1..82d23844c49 100644 --- a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/admin/KafkaAdminTest.kt +++ b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/admin/KafkaAdminTest.kt @@ -1,19 +1,25 @@ package net.corda.messagebus.kafka.admin +import net.corda.v5.base.exceptions.CordaRuntimeException import org.apache.kafka.clients.admin.AdminClient import org.apache.kafka.clients.admin.ListTopicsResult import org.apache.kafka.common.KafkaFuture +import org.apache.kafka.common.errors.TimeoutException import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import java.util.concurrent.ExecutionException class KafkaAdminTest { - private var adminClient = mock() - @Test - fun `When list topics then return all none internal topics from kafka`() { + fun `listTopics returns all internal topics from kafka`() { + var adminClient = mock() + val kafkaFuture = mock>>().apply { whenever(get()).thenReturn(setOf("topic1")) } @@ -23,8 +29,49 @@ class KafkaAdminTest { whenever(adminClient.listTopics()).thenReturn(result) - val target = KafkaAdmin(adminClient) + val admin = KafkaAdmin(adminClient) + + assertThat(admin.getTopics()).containsOnly("topic1") + } + + @Test + fun `getTopics will retry an exception and be successful when retries not exceeded`() { + val adminClient = mock() + val kafkaFuture = mock>>() + + val topicResult = mock() + whenever(topicResult.names()).thenReturn(kafkaFuture) + whenever(adminClient.listTopics()).thenReturn(topicResult) + + //retries hardcoded in getTopics to max 3 attempts + + whenever(kafkaFuture.get()) + .thenThrow(ExecutionException(TimeoutException("timed out"))) + .thenThrow(ExecutionException(TimeoutException("timed out"))) + .thenReturn(setOf("topic1")) + + val admin = KafkaAdmin(adminClient) + + assertThat(admin.getTopics()).containsOnly("topic1") + } + + @Test + fun `getTopics will retry an exception and rethrow when retries exceeded`() { + val adminClient = mock() + val kafkaFuture = mock>>() + + val topicResult = mock() + whenever(topicResult.names()).thenReturn(kafkaFuture) + whenever(adminClient.listTopics()).thenReturn(topicResult) + + //retries hardcoded in getTopics to max 3 attempts + + whenever(kafkaFuture.get()) + .thenThrow(ExecutionException(TimeoutException("timed out"))) + + val admin = KafkaAdmin(adminClient) - assertThat(target.getTopics()).containsOnly("topic1") + assertThrows { admin.getTopics() } + verify(kafkaFuture, times(3)).get() } } From 0b98ca06a6089247671075244abeff20be2b604d Mon Sep 17 00:00:00 2001 From: Ben Millar <44114751+ben-millar@users.noreply.github.com> Date: Mon, 29 Jul 2024 14:22:57 +0100 Subject: [PATCH 06/19] CORE-20797 Adding CordaMessageAPIProducerRequiresReset exception to ExceptionUtils (#6311) This is required so that the exception is correctly handled in the Kafka message patterns. --- .../main/kotlin/net/corda/messaging/utils/ExceptionUtils.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/utils/ExceptionUtils.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/utils/ExceptionUtils.kt index 72e2f763a19..7d5359d40ef 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/utils/ExceptionUtils.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/utils/ExceptionUtils.kt @@ -2,10 +2,12 @@ package net.corda.messaging.utils import net.corda.messaging.api.exception.CordaMessageAPIFatalException import net.corda.messaging.api.exception.CordaMessageAPIIntermittentException +import net.corda.messaging.api.exception.CordaMessageAPIProducerRequiresReset object ExceptionUtils { val CordaMessageAPIException: Set> = setOf( CordaMessageAPIFatalException::class.java, - CordaMessageAPIIntermittentException::class.java + CordaMessageAPIIntermittentException::class.java, + CordaMessageAPIProducerRequiresReset::class.java ) -} \ No newline at end of file +} From 47994ec9cea0f24c60157154504bc5ea08b5cc29 Mon Sep 17 00:00:00 2001 From: Ben Millar <44114751+ben-millar@users.noreply.github.com> Date: Mon, 29 Jul 2024 14:24:28 +0100 Subject: [PATCH 07/19] CORE-20794 Catching ProducerFencedException and rethrowing as a ProducerRequiresReset (#6303) The `ProducerFencedException` in these cases is likely due to an invalid epoch being used in an operation as a result of a timeout on the broker side. The correct action in this case is to reset the producer. --- .../kafka/producer/CordaKafkaProducerImpl.kt | 28 ++++++++++++++----- .../producer/CordaKafkaProducerImplTest.kt | 27 +++++++++--------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImpl.kt b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImpl.kt index 0d33746e35f..c104a478169 100644 --- a/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImpl.kt +++ b/libs/messaging/kafka-message-bus-impl/src/main/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImpl.kt @@ -65,7 +65,6 @@ class CordaKafkaProducerImpl( const val asyncChunkErrorMessage = "Tried to send record which requires chunking using an asynchronous producer" val fatalExceptions: Set> = setOf( - ProducerFencedException::class.java, UnsupportedVersionException::class.java, UnsupportedForMessageFormatException::class.java, AuthorizationException::class.java, @@ -386,20 +385,35 @@ class CordaKafkaProducerImpl( throw CordaMessageAPIFatalException("FatalError occurred $errorString", ex) } - IllegalStateException::class.java -> { - // It's not clear whether the producer is ok to abort and continue or not in this case, so play it safe - // and let the client know to create a new one. - throw CordaMessageAPIProducerRequiresReset("Error occurred $errorString", ex) - } - in transientExceptions -> { if (abortTransaction) { abortTransaction() } throw CordaMessageAPIIntermittentException("Error occurred $errorString", ex) } + in ApiExceptions -> { throw ex } + IllegalStateException::class.java -> { + // It's not clear whether the producer is ok to abort and continue or not in this case, so play it safe + // and let the client know to create a new one. + throw CordaMessageAPIProducerRequiresReset("Error occurred $errorString", ex) + } + + ProducerFencedException::class.java -> { + // There are two scenarios in which a ProducerFencedException can be thrown: + // + // 1. The producer is fenced because another producer with the same transactional.id has been started. + // 2. The producer is fenced due to a timeout on the broker side. + // + // There should be no way for another producer to be started with the same transactional.id, so we can + // assume that the producer is fenced because of a timeout and trigger a reset. + throw CordaMessageAPIProducerRequiresReset( + "ProducerFencedException thrown, likely due to a timeout on the broker side. " + + "Triggering a reset of the producer. $errorString", ex + ) + } + else -> { // Here we do not know what the exact cause of the exception is, but we do know Kafka has not told us we // must close down, nor has it told us we can abort and retry. In this instance the most sensible thing diff --git a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImplTest.kt b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImplTest.kt index a9408c4b365..7bd83c62e8e 100644 --- a/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImplTest.kt +++ b/libs/messaging/kafka-message-bus-impl/src/test/kotlin/net/corda/messagebus/kafka/producer/CordaKafkaProducerImplTest.kt @@ -20,6 +20,7 @@ import org.apache.kafka.clients.consumer.ConsumerGroupMetadata import org.apache.kafka.clients.producer.MockProducer import org.apache.kafka.clients.producer.ProducerRecord import org.apache.kafka.common.KafkaException +import org.apache.kafka.common.errors.AuthorizationException import org.apache.kafka.common.errors.InterruptException import org.apache.kafka.common.errors.InvalidProducerEpochException import org.apache.kafka.common.errors.ProducerFencedException @@ -160,7 +161,7 @@ class CordaKafkaProducerImplTest { @Test fun testBeginTransactionFatal() { - doThrow(ProducerFencedException("")).whenever(producer).beginTransaction() + doThrow(AuthorizationException("")).whenever(producer).beginTransaction() assertThrows { cordaKafkaProducer.beginTransaction() } verify(producer, times(1)).beginTransaction() } @@ -180,9 +181,9 @@ class CordaKafkaProducerImplTest { } @Test - fun testBeginTransactionZombieProducerThrowsFatalException() { + fun testBeginTransactionZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer).beginTransaction() - assertThrows { cordaKafkaProducer.beginTransaction() } + assertThrows { cordaKafkaProducer.beginTransaction() } verify(producer, times(1)).beginTransaction() } @@ -218,9 +219,9 @@ class CordaKafkaProducerImplTest { } @Test - fun testCommitTransactionZombieProducerThrowsFatalException() { + fun testCommitTransactionZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer).commitTransaction() - assertThrows { cordaKafkaProducer.commitTransaction() } + assertThrows { cordaKafkaProducer.commitTransaction() } verify(producer, times(1)).commitTransaction() } @@ -245,9 +246,9 @@ class CordaKafkaProducerImplTest { } @Test - fun testAbortTransactionZombieProducerThrowsFatalException() { + fun testAbortTransactionZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer).abortTransaction() - assertThrows { cordaKafkaProducer.abortTransaction() } + assertThrows { cordaKafkaProducer.abortTransaction() } verify(producer, times(1)).abortTransaction() } @@ -265,10 +266,10 @@ class CordaKafkaProducerImplTest { } @Test - fun testSendAllOffsetsToTransactionsZombieProducerThrowsFatalException() { + fun testSendAllOffsetsToTransactionsZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer) .sendOffsetsToTransaction(any(), Mockito.any(ConsumerGroupMetadata::class.java)) - assertThrows { cordaKafkaProducer.sendAllOffsetsToTransaction(cordaConsumer) } + assertThrows { cordaKafkaProducer.sendAllOffsetsToTransaction(cordaConsumer) } verify(producer, times(1)).sendOffsetsToTransaction(any(), Mockito.any(ConsumerGroupMetadata::class.java)) } @@ -304,10 +305,10 @@ class CordaKafkaProducerImplTest { } @Test - fun testSendRecordOffsetsToTransactionsZombieProducerThrowsFatalException() { + fun testSendRecordOffsetsToTransactionsZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer) .sendOffsetsToTransaction(any(), Mockito.any(ConsumerGroupMetadata::class.java)) - assertThrows { + assertThrows { cordaKafkaProducer.sendRecordOffsetsToTransaction( cordaConsumer, generateMockConsumerRecordList(3, "TOPIC1", 0).map { @@ -342,10 +343,10 @@ class CordaKafkaProducerImplTest { } @Test - fun testSendOffsetsZombieProducerThrowsFatalException() { + fun testSendOffsetsZombieProducerThrowsProducerResetException() { doThrow(ProducerFencedException("")).whenever(producer) .sendOffsetsToTransaction(any(), Mockito.any(ConsumerGroupMetadata::class.java)) - assertThrows { cordaKafkaProducer.sendAllOffsetsToTransaction(cordaConsumer) } + assertThrows { cordaKafkaProducer.sendAllOffsetsToTransaction(cordaConsumer) } verify(producer, times(1)).sendOffsetsToTransaction(any(), Mockito.any(ConsumerGroupMetadata::class.java)) } From 37233f63a22b697c7acd48531a19fb794d1a740f Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:48:12 +0000 Subject: [PATCH 08/19] CORE-20886 stop the registry from being updated when a coordinator is in status ERROR (#6365) Numerous fixes were added to the 5.2 branch to stop Lifecyle coordinators from traversing from ERROR to UP/DOWN, however 2 use cases were missed. The lifecycle registry is the source of truth about the health of a worker and is inspected directly when the /status and /health REST endpoints are called. It is essential these report correctly when called by the k8s liveness probe to allow for unhealthy workers to be restarted. 1.) when a stop a event is received, the registry is directly updated from within the Lifecyle processor with a new status. This is eagerly blocked here when the current status is ERROR. Separately, within the registry itself any status update from ERROR is also blocked as a last resort. 2.) when a close event is received, the coordinator is removed from the registry. This could mask a worker in ERROR state by removing it from the registry. This is also blocked at the registry level. 3.) Additionally, calling start() on a coordinator in ERROR state would reset it to DOWN. This is also blocked eagerly at the LifeCycleProccessor Level. --- .../service/impl/MemberOpsServiceTest.kt | 14 ------- .../lifecycle/impl/LifecycleProcessor.kt | 13 +++++-- .../impl/registry/LifecycleRegistryImpl.kt | 23 +++++++++-- .../impl/LifecycleCoordinatorImplTest.kt | 6 +-- .../lifecycle/impl/LifecycleProcessorTest.kt | 39 +++++++++++++++---- .../registry/LifecycleRegistryImplTest.kt | 28 +++++++++++++ 6 files changed, 93 insertions(+), 30 deletions(-) diff --git a/components/membership/membership-service-impl/src/test/kotlin/net/corda/membership/service/impl/MemberOpsServiceTest.kt b/components/membership/membership-service-impl/src/test/kotlin/net/corda/membership/service/impl/MemberOpsServiceTest.kt index 1399a5f8615..86cb3d02ddf 100644 --- a/components/membership/membership-service-impl/src/test/kotlin/net/corda/membership/service/impl/MemberOpsServiceTest.kt +++ b/components/membership/membership-service-impl/src/test/kotlin/net/corda/membership/service/impl/MemberOpsServiceTest.kt @@ -178,20 +178,6 @@ class MemberOpsServiceTest { } } - @Test - fun `component goes DOWN and comes back UP if subscription has error state and comes back`() { - getMemberOpsServiceTestContext().run { - testClass.start() - bringDependenciesUp() - sendConfigUpdate(configs) - - setDependencyToError(rpcSubName) - verifyIsDown() - bringDependencyUp(rpcSubName) - verifyIsUp() - } - } - private fun getMemberOpsServiceTestContext(): LifecycleTest { return LifecycleTest { addDependency(asyncRetrySubName) diff --git a/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/LifecycleProcessor.kt b/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/LifecycleProcessor.kt index 370d9a785d1..287c888d3d5 100644 --- a/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/LifecycleProcessor.kt +++ b/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/LifecycleProcessor.kt @@ -1,7 +1,5 @@ package net.corda.lifecycle.impl -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.ScheduledFuture import net.corda.lifecycle.DependentComponents import net.corda.lifecycle.ErrorEvent import net.corda.lifecycle.LifecycleCoordinator @@ -18,6 +16,8 @@ import net.corda.lifecycle.registry.LifecycleRegistryException import net.corda.utilities.debug import net.corda.utilities.trace import org.slf4j.LoggerFactory +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ScheduledFuture /** * Perform processing of lifecycle events. @@ -226,10 +226,17 @@ internal class LifecycleProcessor( /** * Perform any logic for updating the status of the coordinator. This includes informing other registered * coordinators of the status change and informing the registry. + * If a coordinator is in status ERROR then block any transition away from ERROR. + * This allows the liveness probe to determine the health of a worker correctly. */ private fun updateStatus(coordinator: LifecycleCoordinatorInternal, newStatus: LifecycleStatus, reason: String) { + if (state.status == LifecycleStatus.ERROR) { + logger.warn("Attempted to update ${coordinator.name} from ERROR to ${newStatus.name}. Transition blocked as ERROR " + + "is a terminal state.") + return + } if (state.status != newStatus) { - logger.info("Updating coordinator ${coordinator.name} from status ${state.status} to $newStatus. Reason: $reason") + logger.info("Attempting to update coordinator ${coordinator.name} from status ${state.status} to $newStatus. Reason: $reason") } state.status = newStatus state.registrations.forEach { it.updateCoordinatorStatus(coordinator, newStatus) } diff --git a/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImpl.kt b/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImpl.kt index a276aa4be77..565ecbd9b22 100644 --- a/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImpl.kt +++ b/libs/lifecycle/lifecycle-impl/src/main/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImpl.kt @@ -49,7 +49,15 @@ class LifecycleRegistryImpl : LifecycleRegistry, LifecycleRegistryCoordinatorAcc // Without `computeIfPresent`, `updateStatus` may (in theory) re-introduce just removed coordinator to `statuses` map only, // but not to `coordinators` map. val coordinatorStatus = CoordinatorStatus(name, status, reason) - statuses.computeIfPresent(name) { _, _ -> coordinatorStatus } + statuses.computeIfPresent(name) { key, currentValue -> + if (currentValue.status == LifecycleStatus.ERROR) { + logger.warn("Attempted to update ${key.componentName} from ERROR to ${status.name}. Transition blocked as ERROR " + + "is a terminal state.") + currentValue + } else { + coordinatorStatus + } + } logger.trace { "Coordinator status update: $name is now $status ($reason)" } } } @@ -80,8 +88,17 @@ class LifecycleRegistryImpl : LifecycleRegistry, LifecycleRegistryCoordinatorAcc */ override fun removeCoordinator(name: LifecycleCoordinatorName) { logger.debug { "Removing coordinator $name from registry" } - coordinators.remove(name) - statuses.remove(name) + + statuses.compute(name) { _, currentStatus -> + if (currentStatus?.status == LifecycleStatus.ERROR) { + logger.warn("Attempt was made to remove coordinator $name with status ERROR. Blocked attempt to allow for worker " + + "health endpoint to report ERROR correctly.") + return@compute currentStatus // Keep the status if it's ERROR + } else { + coordinators.remove(name) + return@compute null // Return null to remove the entry from the map + } + } } /** diff --git a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleCoordinatorImplTest.kt b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleCoordinatorImplTest.kt index 454fd942ebb..42406ae55d2 100644 --- a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleCoordinatorImplTest.kt +++ b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleCoordinatorImplTest.kt @@ -1052,7 +1052,7 @@ internal class LifecycleCoordinatorImplTest { @Test @Suppress("TooGenericExceptionThrown") - fun `when a coordinator stops with an error the status is set to error`() { + fun `when a coordinator stops with an error the status is set to error and stays in error when start is called`() { var startLatch = CountDownLatch(1) val stopLatch = CountDownLatch(1) createCoordinator { event, _ -> @@ -1074,11 +1074,11 @@ internal class LifecycleCoordinatorImplTest { assertTrue(stopLatch.await(TIMEOUT, TimeUnit.MILLISECONDS)) assertEquals(LifecycleStatus.ERROR, it.status) - // Restart and prove that the status is set to DOWN. + // Restart and prove that the status stays in ERROR. startLatch = CountDownLatch(1) it.start() assertTrue(startLatch.await(TIMEOUT, TimeUnit.MILLISECONDS)) - assertEquals(LifecycleStatus.DOWN, it.status) + assertEquals(LifecycleStatus.ERROR, it.status) } } diff --git a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleProcessorTest.kt b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleProcessorTest.kt index 148cb97f6e3..77b43702133 100644 --- a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleProcessorTest.kt +++ b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/LifecycleProcessorTest.kt @@ -1,8 +1,5 @@ package net.corda.lifecycle.impl -import java.util.concurrent.Delayed -import java.util.concurrent.ScheduledFuture -import java.util.concurrent.TimeUnit import net.corda.lifecycle.DependentComponents import net.corda.lifecycle.ErrorEvent import net.corda.lifecycle.LifecycleCoordinatorName @@ -31,6 +28,9 @@ import org.mockito.Mockito.verify import org.mockito.kotlin.any import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever +import java.util.concurrent.Delayed +import java.util.concurrent.ScheduledFuture +import java.util.concurrent.TimeUnit class LifecycleProcessorTest { @@ -402,6 +402,33 @@ class LifecycleProcessorTest { assertEquals(0, processedOtherEvents) } + @Test + fun `stop event when coordinator is in ERROR state does not update coordinator status`() { + val state = LifecycleStateManager(5) + state.isRunning = true + state.status = LifecycleStatus.ERROR + var processedStopEvents = 0 + var processedOtherEvents = 0 + val registry = mock() + val processor = LifecycleProcessor(NAME, state, registry, mock()) { event, _ -> + when (event) { + is StopEvent -> { + processedStopEvents++ + } + else -> { + processedOtherEvents++ + } + } + } + val registration1 = mock() + val registration2 = mock() + val coordinator = setupCoordinatorMock() + listOf(registration1, registration2).forEach { state.registrations.add(it) } + state.postEvent(StopEvent()) + process(processor, coordinator = coordinator) + assertEquals(LifecycleStatus.ERROR, state.status) + } + @Test fun `start event causes a request to notify about all current tracked registrations`() { val state = LifecycleStateManager(5) @@ -492,7 +519,7 @@ class LifecycleProcessorTest { } @Test - fun `starting from an errored state causes the status to be set back to down`() { + fun `starting from an errored state leaves the status in ERROR`() { val state = LifecycleStateManager(5) var processedStartEvents = 0 val registry = mock() @@ -509,9 +536,7 @@ class LifecycleProcessorTest { state.registrations.add(registration) state.postEvent(StartEvent()) process(processor, coordinator = coordinator) - assertEquals(LifecycleStatus.DOWN, state.status) - verify(registration).updateCoordinatorStatus(coordinator, LifecycleStatus.DOWN) - verify(registry).updateStatus(NAME, LifecycleStatus.DOWN, STARTED_REASON) + assertEquals(LifecycleStatus.ERROR, state.status) } @Test diff --git a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImplTest.kt b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImplTest.kt index 19063d3362c..2d7923bd985 100644 --- a/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImplTest.kt +++ b/libs/lifecycle/lifecycle-impl/src/test/kotlin/net/corda/lifecycle/impl/registry/LifecycleRegistryImplTest.kt @@ -141,4 +141,32 @@ class LifecycleRegistryImplTest { registry.getCoordinator(aliceName) } } + + @Test + fun `status update is blocked when current status is ERROR`() { + val registry = LifecycleRegistryImpl() + registry.registerCoordinator(aliceName, mock()) + registry.updateStatus(aliceName, LifecycleStatus.ERROR, "Simulated error") + + // Try updating from ERROR to UP + registry.updateStatus(aliceName, LifecycleStatus.UP, "Alice is up") + + // Verify that the status remains ERROR + val statuses = registry.componentStatus() + assertEquals(CoordinatorStatus(aliceName, LifecycleStatus.ERROR, "Simulated error"), statuses[aliceName]) + } + + @Test + fun `removing a coordinator with ERROR status is blocked`() { + val registry = LifecycleRegistryImpl() + registry.registerCoordinator(aliceName, mock()) + registry.updateStatus(aliceName, LifecycleStatus.ERROR, "Simulated error") + + // Try removing the coordinator + registry.removeCoordinator(aliceName) + + // Verify that the coordinator is still present + assertEquals(1, registry.componentStatus().size) + assertEquals(CoordinatorStatus(aliceName, LifecycleStatus.ERROR, "Simulated error"), registry.componentStatus()[aliceName]) + } } From 2434c4a0eae61aaa07466a77f2b140d5c0bc0c4c Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:31:19 +0000 Subject: [PATCH 09/19] CORE-20867 Implement retry topic to handle persistent transient RPC Client errors (#6385) The current mediator messaging pattern in Corda can encounter an retry loop when transient errors are received from other Corda workers. This retry loop blocks flow topic partitions from progressing and it has been observed that the corda cluster affected can become permanently unstable due to the effects of consumer lag. This pattern is used by the flow worker to perform synchronous HTTP calls to various workers, including verification, token, crypto, uniqueness, and persistence workers. To address this issue, a separate Kafka topic is dedicated to handling retries. This will allow the primary ingestion topics to continue processing unaffected flows, while introducing finite retry logic for flows impacted by transient errors. Additionally AVRO version is bumped to fix a vulnerability --- .../events/impl/ExternalEventManager.kt | 14 ++- .../events/impl/ExternalEventManagerImpl.kt | 7 ++ .../mediator/FlowEventMediatorFactoryImpl.kt | 71 +++++++++++- .../ExternalEventRetryRequestHandler.kt | 69 +++++++++++ .../impl/FlowGlobalPostProcessorImpl.kt | 40 ++++--- .../impl/ExternalEventManagerImplTest.kt | 39 ++++++- .../FlowEventMediatorFactoryImplTest.kt | 4 + .../ExternalEventRetryRequestHandlerTest.kt | 74 ++++++++++++ .../impl/FlowGlobalPostProcessorImplTest.kt | 19 ++- gradle.properties | 4 +- .../mediator/processor/ConsumerProcessor.kt | 2 + .../mediator/processor/EventProcessor.kt | 109 +++++++++++++----- .../kotlin/net/corda/messaging/TestUtils.kt | 4 +- .../mediator/processor/EventProcessorTest.kt | 71 ++++++++++-- .../mediator/config/EventMediatorConfig.kt | 3 + .../config/EventMediatorConfigBuilder.kt | 10 ++ .../api/mediator/config/RetryConfig.kt | 14 +++ 17 files changed, 489 insertions(+), 65 deletions(-) create mode 100644 components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandler.kt create mode 100644 components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandlerTest.kt create mode 100644 libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/RetryConfig.kt diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManager.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManager.kt index 35ae3298aed..352ea549847 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManager.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManager.kt @@ -1,6 +1,5 @@ package net.corda.flow.external.events.impl -import java.time.Instant import net.corda.data.flow.event.external.ExternalEvent import net.corda.data.flow.event.external.ExternalEventResponse import net.corda.data.flow.state.external.ExternalEventState @@ -8,6 +7,7 @@ import net.corda.flow.external.events.factory.ExternalEventFactory import net.corda.flow.external.events.factory.ExternalEventRecord import net.corda.messaging.api.records.Record import java.time.Duration +import java.time.Instant /** * [ExternalEventManager] encapsulates external event behaviour by creating and modifying [ExternalEventState]s. @@ -86,4 +86,16 @@ interface ExternalEventManager { instant: Instant, retryWindow: Duration ): Pair?> + + /** + * Get the external event to send for the transient error retry scenario. + * Returns the event as is from the state. No additional checks required. + * @param externalEventState The [ExternalEventState] to get the event from. + * @param instant The current time. Used to set timestamp. + * @return The external event request to resend + * */ + fun getRetryEvent( + externalEventState: ExternalEventState, + instant: Instant, + ): Record<*, *> } \ No newline at end of file diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImpl.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImpl.kt index 6927f481deb..15d08da0deb 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImpl.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImpl.kt @@ -174,6 +174,13 @@ class ExternalEventManagerImpl( return externalEventState to record } + override fun getRetryEvent( + externalEventState: ExternalEventState, + instant: Instant, + ): Record<*, *> { + return generateRecord(externalEventState, instant) + } + private fun checkRetry(externalEventState: ExternalEventState, instant: Instant, retryWindow: Duration) { when { (externalEventState.sendTimestamp + retryWindow) >= instant -> { diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/messaging/mediator/FlowEventMediatorFactoryImpl.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/messaging/mediator/FlowEventMediatorFactoryImpl.kt index 374f2fce4fc..0c4e8aaa676 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/messaging/mediator/FlowEventMediatorFactoryImpl.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/messaging/mediator/FlowEventMediatorFactoryImpl.kt @@ -1,8 +1,10 @@ package net.corda.flow.messaging.mediator +import com.typesafe.config.ConfigValueFactory import net.corda.avro.serialization.CordaAvroSerializationFactory import net.corda.data.crypto.wire.ops.flow.FlowOpsRequest import net.corda.data.flow.event.FlowEvent +import net.corda.data.flow.event.external.ExternalEventRetryRequest import net.corda.data.flow.event.mapper.FlowMapperEvent import net.corda.data.flow.output.FlowStatus import net.corda.data.flow.state.checkpoint.Checkpoint @@ -30,6 +32,7 @@ import net.corda.messaging.api.mediator.RoutingDestination.Companion.routeTo import net.corda.messaging.api.mediator.RoutingDestination.Type.ASYNCHRONOUS import net.corda.messaging.api.mediator.RoutingDestination.Type.SYNCHRONOUS import net.corda.messaging.api.mediator.config.EventMediatorConfigBuilder +import net.corda.messaging.api.mediator.config.RetryConfig import net.corda.messaging.api.mediator.factory.MediatorConsumerFactory import net.corda.messaging.api.mediator.factory.MediatorConsumerFactoryFactory import net.corda.messaging.api.mediator.factory.MessageRouterFactory @@ -47,12 +50,14 @@ import net.corda.schema.configuration.BootConfig.TOKEN_SELECTION_WORKER_REST_END import net.corda.schema.configuration.BootConfig.UNIQUENESS_WORKER_REST_ENDPOINT import net.corda.schema.configuration.BootConfig.VERIFICATION_WORKER_REST_ENDPOINT import net.corda.schema.configuration.BootConfig.WORKER_MEDIATOR_REPLICAS_FLOW_SESSION +import net.corda.schema.configuration.MessagingConfig.Bus.KAFKA_CONSUMER_MAX_POLL_RECORDS import net.corda.schema.configuration.MessagingConfig.Subscription.MEDIATOR_PROCESSING_MIN_POOL_RECORD_COUNT import net.corda.schema.configuration.MessagingConfig.Subscription.MEDIATOR_PROCESSING_THREAD_POOL_SIZE import org.osgi.service.component.annotations.Activate import org.osgi.service.component.annotations.Component import org.osgi.service.component.annotations.Reference import org.slf4j.LoggerFactory +import java.time.Instant import java.util.UUID @Suppress("LongParameterList") @@ -77,7 +82,9 @@ class FlowEventMediatorFactoryImpl @Activate constructor( private const val CONSUMER_GROUP = "FlowEventConsumer" private const val MESSAGE_BUS_CLIENT = "MessageBusClient" private const val RPC_CLIENT = "RpcClient" - + private const val RETRY_TOPIC_POLL_LIMIT = 5 + private const val RETRY_TOPIC = FLOW_EVENT_TOPIC + private const val TOKEN_RETRY = "TokenRetry" private val logger = LoggerFactory.getLogger(this::class.java.enclosingClass) } @@ -123,12 +130,65 @@ class FlowEventMediatorFactoryImpl @Activate constructor( .threadName("flow-event-mediator") .stateManager(stateManager) .minGroupSize(messagingConfig.getInt(MEDIATOR_PROCESSING_MIN_POOL_RECORD_COUNT)) + .retryConfig(RetryConfig(RETRY_TOPIC, ::buildRetryRequest)) .build() + + /** + * Build a request to trigger a resend of external events via the flow event pipeline. + * Request id is calculated from the previous request payload when possible to allow for some validation in the pipeline. + * This validation is an enhancement and not strictly required. + * A new Timestamp is set on each request to ensure each request is unique for replay logic handling. + * @param key the key of the input record + * @param syncRpcRequest the previous sync request which failed. + * @return list of output retry events. + */ + private fun buildRetryRequest(key: String, syncRpcRequest: MediatorMessage) : List> { + return try { + val requestId = getRequestId(syncRpcRequest) + val externalEventRetryRequest = ExternalEventRetryRequest.newBuilder() + .setRequestId(requestId) + .setTimestamp(Instant.now()) + .build() + val flowEvent = FlowEvent.newBuilder() + .setFlowId(key) + .setPayload(externalEventRetryRequest) + .build() + //ensure key is set correctly on new message destined for flow topic + val properties = syncRpcRequest.properties.toMutableMap().apply { this["key"] = key } + listOf(MediatorMessage(flowEvent, properties)) + } catch (ex: Exception) { + //In this scenario we failed to build the retry event. This will likely result in the flow hanging until the idle processor + // kicks in. This shouldn't be possible and is just a safety net. + logger.warn("Failed to generate a retry event for key $key. No retry will be triggered.", ex) + emptyList() + } + } + + /** + * Determine the external event request id where possible. + * Note, some token events have no request id as there is no response. + * For these use a hardcoded request id which will be ignored at the validation step. + * @param syncRpcRequest the previous request + * @return Request ID to set in the retry event. + */ + private fun getRequestId(syncRpcRequest: MediatorMessage): String { + return when (val entityRequest = deserializer.deserialize(syncRpcRequest.payload as ByteArray)) { + is EntityRequest -> entityRequest.flowExternalEventContext.requestId + is FlowOpsRequest -> entityRequest.flowExternalEventContext.requestId + is LedgerPersistenceRequest -> entityRequest.flowExternalEventContext.requestId + is TransactionVerificationRequest -> entityRequest.flowExternalEventContext.requestId + is UniquenessCheckRequestAvro -> entityRequest.flowExternalEventContext.requestId + is TokenPoolCacheEvent -> TOKEN_RETRY + else -> "InvalidEntityType" + } + } + private fun createMediatorConsumerFactories(messagingConfig: SmartConfig, bootConfig: SmartConfig): List { + val retryTopicMessagingConfig = getRetryTopicConfig(messagingConfig) val mediatorConsumerFactory: MutableList = mutableListOf( mediatorConsumerFactory(FLOW_START, messagingConfig), - mediatorConsumerFactory(FLOW_EVENT_TOPIC, messagingConfig) + mediatorConsumerFactory(FLOW_EVENT_TOPIC, retryTopicMessagingConfig) ) val mediatorReplicas = bootConfig.getIntOrDefault(WORKER_MEDIATOR_REPLICAS_FLOW_SESSION, 1) @@ -140,6 +200,10 @@ class FlowEventMediatorFactoryImpl @Activate constructor( return mediatorConsumerFactory } + private fun getRetryTopicConfig(messagingConfig: SmartConfig): SmartConfig { + return messagingConfig.withValue(KAFKA_CONSUMER_MAX_POLL_RECORDS, ConfigValueFactory.fromAnyRef(RETRY_TOPIC_POLL_LIMIT)) + } + private fun mediatorConsumerFactory( topic: String, messagingConfig: SmartConfig @@ -178,8 +242,9 @@ class FlowEventMediatorFactoryImpl @Activate constructor( rpcEndpoint(VERIFICATION_WORKER_REST_ENDPOINT, VERIFICATION_PATH), SYNCHRONOUS) is UniquenessCheckRequestAvro -> routeTo(rpcClient, rpcEndpoint(UNIQUENESS_WORKER_REST_ENDPOINT, UNIQUENESS_PATH), SYNCHRONOUS) + //RETRIES will appear as FlowEvents is FlowEvent -> routeTo(messageBusClient, - FLOW_EVENT_TOPIC, ASYNCHRONOUS) + RETRY_TOPIC, ASYNCHRONOUS) is String -> routeTo(messageBusClient, // Handling external messaging message.properties[MSG_PROP_TOPIC] as String, ASYNCHRONOUS) else -> { diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandler.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandler.kt new file mode 100644 index 00000000000..b1d6cf8c8c5 --- /dev/null +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandler.kt @@ -0,0 +1,69 @@ +package net.corda.flow.pipeline.handlers.events + +import net.corda.data.flow.event.external.ExternalEventResponse +import net.corda.data.flow.event.external.ExternalEventRetryRequest +import net.corda.flow.pipeline.events.FlowEventContext +import net.corda.flow.pipeline.exceptions.FlowEventException +import net.corda.utilities.debug +import org.osgi.service.component.annotations.Component +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +/** + * Handles pre-processing of events that are intended to trigger a resend of an external event. + * This can be triggered by the mediator in the event of transient errors. + */ +@Component(service = [FlowEventHandler::class]) +class ExternalEventRetryRequestHandler : FlowEventHandler { + + private companion object { + val log: Logger = LoggerFactory.getLogger(this::class.java.enclosingClass) + private const val TOKEN_RETRY = "TokenRetry" + } + + override val type = ExternalEventRetryRequest::class.java + + override fun preProcess(context: FlowEventContext): FlowEventContext { + val checkpoint = context.checkpoint + val externalEventRetryRequest = context.inputEventPayload + + if (!checkpoint.doesExist) { + log.debug { + "Received a ${ExternalEventRetryRequest::class.simpleName} for flow [${context.inputEvent.flowId}] that " + + "does not exist. The event will be discarded. ${ExternalEventRetryRequest::class.simpleName}: " + + externalEventRetryRequest + } + throw FlowEventException( + "ExternalEventRetryRequestHandler received a ${ExternalEventRetryRequest::class.simpleName} for flow" + + " [${context.inputEvent.flowId}] that does not exist" + ) + } + + val externalEventState = checkpoint.externalEventState + val retryRequestId: String = externalEventRetryRequest.requestId + val externalEventStateRequestId = externalEventState?.requestId + if (externalEventState == null) { + log.debug { + "Received an ${ExternalEventRetryRequest::class.simpleName} with request id: " + + "$retryRequestId while flow [${context.inputEvent.flowId} is not waiting " + + "for an ${ExternalEventResponse::class.simpleName}. " + + "${ExternalEventRetryRequest::class.simpleName}: $externalEventRetryRequest" + } + throw FlowEventException( + "ExternalEventRetryRequestHandler received an ${ExternalEventRetryRequest::class.simpleName} with request id: " + + "$retryRequestId while flow [${context.inputEvent.flowId} is not waiting " + + "for an ${ExternalEventResponse::class.simpleName}" + ) + } + //Discard events not related. Some token requests do not contain the external event id so this validation will allow all token + // requests to be resent. e.g TokenForceClaimRelease + else if (externalEventStateRequestId != retryRequestId && retryRequestId != TOKEN_RETRY) { + throw FlowEventException( + "Discarding retry request received with requestId $retryRequestId. This is likely a stale record polled. Checkpoint " + + "is currently waiting to receive a response for requestId $externalEventStateRequestId" + ) + } + + return context + } +} diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImpl.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImpl.kt index 8b8801f3615..e9fa7b251a6 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImpl.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImpl.kt @@ -1,7 +1,9 @@ package net.corda.flow.pipeline.impl +import net.corda.data.flow.event.external.ExternalEventRetryRequest import net.corda.data.flow.event.mapper.FlowMapperEvent import net.corda.data.flow.event.mapper.ScheduleCleanup +import net.corda.data.flow.state.external.ExternalEventState import net.corda.data.flow.state.session.SessionState import net.corda.data.flow.state.session.SessionStateType import net.corda.flow.external.events.impl.ExternalEventManager @@ -51,11 +53,11 @@ class FlowGlobalPostProcessorImpl @Activate constructor( postProcessPendingPlatformError(context) val outputRecords = getSessionEvents(context, now) + - getFlowMapperSessionCleanupEvents(context, now) + - getExternalEvent(context, now) + getFlowMapperSessionCleanupEvents(context, now) + + getExternalEvent(context, now) context.flowMetrics.flowEventCompleted(context.inputEvent.payload::class.java.name) - val metadata = getStateMetadata(context) + val metadata = updateFlowSessionMetadata(context) return context.copy( outputRecords = context.outputRecords + outputRecords, @@ -111,7 +113,7 @@ class FlowGlobalPostProcessorImpl @Activate constructor( if (!counterpartyExists) { val msg = "[${context.checkpoint.holdingIdentity.x500Name}] has failed to create a flow with counterparty: " + - "[${counterparty}] as the recipient doesn't exist in the network." + "[${counterparty}] as the recipient doesn't exist in the network." sessionManager.errorSession(sessionState) if (doesCheckpointExist) { log.debug { "$msg. Throwing FlowFatalException" } @@ -157,24 +159,26 @@ class FlowGlobalPostProcessorImpl @Activate constructor( * Check to see if any external events needs to be sent or resent. */ private fun getExternalEvent(context: FlowEventContext, now: Instant): List> { - val externalEventState = context.checkpoint.externalEventState - return if (externalEventState == null) { - listOf() - } else { - val retryWindow = context.flowConfig.getLong(EXTERNAL_EVENT_MESSAGE_RESEND_WINDOW) - externalEventManager.getEventToSend(externalEventState, now, Duration.ofMillis(retryWindow)) - .let { (updatedExternalEventState, record) -> - context.checkpoint.externalEventState = updatedExternalEventState - if (record != null) { - listOf(record) - } else { - listOf() - } + val externalEventState = context.checkpoint.externalEventState ?: return emptyList() + + return when (context.inputEvent.payload) { + is ExternalEventRetryRequest -> getTransientRetryRequest(externalEventState, now) + else -> { + val retryWindow = Duration.ofMillis(context.flowConfig.getLong(EXTERNAL_EVENT_MESSAGE_RESEND_WINDOW)) + externalEventManager.getEventToSend(externalEventState, now, retryWindow).let { (updatedState, record) -> + context.checkpoint.externalEventState = updatedState + listOfNotNull(record) } + } } } - private fun getStateMetadata(context: FlowEventContext): Metadata? { + private fun getTransientRetryRequest(externalEventState: ExternalEventState, now: Instant): + List> { + return listOf(externalEventManager.getRetryEvent(externalEventState, now)) + } + + private fun updateFlowSessionMetadata(context: FlowEventContext): Metadata? { val checkpoint = context.checkpoint // Find the earliest expiry time for any open sessions. val lastReceivedMessageTime = checkpoint.sessions.filter { diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImplTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImplTest.kt index 87bff35c100..4038583529f 100644 --- a/components/flow/flow-service/src/test/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImplTest.kt +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/external/events/impl/ExternalEventManagerImplTest.kt @@ -1,9 +1,5 @@ package net.corda.flow.external.events.impl -import java.nio.ByteBuffer -import java.time.Instant -import java.time.temporal.ChronoUnit -import java.util.stream.Stream import net.corda.avro.serialization.CordaAvroDeserializer import net.corda.avro.serialization.CordaAvroSerializer import net.corda.data.ExceptionEnvelope @@ -34,7 +30,11 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever +import java.nio.ByteBuffer import java.time.Duration +import java.time.Instant +import java.time.temporal.ChronoUnit +import java.util.stream.Stream class ExternalEventManagerImplTest { @@ -530,4 +530,35 @@ class ExternalEventManagerImplTest { assertEquals(null, record) } + + @Test + fun `getRetryEvent returns an external event`() { + val now = Instant.now().truncatedTo(ChronoUnit.MILLIS) + val key = ByteBuffer.wrap(KEY.toByteArray()) + val payload = ByteBuffer.wrap(byteArrayOf(1, 2, 3)) + + val externalEvent = ExternalEvent().apply { + this.topic = TOPIC + this.key = key + this.payload = payload + this.timestamp = now.minusSeconds(10) + } + + val externalEventState = ExternalEventState().apply { + requestId = REQUEST_ID_1 + eventToSend = externalEvent + sendTimestamp = null + status = ExternalEventStateStatus(ExternalEventStateType.OK, null) + } + + val record = externalEventManager.getRetryEvent( + externalEventState, + now, + ) + + + assertEquals(TOPIC, record.topic) + assertEquals(key.array(), record.key) + assertEquals(payload.array(), record.value) + } } \ No newline at end of file diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/FlowEventMediatorFactoryImplTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/FlowEventMediatorFactoryImplTest.kt index 0eb33f47c40..df31c6b8e05 100644 --- a/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/FlowEventMediatorFactoryImplTest.kt +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/FlowEventMediatorFactoryImplTest.kt @@ -30,6 +30,7 @@ import net.corda.messaging.api.mediator.factory.MediatorConsumerFactoryFactory import net.corda.messaging.api.mediator.factory.MessagingClientFactoryFactory import net.corda.messaging.api.mediator.factory.MessagingClientFinder import net.corda.messaging.api.mediator.factory.MultiSourceEventMediatorFactory +import net.corda.schema.Schemas.Flow.FLOW_EVENT_TOPIC import net.corda.schema.Schemas.Flow.FLOW_MAPPER_SESSION_OUT import net.corda.schema.Schemas.Flow.FLOW_STATUS_TOPIC import net.corda.schema.configuration.ConfigKeys @@ -115,6 +116,9 @@ class FlowEventMediatorFactoryImplTest { assertThat(router.getDestination(MediatorMessage(UniquenessCheckRequestAvro())).endpoint).isEqualTo( endpoint(UNIQUENESS_PATH) ) + assertThat(router.getDestination(MediatorMessage(FlowEvent())).endpoint).isEqualTo( + FLOW_EVENT_TOPIC + ) // External messaging val externalMessagingKafkaTopic = "custom.kafka.topic" diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandlerTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandlerTest.kt new file mode 100644 index 00000000000..0c9a18bdce9 --- /dev/null +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/events/ExternalEventRetryRequestHandlerTest.kt @@ -0,0 +1,74 @@ +package net.corda.flow.pipeline.handlers.events + +import net.corda.data.flow.event.external.ExternalEventRetryRequest +import net.corda.data.flow.state.external.ExternalEventState +import net.corda.flow.pipeline.exceptions.FlowEventException +import net.corda.flow.state.FlowCheckpoint +import net.corda.flow.test.utils.buildFlowEventContext +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.time.Instant + +class ExternalEventRetryRequestHandlerTest { + + private val externalEventRetryRequest = ExternalEventRetryRequest("requestId", Instant.now()) + + private val checkpoint = mock() + private val externalEventRetryRequestHandler = ExternalEventRetryRequestHandler() + + @Test + fun `does not throw a flow event exception if the checkpoint exists and it is the correct request id`() { + whenever(checkpoint.doesExist).thenReturn(true) + whenever(checkpoint.externalEventState).thenReturn(ExternalEventState().apply { requestId = "requestId" }) + val context = buildFlowEventContext(checkpoint, externalEventRetryRequest) + + externalEventRetryRequestHandler.preProcess(context) + } + + @Test + fun `does not throw a flow event exception if the checkpoint exists and it a token request id`() { + whenever(checkpoint.doesExist).thenReturn(true) + whenever(checkpoint.externalEventState).thenReturn(ExternalEventState().apply { requestId = "requestId" }) + + val context = buildFlowEventContext(checkpoint, ExternalEventRetryRequest("TokenRetry", Instant.now())) + externalEventRetryRequestHandler.preProcess(context) + } + + @Test + fun `throws a flow event exception if the checkpoint does not exist`() { + whenever(checkpoint.doesExist).thenReturn(false) + + val context = buildFlowEventContext(checkpoint, externalEventRetryRequest) + + assertThrows { + externalEventRetryRequestHandler.preProcess(context) + } + } + + @Test + fun `throws a flow event exception if the flow is not waiting for an external event response`() { + whenever(checkpoint.doesExist).thenReturn(true) + whenever(checkpoint.externalEventState).thenReturn(null) + + val context = buildFlowEventContext(checkpoint, externalEventRetryRequest) + + assertThrows { + externalEventRetryRequestHandler.preProcess(context) + } + } + + @Test + fun `throws a flow event exception if the flow is waiting for a different external event response`() { + whenever(checkpoint.doesExist).thenReturn(true) + whenever(checkpoint.externalEventState).thenReturn(ExternalEventState().apply { requestId = "OtherRequestId" }) + + val context = buildFlowEventContext(checkpoint, externalEventRetryRequest) + + assertThrows { + externalEventRetryRequestHandler.preProcess(context) + } + } + +} \ No newline at end of file diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImplTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImplTest.kt index 38b41ed43a3..dd1daf6280f 100644 --- a/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImplTest.kt +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/impl/FlowGlobalPostProcessorImplTest.kt @@ -2,6 +2,7 @@ package net.corda.flow.pipeline.impl import net.corda.data.flow.FlowKey import net.corda.data.flow.event.SessionEvent +import net.corda.data.flow.event.external.ExternalEventRetryRequest import net.corda.data.flow.event.mapper.FlowMapperEvent import net.corda.data.flow.event.mapper.ScheduleCleanup import net.corda.data.flow.event.session.SessionData @@ -15,6 +16,7 @@ import net.corda.flow.BOB_X500_NAME import net.corda.flow.FLOW_ID_1 import net.corda.flow.REQUEST_ID_1 import net.corda.flow.external.events.impl.ExternalEventManager +import net.corda.flow.pipeline.events.FlowEventContext import net.corda.flow.pipeline.exceptions.FlowFatalException import net.corda.flow.pipeline.factory.FlowRecordFactory import net.corda.flow.state.FlowCheckpoint @@ -103,7 +105,7 @@ class FlowGlobalPostProcessorImplTest { private val membershipGroupReaderProvider = mock() private val membershipGroupReader = mock() private val checkpoint = mock() - private val testContext = buildFlowEventContext(checkpoint, Any()) + private lateinit var testContext: FlowEventContext private val flowGlobalPostProcessor = FlowGlobalPostProcessorImpl( externalEventManager, sessionManager, @@ -114,6 +116,7 @@ class FlowGlobalPostProcessorImplTest { @Suppress("Unused") @BeforeEach fun setup() { + testContext = buildFlowEventContext(checkpoint, Any()) whenever(checkpoint.sessions).thenReturn(listOf(sessionState1, sessionState2)) whenever(checkpoint.flowKey).thenReturn(FlowKey(FLOW_ID_1, ALICE_X500_HOLDING_IDENTITY)) whenever(checkpoint.holdingIdentity).thenReturn(ALICE_X500_HOLDING_IDENTITY.toCorda()) @@ -262,6 +265,20 @@ class FlowGlobalPostProcessorImplTest { verify(checkpoint).clearPendingPlatformError() } + @Test + fun `Adds external event record when there is a retry instruction`() { + val externalEventState = ExternalEventState() + + testContext = buildFlowEventContext(checkpoint, ExternalEventRetryRequest(REQUEST_ID_1, Instant.now())) + whenever(checkpoint.externalEventState).thenReturn(externalEventState) + whenever(externalEventManager.getRetryEvent(eq(externalEventState), any())) + .thenReturn(externalEventRecord) + + val outputContext = flowGlobalPostProcessor.postProcess(testContext) + + assertThat(outputContext.outputRecords).contains(externalEventRecord) + } + @Test fun `Adds external event record when there is an external event to send`() { val externalEventState = ExternalEventState() diff --git a/gradle.properties b/gradle.properties index 6afaca45353..6c6287a28ae 100644 --- a/gradle.properties +++ b/gradle.properties @@ -33,13 +33,13 @@ activationVersion=1.2.0 ariesDynamicFrameworkExtensionVersion=1.3.6 antlrVersion=2.7.7 asmVersion=9.5 -avroVersion=1.11.3 +avroVersion=1.11.4 commonsVersion = 1.7 commonsLangVersion = 3.12.0 commonsTextVersion = 1.10.0 # Corda API libs revision (change in 4th digit indicates a breaking change) # Change to 5.2.1.xx-SNAPSHOT to pick up maven local published copy -cordaApiVersion=5.2.1.53-beta+ +cordaApiVersion=5.2.1.54-beta+ disruptorVersion=3.4.4 felixConfigAdminVersion=1.9.26 diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/ConsumerProcessor.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/ConsumerProcessor.kt index 446f5bcbab0..a0e3f7674f8 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/ConsumerProcessor.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/ConsumerProcessor.kt @@ -118,6 +118,8 @@ class ConsumerProcessor( metrics.processorTimer.recordCallable { try { val inputs = getInputs(consumer) + // If no records polled return early. + if (inputs.isEmpty()) return@recordCallable val outputs = processInputs(inputs) categorizeOutputs(outputs, failureCounts) commit(consumer, outputs, failureCounts) diff --git a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/EventProcessor.kt b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/EventProcessor.kt index 7463fcf6136..24a67a269c7 100644 --- a/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/EventProcessor.kt +++ b/libs/messaging/messaging-impl/src/main/kotlin/net/corda/messaging/mediator/processor/EventProcessor.kt @@ -15,6 +15,7 @@ import net.corda.messaging.api.records.Record import net.corda.messaging.mediator.StateManagerHelper import net.corda.messaging.mediator.metrics.EventMediatorMetrics import net.corda.tracing.addTraceContextToRecord +import org.slf4j.LoggerFactory /** * Class to process records received from the consumer. @@ -31,6 +32,9 @@ class EventProcessor( ) { private val metrics = EventMediatorMetrics(config.name) + private val retryConfig = config.retryConfig + private val logger = LoggerFactory.getLogger("${this.javaClass.name}-${config.name}") + /** * Process a group of events. @@ -74,13 +78,15 @@ class EventProcessor( var processorState = inputProcessorState val asyncOutputs = mutableMapOf, MutableList>>() val stateChangeAndOperation = try { + var isNoopState = false input.records.forEach { consumerInputEvent -> - val (updatedProcessorState, newAsyncOutputs) = processConsumerInput(consumerInputEvent, processorState, key) + val (updatedProcessorState, newAsyncOutputs, isNoop) = processConsumerInput(consumerInputEvent, processorState, key) processorState = updatedProcessorState asyncOutputs.addOutputs(consumerInputEvent, newAsyncOutputs) + if (isNoop) isNoopState = true } val state = stateManagerHelper.createOrUpdateState(key.toString(), inputState, processorState) - stateChangeAndOperation(inputState, state) + if (isNoopState) StateChangeAndOperation.Noop else stateChangeAndOperation(inputState, state) } catch (e: EventProcessorSyncEventsIntermittentException) { asyncOutputs.clear() StateChangeAndOperation.Transient @@ -127,10 +133,11 @@ class EventProcessor( consumerInputEvent: Record, processorState: StateAndEventProcessor.State?, key: K, - ): Pair?, List>> { + ): ConsumerInputOutput { var processorStateUpdated = processorState val newAsyncOutputs = mutableListOf>() val consumerInputHash = mediatorInputService.getHash(consumerInputEvent) + val isRetryTopic = consumerInputEvent.topic == config.retryConfig?.retryTopic val queue = ArrayDeque(listOf(consumerInputEvent)) while (queue.isNotEmpty()) { val event = getNextEvent(queue, consumerInputHash) @@ -141,14 +148,19 @@ class EventProcessor( } newAsyncOutputs.addAll(asyncEvents) try { - queue.addAll(processSyncEvents(key, syncEvents)) - } catch (e: CordaMessageAPIIntermittentException) { - throw EventProcessorSyncEventsIntermittentException(processorStateUpdated, e) + val (syncResponses, isNoopRetry, asyncOutputs) = processSyncEvents(key, syncEvents, isRetryTopic) + newAsyncOutputs.addAll(asyncOutputs) + queue.addAll(syncResponses) + if (isNoopRetry) { + // return early if no state update is needed + return ConsumerInputOutput(processorStateUpdated, newAsyncOutputs, true) + } } catch (e: Exception) { + logger.warn("Failed process synchronous events for key $key", e) throw EventProcessorSyncEventsFatalException(processorStateUpdated, e) } } - return Pair(processorStateUpdated, newAsyncOutputs) + return ConsumerInputOutput(processorStateUpdated, newAsyncOutputs) } private fun getNextEvent( @@ -177,32 +189,52 @@ class EventProcessor( /** * Send any synchronous events immediately and feed results back onto the queue. - */ + * If a sync request returns from the RPC client with a transient error and retry is enabled + * then push a retry event onto the retry topic. + * If retrying again via the retry topic and transient errors occur, resend retry event and do not update state + **/ private fun processSyncEvents( key: K, - syncEvents: List> - ): List> { - return syncEvents.mapNotNull { message -> + syncEvents: List>, + isRetryTopic: Boolean + ): SyncProcessingOutput { + val outputEvents = syncEvents.mapNotNull { message -> val destination = messageRouter.getDestination(message) - @Suppress("UNCHECKED_CAST") - val reply = with(destination) { - message.addProperty(MessagingClient.MSG_PROP_ENDPOINT, endpoint) - client.send(message) as MediatorMessage? - } - reply?.let { - addTraceContextToRecord( - Record( - "", - key, - reply.payload, - 0, - listOf(Pair(SYNC_RESPONSE_HEADER, "true")) - ), - message.properties - ) + try { + @Suppress("UNCHECKED_CAST") + val reply = with(destination) { + message.addProperty(MessagingClient.MSG_PROP_ENDPOINT, endpoint) + client.send(message) as MediatorMessage? + } + reply?.let { + addTraceContextToRecord( + Record( + "", + key, + reply.payload, + 0, + listOf(Pair(SYNC_RESPONSE_HEADER, "true")) + ), + message.properties + ) + } + } catch (e: CordaMessageAPIIntermittentException) { + val asyncOutputEvents: MutableList> = mutableListOf() + if (retryConfig != null) { + retryConfig.buildRetryRequest?.let { it(key, message) }?.let { + asyncOutputEvents.addAll(it) + } + } + + // If we're on the retry topic and run into another transient error, exit early and do not update the state to save + // performance. + // If we are not on the retry topic then we need to save the state before adding the retry event. This will allow the + // flow cleanup processors to execute on an idle flow checkpoint + return SyncProcessingOutput(emptyList(), isRetryTopic, asyncOutputEvents) } } + return SyncProcessingOutput(outputEvents) } private fun convertToMessage(record: Record<*, *>): MediatorMessage { @@ -219,4 +251,27 @@ class EventProcessor( private fun List>.toMessageProperties() = associateTo(mutableMapOf()) { (key, value) -> key to (value as Any) } + + + /** + * The outputs from processing a single consumer input event from the bus. + * This will includes updates from all sync events which are processed as a result of a consumer input + * @property updatedState The state and event processor output state after processing + */ + data class ConsumerInputOutput( + val updatedState: StateAndEventProcessor.State?, + val outputEvents: List>, + val isNoop: Boolean = false + ) + + /** + * The outputs from processing a single consumer input event from the bus. + * This will includes updates from all sync events which are processed as a result of a consumer input + * @property updatedState The state and event processor output state after processing + */ + data class SyncProcessingOutput( + val syncResponses: List>, + val isNoopRetry: Boolean = false, + val asyncOutputs: List> = emptyList() + ) } \ No newline at end of file diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/TestUtils.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/TestUtils.kt index 928ebeea7b2..7180fa6f25e 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/TestUtils.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/TestUtils.kt @@ -50,10 +50,10 @@ fun generateMockCordaConsumerRecordList(numberOfRecords: Long, topic: String, pa /** * Generate [recordCount] string key/value records */ -fun getStringRecords(recordCount: Int, key: String): List> { +fun getStringRecords(recordCount: Int, key: String, topic: String = "topic"): List> { val records = mutableListOf>() for (j in 1..recordCount) { - records.add(Record("topic", key, j.toString())) + records.add(Record(topic, key, j.toString())) } return records diff --git a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/processor/EventProcessorTest.kt b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/processor/EventProcessorTest.kt index 0969349bd3f..bd33f7cf4d8 100644 --- a/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/processor/EventProcessorTest.kt +++ b/libs/messaging/messaging-impl/src/test/kotlin/net/corda/messaging/mediator/processor/EventProcessorTest.kt @@ -1,6 +1,7 @@ package net.corda.messaging.mediator.processor import net.corda.libs.configuration.SmartConfigImpl +import net.corda.libs.statemanager.api.Metadata import net.corda.libs.statemanager.api.State import net.corda.messaging.api.exception.CordaMessageAPIFatalException import net.corda.messaging.api.exception.CordaMessageAPIIntermittentException @@ -10,6 +11,7 @@ import net.corda.messaging.api.mediator.MessageRouter import net.corda.messaging.api.mediator.MessagingClient import net.corda.messaging.api.mediator.RoutingDestination import net.corda.messaging.api.mediator.config.EventMediatorConfig +import net.corda.messaging.api.mediator.config.RetryConfig import net.corda.messaging.api.mediator.factory.MessageRouterFactory import net.corda.messaging.api.processor.StateAndEventProcessor import net.corda.messaging.api.processor.StateAndEventProcessor.Response @@ -34,6 +36,7 @@ import java.util.UUID @Execution(ExecutionMode.SAME_THREAD) class EventProcessorTest { private lateinit var eventMediatorConfig: EventMediatorConfig + private lateinit var eventMediatorRetryConfig: EventMediatorConfig private lateinit var stateManagerHelper: StateManagerHelper private lateinit var client: MessagingClient private lateinit var messageRouter: MessageRouter @@ -42,6 +45,7 @@ class EventProcessorTest { private lateinit var eventProcessor: EventProcessor private val inputState1: State = mock() + private val retryTopic: String = "flow.event" private val asyncMessage: String = "ASYNC_PAYLOAD" private val syncMessage: String = "SYNC_PAYLOAD" private val updatedProcessingState = StateAndEventProcessor.State("bar", null) @@ -65,6 +69,8 @@ class EventProcessorTest { } else RoutingDestination(client, "endpoint", RoutingDestination.Type.ASYNCHRONOUS) } eventMediatorConfig = buildTestConfig() + val retryConfig = RetryConfig(retryTopic, buildRetryRequest) + eventMediatorRetryConfig = buildTestConfig(retryConfig) whenever(stateAndEventProcessor.onNext(anyOrNull(), any())).thenAnswer { Response( @@ -181,9 +187,11 @@ class EventProcessorTest { } @Test - fun `when sync processing fails with a transient error, a transient state change signal is sent`() { - val mockedState = mock() + fun `when sync processing fails with a transient error, retry is OFF, a CREATE state change signal is set and no retry event is sent` + () { val input = mapOf("key" to EventProcessingInput("key", getStringRecords(1, "key"), null)) + val mockedState = mock() + whenever(stateManagerHelper.createOrUpdateState(any(), anyOrNull(), anyOrNull())).thenReturn(mockedState) whenever(client.send(any())).thenThrow(CordaMessageAPIIntermittentException("baz")) whenever(stateAndEventProcessor.onNext(anyOrNull(), any())).thenAnswer { @@ -194,17 +202,61 @@ class EventProcessorTest { ) ) } - whenever(stateManagerHelper.failStateProcessing(any(), eq(null), any())).thenReturn(mockedState) - val outputMap = eventProcessor.processEvents(input) val output = outputMap["key"] assertEquals(emptyList>(), output?.asyncOutputs) + assertThat(output?.stateChangeAndOperation?.outputState).isEqualTo(mockedState) + assertThat(output?.stateChangeAndOperation).isInstanceOf(StateChangeAndOperation.Create::class.java) + } + + @Test + fun `when transient error while processing retry topic, retry is ON, a NOOP state change signal is set and a retry event is sent`() { + val input = mapOf("key" to EventProcessingInput("key", getStringRecords(1, "key", retryTopic), null)) + val mockedState = mock() + whenever(stateManagerHelper.createOrUpdateState(any(), anyOrNull(), anyOrNull())).thenReturn(mockedState) + whenever(client.send(any())).thenThrow(CordaMessageAPIIntermittentException("baz")) + whenever(stateAndEventProcessor.onNext(anyOrNull(), any())).thenAnswer { + Response( + null, + listOf( + Record("", "key", syncMessage) + ) + ) + } + eventProcessor = EventProcessor(eventMediatorRetryConfig, stateManagerHelper, messageRouter, mediatorInputService) + val outputMap = eventProcessor.processEvents(input) + + val output = outputMap["key"] + assertEquals(1, output?.asyncOutputs?.size) assertThat(output?.stateChangeAndOperation?.outputState).isEqualTo(null) - assertThat(output?.stateChangeAndOperation).isInstanceOf(StateChangeAndOperation.Transient::class.java) + assertThat(output?.stateChangeAndOperation).isInstanceOf(StateChangeAndOperation.Noop::class.java) + } + + @Test + fun `when transient error while processing event topic, retry is ON, a NOOP state change signal is set and a retry event is sent`() { + val input = mapOf("key" to EventProcessingInput("key", getStringRecords(1, "key", "flow.start"), null)) + val mockedState = mock() + whenever(stateManagerHelper.createOrUpdateState(any(), anyOrNull(), any())).thenReturn(mockedState) + whenever(client.send(any())).thenThrow(CordaMessageAPIIntermittentException("baz")) + whenever(stateAndEventProcessor.onNext(anyOrNull(), any())).thenAnswer { + Response( + StateAndEventProcessor.State("", Metadata(mapOf())), + listOf( + Record("", "key", syncMessage) + ) + ) + } + eventProcessor = EventProcessor(eventMediatorRetryConfig, stateManagerHelper, messageRouter, mediatorInputService) + val outputMap = eventProcessor.processEvents(input) + + val output = outputMap["key"] + assertEquals(1, output?.asyncOutputs?.size) + assertThat(output?.stateChangeAndOperation?.outputState).isEqualTo(mockedState) + assertThat(output?.stateChangeAndOperation).isInstanceOf(StateChangeAndOperation.Create::class.java) } - private fun buildTestConfig() = EventMediatorConfig( + private fun buildTestConfig(retryConfig: RetryConfig? = null) = EventMediatorConfig( "", SmartConfigImpl.empty(), emptyList(), @@ -214,6 +266,11 @@ class EventProcessorTest { 1, "", mock(), - 20 + 20, + retryConfig ) + + private val buildRetryRequest: ((String, MediatorMessage) -> List>) = { _, message -> + listOf(message) + } } diff --git a/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfig.kt b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfig.kt index 1d49c60bfb9..032ce9455b0 100644 --- a/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfig.kt +++ b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfig.kt @@ -28,6 +28,8 @@ import java.time.Duration * @property stateManager State manager. * @property minGroupSize Minimum size for group of records passed to task manager for processing in a single thread. Does not block if * group size is not met by polled record count. + * @property retryConfig Topic to push retry events to as well as the function to build retry events. Set to null to not retry transient + * errors originating from the message pattern and fail early instead. */ data class EventMediatorConfig( val name: String, @@ -40,6 +42,7 @@ data class EventMediatorConfig( val threadName: String, val stateManager: StateManager, val minGroupSize: Int, + val retryConfig: RetryConfig? = null ) { /** * Timeout for polling consumers. diff --git a/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfigBuilder.kt b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfigBuilder.kt index a6e502a6b5c..b928ea3eb78 100644 --- a/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfigBuilder.kt +++ b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/EventMediatorConfigBuilder.kt @@ -27,6 +27,7 @@ class EventMediatorConfigBuilder { private var threadName: String? = null private var stateManager: StateManager? = null private var minGroupSize: Int? = null + private var retryConfig: RetryConfig? = null /** Sets name for [MultiSourceEventMediator]. */ fun name(name: String) = @@ -74,6 +75,13 @@ class EventMediatorConfigBuilder { fun stateManager(stateManager: StateManager) = apply { this.stateManager = stateManager } + /** + * Sets the topic to push retry events triggered by transient errors in the message pattern when sending RPC calls. + * As well as setting how to build a retry event from the sync request + */ + fun retryConfig(retryConfig: RetryConfig) = + apply { this.retryConfig = retryConfig } + /** Builds [EventMediatorConfig]. */ fun build(): EventMediatorConfig { check(consumerFactories.isNotEmpty()) { "At least on consumer factory has to be set" } @@ -89,6 +97,8 @@ class EventMediatorConfigBuilder { threadName = checkNotNull(threadName) { "Thread name not set" }, stateManager = checkNotNull(stateManager) { "State manager not set" }, minGroupSize = checkNotNull(minGroupSize) { "Min group size not set" }, + retryConfig = retryConfig ) } + } \ No newline at end of file diff --git a/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/RetryConfig.kt b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/RetryConfig.kt new file mode 100644 index 00000000000..bf7009abd8a --- /dev/null +++ b/libs/messaging/messaging/src/main/kotlin/net/corda/messaging/api/mediator/config/RetryConfig.kt @@ -0,0 +1,14 @@ +package net.corda.messaging.api.mediator.config + +import net.corda.messaging.api.mediator.MediatorMessage + +/** + * Config used to setup retry events in the mediator. + * These are used to retry transient errors that occur. + * @property retryTopic The topic to send retry events to + * @property buildRetryRequest lambda used to generate the retry requests + */ +data class RetryConfig( + val retryTopic: String, + val buildRetryRequest: ((K, MediatorMessage) -> List>)? = null, +) From 57b10aa432a081599699a18c3bbf65f1efa6a5a9 Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 10:07:50 +0000 Subject: [PATCH 10/19] CORE-20898: bump netty version for vulnerability (#6395) --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b5592173ded..7c1a9aab0de 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -41,7 +41,7 @@ pf4jVersion = "3.10.0" unirestVersion = "3.14.5" zipkinCoreVersion = "2.27.1" zipkinReporterVersion = "3.3.0" -nettyVersion = "4.1.108.Final" +nettyVersion = "4.1.115.Final" # Testing assertjVersion = "3.25.3" From ec9f8520becfa2f8e8d9ce7962ae83787176a80e Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:20:40 +0000 Subject: [PATCH 11/19] CORE-20847 validate the flow start event size (#6394) The FlowRestResourceImpl uses a transactional producer to send flow start events to the flow mapper. This means if the start event payload requires chunking, the producer will do so and send the event. The client will receive notification that the flow was received with a status of START_REQUESTED. The flow mapper uses an async producer as it is more performant. It will therefore be unable to pass the message to the flow engine and it will throw a fatal exception due to trying to chunk a message with an async producer. This will cause the flow mappers topic partition to become blocked as the fatal exception will result in a retry. --- .../rest/impl/FlowRestExceptionConstants.kt | 2 ++ .../flow/rest/impl/v1/FlowRestResourceImpl.kt | 16 +++++++++ .../rest/impl/v1/FlowRestResourceImplTest.kt | 33 +++++++++++++++++-- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/FlowRestExceptionConstants.kt b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/FlowRestExceptionConstants.kt index ee7378df04f..c297ea1719d 100644 --- a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/FlowRestExceptionConstants.kt +++ b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/FlowRestExceptionConstants.kt @@ -14,5 +14,7 @@ object FlowRestExceptionConstants { const val INVALID_ID = "Supplied clientRequestId %s is invalid, it must conform to the pattern %s." const val CPI_NOT_FOUND = "Failed to find a CPI for ID = %s." const val FLOW_STATUS_NOT_FOUND = "Failed to find the flow status for holdingId = %s and clientRequestId = %s." + const val MAX_FLOW_START_ARGS_SIZE = "The flow start payload has exceeded the max allowed payload size. Note: max payload size is set" + + " to half the value of maxAllowedMessageSize." } \ No newline at end of file diff --git a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt index 9e599023e27..7c516f7889d 100644 --- a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt +++ b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt @@ -1,5 +1,6 @@ package net.corda.flow.rest.impl.v1 +import net.corda.avro.serialization.CordaAvroSerializationFactory import net.corda.cpiinfo.read.CpiInfoReadService import net.corda.data.flow.FlowKey import net.corda.data.flow.output.FlowStates @@ -41,6 +42,7 @@ import net.corda.rest.response.ResponseEntity import net.corda.rest.security.CURRENT_REST_CONTEXT import net.corda.schema.Schemas.Flow.FLOW_MAPPER_START import net.corda.schema.Schemas.Flow.FLOW_STATUS_TOPIC +import net.corda.schema.configuration.MessagingConfig import net.corda.tracing.TraceTag import net.corda.tracing.addTraceContextToRecord import net.corda.tracing.trace @@ -71,6 +73,8 @@ class FlowRestResourceImpl @Activate constructor( private val permissionValidationService: PermissionValidationService, @Reference(service = PlatformInfoProvider::class) private val platformInfoProvider: PlatformInfoProvider, + @Reference(service = CordaAvroSerializationFactory::class) + private val cordaAvroSerializationFactory: CordaAvroSerializationFactory, ) : FlowRestResource, PluggableRestResource, Lifecycle { private companion object { @@ -82,12 +86,15 @@ class FlowRestResourceImpl @Activate constructor( override val targetInterface: Class = FlowRestResource::class.java override val protocolVersion get() = platformInfoProvider.localWorkerPlatformVersion + private val serializer = cordaAvroSerializationFactory.createAvroSerializer() private var publisher: Publisher? = null private var fatalErrorOccurred = false private lateinit var onFatalError: () -> Unit + private lateinit var messagingConfig: SmartConfig override fun initialise(config: SmartConfig, onFatalError: () -> Unit) { this.onFatalError = onFatalError + this.messagingConfig = config publisher?.close() publisher = publisherFactory.createPublisher(PublisherConfig("FlowRestResource"), config) } @@ -196,6 +203,13 @@ class FlowRestResourceImpl @Activate constructor( startFlow.requestBody.escapedJson, flowContextPlatformProperties ) + val startEventSize = serializer.serialize(startEvent)?.size + val maxAllowedMessageSize = getMaxAllowedMessageSize(messagingConfig) + if (startEventSize != null && startEventSize > maxAllowedMessageSize) { + log.warn(FlowRestExceptionConstants.MAX_FLOW_START_ARGS_SIZE, IllegalArgumentException("Flow start event of size " + + "[$startEventSize] exceeds maxAllowedMessageSize [$maxAllowedMessageSize]")) + throw InvalidInputDataException(FlowRestExceptionConstants.FATAL_ERROR) + } val status = messageFactory.createStartFlowStatus(clientRequestId, vNode, flowClassName) val records = listOf( @@ -320,4 +334,6 @@ class FlowRestResourceImpl @Activate constructor( private fun getVirtualNode(holdingIdentityShortHash: String): VirtualNodeInfo { return virtualNodeInfoReadService.getByHoldingIdentityShortHashOrThrow(holdingIdentityShortHash).toAvro() } + + private fun getMaxAllowedMessageSize(messagingConfig: SmartConfig) = messagingConfig.getLong(MessagingConfig.MAX_ALLOWED_MSG_SIZE) } diff --git a/components/flow/flow-rest-resource-service-impl/src/test/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImplTest.kt b/components/flow/flow-rest-resource-service-impl/src/test/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImplTest.kt index bf3a735125e..4e68dacaf75 100644 --- a/components/flow/flow-rest-resource-service-impl/src/test/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImplTest.kt +++ b/components/flow/flow-rest-resource-service-impl/src/test/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImplTest.kt @@ -1,5 +1,8 @@ package net.corda.flow.rest.impl.v1 +import com.typesafe.config.ConfigValueFactory +import net.corda.avro.serialization.CordaAvroSerializationFactory +import net.corda.avro.serialization.CordaAvroSerializer import net.corda.cpiinfo.read.CpiInfoReadService import net.corda.crypto.core.SecureHashImpl import net.corda.data.flow.FlowKey @@ -10,6 +13,7 @@ import net.corda.flow.rest.factory.MessageFactory import net.corda.flow.rest.v1.FlowRestResource import net.corda.flow.rest.v1.types.request.StartFlowParameters import net.corda.flow.rest.v1.types.response.FlowStatusResponse +import net.corda.libs.configuration.SmartConfig import net.corda.libs.configuration.SmartConfigImpl import net.corda.libs.packaging.core.CordappManifest import net.corda.libs.packaging.core.CpiIdentifier @@ -34,6 +38,7 @@ import net.corda.rest.exception.ResourceNotFoundException import net.corda.rest.exception.ServiceUnavailableException import net.corda.rest.security.CURRENT_REST_CONTEXT import net.corda.rest.security.RestAuthContext +import net.corda.schema.configuration.MessagingConfig import net.corda.test.util.identity.createTestHoldingIdentity import net.corda.utilities.MDC_CLIENT_ID import net.corda.virtualnode.OperationalStatus @@ -46,6 +51,7 @@ import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.doThrow @@ -64,6 +70,8 @@ class FlowRestResourceImplTest { private lateinit var flowStatusLookupService: FlowStatusLookupService private lateinit var virtualNodeInfoReadService: VirtualNodeInfoReadService + private lateinit var cordaAvroSerializationFactory: CordaAvroSerializationFactory + private lateinit var serializer: CordaAvroSerializer private lateinit var publisherFactory: PublisherFactory private lateinit var messageFactory: MessageFactory private lateinit var cpiInfoReadService: CpiInfoReadService @@ -131,6 +139,11 @@ class FlowRestResourceImplTest { permissionValidationService = mock() permissionValidator = mock() fatalErrorFunction = mock() + cordaAvroSerializationFactory = mock() + serializer = mock() + + whenever(cordaAvroSerializationFactory.createAvroSerializer(anyOrNull())).thenReturn(serializer) + whenever(serializer.serialize(anyOrNull())).thenReturn(byteArrayOf(1,2,3)) val cpiMetadata = getMockCPIMeta() whenever(cpiInfoReadService.get(any())).thenReturn(cpiMetadata) @@ -169,7 +182,7 @@ class FlowRestResourceImplTest { ).thenReturn(true) } - private fun createFlowRestResource(initialise: Boolean = true): FlowRestResource { + private fun createFlowRestResource(initialise: Boolean = true, messagingConfigParam: SmartConfig = messagingConfig): FlowRestResource { return FlowRestResourceImpl( virtualNodeInfoReadService, flowStatusLookupService, @@ -177,8 +190,9 @@ class FlowRestResourceImplTest { messageFactory, cpiInfoReadService, permissionValidationService, - mock() - ).apply { if (initialise) (initialise(SmartConfigImpl.empty(), fatalErrorFunction)) } + mock(), + cordaAvroSerializationFactory + ).apply { if (initialise) (initialise(messagingConfigParam, fatalErrorFunction)) } } @Test @@ -349,6 +363,16 @@ class FlowRestResourceImplTest { } } + @Test + fun `start flow fails with InvalidInputDataException when payload is too large`() { + val flowRestResource = createFlowRestResource(true, SmartConfigImpl.empty().withValue(MessagingConfig.MAX_ALLOWED_MSG_SIZE, + ConfigValueFactory.fromAnyRef(1))) + + assertThrows { + flowRestResource.startFlow(VALID_SHORT_HASH, StartFlowParameters(clientRequestId, FLOW1, TestJsonObject())) + } + } + @Test fun `start flow event fails when not initialized`() { val flowRestResource = createFlowRestResource(false) @@ -575,4 +599,7 @@ class FlowRestResourceImplTest { flowRestResource.startFlow(VALID_SHORT_HASH, StartFlowParameters("", FLOW1, TestJsonObject())) } } + + private val messagingConfig = SmartConfigImpl.empty().withValue(MessagingConfig.MAX_ALLOWED_MSG_SIZE, ConfigValueFactory.fromAnyRef + (10000000)) } From 575bd65f195a5c1e43e351a33fd0873716dfa538 Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:21:05 +0000 Subject: [PATCH 12/19] CORE-20909 validate flow result cannot exceed max message size (#6393) This PR ensures the flow result does not exceed the max allowed message size for payloads sent to the message bus. The flow result object is passed to the message bus via the FlowStatus object. If the message size is too large this will result in a fatal exception in the mediator and the flows topic partition will become blocked as the pattern will retry message after the worker restarts. --- .../requests/FlowFinishedRequestHandler.kt | 29 ++++++++++++++++-- .../FlowFinishedRequestHandlerTest.kt | 30 +++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandler.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandler.kt index c340262cb23..0cf5379c105 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandler.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandler.kt @@ -1,12 +1,15 @@ package net.corda.flow.pipeline.handlers.requests +import net.corda.avro.serialization.CordaAvroSerializationFactory import net.corda.data.flow.state.waiting.WaitingFor import net.corda.flow.fiber.FlowIORequest +import net.corda.flow.pipeline.addTerminationKeyToMeta import net.corda.flow.pipeline.events.FlowEventContext +import net.corda.flow.pipeline.exceptions.FlowFatalException import net.corda.flow.pipeline.factory.FlowMessageFactory import net.corda.flow.pipeline.factory.FlowRecordFactory -import net.corda.flow.pipeline.addTerminationKeyToMeta import net.corda.flow.pipeline.handlers.requests.helper.getRecords +import net.corda.flow.state.FlowCheckpoint import org.osgi.service.component.annotations.Activate import org.osgi.service.component.annotations.Component import org.osgi.service.component.annotations.Reference @@ -18,7 +21,9 @@ class FlowFinishedRequestHandler @Activate constructor( @Reference(service = FlowMessageFactory::class) private val flowMessageFactory: FlowMessageFactory, @Reference(service = FlowRecordFactory::class) - private val flowRecordFactory: FlowRecordFactory + private val flowRecordFactory: FlowRecordFactory, + @Reference(service = CordaAvroSerializationFactory::class) + cordaAvroSerializationFactory: CordaAvroSerializationFactory ) : FlowRequestHandler { private companion object { @@ -27,6 +32,8 @@ class FlowFinishedRequestHandler @Activate constructor( override val type = FlowIORequest.FlowFinished::class.java + private val serializer = cordaAvroSerializationFactory.createAvroSerializer() + override fun getUpdatedWaitingFor(context: FlowEventContext, request: FlowIORequest.FlowFinished): WaitingFor? { return null } @@ -36,6 +43,7 @@ class FlowFinishedRequestHandler @Activate constructor( request: FlowIORequest.FlowFinished ): FlowEventContext { val checkpoint = context.checkpoint + validateResultSize(checkpoint, request) val status = flowMessageFactory.createFlowCompleteStatusMessage(checkpoint, request.result) val records = getRecords(flowRecordFactory, context, status) @@ -47,4 +55,21 @@ class FlowFinishedRequestHandler @Activate constructor( val metaDataWithTermination = addTerminationKeyToMeta(context.metadata) return context.copy(outputRecords = context.outputRecords + records, metadata = metaDataWithTermination) } + + /** + * The flow status will contain the result string. Ensure it doesn't exceed the max message size allowed. + */ + private fun validateResultSize(checkpoint: FlowCheckpoint, request: FlowIORequest.FlowFinished) { + val maxAllowedMessageSize = checkpoint.maxMessageSize + val result = request.result + if (result != null) { + val resultSize = serializer.serialize(result)?.size + if (resultSize != null && resultSize > maxAllowedMessageSize) { + throw FlowFatalException( + "Flow attempted to return a result that is greater than the max message size allowed. Flow " + + "result size [$resultSize]. Max Allowed Message Size [$maxAllowedMessageSize]" + ) + } + } + } } diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandlerTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandlerTest.kt index 110d9a64fb6..95f58796c7e 100644 --- a/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandlerTest.kt +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/pipeline/handlers/requests/FlowFinishedRequestHandlerTest.kt @@ -1,5 +1,7 @@ package net.corda.flow.pipeline.handlers.requests +import net.corda.avro.serialization.CordaAvroSerializationFactory +import net.corda.avro.serialization.CordaAvroSerializer import net.corda.data.flow.FlowKey import net.corda.data.flow.event.mapper.FlowMapperEvent import net.corda.data.flow.event.mapper.ScheduleCleanup @@ -8,10 +10,15 @@ import net.corda.flow.BOB_X500_HOLDING_IDENTITY import net.corda.flow.RequestHandlerTestContext import net.corda.flow.SESSION_ID_1 import net.corda.flow.fiber.FlowIORequest +import net.corda.flow.pipeline.exceptions.FlowFatalException import net.corda.messaging.api.records.Record import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.mockito.Mockito.mock import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -22,10 +29,21 @@ class FlowFinishedRequestHandlerTest { val FLOW_KEY = FlowKey(SESSION_ID_1, BOB_X500_HOLDING_IDENTITY) } - private val testContext = RequestHandlerTestContext(Any()) private val flowResult = "ok" private val ioRequest = FlowIORequest.FlowFinished(flowResult) - private val handler = FlowFinishedRequestHandler(testContext.flowMessageFactory, testContext.flowRecordFactory) + private val avroSerializationFactory = mock() + private val serializer = mock>() + private lateinit var testContext: RequestHandlerTestContext + private lateinit var handler: FlowFinishedRequestHandler + + @BeforeEach + fun setup() { + whenever(serializer.serialize(any())).thenReturn(byteArrayOf(1, 2, 3)) + whenever(avroSerializationFactory.createAvroSerializer(anyOrNull())).thenReturn(serializer) + testContext = RequestHandlerTestContext(Any()) + whenever(testContext.flowCheckpoint.maxMessageSize).thenReturn(100000000) + handler = FlowFinishedRequestHandler(testContext.flowMessageFactory, testContext.flowRecordFactory, avroSerializationFactory) + } @Test fun `Updates the waiting for to nothing`() { @@ -33,6 +51,14 @@ class FlowFinishedRequestHandlerTest { assertThat(waitingFor?.value).isNull() } + @Test + fun `Flow Result exceeds max size`() { + whenever(testContext.flowCheckpoint.maxMessageSize).thenReturn(1) + handler = FlowFinishedRequestHandler(testContext.flowMessageFactory, testContext.flowRecordFactory, avroSerializationFactory) + + assertThrows { handler.postProcess(testContext.flowEventContext, ioRequest) } + } + @Test fun `post processing marks context as deleted`() { val flowStatus = FlowStatus() From ef977c59885295ef22bba58aeebe29b83d17edca Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:21:19 +0000 Subject: [PATCH 13/19] CORE-20907: stop external message api from sending messages that are too large (#6392) Currently there is no validation to stop the user from sending a message which exceeds the max allowed message size. It has been observed that this will cause a fatal exception in the mediator message pattern. Subsequently the flow partition with the affected flow will become blocked and unable to progress. --- .../messaging/ExternalMessagingImpl.kt | 29 +++++++++++++++++-- .../messaging/ExternalMessagingImplTest.kt | 28 +++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/components/flow/flow-service/src/main/kotlin/net/corda/flow/application/messaging/ExternalMessagingImpl.kt b/components/flow/flow-service/src/main/kotlin/net/corda/flow/application/messaging/ExternalMessagingImpl.kt index 888eccd741d..d8210778ddf 100644 --- a/components/flow/flow-service/src/main/kotlin/net/corda/flow/application/messaging/ExternalMessagingImpl.kt +++ b/components/flow/flow-service/src/main/kotlin/net/corda/flow/application/messaging/ExternalMessagingImpl.kt @@ -1,10 +1,12 @@ package net.corda.flow.application.messaging +import net.corda.avro.serialization.CordaAvroSerializationFactory import net.corda.flow.fiber.FlowFiberService import net.corda.flow.fiber.FlowIORequest import net.corda.sandbox.type.UsedByFlow import net.corda.v5.application.messaging.ExternalMessaging import net.corda.v5.base.annotations.Suspendable +import net.corda.v5.base.exceptions.CordaRuntimeException import net.corda.v5.serialization.SingletonSerializeAsToken import org.osgi.service.component.annotations.Activate import org.osgi.service.component.annotations.Component @@ -16,25 +18,46 @@ import java.util.UUID @Component(service = [ExternalMessaging::class, UsedByFlow::class], scope = ServiceScope.PROTOTYPE) class ExternalMessagingImpl( private val flowFiberService: FlowFiberService, - private val idFactoryFunc: () -> String + private val idFactoryFunc: () -> String, + cordaAvroSerializationFactory: CordaAvroSerializationFactory ) : ExternalMessaging, UsedByFlow, SingletonSerializeAsToken { + private val serializer = cordaAvroSerializationFactory.createAvroSerializer() + @Activate constructor( @Reference(service = FlowFiberService::class) - flowFiberService: FlowFiberService - ) : this(flowFiberService, { UUID.randomUUID().toString() }) + flowFiberService: FlowFiberService, + @Reference(service = CordaAvroSerializationFactory::class) + cordaAvroSerializationFactory: CordaAvroSerializationFactory + ) : this(flowFiberService, { UUID.randomUUID().toString() }, cordaAvroSerializationFactory) @Suspendable override fun send(channelName: String, message: String) { + validateSize(message) send(channelName, idFactoryFunc(), message) } + private fun validateSize(message: String) { + val bytesSize = serializer.serialize(message)?.size + val maxAllowedMessageSize = maxMessageSize() + if (bytesSize != null && maxAllowedMessageSize < bytesSize) { + throw CordaRuntimeException( + "Cannot send external messaging content as " + + "it exceeds the max message size allowed. Message Size: [$bytesSize], Max Size: [$maxAllowedMessageSize}]" + ) + } + } + @Suspendable override fun send(channelName: String, messageId: String, message: String) { + validateSize(message) + flowFiberService .getExecutingFiber() .suspend(FlowIORequest.SendExternalMessage(channelName, messageId, message)) } + + private fun maxMessageSize() = flowFiberService.getExecutingFiber().getExecutionContext().flowCheckpoint.maxMessageSize } diff --git a/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/ExternalMessagingImplTest.kt b/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/ExternalMessagingImplTest.kt index b7e49d26bb1..017f6dbed7b 100644 --- a/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/ExternalMessagingImplTest.kt +++ b/components/flow/flow-service/src/test/kotlin/net/corda/flow/messaging/ExternalMessagingImplTest.kt @@ -1,15 +1,33 @@ package net.corda.flow.messaging +import net.corda.avro.serialization.CordaAvroSerializationFactory +import net.corda.avro.serialization.CordaAvroSerializer import net.corda.flow.application.messaging.ExternalMessagingImpl import net.corda.flow.application.services.MockFlowFiberService import net.corda.flow.fiber.FlowIORequest +import net.corda.v5.base.exceptions.CordaRuntimeException +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.mockito.Mockito.mock +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever class ExternalMessagingImplTest { private val flowFiberService = MockFlowFiberService() - private val target = ExternalMessagingImpl(flowFiberService) { "random_id" } + private val mockSerializationFactory = mock() + private val mockSerializer = mock>() + private lateinit var target: ExternalMessagingImpl + + @BeforeEach + fun setup() { + whenever(mockSerializationFactory.createAvroSerializer()).thenReturn(mockSerializer) + whenever(mockSerializer.serialize(anyOrNull())).thenReturn(byteArrayOf(1, 2, 3)) + whenever(flowFiberService.flowCheckpoint.maxMessageSize).thenReturn(100000) + target = ExternalMessagingImpl(flowFiberService, { "random_id" }, mockSerializationFactory) + } @Test fun `send channel and message issues IO request`() { @@ -23,6 +41,14 @@ class ExternalMessagingImplTest { verify(flowFiberService.flowFiber).suspend(expectedIoRequest) } + @Test + fun `send payload exceeds max message size`() { + whenever(flowFiberService.flowCheckpoint.maxMessageSize).thenReturn(1) + target = ExternalMessagingImpl(flowFiberService, { "random_id" }, mockSerializationFactory) + + assertThrows { target.send("ch1", "msg1") } + } + @Test fun `send channel, message id and message issues IO request`() { target.send("ch1", "id2", "msg1") From 26c8a044f3bcac25f016cba635fbd73dcd4400d9 Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 15:01:30 +0000 Subject: [PATCH 14/19] CORE-20918: waiver for detekt low severity issue (#6399) --- .snyk | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.snyk b/.snyk index 19592999287..224aef7cb08 100644 --- a/.snyk +++ b/.snyk @@ -9,4 +9,12 @@ ignore: OSGi so for now we have to wait for the next one. expires: 2024-07-31T00:00:00.000Z created: 2024-04-11T15:11:31.735Z + SNYK-JAVA-ORGJETBRAINSKOTLIN-2393744: + - '*': + reason: >- + Corda5 Shippable artifacts do not make use of detekt-cli, which is + where this dependency originates, this is used at compile / build time + only for static code analysis and not shipped in any of our releasable artifacts. + expires: 2025-11-20T14:30:31.735Z + created: 2024-11-20T14:30:31.735Z patch: {} From 31fb9b2a4c8b009b2f24f97f2c85466603c1f149 Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Wed, 20 Nov 2024 16:33:26 +0000 Subject: [PATCH 15/19] CORE-20887: update jetty version to one without CVE-2024-8184 (#6400) --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7c1a9aab0de..93d25996769 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -13,7 +13,7 @@ javalinVersion = "4.6.8" jcipAnnotationsVersion = "1.0_2" # This version of Jetty must be the same major version as used by Javalin, please see above. # Once Javalin version is upgraded to the latest, this override may be removed. -jettyVersion = "9.4.54.v20240208" +jettyVersion = "9.4.56.v20240826" micrometerVersion = "1.12.3" nimbusVersion = "11.9.1" kafkaClientVersion = "3.6.1_1" From 8850133f373bc170cce984222795dda378fe79eb Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:09:28 +0000 Subject: [PATCH 16/19] CORE-20888: add snyk waiver for javalin jetty vulnerability that we are not affected by (#6402) --- .snyk | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.snyk b/.snyk index 224aef7cb08..ec36828022c 100644 --- a/.snyk +++ b/.snyk @@ -17,4 +17,16 @@ ignore: only for static code analysis and not shipped in any of our releasable artifacts. expires: 2025-11-20T14:30:31.735Z created: 2024-11-20T14:30:31.735Z + SNYK-JAVA-ORGJETBRAINSKOTLIN-2393744: + - '*': + reason: >- + This project acknowledges the presence of CVE-2024-6763 in the version of Jetty currently used by Javalin. + The vulnerability affects users of Jetty's HttpURI class, which our project does not directly utilize, + nor is it exposed through Javalin in our application context. + The Javalin team has indicated that they do not use HttpURI, and we have verified that our dependency tree presents no indirect + exposure. We will monitor Javalin updates and adopt a release upgrading Jetty to a patched version (≥12.0.12) when feasible. + Given the limited risk, no immediate action is required beyond ongoing dependency monitoring. + Note: there are currently no versions of Javalin released without this issue. + expires: 2025-11-21T14:30:31.735Z + created: 2024-11-21T14:30:31.735Z patch: {} From e10a302f0409113efcdf13562fc1c782e54a8b7d Mon Sep 17 00:00:00 2001 From: Lorcan Wogan <69468264+LWogan@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:00:10 +0000 Subject: [PATCH 17/19] CORE-20819: unirest snyk issue (#6396) Upgraded following guidance from https://github.com/Kong/unirest-java/blob/main/UPGRADE_GUIDE.md Apache client no longer packaged. Use kotlin HttpClient instead. Gson no longer packaged with unirest. Add it where required. Before v4 the Unirest client uses the GSon Objectmapper by default. In places where we did not override it to use the jackson mapper, i have added the necessary gson dependency. Note: we do set the override to use the jackson mapper in the rest:rest-client modules RemoteClient --- .snyk | 16 ++- gradle/libs.versions.toml | 9 +- .../auth/scheme/AuthenticationScheme.kt | 2 +- .../auth/scheme/BasicAuthenticationScheme.kt | 2 +- .../scheme/BearerTokenAuthenticationScheme.kt | 4 +- .../auth/scheme/NoopAuthenticationScheme.kt | 2 +- .../client/connect/remote/RemoteClient.kt | 42 ++---- libs/rest/rest-server-impl/build.gradle | 3 + .../rest/server/impl/AbstractWebsocketTest.kt | 12 +- .../rest/server/impl/InvalidRequestTest.kt | 24 ++-- .../impl/RestServerApiVersioningTest.kt | 22 +-- .../rest/server/impl/RestServerAzureAdTest.kt | 2 +- .../impl/RestServerCaseSensitiveLoginTest.kt | 8 +- .../impl/RestServerCaseSensitiveUrlTest.kt | 12 +- .../RestServerDurableStreamsRequestsTest.kt | 10 +- .../server/impl/RestServerJsonObjectTest.kt | 26 ++-- .../server/impl/RestServerLifecycleTest.kt | 8 +- .../impl/RestServerMaxContentLengthTest.kt | 6 +- .../impl/RestServerOpenApiNullabilityTest.kt | 6 +- .../rest/server/impl/RestServerOpenApiTest.kt | 20 +-- .../server/impl/RestServerRequestsTest.kt | 134 +++++++++--------- .../impl/RestServerResponseEntityTest.kt | 26 ++-- libs/rest/rest-test-common/build.gradle | 2 + .../corda/rest/test/utils/TestHttpClient.kt | 36 ++--- testing/e2e-test-utilities/build.gradle | 3 + .../e2etest/utilities/UnirestHttpsClient.kt | 41 ++---- .../utilities/config/ConfigTestUtils.kt | 2 +- .../gradle/plugin/queries/QueriesTasksTest.kt | 6 +- .../net/corda/gradle/plugin/ProjectUtils.kt | 6 +- .../cordalifecycle/EnvironmentSetupHelper.kt | 2 +- .../cordalifecycle/EnvironmentSetupTasks.kt | 2 +- .../gradle/plugin/cordapp/DeployCpiHelper.kt | 6 +- .../gradle/plugin/network/VNodeHelper.kt | 6 +- .../gradle/plugin/queries/QueryTasksImpl.kt | 6 +- 34 files changed, 239 insertions(+), 275 deletions(-) diff --git a/.snyk b/.snyk index ec36828022c..9b34c90b5a6 100644 --- a/.snyk +++ b/.snyk @@ -17,7 +17,19 @@ ignore: only for static code analysis and not shipped in any of our releasable artifacts. expires: 2025-11-20T14:30:31.735Z created: 2024-11-20T14:30:31.735Z - SNYK-JAVA-ORGJETBRAINSKOTLIN-2393744: + SNYK-JAVA-ORGECLIPSEJETTY-8186141: + - '*': + reason: >- + This project acknowledges the presence of CVE-2024-6763 in the version of Jetty currently used by Javalin. + The vulnerability affects users of Jetty's HttpURI class, which our project does not directly utilize, + nor is it exposed through Javalin in our application context. + The Javalin team has indicated that they do not use HttpURI, and we have verified that our dependency tree presents no indirect + exposure. We will monitor Javalin updates and adopt a release upgrading Jetty to a patched version (≥12.0.12) when feasible. + Given the limited risk, no immediate action is required beyond ongoing dependency monitoring. + Note: there are currently no versions of Javalin released without this issue. + expires: 2025-11-21T14:30:31.735Z + created: 2024-11-21T12:30:31.735Z + SNYK-JAVA-ORGECLIPSEJETTY-8186158: - '*': reason: >- This project acknowledges the presence of CVE-2024-6763 in the version of Jetty currently used by Javalin. @@ -28,5 +40,5 @@ ignore: Given the limited risk, no immediate action is required beyond ongoing dependency monitoring. Note: there are currently no versions of Javalin released without this issue. expires: 2025-11-21T14:30:31.735Z - created: 2024-11-21T14:30:31.735Z + created: 2024-11-21T12:30:31.735Z patch: {} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 93d25996769..6cab1fbaf6d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,6 +7,7 @@ bouncycastleVersion = "1.78.1" braveVersion = "6.0.1" caffeineVersion = "3.1.8" guavaVersion = "33.0.0-jre" +gsonVersion = "2.11.0" hikariCpVersion = "5.1.0" jacksonVersion = "2.16.1" javalinVersion = "4.6.8" @@ -38,7 +39,7 @@ typeSafeConfigVersion = "1.4.3" okHttpVersion = { strictly = "4.9.2" } okioVersion = { strictly = "3.4.0" } pf4jVersion = "3.10.0" -unirestVersion = "3.14.5" +unirestVersion = "4.4.5" zipkinCoreVersion = "2.27.1" zipkinReporterVersion = "3.3.0" nettyVersion = "4.1.115.Final" @@ -68,6 +69,7 @@ bouncycastle-prov = { group = "org.bouncycastle", name = "bcprov-jdk18on", versi brave-context-slf4j = { group = "io.zipkin.brave", name = "brave-context-slf4j", version.ref = "braveVersion" } brave-instrumentation-servlet = { group = "io.zipkin.brave", name = "brave-instrumentation-servlet", version.ref = "braveVersion" } caffeine = { group = "com.github.ben-manes.caffeine", name = "caffeine", version.ref = "caffeineVersion" } +google-gson = { group = "com.google.code.gson", name = "gson", version.ref = "gsonVersion" } guava = { group = "com.google.guava", name = "guava", version.ref = "guavaVersion" } hikaricp = { group = "com.zaxxer", name = "HikariCP", version.ref = "hikariCpVersion" } jackson-annotations = { group = "com.fasterxml.jackson.core", name = "jackson-annotations", version.ref = "jacksonVersion" } @@ -116,8 +118,9 @@ slf4j-simple = { group = "org.slf4j", name = "slf4j-simple", version.ref = "slf4 slf4j-v2-api = { group = "org.slf4j", name = "slf4j-api", version.ref = "slf4jV2Version" } snakeyaml = { group = "org.yaml", name = "snakeyaml", version.ref = "snakeyamlVersion" } typesafe-config = { group = "com.typesafe", name = "config", version.ref = "typeSafeConfigVersion" } -unirest-java = { group = "com.konghq", name = "unirest-java", version.ref = "unirestVersion"} -unirest-objectmapper-jackson = { group = "com.konghq", name = "unirest-objectmapper-jackson", version.ref = "unirestVersion" } +unirest-java = { group = "com.konghq", name = "unirest-java-core", version.ref = "unirestVersion"} +unirest-objectmapper-jackson = { group = "com.konghq", name = "unirest-modules-jackson", version.ref = "unirestVersion" } +unirest-objectmapper-gson = { group = "com.konghq", name = "unirest-modules-gson", version.ref = "unirestVersion" } okio = { group = "com.squareup.okio", name = "okio", version.ref = "okioVersion" } okHttp = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okHttpVersion" } swagger-core = { group = "io.swagger.core.v3", name = "swagger-core", version.ref = "swaggerVersion" } diff --git a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/AuthenticationScheme.kt b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/AuthenticationScheme.kt index 96e67f266f2..9de4d15b85a 100644 --- a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/AuthenticationScheme.kt +++ b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/AuthenticationScheme.kt @@ -1,6 +1,6 @@ package net.corda.rest.client.auth.scheme -import kong.unirest.HttpRequest +import kong.unirest.core.HttpRequest import net.corda.rest.client.auth.RequestContext /** diff --git a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BasicAuthenticationScheme.kt b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BasicAuthenticationScheme.kt index 8ae99b657c8..dac9316cf57 100644 --- a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BasicAuthenticationScheme.kt +++ b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BasicAuthenticationScheme.kt @@ -1,6 +1,6 @@ package net.corda.rest.client.auth.scheme -import kong.unirest.HttpRequest +import kong.unirest.core.HttpRequest import net.corda.rest.client.auth.RequestContext import net.corda.rest.client.auth.credentials.BasicAuthCredentials diff --git a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BearerTokenAuthenticationScheme.kt b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BearerTokenAuthenticationScheme.kt index 5291d73e994..1c67188638c 100644 --- a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BearerTokenAuthenticationScheme.kt +++ b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/BearerTokenAuthenticationScheme.kt @@ -1,7 +1,7 @@ package net.corda.rest.client.auth.scheme -import kong.unirest.HeaderNames.AUTHORIZATION -import kong.unirest.HttpRequest +import kong.unirest.core.HeaderNames.AUTHORIZATION +import kong.unirest.core.HttpRequest import net.corda.rest.client.auth.RequestContext import net.corda.rest.client.auth.credentials.BearerTokenCredentials diff --git a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/NoopAuthenticationScheme.kt b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/NoopAuthenticationScheme.kt index e4824485404..a0974b2bdd2 100644 --- a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/NoopAuthenticationScheme.kt +++ b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/auth/scheme/NoopAuthenticationScheme.kt @@ -1,6 +1,6 @@ package net.corda.rest.client.auth.scheme -import kong.unirest.HttpRequest +import kong.unirest.core.HttpRequest import net.corda.rest.client.auth.RequestContext internal object NoopAuthenticationScheme : AuthenticationScheme { diff --git a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/connect/remote/RemoteClient.kt b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/connect/remote/RemoteClient.kt index 0f2abcdd212..5d5909e6bfe 100644 --- a/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/connect/remote/RemoteClient.kt +++ b/libs/rest/rest-client/src/main/kotlin/net/corda/rest/client/connect/remote/RemoteClient.kt @@ -1,16 +1,15 @@ package net.corda.rest.client.connect.remote -import kong.unirest.Config -import kong.unirest.GenericType -import kong.unirest.HttpRequest -import kong.unirest.HttpRequestWithBody -import kong.unirest.HttpResponse -import kong.unirest.HttpStatus -import kong.unirest.MultipartBody -import kong.unirest.UnirestException -import kong.unirest.UnirestInstance -import kong.unirest.apache.ApacheClient -import kong.unirest.jackson.JacksonObjectMapper +import kong.unirest.core.Config +import kong.unirest.core.GenericType +import kong.unirest.core.HttpRequest +import kong.unirest.core.HttpRequestWithBody +import kong.unirest.core.HttpResponse +import kong.unirest.core.HttpStatus +import kong.unirest.core.MultipartBody +import kong.unirest.core.UnirestException +import kong.unirest.core.UnirestInstance +import kong.unirest.modules.jackson.JacksonObjectMapper import net.corda.rest.client.auth.RequestContext import net.corda.rest.client.exceptions.ClientSslHandshakeException import net.corda.rest.client.exceptions.InternalErrorException @@ -23,13 +22,8 @@ import net.corda.rest.client.processing.WebResponse import net.corda.rest.exception.ResourceAlreadyExistsException import net.corda.rest.tools.HttpVerb import net.corda.utilities.debug -import org.apache.http.conn.ssl.NoopHostnameVerifier -import org.apache.http.conn.ssl.TrustAllStrategy -import org.apache.http.impl.client.HttpClients -import org.apache.http.ssl.SSLContexts import org.slf4j.LoggerFactory import java.lang.reflect.Type -import javax.net.ssl.SSLContext import javax.net.ssl.SSLHandshakeException /** @@ -187,23 +181,13 @@ internal class RemoteUnirestClient( if (enableSsl) { log.debug { "Add Ssl params." } - val apacheClient = if (secureSsl) { + if (secureSsl) { log.debug { "Creating secure SSL context" } - ApacheClient.builder().apply(this) + // Use the default Unirest SSL handling for secure SSL (no changes required) } else { log.debug { "Creating insecure SSL context" } - val sslContext: SSLContext = SSLContexts.custom() - .loadTrustMaterial(TrustAllStrategy()) - .build() - - val httpClient = HttpClients.custom() - .setSSLContext(sslContext) - .setSSLHostnameVerifier(NoopHostnameVerifier()) - .build() - - ApacheClient.builder(httpClient).apply(this) + this.verifySsl(false) } - this.httpClient(apacheClient) log.debug { "Add Ssl params completed." } } diff --git a/libs/rest/rest-server-impl/build.gradle b/libs/rest/rest-server-impl/build.gradle index cf4a15c97dd..c5e9693429c 100644 --- a/libs/rest/rest-server-impl/build.gradle +++ b/libs/rest/rest-server-impl/build.gradle @@ -46,6 +46,7 @@ dependencies { implementation libs.jackson.datatype.jsr310 testImplementation project(":libs:rest:rest-test-common") + testImplementation libs.google.gson testRuntimeOnly 'org.osgi:osgi.core' testRuntimeOnly libs.slf4j.simple @@ -56,6 +57,8 @@ dependencies { runtimeOnly "com.sun.activation:javax.activation:$activationVersion" integrationTestImplementation libs.unirest.java + // some tests expect Gson as this was the default in 3.X and it was not overridden previously in TestHttpClientUnirestImpl + integrationTestImplementation libs.unirest.objectmapper.gson integrationTestImplementation project(':libs:rest:ssl-cert-read-impl') integrationTestImplementation project(":testing:test-utilities") diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt index 25f890ad039..4205e37173d 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt @@ -1,10 +1,10 @@ package net.corda.rest.server.impl import io.javalin.core.util.Header +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.utils.WebRequest import net.corda.rest.tools.HttpVerb -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.eclipse.jetty.io.EofException import org.eclipse.jetty.websocket.api.CloseStatus @@ -20,7 +20,7 @@ import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.slf4j.Logger import java.net.URI -import java.util.* +import java.util.Base64 import java.util.concurrent.CountDownLatch import java.util.concurrent.ExecutionException import java.util.concurrent.TimeUnit @@ -49,10 +49,10 @@ abstract class AbstractWebsocketTest : RestServerTestBase() { @Test fun `valid path returns 200 OK`() { val getPathResponse = client.call(HttpVerb.GET, WebRequest("health/sanity"), userName, password) - assertEquals(HttpStatus.SC_OK, getPathResponse.responseStatus) - assertEquals("localhost", getPathResponse.headers[Header.ACCESS_CONTROL_ALLOW_ORIGIN]) - assertEquals("true", getPathResponse.headers[Header.ACCESS_CONTROL_ALLOW_CREDENTIALS]) - assertEquals("no-cache", getPathResponse.headers[Header.CACHE_CONTROL]) + assertEquals(HttpStatus.OK, getPathResponse.responseStatus) + assertEquals("localhost", getPathResponse.headers[Header.ACCESS_CONTROL_ALLOW_ORIGIN.lowercase()]) + assertEquals("true", getPathResponse.headers[Header.ACCESS_CONTROL_ALLOW_CREDENTIALS.lowercase()]) + assertEquals("no-cache", getPathResponse.headers[Header.CACHE_CONTROL.lowercase()]) } @Test diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/InvalidRequestTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/InvalidRequestTest.kt index 89826f87ec3..9b936b4374e 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/InvalidRequestTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/InvalidRequestTest.kt @@ -2,6 +2,7 @@ package net.corda.rest.server.impl import com.google.gson.JsonObject import com.google.gson.JsonParser +import kong.unirest.core.HttpStatus import net.corda.rest.server.apigen.test.TestJavaPrimitivesRestResourceImpl import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.TestHealthCheckAPIImpl @@ -11,7 +12,6 @@ import net.corda.rest.test.utils.WebResponse import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.Assertions.assertFalse @@ -70,9 +70,9 @@ class InvalidRequestTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, pingResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, pingResponse.responseStatus) assertNotNull(pingResponse.body) - assertEquals("application/json", pingResponse.headers["Content-Type"]) + assertEquals("application/json", pingResponse.headers["content-type"]) assertThat(pingResponse.body).contains("Duplicate field 'data'") assertThat(pingResponse.body).doesNotContain("\"type\":") } @@ -85,7 +85,7 @@ class InvalidRequestTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, plusDoubleResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, plusDoubleResponse.responseStatus) assertNotNull(plusDoubleResponse.body) assertThat(plusDoubleResponse.body).contains("Unexpected character ('0' (code 48))") assertThat(plusDoubleResponse.body).doesNotContain("\"type\":") @@ -99,7 +99,7 @@ class InvalidRequestTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, negateIntResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, negateIntResponse.responseStatus) val responseBody = negateIntResponse.body assertNotNull(responseBody) assertTrue(responseBody.contains("Numeric value (3147483647) out of range of int (-2147483648 - 2147483647)")) @@ -109,7 +109,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `POST ping null value for non-nullable String should return 400 BAD REQUEST`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) assertThat(responseBody).contains(MISSING_VALUE_ERROR) @@ -135,7 +135,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `POST ping missing value for non-nullable String should return 400 BAD REQUEST`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) assertThat(responseBody).contains(MISSING_VALUE_ERROR) @@ -161,7 +161,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `Timezone specified in date should return 400 BAD REQUEST`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) assertThat(responseBody).contains(WRONG_PARAMETER) @@ -187,7 +187,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `Wrong date format should return 400 BAD REQUEST`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) assertThat(responseBody).contains(WRONG_PARAMETER) @@ -219,7 +219,7 @@ class InvalidRequestTest : RestServerTestBase() { fun `passing 3 backslashes as UUID should be handled properly`() { val parseUuidResponse = client.call(HttpVerb.POST, WebRequest("health/parseuuid/%5C%5C%5C"), userName, password) - assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, parseUuidResponse.responseStatus) + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, parseUuidResponse.responseStatus) val responseBody = parseUuidResponse.body assertNotNull(responseBody) assertDoesNotThrow(responseBody) { JsonParser.parseString(responseBody) } @@ -228,7 +228,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `pass integer in query that cannot be parsed`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) @@ -243,7 +243,7 @@ class InvalidRequestTest : RestServerTestBase() { @Test fun `pass integer in path that cannot be parsed`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_BAD_REQUEST, responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, responseStatus) val responseBody = body assertNotNull(responseBody) diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerApiVersioningTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerApiVersioningTest.kt index 5bf26716d47..6a258067f98 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerApiVersioningTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerApiVersioningTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.annotations.RestApiVersion import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.TestEndpointVersioningRestResourceImpl @@ -10,7 +11,6 @@ import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -71,8 +71,8 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) - assertEquals(HttpStatus.SC_OK, response2.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) + assertEquals(HttpStatus.OK, response2.responseStatus) } @Test @@ -83,7 +83,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_NOT_FOUND, response.responseStatus) + assertEquals(HttpStatus.NOT_FOUND, response.responseStatus) val response2 = client.call( HttpVerb.GET, @@ -91,7 +91,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response2.responseStatus) + assertEquals(HttpStatus.OK, response2.responseStatus) } @Test @@ -102,7 +102,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) val response2 = client.call( HttpVerb.GET, @@ -110,7 +110,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_NOT_FOUND, response2.responseStatus) + assertEquals(HttpStatus.NOT_FOUND, response2.responseStatus) } @Test @@ -121,7 +121,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) } @Test @@ -132,7 +132,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_NOT_FOUND, response.responseStatus) + assertEquals(HttpStatus.NOT_FOUND, response.responseStatus) } @Test @@ -143,7 +143,7 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) val response2 = client.call( HttpVerb.GET, @@ -151,6 +151,6 @@ class RestServerApiVersioningTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response2.responseStatus) + assertEquals(HttpStatus.OK, response2.responseStatus) } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerAzureAdTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerAzureAdTest.kt index 5028546c6f6..a40ad80719f 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerAzureAdTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerAzureAdTest.kt @@ -1,6 +1,6 @@ package net.corda.rest.server.impl -import kong.unirest.HttpStatus +import kong.unirest.core.HttpStatus import net.corda.rest.security.read.RestSecurityManager import net.corda.rest.server.RestServer import net.corda.rest.server.config.models.AzureAdSettings diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveLoginTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveLoginTest.kt index fa6b51ae664..e17df869c5d 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveLoginTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveLoginTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.server.impl.security.provider.basic.UsernamePasswordAuthenticationProvider import net.corda.rest.test.TestHealthCheckAPIImpl @@ -7,7 +8,6 @@ import net.corda.rest.test.utils.TestHttpClientUnirestImpl import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -57,7 +57,7 @@ class RestServerCaseSensitiveLoginTest : RestServerTestBase() { "aDmIn", password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(listOf(2.0, 3.0), plusOneResponse.body) } @@ -70,7 +70,7 @@ class RestServerCaseSensitiveLoginTest : RestServerTestBase() { userName, "aDmIn" ) - assertEquals(HttpStatus.SC_UNAUTHORIZED, plusOneResponse.responseStatus) + assertEquals(HttpStatus.UNAUTHORIZED, plusOneResponse.responseStatus) } @Test @@ -84,7 +84,7 @@ class RestServerCaseSensitiveLoginTest : RestServerTestBase() { ) assertEquals( "Basic realm=\"${UsernamePasswordAuthenticationProvider.REALM_VALUE}\"", - plusOneResponse.headers["WWW-Authenticate"] + plusOneResponse.headers["WWW-Authenticate".lowercase()] ) } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveUrlTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveUrlTest.kt index 5f5cf96b611..5c4d5605556 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveUrlTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerCaseSensitiveUrlTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.TestHealthCheckAPIImpl import net.corda.rest.test.utils.TestHttpClientUnirestImpl @@ -8,7 +9,6 @@ import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb.GET import net.corda.rest.tools.HttpVerb.POST import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -62,12 +62,12 @@ class RestServerCaseSensitiveUrlTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(listOf(2.0, 3.0), plusOneResponse.body) } @Test - fun `Uppercase POST will return 301 Moved Permanently`() { + fun `Uppercase POST will return 404 Not Found`() { val pingResponse = client.call( POST, WebRequest( @@ -77,7 +77,7 @@ class RestServerCaseSensitiveUrlTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_MOVED_PERMANENTLY, pingResponse.responseStatus) + assertEquals(HttpStatus.NOT_FOUND, pingResponse.responseStatus) } @Test @@ -93,7 +93,7 @@ class RestServerCaseSensitiveUrlTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(requestStringValue, plusOneResponse.body) } @@ -107,7 +107,7 @@ class RestServerCaseSensitiveUrlTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(requestStringValue, plusOneResponse.body) } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerDurableStreamsRequestsTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerDurableStreamsRequestsTest.kt index 73b30fb2599..7bb3db7ce1a 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerDurableStreamsRequestsTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerDurableStreamsRequestsTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.server.impl.utils.compact import net.corda.rest.test.CalendarRestResourceImpl @@ -11,7 +12,6 @@ import net.corda.rest.test.utils.TestHttpClientUnirestImpl import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -77,7 +77,7 @@ class RestServerDurableStreamsRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus, response.toString()) + assertEquals(HttpStatus.OK, response.responseStatus, response.toString()) assertEquals(responseBody, response.body) } @@ -98,7 +98,7 @@ class RestServerDurableStreamsRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, secondResponse.responseStatus) + assertEquals(HttpStatus.OK, secondResponse.responseStatus) assertEquals(responseBodyNewPosition, secondResponse.body) } @@ -119,7 +119,7 @@ class RestServerDurableStreamsRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(responseBody, response.body) } @@ -140,7 +140,7 @@ class RestServerDurableStreamsRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, secondResponse.responseStatus) + assertEquals(HttpStatus.OK, secondResponse.responseStatus) assertEquals(responseBodyNewPosition, secondResponse.body) } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerJsonObjectTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerJsonObjectTest.kt index 6f0aec6af8f..dcdc04083fd 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerJsonObjectTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerJsonObjectTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.CustomNonSerializableString import net.corda.rest.test.CustomUnsafeString @@ -9,7 +10,6 @@ import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb.POST import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeAll @@ -147,7 +147,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedObjectResponse, response.body) } @@ -162,7 +162,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedObjectResponse, response.body) } @@ -177,7 +177,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedObjectResponse, response.body) } @@ -192,7 +192,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedObjectResponse, response.body) } @@ -207,7 +207,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedMapResponse, response.body) } @@ -222,7 +222,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedMapResponse, response.body) } @@ -237,7 +237,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedMapResponse, response.body) } @@ -252,7 +252,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedMapResponse, response.body) } @@ -267,7 +267,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedArrayResponse, response.body) } @@ -282,7 +282,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedArrayResponse, response.body) } @@ -297,7 +297,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedArrayResponse, response.body) } @@ -312,7 +312,7 @@ class RestServerJsonObjectTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals(expectedArrayResponse, response.body) } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerLifecycleTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerLifecycleTest.kt index 91f1c43f657..658127ba64c 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerLifecycleTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerLifecycleTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.LifecycleRestResourceImpl import net.corda.rest.test.utils.TestHttpClientUnirestImpl @@ -8,7 +9,6 @@ import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb.GET import net.corda.test.util.lifecycle.usingLifecycle import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -58,7 +58,7 @@ class RestServerLifecycleTest : RestServerTestBase() { // Should report unavailable when REST implementation is not started with(client.call(GET, WebRequest("lifecycle/hello/world?id=1"), userName, password)) { println("### $responseStatus") - assertEquals(HttpStatus.SC_SERVICE_UNAVAILABLE, responseStatus) + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, responseStatus) } // Do start @@ -67,7 +67,7 @@ class RestServerLifecycleTest : RestServerTestBase() { // Assert functions normally lifecycleRestResourceImpl.usingLifecycle { with(client.call(GET, WebRequest("lifecycle/hello/world?id=1"), userName, password)) { - assertEquals(HttpStatus.SC_OK, responseStatus) + assertEquals(HttpStatus.OK, responseStatus) assertEquals("Hello 1 : world", body) } lifecycleRestResourceImpl.stop() @@ -75,7 +75,7 @@ class RestServerLifecycleTest : RestServerTestBase() { // Assert back to unavailable after stop/close with(client.call(GET, WebRequest("lifecycle/hello/world?id=1"), userName, password)) { - assertEquals(HttpStatus.SC_SERVICE_UNAVAILABLE, responseStatus) + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, responseStatus) } } } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerMaxContentLengthTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerMaxContentLengthTest.kt index f7fdcaac9ac..2aaa2acaa6c 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerMaxContentLengthTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerMaxContentLengthTest.kt @@ -1,5 +1,6 @@ package net.corda.rest.server.impl +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.TestHealthCheckAPIImpl import net.corda.rest.test.utils.TestHttpClientUnirestImpl @@ -8,7 +9,6 @@ import net.corda.rest.test.utils.WebResponse import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -57,7 +57,7 @@ class RestServerMaxContentLengthTest : RestServerTestBase() { fun `Content length exceeding maxContentLength returns Bad Request`() { val dataExceedsMax = "1".repeat(MAX_CONTENT_LENGTH + 5) val pingResponse = client.call(HttpVerb.POST, WebRequest("health/ping", dataExceedsMax), userName, password) - assertEquals(HttpStatus.SC_BAD_REQUEST, pingResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, pingResponse.responseStatus) val actual = pingResponse.body assertNotNull(actual) assertTrue(actual.contains("Content length is ${MAX_CONTENT_LENGTH + 5} which exceeds the maximum limit of $MAX_CONTENT_LENGTH.")) @@ -66,7 +66,7 @@ class RestServerMaxContentLengthTest : RestServerTestBase() { @Test fun `Content length below maxContentLength returns 200`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_OK, responseStatus) + assertEquals(HttpStatus.OK, responseStatus) assertEquals("Pong for str = stringdata", body) } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiNullabilityTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiNullabilityTest.kt index e0182c5b6ee..8b8568111be 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiNullabilityTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiNullabilityTest.kt @@ -2,6 +2,7 @@ package net.corda.rest.server.impl import io.swagger.v3.core.util.Json import io.swagger.v3.oas.models.OpenAPI +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.server.impl.utils.compact import net.corda.rest.test.NullabilityRestResourceImpl @@ -10,7 +11,6 @@ import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb.GET import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll @@ -62,8 +62,8 @@ class RestServerOpenApiNullabilityTest : RestServerTestBase() { @Test fun `GET openapi should return the OpenApi spec json`() { val apiSpec = client.call(GET, WebRequest("swagger.json")) - assertEquals(HttpStatus.SC_OK, apiSpec.responseStatus) - assertEquals("application/json", apiSpec.headers["Content-Type"]) + assertEquals(HttpStatus.OK, apiSpec.responseStatus) + assertEquals("application/json", apiSpec.headers["content-type"]) val body = apiSpec.body!!.compact() assertTrue(body.contains(""""openapi" : "3.0.1"""")) assertFalse(body.contains("\"null\"")) diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiTest.kt index 5e564109f1f..f74b5d0c4ca 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerOpenApiTest.kt @@ -10,6 +10,7 @@ import io.swagger.v3.oas.models.media.NumberSchema import io.swagger.v3.oas.models.media.ObjectSchema import io.swagger.v3.oas.models.media.Schema import io.swagger.v3.oas.models.media.StringSchema +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.server.impl.internal.OptionalDependency import net.corda.rest.server.impl.utils.compact @@ -23,7 +24,6 @@ import net.corda.rest.test.utils.WebRequest import net.corda.rest.test.utils.multipartDir import net.corda.rest.tools.HttpVerb.GET import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll @@ -81,8 +81,8 @@ class RestServerOpenApiTest : RestServerTestBase() { @Test fun `GET openapi should return the OpenApi spec json`() { val apiSpec = client.call(GET, WebRequest("swagger.json")) - assertEquals(HttpStatus.SC_OK, apiSpec.responseStatus) - assertEquals("application/json", apiSpec.headers["Content-Type"]) + assertEquals(HttpStatus.OK, apiSpec.responseStatus) + assertEquals("application/json", apiSpec.headers["content-type"]) val body = apiSpec.body!!.compact() assertTrue(body.contains(""""openapi" : "3.0.1"""")) assertFalse(body.contains("\"null\"")) @@ -230,8 +230,8 @@ class RestServerOpenApiTest : RestServerTestBase() { @Test fun `OpenApi spec json should include correctly formatted multipart file upload endpoints`() { val apiSpec = client.call(GET, WebRequest("swagger.json")) - assertEquals(HttpStatus.SC_OK, apiSpec.responseStatus) - assertEquals("application/json", apiSpec.headers["Content-Type"]) + assertEquals(HttpStatus.OK, apiSpec.responseStatus) + assertEquals("application/json", apiSpec.headers["content-type"]) val body = apiSpec.body!!.compact() assertTrue(body.contains(""""openapi" : "3.0.1"""")) assertFalse(body.contains("\"null\"")) @@ -496,8 +496,8 @@ class RestServerOpenApiTest : RestServerTestBase() { @Test fun `GET swagger UI should return html with reference to swagger json`() { val apiSpec = client.call(GET, WebRequest("swagger")) - assertEquals(HttpStatus.SC_OK, apiSpec.responseStatus) - assertEquals("text/html", apiSpec.headers["Content-Type"]) + assertEquals(HttpStatus.OK, apiSpec.responseStatus) + assertEquals("text/html", apiSpec.headers["content-type"]) val expected = """url: "/${context.basePath}/${apiVersion.versionPath}/swagger.json"""" assertTrue(apiSpec.body!!.contains(expected)) } @@ -510,9 +510,9 @@ class RestServerOpenApiTest : RestServerTestBase() { val swaggerUIBundleJS = baseClient.call(GET, WebRequest("webjars/swagger-ui/$swaggerUIversion/swagger-ui-bundle.js")) val swaggerUIcss = baseClient.call(GET, WebRequest("webjars/swagger-ui/$swaggerUIversion/swagger-ui-bundle.js")) - assertEquals(HttpStatus.SC_OK, swagger.responseStatus) - assertEquals(HttpStatus.SC_OK, swaggerUIBundleJS.responseStatus) - assertEquals(HttpStatus.SC_OK, swaggerUIcss.responseStatus) + assertEquals(HttpStatus.OK, swagger.responseStatus) + assertEquals(HttpStatus.OK, swaggerUIBundleJS.responseStatus) + assertEquals(HttpStatus.OK, swaggerUIcss.responseStatus) assertNotNull(swaggerUIBundleJS.body) assertNotNull(swaggerUIcss.body) } diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt index ef885bcb4cc..c9563c50106 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt @@ -5,6 +5,7 @@ import io.javalin.core.util.Header.ACCESS_CONTROL_ALLOW_CREDENTIALS import io.javalin.core.util.Header.ACCESS_CONTROL_ALLOW_ORIGIN import io.javalin.core.util.Header.CACHE_CONTROL import io.javalin.core.util.Header.WWW_AUTHENTICATE +import kong.unirest.core.HttpStatus import net.corda.rest.server.apigen.test.TestJavaPrimitivesRestResourceImpl import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.server.impl.apigen.processing.openapi.schema.toExample @@ -26,7 +27,6 @@ import net.corda.rest.tools.HttpVerb.GET import net.corda.rest.tools.HttpVerb.POST import net.corda.rest.tools.HttpVerb.PUT import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.AfterEach @@ -92,7 +92,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_NOT_FOUND, invalidPathResponse.responseStatus) + assertEquals(HttpStatus.NOT_FOUND, invalidPathResponse.responseStatus) } @Test @@ -103,10 +103,10 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, getPathResponse.responseStatus) - assertEquals("localhost", getPathResponse.headers[ACCESS_CONTROL_ALLOW_ORIGIN]) - assertEquals("true", getPathResponse.headers[ACCESS_CONTROL_ALLOW_CREDENTIALS]) - assertEquals("no-cache", getPathResponse.headers[CACHE_CONTROL]) + assertEquals(HttpStatus.OK, getPathResponse.responseStatus) + assertEquals("localhost", getPathResponse.headers[ACCESS_CONTROL_ALLOW_ORIGIN.lowercase()]) + assertEquals("true", getPathResponse.headers[ACCESS_CONTROL_ALLOW_CREDENTIALS.lowercase()]) + assertEquals("no-cache", getPathResponse.headers[CACHE_CONTROL.lowercase()]) } @Test @@ -117,15 +117,15 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, sanityResponse.responseStatus) + assertEquals(HttpStatus.OK, sanityResponse.responseStatus) assertEquals("Sane", sanityResponse.body) } @Test fun `POST ping returns Pong with custom deserializer`() { fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_OK, responseStatus) - assertEquals("application/json", headers["Content-Type"]) + assertEquals(HttpStatus.OK, responseStatus) + assertEquals("application/json", headers["content-type"]) assertEquals("Pong for str = stringdata", body) } @@ -157,7 +157,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, pingResponse.responseStatus) + assertEquals(HttpStatus.OK, pingResponse.responseStatus) assertEquals("Pong for null", pingResponse.body) } @@ -169,7 +169,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_NO_CONTENT, pingResponse.responseStatus) + assertEquals(HttpStatus.NO_CONTENT, pingResponse.responseStatus) assertEquals("", pingResponse.body) } @@ -182,8 +182,8 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) - assertEquals("application/json", plusOneResponse.headers["Content-Type"]) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) + assertEquals("application/json", plusOneResponse.headers["content-type"]) assertEquals(listOf(2.0, 3.0), plusOneResponse.body) } @@ -196,7 +196,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(emptyList(), plusOneResponse.body) } @@ -209,7 +209,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, plusOneResponse.responseStatus) + assertEquals(HttpStatus.OK, plusOneResponse.responseStatus) assertEquals(listOf(2.0, 3.0), plusOneResponse.body) } @@ -221,7 +221,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Hello 1 : world", helloResponse.body) } @@ -233,7 +233,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Hello null : world", helloResponse.body) } @@ -246,7 +246,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Hello queryParam: id, pathParam : pathString", helloResponse.body) // Check full URL received by the Security Manager @@ -262,7 +262,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("3", helloResponse.body) // Check that security managed has not been called for GetProtocolVersion which is exempt from permissions check @@ -279,7 +279,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Retrieved using id: 1234", helloResponse.body) // Check full URL received by the Security Manager @@ -295,7 +295,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Hello queryParam: null, pathParam : pathString", helloResponse.body) } @@ -307,8 +307,8 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, reverseTextResponse.responseStatus) - assertEquals("application/json", reverseTextResponse.headers["Content-Type"]) + assertEquals(HttpStatus.OK, reverseTextResponse.responseStatus) + assertEquals("application/json", reverseTextResponse.headers["content-type"]) assertEquals("3000000000", reverseTextResponse.body) } @@ -320,8 +320,8 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, negateIntResponse.responseStatus) - assertEquals("application/json", negateIntResponse.headers["Content-Type"]) + assertEquals(HttpStatus.OK, negateIntResponse.responseStatus) + assertEquals("application/json", negateIntResponse.headers["content-type"]) assertEquals("-1", negateIntResponse.body) } @@ -333,8 +333,8 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, negateIntResponse.responseStatus) - assertEquals("application/json", negateIntResponse.headers["Content-Type"]) + assertEquals(HttpStatus.OK, negateIntResponse.responseStatus) + assertEquals("application/json", negateIntResponse.headers["content-type"]) assertEquals("-1", negateIntResponse.body) } @@ -346,7 +346,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, negateLongResponse.responseStatus) + assertEquals(HttpStatus.OK, negateLongResponse.responseStatus) assertEquals("-3000000000", negateLongResponse.body) } @@ -358,7 +358,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, negateBooleanResponse.responseStatus) + assertEquals(HttpStatus.OK, negateBooleanResponse.responseStatus) assertEquals("false", negateBooleanResponse.body) } @@ -370,7 +370,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.OK, reverseTextResponse.responseStatus) assertEquals("text", reverseTextResponse.body) } @@ -382,7 +382,7 @@ class RestServerRequestsTest : RestServerTestBase() { GET, WebRequest("health/sanity") ) - assertEquals(HttpStatus.SC_UNAUTHORIZED, getPathResponse.responseStatus) + assertEquals(HttpStatus.UNAUTHORIZED, getPathResponse.responseStatus) assertEquals("User credentials are empty or cannot be resolved", getPathResponse.body!!.asMapFromJson()["title"]) } @@ -392,7 +392,7 @@ class RestServerRequestsTest : RestServerTestBase() { GET, WebRequest("health/sanity") ) - val headerValue = getPathResponse.headers[WWW_AUTHENTICATE] + val headerValue = getPathResponse.headers[WWW_AUTHENTICATE.lowercase()] assertEquals("Basic realm=\"${UsernamePasswordAuthenticationProvider.REALM_VALUE}\"", headerValue) } @@ -404,7 +404,7 @@ class RestServerRequestsTest : RestServerTestBase() { "invalidUser", password ) - assertEquals(HttpStatus.SC_UNAUTHORIZED, getPathResponse.responseStatus) + assertEquals(HttpStatus.UNAUTHORIZED, getPathResponse.responseStatus) assertEquals("Error during user authentication", getPathResponse.body!!.asMapFromJson()["title"]) } @@ -416,7 +416,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, "invalidPassword" ) - assertEquals(HttpStatus.SC_UNAUTHORIZED, getPathResponse.responseStatus) + assertEquals(HttpStatus.UNAUTHORIZED, getPathResponse.responseStatus) assertEquals("Error during user authentication", getPathResponse.body!!.asMapFromJson()["title"]) } @@ -428,7 +428,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, getPathResponse.responseStatus) + assertEquals(HttpStatus.OK, getPathResponse.responseStatus) } @Test @@ -439,7 +439,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, getPathResponse.responseStatus) + assertEquals(HttpStatus.OK, getPathResponse.responseStatus) assertEquals("2", getPathResponse.body) } @@ -451,7 +451,7 @@ class RestServerRequestsTest : RestServerTestBase() { "invalid", "invalid" ) - assertEquals(HttpStatus.SC_OK, getPathResponse.responseStatus) + assertEquals(HttpStatus.OK, getPathResponse.responseStatus) assertEquals("2", getPathResponse.body) } @@ -463,7 +463,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.OK, reverseTextResponse.responseStatus) assertEquals("a b", reverseTextResponse.body) } @@ -475,7 +475,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.OK, reverseTextResponse.responseStatus) assertEquals("null null", reverseTextResponse.body) } @@ -487,7 +487,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.OK, reverseTextResponse.responseStatus) assertEquals("null null", reverseTextResponse.body) } @@ -499,7 +499,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, reverseTextResponse.responseStatus) } @Test @@ -510,7 +510,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, reverseTextResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, reverseTextResponse.responseStatus) } @Test @@ -524,7 +524,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, timeCallResponse.responseStatus) + assertEquals(HttpStatus.OK, timeCallResponse.responseStatus) assertEquals("2020-01-01T11:00Z", timeCallResponse.body) } @@ -539,7 +539,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, timeCallResponse.responseStatus) + assertEquals(HttpStatus.OK, timeCallResponse.responseStatus) } @Test @@ -547,7 +547,7 @@ class RestServerRequestsTest : RestServerTestBase() { val date = "2021-07-29T13:13:14" fun WebResponse.doAssert() { - assertEquals(HttpStatus.SC_OK, responseStatus) + assertEquals(HttpStatus.OK, responseStatus) assertThat(body!!).contains(date) } @@ -582,7 +582,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, instantCallResponse.responseStatus) + assertEquals(HttpStatus.OK, instantCallResponse.responseStatus) assertEquals(instant, instantCallResponse.body) } @@ -595,7 +595,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, timeCallResponse.responseStatus) + assertEquals(HttpStatus.OK, timeCallResponse.responseStatus) assertEquals("{\"data\":\"custom text\"}", timeCallResponse.body) } @@ -616,7 +616,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, throwExceptionResponse.responseStatus) + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, throwExceptionResponse.responseStatus) } @Test @@ -628,7 +628,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals("Created using: CreationParams(name=TestName, amount=20)", createEntityResponse.body) } @@ -636,7 +636,7 @@ class RestServerRequestsTest : RestServerTestBase() { fun `Call get using path on test entity`() { val createEntityResponse = client.call(GET, WebRequest("testentity/myId"), userName, password) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals("Retrieved using id: myId", createEntityResponse.body) } @@ -649,7 +649,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals("Retrieved using query: MyQuery", createEntityResponse.body) } @@ -665,7 +665,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals( "Updated using params: UpdateParams(id=myId, name=TestName, amount=20)", createEntityResponse.body @@ -676,7 +676,7 @@ class RestServerRequestsTest : RestServerTestBase() { fun `Call delete using path on test entity`() { val createEntityResponse = client.call(DELETE, WebRequest("testentity/myId"), userName, password) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals("Deleted using id: myId", createEntityResponse.body) } @@ -689,7 +689,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals("Deleted using query: MyQuery", createEntityResponse.body) } @@ -721,7 +721,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedChecksum = ChecksumUtil.generateChecksum(text.byteInputStream()) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedChecksum, createEntityResponse.body) } @@ -743,7 +743,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedResult = "some-text-as-parameter, ${ChecksumUtil.generateChecksum(text.byteInputStream())}" - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedResult, createEntityResponse.body) } @@ -764,7 +764,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedResult = ChecksumUtil.generateChecksum(text.byteInputStream()) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedResult, createEntityResponse.body) } @@ -788,7 +788,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedResult = ChecksumUtil.generateChecksum(text1.byteInputStream()) + ", " + ChecksumUtil.generateChecksum(text2.byteInputStream()) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedResult, createEntityResponse.body) } @@ -814,7 +814,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedResult = ChecksumUtil.generateChecksum(text1.byteInputStream()) + ", " + ChecksumUtil.generateChecksum(text2.byteInputStream()) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedResult, createEntityResponse.body) } @@ -835,7 +835,7 @@ class RestServerRequestsTest : RestServerTestBase() { val expectedResult = ChecksumUtil.generateChecksum(text1.byteInputStream()) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals(expectedResult, createEntityResponse.body) } @@ -848,7 +848,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_BAD_REQUEST, createEntityResponse.responseStatus) + assertEquals(HttpStatus.BAD_REQUEST, createEntityResponse.responseStatus) } @Test @@ -863,7 +863,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("Completed foo", helloResponse.body) } @@ -878,7 +878,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("""null""", helloResponse.body) } @@ -893,7 +893,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("""null""", helloResponse.body) } @@ -908,7 +908,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, helloResponse.responseStatus) + assertEquals(HttpStatus.OK, helloResponse.responseStatus) assertEquals("""{"str":null}""", helloResponse.body) } @@ -928,7 +928,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus, "for $testBody") + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus, "for $testBody") assertEquals(testBody, createEntityResponse.body, "for $testBody") } } @@ -945,7 +945,7 @@ class RestServerRequestsTest : RestServerTestBase() { password ) - assertEquals(HttpStatus.SC_OK, createEntityResponse.responseStatus) + assertEquals(HttpStatus.OK, createEntityResponse.responseStatus) assertEquals( "Updated using params: UpdateParams(id=myId, name=TestName%, amount=20)", createEntityResponse.body diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerResponseEntityTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerResponseEntityTest.kt index df5aed3a718..b54f7a46e5d 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerResponseEntityTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerResponseEntityTest.kt @@ -2,6 +2,7 @@ package net.corda.rest.server.impl import io.swagger.v3.core.util.Json import io.swagger.v3.oas.models.OpenAPI +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.CustomNonSerializableString import net.corda.rest.test.CustomUnsafeString @@ -14,7 +15,6 @@ import net.corda.rest.tools.HttpVerb.DELETE import net.corda.rest.tools.HttpVerb.POST import net.corda.rest.tools.HttpVerb.PUT import net.corda.utilities.NetworkHostAndPort -import org.apache.http.HttpStatus import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.AfterEach @@ -71,81 +71,81 @@ class RestServerResponseEntityTest : RestServerTestBase() { @Test fun `endpoint returns ResponseEntity with null responseBody returns null in json with given status code`() { val response = client.call(PUT, WebRequest("responseentity/put-returns-nullable-string"), userName, password) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals("null", response.body) } @Test fun `endpoint returns ResponseEntity with responseBody and given status code`() { val response = client.call(PUT, WebRequest("responseentity/put-returns-ok-string"), userName, password) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals("some string that isn't json inside response", response.body) } @Test fun `endpoint returns string result the same as if using ResponseEntity with OK status`() { val response = client.call(PUT, WebRequest("responseentity/put-returns-string"), userName, password) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals("put string", response.body) } @Test fun `endpoint returns an object the same as using ResponseEntity with OK status`() { val response = client.call(POST, WebRequest("responseentity/post-returns-raw-entity"), userName, password) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals("{\"id\":\"no response entity used\"}", response.body) } @Test fun `endpoint with no return type completes with no_content status`() { val response = client.call(DELETE, WebRequest("responseentity/delete-returns-void"), userName, password) - assertEquals(HttpStatus.SC_NO_CONTENT, response.responseStatus) + assertEquals(HttpStatus.NO_CONTENT, response.responseStatus) assertEquals("", response.body) } @Test fun `post returns void has no_content and empty body`() { val response = client.call(POST, WebRequest("responseentity/post-returns-void"), userName, password) - assertEquals(HttpStatus.SC_NO_CONTENT, response.responseStatus) + assertEquals(HttpStatus.NO_CONTENT, response.responseStatus) assertEquals("", response.body) } @Test fun `post returns a string and has status OK with the string in the response body`() { val response = client.call(POST, WebRequest("responseentity/post-returns-ok-string-json"), userName, password) - assertEquals(HttpStatus.SC_OK, response.responseStatus) + assertEquals(HttpStatus.OK, response.responseStatus) assertEquals("{\"somejson\": \"for confusion\"}", response.body) } @Test fun `put returning void has no_content and empty body`() { val response = client.call(PUT, WebRequest("responseentity/put-returns-void"), userName, password) - assertEquals(HttpStatus.SC_NO_CONTENT, response.responseStatus) + assertEquals(HttpStatus.NO_CONTENT, response.responseStatus) assertEquals("", response.body) } @Test fun `async api returning status code accepted`() { val response = client.call(DELETE, WebRequest("responseentity/async-delete-returns-accepted"), userName, password) - assertEquals(HttpStatus.SC_ACCEPTED, response.responseStatus) + assertEquals(HttpStatus.ACCEPTED, response.responseStatus) assertEquals("\"DELETING\"", response.body) } @Test fun `post returns a string and has status CREATED with the string in the response body`() { val response = client.call(POST, WebRequest("responseentity/post-returns-created-string-json"), userName, password) - assertEquals(HttpStatus.SC_CREATED, response.responseStatus) + assertEquals(HttpStatus.CREATED, response.responseStatus) assertEquals("{\"somejson\": \"for confusion\"}", response.body) // Validate OpenAPI val apiSpec = client.call(HttpVerb.GET, WebRequest("swagger.json")) - assertEquals(HttpStatus.SC_OK, apiSpec.responseStatus) + assertEquals(HttpStatus.OK, apiSpec.responseStatus) val openAPI = Json.mapper().readValue(apiSpec.body, OpenAPI::class.java) with(openAPI.paths["/responseentity/post-returns-created-string-json"]) { assertNotNull(this) - val createdResponse = assertNotNull(this.post.responses[HttpStatus.SC_CREATED.toString()]) + val createdResponse = assertNotNull(this.post.responses[HttpStatus.CREATED.toString()]) assertThat(createdResponse.description).isEqualTo("Description of postReturnsCreatedWithEscapedJson") } } diff --git a/libs/rest/rest-test-common/build.gradle b/libs/rest/rest-test-common/build.gradle index 30ae36c1d2c..4f2f1e7ac7a 100644 --- a/libs/rest/rest-test-common/build.gradle +++ b/libs/rest/rest-test-common/build.gradle @@ -16,6 +16,8 @@ dependencies { implementation libs.jackson.module.kotlin implementation libs.nimbus.sdk implementation libs.unirest.java + // Add Gson as this was the default in 3.X, but not packed in 4.X. Gson would have been chosen by default in TestHttpClientUnirestImpl + implementation libs.unirest.objectmapper.gson implementation libs.mockito.kotlin diff --git a/libs/rest/rest-test-common/src/main/kotlin/net/corda/rest/test/utils/TestHttpClient.kt b/libs/rest/rest-test-common/src/main/kotlin/net/corda/rest/test/utils/TestHttpClient.kt index 976d13c63bc..50128e9dbc4 100644 --- a/libs/rest/rest-test-common/src/main/kotlin/net/corda/rest/test/utils/TestHttpClient.kt +++ b/libs/rest/rest-test-common/src/main/kotlin/net/corda/rest/test/utils/TestHttpClient.kt @@ -1,17 +1,11 @@ package net.corda.rest.test.utils -import kong.unirest.HttpRequest -import kong.unirest.HttpRequestWithBody -import kong.unirest.MultipartBody -import kong.unirest.Unirest -import kong.unirest.apache.ApacheClient +import kong.unirest.core.HttpRequest +import kong.unirest.core.HttpRequestWithBody +import kong.unirest.core.MultipartBody +import kong.unirest.core.Unirest import net.corda.rest.tools.HttpVerb -import org.apache.http.conn.ssl.NoopHostnameVerifier -import org.apache.http.conn.ssl.TrustAllStrategy -import org.apache.http.impl.client.HttpClients -import org.apache.http.ssl.SSLContexts import java.io.InputStream -import javax.net.ssl.SSLContext data class TestClientFileUpload(val fileContent: InputStream, val fileName: String) @@ -35,11 +29,12 @@ interface TestHttpClient { } class TestHttpClientUnirestImpl(override val baseAddress: String, private val enableSsl: Boolean = false) : TestHttpClient { + init { + addSslParams() + } override fun call(verb: HttpVerb, webRequest: WebRequest, responseClass: Class, userName: String, password: String): WebResponse where R : Any { - addSslParams() - var request = when (verb) { HttpVerb.GET -> Unirest.get(baseAddress + webRequest.path).basicAuth(userName, password) HttpVerb.POST -> Unirest.post(baseAddress + webRequest.path).basicAuth(userName, password) @@ -76,8 +71,6 @@ class TestHttpClientUnirestImpl(override val baseAddress: String, private val en } private fun doCall(verb: HttpVerb, webRequest: WebRequest, encodeAuth: HttpRequest<*>.() -> Unit): WebResponse { - addSslParams() - val path = baseAddress + webRequest.path var request: HttpRequest<*> = when (verb) { HttpVerb.GET -> Unirest.get(path) @@ -194,19 +187,8 @@ class TestHttpClientUnirestImpl(override val baseAddress: String, private val en } private fun addSslParams() { - if (enableSsl) { - val sslContext: SSLContext = SSLContexts.custom() - .loadTrustMaterial(TrustAllStrategy()) - .build() - - val httpClient = HttpClients.custom() - .setSSLContext(sslContext) - .setSSLHostnameVerifier(NoopHostnameVerifier()) - .build() - - Unirest.config().let { config -> - config.httpClient(ApacheClient.builder(httpClient).apply(config)) - } + if (!enableSsl) { + Unirest.config().verifySsl(false) } } } diff --git a/testing/e2e-test-utilities/build.gradle b/testing/e2e-test-utilities/build.gradle index 739ff7c3680..33b78993769 100644 --- a/testing/e2e-test-utilities/build.gradle +++ b/testing/e2e-test-utilities/build.gradle @@ -17,6 +17,9 @@ dependencies { implementation libs.jackson.module.kotlin implementation libs.jackson.datatype.jsr310 implementation libs.unirest.java + // Add Gson as this was the default in 3.X, but not packed in 4.X. + // Object mapper was never overridden previous to jackson OnjectMapper UnirestHttpsClient in this module + implementation libs.unirest.objectmapper.gson implementation libs.typesafe.config implementation "org.apache.commons:commons-text:$commonsTextVersion" implementation libs.assertj.core diff --git a/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/UnirestHttpsClient.kt b/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/UnirestHttpsClient.kt index c4f9c475034..20803fae6d4 100644 --- a/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/UnirestHttpsClient.kt +++ b/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/UnirestHttpsClient.kt @@ -1,18 +1,11 @@ package net.corda.e2etest.utilities -import kong.unirest.Headers -import kong.unirest.MultipartBody -import kong.unirest.Unirest -import kong.unirest.apache.ApacheClient +import kong.unirest.core.Headers +import kong.unirest.core.MultipartBody +import kong.unirest.core.Unirest import net.corda.tracing.addTraceContextToHttpRequest -import org.apache.http.client.config.RequestConfig -import org.apache.http.conn.ssl.NoopHostnameVerifier -import org.apache.http.conn.ssl.TrustAllStrategy -import org.apache.http.impl.client.HttpClients -import org.apache.http.ssl.SSLContexts import java.net.URI import java.net.http.HttpRequest -import javax.net.ssl.SSLContext class UnirestHttpsClient(private val endpoint: URI, private val username: String, private val password: String) : HttpsClient { @@ -23,12 +16,11 @@ class UnirestHttpsClient(private val endpoint: URI, private val username: String } init { - addSslParams() + configureUnirest() } override fun postMultiPart(cmd: String, fields: Map, files: Map): SimpleResponse { val url = endpoint.resolve(cmd).toURL().toString() - val response = Unirest.post(url) .basicAuth(username, password) .multiPartContent() @@ -99,27 +91,12 @@ class UnirestHttpsClient(private val endpoint: URI, private val username: String return SimpleResponse(response.status, response.body, url, response.headers.toInternal()) } - private fun addSslParams() { - val sslContext: SSLContext = SSLContexts.custom() - .loadTrustMaterial(TrustAllStrategy()) - .build() - - val requestConfig = RequestConfig.custom() - .setConnectionRequestTimeout(60_000) - .setConnectTimeout(60_000) - .setSocketTimeout(60_000) - .build() - - val httpClient = HttpClients.custom() - .setSSLContext(sslContext) - .setSSLHostnameVerifier(NoopHostnameVerifier()) - .setDefaultRequestConfig(requestConfig) - .build() - + private fun configureUnirest() { Unirest.config() - .let { config -> - config.httpClient(ApacheClient.builder(httpClient).apply(config)) - } + .reset() + .verifySsl(false) + .requestTimeout(60000) + .connectTimeout(60000) } private fun MultipartBody.fields(fields: Map): MultipartBody { diff --git a/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/config/ConfigTestUtils.kt b/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/config/ConfigTestUtils.kt index ca79cb3885e..1c0375e952b 100644 --- a/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/config/ConfigTestUtils.kt +++ b/testing/e2e-test-utilities/src/main/kotlin/net/corda/e2etest/utilities/config/ConfigTestUtils.kt @@ -1,7 +1,7 @@ package net.corda.e2etest.utilities.config import com.fasterxml.jackson.databind.JsonNode -import kong.unirest.UnirestException +import kong.unirest.core.UnirestException import net.corda.e2etest.utilities.ClusterInfo import net.corda.e2etest.utilities.ClusterReadinessChecker import net.corda.e2etest.utilities.DEFAULT_CLUSTER diff --git a/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt b/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt index bb06dd4bb92..0ae5ddda51c 100644 --- a/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt +++ b/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt @@ -22,16 +22,14 @@ class QueriesTasksTest : FunctionalBaseTest() { fun listVNodesFailsConnectionRefused() { appendCordaRuntimeGradlePluginExtension() val result = executeAndFailWithRunner(LIST_VNODES_TASK_NAME) - assertTrue(result.output.contains("Connect to $restHostnameWithPort")) - assertTrue(result.output.contains("Connection refused")) + assertTrue(result.output.contains("java.net.ConnectException"), "Output was: [${result.output}]") } @Test fun listCPIsFailsConnectionRefused() { appendCordaRuntimeGradlePluginExtension() val result = executeAndFailWithRunner(LIST_CPIS_TASK_NAME) - assertTrue(result.output.contains("Connect to $restHostnameWithPort")) - assertTrue(result.output.contains("Connection refused")) + assertTrue(result.output.contains("java.net.ConnectException"), "Output was: [${result.output}]") } @Test diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/ProjectUtils.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/ProjectUtils.kt index 3026f9098e8..1796e378987 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/ProjectUtils.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/ProjectUtils.kt @@ -2,9 +2,9 @@ package net.corda.gradle.plugin import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.ObjectMapper -import kong.unirest.HttpResponse -import kong.unirest.JsonNode -import kong.unirest.Unirest +import kong.unirest.core.HttpResponse +import kong.unirest.core.JsonNode +import kong.unirest.core.Unirest import net.corda.gradle.plugin.configuration.ProjectContext import net.corda.gradle.plugin.dtos.VirtualNodeInfoDTO import net.corda.gradle.plugin.dtos.VirtualNodesDTO diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt index e1d3d0e2605..82657c11e85 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt @@ -1,6 +1,6 @@ package net.corda.gradle.plugin.cordalifecycle -import kong.unirest.Unirest +import kong.unirest.core.Unirest import net.corda.gradle.plugin.exception.CordaRuntimeGradlePluginException import net.corda.gradle.plugin.retryAttempts import java.io.File diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt index 3ca40fbe560..c32594e7f58 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt @@ -1,6 +1,6 @@ package net.corda.gradle.plugin.cordalifecycle -import kong.unirest.Unirest +import kong.unirest.core.Unirest import net.corda.gradle.plugin.configuration.PluginConfiguration import net.corda.gradle.plugin.configuration.ProjectContext import org.gradle.api.DefaultTask diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordapp/DeployCpiHelper.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordapp/DeployCpiHelper.kt index 1f35ece865f..cbdec10c1de 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordapp/DeployCpiHelper.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordapp/DeployCpiHelper.kt @@ -2,9 +2,9 @@ package net.corda.gradle.plugin.cordapp import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.ObjectMapper -import kong.unirest.HttpResponse -import kong.unirest.JsonNode -import kong.unirest.Unirest +import kong.unirest.core.HttpResponse +import kong.unirest.core.JsonNode +import kong.unirest.core.Unirest import net.corda.gradle.plugin.dtos.CpiUploadResponseDTO import net.corda.gradle.plugin.dtos.CpiUploadStatus import net.corda.gradle.plugin.dtos.GetCPIsResponseDTO diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/network/VNodeHelper.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/network/VNodeHelper.kt index 058a3a4a0e2..a9b540653ee 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/network/VNodeHelper.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/network/VNodeHelper.kt @@ -3,9 +3,9 @@ package net.corda.gradle.plugin.network import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.ObjectMapper -import kong.unirest.HttpResponse -import kong.unirest.JsonNode -import kong.unirest.Unirest +import kong.unirest.core.HttpResponse +import kong.unirest.core.JsonNode +import kong.unirest.core.Unirest import net.corda.gradle.plugin.dtos.CpiUploadStatus import net.corda.gradle.plugin.dtos.GetCPIsResponseDTO import net.corda.gradle.plugin.dtos.RegistrationRequestProgressDTO diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/queries/QueryTasksImpl.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/queries/QueryTasksImpl.kt index 9ca91e354a9..b8d671d9195 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/queries/QueryTasksImpl.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/queries/QueryTasksImpl.kt @@ -2,9 +2,9 @@ package net.corda.gradle.plugin.queries import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.ObjectMapper -import kong.unirest.HttpResponse -import kong.unirest.JsonNode -import kong.unirest.Unirest +import kong.unirest.core.HttpResponse +import kong.unirest.core.JsonNode +import kong.unirest.core.Unirest import net.corda.gradle.plugin.configuration.ProjectContext import net.corda.gradle.plugin.dtos.CpiMetadataDTO import net.corda.gradle.plugin.dtos.GetCPIsResponseDTO From 4d0bdefbfe9d7dbfa728f3102f7b9879612f5c03 Mon Sep 17 00:00:00 2001 From: LWogan Date: Mon, 2 Dec 2024 10:52:41 +0000 Subject: [PATCH 18/19] CORE-XXXX: detekt --- .../corda/flow/rest/impl/v1/FlowRestResourceImpl.kt | 1 - .../corda/rest/server/impl/AbstractWebsocketTest.kt | 3 +-- .../corda/rest/server/impl/RestServerRequestsTest.kt | 10 +++------- .../corda/gradle/plugin/queries/QueriesTasksTest.kt | 1 - .../plugin/cordalifecycle/EnvironmentSetupHelper.kt | 3 +-- .../plugin/cordalifecycle/EnvironmentSetupTasks.kt | 1 - 6 files changed, 5 insertions(+), 14 deletions(-) diff --git a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt index c596402bf23..8f83c2536df 100644 --- a/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt +++ b/components/flow/flow-rest-resource-service-impl/src/main/kotlin/net/corda/flow/rest/impl/v1/FlowRestResourceImpl.kt @@ -42,7 +42,6 @@ import net.corda.rest.messagebus.MessageBusUtils.tryWithExceptionHandling import net.corda.rest.response.ResponseEntity import net.corda.rest.security.CURRENT_REST_CONTEXT import net.corda.schema.Schemas.Flow.FLOW_MAPPER_START -import net.corda.schema.Schemas.Flow.FLOW_STATUS_TOPIC import net.corda.schema.configuration.MessagingConfig import net.corda.tracing.TraceTag import net.corda.tracing.addTraceContextToRecord diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt index 4b3fd46b012..17bd07d183e 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/AbstractWebsocketTest.kt @@ -1,8 +1,7 @@ package net.corda.rest.server.impl -import io.javalin.core.util.Header -import kong.unirest.core.HttpStatus import io.javalin.http.Header +import kong.unirest.core.HttpStatus import net.corda.rest.server.config.models.RestServerSettings import net.corda.rest.test.utils.WebRequest import net.corda.rest.tools.HttpVerb diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt index ff7fe9f7bfc..0ea5533fa57 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt @@ -1,11 +1,7 @@ package net.corda.rest.server.impl import com.google.gson.Gson -import io.javalin.core.util.Header -import io.javalin.http.Header.ACCESS_CONTROL_ALLOW_CREDENTIALS -import io.javalin.http.Header.ACCESS_CONTROL_ALLOW_ORIGIN -import io.javalin.http.Header.CACHE_CONTROL -import io.javalin.http.Header.WWW_AUTHENTICATE +import io.javalin.http.Header import kong.unirest.core.HttpStatus import net.corda.rest.annotations.RestApiVersion import net.corda.rest.server.apigen.test.TestJavaPrimitivesRestResourceImpl @@ -122,7 +118,7 @@ class RestServerRequestsTest : RestServerTestBase() { userName, password ) - assertEquals(HttpStatus.SC_OK, sanityResponse.responseStatus) + assertEquals(HttpStatus.OK, sanityResponse.responseStatus) assertEquals("Sane", sanityResponse.body) } @@ -404,7 +400,7 @@ class RestServerRequestsTest : RestServerTestBase() { GET, WebRequest("health/sanity") ) - val headerValue = getPathResponse.headers[WWW_AUTHENTICATE.lowercase()] + val headerValue = getPathResponse.headers[Header.WWW_AUTHENTICATE.lowercase()] assertEquals("Basic realm=\"${UsernamePasswordAuthenticationProvider.REALM_VALUE}\"", headerValue) } diff --git a/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt b/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt index 061c63ed152..c7afd41ed3b 100644 --- a/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt +++ b/tools/corda-runtime-gradle-plugin/src/integrationTest/kotlin/net/corda/gradle/plugin/queries/QueriesTasksTest.kt @@ -8,7 +8,6 @@ import net.corda.libs.virtualnode.endpoints.v1.types.HoldingIdentity import net.corda.libs.virtualnode.endpoints.v1.types.VirtualNodeInfo import net.corda.libs.virtualnode.endpoints.v1.types.VirtualNodes import net.corda.virtualnode.OperationalStatus -import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt index 2a84353f49a..282d20b6175 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupHelper.kt @@ -1,7 +1,5 @@ package net.corda.gradle.plugin.cordalifecycle -import kong.unirest.core.Unirest -import net.corda.sdk.network.config.NetworkConfig import net.corda.gradle.plugin.exception.CordaRuntimeGradlePluginException import net.corda.gradle.plugin.retryAttempts import net.corda.restclient.CordaRestClient @@ -9,6 +7,7 @@ import net.corda.restclient.generated.models.ConfigSchemaVersion import net.corda.restclient.generated.models.UpdateConfigParameters import net.corda.schema.configuration.ConfigKeys.RootConfigKey import net.corda.sdk.config.ClusterConfig +import net.corda.sdk.network.config.NetworkConfig import java.io.File import java.net.Authenticator import java.net.PasswordAuthentication diff --git a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt index 7a6f4faa1b2..d6d46a38f05 100644 --- a/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt +++ b/tools/corda-runtime-gradle-plugin/src/main/kotlin/net/corda/gradle/plugin/cordalifecycle/EnvironmentSetupTasks.kt @@ -1,6 +1,5 @@ package net.corda.gradle.plugin.cordalifecycle -import kong.unirest.core.Unirest import net.corda.gradle.plugin.configuration.PluginConfiguration import net.corda.gradle.plugin.configuration.ProjectContext import net.corda.schema.configuration.ConfigKeys.RootConfigKey From 9c1aca92d539c210e1f1535a9e4090065175a095 Mon Sep 17 00:00:00 2001 From: LWogan Date: Mon, 2 Dec 2024 12:57:52 +0000 Subject: [PATCH 19/19] CORE-XXXX: update test --- .../kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt index 0ea5533fa57..c7009ed14a9 100644 --- a/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt +++ b/libs/rest/rest-server-impl/src/integrationTest/kotlin/net/corda/rest/server/impl/RestServerRequestsTest.kt @@ -271,7 +271,7 @@ class RestServerRequestsTest : RestServerTestBase() { assertEquals("3", helloResponse.body) // Check that the response returned a deprecation warning in the header - assertThat(helloResponse.headers.toMap()["Warning"]!!.contains("299")) + assertThat(helloResponse.headers.toMap()["warning"]!!.contains("299")) // Check that security managed has not been called for GetProtocolVersion which is exempt from permissions check assertThat(securityManager.checksExecuted).hasSize(0)