From 26ade66399cfdcedb2e40e34287c58bdda5d19fb Mon Sep 17 00:00:00 2001 From: strehle Date: Fri, 8 Nov 2024 16:57:51 +0100 Subject: [PATCH] fix rebase --- server/build.gradle | 1 - .../identity/uaa/cache/StaleUrlCache.java | 2 +- .../uaa/cache/StaleUrlCacheTests.java | 143 ++++++++---------- 3 files changed, 66 insertions(+), 80 deletions(-) diff --git a/server/build.gradle b/server/build.gradle index 8111360a22..c7819987f4 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -111,7 +111,6 @@ dependencies { testImplementation(libraries.jsonPathAssert) testImplementation(libraries.guavaTestLib) testImplementation(libraries.xmlUnit) - testImplementation(libraries.awaitility) implementation(libraries.commonsIo) } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCache.java b/server/src/main/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCache.java index c8ff7cd88c..984a6f5f6b 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCache.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCache.java @@ -119,7 +119,7 @@ static class CacheEntry { } } - class UrlCacheLoader extends CacheLoader { + class UrlCacheLoader implements CacheLoader { private final TimeService timeService; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCacheTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCacheTests.java index 9f6ac23922..226f24a741 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCacheTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCacheTests.java @@ -25,17 +25,18 @@ import java.net.URISyntaxException; import java.time.Duration; import java.util.Arrays; -import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTimeout; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -44,12 +45,24 @@ class StaleUrlCacheTests { private static final Duration CACHE_EXPIRATION = Duration.ofMinutes(10); - private static final Duration CACHE_EXPIRED = CACHE_EXPIRATION.multipliedBy(2).plusMinutes(1); - private static final String URI = "http://localhost:8080/uaa/.well-known/openid-configuration"; + private static final Duration CACHE_EXPIRED = CACHE_EXPIRATION.plusMinutes(1); + private static final String URL = "http://localhost:8080/uaa/.well-known/openid-configuration"; private static final byte[] content1; private static final byte[] content2; private static final byte[] content3; + private StaleUrlCache cache; + @Mock + private TimeService mockTimeService; + @Mock + private RestTemplate mockRestTemplate; + @Mock + HttpEntity httpEntity; + @Mock + ResponseEntity responseEntity; + + private TestTicker ticker; + static { content1 = new byte[8]; Arrays.fill(content1, (byte) 1); @@ -59,17 +72,6 @@ class StaleUrlCacheTests { Arrays.fill(content3, (byte) 3); } - @Mock - HttpEntity httpEntity; - @Mock - ResponseEntity responseEntity; - private StaleUrlCache cache; - @Mock - private TimeService mockTimeService; - @Mock - private RestTemplate mockRestTemplate; - private TestTicker ticker; - @BeforeEach void setup() { ticker = new TestTicker(System.nanoTime()); @@ -79,8 +81,8 @@ void setup() { @Test void correct_method_invoked_on_rest_template() throws URISyntaxException { - cache.getUrlContent(URI, mockRestTemplate); - verify(mockRestTemplate, times(1)).getForObject(eq(new URI(URI)), same(byte[].class)); + cache.getUrlContent(URL, mockRestTemplate); + verify(mockRestTemplate, times(1)).getForObject(eq(new URI(URL)), same(byte[].class)); } @Test @@ -91,35 +93,37 @@ void incorrect_uri_throws_illegal_argument_exception() { @Test void rest_client_exception_is_propagated() { when(mockRestTemplate.getForObject(any(URI.class), any())).thenThrow(new RestClientException("mock")); - assertThatExceptionOfType(RestClientException.class).isThrownBy(() -> cache.getUrlContent(URI, mockRestTemplate)); + assertThatExceptionOfType(RestClientException.class).isThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate)); } @Test void calling_twice_uses_cache() throws Exception { - byte[] c1 = cache.getUrlContent(URI, mockRestTemplate); - byte[] c2 = cache.getUrlContent(URI, mockRestTemplate); - verify(mockRestTemplate, times(1)).getForObject(eq(new URI(URI)), same(byte[].class)); + byte[] c1 = cache.getUrlContent(URL, mockRestTemplate); + byte[] c2 = cache.getUrlContent(URL, mockRestTemplate); + verify(mockRestTemplate, times(1)).getForObject(eq(new URI(URL)), same(byte[].class)); assertThat(c2).isSameAs(c1); assertThat(cache.size()).isOne(); } @Test - void entry_refreshes_after_time() { + void entry_refreshes_after_time() throws Exception { when(mockTimeService.getCurrentTimeMillis()).thenAnswer(e -> System.currentTimeMillis()); when(mockRestTemplate.getForObject(any(URI.class), any())).thenReturn(content1, content2, content3); // populate the cache - byte[] c1 = cache.getUrlContent(URI, mockRestTemplate); + byte[] c1 = cache.getUrlContent(URL, mockRestTemplate); ticker.advance(CACHE_EXPIRED); // next call after timeout, should force async refresh - byte[] c2 = cache.getUrlContent(URI, mockRestTemplate); + byte[] c2 = cache.getUrlContent(URL, mockRestTemplate); assertThat(c2).isSameAs(c1); - // Allow time for the async getUrlContent to be called - await().atMost(1, TimeUnit.SECONDS).untilAsserted(() -> verify(mockRestTemplate, times(2)).getForObject(eq(new URI(URI)), same(byte[].class))); - // Allow time for the async update to caffeine's cache. - await().atMost(1, TimeUnit.SECONDS).untilAsserted(() -> assertThat(cache.getUrlContent(URI, mockRestTemplate)).isNotSameAs(c1)); + // allow the async refresh to complete + verify(mockRestTemplate, timeout(1000).times(2)).getForObject(eq(new URI(URL)), same(byte[].class)); + + // the next call should return the new content + byte[] c3 = cache.getUrlContent(URL, mockRestTemplate); + assertThat(c3).isNotSameAs(c1); } @Test @@ -150,22 +154,23 @@ void max_entries_is_respected() throws URISyntaxException { } @Test - void stale_entry_returned_on_failure() { + void stale_entry_returned_on_failure() throws Exception { when(mockRestTemplate.getForObject(any(URI.class), any())).thenReturn(content3).thenThrow(new RestClientException("mock")); // populate the cache - byte[] c1 = cache.getUrlContent(URI, mockRestTemplate); + byte[] c1 = cache.getUrlContent(URL, mockRestTemplate); ticker.advance(CACHE_EXPIRED); // next call after timeout, should force async refresh - byte[] c2 = cache.getUrlContent(URI, mockRestTemplate); + byte[] c2 = cache.getUrlContent(URL, mockRestTemplate); assertThat(c2).isSameAs(c1); - // Allow time for the async getUrlContent to be called - await().atMost(1, TimeUnit.SECONDS).untilAsserted(() -> verify(mockRestTemplate, times(2)).getForObject(eq(new URI(URI)), same(byte[].class))); - // Allow time for the async update to caffeine's cache. - // It should continue returning the stale content due to the exception - await().during(200, TimeUnit.MILLISECONDS).untilAsserted(() -> assertThat(cache.getUrlContent(URI, mockRestTemplate)).isSameAs(c1)); + // allow the async refresh to complete + verify(mockRestTemplate, timeout(1000).times(2)).getForObject(eq(new URI(URL)), same(byte[].class)); + + // the next call would normally return the new content, in this case it should return the stale content + byte[] c3 = cache.getUrlContent(URL, mockRestTemplate); + assertThat(c3).isSameAs(c1); } @Test @@ -173,29 +178,12 @@ void extended_method_invoked_on_rest_template() throws URISyntaxException { when(mockRestTemplate.exchange(any(URI.class), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))).thenReturn(responseEntity); when(responseEntity.getStatusCode()).thenReturn(HttpStatus.OK); when(responseEntity.getBody()).thenReturn(new byte[1]); - cache.getUrlContent(URI, mockRestTemplate, HttpMethod.GET, httpEntity); - verify(mockRestTemplate, times(1)).exchange(eq(new URI(URI)), + cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity); + verify(mockRestTemplate, times(1)).exchange(eq(new URI(URL)), eq(HttpMethod.GET), any(HttpEntity.class), same(byte[].class)); } @Test - void exception_invoked_on_rest_template() { - when(mockRestTemplate.exchange(any(URI.class), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))).thenThrow(new UncheckedExecutionException(new IllegalArgumentException("illegal"))); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> cache.getUrlContent(URI, mockRestTemplate, HttpMethod.GET, httpEntity)); - } - - @Test - void test_equal() { - StaleUrlCache.UriRequest uriRequest = new StaleUrlCache.UriRequest(URI, mockRestTemplate, HttpMethod.GET, responseEntity); - assertThat(uriRequest.equals(uriRequest)).isTrue(); - assertThat(uriRequest.equals(null)).isFalse(); - assertThat(uriRequest.equals(URI)).isFalse(); - assertThat(new StaleUrlCache.UriRequest(URI, mockRestTemplate, HttpMethod.GET, responseEntity).equals(uriRequest)).isTrue(); - assertThat(new StaleUrlCache.UriRequest(null, mockRestTemplate, HttpMethod.GET, responseEntity).equals(uriRequest)).isFalse(); - } - - @Test - void extended_method_invoked_on_rest_template_invalid_http_response() { void exception_invoked_on_rest_template() { when(mockRestTemplate.exchange(any(URI.class), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))).thenThrow(new UncheckedExecutionException(new IllegalArgumentException("illegal"))); assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity)); @@ -215,8 +203,7 @@ void test_equal() { void extended_method_invoked_on_rest_template_invalid_http_response() { when(mockRestTemplate.exchange(any(URI.class), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))).thenReturn(responseEntity); when(responseEntity.getStatusCode()).thenReturn(HttpStatus.TEMPORARY_REDIRECT); - assertThatThrownBy(() -> cache.getUrlContent(URI, mockRestTemplate, HttpMethod.GET, httpEntity)) - .isInstanceOf(IllegalArgumentException.class); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity)); } @Test @@ -228,23 +215,6 @@ void constructor_executed() { assertThat(urlCache.size()).isZero(); } - static class TestTicker implements Ticker { - long nanos; - - public TestTicker(long initialNanos) { - nanos = initialNanos; - } - - @Override - public long read() { - return nanos; - } - - public void advance(Duration duration) { - nanos += duration.toNanos(); - } - } - @Nested @DisplayName("When a http server never returns a http response") class DeadHttpServer { @@ -268,9 +238,26 @@ void throwUnavailableIdpWhenServerMetadataDoesNotReply() { RestTemplate restTemplate = restTemplateConfig.trustingRestTemplate(); String url = slowHttpServer.getUrl(); - await().atMost(60, TimeUnit.SECONDS).untilAsserted(() -> - assertThatThrownBy(() -> cache.getUrlContent(url, restTemplate)) - .isInstanceOf(ResourceAccessException.class)); + assertTimeout(Duration.ofSeconds(60), () -> assertThatThrownBy(() -> cache.getUrlContent(url, restTemplate)) + .isInstanceOf(ResourceAccessException.class) + ); + } + } + + static class TestTicker implements Ticker { + long nanos; + + public TestTicker(long initialNanos) { + nanos = initialNanos; + } + + @Override + public long read() { + return nanos; + } + + public void advance(Duration duration) { + nanos += duration.toNanos(); } } -} +} \ No newline at end of file