diff --git a/src/main/java/no/ssb/guardian/RetryListener.java b/src/main/java/no/ssb/guardian/RetryListener.java new file mode 100644 index 0000000..31332fe --- /dev/null +++ b/src/main/java/no/ssb/guardian/RetryListener.java @@ -0,0 +1,24 @@ +package no.ssb.guardian; + +import io.micronaut.core.type.MutableArgumentValue; +import io.micronaut.retry.event.RetryEvent; +import io.micronaut.retry.event.RetryEventListener; +import jakarta.inject.Singleton; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@Singleton +public class RetryListener implements RetryEventListener { + @Override + public void onApplicationEvent(RetryEvent event) { + final MutableArgumentValue request = event.getSource().getParameters().get("request"); + log.error("Request failed {} time(s) for {}:", event.getRetryState().currentAttempt(), + request != null ? request.getValue(): "", + event.getThrowable()); + } + + @Override + public boolean supports(RetryEvent event) { + return true; + } +} diff --git a/src/main/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenController.java b/src/main/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenController.java index dd71cfb..7a67ed4 100644 --- a/src/main/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenController.java +++ b/src/main/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenController.java @@ -9,6 +9,8 @@ import io.micronaut.http.annotation.Post; import io.micronaut.http.hateoas.JsonError; import io.micronaut.http.hateoas.Link; +import io.micronaut.retry.annotation.RetryPredicate; +import io.micronaut.retry.annotation.Retryable; import io.micronaut.security.annotation.Secured; import io.micronaut.security.rules.SecurityRule; import lombok.AllArgsConstructor; @@ -23,6 +25,7 @@ import no.ssb.guardian.maskinporten.config.MaskinportenClientConfig; import no.ssb.guardian.maskinporten.config.MaskinportenConfig; +import java.net.SocketException; import java.security.Principal; import java.util.Set; @@ -37,6 +40,7 @@ public class MaskinportenAccessTokenController { private final MaskinportenConfig maskinportenConfig; @Post("/maskinporten/access-token") + @Retryable(predicate = WrappedSocketExceptionRetryPredicate.class) public HttpResponse fetchMaskinportenAccessToken(Principal principal, FetchMaskinportenAccessTokenRequest request) { log.info("Request: {}", request); log.info("AUDIT {}", PrincipalUtil.auditInfoOf(principal)); @@ -130,6 +134,17 @@ public ClientRequestException(HttpStatus httpStatus, String errorTag, String mes } + /** + * The {@link io.micronaut.retry.annotation.DefaultRetryPredicate} does not support wrapped exceptions, so, this + * prediate checks the exception {@code cause} for a {@link SocketException}. + */ + public static class WrappedSocketExceptionRetryPredicate implements RetryPredicate { + @Override + public boolean test(Throwable throwable) { + return throwable.getCause() != null && throwable.getCause().getClass().equals(SocketException.class); + } + } + @Data @NoArgsConstructor @AllArgsConstructor @Builder @@ -168,6 +183,11 @@ public HttpResponse clientRequestError(HttpRequest request, Maskinpor return error(request, e, e.getHttpStatus(), e.getMessage()); } + @Error + public HttpResponse runtimeError(HttpRequest request, RuntimeException e) { + metrics.incrementServerError("runtime-exception"); + return error(request, e, HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage()); + } private static HttpResponse error(HttpRequest request, Exception e, HttpStatus httpStatus, String httpStatusReason) { JsonError error = new JsonError(e.getMessage()) .link(Link.SELF, Link.of(request.getUri())); diff --git a/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerRetryTest.java b/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerRetryTest.java new file mode 100644 index 0000000..33d7303 --- /dev/null +++ b/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerRetryTest.java @@ -0,0 +1,76 @@ +package no.ssb.guardian.maskinporten; + +import io.micronaut.http.HttpStatus; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.annotation.MockBean; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import io.restassured.RestAssured; +import io.restassured.http.ContentType; +import jakarta.inject.Inject; +import no.ssb.guardian.maskinporten.MaskinportenAccessTokenController.FetchMaskinportenAccessTokenRequest; +import no.ssb.guardian.maskinporten.client.MaskinportenClient; +import no.ssb.guardian.maskinporten.client.MaskinportenClientRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.net.SocketException; +import java.util.Set; + +import static io.restassured.RestAssured.*; +import static no.ssb.guardian.testsupport.KeycloakDevTokenIssuer.*; +import static org.hamcrest.Matchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; + +@MicronautTest +public class MaskinportenAccessTokenControllerRetryTest { + + private final static String MASKINPORTEN_ACCESS_TOKEN_ENDPOINT = "/maskinporten/access-token"; + private final static String MASKINPORTEN_DUMMY_ACCESS_TOKEN = "maskinporten-dummy-token"; + private final static String MASKINPORTEN_CLIENT_ID_1 = "7ea43b76-6b7d-49e8-af2b-4114ebb66c80"; + private final static Set REQUESTED_SCOPES = Set.of("some:scope1"); + + @Inject + private EmbeddedServer embeddedServer; + + @Inject + MaskinportenClientRegistry maskinportenClientRegistry; + + @MockBean(MaskinportenClientRegistry.class) + MaskinportenClientRegistry maskinportenklientRegistry() { + return mock(MaskinportenClientRegistry.class); + } + + private MaskinportenClient maskinportenClientMock = mock(MaskinportenClient.class); + @BeforeEach + void setUp() { + RestAssured.port = embeddedServer.getPort(); + when(maskinportenClientRegistry.get(any())).thenReturn(maskinportenClientMock); + } + + @Test + void validRequest_shouldRetryOnConnectionError() { + when(maskinportenClientMock.getAccessToken(anySet())) + .thenThrow(new RuntimeException(new SocketException(("Connection reset")))) + .thenReturn(MASKINPORTEN_DUMMY_ACCESS_TOKEN); + + given() + .auth().oauth2(personalAccessToken("Kjell", "Fjell")) + .contentType(ContentType.JSON) + .when() + .body(FetchMaskinportenAccessTokenRequest.builder() + .maskinportenClientId(MASKINPORTEN_CLIENT_ID_1) + .scopes(REQUESTED_SCOPES) + .build() + ) + .post(MASKINPORTEN_ACCESS_TOKEN_ENDPOINT) + .then() + .statusCode(HttpStatus.OK.getCode()) + .body( + "accessToken", equalTo(MASKINPORTEN_DUMMY_ACCESS_TOKEN) + ); + verify(maskinportenClientMock, times(2)).getAccessToken(anySet()); + } +} diff --git a/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerTest.java b/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerTest.java index 10c6f42..36e945e 100644 --- a/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerTest.java +++ b/src/test/java/no/ssb/guardian/maskinporten/MaskinportenAccessTokenControllerTest.java @@ -23,8 +23,7 @@ import static org.hamcrest.Matchers.startsWith; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anySet; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; @MicronautTest public class MaskinportenAccessTokenControllerTest { @@ -33,7 +32,6 @@ public class MaskinportenAccessTokenControllerTest { private final static String MASKINPORTEN_DUMMY_ACCESS_TOKEN = "maskinporten-dummy-token"; private final static String MASKINPORTEN_CLIENT_ID_1 = "7ea43b76-6b7d-49e8-af2b-4114ebb66c80"; private final static String MASKINPORTEN_CLIENT_ID_2 = "675c0111-2035-4d15-9cce-037f55439e80"; - private final static Set DEFAULT_SCOPES = Set.of("some:scope1", "some:scope2"); private final static Set REQUESTED_SCOPES = Set.of("some:scope1"); @Inject @@ -41,6 +39,8 @@ public class MaskinportenAccessTokenControllerTest { @Inject MaskinportenClientRegistry maskinportenClientRegistry; + private MaskinportenClient maskinportenClientMock = mock(MaskinportenClient.class); + @MockBean(MaskinportenClientRegistry.class) MaskinportenClientRegistry maskinportenklientRegistry() { @@ -50,10 +50,8 @@ MaskinportenClientRegistry maskinportenklientRegistry() { @BeforeEach void setUp() { RestAssured.port = embeddedServer.getPort(); - - MaskinportenClient maskinportenClient = mock(MaskinportenClient.class); - when(maskinportenClient.getAccessToken(anySet())).thenReturn(MASKINPORTEN_DUMMY_ACCESS_TOKEN); - when(maskinportenClientRegistry.get(any())).thenReturn(maskinportenClient); + when(maskinportenClientMock.getAccessToken(anySet())).thenReturn(MASKINPORTEN_DUMMY_ACCESS_TOKEN); + when(maskinportenClientRegistry.get(any())).thenReturn(maskinportenClientMock); } private String serviceAccountKeycloakToken() { @@ -76,6 +74,7 @@ void validServiceAccount_getAccessToken_shouldReturnToken() { .body( "accessToken", equalTo(MASKINPORTEN_DUMMY_ACCESS_TOKEN) ); + verify(maskinportenClientMock, times(1)).getAccessToken(anySet()); } @Test @@ -91,6 +90,7 @@ void serviceAccount_getAccessTokenWithEmptyRequestBody_shouldReturnToken() { .body( "accessToken", equalTo(MASKINPORTEN_DUMMY_ACCESS_TOKEN) ); + verify(maskinportenClientMock, times(1)).getAccessToken(anySet()); } @Test @@ -109,6 +109,7 @@ void serviceAccount_getAccessTokenWithExplicitlySpecifiedMaskinportenClientId_sh .body( "message", equalTo("maskinportenClientId cannot be explicitly specified for service account users") ); + verifyNoInteractions(maskinportenClientMock); } @Test