From 68e6ec6a71ee11e0e85824741386df210ee09110 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Wed, 20 Oct 2021 15:33:41 +0200 Subject: [PATCH 1/5] [DPP-645][Self-service error codes] Adapt ApiConfigManagementService --- .../error/definitions/LedgerApiErrors.scala | 16 + .../api/validation/ErrorFactories.scala | 9 + .../api/validation/ErrorFactoriesSpec.scala | 15 + .../platform/apiserver/ApiServices.scala | 1 + .../admin/ApiConfigManagementService.scala | 28 +- .../ApiConfigManagementServiceSpec.scala | 308 ++++++++++-------- 6 files changed, 226 insertions(+), 151 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 25fe24578d77..7fae8a7c18fd 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 @@ -32,6 +32,22 @@ object LedgerApiErrors extends LedgerApiErrorGroup { ) } + object WriteErrors extends ErrorGroup() { + @Explanation("This rejection is given when a configuration entry write was rejected.") + @Resolution("Fetch newest configuration and/or retry.") + object ConfigurationEntryRejected + extends ErrorCode( + id = "CONFIGURATION_ENTRY_REJECTED", + ErrorCategory.InvalidGivenCurrentSystemStateOther, + ) { + case class Reject(message: String)(implicit + loggingContext: ContextualizedErrorLogger + ) extends LoggingTransactionErrorImpl( + cause = message + ) + } + } + object ReadErrors extends ErrorGroup() { @Explanation("This rejection is given when a read request tries to access pruned data.") @Resolution("Use an offset that is after the pruning offset.") 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 6d7ea01906b6..e46abfdbe93f 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 @@ -166,6 +166,15 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch grpcError(statusBuilder.build()) } + def configurationEntryRejected(message: String, definiteAnswer: Option[Boolean])(implicit + contextualizedErrorLogger: ContextualizedErrorLogger + ): StatusRuntimeException = { + errorCodesVersionSwitcher.choose( + v1 = aborted(message, definiteAnswer), + v2 = LedgerApiErrors.WriteErrors.ConfigurationEntryRejected.Reject(message).asGrpcError, + ) + } + // permission denied is intentionally without description to ensure we don't leak security relevant information by accident def permissionDenied()(implicit contextualizedErrorLogger: ContextualizedErrorLogger 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 26cd1a25d3ab..23d634226aa6 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 @@ -35,6 +35,21 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope ErrorDetails.RequestInfoDetail("trace-id") "ErrorFactories" should { + + "return the configurationEntryRejected" in { + assertVersionedError(_.configurationEntryRejected("message123", None))( + v1_code = Code.ABORTED, + v1_message = "message123", + v1_details = Seq.empty, + v2_code = Code.FAILED_PRECONDITION, + v2_message = s"CONFIGURATION_ENTRY_REJECTED(9,$correlationId): message123", + v2_details = Seq[ErrorDetails.ErrorDetail]( + ErrorDetails.ErrorInfoDetail("CONFIGURATION_ENTRY_REJECTED"), + DefaultTraceIdRequestInfo, + ), + ) + } + "return the DuplicateCommandException" in { assertVersionedError(_.duplicateCommandException)( v1_code = Code.ALREADY_EXISTS, diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala index d3ccb7ce8593..c8f9d0e7a313 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala @@ -285,6 +285,7 @@ private[daml] object ApiServices { configManagementService, writeService, timeProvider, + errorsVersionsSwitcher, ) val apiParticipantPruningService = diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/admin/ApiConfigManagementService.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/admin/ApiConfigManagementService.scala index 647295c8d36b..964248b894f8 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/admin/ApiConfigManagementService.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/admin/ApiConfigManagementService.scala @@ -7,7 +7,11 @@ import java.time.{Duration => JDuration} import akka.stream.Materializer import akka.stream.scaladsl.Source import com.daml.api.util.{DurationConversion, TimeProvider, TimestampConversion} -import com.daml.error.{DamlContextualizedErrorLogger, ContextualizedErrorLogger} +import com.daml.error.{ + ContextualizedErrorLogger, + DamlContextualizedErrorLogger, + ErrorCodesVersionSwitcher, +} import com.daml.ledger.api.domain import com.daml.ledger.api.domain.{ConfigurationEntry, LedgerOffset} import com.daml.ledger.api.v1.admin.config_management_service.ConfigManagementServiceGrpc.ConfigManagementService @@ -36,6 +40,7 @@ private[apiserver] final class ApiConfigManagementService private ( writeService: state.WriteConfigService, timeProvider: TimeProvider, submissionIdGenerator: String => Ref.SubmissionId, + errorCodesVersionSwitcher: ErrorCodesVersionSwitcher, )(implicit materializer: Materializer, executionContext: ExecutionContext, @@ -46,6 +51,9 @@ private[apiserver] final class ApiConfigManagementService private ( private implicit val contextualizedErrorLogger: ContextualizedErrorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None) + private val errorFactories = + com.daml.platform.server.api.validation.ErrorFactories(errorCodesVersionSwitcher) + override def close(): Unit = () override def bindService(): ServerServiceDefinition = @@ -60,7 +68,7 @@ private[apiserver] final class ApiConfigManagementService private ( Future.successful(configurationToResponse(configuration)) case None => // TODO error codes: Duplicate of missingLedgerConfig - Future.failed(ErrorFactories.missingLedgerConfigUponRequest) + Future.failed(errorFactories.missingLedgerConfigUponRequest) } .andThen(logger.logErrorsOnCall[GetTimeModelResponse]) } @@ -106,7 +114,7 @@ private[apiserver] final class ApiConfigManagementService private ( logger.warn( "Could not get the current time model. The index does not yet have any ledger configuration." ) - Future.failed(ErrorFactories.missingLedgerConfig(None)) + Future.failed(errorFactories.missingLedgerConfig(None)) } (ledgerEndBeforeRequest, currentConfig) = configuration @@ -117,7 +125,7 @@ private[apiserver] final class ApiConfigManagementService private ( Future.failed( ValidationLogger.logFailureWithContext( request, - ErrorFactories.invalidArgument(None)( + errorFactories.invalidArgument(None)( s"Mismatching configuration generation, expected $expectedGeneration, received ${request.configurationGeneration}" ), ) @@ -139,6 +147,7 @@ private[apiserver] final class ApiConfigManagementService private ( writeService, index, ledgerEndBeforeRequest, + errorFactories, ), timeToLive = JDuration.ofMillis(params.timeToLive.toMillis), ) @@ -176,7 +185,7 @@ private[apiserver] final class ApiConfigManagementService private ( minSkew = DurationConversion.fromProto(pMinSkew), maxSkew = DurationConversion.fromProto(pMaxSkew), ) match { - case Failure(err) => Left(ErrorFactories.invalidArgument(None)(err.toString)) + case Failure(err) => Left(errorFactories.invalidArgument(None)(err.toString)) case Success(ok) => Right(ok) } // TODO(JM): The maximum record time should be constrained, probably by the current active time model? @@ -189,7 +198,7 @@ private[apiserver] final class ApiConfigManagementService private ( } maximumRecordTime <- Time.Timestamp .fromInstant(mrtInstant) - .fold(err => Left(ErrorFactories.invalidArgument(None)(err)), Right(_)) + .fold(err => Left(errorFactories.invalidArgument(None)(err)), Right(_)) } yield SetTimeModelParameters(newTimeModel, maximumRecordTime, timeToLive) } @@ -201,6 +210,7 @@ private[apiserver] object ApiConfigManagementService { readBackend: IndexConfigManagementService, writeBackend: state.WriteConfigService, timeProvider: TimeProvider, + errorCodesVersionSwitcher: ErrorCodesVersionSwitcher, submissionIdGenerator: String => Ref.SubmissionId = augmentSubmissionId, )(implicit materializer: Materializer, @@ -212,13 +222,15 @@ private[apiserver] object ApiConfigManagementService { writeBackend, timeProvider, submissionIdGenerator, + errorCodesVersionSwitcher, ) private final class SynchronousResponseStrategy( writeConfigService: state.WriteConfigService, configManagementService: IndexConfigManagementService, ledgerEnd: LedgerOffset.Absolute, - )(implicit loggingContext: LoggingContext) + errorFactories: ErrorFactories, + )(implicit loggingContext: LoggingContext, contextualizedErrorLogger: ContextualizedErrorLogger) extends SynchronousResponse.Strategy[ (Time.Timestamp, Configuration), ConfigurationEntry, @@ -252,7 +264,7 @@ private[apiserver] object ApiConfigManagementService { submissionId: Ref.SubmissionId ): PartialFunction[ConfigurationEntry, StatusRuntimeException] = { case domain.ConfigurationEntry.Rejected(`submissionId`, reason, _) => - ErrorFactories.aborted(reason, None) + errorFactories.configurationEntryRejected(reason, None) } } diff --git a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala index 8d2f7aaee8be..ce004a9ff594 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala @@ -7,11 +7,11 @@ import java.time.Duration import java.util.concurrent.CompletableFuture.completedFuture import java.util.concurrent.CompletionStage import java.util.concurrent.atomic.{AtomicLong, AtomicReference} - import akka.NotUsed import akka.stream.Materializer import akka.stream.scaladsl.Source import com.daml.api.util.TimeProvider +import com.daml.error.ErrorCodesVersionSwitcher import com.daml.grpc.{GrpcException, GrpcStatus} import com.daml.ledger.api.domain.{ConfigurationEntry, LedgerOffset} import com.daml.ledger.api.testing.utils.AkkaBeforeAndAfterAll @@ -53,166 +53,188 @@ class ApiConfigManagementServiceSpec private implicit val loggingContext: LoggingContext = LoggingContext.ForTesting - "ApiConfigManagementService" should { - "get the time model" in { - val indexedTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(5), - minSkew = Duration.ofMinutes(3), - maxSkew = Duration.ofMinutes(2), - ).get - val expectedTimeModel = TimeModel.of( - avgTransactionLatency = Some(DurationProto.of(5 * 60, 0)), - minSkew = Some(DurationProto.of(3 * 60, 0)), - maxSkew = Some(DurationProto.of(2 * 60, 0)), - ) + addTests(true) + addTests(false) + + def addTests(useSelfServiceErrorCodes: Boolean): Unit = { + val suffix = s"(enableSelfServiceErrorCodes=${useSelfServiceErrorCodes})" + val errorCodesVersionSwitcher = new ErrorCodesVersionSwitcher(useSelfServiceErrorCodes) + + s"ApiConfigManagementService $suffix" should { + "get the time model" in { + val indexedTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(5), + minSkew = Duration.ofMinutes(3), + maxSkew = Duration.ofMinutes(2), + ).get + val expectedTimeModel = TimeModel.of( + avgTransactionLatency = Some(DurationProto.of(5 * 60, 0)), + minSkew = Some(DurationProto.of(3 * 60, 0)), + maxSkew = Some(DurationProto.of(2 * 60, 0)), + ) - val writeService = mock[state.WriteConfigService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - new FakeCurrentIndexConfigManagementService( - LedgerOffset.Absolute(Ref.LedgerString.assertFromString("0")), - Configuration(aConfigurationGeneration, indexedTimeModel, Duration.ZERO), - ), - writeService, - TimeProvider.UTC, - ) + val writeService = mock[state.WriteConfigService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + new FakeCurrentIndexConfigManagementService( + LedgerOffset.Absolute(Ref.LedgerString.assertFromString("0")), + Configuration(aConfigurationGeneration, indexedTimeModel, Duration.ZERO), + ), + writeService, + TimeProvider.UTC, + errorCodesVersionSwitcher, + ) - apiConfigManagementService.getTimeModel(GetTimeModelRequest.defaultInstance).map { response => - response.timeModel should be(Some(expectedTimeModel)) - verifyZeroInteractions(writeService) - succeed + apiConfigManagementService.getTimeModel(GetTimeModelRequest.defaultInstance).map { + response => + response.timeModel should be(Some(expectedTimeModel)) + verifyZeroInteractions(writeService) + succeed + } } - } - "return a `NOT_FOUND` error if a time model is not found" in { - val writeService = mock[state.WriteConfigService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - EmptyIndexConfigManagementService, - writeService, - TimeProvider.UTC, - ) + "return a `NOT_FOUND` error if a time model is not found" in { + val writeService = mock[state.WriteConfigService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + EmptyIndexConfigManagementService, + writeService, + TimeProvider.UTC, + errorCodesVersionSwitcher, + ) - apiConfigManagementService - .getTimeModel(GetTimeModelRequest.defaultInstance) - .transform(Success.apply) - .map { response => - response should matchPattern { case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => } - } - } + apiConfigManagementService + .getTimeModel(GetTimeModelRequest.defaultInstance) + .transform(Success.apply) + .map { response => + response should matchPattern { case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => + } + } + } - "set a new time model" in { - val maximumDeduplicationTime = Duration.ofHours(6) - val initialGeneration = 2L - val initialTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(1), - minSkew = Duration.ofMinutes(2), - maxSkew = Duration.ofMinutes(3), - ).get - val initialConfiguration = Configuration( - generation = initialGeneration, - timeModel = initialTimeModel, - maxDeduplicationTime = maximumDeduplicationTime, - ) - val expectedGeneration = 3L - val expectedTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(2), - minSkew = Duration.ofMinutes(1), - maxSkew = Duration.ofSeconds(30), - ).get - val expectedConfiguration = Configuration( - generation = expectedGeneration, - timeModel = expectedTimeModel, - maxDeduplicationTime = maximumDeduplicationTime, - ) + "set a new time model" in { + val maximumDeduplicationTime = Duration.ofHours(6) + val initialGeneration = 2L + val initialTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(1), + minSkew = Duration.ofMinutes(2), + maxSkew = Duration.ofMinutes(3), + ).get + val initialConfiguration = Configuration( + generation = initialGeneration, + timeModel = initialTimeModel, + maxDeduplicationTime = maximumDeduplicationTime, + ) + val expectedGeneration = 3L + val expectedTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(2), + minSkew = Duration.ofMinutes(1), + maxSkew = Duration.ofSeconds(30), + ).get + val expectedConfiguration = Configuration( + generation = expectedGeneration, + timeModel = expectedTimeModel, + maxDeduplicationTime = maximumDeduplicationTime, + ) - val timeProvider = TimeProvider.UTC - val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) + val timeProvider = TimeProvider.UTC + val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) - val (indexService, writeService, currentConfiguration) = fakeServices( - startingOffset = 7, - submissions = Seq(Ref.SubmissionId.assertFromString("one") -> initialConfiguration), - ) - val apiConfigManagementService = ApiConfigManagementService.createApiService( - indexService, - writeService, - timeProvider, - ) + val (indexService, writeService, currentConfiguration) = fakeServices( + startingOffset = 7, + submissions = Seq(Ref.SubmissionId.assertFromString("one") -> initialConfiguration), + ) + val apiConfigManagementService = ApiConfigManagementService.createApiService( + indexService, + writeService, + timeProvider, + errorCodesVersionSwitcher, + ) - apiConfigManagementService - .setTimeModel( - SetTimeModelRequest.of( - "some submission ID", - maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), - configurationGeneration = initialGeneration, - newTimeModel = Some( - TimeModel( - avgTransactionLatency = Some(DurationProto.of(2 * 60, 0)), - minSkew = Some(DurationProto.of(60, 0)), - maxSkew = Some(DurationProto.of(30, 0)), - ) - ), + apiConfigManagementService + .setTimeModel( + SetTimeModelRequest.of( + "some submission ID", + maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), + configurationGeneration = initialGeneration, + newTimeModel = Some( + TimeModel( + avgTransactionLatency = Some(DurationProto.of(2 * 60, 0)), + minSkew = Some(DurationProto.of(60, 0)), + maxSkew = Some(DurationProto.of(30, 0)), + ) + ), + ) ) - ) - .map { response => - response.configurationGeneration should be(expectedGeneration) - currentConfiguration() should be(Some(expectedConfiguration)) - } - } + .map { response => + response.configurationGeneration should be(expectedGeneration) + currentConfiguration() should be(Some(expectedConfiguration)) + } + } - "refuse to set a new time model if none is indexed yet" in { - val initialGeneration = 0L + "refuse to set a new time model if none is indexed yet" in { + val initialGeneration = 0L - val timeProvider = TimeProvider.UTC - val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) + val timeProvider = TimeProvider.UTC + val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) - val writeService = mock[state.WriteService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - EmptyIndexConfigManagementService, - writeService, - timeProvider, - ) + val writeService = mock[state.WriteService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + EmptyIndexConfigManagementService, + writeService, + timeProvider, + errorCodesVersionSwitcher, + ) - apiConfigManagementService - .setTimeModel( - SetTimeModelRequest.of( - "a submission ID", - maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), - configurationGeneration = initialGeneration, - newTimeModel = Some( - TimeModel( - avgTransactionLatency = Some(DurationProto.of(10, 0)), - minSkew = Some(DurationProto.of(20, 0)), - maxSkew = Some(DurationProto.of(40, 0)), - ) - ), + apiConfigManagementService + .setTimeModel( + SetTimeModelRequest.of( + "a submission ID", + maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), + configurationGeneration = initialGeneration, + newTimeModel = Some( + TimeModel( + avgTransactionLatency = Some(DurationProto.of(10, 0)), + minSkew = Some(DurationProto.of(20, 0)), + maxSkew = Some(DurationProto.of(40, 0)), + ) + ), + ) ) - ) - .transform(Success.apply) - .map { response => - verifyZeroInteractions(writeService) - response should matchPattern { case Failure(GrpcException(GrpcStatus.UNAVAILABLE(), _)) => + .transform(Success.apply) + .map { response => + verifyZeroInteractions(writeService) + if (useSelfServiceErrorCodes) { + response should matchPattern { + case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => + } + } else { + response should matchPattern { + case Failure(GrpcException(GrpcStatus.UNAVAILABLE(), _)) => + } + } } - } - } + } - "propagate trace context" in { - val apiConfigManagementService = ApiConfigManagementService.createApiService( - new FakeStreamingIndexConfigManagementService(someConfigurationEntries), - TestWriteConfigService, - TimeProvider.UTC, - _ => Ref.SubmissionId.assertFromString("aSubmission"), - ) + "propagate trace context" in { + val apiConfigManagementService = ApiConfigManagementService.createApiService( + new FakeStreamingIndexConfigManagementService(someConfigurationEntries), + TestWriteConfigService, + TimeProvider.UTC, + errorCodesVersionSwitcher, + _ => Ref.SubmissionId.assertFromString("aSubmission"), + ) - val span = anEmptySpan() - val scope = span.makeCurrent() - apiConfigManagementService - .setTimeModel(aSetTimeModelRequest) - .andThen { case _ => - scope.close() - span.end() - } - .map { _ => - spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) - } + val span = anEmptySpan() + val scope = span.makeCurrent() + apiConfigManagementService + .setTimeModel(aSetTimeModelRequest) + .andThen { case _ => + scope.close() + span.end() + } + .map { _ => + spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) + } + } } } } From a2ab0c00f251016ccf6ee27d4499c87039d6eb57 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Thu, 21 Oct 2021 14:14:26 +0200 Subject: [PATCH 2/5] TV review --- .../platform/server/api/validation/ErrorFactories.scala | 2 ++ 1 file changed, 2 insertions(+) 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 548342d65374..83da071bb183 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 @@ -311,6 +311,8 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch addDefiniteAnswerDetails(definiteAnswer, statusBuilder) grpcError(statusBuilder.build()) }, + // TODO error codes: This error group is confusing for this generic error as it can be dispatched + // from call-sites that do not involve Daml interpreter. v2 = LedgerApiErrors.InterpreterErrors.LookupErrors.LedgerConfigurationNotFound .Reject() .asGrpcError, From 7e4c11b2bf0eb739d8c39ef93cad8888180858a9 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Thu, 21 Oct 2021 16:21:34 +0200 Subject: [PATCH 3/5] drop addTests(bool) and use mocks instead --- .../error/ErrorCodesVersionSwitcher.scala | 2 +- .../ApiConfigManagementServiceSpec.scala | 360 ++++++++++-------- 2 files changed, 193 insertions(+), 169 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/ErrorCodesVersionSwitcher.scala b/ledger/error/src/main/scala/com/daml/error/ErrorCodesVersionSwitcher.scala index 0172b67e9be1..e3dbc139cc5f 100644 --- a/ledger/error/src/main/scala/com/daml/error/ErrorCodesVersionSwitcher.scala +++ b/ledger/error/src/main/scala/com/daml/error/ErrorCodesVersionSwitcher.scala @@ -6,7 +6,7 @@ import io.grpc.StatusRuntimeException import scala.concurrent.Future -final class ErrorCodesVersionSwitcher(enableSelfServiceErrorCodes: Boolean) +class ErrorCodesVersionSwitcher(enableSelfServiceErrorCodes: Boolean) extends ValueSwitch[StatusRuntimeException](enableSelfServiceErrorCodes) { def chooseAsFailedFuture[T]( diff --git a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala index ce004a9ff594..4f377fe8b04d 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala @@ -3,10 +3,6 @@ package com.daml.platform.apiserver.services.admin -import java.time.Duration -import java.util.concurrent.CompletableFuture.completedFuture -import java.util.concurrent.CompletionStage -import java.util.concurrent.atomic.{AtomicLong, AtomicReference} import akka.NotUsed import akka.stream.Materializer import akka.stream.scaladsl.Source @@ -22,7 +18,7 @@ import com.daml.ledger.api.v1.admin.config_management_service.{ } import com.daml.ledger.configuration.{Configuration, LedgerTimeModel} import com.daml.ledger.participant.state.index.v2.IndexConfigManagementService -import com.daml.ledger.participant.state.v2.{SubmissionResult, WriteConfigService} +import com.daml.ledger.participant.state.v2.{SubmissionResult, WriteConfigService, WriteService} import com.daml.ledger.participant.state.{v2 => state} import com.daml.lf.data.Ref.SubmissionId import com.daml.lf.data.{Ref, Time} @@ -37,9 +33,13 @@ import org.scalatest.Inside import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AsyncWordSpec +import java.time.Duration +import java.util.concurrent.CompletableFuture.completedFuture +import java.util.concurrent.CompletionStage +import java.util.concurrent.atomic.{AtomicLong, AtomicReference} import scala.collection.immutable -import scala.concurrent.{Await, Future, Promise} import scala.concurrent.duration.{Duration => ScalaDuration} +import scala.concurrent.{Await, Future, Promise} import scala.util.{Failure, Success} class ApiConfigManagementServiceSpec @@ -53,189 +53,213 @@ class ApiConfigManagementServiceSpec private implicit val loggingContext: LoggingContext = LoggingContext.ForTesting - addTests(true) - addTests(false) + val useSelfServiceErrorCodes = mock[ErrorCodesVersionSwitcher] + + s"ApiConfigManagementService" should { + "get the time model" in { + val indexedTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(5), + minSkew = Duration.ofMinutes(3), + maxSkew = Duration.ofMinutes(2), + ).get + val expectedTimeModel = TimeModel.of( + avgTransactionLatency = Some(DurationProto.of(5 * 60, 0)), + minSkew = Some(DurationProto.of(3 * 60, 0)), + maxSkew = Some(DurationProto.of(2 * 60, 0)), + ) - def addTests(useSelfServiceErrorCodes: Boolean): Unit = { - val suffix = s"(enableSelfServiceErrorCodes=${useSelfServiceErrorCodes})" - val errorCodesVersionSwitcher = new ErrorCodesVersionSwitcher(useSelfServiceErrorCodes) + val writeService = mock[state.WriteConfigService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + new FakeCurrentIndexConfigManagementService( + LedgerOffset.Absolute(Ref.LedgerString.assertFromString("0")), + Configuration(aConfigurationGeneration, indexedTimeModel, Duration.ZERO), + ), + writeService, + TimeProvider.UTC, + useSelfServiceErrorCodes, + ) - s"ApiConfigManagementService $suffix" should { - "get the time model" in { - val indexedTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(5), - minSkew = Duration.ofMinutes(3), - maxSkew = Duration.ofMinutes(2), - ).get - val expectedTimeModel = TimeModel.of( - avgTransactionLatency = Some(DurationProto.of(5 * 60, 0)), - minSkew = Some(DurationProto.of(3 * 60, 0)), - maxSkew = Some(DurationProto.of(2 * 60, 0)), - ) + apiConfigManagementService + .getTimeModel(GetTimeModelRequest.defaultInstance) + .map { response => + response.timeModel should be(Some(expectedTimeModel)) + verifyZeroInteractions(writeService) + succeed + } + .map { x => + verifyZeroInteractions(useSelfServiceErrorCodes) + x + } - val writeService = mock[state.WriteConfigService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - new FakeCurrentIndexConfigManagementService( - LedgerOffset.Absolute(Ref.LedgerString.assertFromString("0")), - Configuration(aConfigurationGeneration, indexedTimeModel, Duration.ZERO), - ), - writeService, - TimeProvider.UTC, - errorCodesVersionSwitcher, - ) + } - apiConfigManagementService.getTimeModel(GetTimeModelRequest.defaultInstance).map { - response => - response.timeModel should be(Some(expectedTimeModel)) - verifyZeroInteractions(writeService) - succeed - } - } + "return a `NOT_FOUND` error if a time model is not found (V1 error codes)" in { + testReturnANotFoundErrorIfTimeModelNotFound(false) + } - "return a `NOT_FOUND` error if a time model is not found" in { - val writeService = mock[state.WriteConfigService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - EmptyIndexConfigManagementService, - writeService, - TimeProvider.UTC, - errorCodesVersionSwitcher, - ) + "return a `NOT_FOUND` error if a time model is not found (V2 self-service error codes)" in { + testReturnANotFoundErrorIfTimeModelNotFound(true) + } - apiConfigManagementService - .getTimeModel(GetTimeModelRequest.defaultInstance) - .transform(Success.apply) - .map { response => - response should matchPattern { case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => - } - } - } + "set a new time model" in { + val maximumDeduplicationTime = Duration.ofHours(6) + val initialGeneration = 2L + val initialTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(1), + minSkew = Duration.ofMinutes(2), + maxSkew = Duration.ofMinutes(3), + ).get + val initialConfiguration = Configuration( + generation = initialGeneration, + timeModel = initialTimeModel, + maxDeduplicationTime = maximumDeduplicationTime, + ) + val expectedGeneration = 3L + val expectedTimeModel = LedgerTimeModel( + avgTransactionLatency = Duration.ofMinutes(2), + minSkew = Duration.ofMinutes(1), + maxSkew = Duration.ofSeconds(30), + ).get + val expectedConfiguration = Configuration( + generation = expectedGeneration, + timeModel = expectedTimeModel, + maxDeduplicationTime = maximumDeduplicationTime, + ) - "set a new time model" in { - val maximumDeduplicationTime = Duration.ofHours(6) - val initialGeneration = 2L - val initialTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(1), - minSkew = Duration.ofMinutes(2), - maxSkew = Duration.ofMinutes(3), - ).get - val initialConfiguration = Configuration( - generation = initialGeneration, - timeModel = initialTimeModel, - maxDeduplicationTime = maximumDeduplicationTime, - ) - val expectedGeneration = 3L - val expectedTimeModel = LedgerTimeModel( - avgTransactionLatency = Duration.ofMinutes(2), - minSkew = Duration.ofMinutes(1), - maxSkew = Duration.ofSeconds(30), - ).get - val expectedConfiguration = Configuration( - generation = expectedGeneration, - timeModel = expectedTimeModel, - maxDeduplicationTime = maximumDeduplicationTime, - ) + val timeProvider = TimeProvider.UTC + val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) - val timeProvider = TimeProvider.UTC - val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) + val (indexService, writeService, currentConfiguration) = fakeServices( + startingOffset = 7, + submissions = Seq(Ref.SubmissionId.assertFromString("one") -> initialConfiguration), + ) + val apiConfigManagementService = ApiConfigManagementService.createApiService( + indexService, + writeService, + timeProvider, + useSelfServiceErrorCodes, + ) - val (indexService, writeService, currentConfiguration) = fakeServices( - startingOffset = 7, - submissions = Seq(Ref.SubmissionId.assertFromString("one") -> initialConfiguration), - ) - val apiConfigManagementService = ApiConfigManagementService.createApiService( - indexService, - writeService, - timeProvider, - errorCodesVersionSwitcher, + apiConfigManagementService + .setTimeModel( + SetTimeModelRequest.of( + "some submission ID", + maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), + configurationGeneration = initialGeneration, + newTimeModel = Some( + TimeModel( + avgTransactionLatency = Some(DurationProto.of(2 * 60, 0)), + minSkew = Some(DurationProto.of(60, 0)), + maxSkew = Some(DurationProto.of(30, 0)), + ) + ), + ) ) + .map { response => + response.configurationGeneration should be(expectedGeneration) + currentConfiguration() should be(Some(expectedConfiguration)) + } + .map { x => + verifyZeroInteractions(useSelfServiceErrorCodes) + x + } + } - apiConfigManagementService - .setTimeModel( - SetTimeModelRequest.of( - "some submission ID", - maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), - configurationGeneration = initialGeneration, - newTimeModel = Some( - TimeModel( - avgTransactionLatency = Some(DurationProto.of(2 * 60, 0)), - minSkew = Some(DurationProto.of(60, 0)), - maxSkew = Some(DurationProto.of(30, 0)), - ) - ), - ) - ) - .map { response => - response.configurationGeneration should be(expectedGeneration) - currentConfiguration() should be(Some(expectedConfiguration)) - } - } + "refuse to set a new time model if none is indexed (V1 error codes)" in { + testRefuseToSetANewTimeModelIfNoneIsIndexedYet(false) + } - "refuse to set a new time model if none is indexed yet" in { - val initialGeneration = 0L + "refuse to set a new time model if none is indexed (V2 self-service error codes)" in { + testRefuseToSetANewTimeModelIfNoneIsIndexedYet(true) + } - val timeProvider = TimeProvider.UTC - val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) + "propagate trace context" in { + val apiConfigManagementService = ApiConfigManagementService.createApiService( + new FakeStreamingIndexConfigManagementService(someConfigurationEntries), + TestWriteConfigService, + TimeProvider.UTC, + useSelfServiceErrorCodes, + _ => Ref.SubmissionId.assertFromString("aSubmission"), + ) - val writeService = mock[state.WriteService] - val apiConfigManagementService = ApiConfigManagementService.createApiService( - EmptyIndexConfigManagementService, - writeService, - timeProvider, - errorCodesVersionSwitcher, - ) + val span = anEmptySpan() + val scope = span.makeCurrent() + apiConfigManagementService + .setTimeModel(aSetTimeModelRequest) + .andThen { case _ => + scope.close() + span.end() + } + .map { _ => + spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) + } + .map { x => + verifyZeroInteractions(useSelfServiceErrorCodes) + x + } + } + } - apiConfigManagementService - .setTimeModel( - SetTimeModelRequest.of( - "a submission ID", - maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), - configurationGeneration = initialGeneration, - newTimeModel = Some( - TimeModel( - avgTransactionLatency = Some(DurationProto.of(10, 0)), - minSkew = Some(DurationProto.of(20, 0)), - maxSkew = Some(DurationProto.of(40, 0)), - ) - ), - ) - ) - .transform(Success.apply) - .map { response => - verifyZeroInteractions(writeService) - if (useSelfServiceErrorCodes) { - response should matchPattern { - case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => - } - } else { - response should matchPattern { - case Failure(GrpcException(GrpcStatus.UNAVAILABLE(), _)) => - } - } - } + private def testReturnANotFoundErrorIfTimeModelNotFound(useSelfServiceErrorCodes: Boolean) = { + val errorCodesVersionSwitcher = new ErrorCodesVersionSwitcher(useSelfServiceErrorCodes) + val writeService = mock[WriteConfigService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + EmptyIndexConfigManagementService, + writeService, + TimeProvider.UTC, + errorCodesVersionSwitcher, + ) + + apiConfigManagementService + .getTimeModel(GetTimeModelRequest.defaultInstance) + .transform(Success.apply) + .map { response => + response should matchPattern { case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => + } } + } - "propagate trace context" in { - val apiConfigManagementService = ApiConfigManagementService.createApiService( - new FakeStreamingIndexConfigManagementService(someConfigurationEntries), - TestWriteConfigService, - TimeProvider.UTC, - errorCodesVersionSwitcher, - _ => Ref.SubmissionId.assertFromString("aSubmission"), + private def testRefuseToSetANewTimeModelIfNoneIsIndexedYet(useSelfServiceErrorCodes: Boolean) = { + val errorCodesVersionSwitcher = new ErrorCodesVersionSwitcher(useSelfServiceErrorCodes) + val initialGeneration = 0L + + val timeProvider = TimeProvider.UTC + val maximumRecordTime = timeProvider.getCurrentTime.plusSeconds(60) + + val writeService = mock[WriteService] + val apiConfigManagementService = ApiConfigManagementService.createApiService( + EmptyIndexConfigManagementService, + writeService, + timeProvider, + errorCodesVersionSwitcher, + ) + + apiConfigManagementService + .setTimeModel( + SetTimeModelRequest.of( + "a submission ID", + maximumRecordTime = Some(Timestamp.of(maximumRecordTime.getEpochSecond, 0)), + configurationGeneration = initialGeneration, + newTimeModel = Some( + TimeModel( + avgTransactionLatency = Some(DurationProto.of(10, 0)), + minSkew = Some(DurationProto.of(20, 0)), + maxSkew = Some(DurationProto.of(40, 0)), + ) + ), ) - - val span = anEmptySpan() - val scope = span.makeCurrent() - apiConfigManagementService - .setTimeModel(aSetTimeModelRequest) - .andThen { case _ => - scope.close() - span.end() + ) + .transform(Success.apply) + .map { response => + verifyZeroInteractions(writeService) + if (useSelfServiceErrorCodes) { + response should matchPattern { case Failure(GrpcException(GrpcStatus.NOT_FOUND(), _)) => } - .map { _ => - spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) + } else { + response should matchPattern { case Failure(GrpcException(GrpcStatus.UNAVAILABLE(), _)) => } + } } - } } } From 0430cf9ac85f64348642e1609b0a95abb4e1901c Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Thu, 21 Oct 2021 16:22:35 +0200 Subject: [PATCH 4/5] 1 --- .../services/admin/ApiConfigManagementServiceSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala index 4f377fe8b04d..076f1ae4b564 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala @@ -55,7 +55,7 @@ class ApiConfigManagementServiceSpec val useSelfServiceErrorCodes = mock[ErrorCodesVersionSwitcher] - s"ApiConfigManagementService" should { + "ApiConfigManagementService" should { "get the time model" in { val indexedTimeModel = LedgerTimeModel( avgTransactionLatency = Duration.ofMinutes(5), From aca683da8534e8d3994f4886d0a05ccd0046ad1e Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Thu, 21 Oct 2021 16:59:06 +0200 Subject: [PATCH 5/5] TV review --- .../admin/ApiConfigManagementServiceSpec.scala | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala index 076f1ae4b564..7e0eb500c0ce 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/db/migration/translation/apiserver/services/admin/ApiConfigManagementServiceSpec.scala @@ -53,7 +53,7 @@ class ApiConfigManagementServiceSpec private implicit val loggingContext: LoggingContext = LoggingContext.ForTesting - val useSelfServiceErrorCodes = mock[ErrorCodesVersionSwitcher] + private val useSelfServiceErrorCodes = mock[ErrorCodesVersionSwitcher] "ApiConfigManagementService" should { "get the time model" in { @@ -84,13 +84,9 @@ class ApiConfigManagementServiceSpec .map { response => response.timeModel should be(Some(expectedTimeModel)) verifyZeroInteractions(writeService) - succeed - } - .map { x => verifyZeroInteractions(useSelfServiceErrorCodes) - x + succeed } - } "return a `NOT_FOUND` error if a time model is not found (V1 error codes)" in { @@ -158,10 +154,8 @@ class ApiConfigManagementServiceSpec .map { response => response.configurationGeneration should be(expectedGeneration) currentConfiguration() should be(Some(expectedConfiguration)) - } - .map { x => verifyZeroInteractions(useSelfServiceErrorCodes) - x + succeed } } @@ -192,10 +186,8 @@ class ApiConfigManagementServiceSpec } .map { _ => spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) - } - .map { x => verifyZeroInteractions(useSelfServiceErrorCodes) - x + succeed } } }