[java] Fix unsubscribe event BiDi API methods

craigfowler · Jun 22, 2022 · cc00a3c · cc00a3c
1 parent 01fe86e
commit cc00a3c
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 39 deletions.
diff --git a/java/src/org/openqa/selenium/bidi/BiDi.java b/java/src/org/openqa/selenium/bidi/BiDi.java
@@ -42,6 +42,7 @@ public BiDi(Connection connection) {
   public void close() {
     clearListeners();
     disconnectSession();
+    connection.close();
   }
 
   public void disconnectSession() {
@@ -64,8 +65,16 @@ public <X> void addListener(Event<X> event, Consumer<X> handler) {
     connection.addListener(event, handler);
   }
 
+  public <X> void clearListener(Event<X> event) {
+    Require.nonNull("Event to listen for", event);
+
+    send(new Command<>("session.unsubscribe",
+                       ImmutableMap.of("events", Collections.singletonList(event.getMethod()))));
+
+    connection.clearListener(event);
+  }
+
   public void clearListeners() {
-    send(new Command<>("session.unsubscribe", Collections.emptyMap()));
     connection.clearListeners();
   }
 

diff --git a/java/src/org/openqa/selenium/bidi/Connection.java b/java/src/org/openqa/selenium/bidi/Connection.java
@@ -39,6 +39,8 @@
 import java.io.Closeable;
 import java.io.StringReader;
 import java.time.Duration;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
@@ -53,6 +55,7 @@
 import java.util.function.Consumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 public class Connection implements Closeable {
 
@@ -166,10 +169,28 @@ public <X> void addListener(Event<X> event, Consumer<X> handler) {
     }
   }
 
+  public <X> void clearListener(Event<X> event) {
+    Lock lock = callbacksLock.writeLock();
+    lock.lock();
+    try {
+      eventCallbacks.removeAll(event);
+    } finally {
+      lock.unlock();
+    }
+  }
+
   public void clearListeners() {
     Lock lock = callbacksLock.writeLock();
     lock.lock();
     try {
+      List<String> events = eventCallbacks.keySet()
+        .stream()
+        .map(Event::getMethod)
+        .collect(Collectors.toList());
+
+      send(new Command<>("session.unsubscribe",
+                         ImmutableMap.of("events", events)));
+
       eventCallbacks.clear();
     } finally {
       lock.unlock();

diff --git a/java/test/org/openqa/selenium/bidi/log/BiDiLogTest.java b/java/test/org/openqa/selenium/bidi/log/BiDiLogTest.java
@@ -61,23 +61,24 @@ public void canListenToConsoleLog()
 
     CompletableFuture<LogEntry> future = new CompletableFuture<>();
 
-    BiDi biDi = driver.getBiDi();
+    try (BiDi biDi = driver.getBiDi()) {
 
-    biDi.addListener(Log.entryAdded(), future::complete);
+      biDi.addListener(Log.entryAdded(), future::complete);
 
-    driver.findElement(By.id("consoleLog")).click();
-    LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
+      driver.findElement(By.id("consoleLog")).click();
+      LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
 
-    assertThat(logEntry.getConsoleLogEntry().isPresent()).isTrue();
+      assertThat(logEntry.getConsoleLogEntry().isPresent()).isTrue();
 
-    ConsoleLogEntry consoleLogEntry = logEntry.getConsoleLogEntry().get();
-    assertThat(consoleLogEntry.getText()).isEqualTo("Hello, world!");
-    assertThat(consoleLogEntry.getRealm()).isNull();
-    assertThat(consoleLogEntry.getArgs().size()).isEqualTo(1);
-    assertThat(consoleLogEntry.getType()).isEqualTo("console");
-    assertThat(consoleLogEntry.getLevel()).isEqualTo(BaseLogEntry.LogLevel.INFO);
-    assertThat(consoleLogEntry.getMethod()).isEqualTo("log");
-    assertThat(consoleLogEntry.getStackTrace()).isNull();
+      ConsoleLogEntry consoleLogEntry = logEntry.getConsoleLogEntry().get();
+      assertThat(consoleLogEntry.getText()).isEqualTo("Hello, world!");
+      assertThat(consoleLogEntry.getRealm()).isNull();
+      assertThat(consoleLogEntry.getArgs().size()).isEqualTo(1);
+      assertThat(consoleLogEntry.getType()).isEqualTo("console");
+      assertThat(consoleLogEntry.getLevel()).isEqualTo(BaseLogEntry.LogLevel.INFO);
+      assertThat(consoleLogEntry.getMethod()).isEqualTo("log");
+      assertThat(consoleLogEntry.getStackTrace()).isNull();
+    }
   }
 
   @Test
@@ -89,19 +90,19 @@ public void canListenToJavascriptLog()
 
     CompletableFuture<LogEntry> future = new CompletableFuture<>();
 
-    BiDi biDi = driver.getBiDi();
-
-    biDi.addListener(Log.entryAdded(), future::complete);
+    try (BiDi biDi = driver.getBiDi()) {
+      biDi.addListener(Log.entryAdded(), future::complete);
 
-    driver.findElement(By.id("jsException")).click();
-    LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
+      driver.findElement(By.id("jsException")).click();
+      LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
 
-    assertThat(logEntry.getJavascriptLogEntry().isPresent()).isTrue();
+      assertThat(logEntry.getJavascriptLogEntry().isPresent()).isTrue();
 
-    GenericLogEntry javascriptLogEntry = logEntry.getJavascriptLogEntry().get();
-    assertThat(javascriptLogEntry.getText()).isEqualTo("Error: Not working");
-    assertThat(javascriptLogEntry.getType()).isEqualTo("javascript");
-    assertThat(javascriptLogEntry.getLevel()).isEqualTo(BaseLogEntry.LogLevel.ERROR);
+      GenericLogEntry javascriptLogEntry = logEntry.getJavascriptLogEntry().get();
+      assertThat(javascriptLogEntry.getText()).isEqualTo("Error: Not working");
+      assertThat(javascriptLogEntry.getType()).isEqualTo("javascript");
+      assertThat(javascriptLogEntry.getLevel()).isEqualTo(BaseLogEntry.LogLevel.ERROR);
+    }
   }
 
   @Test
@@ -113,17 +114,18 @@ public void canRetrieveStacktraceForALog()
 
     CompletableFuture<LogEntry> future = new CompletableFuture<>();
 
-    BiDi biDi = driver.getBiDi();
+    try (BiDi biDi = driver.getBiDi()) {
 
-    biDi.addListener(Log.entryAdded(), future::complete);
+      biDi.addListener(Log.entryAdded(), future::complete);
 
-    driver.findElement(By.id("logWithStacktrace")).click();
-    LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
+      driver.findElement(By.id("logWithStacktrace")).click();
+      LogEntry logEntry = future.get(5, TimeUnit.SECONDS);
 
-    assertThat(logEntry.getJavascriptLogEntry().isPresent()).isTrue();
-    StackTrace stackTrace = logEntry.getJavascriptLogEntry().get().getStackTrace();
-    assertThat(stackTrace).isNotNull();
-    assertThat(stackTrace.getCallFrames().size()).isEqualTo(4);
+      assertThat(logEntry.getJavascriptLogEntry().isPresent()).isTrue();
+      StackTrace stackTrace = logEntry.getJavascriptLogEntry().get().getStackTrace();
+      assertThat(stackTrace).isNotNull();
+      assertThat(stackTrace.getCallFrames().size()).isEqualTo(4);
+    }
   }
 
   @After

diff --git a/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java b/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java
@@ -54,12 +54,11 @@ void ensureBiDiSessionCreation() {
     WebDriver driver = new RemoteWebDriver(deployment.getServer().getUrl(), options);
     driver = new Augmenter().augment(driver);
 
-    BiDi biDi = ((HasBiDi) driver).getBiDi();
-
-    BiDiSessionStatus status =
-      biDi.send(new Command<>("session.status", Collections.emptyMap(), BiDiSessionStatus.class));
-    assertThat(status).isNotNull();
-    assertThat(status.getMessage()).isEqualTo("Session already started");
-
+    try (BiDi biDi = ((HasBiDi) driver).getBiDi()) {
+      BiDiSessionStatus status =
+        biDi.send(new Command<>("session.status", Collections.emptyMap(), BiDiSessionStatus.class));
+      assertThat(status).isNotNull();
+      assertThat(status.getMessage()).isEqualTo("Session already started");
+    }
   }
 }