mirror of
https://github.com/digital-asset/daml.git
synced 2024-09-20 01:07:18 +03:00
Correctly document and report malformed party names when allocating (#8642)
* Correctly document and report malformed party names when allocating changelog_begin [Ledger API] Documented the hard-coded limit of 255 characters for Daml-LF party names. [Ledger API] Malformed party names are now correctly reported back with an INVALID_ARGUMENT error changelog_end * Make sure that what is given is a suggestion Co-authored-by: Robert Autenrieth <31539813+rautenrieth-da@users.noreply.github.com> * Use Future.successful where possible Co-authored-by: Robert Autenrieth <31539813+rautenrieth-da@users.noreply.github.com> * Address https://github.com/digital-asset/daml/pull/8642#discussion_r565116571 * Address https://github.com/digital-asset/daml/pull/8642#discussion_r565122362 * Keep fields private where possible * Whitelist new tests on compatibility suite * Don't run the new tests on Canton Co-authored-by: Robert Autenrieth <31539813+rautenrieth-da@users.noreply.github.com>
This commit is contained in:
parent
48daf33586
commit
793fd8fdfc
@ -155,6 +155,19 @@ excluded_test_tool_tests = [
|
|||||||
},
|
},
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"start": "0.0.0", # TODO This should become the latest available snapshot when it becomes available
|
||||||
|
"platform_ranges": [
|
||||||
|
{
|
||||||
|
"end": "1.10.0-snapshot.20210120.6106.0.58ef725a",
|
||||||
|
"exclusions": [
|
||||||
|
# See https://github.com/digital-asset/daml/pull/8642
|
||||||
|
"PartyManagementServiceIT:PMRejectLongPartyHints",
|
||||||
|
"PartyManagementServiceIT:PMRejectInvalidPartyHints",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
]
|
]
|
||||||
|
|
||||||
def in_range(version, range):
|
def in_range(version, range):
|
||||||
|
@ -61,6 +61,8 @@ service PartyManagementService {
|
|||||||
// canton: completely different globally unique identifier is allocated.
|
// canton: completely different globally unique identifier is allocated.
|
||||||
// Behind the scenes calls to an internal protocol are made. As that protocol
|
// Behind the scenes calls to an internal protocol are made. As that protocol
|
||||||
// is richer than the surface protocol, the arguments take implicit values
|
// is richer than the surface protocol, the arguments take implicit values
|
||||||
|
// The party identifier suggestion must be a valid party name. Party names are required to be non-empty US-ASCII strings built from letters, digits, space,
|
||||||
|
// colon, minus and underscore limited to 255 chars
|
||||||
rpc AllocateParty (AllocatePartyRequest) returns (AllocatePartyResponse);
|
rpc AllocateParty (AllocatePartyRequest) returns (AllocatePartyResponse);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -75,7 +75,9 @@ conformance_test(
|
|||||||
"--exclude=ContractKeysIT,ContractKeysIT:CKFetchOrLookup,ContractKeysIT:CKNoFetchUndisclosed,ContractKeysIT:CKMaintainerScoped" +
|
"--exclude=ContractKeysIT,ContractKeysIT:CKFetchOrLookup,ContractKeysIT:CKNoFetchUndisclosed,ContractKeysIT:CKMaintainerScoped" +
|
||||||
",ParticipantPruningIT" + # see "conformance-test-participant-pruning" below
|
",ParticipantPruningIT" + # see "conformance-test-participant-pruning" below
|
||||||
",ConfigManagementServiceIT,LedgerConfigurationServiceIT" + # dynamic config management not supported by Canton
|
",ConfigManagementServiceIT,LedgerConfigurationServiceIT" + # dynamic config management not supported by Canton
|
||||||
",ClosedWorldIT", # Canton currently fails this test with a different error (missing namespace in "unallocated" party id)
|
",ClosedWorldIT" + # Canton currently fails this test with a different error (missing namespace in "unallocated" party id)
|
||||||
|
# The following tests fail because Canton raises a different error (see https://github.com/digital-asset/daml/pull/8642)
|
||||||
|
",PartyManagementServiceIT:PMRejectLongPartyHints,PartyManagementServiceIT:PMRejectInvalidPartyHints",
|
||||||
],
|
],
|
||||||
) if not is_windows else None
|
) if not is_windows else None
|
||||||
|
|
||||||
|
@ -4,11 +4,13 @@
|
|||||||
package com.daml.ledger.api.testtool.suites
|
package com.daml.ledger.api.testtool.suites
|
||||||
|
|
||||||
import com.daml.ledger.api.testtool.infrastructure.Allocation._
|
import com.daml.ledger.api.testtool.infrastructure.Allocation._
|
||||||
|
import com.daml.ledger.api.testtool.infrastructure.Assertions.assertGrpcError
|
||||||
import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite
|
import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite
|
||||||
import com.daml.ledger.api.v1.admin.party_management_service.PartyDetails
|
import com.daml.ledger.api.v1.admin.party_management_service.PartyDetails
|
||||||
import com.daml.ledger.client.binding
|
import com.daml.ledger.client.binding
|
||||||
import com.daml.ledger.test.model.Test.Dummy
|
import com.daml.ledger.test.model.Test.Dummy
|
||||||
import com.daml.lf.data.Ref
|
import com.daml.lf.data.Ref
|
||||||
|
import io.grpc.Status
|
||||||
import scalaz.Tag
|
import scalaz.Tag
|
||||||
import scalaz.syntax.tag.ToTagOps
|
import scalaz.syntax.tag.ToTagOps
|
||||||
|
|
||||||
@ -90,6 +92,41 @@ final class PartyManagementServiceIT extends LedgerTestSuite {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test(
|
||||||
|
"PMRejectLongPartyHints",
|
||||||
|
"A party identifier which is too long should be rejected with the proper error",
|
||||||
|
allocate(NoParties),
|
||||||
|
)(implicit ec => { case Participants(Participant(ledger)) =>
|
||||||
|
for {
|
||||||
|
error <- ledger
|
||||||
|
.allocateParty(
|
||||||
|
partyIdHint = Some(Random.alphanumeric.take(256).mkString),
|
||||||
|
displayName = None,
|
||||||
|
)
|
||||||
|
.failed
|
||||||
|
} yield {
|
||||||
|
assertGrpcError(error, Status.Code.INVALID_ARGUMENT, "Party is too long")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test(
|
||||||
|
"PMRejectInvalidPartyHints",
|
||||||
|
"A party identifier that contains invalid characters should be rejected with the proper error",
|
||||||
|
allocate(NoParties),
|
||||||
|
)(implicit ec => { case Participants(Participant(ledger)) =>
|
||||||
|
for {
|
||||||
|
error <- ledger
|
||||||
|
.allocateParty(
|
||||||
|
// Assumption: emojis will never be acceptable in a party identifier
|
||||||
|
partyIdHint = Some("\uD83D\uDE00"),
|
||||||
|
displayName = None,
|
||||||
|
)
|
||||||
|
.failed
|
||||||
|
} yield {
|
||||||
|
assertGrpcError(error, Status.Code.INVALID_ARGUMENT, "non expected character")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
test(
|
test(
|
||||||
"PMAllocateOneHundred",
|
"PMAllocateOneHundred",
|
||||||
"It should create unique party names when allocating many parties",
|
"It should create unique party names when allocating many parties",
|
||||||
|
@ -40,6 +40,8 @@ private[apiserver] final class ApiPartyManagementService private (
|
|||||||
) extends PartyManagementService
|
) extends PartyManagementService
|
||||||
with GrpcApiService {
|
with GrpcApiService {
|
||||||
|
|
||||||
|
import ApiPartyManagementService.CreateSubmissionId
|
||||||
|
|
||||||
private val logger = ContextualizedLogger.get(this.getClass)
|
private val logger = ContextualizedLogger.get(this.getClass)
|
||||||
|
|
||||||
private val synchronousResponse = new SynchronousResponse(
|
private val synchronousResponse = new SynchronousResponse(
|
||||||
@ -81,30 +83,36 @@ private[apiserver] final class ApiPartyManagementService private (
|
|||||||
.andThen(logger.logErrorsOnCall[ListKnownPartiesResponse])
|
.andThen(logger.logErrorsOnCall[ListKnownPartiesResponse])
|
||||||
|
|
||||||
override def allocateParty(request: AllocatePartyRequest): Future[AllocatePartyResponse] = {
|
override def allocateParty(request: AllocatePartyRequest): Future[AllocatePartyResponse] = {
|
||||||
// TODO: This should do proper validation.
|
|
||||||
def randomSubmissionId(prefix: String): Ref.IdString.LedgerString =
|
|
||||||
v1.SubmissionId.assertFromString(s"${prefix}_${UUID.randomUUID().toString}")
|
|
||||||
|
|
||||||
val party =
|
val validatedPartyIdentifier =
|
||||||
if (request.partyIdHint.isEmpty) None
|
if (request.partyIdHint.isEmpty) {
|
||||||
else Some(Ref.Party.assertFromString(request.partyIdHint))
|
Future.successful(None)
|
||||||
val submissionId = randomSubmissionId(prefix = party.getOrElse(""))
|
} else {
|
||||||
|
Ref.Party
|
||||||
val displayName = if (request.displayName.isEmpty) None else Some(request.displayName)
|
.fromString(request.partyIdHint)
|
||||||
|
.fold(
|
||||||
synchronousResponse
|
error => Future.failed(ErrorFactories.invalidArgument(error)),
|
||||||
.submitAndWait(submissionId, (party, displayName))
|
party => Future.successful(Some(party)),
|
||||||
.map { case PartyEntry.AllocationAccepted(_, partyDetails) =>
|
|
||||||
AllocatePartyResponse(
|
|
||||||
Some(
|
|
||||||
PartyDetails(
|
|
||||||
partyDetails.party,
|
|
||||||
partyDetails.displayName.getOrElse(""),
|
|
||||||
partyDetails.isLocal,
|
|
||||||
)
|
|
||||||
)
|
)
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
validatedPartyIdentifier
|
||||||
|
.flatMap(party => {
|
||||||
|
val displayName = if (request.displayName.isEmpty) None else Some(request.displayName)
|
||||||
|
synchronousResponse
|
||||||
|
.submitAndWait(CreateSubmissionId.withPrefix(party), (party, displayName))
|
||||||
|
.map { case PartyEntry.AllocationAccepted(_, partyDetails) =>
|
||||||
|
AllocatePartyResponse(
|
||||||
|
Some(
|
||||||
|
PartyDetails(
|
||||||
|
partyDetails.party,
|
||||||
|
partyDetails.displayName.getOrElse(""),
|
||||||
|
partyDetails.isLocal,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
.andThen(logger.logErrorsOnCall[AllocatePartyResponse])
|
.andThen(logger.logErrorsOnCall[AllocatePartyResponse])
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -129,6 +137,18 @@ private[apiserver] object ApiPartyManagementService {
|
|||||||
managementServiceTimeout,
|
managementServiceTimeout,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
private object CreateSubmissionId {
|
||||||
|
// Suffix is `-` followed by a random UUID as a string
|
||||||
|
private val SuffixLength: Int = 1 + UUID.randomUUID().toString.length
|
||||||
|
private val MaxLength: Int = 255
|
||||||
|
private val PrefixMaxLength: Int = MaxLength - SuffixLength
|
||||||
|
def withPrefix(maybeParty: Option[Ref.Party]): v1.SubmissionId = {
|
||||||
|
val uuid = UUID.randomUUID().toString
|
||||||
|
val raw = maybeParty.fold(uuid)(party => s"${party.take(PrefixMaxLength)}-$uuid")
|
||||||
|
v1.SubmissionId.assertFromString(raw)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private final class SynchronousResponseStrategy(
|
private final class SynchronousResponseStrategy(
|
||||||
ledgerEndService: LedgerEndService,
|
ledgerEndService: LedgerEndService,
|
||||||
writeService: WritePartyService,
|
writeService: WritePartyService,
|
||||||
|
Loading…
Reference in New Issue
Block a user