diff --git a/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/pushsubscription/CassandraPushSubscriptionDAO.java b/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/pushsubscription/CassandraPushSubscriptionDAO.java index fa04634d00a..4437a1bac37 100644 --- a/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/pushsubscription/CassandraPushSubscriptionDAO.java +++ b/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/pushsubscription/CassandraPushSubscriptionDAO.java @@ -177,7 +177,7 @@ private PushSubscriptionExpiredTime toExpires(Row row) { private Seq toTypes(Row row) { return CollectionConverters.asScala(row.getSet(TYPES, String.class).stream() - .map(string -> typeStateFactory.parse(string).right().get()) + .flatMap(string -> OptionConverters.toJava(typeStateFactory.lenientParse(string)).stream()) .collect(ImmutableSet.toImmutableSet())) .toSeq(); } diff --git a/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/change/TypeStateFactory.scala b/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/change/TypeStateFactory.scala index 0efac72617a..a2c49af2702 100644 --- a/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/change/TypeStateFactory.scala +++ b/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/change/TypeStateFactory.scala @@ -21,15 +21,25 @@ package org.apache.james.jmap.api.change import jakarta.inject.Inject import org.apache.james.jmap.api.model.TypeName +import org.slf4j.{Logger, LoggerFactory} import scala.jdk.CollectionConverters._ case class TypeStateFactory @Inject()(setTypeName: java.util.Set[TypeName]) { + val logger: Logger = LoggerFactory.getLogger(classOf[TypeStateFactory]) val all: scala.collection.mutable.Set[TypeName] = setTypeName.asScala - def parse(string: String): Either[IllegalArgumentException, TypeName] = + def strictParse(string: String): Either[IllegalArgumentException, TypeName] = all.flatMap(_.parse(string)) .headOption .map(Right(_)) .getOrElse(Left(new IllegalArgumentException(s"Unknown typeName $string"))) + + def lenientParse(string: String): Option[TypeName] = + all.flatMap(_.parse(string)).headOption match { + case Some(value) => Some(value) + case None => + logger.warn("Leniently ignore {} while parsing TypeName", string) + None + } } diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala index bb77811a0b6..3dc43c86709 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala @@ -40,7 +40,7 @@ case class StateChangeEventDTOFactory @Inject()(typeStateFactory: TypeStateFacto .toDomainObjectConverter(dto => dto.toDomainObject(typeStateFactory)) .toDTOConverter((event, aType) => toDTO(event)) .typeName(classOf[StateChangeEvent].getCanonicalName) - .withFactory(EventDTOModule.apply); + .withFactory(EventDTOModule.apply) def toDTO(event: StateChangeEvent): StateChangeEventDTO = StateChangeEventDTO( getType = classOf[StateChangeEvent].getCanonicalName, @@ -66,7 +66,7 @@ case class StateChangeEventDTO(@JsonProperty("type") getType: String, map = typeStatesFromMap(typeStateFactory)) private def typeStatesFromMap(typeStateFactory: TypeStateFactory): Map[TypeName, State] = - getTypeStates.toScala.map(typeStates => typeStates.asScala.flatMap(element => typeStateFactory.parse(element._1).toOption + getTypeStates.toScala.map(typeStates => typeStates.asScala.flatMap(element => typeStateFactory.lenientParse(element._1) .flatMap(typeName => typeName.parseState(element._2).toOption.map(state => typeName -> state))).toMap) .getOrElse(fallbackToOldFormat()) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/PushSubscriptionSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/PushSubscriptionSet.scala index 9e16c178b60..9125567f01e 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/PushSubscriptionSet.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/PushSubscriptionSet.scala @@ -137,7 +137,7 @@ object TypesUpdate { case _ => Left(InvalidUpdateException("types", "Expecting an array of JSON strings as an argument")) } def parseType(jsValue: JsValue, typeStateFactory: TypeStateFactory): Either[PatchUpdateValidationException, TypeName] = jsValue match { - case JsString(aString) => typeStateFactory.parse(aString).left.map(e => InvalidUpdateException("types", e.getMessage)) + case JsString(aString) => typeStateFactory.strictParse(aString).left.map(e => InvalidUpdateException("types", e.getMessage)) case _ => Left(InvalidUpdateException("types", "Expecting an array of JSON strings as an argument")) } } diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSerializer.scala index 63cd65d2d76..7328de2fd80 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSerializer.scala @@ -56,7 +56,7 @@ case class PushSerializer @Inject()(typeStateFactory: TypeStateFactory) { case _ => JsError("Expecting a JsObject to represent a webSocket inbound request") } private implicit val typeNameReads: Reads[TypeName] = { - case JsString(string) => typeStateFactory.parse(string) + case JsString(string) => typeStateFactory.strictParse(string) .fold(throwable => JsError(throwable.getMessage), JsSuccess(_)) case _ => JsError("Expecting a JsString as typeName") } diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSubscriptionSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSubscriptionSerializer.scala index f7c6d2666d7..2d652286502 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSubscriptionSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/PushSubscriptionSerializer.scala @@ -56,7 +56,7 @@ class PushSubscriptionSerializer @Inject()(typeStateFactory: TypeStateFactory) { ) (PushSubscriptionKeys.apply _) private implicit val typeNameReads: Reads[TypeName] = { - case JsString(serializeValue) => typeStateFactory.parse(serializeValue) + case JsString(serializeValue) => typeStateFactory.strictParse(serializeValue) .fold(e => JsError(e.getMessage), v => JsSuccess(v)) case _ => JsError() } diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/EventSourceRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/EventSourceRoutes.scala index 4ca02ff85dc..a51f1de8a5b 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/EventSourceRoutes.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/EventSourceRoutes.scala @@ -74,7 +74,7 @@ case class EventSourceOptionsFactory @Inject() (typeStateFactory: TypeStateFacto case None => Left(new IllegalArgumentException("types parameter is compulsory")) case Some(List("*")) => Right(typeStateFactory.all.toSet) case Some(list) => list.flatMap(_.split(',')) - .map(string => typeStateFactory.parse(string)) + .map(string => typeStateFactory.strictParse(string)) .sequence.map(_.toSet) } diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/StateChangeEventSerializerTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/StateChangeEventSerializerTest.scala index 62b6648c3a5..4d2ea8da8e3 100644 --- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/StateChangeEventSerializerTest.scala +++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/StateChangeEventSerializerTest.scala @@ -101,6 +101,25 @@ class StateChangeEventSerializerTest { }) .verify() + @Test + def deserializeEventShouldLenientlyIgnoreUnknownTypeName(): Unit = + assertThat(JsonGenericSerializer + .forModules(stateChangeEventDTOFactory.dtoModule) + .withoutNestedType() + .deserialize("""{ + | "eventId": "6e0dd59d-660e-4d9b-b22f-0354479f47b4", + | "username": "bob", + | "typeStates": { + | "Mailbox": "2c9f1b12-b35a-43e6-9af2-0106fb53a943", + | "Email": "2d9f1b12-b35a-43e6-9af2-0106fb53a943", + | "EmailDelivery": "2d9f1b12-0000-1111-3333-0106fb53a943", + | "VacationResponse": "2d9f1b12-3333-4444-5555-0106fb53a943", + | "FastForwardTypeName": "2d9f1b12-3333-4444-5555-0106fb53a943" + | }, + | "type": "org.apache.james.jmap.change.StateChangeEvent" + |}""".stripMargin)) + .isEqualTo(EVENT) + @Test def shouldThrowWhenDeserializeUnknownEvent(): Unit = assertThatThrownBy(() => diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/TypeStateFactoryTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/TypeStateFactoryTest.scala index 9e230c083bc..9c9abe4daa5 100644 --- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/TypeStateFactoryTest.scala +++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/TypeStateFactoryTest.scala @@ -32,44 +32,67 @@ class TypeStateFactoryTest { val factory: TypeStateFactory = TypeStateFactory(ALL.asJava) @Test - def parseEmailTypeNameStringShouldReturnRightEmailTypeName(): Unit = - assertThat(factory.parse("Email")).isEqualTo(Right(EmailTypeName)) + def strictParseEmailTypeNameStringShouldReturnRightEmailTypeName(): Unit = + assertThat(factory.strictParse("Email")).isEqualTo(Right(EmailTypeName)) @Test - def parseMailboxTypeNameStringShouldReturnRightMailboxTypeName(): Unit = - assertThat(factory.parse("Mailbox")).isEqualTo(Right(MailboxTypeName)) + def strictParseMailboxTypeNameStringShouldReturnRightMailboxTypeName(): Unit = + assertThat(factory.strictParse("Mailbox")).isEqualTo(Right(MailboxTypeName)) @Test - def parseThreadTypeNameStringShouldReturnRightThreadTypeName(): Unit = - assertThat(factory.parse("Thread")).isEqualTo(Right(ThreadTypeName)) + def strictParseThreadTypeNameStringShouldReturnRightThreadTypeName(): Unit = + assertThat(factory.strictParse("Thread")).isEqualTo(Right(ThreadTypeName)) @Test - def parseIdentityTypeNameStringShouldReturnRightIdentityTypeName(): Unit = - assertThat(factory.parse("Identity")).isEqualTo(Right(IdentityTypeName)) + def strictParseIdentityTypeNameStringShouldReturnRightIdentityTypeName(): Unit = + assertThat(factory.strictParse("Identity")).isEqualTo(Right(IdentityTypeName)) @Test - def parseEmailSubmissionTypeNameStringShouldReturnRightEmailSubmissionTypeName(): Unit = - assertThat(factory.parse("EmailSubmission")).isEqualTo(Right(EmailSubmissionTypeName)) + def strictParseEmailSubmissionTypeNameStringShouldReturnRightEmailSubmissionTypeName(): Unit = + assertThat(factory.strictParse("EmailSubmission")).isEqualTo(Right(EmailSubmissionTypeName)) @Test - def parseEmailDeliveryTypeNameStringShouldReturnRightEmailDeliveryTypeName(): Unit = - assertThat(factory.parse("EmailDelivery")).isEqualTo(Right(EmailDeliveryTypeName)) + def strictParseEmailDeliveryTypeNameStringShouldReturnRightEmailDeliveryTypeName(): Unit = + assertThat(factory.strictParse("EmailDelivery")).isEqualTo(Right(EmailDeliveryTypeName)) @Test - def parseVacationResponseTypeNameStringShouldReturnRightVacationResponseTypeName(): Unit = - assertThat(factory.parse("VacationResponse")).isEqualTo(Right(VacationResponseTypeName)) + def strictParseVacationResponseTypeNameStringShouldReturnRightVacationResponseTypeName(): Unit = + assertThat(factory.strictParse("VacationResponse")).isEqualTo(Right(VacationResponseTypeName)) @Test - def parseWrongTypeNameStringShouldReturnLeft(): Unit = + def strictParseWrongTypeNameStringShouldReturnLeft(): Unit = SoftAssertions.assertSoftly(softly => { - softly.assertThat(factory.parse("email").isLeft).isTrue - softly.assertThat(factory.parse("mailbox").isLeft).isTrue - softly.assertThat(factory.parse("thread").isLeft).isTrue - softly.assertThat(factory.parse("identity").isLeft).isTrue - softly.assertThat(factory.parse("emailSubmission").isLeft).isTrue - softly.assertThat(factory.parse("emailDelivery").isLeft).isTrue - softly.assertThat(factory.parse("filter").isLeft).isTrue - softly.assertThat(factory.parse("vacationResponse").isLeft).isTrue + softly.assertThat(factory.strictParse("email").isLeft).isTrue + softly.assertThat(factory.strictParse("mailbox").isLeft).isTrue + softly.assertThat(factory.strictParse("thread").isLeft).isTrue + softly.assertThat(factory.strictParse("identity").isLeft).isTrue + softly.assertThat(factory.strictParse("emailSubmission").isLeft).isTrue + softly.assertThat(factory.strictParse("emailDelivery").isLeft).isTrue + softly.assertThat(factory.strictParse("filter").isLeft).isTrue + softly.assertThat(factory.strictParse("vacationResponse").isLeft).isTrue }) + @Test + def lenientParseValidTypeNameStringShouldReturnTypeName(): Unit = { + assertThat(factory.lenientParse("Email")).isEqualTo(Some(EmailTypeName)) + assertThat(factory.lenientParse("Mailbox")).isEqualTo(Some(MailboxTypeName)) + assertThat(factory.lenientParse("Thread")).isEqualTo(Some(ThreadTypeName)) + assertThat(factory.lenientParse("Identity")).isEqualTo(Some(IdentityTypeName)) + assertThat(factory.lenientParse("EmailSubmission")).isEqualTo(Some(EmailSubmissionTypeName)) + assertThat(factory.lenientParse("EmailDelivery")).isEqualTo(Some(EmailDeliveryTypeName)) + assertThat(factory.lenientParse("VacationResponse")).isEqualTo(Some(VacationResponseTypeName)) + } + + @Test + def lenientParseWrongTypeNameStringShouldReturnEmpty(): Unit = + SoftAssertions.assertSoftly(softly => { + softly.assertThat(factory.lenientParse("email").isEmpty).isTrue + softly.assertThat(factory.lenientParse("mailbox").isEmpty).isTrue + softly.assertThat(factory.lenientParse("thread").isEmpty).isTrue + softly.assertThat(factory.lenientParse("identity").isEmpty).isTrue + softly.assertThat(factory.lenientParse("emailSubmission").isEmpty).isTrue + softly.assertThat(factory.lenientParse("emailDelivery").isEmpty).isTrue + softly.assertThat(factory.lenientParse("filter").isEmpty).isTrue + softly.assertThat(factory.lenientParse("vacationResponse").isEmpty).isTrue + }) }