Skip to content

Commit

Permalink
fix rebase
Browse files Browse the repository at this point in the history
  • Loading branch information
@strehle
strehle committed Nov 8, 2024
1 parent 06c110d commit 26ade66
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 80 deletions.
1 change: 0 additions & 1 deletion server/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ dependencies {
testImplementation(libraries.jsonPathAssert)
testImplementation(libraries.guavaTestLib)
testImplementation(libraries.xmlUnit)
testImplementation(libraries.awaitility)

implementation(libraries.commonsIo)
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/cloudfoundry/identity/uaa/cache/StaleUrlCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ static class CacheEntry {
}
}

class UrlCacheLoader extends CacheLoader<UriRequest, CacheEntry> {
class UrlCacheLoader implements CacheLoader<UriRequest, CacheEntry> {

private final TimeService timeService;

Expand Down
143 changes: 65 additions & 78 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,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;
Expand All @@ -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<byte[]> responseEntity;

private TestTicker ticker;

static {
content1 = new byte[8];
Arrays.fill(content1, (byte) 1);
Expand All @@ -59,17 +72,6 @@ class StaleUrlCacheTests {
Arrays.fill(content3, (byte) 3);
}

@Mock
HttpEntity<?> httpEntity;
@Mock
ResponseEntity<byte[]> responseEntity;
private StaleUrlCache cache;
@Mock
private TimeService mockTimeService;
@Mock
private RestTemplate mockRestTemplate;
private TestTicker ticker;

@BeforeEach
void setup() {
ticker = new TestTicker(System.nanoTime());
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -150,52 +154,36 @@ 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
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));
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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();
}
}
}
}

0 comments on commit 26ade66

Please sign in to comment.