Skip to content

Commit

Permalink
Merge pull request #3179 from cloudfoundry/fix-flaky-stale-url-test
Browse files Browse the repository at this point in the history 
Fix flaky StaleUrlCacheTests
  • Loading branch information
@duanemay
duanemay authored Dec 5, 2024
2 parents 26afaec + fba5a6f commit aa1bec0
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 34 deletions.
1 change: 1 addition & 0 deletions server/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ dependencies {
testImplementation(libraries.jsonPathAssert)
testImplementation(libraries.guavaTestLib)
testImplementation(libraries.xmlUnit)
testImplementation(libraries.awaitility)

implementation(libraries.commonsIo)
}
Expand Down
84 changes: 50 additions & 34 deletions server/src/test/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCacheTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,16 @@
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.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTimeout;
import static org.awaitility.Awaitility.await;
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;
Expand Down Expand Up @@ -87,13 +85,15 @@ void correct_method_invoked_on_rest_template() throws URISyntaxException {

@Test
void incorrect_uri_throws_illegal_argument_exception() {
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> cache.getUrlContent("invalid value", mockRestTemplate));
assertThatThrownBy(() -> cache.getUrlContent("invalid value", mockRestTemplate))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void rest_client_exception_is_propagated() {
when(mockRestTemplate.getForObject(any(URI.class), any())).thenThrow(new RestClientException("mock"));
assertThatExceptionOfType(RestClientException.class).isThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate));
assertThatThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate))
.isInstanceOf(RestClientException.class);
}

@Test
Expand All @@ -106,7 +106,7 @@ void calling_twice_uses_cache() throws Exception {
}

@Test
void entry_refreshes_after_time() throws Exception {
void entry_refreshes_after_time() {
when(mockTimeService.getCurrentTimeMillis()).thenAnswer(e -> System.currentTimeMillis());
when(mockRestTemplate.getForObject(any(URI.class), any())).thenReturn(content1, content2, content3);

Expand All @@ -118,12 +118,17 @@ void entry_refreshes_after_time() throws Exception {
byte[] c2 = cache.getUrlContent(URL, mockRestTemplate);
assertThat(c2).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 should return the new content
byte[] c3 = cache.getUrlContent(URL, mockRestTemplate);
assertThat(c3).isNotSameAs(c1);
// Allow time for the async getUrlContent to be called
await().atMost(1, TimeUnit.SECONDS)
.untilAsserted(() -> verify(mockRestTemplate, times(2))
.getForObject(eq(new URI(URL)), same(byte[].class))
);

// Allow time for the async update to caffeine's cache.
await().atMost(1, TimeUnit.SECONDS)
.untilAsserted(() -> assertThat(cache.getUrlContent(URL, mockRestTemplate))
.isNotSameAs(c1)
);
}

@Test
Expand All @@ -133,9 +138,9 @@ void cache_should_start_empty() {

@Test
void max_entries_is_respected() throws URISyntaxException {
String uri1 = "http://test1.com";
String uri2 = "http://test2.com";
String uri3 = "http://test3.com";
String uri1 = "https://test1.com";
String uri2 = "https://test2.com";
String uri3 = "https://test3.com";
byte[] c1 = new byte[1024];
byte[] c2 = new byte[1024];
byte[] c3 = new byte[1024];
Expand All @@ -154,7 +159,7 @@ void max_entries_is_respected() throws URISyntaxException {
}

@Test
void stale_entry_returned_on_failure() throws Exception {
void stale_entry_returned_on_failure() {
when(mockRestTemplate.getForObject(any(URI.class), any())).thenReturn(content3).thenThrow(new RestClientException("mock"));

// populate the cache
Expand All @@ -165,12 +170,18 @@ void stale_entry_returned_on_failure() throws Exception {
byte[] c2 = cache.getUrlContent(URL, mockRestTemplate);
assertThat(c2).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);
// Allow time for the async getUrlContent to be called
await().atMost(1, TimeUnit.SECONDS)
.untilAsserted(() -> verify(mockRestTemplate, times(2))
.getForObject(eq(new URI(URL)), 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(URL, mockRestTemplate))
.isSameAs(c1)
);
}

@Test
Expand All @@ -185,25 +196,29 @@ void extended_method_invoked_on_rest_template() throws URISyntaxException {

@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(URL, mockRestTemplate, HttpMethod.GET, httpEntity));
when(mockRestTemplate.exchange(any(URI.class), any(HttpMethod.class), any(HttpEntity.class), any(Class.class)))
.thenThrow(new UncheckedExecutionException(new IllegalArgumentException("illegal")));
assertThatThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void test_equal() {
StaleUrlCache.UriRequest uriRequest = new StaleUrlCache.UriRequest(URL, mockRestTemplate, HttpMethod.GET, responseEntity);
assertEquals(uriRequest, uriRequest);
assertThat(uriRequest.equals(null)).isFalse();
assertThat(uriRequest.equals(URL)).isFalse();
assertThat(new StaleUrlCache.UriRequest(URL, mockRestTemplate, HttpMethod.GET, responseEntity).equals(uriRequest)).isTrue();
assertThat(new StaleUrlCache.UriRequest(null, mockRestTemplate, HttpMethod.GET, responseEntity).equals(uriRequest)).isFalse();
assertThat(uriRequest)
.isEqualTo(uriRequest)
.isNotEqualTo(null)
.isNotEqualTo(URL);
assertThat(new StaleUrlCache.UriRequest(URL, mockRestTemplate, HttpMethod.GET, responseEntity)).isEqualTo(uriRequest);
assertThat(new StaleUrlCache.UriRequest(null, mockRestTemplate, HttpMethod.GET, responseEntity)).isNotEqualTo(uriRequest);
}

@Test
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);
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity));
assertThatThrownBy(() -> cache.getUrlContent(URL, mockRestTemplate, HttpMethod.GET, httpEntity))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
Expand Down Expand Up @@ -238,9 +253,10 @@ void throwUnavailableIdpWhenServerMetadataDoesNotReply() {
RestTemplate restTemplate = restTemplateConfig.trustingRestTemplate();

String url = slowHttpServer.getUrl();
assertTimeout(Duration.ofSeconds(60), () -> assertThatThrownBy(() -> cache.getUrlContent(url, restTemplate))
.isInstanceOf(ResourceAccessException.class)
);
await().atMost(60, TimeUnit.SECONDS)
.untilAsserted(() -> assertThatThrownBy(() -> cache.getUrlContent(url, restTemplate))
.isInstanceOf(ResourceAccessException.class)
);
}
}

Expand Down

0 comments on commit aa1bec0

Please sign in to comment.