chibenwa commented on a change in pull request #391:
URL: https://github.com/apache/james-project/pull/391#discussion_r623186096
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedMDNSendMethodTest.java
##########
@@ -56,8 +54,7 @@
.extension(new CassandraExtension())
.extension(new RabbitMQExtension())
.extension(new AwsS3BlobStoreExtension())
- .server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
- .overrideWith(new TestJMAPServerModule()))
+ .server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration))
Review comment:
You lost TestJMAPServerModule
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/CustomMethodContract.scala
##########
@@ -179,6 +231,80 @@ trait CustomMethodContract {
.build
}
+ @Test
+ def
pushShouldSupportCustomTypeNameAndIntStateWithDataTypesAreMyTypeName(server:
GuiceJamesServer): Unit = {
+ val accountId: AccountId = AccountId.fromUsername(BOB)
+ val intState = IntState.fromString("1")
+ val stateChangeEvent: StateChangeEvent = StateChangeEvent(eventId =
CustomMethodContract.eventId, username = BOB, map = Map(CustomTypeName ->
intState))
+ Thread.sleep(100)
+
+ val response: Either[String, List[String]] =
+ authenticatedRequest(server)
+ .response(asWebSocket[Identity, List[String]] {
+ ws =>
+ ws.send(WebSocketFrame.text(
+ """{
+ | "@type": "WebSocketPushEnable",
+ | "dataTypes": ["MyTypeName"]
+ |}""".stripMargin))
+
+ Thread.sleep(100)
+
+ server.getProbe(classOf[JmapEventBusProbe])
+ .emitStateChange(stateChangeEvent, accountId)
+
+ List(ws.receive()
+ .map { case t: Text =>
+ t.payload
+ })
+ })
+ .send(backend)
+ .body
+
+ Thread.sleep(100)
+ val stateChange: String =
s"""{"@type":"StateChange","changed":{"$ACCOUNT_ID":{"MyTypeName":"${intState.serialize}"}}}"""
+
+ assertThat(response.toOption.get.asJava)
+ .containsOnly(stateChange)
+ }
+
+ @Test
+ def pushShouldSupportCustomTypeNameAndIntStateWithDataTypesAreNull(server:
GuiceJamesServer): Unit = {
+ val accountId: AccountId = AccountId.fromUsername(BOB)
+ val intState = IntState(1)
+ val stateChangeEvent: StateChangeEvent = StateChangeEvent(eventId =
CustomMethodContract.eventId, username = BOB, map = Map(CustomTypeName ->
intState))
+ Thread.sleep(100)
+
+ val response: Either[String, List[String]] =
+ authenticatedRequest(server)
+ .response(asWebSocket[Identity, List[String]] {
+ ws =>
+ ws.send(WebSocketFrame.text(
+ """{
+ | "@type": "WebSocketPushEnable",
+ | "dataTypes": null
+ |}""".stripMargin))
+
+ Thread.sleep(100)
+
+ server.getProbe(classOf[JmapEventBusProbe])
+ .emitStateChange(stateChangeEvent, accountId)
+
+ List(ws.receive()
Review comment:
Idem you can remove this list...
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/CustomMethodContract.scala
##########
@@ -179,6 +231,80 @@ trait CustomMethodContract {
.build
}
+ @Test
+ def
pushShouldSupportCustomTypeNameAndIntStateWithDataTypesAreMyTypeName(server:
GuiceJamesServer): Unit = {
+ val accountId: AccountId = AccountId.fromUsername(BOB)
+ val intState = IntState.fromString("1")
+ val stateChangeEvent: StateChangeEvent = StateChangeEvent(eventId =
CustomMethodContract.eventId, username = BOB, map = Map(CustomTypeName ->
intState))
+ Thread.sleep(100)
+
+ val response: Either[String, List[String]] =
Review comment:
1. Put `Either[String, String]` here...
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/TypeStateFactory.scala
##########
@@ -0,0 +1,15 @@
+package org.apache.james.jmap.change
Review comment:
Missing license
##########
File path:
server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/TypeStateFactoryTest.scala
##########
@@ -0,0 +1,49 @@
+package org.apache.james.jmap.change
Review comment:
License
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -42,10 +41,10 @@ object StateChangeEventDTO {
getType = classOf[StateChangeEvent].getCanonicalName,
getEventId = event.eventId.getId.toString,
getUsername = event.username.asString(),
- getMailboxState = event.mailboxState.map(_.value).map(_.toString).toJava,
- getEmailState = event.emailState.map(_.value).map(_.toString).toJava,
- getVacationResponseState =
event.vacationResponseState.map(_.value).map(_.toString).toJava,
- getEmailDeliveryState =
event.emailDeliveryState.map(_.value).map(_.toString).toJava)
+ getMailboxState = event.getState(MailboxTypeName).map(state =>
state.serialize).toJava,
+ getEmailState = event.getState(EmailTypeName).map(state =>
state.serialize).toJava,
Review comment:
```suggestion
getEmailState = event.getState(EmailTypeName).map(_.serialize).toJava,
```
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -42,10 +41,10 @@ object StateChangeEventDTO {
getType = classOf[StateChangeEvent].getCanonicalName,
getEventId = event.eventId.getId.toString,
getUsername = event.username.asString(),
- getMailboxState = event.mailboxState.map(_.value).map(_.toString).toJava,
- getEmailState = event.emailState.map(_.value).map(_.toString).toJava,
- getVacationResponseState =
event.vacationResponseState.map(_.value).map(_.toString).toJava,
- getEmailDeliveryState =
event.emailDeliveryState.map(_.value).map(_.toString).toJava)
+ getMailboxState = event.getState(MailboxTypeName).map(state =>
state.serialize).toJava,
+ getEmailState = event.getState(EmailTypeName).map(state =>
state.serialize).toJava,
+ getVacationResponseState =
event.getState(VacationResponseTypeName).map(state => state.serialize).toJava,
+ getEmailDeliveryState = event.getState(EmailDeliveryTypeName).map(state =>
state.serialize).toJava)
}
case class StateChangeEventDTO(@JsonProperty("type") getType: String,
Review comment:
The DTO should rely on a Map<String, String> so that it carries over
thegeneric typeStates no?
(Did you run the distributed version of your 'custom typeName' test?)
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -42,10 +41,10 @@ object StateChangeEventDTO {
getType = classOf[StateChangeEvent].getCanonicalName,
getEventId = event.eventId.getId.toString,
getUsername = event.username.asString(),
- getMailboxState = event.mailboxState.map(_.value).map(_.toString).toJava,
- getEmailState = event.emailState.map(_.value).map(_.toString).toJava,
- getVacationResponseState =
event.vacationResponseState.map(_.value).map(_.toString).toJava,
- getEmailDeliveryState =
event.emailDeliveryState.map(_.value).map(_.toString).toJava)
+ getMailboxState = event.getState(MailboxTypeName).map(state =>
state.serialize).toJava,
+ getEmailState = event.getState(EmailTypeName).map(state =>
state.serialize).toJava,
+ getVacationResponseState =
event.getState(VacationResponseTypeName).map(state => state.serialize).toJava,
+ getEmailDeliveryState = event.getState(EmailDeliveryTypeName).map(state =>
state.serialize).toJava)
Review comment:
```suggestion
getEmailDeliveryState =
event.getState(EmailDeliveryTypeName).map(_.serialize).toJava)
```
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/CustomMethodContract.scala
##########
@@ -179,6 +231,80 @@ trait CustomMethodContract {
.build
}
+ @Test
+ def
pushShouldSupportCustomTypeNameAndIntStateWithDataTypesAreMyTypeName(server:
GuiceJamesServer): Unit = {
+ val accountId: AccountId = AccountId.fromUsername(BOB)
+ val intState = IntState.fromString("1")
+ val stateChangeEvent: StateChangeEvent = StateChangeEvent(eventId =
CustomMethodContract.eventId, username = BOB, map = Map(CustomTypeName ->
intState))
+ Thread.sleep(100)
+
+ val response: Either[String, List[String]] =
+ authenticatedRequest(server)
+ .response(asWebSocket[Identity, List[String]] {
+ ws =>
+ ws.send(WebSocketFrame.text(
+ """{
+ | "@type": "WebSocketPushEnable",
+ | "dataTypes": ["MyTypeName"]
+ |}""".stripMargin))
+
+ Thread.sleep(100)
+
+ server.getProbe(classOf[JmapEventBusProbe])
+ .emitStateChange(stateChangeEvent, accountId)
+
+ List(ws.receive()
Review comment:
And 2. you can remove the `List` here
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -58,10 +57,13 @@ case class StateChangeEventDTO(@JsonProperty("type")
getType: String,
def toDomainObject: StateChangeEvent = StateChangeEvent(
eventId = EventId.of(getEventId),
username = Username.of(getUsername),
- vacationResponseState =
getVacationResponseState.toScala.map(State.fromStringUnchecked),
- mailboxState = getMailboxState.toScala.map(State.fromStringUnchecked),
- emailState = getEmailState.toScala.map(State.fromStringUnchecked),
- emailDeliveryState =
getEmailDeliveryState.toScala.map(State.fromStringUnchecked))
+ map = (toMap(getVacationResponseState, VacationResponseTypeName) ++
+ toMap(getMailboxState, MailboxTypeName) ++
+ toMap(getEmailState, EmailTypeName) ++
+ toMap(getEmailDeliveryState, EmailDeliveryTypeName)))
Review comment:
```suggestion
map = List(
VacationResponseTypeName -> getVacationResponseState, // state can be
optional...
MailboxResponseTypeName -> getMailboxResponseState, // state can be
optional...
// etc... )
.flatMap {
case (typeName, None) => None
case (typeName, Some(state)) => Some(typeName -> state)
}.toMap
```
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Session.scala
##########
@@ -31,7 +27,11 @@ import org.apache.james.core.Username
import org.apache.james.jmap.api.change.{EmailChanges, MailboxChanges, State
=> JavaState}
import org.apache.james.jmap.core.CapabilityIdentifier.CapabilityIdentifier
import org.apache.james.jmap.core.Id.Id
-import org.apache.james.jmap.core.State.INSTANCE
+import org.apache.james.jmap.core.UuidState.INSTANCE
+
+import java.net.URL
+import java.nio.charset.StandardCharsets
+import java.util.UUID
Review comment:
Oups wrong import reorder
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -42,10 +41,10 @@ object StateChangeEventDTO {
getType = classOf[StateChangeEvent].getCanonicalName,
getEventId = event.eventId.getId.toString,
getUsername = event.username.asString(),
- getMailboxState = event.mailboxState.map(_.value).map(_.toString).toJava,
- getEmailState = event.emailState.map(_.value).map(_.toString).toJava,
- getVacationResponseState =
event.vacationResponseState.map(_.value).map(_.toString).toJava,
- getEmailDeliveryState =
event.emailDeliveryState.map(_.value).map(_.toString).toJava)
+ getMailboxState = event.getState(MailboxTypeName).map(state =>
state.serialize).toJava,
+ getEmailState = event.getState(EmailTypeName).map(state =>
state.serialize).toJava,
+ getVacationResponseState =
event.getState(VacationResponseTypeName).map(state => state.serialize).toJava,
Review comment:
```suggestion
getVacationResponseState =
event.getState(VacationResponseTypeName).map(_.serialize).toJava,
```
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/JmapEventSerializer.scala
##########
@@ -42,10 +41,10 @@ object StateChangeEventDTO {
getType = classOf[StateChangeEvent].getCanonicalName,
getEventId = event.eventId.getId.toString,
getUsername = event.username.asString(),
- getMailboxState = event.mailboxState.map(_.value).map(_.toString).toJava,
- getEmailState = event.emailState.map(_.value).map(_.toString).toJava,
- getVacationResponseState =
event.vacationResponseState.map(_.value).map(_.toString).toJava,
- getEmailDeliveryState =
event.emailDeliveryState.map(_.value).map(_.toString).toJava)
+ getMailboxState = event.getState(MailboxTypeName).map(state =>
state.serialize).toJava,
Review comment:
```suggestion
getMailboxState =
event.getState(MailboxTypeName).map(_.serialize).toJava,
```
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/ResponseSerializer.scala
##########
@@ -35,9 +31,10 @@ import org.apache.james.jmap.core.{Account, Invocation,
Session, _}
import play.api.libs.functional.syntax._
import play.api.libs.json._
+import java.io.InputStream
+import java.net.URL
Review comment:
Oups wrong import reorder
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/CustomMethodContract.scala
##########
@@ -179,6 +231,80 @@ trait CustomMethodContract {
.build
}
+ @Test
+ def
pushShouldSupportCustomTypeNameAndIntStateWithDataTypesAreMyTypeName(server:
GuiceJamesServer): Unit = {
+ val accountId: AccountId = AccountId.fromUsername(BOB)
+ val intState = IntState.fromString("1")
+ val stateChangeEvent: StateChangeEvent = StateChangeEvent(eventId =
CustomMethodContract.eventId, username = BOB, map = Map(CustomTypeName ->
intState))
+ Thread.sleep(100)
+
+ val response: Either[String, List[String]] =
+ authenticatedRequest(server)
+ .response(asWebSocket[Identity, List[String]] {
+ ws =>
+ ws.send(WebSocketFrame.text(
+ """{
+ | "@type": "WebSocketPushEnable",
+ | "dataTypes": ["MyTypeName"]
+ |}""".stripMargin))
+
+ Thread.sleep(100)
+
+ server.getProbe(classOf[JmapEventBusProbe])
+ .emitStateChange(stateChangeEvent, accountId)
+
+ List(ws.receive()
+ .map { case t: Text =>
+ t.payload
+ })
+ })
+ .send(backend)
+ .body
+
+ Thread.sleep(100)
+ val stateChange: String =
s"""{"@type":"StateChange","changed":{"$ACCOUNT_ID":{"MyTypeName":"${intState.serialize}"}}}"""
+
+ assertThat(response.toOption.get.asJava)
Review comment:
3... that would make this assertion easier...
##########
File path:
server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryMDNSendMethodTest.java
##########
@@ -21,23 +21,21 @@
import static
org.apache.james.MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE;
+import java.util.concurrent.ThreadLocalRandom;
+
import org.apache.james.GuiceJamesServer;
import org.apache.james.JamesServerBuilder;
import org.apache.james.JamesServerExtension;
import org.apache.james.jmap.rfc8621.contract.MDNSendMethodContract;
import org.apache.james.mailbox.inmemory.InMemoryMessageId;
import org.apache.james.mailbox.model.MessageId;
-import org.apache.james.modules.TestJMAPServerModule;
import org.junit.jupiter.api.extension.RegisterExtension;
-import java.util.concurrent.ThreadLocalRandom;
-
public class MemoryMDNSendMethodTest implements MDNSendMethodContract {
@RegisterExtension
static JamesServerExtension testExtension = new
JamesServerBuilder<>(JamesServerBuilder.defaultConfigurationProvider())
.server(configuration ->
GuiceJamesServer.forConfiguration(configuration)
- .combineWith(IN_MEMORY_SERVER_AGGREGATE_MODULE)
- .overrideWith(new TestJMAPServerModule()))
+ .combineWith(IN_MEMORY_SERVER_AGGREGATE_MODULE))
Review comment:
You lost TestJMAPServerModule
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
