Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Add more data for Telemetry #9345

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package de.tum.cit.aet.artemis.core.service.telemetry;

import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Collections;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;
import org.springframework.web.client.RestClientException;
import org.springframework.web.client.RestTemplate;

import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;

@Service
public class EurekaClientService {
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the @Profile annotation here?


Rule 'classes that are annotated with @Service or are annotated with @Component or are annotated with @Configuration and do not belong to any of [de.tum.cit.aet.artemis.core.config.ApplicationConfiguration, de.tum.cit.aet.artemis.core.config.ConditionalMetricsExclusionConfiguration] should be annotated with @Profile, because we want to be able to exclude these classes from application startup by specifying profiles' was violated


private static final Logger log = LoggerFactory.getLogger(EurekaClientService.class);

@Value("${eureka.client.service-url.defaultZone}")
private String eurekaServiceUrl;

private final RestTemplate restTemplate;

public EurekaClientService(RestTemplate restTemplate) {
this.restTemplate = restTemplate;
}

/**
* Retrieves the number of Artemis application instances registered with the Eureka server.
* <p>
* This method makes an HTTP GET request to the Eureka server's `/api/eureka/applications` endpoint to
* retrieve a list of all registered applications. It then filters out the Artemis application
* and returns the number of instances associated with it. If any error occurs during the request
* (e.g., network issues, parsing issues, invalid URI), the method returns 1 as the default value.
*
* @return the number of Artemis application instances, or 1 if an error occurs.
*/
public long getNumberOfReplicas() {
Comment on lines +39 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How quickly do the Eureka instances register? Could the telemetry data be sent before all nodes have been started?

try {
var eurekaURI = new URI(eurekaServiceUrl);
HttpHeaders headers = createHeaders(eurekaURI.getUserInfo());
HttpEntity<String> request = new HttpEntity<>(headers);
var requestUrl = eurekaURI.getScheme() + "://" + eurekaURI.getAuthority() + "/api/eureka/applications";

ResponseEntity<String> response = restTemplate.exchange(requestUrl, HttpMethod.GET, request, String.class);

ObjectMapper objectMapper = new ObjectMapper();
JsonNode rootNode = objectMapper.readTree(response.getBody());
JsonNode applicationsNode = rootNode.get("applications");
for (JsonNode application : applicationsNode) {
if (application.get("name").asText().equals("ARTEMIS")) {
JsonNode instancesNode = application.get("instances");
return instancesNode.size();
}
}
}
catch (RestClientException | JacksonException | URISyntaxException e) {
log.warn("Error while trying to retrieve number of replicas.");
}

return 1;
}

/**
* Creates HTTP headers with Basic Authentication and JSON content type.
*
* @param auth the user credentials in the format "username:password" to be encoded and included in the Authorization header.
* @return HttpHeaders with Basic Authentication and JSON content types.
*/
private HttpHeaders createHeaders(String auth) {
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));

byte[] encodedAuth = Base64.getEncoder().encode(auth.getBytes(StandardCharsets.US_ASCII));
String authHeader = "Basic " + new String(encodedAuth);

headers.set("Authorization", authHeader);
return headers;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package de.tum.cit.aet.artemis.core.service;
package de.tum.cit.aet.artemis.core.service.telemetry;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_SCHEDULING;

Expand All @@ -21,23 +21,32 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;

import de.tum.cit.aet.artemis.core.service.ProfileService;

@Service
@Profile(PROFILE_SCHEDULING)
public class TelemetrySendingService {

private static final Logger log = LoggerFactory.getLogger(TelemetrySendingService.class);

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record TelemetryData(String version, String serverUrl, String operator, String contact, List<String> profiles, String adminName) {
public record TelemetryData(String version, String serverUrl, String operator, List<String> profiles, boolean isProductionInstance, String dataSource, long numberOfNodes,
long buildAgentCount, String contact, String adminName) {
}
Comment on lines +33 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure compliance with data privacy regulations when transmitting PII

The TelemetryData record now includes potentially sensitive information such as contact and adminName. Please verify that collecting and transmitting this data complies with applicable data privacy laws and organizational policies. Consider anonymizing or securely handling PII before transmission.


private final Environment env;

private final RestTemplate restTemplate;

public TelemetrySendingService(Environment env, RestTemplate restTemplate) {
private final EurekaClientService eurekaClientService;

private final ProfileService profileService;

public TelemetrySendingService(Environment env, RestTemplate restTemplate, EurekaClientService eurekaClientService, ProfileService profileService) {
this.env = env;
this.restTemplate = restTemplate;
this.eurekaClientService = eurekaClientService;
this.profileService = profileService;
}

@Value("${artemis.version}")
Expand All @@ -53,29 +62,41 @@ public TelemetrySendingService(Environment env, RestTemplate restTemplate) {
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;

Comment on lines +70 to +71
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

Implement validation and error handling for datasourceUrl

The current implementation lacks necessary checks and error handling for datasourceUrl, which could lead to runtime errors. Consider the following improvements:

  • Add null and empty checks for datasourceUrl before usage.
  • Implement error handling for cases where the URL doesn't match expected patterns.
  • Use a more robust method to determine the database type, possibly leveraging Spring's DataSourceProperties or Environment for consistency.

Example:

@Autowired
private Environment env;

// In the method where datasourceUrl is used
if (datasourceUrl == null || datasourceUrl.isEmpty()) {
    throw new IllegalStateException("Datasource URL is not configured properly");
}

String dataSource;
if (datasourceUrl.startsWith("jdbc:mysql")) {
    dataSource = "mysql";
} else if (datasourceUrl.startsWith("jdbc:postgresql")) {
    dataSource = "postgresql";
} else {
    throw new IllegalStateException("Unsupported database type in datasource URL");
}

These changes will improve the robustness of the TelemetrySendingService and prevent potential runtime errors.

Analysis chain

Validate the datasource URL before usage

While retrieving the datasourceUrl, ensure that it's validated to prevent potential NullPointerExceptions or malformed URLs, which could lead to runtime errors.

Please run the following script to check for null or empty datasourceUrl assignments:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for assignments of 'datasourceUrl' that might be null or empty.

# Test: Search for places where 'datasourceUrl' could be null. Expect: No assignments of null values to 'datasourceUrl'.
rg --type java 'datasourceUrl\s*=\s*null' -A 2

Length of output: 48


Script:

#!/bin/bash
# Description: Analyze usage and potential validation of datasourceUrl

# Search for usage of datasourceUrl
echo "Usage of datasourceUrl:"
rg --type java "datasourceUrl" -A 2 -B 2

# Look for potential validation methods or checks
echo -e "\nPotential validation or checks:"
rg --type java "(validate|check|verify).*datasourceUrl" -A 2 -B 2

# Check for error handling mechanisms
echo -e "\nError handling related to datasourceUrl:"
rg --type java "(try|catch|throw).*datasourceUrl" -A 2 -B 2

Length of output: 1678


Script:

#!/bin/bash
# Description: Double-check for any other usages or validations of datasourceUrl

# Search for any other usages of datasourceUrl in the entire codebase
echo "All usages of datasourceUrl:"
rg --type java "datasourceUrl" -A 2 -B 2

# Look for any potential initialization or configuration of datasourceUrl
echo -e "\nPotential initialization or configuration:"
rg --type java "spring\.datasource\.url" -A 2 -B 2

Length of output: 2088

@Value("${artemis.continuous-integration.concurrent-build-size}")
private long buildAgentCount;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value may not be defined when not using the localci or buildagent profiles and then result in a failure to start Artemis. The suggestion sets a default value in such cases.

Suggested change
@Value("${artemis.continuous-integration.concurrent-build-size}")
@Value("${artemis.continuous-integration.concurrent-build-size:0}")


/**
* Assembles the telemetry data, and sends it to the external telemetry server.
*
* @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());
public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws Exception {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify precise exceptions instead of general Exception

Declaring throws Exception is too broad and can obscure specific issues. It's better to specify the exact exceptions that might be thrown to improve code readability and error handling.

Apply this diff to specify precise exceptions:

-public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws Exception {
+public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws JsonProcessingException, RestClientException {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws Exception {
public void sendTelemetryByPostRequest(boolean eurekaEnabled, boolean sendAdminDetails) throws JsonProcessingException, RestClientException {

Comment on lines 76 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the javadocs as well

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, contact, activeProfiles, operatorAdminName);
}
else {
telemetryData = new TelemetryData(version, serverUrl, operator, null, activeProfiles, 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package de.tum.cit.aet.artemis.core.service;
package de.tum.cit.aet.artemis.core.service.telemetry;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_SCHEDULING;

Expand All @@ -12,6 +12,8 @@

import com.fasterxml.jackson.core.JsonProcessingException;

import de.tum.cit.aet.artemis.core.service.ProfileService;

@Service
@Profile(PROFILE_SCHEDULING)
public class TelemetryService {
Expand All @@ -22,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;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restrict field access to adhere to the principle of least access.

The fields sendAdminDetails and eurekaEnabled are declared as public. To follow best practices and the principle of least access, they should be declared as private unless they need to be accessed from outside this class.

Apply this diff to adjust the access modifiers:

- public boolean sendAdminDetails;
+ private boolean sendAdminDetails;

- public boolean eurekaEnabled;
+ private boolean eurekaEnabled;

Also applies to: 37-37

@Value("${eureka.client.enabled}")
public boolean eurekaEnabled;

public TelemetryService(ProfileService profileService, TelemetrySendingService telemetrySendingService) {
this.profileService = profileService;
Expand All @@ -46,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
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;
import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError;
import static org.springframework.test.web.client.response.MockRestResponseCreators.withStatus;
import static org.testcontainers.shaded.org.awaitility.Awaitility.await;

import java.net.URI;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.test.web.client.ExpectedCount;
import org.springframework.test.web.client.MockRestServiceServer;
import org.springframework.web.client.RestTemplate;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import de.tum.cit.aet.artemis.AbstractSpringIntegrationIndependentTest;
import de.tum.cit.aet.artemis.core.service.TelemetryService;
import de.tum.cit.aet.artemis.core.service.telemetry.TelemetryService;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Organize Imports According to Project Standards

Ensure that the import statements are organized as per the project's coding guidelines. Import statements should be grouped logically, and wildcards should be avoided unless necessary.


@ExtendWith(MockitoExtension.class)
class TelemetryServiceTest extends AbstractSpringIntegrationIndependentTest {
Expand All @@ -46,19 +52,58 @@ class TelemetryServiceTest extends AbstractSpringIntegrationIndependentTest {
@Value("${artemis.telemetry.destination}")
private String destination;

@Value("${eureka.client.service-url.defaultZone}")
private String defaultZoneUrl;
Comment on lines +55 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Descriptive Variable Names for Clarity

Consider renaming defaultZoneUrl to eurekaDefaultZoneUrl to make it clear that this URL is specific to Eureka's default zone. This enhances readability and helps other developers understand the context of the variable.

Apply this diff to rename the variable:

- @Value("${eureka.client.service-url.defaultZone}")
- private String defaultZoneUrl;
+ @Value("${eureka.client.service-url.defaultZone}")
+ private String eurekaDefaultZoneUrl;

Also, update all references to defaultZoneUrl accordingly.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Value("${eureka.client.service-url.defaultZone}")
private String defaultZoneUrl;
@Value("${eureka.client.service-url.defaultZone}")
private String eurekaDefaultZoneUrl;


private String eurekaRequestUrl;

private byte[] applicationsBody;

@BeforeEach
void init() {
void init() throws JsonProcessingException {
try {
var eurekaURI = new URI(defaultZoneUrl);
eurekaRequestUrl = eurekaURI.getScheme() + "://" + eurekaURI.getAuthority() + "/api/eureka/applications";

}
catch (Exception ignored) {
// Exception can be ignored because defaultZoneUrl is guaranteed to be valid in the test environment
}
telemetryServiceSpy = spy(telemetryService);
mockServer = MockRestServiceServer.createServer(restTemplate);
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(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(1, SECONDS).untilAsserted(() -> mockServer.verify());

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!")));
Comment on lines +102 to +103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand Verification of Personal Data Exclusion

Currently, the test only checks that the request body does not contain "adminName". Consider verifying that other personal data fields are also excluded when sendAdminDetails is set to false.

Update the test assertion to check for additional personal data fields:

.andExpect(request -> {
    String requestBody = request.getBody().toString();
    assertThat(requestBody).doesNotContain("adminName");
+   assertThat(requestBody).doesNotContain("adminEmail");
+   assertThat(requestBody).doesNotContain("adminPassword");
})

Committable suggestion was skipped due to low confidence.

telemetryServiceSpy.sendTelemetry();

Comment on lines +94 to +105
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid Code Duplication in Test Setup

The setup code in testSendTelemetry_TelemetryEnabledWithoutPersonalData() is similar to other test methods. To promote maintainability and reduce duplication, consider refactoring the common setup code into a separate method or using parameterized tests.

Extract common mock server expectations into a helper method:

private void setupMockServerExpectations(boolean includeAdminName) throws URISyntaxException {
    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 -> {
            if (includeAdminName) {
                assertThat(request.getBody().toString()).contains("adminName");
            } else {
                assertThat(request.getBody().toString()).doesNotContain("adminName");
            }
        })
        .andRespond(withStatus(HttpStatus.OK)
            .contentType(MediaType.APPLICATION_JSON)
            .body(mapper.writeValueAsString("Success!")));
}

Then call this method in your tests:

@Test
void testSendTelemetry_TelemetryEnabled() throws Exception {
    setupMockServerExpectations(true);
    telemetryServiceSpy.sendTelemetry();
    await().atMost(2, SECONDS).untilAsserted(() -> mockServer.verify());
}

@Test
void testSendTelemetry_TelemetryEnabledWithoutPersonalData() throws Exception {
    telemetryServiceSpy.sendAdminDetails = false;
    setupMockServerExpectations(false);
    telemetryServiceSpy.sendTelemetry();
    await().atMost(2, SECONDS).untilAsserted(() -> mockServer.verify());
}

await().atMost(2, SECONDS).untilAsserted(() -> mockServer.verify());
}

@Test
Expand All @@ -72,9 +117,12 @@ 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(applicationsBody));
mockServer.expect(ExpectedCount.once(), requestTo(new URI(destination + "/api/telemetry"))).andExpect(method(HttpMethod.POST))
.andRespond(withServerError().body(mapper.writeValueAsString("Failure!")));

telemetryServiceSpy.sendTelemetry();
await().atMost(1, SECONDS).untilAsserted(() -> mockServer.verify());
await().atMost(2, SECONDS).untilAsserted(() -> mockServer.verify());
Comment on lines +120 to +126
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle Server Errors Appropriately in Exception Handling Test

In testSendTelemetry_ExceptionHandling(), ensure that the test not only verifies that an exception doesn't crash the application but also that it behaves as expected in case of server errors. Consider asserting the state after the error is handled.

Add assertions to verify the application's state post-error handling:

// After sending telemetry and simulating server error
telemetryServiceSpy.sendTelemetry();

// Verify that the application handled the error gracefully
// For example, check logs or internal flags if applicable

}
}
1 change: 1 addition & 0 deletions src/test/resources/config/application-artemis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ artemis:
password: fake-password
token: fake-token
url: https://continuous-integration.fake.fake
concurrent-build-size: 1
secret-push-token: fake-token-hash
vcs-credentials: fake-key
artemis-authentication-token-key: fake-key
Expand Down
8 changes: 8 additions & 0 deletions src/test/resources/config/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ spring:
application:
name: Artemis
datasource:
url: jdbc:mysql://artemis-mysql:3306/
type: com.zaxxer.hikari.HikariDataSource
name:
username:
Expand Down Expand Up @@ -288,3 +289,10 @@ jhipster:

aeolus:
url: http://mock-aeolus-url:8090

# Eureka configuration
eureka:
client:
enabled: false
service-url:
defaultZone: http://admin:admin@localhost:8761/eureka/
Loading