Improve DAML LF Party, PackageName, PackageId fromString error messages (#5855)

* Add description to daml.lf.data.StringModule implementations

Make sure DAML LF Party deserialization picks up the added description.

changelog_begin
changelog_end

* addressing code review comments. @S11001001, thanks.

daml-lf/data/src/main/scala/com/digitalasset/daml/lf/data/IdString.scala

@@ -155,13 +155,17 @@ private object HexStringModuleImpl extends StringModuleImpl with HexStringModule
     Bytes.fromInputStream(baseEncode.decodingStream(new StringReader(a)))
 }
 
-private final class MatchingStringModule(string_regex: String) extends StringModuleImpl {
+private final class MatchingStringModule(description: String, string_regex: String)
+    extends StringModuleImpl {
 
   private val regex = string_regex.r
   private val pattern = regex.pattern
 
   override def fromString(s: String): Either[String, T] =
-    Either.cond(pattern.matcher(s).matches(), s, s"""string "$s" does not match regex "$regex"""")
+    Either.cond(
+      pattern.matcher(s).matches(),
+      s,
+      s"""$description "$s" does not match regex "$regex"""")
 
 }
 
@@ -176,20 +180,21 @@ private final class MatchingStringModule(string_regex: String) extends StringMod
   * ids.
   */
 private final class ConcatenableMatchingStringModule(
+    description: String,
     extraAllowedChars: Char => Boolean,
-    maxLength: Int = Int.MaxValue
+    maxLength: Int = Int.MaxValue,
 ) extends StringModuleImpl
     with ConcatenableStringModule[String, String] {
 
   override def fromString(s: String): Either[String, T] =
     if (s.isEmpty)
-      Left(s"""empty string""")
+      Left(s"""$description is empty""")
     else if (s.length > maxLength)
-      Left(s"""string too long""")
+      Left(s"""$description is too long""")
     else
       s.find(c => c > 0x7f || !(c.isLetterOrDigit || extraAllowedChars(c)))
         .fold[Either[String, T]](Right(s))(c =>
-          Left(s"""non expected character 0x${c.toInt.toHexString} in "$s""""))
+          Left(s"""non expected character 0x${c.toInt.toHexString} in $description "$s""""))
 
   override def fromLong(i: Long): T = i.toString
 
@@ -220,20 +225,20 @@ private[data] final class IdStringImpl extends IdString {
   // In a language like C# you'll need to use some other unicode char for `$`.
   override type Name = String
   override val Name: StringModule[Name] =
-    new MatchingStringModule("""[A-Za-z\$_][A-Za-z0-9\$_]*""")
+    new MatchingStringModule("DAML LF Name", """[A-Za-z\$_][A-Za-z0-9\$_]*""")
 
   /** Package names are non-empty US-ASCII strings built from letters, digits, minus and underscore.
     */
   override type PackageName = String
   override val PackageName: ConcatenableStringModule[PackageName, HexString] =
-    new ConcatenableMatchingStringModule("-_".contains(_))
+    new ConcatenableMatchingStringModule("DAML LF Package Name", "-_".contains(_))
 
   /** Package versions are non-empty strings consisting of segments of digits (without leading zeros)
       separated by dots.
     */
   override type PackageVersion = String
   override val PackageVersion: StringModule[PackageVersion] =
-    new MatchingStringModule("""(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*""")
+    new MatchingStringModule("DAML LF Package Version", """(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*""")
 
   /** Party identifiers are non-empty US-ASCII strings built from letters, digits, space, colon, minus and,
     * underscore limited to 255 chars. We use them to represent [Party] literals. In this way, we avoid
@@ -241,13 +246,13 @@ private[data] final class IdStringImpl extends IdString {
     */
   override type Party = String
   override val Party: ConcatenableStringModule[Party, HexString] =
-    new ConcatenableMatchingStringModule(":-_ ".contains(_), 255)
+    new ConcatenableMatchingStringModule("DAML LF Party", ":-_ ".contains(_), 255)
 
   /** Reference to a package via a package identifier. The identifier is the ascii7
     * lowercase hex-encoded hash of the package contents found in the DAML LF Archive. */
   override type PackageId = String
   override val PackageId: ConcatenableStringModule[PackageId, HexString] =
-    new ConcatenableMatchingStringModule("-_ ".contains(_))
+    new ConcatenableMatchingStringModule("DAML LF Package ID", "-_ ".contains(_))
 
   /**
     * Used to reference to leger objects like contractIds, ledgerIds,
@@ -257,7 +262,7 @@ private[data] final class IdStringImpl extends IdString {
   // We allow space because the navigator's applicationId used it.
   override type LedgerString = String
   override val LedgerString: ConcatenableStringModule[LedgerString, HexString] =
-    new ConcatenableMatchingStringModule("._:-#/ ".contains(_), 255)
+    new ConcatenableMatchingStringModule("DAML LF Ledger String", "._:-#/ ".contains(_), 255)
 
   override type ParticipantId = String
   override val ParticipantId = LedgerString
@@ -267,6 +272,6 @@ private[data] final class IdStringImpl extends IdString {
     */
   override type ContractIdString = String
   override val ContractIdString: StringModule[ContractIdString] =
-    new MatchingStringModule("""#[\w._:\-#/ ]{0,254}""")
+    new MatchingStringModule("DAML LF Contract ID", """#[\w._:\-#/ ]{0,254}""")
 
 }

ledger-service/http-json/src/test/scala/com/digitalasset/http/AbstractHttpServiceIntegrationTest.scala

@@ -993,6 +993,23 @@ abstract class AbstractHttpServiceIntegrationTest
       }: Future[Assertion]
   }
 
+  "parties endpoint returns error if empty party string passed" in withHttpServiceAndClient {
+    (uri, _, _, _) =>
+      val requestedPartyIds: Vector[domain.Party] = domain.Party.subst(Vector(""))
+
+      postJsonRequest(
+        uri = uri.withPath(Uri.Path("/v1/parties")),
+        JsArray(requestedPartyIds.map(x => JsString(x.unwrap)))
+      ).flatMap {
+        case (status, output) =>
+          status shouldBe StatusCodes.BadRequest
+          inside(decode1[domain.SyncResponse, List[domain.PartyDetails]](output)) {
+            case \/-(domain.ErrorResponse(List(error), None, StatusCodes.BadRequest)) =>
+              error should include("DAML LF Party is empty")
+          }
+      }: Future[Assertion]
+  }
+
   "parties endpoint returns empty result with warnings and OK status if nothing found" in withHttpServiceAndClient {
     (uri, _, _, _) =>
       val requestedPartyIds: Vector[domain.Party] =

ledger-service/lf-value-json/src/test/scala/com/digitalasset/daml/lf/value/json/ApiCodecCompressedSpec.scala

@@ -320,17 +320,19 @@ class ApiCodecCompressedSpec
     )
 
     val failures = Table(
-      ("JSON", "type"),
-      ("42.3", VA.int64),
-      ("\"42.3\"", VA.int64),
-      ("9223372036854775808", VA.int64),
-      ("-9223372036854775809", VA.int64),
-      ("\"garbage\"", VA.int64),
-      ("\"   42 \"", VA.int64),
-      ("\"1970-01-01T00:00:00\"", VA.timestamp),
-      ("\"1970-01-01T00:00:00+01:00\"", VA.timestamp),
-      ("\"1970-01-01T00:00:00+01:00[Europe/Paris]\"", VA.timestamp),
-      ("""{"a": "b", "c": "d"}""", VA.genMap(VA.text, VA.text)),
+      ("JSON", "type", "errorSubstring"),
+      ("42.3", VA.int64, ""),
+      ("\"42.3\"", VA.int64, ""),
+      ("9223372036854775808", VA.int64, ""),
+      ("-9223372036854775809", VA.int64, ""),
+      ("\"garbage\"", VA.int64, ""),
+      ("\"   42 \"", VA.int64, ""),
+      ("\"1970-01-01T00:00:00\"", VA.timestamp, ""),
+      ("\"1970-01-01T00:00:00+01:00\"", VA.timestamp, ""),
+      ("\"1970-01-01T00:00:00+01:00[Europe/Paris]\"", VA.timestamp, ""),
+      ("""{"a": "b", "c": "d"}""", VA.genMap(VA.text, VA.text), ""),
+      ("\"\"", VA.party, "DAML LF Party is empty"),
+      (List.fill(256)('a').mkString("\"", "", "\""), VA.party, "DAML LF Party is too long"),
     )
 
     "dealing with particular formats" should {
@@ -350,11 +352,12 @@ class ApiCodecCompressedSpec
           }
       }
 
-      "fail in cases" in forEvery(failures) { (serialized, typ) =>
+      "fail in cases" in forEvery(failures) { (serialized, typ, errorSubstring) =>
         val json = serialized.parseJson // we don't test *the JSON decoder*
-        a[DeserializationException] shouldBe thrownBy {
+        val exception = the[DeserializationException] thrownBy {
           jsValueToApiValue(json, typ.t, typeLookup)
         }
+        exception.getMessage should include(errorSubstring)
       }
     }
 

ledger/ledger-api-common/src/test/lib/scala/com/digitalasset/ledger/api/DomainMocks.scala

@@ -38,7 +38,7 @@ object DomainMocks {
     private val invalidPartyString = "p@rty"
     val invalidApiParty = Value(Sum.Party(invalidPartyString))
     val invalidPartyMsg =
-      """Invalid argument: non expected character 0x40 in "p@rty""""
+      """Invalid argument: non expected character 0x40 in DAML LF Party "p@rty""""
   }
 
 }