You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/02/03 02:51:17 UTC

[james-project] 11/12: JAMES-3491 Cleanup WebSocket tests

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 72a1ae1da39fcd4125ea406ab977c58f7871f836
Author: Raphael Ouazana <ra...@linagora.com>
AuthorDate: Thu Jan 28 15:53:32 2021 +0100

    JAMES-3491 Cleanup WebSocket tests
---
 .../jmap/rfc8621/contract/WebSocketContract.scala  | 252 ++++++---------------
 ...ocketContract.java => MemoryWebSocketTest.java} |   2 +-
 .../apache/james/jmap/routes/WebSocketRoutes.scala |   6 +-
 3 files changed, 68 insertions(+), 192 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/WebSocketContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/WebSocketContract.scala
index 8b87d55..7bf45a3 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/WebSocketContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/WebSocketContract.scala
@@ -20,19 +20,32 @@ package org.apache.james.jmap.rfc8621.contract
 
 import java.net.URI
 import java.util
+import java.util.concurrent.TimeUnit
 
 import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson
 import org.apache.james.GuiceJamesServer
 import org.apache.james.jmap.draft.JmapGuiceProbe
 import org.apache.james.jmap.rfc8621.contract.Fixture._
+import org.apache.james.jmap.rfc8621.contract.WebSocketContract.{LOGGER, await}
 import org.apache.james.jmap.rfc8621.contract.tags.CategoryTags
 import org.apache.james.utils.DataProbeImpl
 import org.assertj.core.api.Assertions.assertThat
+import org.awaitility.Awaitility
 import org.java_websocket.client.WebSocketClient
 import org.java_websocket.handshake.ServerHandshake
 import org.junit.jupiter.api.{BeforeEach, Tag, Test}
+import org.slf4j.{Logger, LoggerFactory}
+
+object WebSocketContract {
+  val LOGGER: Logger = LoggerFactory.getLogger(classOf[WebSocketContract])
+
+  val await = Awaitility.await
+    .atMost(1, TimeUnit.SECONDS)
+    .pollInterval(100, TimeUnit.MILLISECONDS)
+}
 
 trait WebSocketContract {
+
   @BeforeEach
   def setUp(server: GuiceJamesServer): Unit = {
     server.getProbe(classOf[DataProbeImpl])
@@ -41,46 +54,24 @@ trait WebSocketContract {
       .addUser(BOB.asString(), BOB_PASSWORD)
   }
 
-  class ExampleClient(uri: URI) extends WebSocketClient(uri) {
+  class TestClient(uri: URI) extends WebSocketClient(uri) {
     val receivedResponses: util.LinkedList[String] = new util.LinkedList[String]()
-    var closeCode: Option[Integer] = None
     var closeString: Option[String] = None
 
-    override def onOpen(serverHandshake: ServerHandshake): Unit = {
-      println(s"handshake ${serverHandshake.getHttpStatus}")
-    }
+    override def onOpen(serverHandshake: ServerHandshake): Unit = {}
 
-    override def onMessage(s: String): Unit = {
-      println(s"Received: $s")
-      receivedResponses.add(s)
-    }
+    override def onMessage(s: String): Unit = receivedResponses.add(s)
 
-    override def onClose(i: Int, s: String, b: Boolean): Unit = {
-      closeCode = Some(i)
-      closeString = Some(s)
-      println(s"Closing connection $i $s $b")
-    }
+    override def onClose(i: Int, s: String, b: Boolean): Unit = closeString = Some(s)
 
-    override def onError(e: Exception): Unit = {
-      println("Error: " + e.getMessage)
-    }
+    override def onError(e: Exception): Unit = LOGGER.error("WebSocket error", e)
   }
 
   @Test
   @Tag(CategoryTags.BASIC_FEATURE)
   def apiRequestsShouldBeProcessed(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send("""{
                   |  "@type": "Request",
                   |  "requestId": "req-36",
@@ -97,35 +88,19 @@ trait WebSocketContract {
                   |  ]
                   |}""".stripMargin)
 
-    Thread.sleep(500)
-
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.until(() => client.receivedResponses.size() == 1)
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "@type":"Response",
         |  "requestId":"req-36",
         |  "sessionState":"2c9f1b12-b35a-43e6-9af2-0106fb53a943",
         |  "methodResponses":[["Core/echo",{"arg1":"arg1data","arg2":"arg2data"},"c1"]]
-        |}
-        |""".stripMargin)
+        |}""".stripMargin)
   }
 
   @Test
   def apiRequestsShouldBeProcessedWhenNoRequestId(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send("""{
                   |  "@type": "Request",
                   |  "using": [ "urn:ietf:params:jmap:core"],
@@ -141,82 +116,44 @@ trait WebSocketContract {
                   |  ]
                   |}""".stripMargin)
 
-    Thread.sleep(500)
-
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "@type":"Response",
         |  "requestId":null,
         |  "sessionState":"2c9f1b12-b35a-43e6-9af2-0106fb53a943",
         |  "methodResponses":[["Core/echo",{"arg1":"arg1data","arg2":"arg2data"},"c1"]]
-        |}
-        |""".stripMargin)
+        |}""".stripMargin)
   }
 
   @Test
   def nonJsonPayloadShouldTriggerError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send("The quick brown fox".stripMargin)
 
-    Thread.sleep(500)
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "status":400,
         |  "detail":"The request was successfully parsed as JSON but did not match the type signature of the Request object: List((,List(JsonValidationError(List(Unrecognized token 'The': was expecting ('true', 'false' or 'null')\n at [Source: (String)\"The quick brown fox\"; line: 1, column: 4]),ArraySeq()))))",
         |  "type":"urn:ietf:params:jmap:error:notRequest",
         |  "requestId":null,
         |  "@type":"RequestError"
-        |}
-        |""".stripMargin)
+        |}""".stripMargin)
   }
 
   @Test
   def handshakeShouldBeAuthenticated(server: GuiceJamesServer): Unit = {
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = unauthenticatedWebSocketClient(server)
     client.connectBlocking()
 
-    Thread.sleep(100)
-
     assertThat(client.isClosed).isTrue
     assertThat(client.closeString).isEqualTo(Some("Invalid status code received: 401 Status line: HTTP/1.1 401 Unauthorized"))
   }
 
   @Test
   def noTypeFiledShouldTriggerError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send("""{
                   |  "requestId": "req-36",
                   |  "using": [ "urn:ietf:params:jmap:core"],
@@ -232,37 +169,20 @@ trait WebSocketContract {
                   |  ]
                   |}""".stripMargin)
 
-    Thread.sleep(500)
-
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.until(() => client.receivedResponses.size() == 1)
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "status":400,
         |  "detail":"The request was successfully parsed as JSON but did not match the type signature of the Request object: List((,List(JsonValidationError(List(Missing @type filed on a webSocket inbound message),ArraySeq()))))",
         |  "type":"urn:ietf:params:jmap:error:notRequest",
         |  "requestId":null,
         |  "@type":"RequestError"
-        |}
-        |""".stripMargin
-    )
+        |}""".stripMargin)
   }
 
   @Test
   def badTypeFieldShouldTriggerError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send("""{
                   |  "@type": 42,
                   |  "requestId": "req-36",
@@ -279,37 +199,21 @@ trait WebSocketContract {
                   |  ]
                   |}""".stripMargin)
 
-    Thread.sleep(500)
-
-    assertThat(client.receivedResponses).hasSize(1)
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
     assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+      """{
         |  "status":400,
         |  "detail":"The request was successfully parsed as JSON but did not match the type signature of the Request object: List((,List(JsonValidationError(List(Invalid @type filed on a webSocket inbound message: expecting a JsString, got 42),ArraySeq()))))",
         |  "type":"urn:ietf:params:jmap:error:notRequest",
         |  "requestId":null,
         |  "@type":"RequestError"
-        |}
-        |""".stripMargin
-    )
-
+        |}""".stripMargin)
   }
 
   @Test
   def unknownTypeFieldShouldTriggerError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send(
       """{
         |  "@type": "unknown",
@@ -327,37 +231,21 @@ trait WebSocketContract {
         |  ]
         |}""".stripMargin)
 
-    Thread.sleep(500)
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "status":400,
         |  "detail":"The request was successfully parsed as JSON but did not match the type signature of the Request object: List((,List(JsonValidationError(List(Unknown @type filed on a webSocket inbound message: unknown),ArraySeq()))))",
         |  "type":"urn:ietf:params:jmap:error:notRequest",
         |  "requestId":null,
         |  "@type":"RequestError"
-        |}
-        |""".stripMargin
-    )
+        |}""".stripMargin)
   }
 
 
   @Test
   def clientSendingARespondTypeFieldShouldTriggerError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send(
       """{
         |  "@type": "Response",
@@ -375,37 +263,20 @@ trait WebSocketContract {
         |  ]
         |}""".stripMargin)
 
-    Thread.sleep(500)
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "status":400,
         |  "detail":"The request was successfully parsed as JSON but did not match the type signature of the Request object: List((,List(JsonValidationError(List(Unknown @type filed on a webSocket inbound message: Response),ArraySeq()))))",
         |  "type":"urn:ietf:params:jmap:error:notRequest",
         |  "requestId":null,
         |  "@type":"RequestError"
-        |}
-        |""".stripMargin
-    )
-
+        |}""".stripMargin)
   }
 
   @Test
   def requestLevelErrorShouldReturnAPIError(server: GuiceJamesServer): Unit = {
-    println("started")
-    val port = server.getProbe(classOf[JmapGuiceProbe])
-      .getJmapPort
-      .getValue
-    val client = new ExampleClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
-    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
-    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
-
+    val client: TestClient = authenticatedWebSocketClient(server)
     client.connectBlocking()
-
-    Thread.sleep(500)
-
     client.send(s"""{
                    |  "@type": "Request",
                    |  "using": [
@@ -420,18 +291,27 @@ trait WebSocketContract {
                    |      "c1"]]
                    |}""".stripMargin)
 
-    Thread.sleep(500)
-
-
-    assertThat(client.receivedResponses).hasSize(1)
-    assertThatJson(client.receivedResponses.get(0)).isEqualTo(
-      """
-        |{
+    await.untilAsserted(() => assertThat(client.receivedResponses).hasSize(1))
+    assertThatJson(client.receivedResponses.get(0)).isEqualTo("""{
         |  "@type": "Response",
         |  "requestId": null,
         |  "sessionState": "2c9f1b12-b35a-43e6-9af2-0106fb53a943",
         |  "methodResponses": [["error",{"type":"invalidArguments","description":"The following properties [invalidProperty] do not exist."},"c1"]]
-        |}
-        |""".stripMargin)
+        |}""".stripMargin)
+  }
+
+  private def unauthenticatedWebSocketClient(server: GuiceJamesServer): TestClient = {
+    val port = server.getProbe(classOf[JmapGuiceProbe])
+      .getJmapPort
+      .getValue
+    val client = new TestClient(new URI(s"ws://127.0.0.1:$port/jmap/ws"))
+    client.addHeader("Accept", ACCEPT_RFC8621_VERSION_HEADER)
+    client
+  }
+
+  private def authenticatedWebSocketClient(server: GuiceJamesServer): TestClient = {
+    val client = unauthenticatedWebSocketClient(server)
+    client.addHeader("Authorization", "Basic Ym9iQGRvbWFpbi50bGQ6Ym9icGFzc3dvcmQ=")
+    client
   }
 }
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketContract.java b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketTest.java
similarity index 96%
rename from server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketContract.java
rename to server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketTest.java
index b128f72..ce526e6 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketContract.java
+++ b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryWebSocketTest.java
@@ -28,7 +28,7 @@ import org.apache.james.jmap.rfc8621.contract.WebSocketContract;
 import org.apache.james.modules.TestJMAPServerModule;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-public class MemoryWebSocketContract implements WebSocketContract {
+public class MemoryWebSocketTest implements WebSocketContract {
     @RegisterExtension
     static JamesServerExtension testExtension = new JamesServerBuilder<>(JamesServerBuilder.defaultConfigurationProvider())
         .server(configuration -> GuiceJamesServer.forConfiguration(configuration)
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala
index 3965ac7..4664eba 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala
@@ -79,14 +79,10 @@ class WebSocketRoutes @Inject() (@Named(InjectionKeys.RFC_8621) val authenticato
         new String(bytes, StandardCharsets.UTF_8)
       })
       .flatMap(handleClientMessages(session))
-      .onErrorResume(e => SMono.just(asError(None)(e)))
+      .onErrorResume(e => SMono.just[WebSocketOutboundMessage](asError(None)(e)))
       .map(ResponseSerializer.serialize)
       .map(_.toString)
       .flatMap(response => out.sendString(SMono.just(response), StandardCharsets.UTF_8))
-      .onErrorResume(e => {
-        e.printStackTrace()
-        SMono.empty
-      })
       .`then`()
       .asJava()
       .`then`()


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