Skip to content

Commit

Permalink
Return covariant future types from session async methods
Browse files Browse the repository at this point in the history 
Motivation:

We experienced problems by the past trying to extend classes and
override methods returning CompletionStage<SomeInterface>. It is
usually better to return CompletionStage<? extends SomeInterface> since
this can be overridden by CompletionStage<? extends SomeChildInterface>
thanks to type covariance.

We modified lots of methods but for some reason we forgot a few ones in
CqlSession and DefaultSession.

Modifications:

- Modify return types of methods in CqlSession and DefaultSession from
  CompletionStage<T> to CompletionStage<? extends T>.

Result:

Methods in CqlSession and DefaultSession can now be overriden more
easily.
  • Loading branch information
@adutra @olim7t
adutra authored and olim7t committed Sep 26, 2018
1 parent a4cf1ae commit eb4dcda
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 17 deletions.
75 changes: 75 additions & 0 deletions core/revapi.json
View file
Expand Up @@ -522,6 +522,81 @@
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Add ability to query specific nodes for virtual tables"
},
{
"code": "java.method.returnTypeTypeParametersChanged",
"old": "method java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.AsyncResultSet> com.datastax.oss.driver.api.core.CqlSession::executeAsync(com.datastax.oss.driver.api.core.cql.Statement<?>)",
"new": "method java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.AsyncResultSet> com.datastax.oss.driver.api.core.CqlSession::executeAsync(com.datastax.oss.driver.api.core.cql.Statement<?>)",
"oldType": "java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.AsyncResultSet>",
"newType": "java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.AsyncResultSet>",
"package": "com.datastax.oss.driver.api.core",
"classQualifiedName": "com.datastax.oss.driver.api.core.CqlSession",
"classSimpleName": "CqlSession",
"methodName": "executeAsync",
"oldArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta1",
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Return covariant future types from session async methods"
},
{
"code": "java.method.returnTypeTypeParametersChanged",
"old": "method java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.AsyncResultSet> com.datastax.oss.driver.api.core.CqlSession::executeAsync(java.lang.String)",
"new": "method java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.AsyncResultSet> com.datastax.oss.driver.api.core.CqlSession::executeAsync(java.lang.String)",
"oldType": "java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.AsyncResultSet>",
"newType": "java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.AsyncResultSet>",
"package": "com.datastax.oss.driver.api.core",
"classQualifiedName": "com.datastax.oss.driver.api.core.CqlSession",
"classSimpleName": "CqlSession",
"methodName": "executeAsync",
"oldArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta1",
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Return covariant future types from session async methods"
},
{
"code": "java.method.returnTypeTypeParametersChanged",
"old": "method java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(com.datastax.oss.driver.api.core.cql.PrepareRequest)",
"new": "method java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(com.datastax.oss.driver.api.core.cql.PrepareRequest)",
"oldType": "java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"newType": "java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"package": "com.datastax.oss.driver.api.core",
"classQualifiedName": "com.datastax.oss.driver.api.core.CqlSession",
"classSimpleName": "CqlSession",
"methodName": "prepareAsync",
"oldArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta1",
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Return covariant future types from session async methods"
},
{
"code": "java.method.returnTypeTypeParametersChanged",
"old": "method java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(com.datastax.oss.driver.api.core.cql.SimpleStatement)",
"new": "method java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(com.datastax.oss.driver.api.core.cql.SimpleStatement)",
"oldType": "java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"newType": "java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"package": "com.datastax.oss.driver.api.core",
"classQualifiedName": "com.datastax.oss.driver.api.core.CqlSession",
"classSimpleName": "CqlSession",
"methodName": "prepareAsync",
"oldArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta1",
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Return covariant future types from session async methods"
},
{
"code": "java.method.returnTypeTypeParametersChanged",
"old": "method java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(java.lang.String)",
"new": "method java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement> com.datastax.oss.driver.api.core.CqlSession::prepareAsync(java.lang.String)",
"oldType": "java.util.concurrent.CompletionStage<com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"newType": "java.util.concurrent.CompletionStage<? extends com.datastax.oss.driver.api.core.cql.PreparedStatement>",
"package": "com.datastax.oss.driver.api.core",
"classQualifiedName": "com.datastax.oss.driver.api.core.CqlSession",
"classSimpleName": "CqlSession",
"methodName": "prepareAsync",
"oldArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta1",
"newArchive": "com.datastax.oss:java-driver-core:jar:4.0.0-beta2-SNAPSHOT",
"elementKind": "method",
"justification": "Return covariant future types from session async methods"
}
]
}
Expand Down
11 changes: 6 additions & 5 deletions core/src/main/java/com/datastax/oss/driver/api/core/CqlSession.java
View file
Expand Up @@ -61,7 +61,7 @@ default ResultSet execute(@NonNull String query) {
* generally before the result is available).
*/
@NonNull
default CompletionStage<AsyncResultSet> executeAsync(@NonNull Statement<?> statement) {
default CompletionStage<? extends AsyncResultSet> executeAsync(@NonNull Statement<?> statement) {
return Objects.requireNonNull(
execute(statement, Statement.ASYNC), "The CQL processor should never return a null result");
}
Expand All @@ -71,7 +71,7 @@ default CompletionStage<AsyncResultSet> executeAsync(@NonNull Statement<?> state
* generally before the result is available).
*/
@NonNull
default CompletionStage<AsyncResultSet> executeAsync(@NonNull String query) {
default CompletionStage<? extends AsyncResultSet> executeAsync(@NonNull String query) {
return executeAsync(SimpleStatement.newInstance(query));
}

Expand Down Expand Up @@ -167,7 +167,8 @@ default PreparedStatement prepare(@NonNull PrepareRequest request) {
* details.
*/
@NonNull
default CompletionStage<PreparedStatement> prepareAsync(@NonNull SimpleStatement statement) {
default CompletionStage<? extends PreparedStatement> prepareAsync(
@NonNull SimpleStatement statement) {
return Objects.requireNonNull(
execute(new DefaultPrepareRequest(statement), PrepareRequest.ASYNC),
"The CQL prepare processor should never return a null result");
Expand All @@ -178,7 +179,7 @@ default CompletionStage<PreparedStatement> prepareAsync(@NonNull SimpleStatement
* sent, generally before the statement is prepared).
*/
@NonNull
default CompletionStage<PreparedStatement> prepareAsync(@NonNull String query) {
default CompletionStage<? extends PreparedStatement> prepareAsync(@NonNull String query) {
return Objects.requireNonNull(
execute(new DefaultPrepareRequest(query), PrepareRequest.ASYNC),
"The CQL prepare processor should never return a null result");
Expand All @@ -194,7 +195,7 @@ default CompletionStage<PreparedStatement> prepareAsync(@NonNull String query) {
* with {@link PrepareRequest} directly.
*/
@NonNull
default CompletionStage<PreparedStatement> prepareAsync(PrepareRequest request) {
default CompletionStage<? extends PreparedStatement> prepareAsync(PrepareRequest request) {
return Objects.requireNonNull(
execute(request, PrepareRequest.ASYNC),
"The CQL prepare processor should never return a null result");
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastax/oss/driver/internal/core/cql/DefaultAsyncResultSet.java
View file
Expand Up @@ -95,7 +95,7 @@ public boolean hasMorePages() {

@NonNull
@Override
public CompletionStage<AsyncResultSet> fetchNextPage() throws IllegalStateException {
public CompletionStage<? extends AsyncResultSet> fetchNextPage() throws IllegalStateException {
ByteBuffer nextState = executionInfo.getPagingState();
if (nextState == null) {
throw new IllegalStateException(
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/com/datastax/oss/driver/internal/core/session/DefaultSession.java
View file
Expand Up @@ -128,13 +128,13 @@ public boolean isSchemaMetadataEnabled() {

@NonNull
@Override
public CompletionStage<Metadata> setSchemaMetadataEnabled(@Nullable Boolean newValue) {
public CompletionStage<? extends Metadata> setSchemaMetadataEnabled(@Nullable Boolean newValue) {
return metadataManager.setSchemaEnabled(newValue);
}

@NonNull
@Override
public CompletionStage<Metadata> refreshSchemaAsync() {
public CompletionStage<? extends Metadata> refreshSchemaAsync() {
return metadataManager.refreshSchema(null, true, true);
}

Expand Down
7 changes: 4 additions & 3 deletions core/src/test/java/com/datastax/oss/driver/internal/core/cql/DefaultAsyncResultSetTest.java
View file
Expand Up @@ -55,7 +55,7 @@ public class DefaultAsyncResultSetTest {
public void setup() {
MockitoAnnotations.initMocks(this);

Mockito.when(executionInfo.getStatement()).thenReturn((Statement) statement);
Mockito.when(executionInfo.getStatement()).thenAnswer(invocation -> statement);
Mockito.when(context.getCodecRegistry()).thenReturn(CodecRegistry.DEFAULT);
Mockito.when(context.getProtocolVersion()).thenReturn(DefaultProtocolVersion.DEFAULT);
}
Expand Down Expand Up @@ -85,14 +85,15 @@ public void should_invoke_session_to_fetch_next_page() {
Mockito.when(((Statement) statement).copy(mockPagingState)).thenReturn(mockNextStatement);

CompletableFuture<AsyncResultSet> mockResultFuture = new CompletableFuture<>();
Mockito.when(session.executeAsync(Mockito.any(Statement.class))).thenReturn(mockResultFuture);
Mockito.when(session.executeAsync(Mockito.any(Statement.class)))
.thenAnswer(invocation -> mockResultFuture);

// When
DefaultAsyncResultSet resultSet =
new DefaultAsyncResultSet(
columnDefinitions, executionInfo, new ArrayDeque<>(), session, context);
assertThat(resultSet.hasMorePages()).isTrue();
CompletionStage<AsyncResultSet> nextPageFuture = resultSet.fetchNextPage();
CompletionStage<? extends AsyncResultSet> nextPageFuture = resultSet.fetchNextPage();

// Then
Mockito.verify(statement).copy(mockPagingState);
Expand Down
15 changes: 11 additions & 4 deletions core/src/test/java/com/datastax/oss/driver/internal/core/cql/QueryTraceFetcherTest.java
View file
Expand Up @@ -116,7 +116,8 @@ public void should_succeed_when_both_queries_succeed_immediately() {
CompletionStage<AsyncResultSet> sessionRow = completeSessionRow();
CompletionStage<AsyncResultSet> eventRows = singlePageEventRows();
Mockito.when(session.executeAsync(any(SimpleStatement.class)))
.thenReturn(sessionRow, eventRows);
.thenAnswer(invocation -> sessionRow)
.thenAnswer(invocation -> eventRows);

// When
QueryTraceFetcher fetcher = new QueryTraceFetcher(TRACING_ID, session, context, config);
Expand Down Expand Up @@ -167,7 +168,9 @@ public void should_succeed_when_events_query_is_paged() {
CompletionStage<AsyncResultSet> eventRows1 = multiPageEventRows1();
CompletionStage<AsyncResultSet> eventRows2 = multiPageEventRows2();
Mockito.when(session.executeAsync(any(SimpleStatement.class)))
.thenReturn(sessionRow, eventRows1, eventRows2);
.thenAnswer(invocation -> sessionRow)
.thenAnswer(invocation -> eventRows1)
.thenAnswer(invocation -> eventRows2);

// When
QueryTraceFetcher fetcher = new QueryTraceFetcher(TRACING_ID, session, context, config);
Expand All @@ -192,7 +195,9 @@ public void should_retry_when_session_row_is_incomplete() {
CompletionStage<AsyncResultSet> sessionRow2 = completeSessionRow();
CompletionStage<AsyncResultSet> eventRows = singlePageEventRows();
Mockito.when(session.executeAsync(any(SimpleStatement.class)))
.thenReturn(sessionRow1, sessionRow2, eventRows);
.thenAnswer(invocation -> sessionRow1)
.thenAnswer(invocation -> sessionRow2)
.thenAnswer(invocation -> eventRows);

// When
QueryTraceFetcher fetcher = new QueryTraceFetcher(TRACING_ID, session, context, config);
Expand Down Expand Up @@ -259,7 +264,9 @@ public void should_fail_when_session_query_still_incomplete_after_max_tries() {
CompletionStage<AsyncResultSet> sessionRow2 = incompleteSessionRow();
CompletionStage<AsyncResultSet> sessionRow3 = incompleteSessionRow();
Mockito.when(session.executeAsync(any(SimpleStatement.class)))
.thenReturn(sessionRow1, sessionRow2, sessionRow3);
.thenAnswer(invocation -> sessionRow1)
.thenAnswer(invocation -> sessionRow2)
.thenAnswer(invocation -> sessionRow3);

// When
QueryTraceFetcher fetcher = new QueryTraceFetcher(TRACING_ID, session, context, config);
Expand Down
2 changes: 1 addition & 1 deletion ...ration-tests/src/test/java/com/datastax/oss/driver/api/core/connection/FrameLengthIT.java
View file
Expand Up @@ -92,7 +92,7 @@ public void should_fail_if_request_exceeds_max_frame_length() {

@Test
public void should_fail_if_response_exceeds_max_frame_length() {
CompletionStage<AsyncResultSet> slowResultFuture =
CompletionStage<? extends AsyncResultSet> slowResultFuture =
sessionRule.session().executeAsync(SLOW_QUERY);
try {
sessionRule.session().execute(LARGE_QUERY);
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/test/java/com/datastax/oss/driver/api/core/cql/AsyncResultSetIT.java
View file
Expand Up @@ -85,7 +85,7 @@ public static void setupSchema() {
public void should_only_iterate_over_rows_in_current_page() throws Exception {
// very basic test that just ensures that iterating over an AsyncResultSet only visits the first
// page.
CompletionStage<AsyncResultSet> result =
CompletionStage<? extends AsyncResultSet> result =
sessionRule
.session()
.executeAsync(
Expand Down

0 comments on commit eb4dcda

Please sign in to comment.