From 9b94fa97699b5210a19403c01cf77069eec8ec1c Mon Sep 17 00:00:00 2001 From: nicu-da Date: Mon, 8 Nov 2021 06:45:35 -0800 Subject: [PATCH] [ledger-api] - metadata for invalid deduplication period [KVL-1170] (#11534) CHANGELOG_BEGIN [ledger-api] - Return max_deduplication_duration as part of the metadata sent with the gRPC status for commands rejected because of invalid deduplication periods CHANGELOG_END --- .../error/definitions/LedgerApiErrors.scala | 9 +++++++-- ledger/ledger-api-common/BUILD.bazel | 2 ++ .../server/api/validation/ErrorFactories.scala | 4 +++- .../api/validation/FieldValidations.scala | 13 +++++++------ .../api/validation/ValidatorTestUtils.scala | 17 +++++++++++++++-- .../validation/SubmitRequestValidatorTest.scala | 3 +++ .../api/validation/ErrorFactoriesSpec.scala | 7 +++++-- 7 files changed, 42 insertions(+), 13 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index aa4bfeb8567c..c8ba647fbae1 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -3,6 +3,8 @@ package com.daml.error.definitions +import java.time.Duration + import com.daml.error._ import com.daml.error.definitions.ErrorGroups.ParticipantErrorGroup.TransactionErrorGroup.LedgerApiErrorGroup import com.daml.lf.data.Ref @@ -295,11 +297,14 @@ object LedgerApiErrors extends LedgerApiErrorGroup { id = "INVALID_DEDUPLICATION_PERIOD", ErrorCategory.InvalidGivenCurrentSystemStateOther, ) { - case class Reject(_reason: String)(implicit + case class Reject(_reason: String, _maxDeduplicationDuration: Duration)(implicit loggingContext: ContextualizedErrorLogger ) extends LoggingTransactionErrorImpl( cause = s"The submitted command had an invalid deduplication period: ${_reason}" - ) + ) { + override def context: Map[String, String] = + super.context + ("max_deduplication_duration" -> _maxDeduplicationDuration.toString) + } } } diff --git a/ledger/ledger-api-common/BUILD.bazel b/ledger/ledger-api-common/BUILD.bazel index f59120241aed..73d6d2c3e9e9 100644 --- a/ledger/ledger-api-common/BUILD.bazel +++ b/ledger/ledger-api-common/BUILD.bazel @@ -79,6 +79,8 @@ da_scala_library( "//language-support/scala/bindings", "//ledger/ledger-api-domain", "//libs-scala/concurrent", + "//libs-scala/grpc-utils", + "@maven//:com_google_api_grpc_proto_google_common_protos", "@maven//:org_scalatest_scalatest_compatible", ], ) diff --git a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala index 169fd59cea2d..5f4f9cda3f08 100644 --- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala +++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala @@ -4,6 +4,7 @@ package com.daml.platform.server.api.validation import java.sql.{SQLNonTransientException, SQLTransientException} +import java.time.Duration import com.daml.error.ErrorCode.ApiException import com.daml.error.definitions.{IndexErrors, LedgerApiErrors} @@ -231,11 +232,12 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch fieldName: String, message: String, definiteAnswer: Option[Boolean], + maxDeduplicationDuration: Duration, )(implicit contextualizedErrorLogger: ContextualizedErrorLogger): StatusRuntimeException = errorCodesVersionSwitcher.choose( legacyInvalidField(fieldName, message, definiteAnswer), LedgerApiErrors.CommandValidation.InvalidDeduplicationPeriodField - .Reject(message) + .Reject(message, maxDeduplicationDuration) .asGrpcError, ) diff --git a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala index 20a50b060db1..bf77612ec907 100644 --- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala +++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala @@ -144,24 +144,25 @@ class FieldValidations private (errorFactories: ErrorFactories) { */ def validateDeduplicationPeriod( deduplicationPeriod: DeduplicationPeriodProto, - optMaxDeduplicationTime: Option[Duration], + optMaxDeduplicationDuration: Option[Duration], fieldName: String, )(implicit contextualizedErrorLogger: ContextualizedErrorLogger ): Either[StatusRuntimeException, DeduplicationPeriod] = { - optMaxDeduplicationTime.fold[Either[StatusRuntimeException, DeduplicationPeriod]]( + optMaxDeduplicationDuration.fold[Either[StatusRuntimeException, DeduplicationPeriod]]( Left(missingLedgerConfig(definiteAnswer = Some(false))) - )(maxDeduplicationTime => { + )(maxDeduplicationDuration => { def validateDuration(duration: Duration, exceedsMaxDurationMessage: String) = { if (duration.isNegative) Left(invalidField(fieldName, "Duration must be positive", definiteAnswer = Some(false))) - else if (duration.compareTo(maxDeduplicationTime) > 0) + else if (duration.compareTo(maxDeduplicationDuration) > 0) Left( invalidDeduplicationDuration( fieldName, exceedsMaxDurationMessage, definiteAnswer = Some(false), + maxDeduplicationDuration, ) ) else Right(duration) @@ -171,13 +172,13 @@ class FieldValidations private (errorFactories: ErrorFactories) { val result = DurationConversion.fromProto(duration) validateDuration( result, - s"The given deduplication duration of $result exceeds the maximum deduplication time of $maxDeduplicationTime", + s"The given deduplication duration of $result exceeds the maximum deduplication time of $maxDeduplicationDuration", ).map(DeduplicationPeriod.DeduplicationDuration) } deduplicationPeriod match { case DeduplicationPeriodProto.Empty => - Right(DeduplicationPeriod.DeduplicationDuration(maxDeduplicationTime)) + Right(DeduplicationPeriod.DeduplicationDuration(maxDeduplicationDuration)) case DeduplicationPeriodProto.DeduplicationTime(duration) => protoDurationToDurationPeriod(duration) case DeduplicationPeriodProto.DeduplicationDuration(duration) => diff --git a/ledger/ledger-api-common/src/test/lib/scala/com/digitalasset/ledger/api/validation/ValidatorTestUtils.scala b/ledger/ledger-api-common/src/test/lib/scala/com/digitalasset/ledger/api/validation/ValidatorTestUtils.scala index 537bd0754ef8..530b0aba0ac2 100644 --- a/ledger/ledger-api-common/src/test/lib/scala/com/digitalasset/ledger/api/validation/ValidatorTestUtils.scala +++ b/ledger/ledger-api-common/src/test/lib/scala/com/digitalasset/ledger/api/validation/ValidatorTestUtils.scala @@ -3,9 +3,11 @@ package com.daml.ledger.api.validation +import com.daml.grpc.GrpcStatus import com.daml.ledger.api.domain import com.daml.ledger.api.messages.transaction import com.daml.lf.data.Ref +import com.google.rpc.error_details import io.grpc.Status.Code import io.grpc.StatusRuntimeException import org.scalatest._ @@ -40,16 +42,19 @@ trait ValidatorTestUtils extends Matchers with Inside with OptionValues { self: expectedDescriptionV1: String, expectedCodeV2: Code, expectedDescriptionV2: String, + metadataV2: Map[String, String] = Map.empty, ): Assertion = { requestMustFailWith( request = testedRequest(testedFactory(false)), code = expectedCodeV1, description = expectedDescriptionV1, + metadata = Map.empty[String, String], ) requestMustFailWith( request = testedRequest(testedFactory(true)), code = expectedCodeV2, description = expectedDescriptionV2, + metadataV2, ) } @@ -85,24 +90,32 @@ trait ValidatorTestUtils extends Matchers with Inside with OptionValues { self: request: Future[_], code: Code, description: String, + metadata: Map[String, String], ): Future[Assertion] = { val f = request.map(Right(_)).recover { case ex: StatusRuntimeException => Left(ex) } - f.map(inside(_)(isError(code, description))) + f.map(inside(_)(isError(code, description, metadata))) } protected def requestMustFailWith( request: Either[StatusRuntimeException, _], code: Code, description: String, + metadata: Map[String, String], ): Assertion = { - inside(request)(isError(code, description)) + inside(request)(isError(code, description, metadata)) } protected def isError( expectedCode: Code, expectedDescription: String, + metadata: Map[String, String], ): PartialFunction[Either[StatusRuntimeException, _], Assertion] = { case Left(err) => err.getStatus should have(Symbol("code")(expectedCode)) err.getStatus should have(Symbol("description")(expectedDescription)) + GrpcStatus + .toProto(err.getStatus, err.getTrailers) + .details + .flatMap(_.unpack[error_details.ErrorInfo].metadata) + .toMap should contain allElementsOf metadata } } diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala index 14637ebf6ed6..d97abcb9ec80 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala @@ -418,6 +418,9 @@ class SubmitRequestValidatorTest expectedCodeV2 = FAILED_PRECONDITION, expectedDescriptionV2 = s"INVALID_DEDUPLICATION_PERIOD(9,0): The submitted command had an invalid deduplication period: The given deduplication duration of ${java.time.Duration .ofSeconds(durationSecondsExceedingMax)} exceeds the maximum deduplication time of ${internal.maxDeduplicationDuration}", + metadataV2 = Map( + "max_deduplication_duration" -> internal.maxDeduplicationDuration.toString + ), ) } } diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 0f2777a76af3..6bef5214d16f 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -22,8 +22,9 @@ import org.mockito.MockitoSugar import org.scalatest.matchers.should.Matchers import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.wordspec.AnyWordSpec - import java.sql.{SQLNonTransientException, SQLTransientException} +import java.time.Duration + import scala.jdk.CollectionConverters._ class ErrorFactoriesSpec @@ -316,7 +317,9 @@ class ErrorFactoriesSpec "return an invalid deduplication period error" in { val errorDetailMessage = "message" val field = "field" - assertVersionedError(_.invalidDeduplicationDuration(field, errorDetailMessage, None))( + assertVersionedError( + _.invalidDeduplicationDuration(field, errorDetailMessage, None, Duration.ofSeconds(5)) + )( v1_code = Code.INVALID_ARGUMENT, v1_message = s"Invalid field $field: $errorDetailMessage", v1_details = Seq.empty,