Skip to content

Commit

Permalink
Add method to RetryException to get a typed last Attempt (#225)
Browse files Browse the repository at this point in the history
* Add getLastFailedAttempt(Class<T> type) to RetryException
* Add reference to this method in API Note in getLastFailedAttempt()

Details:

Instead of just casting, check that the Attempt either:

1. does not contain a result (it contains an Exception)
2. contains a result that can be assigned to the specified type

If either the above conditions is true, then a cast is safe.

Define the function to throw IlegalStateException with a
descriptive message instead of just letting th JVM throw
a ClassCastException with no message.

Closes #224
  • Loading branch information
sleberknight authored Jun 6, 2024
1 parent cd55261 commit 7f89ffe
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 2 deletions.
32 changes: 30 additions & 2 deletions src/main/java/org/kiwiproject/retry/RetryException.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.kiwiproject.retry;

import static com.google.common.base.Preconditions.checkState;
import static java.util.Objects.isNull;
import static java.util.Objects.nonNull;

import com.google.errorprone.annotations.Immutable;
Expand Down Expand Up @@ -53,19 +54,46 @@ public int getNumberOfFailedAttempts() {
}

/**
* Returns the last failed attempt
* Returns the last failed attempt. The result type is unknown and must be cast.
* Consider using {@link #getLastFailedAttempt(Class)} to avoid the explicit cast.
*
* @return the last failed attempt
* @see #getLastFailedAttempt(Class)
* @apiNote This method returns {@code Attempt<?>} because the Java Language Specification does not
* permit generic subclasses of Throwable. In section
* <a href="https://docs.oracle.com/javase/specs/jls/se17/html/jls-8.html#jls-8.1.2">8.1.2, Generic Classes and Type Parameters</a>,
* the (Java SE 17) specification states that <em>"It is a compile-time error if a generic class is a direct or
* indirect subclassof Throwable"</em>. It further provides the reason, stating <em>"This restriction is needed
* since the catch mechanism of the Java Virtual Machine works only with non-generic classes."</em> As a result,
* this exception class has no (good) way to capture the {@code Attempt} type parameter. Callers of this
* method must know the expected type and cast the returned value.
* method must know the expected type and cast the returned value. An alternative to casting is to call
* the {@link #getLastFailedAttempt(Class)} method, though callers must still specify the type as an
* explicit argument.
*/
public Attempt<?> getLastFailedAttempt() {
return lastFailedAttempt;
}

/**
* Returns the last failed attempt with the given {@code resultType}.
* <p>
* If the attempt does not contain a result, and instead contains an Exception,
* then {@code resultType} is ignored.
*
* @param resultType the type of result which the Attempt must contain
* @param <T> the generic type of the Attempt
* @return the last failed attempt
* @throws IllegalStateException if the Attempt has a result that is not an instance of {@code resultType}
* @apiNote The type {@code T} of the {@code Attempt} must be explicitly specified
* because the Java Language Specification does not permit generic subclasses of Throwable.
* See the API Note in {@link #getLastFailedAttempt()} for more details.
*/
@SuppressWarnings("unchecked")
public <T> Attempt<T> getLastFailedAttempt(Class<T> resultType) {
Attempt<?> attempt = getLastFailedAttempt();
Object result = attempt.hasResult() ? attempt.getResult() : null;
checkState(isNull(result) || resultType.isAssignableFrom(result.getClass()),
"Attempt.result is not an instance of %s", resultType.getName());
return (Attempt<T>) attempt;
}
}
119 changes: 119 additions & 0 deletions src/test/java/org/kiwiproject/retry/RetryExceptionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package org.kiwiproject.retry;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.junit.jupiter.api.Assertions.assertAll;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.List;

@DisplayName("RetryException")
class RetryExceptionTest {

@Test
void shouldReturnLastAttempt_ContainingResult() {
var attempt = Attempt.newResultAttempt(42, 5, 2_000);
var e = new RetryException(attempt);

Attempt<?> lastFailedAttempt = e.getLastFailedAttempt();
assertAll(
() -> assertThat(lastFailedAttempt.hasResult()).isTrue(),
() -> assertThat(lastFailedAttempt.hasException()).isFalse(),
() -> assertThat(lastFailedAttempt.getAttemptNumber()).isEqualTo(5),
() -> assertThat(lastFailedAttempt.getDelaySinceFirstAttempt()).isEqualTo(2_000)
);

Object result = lastFailedAttempt.getResult();
assertThat(result).isEqualTo(42);
}

@Test
void shouldReturnLastAttempt_ContainingResult_WithGenericType() {
var attempt = Attempt.newResultAttempt(42, 3, 625);
var e = new RetryException(attempt);

Attempt<Integer> lastFailedAttempt = e.getLastFailedAttempt(Integer.class);
assertAll(
() -> assertThat(lastFailedAttempt.hasResult()).isTrue(),
() -> assertThat(lastFailedAttempt.hasException()).isFalse(),
() -> assertThat(lastFailedAttempt.getAttemptNumber()).isEqualTo(3),
() -> assertThat(lastFailedAttempt.getDelaySinceFirstAttempt()).isEqualTo(625)
);

Integer result = lastFailedAttempt.getResult();
assertThat(result).isEqualTo(42);
}

@Test
void shouldThrowIllegalStateException_WhenGivenIncorrectResultType() {
var attempt = Attempt.newResultAttempt(42, 3, 625);
var e = new RetryException(attempt);

assertThatIllegalStateException()
.isThrownBy(() -> e.getLastFailedAttempt(String.class))
.withMessage("Attempt.result is not an instance of java.lang.String");
}

@Test
void shouldReturnLastAttempt_ContainingException() {
var cause = new UncheckedIOException(new IOException("I/O error"));
var attempt = Attempt.newExceptionAttempt(cause, 2, 250);
var e = new RetryException(attempt);

Attempt<?> lastFailedAttempt = e.getLastFailedAttempt();
assertAll(
() -> assertThat(lastFailedAttempt.hasResult()).isFalse(),
() -> assertThat(lastFailedAttempt.hasException()).isTrue(),
() -> assertThat(lastFailedAttempt.getException()).isSameAs(cause),
() -> assertThat(lastFailedAttempt.getAttemptNumber()).isEqualTo(2),
() -> assertThat(lastFailedAttempt.getDelaySinceFirstAttempt()).isEqualTo(250)
);
}

@Test
void shouldReturnLastFailedAttempt_ContainingException_AndIgnoreSpecificResultTypes() {
var cause = new UncheckedIOException(new IOException("Disk full"));
var attempt = Attempt.newExceptionAttempt(cause, 2, 150);
var e = new RetryException(attempt);

// Define a custom result type
record User(int id, String username, String password, List<String> roleNames) {}

// These calls should all succeed, since the last Attempt does not have a result
Attempt<String> lastFailedAttempt1 = e.getLastFailedAttempt(String.class);
Attempt<Integer> lastFailedAttempt2 = e.getLastFailedAttempt(Integer.class);
Attempt<Double> lastFailedAttempt3 = e.getLastFailedAttempt(Double.class);
Attempt<User> lastFailedAttempt4 = e.getLastFailedAttempt(User.class);

assertThat(lastFailedAttempt1)
.describedAs("The exact same Attempt instance should be returned")
.isSameAs(lastFailedAttempt2)
.isSameAs(lastFailedAttempt3)
.isSameAs(lastFailedAttempt4);
}

@ParameterizedTest
@ValueSource(classes = {
String.class, Integer.class, Long.class, Double.class
})
void shouldReturnLastFailedAttempt_ContainingException_AndIgnoreResultType(Class<?> resultType) {
var cause = new UncheckedIOException(new IOException("File not found"));
var attempt = Attempt.newExceptionAttempt(cause, 3, 500);
var e = new RetryException(attempt);

Attempt<?> lastFailedAttempt = e.getLastFailedAttempt(resultType);
assertAll(
() -> assertThat(lastFailedAttempt.hasResult()).isFalse(),
() -> assertThat(lastFailedAttempt.hasException()).isTrue(),
() -> assertThat(lastFailedAttempt.getException()).isSameAs(cause),
() -> assertThat(lastFailedAttempt.getAttemptNumber()).isEqualTo(3),
() -> assertThat(lastFailedAttempt.getDelaySinceFirstAttempt()).isEqualTo(500)
);
}
}

0 comments on commit 7f89ffe

Please sign in to comment.