diff options
4 files changed, 38 insertions, 18 deletions
diff --git a/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/ServerPushClient.java b/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/ServerPushClient.java index 6d8b552e24a..a2004270b23 100644 --- a/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/ServerPushClient.java +++ b/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/ServerPushClient.java @@ -85,7 +85,7 @@ public abstract class ServerPushClient { private void handleIOException(IOException e) { String remoteAddr = asyncContext.getRequest().getRemoteAddr(); LOG.info(String.format("The server push client %s gone without notice, closing the connection (%s)", remoteAddr, e.getMessage())); - close(); + throw new IllegalStateException(e.getMessage()); } public synchronized void close() { diff --git a/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistry.java b/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistry.java index 008fdb0c3f9..6bdc3b7b580 100644 --- a/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistry.java +++ b/server/sonar-webserver-pushapi/src/main/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistry.java @@ -37,8 +37,8 @@ import org.sonar.core.util.ParamChange; import org.sonar.core.util.RuleActivationListener; import org.sonar.core.util.RuleChange; import org.sonar.core.util.RuleSetChangeEvent; -import org.sonar.server.pushapi.qualityprofile.RuleActivatorEventsDistributor; import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.pushapi.qualityprofile.RuleActivatorEventsDistributor; import static java.util.Arrays.asList; @@ -47,21 +47,19 @@ public class SonarLintClientsRegistry implements RuleActivationListener { private static final Logger LOG = Loggers.get(SonarLintClientsRegistry.class); - private final RuleActivatorEventsDistributor ruleActivatorEventsDistributor; private final SonarLintClientPermissionsValidator sonarLintClientPermissionsValidator; private final List<SonarLintClient> clients = new CopyOnWriteArrayList<>(); public SonarLintClientsRegistry(RuleActivatorEventsDistributor ruleActivatorEventsDistributor, SonarLintClientPermissionsValidator permissionsValidator) { - this.ruleActivatorEventsDistributor = ruleActivatorEventsDistributor; this.sonarLintClientPermissionsValidator = permissionsValidator; + + ruleActivatorEventsDistributor.subscribe(this); } public void registerClient(SonarLintClient sonarLintClient) { clients.add(sonarLintClient); sonarLintClient.scheduleHeartbeat(); sonarLintClient.addListener(new SonarLintClientEventsListener(sonarLintClient)); - ruleActivatorEventsDistributor.subscribe(this); - LOG.debug("Registering new SonarLint client"); } @@ -77,7 +75,6 @@ public class SonarLintClientsRegistry implements RuleActivationListener { @Override public void listen(RuleSetChangeEvent ruleChangeEvent) { - LOG.info("Generating a RuleSetChangeEvent"); broadcastMessage(ruleChangeEvent, getFilterForEvent(ruleChangeEvent)); } @@ -101,9 +98,11 @@ public class SonarLintClientsRegistry implements RuleActivationListener { String jsonString = getJSONString(personalizedEvent); c.writeAndFlush(jsonString); } catch (ForbiddenException forbiddenException) { + LOG.debug("Client is no longer authenticated: " + forbiddenException.getMessage()); unregisterClient(c); - } catch (IOException e) { + } catch (IllegalStateException | IOException e) { LOG.error("Unable to send message to a client: " + e.getMessage()); + unregisterClient(c); } }); } diff --git a/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/ServerPushClientTest.java b/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/ServerPushClientTest.java index e7eca1cf790..462d4a849fe 100644 --- a/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/ServerPushClientTest.java +++ b/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/ServerPushClientTest.java @@ -34,6 +34,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doThrow; @@ -123,20 +124,18 @@ public class ServerPushClientTest { when(executorService.schedule(any(HeartbeatTask.class), anyLong(), any(TimeUnit.class))).thenReturn(task); underTest.scheduleHeartbeat(); - underTest.write('a'); - - verify(asyncContext).complete(); + assertThatThrownBy(() -> underTest.write('a')) + .isInstanceOf(IllegalStateException.class); } @Test - public void flush_exceptionCausesConnectionToClose() throws IOException { + public void flush_exceptionIsPropagated() throws IOException { when(servletResponse.getOutputStream()).thenThrow(new IOException("mock exception")); when(executorService.schedule(any(HeartbeatTask.class), anyLong(), any(TimeUnit.class))).thenReturn(task); underTest.scheduleHeartbeat(); - underTest.flush(); - - verify(asyncContext).complete(); + assertThatThrownBy(underTest::flush) + .isInstanceOf(IllegalStateException.class); } @Test diff --git a/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistryTest.java b/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistryTest.java index 351ff62fb3e..208fa45715f 100644 --- a/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistryTest.java +++ b/server/sonar-webserver-pushapi/src/test/java/org/sonar/server/pushapi/sonarlint/SonarLintClientsRegistryTest.java @@ -39,6 +39,7 @@ import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -64,8 +65,8 @@ public class SonarLintClientsRegistryTest { } @Test - public void registerClientAndUnregister_changesNumberOfClients() { - SonarLintClient sonarLintClient = new SonarLintClient(defaultAsyncContext, exampleKeys, languageKeys, USER_UUID); + public void registerClientAndUnregister_changesNumberOfClients_andClosesClient() { + SonarLintClient sonarLintClient = mock(SonarLintClient.class); underTest.registerClient(sonarLintClient); @@ -74,6 +75,7 @@ public class SonarLintClientsRegistryTest { underTest.unregisterClient(sonarLintClient); assertThat(underTest.countConnectedClients()).isZero(); + verify(sonarLintClient).close(); } @Test @@ -168,6 +170,27 @@ public class SonarLintClientsRegistryTest { verify(sonarLintClient).close(); } + @Test + public void listen_givenUnregisteredClient_closeConnection() throws IOException { + RuleChange javaRuleChange = createRuleChange(); + RuleChange[] activatedRules = {}; + RuleChange[] deactivatedRules = {javaRuleChange}; + RuleSetChangeEvent ruleChangeEvent = new RuleSetChangeEvent(exampleKeys.toArray(String[]::new), activatedRules, deactivatedRules); + + SonarLintClient sonarLintClient = createSampleSLClient(); + underTest.registerClient(sonarLintClient); + doThrow(new IOException("Broken pipe")).when(sonarLintClient).writeAndFlush(anyString()); + + underTest.listen(ruleChangeEvent); + + underTest.registerClient(sonarLintClient); + doThrow(new IllegalStateException("Things went wrong")).when(sonarLintClient).writeAndFlush(anyString()); + + underTest.listen(ruleChangeEvent); + + verify(sonarLintClient, times(2)).close(); + } + private SonarLintClient createSampleSLClient() { SonarLintClient mock = mock(SonarLintClient.class); when(mock.getLanguages()).thenReturn(Set.of("java")); @@ -185,5 +208,4 @@ public class SonarLintClientsRegistryTest { javaRule.setKey("rule-key"); return javaRule; } - } |