simplify refactor and improve coverage

ls1intum · Sep 21, 2024 · e5c690a · e5c690a
1 parent 3829a34
commit e5c690a
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 31 deletions.
diff --git a/src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetrySendingService.java b/src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetrySendingService.java
@@ -62,20 +62,14 @@ public TelemetrySendingService(Environment env, RestTemplate restTemplate, Eurek
     private String operatorAdminName;
 
     @Value("${info.contact}")
-    private String contact;
-
-    @Value("${artemis.telemetry.sendAdminDetails}")
-    private boolean sendAdminDetails;
+    private String operatorContact;
 
     @Value("${artemis.telemetry.destination}")
     private String destination;
 
     @Value("${spring.datasource.url}")
     private String datasourceUrl;
 
-    @Value("${eureka.client.enabled}")
-    private boolean eurekaEnabled;
-
     @Value("${artemis.continuous-integration.concurrent-build-size}")
     private long buildAgentCount;
 
@@ -85,23 +79,24 @@ public TelemetrySendingService(Environment env, RestTemplate restTemplate, Eurek
      * @throws Exception if the writing the telemetry data to a json format fails, or the connection to the telemetry server fails
      */
     @Async
-    public void sendTelemetryByPostRequest() throws Exception {
-        List<String> activeProfiles = Arrays.asList(env.getActiveProfiles());
-        TelemetryData telemetryData;
-        var dataSource = datasourceUrl.startsWith("jdbc:mysql") ? "mysql" : "postgresql";
+    public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws Exception {
+
         long numberOfInstances = 1;
         if (eurekaEnabled) {
             numberOfInstances = eurekaClientService.getNumberOfReplicas();
         }
 
+        TelemetryData telemetryData;
+        var dataSource = datasourceUrl.startsWith("jdbc:mysql") ? "mysql" : "postgresql";
+        List<String> activeProfiles = Arrays.asList(env.getActiveProfiles());
+        String contact = null;
+        String adminName = null;
         if (sendAdminDetails) {
-            telemetryData = new TelemetryData(version, serverUrl, operator, activeProfiles, profileService.isProductionActive(), dataSource, numberOfInstances, buildAgentCount,
-                    contact, operatorAdminName);
-        }
-        else {
-            telemetryData = new TelemetryData(version, serverUrl, operator, activeProfiles, profileService.isProductionActive(), dataSource, numberOfInstances, buildAgentCount,
-                    null, null);
+            contact = operatorContact;
+            adminName = operatorAdminName;
         }
+        telemetryData = new TelemetryData(version, serverUrl, operator, activeProfiles, profileService.isProductionActive(), dataSource, numberOfInstances, buildAgentCount,
+                contact, adminName);
 
         HttpHeaders headers = new HttpHeaders();
         headers.setContentType(MediaType.APPLICATION_JSON);

diff --git a/src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetryService.java b/src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetryService.java
@@ -24,11 +24,17 @@ public class TelemetryService {
 
     private final TelemetrySendingService telemetrySendingService;
 
+    @Value("${artemis.telemetry.destination}")
+    private String destination;
+
     @Value("${artemis.telemetry.enabled}")
     public boolean useTelemetry;
 
-    @Value("${artemis.telemetry.destination}")
-    private String destination;
+    @Value("${artemis.telemetry.sendAdminDetails}")
+    public boolean sendAdminDetails;
+
+    @Value("${eureka.client.enabled}")
+    public boolean eurekaEnabled;
 
     public TelemetryService(ProfileService profileService, TelemetrySendingService telemetrySendingService) {
         this.profileService = profileService;
@@ -48,7 +54,7 @@ public void sendTelemetry() {
 
         log.info("Sending telemetry information");
         try {
-            telemetrySendingService.sendTelemetryByPostRequest();
+            telemetrySendingService.sendTelemetryByPostRequest(eurekaEnabled, sendAdminDetails);
         }
         catch (JsonProcessingException e) {
             log.warn("JsonProcessingException in sendTelemetry.", e);

diff --git a/src/test/java/de/tum/cit/aet/artemis/telemetry/TelemetryServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/telemetry/TelemetryServiceTest.java
@@ -1,6 +1,7 @@
 package de.tum.cit.aet.artemis.telemetry;
 
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.mockito.Mockito.spy;
 import static org.springframework.test.web.client.match.MockRestRequestMatchers.header;
 import static org.springframework.test.web.client.match.MockRestRequestMatchers.method;
@@ -56,7 +57,7 @@ class TelemetryServiceTest extends AbstractSpringIntegrationIndependentTest {
 
     private String eurekaRequestUrl;
 
-    private byte[] appliciationsBody;
+    private byte[] applicationsBody;
 
     @BeforeEach
     void init() throws JsonProcessingException {
@@ -66,26 +67,39 @@ void init() throws JsonProcessingException {
 
         }
         catch (Exception ignored) {
+            // Exception can be ignored because defaultZoneUrl is guaranteed to be valid in the test environment
         }
-
         telemetryServiceSpy = spy(telemetryService);
-
-        appliciationsBody = mapper.writeValueAsBytes(Map.of("applications", List.of(Map.of("name", "ARTEMIS", "instances", List.of(Map.of()) // Mocking an instance object
-        ))));
-        MockRestServiceServer.MockRestServiceServerBuilder builder = MockRestServiceServer.bindTo(restTemplate);
-        builder.ignoreExpectOrder(true);
-        mockServer = builder.build();
+        applicationsBody = mapper.writeValueAsBytes(Map.of("applications", List.of(Map.of("name", "ARTEMIS", "instances", List.of(Map.of())))));
+        mockServer = MockRestServiceServer.bindTo(restTemplate).ignoreExpectOrder(true).build();
 
         telemetryServiceSpy.useTelemetry = true;
+        telemetryServiceSpy.eurekaEnabled = true;
     }
 
     @Test
     void testSendTelemetry_TelemetryEnabled() throws Exception {
         mockServer.expect(ExpectedCount.once(), requestTo(new URI(eurekaRequestUrl))).andExpect(method(HttpMethod.GET))
                 .andExpect(header(HttpHeaders.AUTHORIZATION, "Basic YWRtaW46YWRtaW4="))
-                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(appliciationsBody));
+                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(applicationsBody));
+
+        mockServer.expect(ExpectedCount.once(), requestTo(new URI(destination + "/api/telemetry"))).andExpect(method(HttpMethod.POST))
+                .andExpect(request -> assertThat(request.getBody().toString()).contains("adminName"))
+                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(mapper.writeValueAsString("Success!")));
+        telemetryServiceSpy.sendTelemetry();
+
+        await().atMost(2, SECONDS).untilAsserted(() -> mockServer.verify());
+    }
+
+    @Test
+    void testSendTelemetry_TelemetryEnabledWithoutPersonalData() throws Exception {
+        telemetryServiceSpy.sendAdminDetails = false;
+        mockServer.expect(ExpectedCount.once(), requestTo(new URI(eurekaRequestUrl))).andExpect(method(HttpMethod.GET))
+                .andExpect(header(HttpHeaders.AUTHORIZATION, "Basic YWRtaW46YWRtaW4="))
+                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(applicationsBody));
 
         mockServer.expect(ExpectedCount.once(), requestTo(new URI(destination + "/api/telemetry"))).andExpect(method(HttpMethod.POST))
+                .andExpect(request -> assertThat(request.getBody().toString()).doesNotContain("adminName"))
                 .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(mapper.writeValueAsString("Success!")));
         telemetryServiceSpy.sendTelemetry();
 
@@ -104,7 +118,7 @@ void testSendTelemetry_TelemetryDisabled() throws Exception {
     @Test
     void testSendTelemetry_ExceptionHandling() throws Exception {
         mockServer.expect(ExpectedCount.once(), requestTo(new URI(eurekaRequestUrl))).andExpect(method(HttpMethod.GET))
-                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(appliciationsBody));
+                .andRespond(withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON).body(applicationsBody));
         mockServer.expect(ExpectedCount.once(), requestTo(new URI(destination + "/api/telemetry"))).andExpect(method(HttpMethod.POST))
                 .andRespond(withServerError().body(mapper.writeValueAsString("Failure!")));
 

diff --git a/src/test/resources/config/application.yml b/src/test/resources/config/application.yml
@@ -293,6 +293,6 @@ aeolus:
 # Eureka configuration
 eureka:
     client:
-        enabled: true
+        enabled: false
         service-url:
             defaultZone: http://admin:admin@localhost:8761/eureka/