From 9ce595030c070e5f0f7fcf1b6ab62aece00b4c06 Mon Sep 17 00:00:00 2001 From: Levin Kerschberger Date: Tue, 12 Nov 2024 23:18:30 +0100 Subject: [PATCH] refactor: Kiss (my ass) - No Events --- .../gepard/agentmanager/Application.java | 2 + .../application/config/EventConfig.java | 20 ----- .../controller/ConfigurationController.java | 10 ++- .../events/ConfigurationRequestEvent.java | 19 ----- .../service/ConfigurationService.java | 24 ------ .../connection/service/ConnectionService.java | 25 +++--- .../ConfigurationControllerTest.java | 32 ++++++-- .../service/ConfigurationServiceTest.java | 10 +-- .../service/ConnectionServiceTest.java | 79 +++++++++---------- .../exception/GlobalExceptionHandlerTest.java | 15 ++++ 10 files changed, 100 insertions(+), 136 deletions(-) delete mode 100644 backend/src/main/java/rocks/inspectit/gepard/agentmanager/application/config/EventConfig.java delete mode 100644 backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/events/ConfigurationRequestEvent.java diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/Application.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/Application.java index 9c03276a..20d27589 100644 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/Application.java +++ b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/Application.java @@ -3,8 +3,10 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.web.servlet.ServletComponentScan; @SpringBootApplication +@ServletComponentScan public class Application { public static void main(String[] args) { diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/application/config/EventConfig.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/application/config/EventConfig.java deleted file mode 100644 index 7ba66ac3..00000000 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/application/config/EventConfig.java +++ /dev/null @@ -1,20 +0,0 @@ -/* (C) 2024 */ -package rocks.inspectit.gepard.agentmanager.application.config; - -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.event.ApplicationEventMulticaster; -import org.springframework.context.event.SimpleApplicationEventMulticaster; -import org.springframework.core.task.SimpleAsyncTaskExecutor; - -@Configuration -public class EventConfig { - - @Bean - public ApplicationEventMulticaster simpleApplicationEventMulticaster() { - SimpleApplicationEventMulticaster eventMulticaster = new SimpleApplicationEventMulticaster(); - - eventMulticaster.setTaskExecutor(new SimpleAsyncTaskExecutor()); - return eventMulticaster; - } -} diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationController.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationController.java index 75dedcc6..aa581088 100644 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationController.java +++ b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationController.java @@ -9,6 +9,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; import rocks.inspectit.gepard.agentmanager.configuration.service.ConfigurationService; +import rocks.inspectit.gepard.agentmanager.connection.service.ConnectionService; import rocks.inspectit.gepard.config.model.InspectitConfiguration; @RestController @@ -17,6 +18,7 @@ public class ConfigurationController { private final ConfigurationService configurationService; + private final ConnectionService connectionService; @GetMapping("/{agentId}") @Operation( @@ -25,15 +27,17 @@ public class ConfigurationController { public ResponseEntity getAgentConfiguration( @PathVariable String agentId, @RequestHeader Map headers) { - InspectitConfiguration configuration = - configurationService.handleConfigurationRequest(agentId, headers); + boolean isFirstRequest = connectionService.handleConfigurationRequest(agentId, headers); + InspectitConfiguration configuration = configurationService.getConfiguration(); // No config available if (Objects.isNull(configuration)) { return ResponseEntity.noContent().build(); } - return ResponseEntity.ok().body(configuration); + return ResponseEntity.ok() + .header("x-gepard-service-registered", String.valueOf(isFirstRequest)) + .body(configuration); } @PutMapping diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/events/ConfigurationRequestEvent.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/events/ConfigurationRequestEvent.java deleted file mode 100644 index d2753192..00000000 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/events/ConfigurationRequestEvent.java +++ /dev/null @@ -1,19 +0,0 @@ -/* (C) 2024 */ -package rocks.inspectit.gepard.agentmanager.configuration.events; - -import java.util.Map; -import lombok.Getter; -import org.springframework.context.ApplicationEvent; - -@Getter -public class ConfigurationRequestEvent extends ApplicationEvent { - - private final String agentId; - private final Map headers; - - public ConfigurationRequestEvent(Object source, String agentId, Map headers) { - super(source); - this.agentId = agentId; - this.headers = headers; - } -} diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationService.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationService.java index 84ec8f01..50acc6ff 100644 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationService.java +++ b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationService.java @@ -3,11 +3,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import java.util.Map; import lombok.AllArgsConstructor; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Service; -import rocks.inspectit.gepard.agentmanager.configuration.events.ConfigurationRequestEvent; import rocks.inspectit.gepard.agentmanager.exception.JsonParseException; import rocks.inspectit.gepard.config.model.InspectitConfiguration; @@ -19,8 +16,6 @@ public class ConfigurationService { private final ObjectMapper objectMapper; - private final ApplicationEventPublisher applicationEventPublisher; - /** * Retrieves the current configuration from the local Git repository. * @@ -49,23 +44,4 @@ public void updateConfiguration(InspectitConfiguration configuration) { throw new JsonParseException("Failed to serialize InspectitConfiguration to JSON", e); } } - - /** - * Handles a configuration request from an agent. If the agent is not connected, it will be - * connected. The last fetch time of the agent will be updated. The configuration will be - * returned. - * - * @param agentId the id of the agent requesting the configuration - * @param headers the request headers, which should contain the agent information - * @return the inspectit configuration for this agent - */ - public InspectitConfiguration handleConfigurationRequest( - String agentId, Map headers) { - - // Event Emitter instead - ConfigurationRequestEvent event = new ConfigurationRequestEvent(this, agentId, headers); - applicationEventPublisher.publishEvent(event); - - return getConfiguration(); - } } diff --git a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionService.java b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionService.java index 1048b533..5e6edc9d 100644 --- a/backend/src/main/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionService.java +++ b/backend/src/main/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionService.java @@ -9,9 +9,7 @@ import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.springframework.context.event.EventListener; import org.springframework.stereotype.Service; -import rocks.inspectit.gepard.agentmanager.configuration.events.ConfigurationRequestEvent; import rocks.inspectit.gepard.agentmanager.connection.model.Connection; import rocks.inspectit.gepard.agentmanager.connection.model.ConnectionStatus; import rocks.inspectit.gepard.agentmanager.connection.model.dto.ConnectionDto; @@ -27,22 +25,25 @@ public class ConnectionService { private final ConcurrentHashMap connectionCache; - private final RegexQueryService regexQueryService; /** - * Handles a ConfigurationRequestEvent. If the agent is not connected, it will be connected. If it - * is already connected, the last fetch time of the agent will be updated. + * Handles a ConfigurationRequest. If the agent is not connected, it will be connected. If it is + * already connected, the last fetch time of the agent will be updated. * - * @param event The configuration request event. + * @param agentId The id of the agent to be connected. + * @param headers The request headers, which should contain the agent information. */ - @EventListener - public void handleConfigurationRequestEvent(ConfigurationRequestEvent event) { - if (!isAgentConnected(event.getAgentId())) { - connectAgent(event.getAgentId(), event.getHeaders()); + public boolean handleConfigurationRequest(String agentId, Map headers) { + boolean isNewRegistration; + if (!isAgentConnected(agentId)) { + connectAgent(agentId, headers); + isNewRegistration = true; } else { - updateConnectionLastFetch(event.getAgentId()); + updateConnectionLastFetch(agentId); + isNewRegistration = false; } + return isNewRegistration; } /** @@ -106,7 +107,7 @@ public ConnectionDto getConnection(String id) { * @param agentId The id of the agent to be searched for. * @return true if the agent was found in the cache, false otherwise. */ - private boolean isAgentConnected(String agentId) { + public boolean isAgentConnected(String agentId) { return connectionCache.get(agentId) != null; } diff --git a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationControllerTest.java b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationControllerTest.java index 1ee26c1e..aea2f1d6 100644 --- a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationControllerTest.java +++ b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/controller/ConfigurationControllerTest.java @@ -4,12 +4,10 @@ import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; import static rocks.inspectit.gepard.agentmanager.testutils.ConfigurationRequestTestUtils.getGepardHeaders; import com.fasterxml.jackson.databind.ObjectMapper; -import java.util.Map; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; @@ -48,10 +46,8 @@ void getConfiguration_whenNoConfigAvailable_shouldReturnNoContent() throws Excep void getConfiguration_whenConfigAvailable_shouldReturnOk() throws Exception { HttpHeaders headers = getGepardHeaders(); - Map headersMap = headers.toSingleValueMap(); - when(configurationService.handleConfigurationRequest(agentId, headersMap)) - .thenReturn(new InspectitConfiguration()); + when(configurationService.getConfiguration()).thenReturn(new InspectitConfiguration()); mockMvc .perform(get("/api/v1/agent-configuration/" + agentId).headers(headers)) @@ -60,6 +56,30 @@ void getConfiguration_whenConfigAvailable_shouldReturnOk() throws Exception { .andExpect(content().json(objectMapper.writeValueAsString(new InspectitConfiguration()))); } + @Test + void getConfiguration_whenAgentIsNew_shouldReturnRegistrationHeaderTrue() throws Exception { + HttpHeaders headers = getGepardHeaders(); + when(configurationService.getConfiguration()).thenReturn(new InspectitConfiguration()); + when(connectionService.handleConfigurationRequest(agentId, headers.toSingleValueMap())) + .thenReturn(true); + + mockMvc + .perform(get("/api/v1/agent-configuration/" + agentId).headers(headers)) + .andExpect(header().string("x-gepard-service-registered", "true")); + } + + @Test + void getConfiguration_whenAgentIsNew_shouldReturnRegistrationHeaderFalse() throws Exception { + HttpHeaders headers = getGepardHeaders(); + when(configurationService.getConfiguration()).thenReturn(new InspectitConfiguration()); + when(connectionService.handleConfigurationRequest(agentId, headers.toSingleValueMap())) + .thenReturn(false); + + mockMvc + .perform(get("/api/v1/agent-configuration/" + agentId).headers(headers)) + .andExpect(header().string("x-gepard-service-registered", "false")); + } + @Test void updateConfiguration_shouldReturnOkAndConfiguration() throws Exception { InspectitConfiguration configuration = InspectitConfigurationTestUtil.createConfiguration(); diff --git a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationServiceTest.java b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationServiceTest.java index 566b718a..f923159a 100644 --- a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationServiceTest.java +++ b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/configuration/service/ConfigurationServiceTest.java @@ -4,7 +4,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; -import static rocks.inspectit.gepard.agentmanager.testutils.ConfigurationRequestTestUtils.getGepardHeaders; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -14,9 +13,6 @@ import org.mockito.Mock; import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.context.ApplicationEventPublisher; -import org.springframework.http.HttpHeaders; -import rocks.inspectit.gepard.agentmanager.configuration.events.ConfigurationRequestEvent; import rocks.inspectit.gepard.agentmanager.exception.JsonParseException; import rocks.inspectit.gepard.agentmanager.testutils.InspectitConfigurationTestUtil; import rocks.inspectit.gepard.config.model.InspectitConfiguration; @@ -25,7 +21,6 @@ class ConfigurationServiceTest { @InjectMocks private ConfigurationService configurationService; - @Mock private ApplicationEventPublisher applicationEventPublisher; @Mock private GitService gitService; @Spy private ObjectMapper objectMapper; @@ -130,13 +125,10 @@ void testHandleConfigurationRequest() throws JsonProcessingException { } """; - HttpHeaders headers = getGepardHeaders(); when(gitService.getFileContent()).thenReturn(fileContent); when(objectMapper.readValue(fileContent, InspectitConfiguration.class)) .thenReturn(configuration); - InspectitConfiguration result = - configurationService.handleConfigurationRequest("agentId", headers.toSingleValueMap()); - verify(applicationEventPublisher).publishEvent(any(ConfigurationRequestEvent.class)); + InspectitConfiguration result = configurationService.getConfiguration(); assertEquals(configuration, result); } } diff --git a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionServiceTest.java b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionServiceTest.java index 8a365643..f870ca90 100644 --- a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionServiceTest.java +++ b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/connection/service/ConnectionServiceTest.java @@ -2,15 +2,16 @@ package rocks.inspectit.gepard.agentmanager.connection.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.*; import static rocks.inspectit.gepard.agentmanager.testutils.ConfigurationRequestTestUtils.getGepardHeadersAsMap; -import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -18,7 +19,6 @@ import org.mockito.InjectMocks; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.boot.test.mock.mockito.MockBean; -import rocks.inspectit.gepard.agentmanager.configuration.events.ConfigurationRequestEvent; import rocks.inspectit.gepard.agentmanager.connection.model.Connection; import rocks.inspectit.gepard.agentmanager.connection.model.ConnectionStatus; import rocks.inspectit.gepard.agentmanager.connection.model.dto.ConnectionDto; @@ -88,6 +88,40 @@ void testGetConnection() { } } + @Nested + class HandleConfigurationRequest { + @Test + void testHandleConfigurationRequestUpdatesLastFetchOnExistingEntity() + throws InterruptedException { + String id = "7e4686b7998c88427b14700f1c2aa69304a1c2fdb899067efe8ba9542fc02029"; + + boolean isFirstRequest; + isFirstRequest = connectionService.handleConfigurationRequest(id, getGepardHeadersAsMap()); + ConnectionDto connectionDto = connectionService.getConnection(id); + assertTrue(isFirstRequest); + isFirstRequest = connectionService.handleConfigurationRequest(id, getGepardHeadersAsMap()); + ConnectionDto connectionDto2 = connectionService.getConnection(id); + + await().atMost(1, TimeUnit.SECONDS); + assertFalse(isFirstRequest); + ConnectionDto connectionDto3 = connectionService.getConnection(id); + + assertTrue( + connectionDto2.timeSinceLastFetch().compareTo(connectionDto3.timeSinceLastFetch()) < 0); + } + + @Test + void testHandleConfigurationRequestCreatesNewEntity() { + String id = "7e4686b7998c88427b14700f1c2aa69304a1c2fdb899067efe8ba9542fc02029"; + + boolean isFirstRequest = + connectionService.handleConfigurationRequest(id, getGepardHeadersAsMap()); + ConnectionDto connectionDto = connectionService.getConnection(id); + assertTrue(isFirstRequest); + assertNotNull(connectionDto); + } + } + @Nested class HandleUpdateRequest { @@ -391,47 +425,6 @@ void testQueryShouldFindConnectionsByRegexAgentAttributes() { assertThat(result).hasSize(1); assertThat(result.get(0).attributes()).containsEntry("key1", "value-123"); } - - @Nested - class handleConfigurationRequestEvent { - @Test - void testHandleConfigurationRequestCreatesNewConnectionForUnknownService() - throws InterruptedException { - String id = "7e4686b7998c88427b14700f1c2aa69304a1c2fdb899067efe8ba9542fc02029"; - - Map headers = getGepardHeadersAsMap(); - - ConfigurationRequestEvent event = new ConfigurationRequestEvent(this, id, headers); - connectionService.handleConfigurationRequestEvent(event); - - Connection response = connectionCache.get(id); - - assertEquals( - Instant.parse(headers.get("x-gepard-start-time")), response.getAgent().getStartTime()); - assertEquals(headers.get("x-gepard-java-version"), response.getAgent().getJavaVersion()); - assertEquals(headers.get("x-gepard-otel-version"), response.getAgent().getOtelVersion()); - assertEquals( - headers.get("x-gepard-gepard-version"), response.getAgent().getGepardVersion()); - assertEquals(headers.get("x-gepard-vm-id"), response.getAgent().getVmId()); - assertEquals(headers.get("x-gepard-service-name"), response.getAgent().getServiceName()); - } - - @Test - void testHandleConfigurationRequestUpdatesLastFetchTimeForKnownService() { - String id = "7e4686b7998c88427b14700f1c2aa69304a1c2fdb899067efe8ba9542fc02029"; - - Map headers = getGepardHeadersAsMap(); - - connectionService.handleConfigurationRequestEvent( - new ConfigurationRequestEvent(this, id, headers)); - - Duration firstFetchTime = connectionService.getConnection(id).timeSinceLastFetch(); - connectionService.handleConfigurationRequestEvent( - new ConfigurationRequestEvent(this, id, headers)); - Duration secondFetchTime = connectionService.getConnection(id).timeSinceLastFetch(); - assertTrue(secondFetchTime.compareTo(firstFetchTime) < 0); - } - } } private Connection createTestConnection() { diff --git a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/exception/GlobalExceptionHandlerTest.java b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/exception/GlobalExceptionHandlerTest.java index 97d37d03..62d18964 100644 --- a/backend/src/test/java/rocks/inspectit/gepard/agentmanager/exception/GlobalExceptionHandlerTest.java +++ b/backend/src/test/java/rocks/inspectit/gepard/agentmanager/exception/GlobalExceptionHandlerTest.java @@ -195,4 +195,19 @@ void handleInvalidPatternSyntax() { assertEquals("requestURI", response.getBody().path()); assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } + + @Test + void handleMissingHeaderException() { + MissingHeaderException exception = Mockito.mock(MissingHeaderException.class); + HttpServletRequest httpServletRequest = Mockito.mock(HttpServletRequest.class); + Mockito.when(httpServletRequest.getRequestURI()).thenReturn("requestURI"); + Mockito.when(exception.getMessage()).thenReturn("exception message"); + + ResponseEntity response = + globalExceptionHandler.handleMissingHeaderException(exception, httpServletRequest); + + assertEquals(List.of("exception message"), Objects.requireNonNull(response.getBody()).errors()); + assertEquals("requestURI", response.getBody().path()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); + } }