[GitHub] [james-project] mbaechler commented on a change in pull request #358: [REFACTORING] JMAP: All ids should be backed by IdConstraint

[GitBox <gi...@apache.org>]

mbaechler commented on a change in pull request #358:
URL: https://github.com/apache/james-project/pull/358#discussion_r607213566



##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
##########
@@ -46,7 +47,7 @@ case class MailboxSetRequest(accountId: AccountId,
                              onDestroyRemoveEmails: Option[RemoveEmailsOnDestroy]) extends WithAccountId
 
 object MailboxSetRequest {
-  type MailboxCreationId = String Refined NonEmpty
+  type MailboxCreationId = String Refined IdConstraint

Review comment:
       As far as I understand Refined, `MailboxCreationId` type is equal to `UnparsedVacationResponseId` and thus defeat the purpose of having different types. You probably need newtype here. (see https://stackoverflow.com/questions/62579057/how-to-ensure-type-safety-with-scalas-refined-library-when-using-the-same-predi for example)

##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
##########
@@ -1059,7 +1059,7 @@ trait MailboxSetMethodContract {
          |      "notCreated": {
          |        "C42": {
          |          "type": "invalidArguments",
-         |          "description": "'/parentId' property in mailbox object is not valid: Predicate isEmpty() did not fail."
+         |          "description": "'/parentId' property in mailbox object is not valid: Left predicate of ((!(0 < 1) && !(0 > 255)) && \\"\\".matches(\\"^[#a-zA-Z0-9-_]*$$\\")) failed: Predicate taking size() = 0 failed: Left predicate of (!(0 < 1) && !(0 > 255)) failed: Predicate (0 < 1) did not fail."

Review comment:
       this looks like an over-precise check: it expects an error specific to Refined but should not




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org