[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
digital-asset · Nov 8, 2021 · 9b94fa9 · 9b94fa9
1 parent e7eb60f
commit 9b94fa9
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 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
@@ -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
@@ -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/...ommon/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala b/...ommon/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/...mon/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala b/...mon/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/...common/src/test/lib/scala/com/digitalasset/ledger/api/validation/ValidatorTestUtils.scala b/...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/.../test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala b/.../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/...test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/...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,