SONAR-9721 Refactor TelemetryDaemon

3 files changed, 74 insertions, 55 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryClient.java b/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryClient.java
index 86c3766172f..1bc8a656b67 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryClient.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryClient.java
@@ -45,7 +45,7 @@ public class TelemetryClient {
     this.config = config;
   }
 
-  void send(String json) throws IOException {
+  void upload(String json) throws IOException {
     Request request = buildHttpRequest(json);
     okHttpClient.newCall(request).execute();
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java b/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
index 8567c61d51a..6105c25507e 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
@@ -20,10 +20,12 @@
 package org.sonar.server.telemetry;
 
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.io.IOException;
 import java.io.StringWriter;
 import java.util.Optional;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 import org.picocontainer.Startable;
 import org.sonar.api.config.Configuration;
@@ -68,53 +70,22 @@ public class TelemetryDaemon implements Startable {
   @Override
   public void start() {
     boolean isTelemetryActivated = config.getBoolean(PROP_ENABLE).orElseThrow(() -> new IllegalStateException(String.format("Setting '%s' must be provided.", PROP_URL)));
-    if (!internalProperties.read(I_PROP_OPT_OUT).isPresent()) {
-      if (!isTelemetryActivated) {
-        StringWriter json = new StringWriter();
-        try (JsonWriter writer = JsonWriter.of(json)) {
-          writer.beginObject();
-          writer.prop("id", server.getId());
-          writer.endObject();
-        }
-        telemetryClient.optOut(json.toString());
-        internalProperties.write(I_PROP_OPT_OUT, String.valueOf(system2.now()));
-        LOG.info("Sharing of SonarQube statistics is disabled.");
-      } else {
-        internalProperties.write(I_PROP_OPT_OUT, null);
-      }
+    boolean hasOptOut = internalProperties.read(I_PROP_OPT_OUT).isPresent();
+    if (!isTelemetryActivated && !hasOptOut) {
+      optOut();
+      internalProperties.write(I_PROP_OPT_OUT, String.valueOf(system2.now()));
+      LOG.info("Sharing of SonarQube statistics is disabled.");
+    }
+    if (isTelemetryActivated && hasOptOut) {
+      internalProperties.write(I_PROP_OPT_OUT, null);
     }
-
     if (!isTelemetryActivated) {
       return;
     }
     LOG.info("Sharing of SonarQube statistics is enabled.");
-    executorService = Executors.newSingleThreadScheduledExecutor(
-      new ThreadFactoryBuilder()
-        .setNameFormat(THREAD_NAME_PREFIX + "%d")
-        .setPriority(Thread.MIN_PRIORITY)
-        .build());
+    executorService = Executors.newSingleThreadScheduledExecutor(newThreadFactory());
     int frequencyInSeconds = frequency();
-    executorService.scheduleWithFixedDelay(() -> {
-      try {
-        Optional<Long> lastPing = internalProperties.read(I_PROP_LAST_PING).map(Long::valueOf);
-        long now = system2.now();
-        if (lastPing.isPresent() && now - lastPing.get() < SEVEN_DAYS) {
-          return;
-        }
-
-        StringWriter json = new StringWriter();
-        try (JsonWriter writer = JsonWriter.of(json)) {
-          writer.beginObject();
-          writer.prop("id", server.getId());
-          writer.endObject();
-        }
-        telemetryClient.send(json.toString());
-        internalProperties.write(I_PROP_LAST_PING, String.valueOf(startOfDay(now)));
-      } catch (Exception e) {
-        LOG.debug("Error while checking SonarQube statistics: {}", e.getMessage());
-      }
-    // do not check at start up to exclude test instance which are not up for a long time
-    }, frequencyInSeconds, frequencyInSeconds, TimeUnit.SECONDS);
+    executorService.scheduleWithFixedDelay(telemetryCommand(), frequencyInSeconds, frequencyInSeconds, TimeUnit.SECONDS);
   }
 
   @Override
@@ -127,6 +98,53 @@ public class TelemetryDaemon implements Startable {
     }
   }
 
+  private static ThreadFactory newThreadFactory() {
+    return new ThreadFactoryBuilder()
+      .setNameFormat(THREAD_NAME_PREFIX + "%d")
+      .setPriority(Thread.MIN_PRIORITY)
+      .build();
+  }
+
+  private Runnable telemetryCommand() {
+    return () -> {
+      try {
+        long now = system2.now();
+        if (shouldUploadStatistics(now)) {
+          uploadStatistics();
+          internalProperties.write(I_PROP_LAST_PING, String.valueOf(startOfDay(now)));
+        }
+      } catch (Exception e) {
+        LOG.debug("Error while checking SonarQube statistics: {}", e.getMessage());
+      }
+      // do not check at start up to exclude test instance which are not up for a long time
+    };
+  }
+
+  private void optOut() {
+    StringWriter json = new StringWriter();
+    try (JsonWriter writer = JsonWriter.of(json)) {
+      writer.beginObject();
+      writer.prop("id", server.getId());
+      writer.endObject();
+    }
+    telemetryClient.optOut(json.toString());
+  }
+
+  private void uploadStatistics() throws IOException {
+    StringWriter json = new StringWriter();
+    try (JsonWriter writer = JsonWriter.of(json)) {
+      writer.beginObject();
+      writer.prop("id", server.getId());
+      writer.endObject();
+    }
+    telemetryClient.upload(json.toString());
+  }
+
+  private boolean shouldUploadStatistics(long now) {
+    Optional<Long> lastPing = internalProperties.read(I_PROP_LAST_PING).map(Long::valueOf);
+    return !lastPing.isPresent() || now - lastPing.get() >= SEVEN_DAYS;
+  }
+
   private static long startOfDay(long now) {
     return parseDate(formatDate(now)).getTime();
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java b/server/sonar-server/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
index cb1aeb72ff1..1f07c1484d8 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
@@ -20,6 +20,7 @@
 
 package org.sonar.server.telemetry;
 
+import java.io.IOException;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.config.PropertyDefinitions;
@@ -63,39 +64,39 @@ public class TelemetryDaemonTest {
   }
 
   @Test
-  public void send_data_via_client_at_startup_after_initial_delay() {
+  public void send_data_via_client_at_startup_after_initial_delay() throws IOException {
     settings.setProperty(PROP_FREQUENCY, "1");
     underTest.start();
 
-    verify(client, timeout(1_000).atLeastOnce()).send(anyString());
+    verify(client, timeout(1_000).atLeastOnce()).upload(anyString());
   }
 
   @Test
-  public void check_if_should_send_data_periodically() {
+  public void check_if_should_send_data_periodically() throws IOException {
     long now = system2.now();
     long sixDaysAgo = now - (ONE_DAY * 6L);
     long sevenDaysAgo = now - (ONE_DAY * 7L);
     internalProperties.write(I_PROP_LAST_PING, String.valueOf(sixDaysAgo));
     settings.setProperty(PROP_FREQUENCY, "1");
     underTest.start();
-    verify(client, timeout(1_000).never()).send(anyString());
+    verify(client, timeout(1_000).never()).upload(anyString());
     internalProperties.write(I_PROP_LAST_PING, String.valueOf(sevenDaysAgo));
 
-    verify(client, timeout(1_000).atLeastOnce()).send(anyString());
+    verify(client, timeout(1_000).atLeastOnce()).upload(anyString());
   }
 
   @Test
-  public void send_server_id() {
+  public void send_server_id() throws IOException {
     settings.setProperty(PROP_FREQUENCY, "1");
     String id = randomAlphanumeric(40);
     server.setId(id);
     underTest.start();
 
-    verify(client, timeout(2_000).atLeastOnce()).send(contains(id));
+    verify(client, timeout(2_000).atLeastOnce()).upload(contains(id));
   }
 
   @Test
-  public void do_not_send_data_if_last_ping_earlier_than_one_week_ago() {
+  public void do_not_send_data_if_last_ping_earlier_than_one_week_ago() throws IOException {
     settings.setProperty(PROP_FREQUENCY, "1");
     long now = system2.now();
     long sixDaysAgo = now - (ONE_DAY * 6L);
@@ -103,11 +104,11 @@ public class TelemetryDaemonTest {
     internalProperties.write(I_PROP_LAST_PING, String.valueOf(sixDaysAgo));
     underTest.start();
 
-    verify(client, timeout(2_000).never()).send(anyString());
+    verify(client, timeout(2_000).never()).upload(anyString());
   }
 
   @Test
-  public void send_data_if_last_ping_is_one_week_ago() {
+  public void send_data_if_last_ping_is_one_week_ago() throws IOException {
     settings.setProperty(PROP_FREQUENCY, "1");
     long today = parseDate("2017-08-01").getTime();
     system2.setNow(today + 15 * ONE_HOUR);
@@ -117,19 +118,19 @@ public class TelemetryDaemonTest {
 
     underTest.start();
 
-    verify(client, timeout(1_000).atLeastOnce()).send(anyString());
+    verify(client, timeout(1_000).atLeastOnce()).upload(anyString());
     assertThat(internalProperties.read(I_PROP_LAST_PING).get()).isEqualTo(String.valueOf(today));
   }
 
   @Test
-  public void opt_out_sent_once() {
+  public void opt_out_sent_once() throws IOException {
     settings.setProperty(PROP_FREQUENCY, "1");
     settings.setProperty(PROP_ENABLE, "false");
     underTest.start();
     underTest.start();
 
 
-    verify(client, timeout(1_000).never()).send(anyString());
+    verify(client, timeout(1_000).never()).upload(anyString());
     verify(client, timeout(1_000).times(1)).optOut(anyString());
   }
 }