Skip to content

Commit 76a5354

Browse files
nikagradkropachev
andcommitted
[#596] Improving Scylla test coverage in PreparedStatementInvalidationTest. Registering for closure resources implementing AutoCloseable interface in flaky tests class.
[#596] Improving Scylla test coverage in `PreparedStatementInvalidationTest`. Apply suggestion from @dkropachev Rearranging assertions and minor improvements in `should_never_update_statement_id_for_conditional_updates` Co-authored-by: Dmitry Kropachev <[email protected]>
1 parent ad0b94a commit 76a5354

File tree

4 files changed

+155
-61
lines changed

4 files changed

+155
-61
lines changed

driver-core/src/main/java/com/datastax/driver/core/ArrayBackedResultSet.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ public void onSet(
443443
bs.preparedStatement().getPreparedId().resultSetMetadata =
444444
new PreparedId.PreparedMetadata(
445445
rows.metadata.metadataId, rows.metadata.columns);
446+
} else if (rows.metadata.columns != null
447+
&& rows.metadata.columns.size() > 0) {
448+
newMetadata = rows.metadata.columns;
446449
}
447450
MultiPage.this.nextPages.offer(new NextPage(newMetadata, rows.data));
448451
MultiPage.this.fetchState =

driver-core/src/main/java/com/datastax/driver/core/Host.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ public interface StateListener {
600600
/**
601601
* Gets invoked when the tracker is registered with a cluster, or at cluster startup if the
602602
* tracker was registered at initialization with {@link
603-
* Cluster.Initializer#register(LatencyTracker)}.
603+
* com.datastax.driver.core.Cluster.Initializer#register(LatencyTracker)}.
604604
*
605605
* @param cluster the cluster that this tracker is registered with.
606606
*/
@@ -624,7 +624,8 @@ public interface StateListener {
624624
public interface LifecycleAwareStateListener extends StateListener {
625625
/**
626626
* Gets invoked when the listener is registered with a cluster, or at cluster startup if the
627-
* listener was registered at initialization with {@link Cluster#register(StateListener)}.
627+
* listener was registered at initialization with {@link
628+
* com.datastax.driver.core.Cluster#register(Host.StateListener)}.
628629
*
629630
* @param cluster the cluster that this listener is registered with.
630631
*/

driver-core/src/test/java/com/datastax/driver/core/AuthenticationTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void sleepIf12() {
6161
@Test(groups = "short")
6262
public void should_connect_with_credentials() {
6363
PlainTextAuthProvider authProvider = spy(new PlainTextAuthProvider("cassandra", "cassandra"));
64-
Cluster cluster = createClusterBuilder().withAuthProvider(authProvider).build();
64+
Cluster cluster = register(createClusterBuilder().withAuthProvider(authProvider).build());
6565
cluster.connect();
6666
verify(authProvider, atLeastOnce())
6767
.newAuthenticator(
@@ -116,10 +116,11 @@ public void should_fail_to_connect_without_credentials() {
116116
@CCMConfig(dirtiesContext = true)
117117
public void should_connect_with_slow_server() {
118118
Cluster cluster =
119-
createClusterBuilder()
120-
.withAuthProvider(new SlowAuthProvider())
121-
.withPoolingOptions(new PoolingOptions().setHeartbeatIntervalSeconds(1))
122-
.build();
119+
register(
120+
createClusterBuilder()
121+
.withAuthProvider(new SlowAuthProvider())
122+
.withPoolingOptions(new PoolingOptions().setHeartbeatIntervalSeconds(1))
123+
.build());
123124
cluster.connect();
124125
}
125126

driver-core/src/test/java/com/datastax/driver/core/PreparedStatementInvalidationTest.java

Lines changed: 143 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,17 @@
3737

3838
import com.datastax.driver.core.exceptions.NoHostAvailableException;
3939
import com.datastax.driver.core.utils.CassandraVersion;
40+
import com.datastax.driver.core.utils.ScyllaVersion;
41+
import java.util.Objects;
4042
import org.testng.annotations.AfterMethod;
4143
import org.testng.annotations.BeforeMethod;
4244
import org.testng.annotations.DataProvider;
4345
import org.testng.annotations.Test;
4446

4547
public class PreparedStatementInvalidationTest extends CCMTestsSupport {
4648

49+
private final VersionNumber SCYLLA_METADATA_ID_SUPPORT_VERSION = VersionNumber.parse("2025.3");
50+
4751
@BeforeMethod(groups = "short", alwaysRun = true)
4852
public void setup() throws Exception {
4953
execute("CREATE TABLE prepared_statement_invalidation_test (a int PRIMARY KEY, b int, c int);");
@@ -59,39 +63,64 @@ public void teardown() throws Exception {
5963
}
6064

6165
@CassandraVersion("4.0")
66+
@ScyllaVersion()
6267
@Test(groups = "short")
6368
public void should_update_statement_id_when_metadata_changed_across_executions() {
6469
// given
70+
boolean supportsUseMetadataId = isSupportsUseMetadataId();
6571
PreparedStatement ps =
6672
session().prepare("SELECT * FROM prepared_statement_invalidation_test WHERE a = ?");
6773
MD5Digest idBefore = ps.getPreparedId().resultSetMetadata.id;
74+
6875
// when
6976
session().execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
7077
BoundStatement bs = ps.bind(1);
7178
ResultSet rows = session().execute(bs);
79+
7280
// then
73-
MD5Digest idAfter = ps.getPreparedId().resultSetMetadata.id;
74-
assertThat(idBefore).isNotEqualTo(idAfter);
75-
assertThat(ps.getPreparedId().resultSetMetadata.variables)
76-
.hasSize(4)
77-
.containsVariable("d", DataType.cint());
78-
assertThat(bs.preparedStatement().getPreparedId().resultSetMetadata.variables)
79-
.hasSize(4)
80-
.containsVariable("d", DataType.cint());
81+
if (supportsUseMetadataId) {
82+
MD5Digest idAfter = ps.getPreparedId().resultSetMetadata.id;
83+
assertThat(idBefore).isNotEqualTo(idAfter);
84+
assertThat(ps.getPreparedId().resultSetMetadata.variables)
85+
.hasSize(4)
86+
.containsVariable("d", DataType.cint());
87+
assertThat(bs.preparedStatement().getPreparedId().resultSetMetadata.variables)
88+
.hasSize(4)
89+
.containsVariable("d", DataType.cint());
90+
} else {
91+
assertThat(idBefore).isNull(); // Scylla does not support CQL5 extensions and metadata id
92+
MD5Digest idAfter = ps.getPreparedId().resultSetMetadata.id;
93+
assertThat(idAfter).isNull();
94+
assertThat(ps.getPreparedId().resultSetMetadata.variables)
95+
.hasSize(3)
96+
.doesNotContainVariable("d");
97+
assertThat(bs.preparedStatement().getPreparedId().resultSetMetadata.variables)
98+
.hasSize(3)
99+
.doesNotContainVariable("d");
100+
}
81101
assertThat(rows.getColumnDefinitions()).hasSize(4).containsVariable("d", DataType.cint());
82102
}
83103

84104
@CassandraVersion("4.0")
105+
@ScyllaVersion()
85106
@Test(groups = "short")
86-
public void should_update_statement_id_when_metadata_changed_across_pages() throws Exception {
107+
public void should_update_statement_id_when_metadata_changed_across_pages() {
87108
// given
109+
boolean supportsUseMetadataId = isSupportsUseMetadataId();
88110
PreparedStatement ps = session().prepare("SELECT * FROM prepared_statement_invalidation_test");
89111
ResultSet rows = session().execute(ps.bind().setFetchSize(2));
90112
assertThat(rows.isFullyFetched()).isFalse();
91113
MD5Digest idBefore = ps.getPreparedId().resultSetMetadata.id;
114+
if (!supportsUseMetadataId) {
115+
assertThat(idBefore).isNull();
116+
}
92117
ColumnDefinitions definitionsBefore = rows.getColumnDefinitions();
93118
assertThat(definitionsBefore).hasSize(3).doesNotContainVariable("d");
94-
// consume the first page
119+
120+
// when
121+
session().execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
122+
123+
// consume the first page only after altering table to prevent early second page prefetching
95124
int remaining = rows.getAvailableWithoutFetching();
96125
while (remaining-- > 0) {
97126
try {
@@ -102,24 +131,27 @@ public void should_update_statement_id_when_metadata_changed_across_pages() thro
102131
}
103132
}
104133

105-
// when
106-
session().execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
107-
108134
// then
109-
// this should trigger a background fetch of the second page, and therefore update the
110-
// definitions
135+
// since `prepareNextRow` (which in turn calls `fetchMoreResults`) is called for each call of
136+
// `rows.one()`, new page will be already fetched at this point
111137
for (Row row : rows) {
112138
assertThat(row.isNull("d")).isTrue();
113139
}
114140
MD5Digest idAfter = ps.getPreparedId().resultSetMetadata.id;
115141
ColumnDefinitions definitionsAfter = rows.getColumnDefinitions();
116-
assertThat(idBefore).isNotEqualTo(idAfter);
142+
if (supportsUseMetadataId) {
143+
assertThat(idBefore).isNotEqualTo(idAfter);
144+
} else {
145+
assertThat(idAfter).isNull();
146+
}
117147
assertThat(definitionsAfter).hasSize(4).containsVariable("d", DataType.cint());
118148
}
119149

120150
@CassandraVersion("4.0")
151+
@ScyllaVersion()
121152
@Test(groups = "short")
122153
public void should_update_statement_id_when_metadata_changed_across_sessions() {
154+
boolean supportsUseMetadataId = isSupportsUseMetadataId();
123155
Session session1 = cluster().connect();
124156
useKeyspace(session1, keyspace);
125157
Session session2 = cluster().connect();
@@ -131,7 +163,15 @@ public void should_update_statement_id_when_metadata_changed_across_sessions() {
131163
session2.prepare("SELECT * FROM prepared_statement_invalidation_test WHERE a = ?");
132164

133165
MD5Digest id1a = ps1.getPreparedId().resultSetMetadata.id;
166+
134167
MD5Digest id2a = ps2.getPreparedId().resultSetMetadata.id;
168+
if (supportsUseMetadataId) {
169+
assertThat(id1a).isNotNull();
170+
assertThat(id2a).isNotNull();
171+
} else {
172+
assertThat(id1a).isNull();
173+
assertThat(id2a).isNull();
174+
}
135175

136176
ResultSet rows1 = session1.execute(ps1.bind(1));
137177
ResultSet rows2 = session2.execute(ps2.bind(1));
@@ -154,16 +194,27 @@ public void should_update_statement_id_when_metadata_changed_across_sessions() {
154194

155195
MD5Digest id1b = ps1.getPreparedId().resultSetMetadata.id;
156196
MD5Digest id2b = ps2.getPreparedId().resultSetMetadata.id;
157-
158-
assertThat(id1a).isNotEqualTo(id1b);
159-
assertThat(id2a).isNotEqualTo(id2b);
160-
161-
assertThat(ps1.getPreparedId().resultSetMetadata.variables)
162-
.hasSize(4)
163-
.containsVariable("d", DataType.cint());
164-
assertThat(ps2.getPreparedId().resultSetMetadata.variables)
165-
.hasSize(4)
166-
.containsVariable("d", DataType.cint());
197+
if (supportsUseMetadataId) {
198+
assertThat(id1a).isNotEqualTo(id1b);
199+
assertThat(id2a).isNotEqualTo(id2b);
200+
201+
assertThat(ps1.getPreparedId().resultSetMetadata.variables)
202+
.hasSize(4)
203+
.containsVariable("d", DataType.cint());
204+
assertThat(ps2.getPreparedId().resultSetMetadata.variables)
205+
.hasSize(4)
206+
.containsVariable("d", DataType.cint());
207+
} else {
208+
assertThat(id1b).isNull();
209+
assertThat(id2b).isNull();
210+
211+
assertThat(ps1.getPreparedId().resultSetMetadata.variables)
212+
.hasSize(3)
213+
.doesNotContainVariable("d");
214+
assertThat(ps2.getPreparedId().resultSetMetadata.variables)
215+
.hasSize(3)
216+
.doesNotContainVariable("d");
217+
}
167218
assertThat(rows1.getColumnDefinitions()).hasSize(4).containsVariable("d", DataType.cint());
168219
assertThat(rows2.getColumnDefinitions()).hasSize(4).containsVariable("d", DataType.cint());
169220
}
@@ -182,21 +233,42 @@ public void should_not_reprepare_invalid_statements() {
182233
}
183234

184235
@CassandraVersion("4.0")
185-
@Test(groups = "short")
236+
@ScyllaVersion()
237+
@Test()
186238
public void should_never_update_statement_id_for_conditional_updates_in_modern_protocol() {
187239
should_never_update_statement_id_for_conditional_updates(session());
188240
}
189241

242+
@CassandraVersion("4.0")
243+
@ScyllaVersion()
244+
@Test()
245+
public void should_never_update_statement_for_conditional_updates_in_legacy_protocols() {
246+
Cluster cluster =
247+
register(
248+
Cluster.builder()
249+
.addContactPoints(getContactPoints())
250+
.withPort(ccm().getBinaryPort())
251+
.withProtocolVersion(ccm().getProtocolVersion(V4))
252+
.build());
253+
Session session = cluster.connect(keyspace);
254+
should_never_update_statement_id_for_conditional_updates(session);
255+
}
256+
190257
private void should_never_update_statement_id_for_conditional_updates(Session session) {
191258
// Given
259+
boolean isScylla = Objects.nonNull(ccm().getScyllaVersion());
260+
boolean supportsUseMetadataId = isSupportsUseMetadataId();
192261
PreparedStatement ps =
193262
session.prepare(
194263
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (?, ?, ?) IF NOT EXISTS");
195264

196265
// Never store metadata in the prepared statement for conditional updates, since the result set
197-
// can change
198-
// depending on the outcome.
199-
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
266+
// can change depending on the outcome.
267+
if (isScylla) {
268+
assertThat(ps.getPreparedId().resultSetMetadata.variables).hasSize(4);
269+
} else {
270+
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
271+
}
200272
MD5Digest idBefore = ps.getPreparedId().resultSetMetadata.id;
201273

202274
// When
@@ -205,11 +277,21 @@ private void should_never_update_statement_id_for_conditional_updates(Session se
205277
// Then
206278
// Successful conditional update => only contains the [applied] column
207279
assertThat(rs.wasApplied()).isTrue();
208-
assertThat(rs.getColumnDefinitions())
209-
.hasSize(1)
210-
.containsVariable("[applied]", DataType.cboolean());
280+
if (isScylla) {
281+
assertThat(rs.getColumnDefinitions())
282+
.hasSize(4)
283+
.containsVariable("[applied]", DataType.cboolean());
284+
} else {
285+
assertThat(rs.getColumnDefinitions())
286+
.hasSize(1)
287+
.containsVariable("[applied]", DataType.cboolean());
288+
}
211289
// However the prepared statement shouldn't have changed
212-
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
290+
if (isScylla) {
291+
assertThat(ps.getPreparedId().resultSetMetadata.variables).hasSize(4);
292+
} else {
293+
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
294+
}
213295
assertThat(ps.getPreparedId().resultSetMetadata.id).isEqualTo(idBefore);
214296

215297
// When
@@ -225,7 +307,11 @@ private void should_never_update_statement_id_for_conditional_updates(Session se
225307
assertThat(row.getInt("b")).isEqualTo(5);
226308
assertThat(row.getInt("c")).isEqualTo(5);
227309
// The prepared statement still shouldn't have changed
228-
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
310+
if (isScylla) {
311+
assertThat(ps.getPreparedId().resultSetMetadata.variables).hasSize(4);
312+
} else {
313+
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
314+
}
229315
assertThat(ps.getPreparedId().resultSetMetadata.id).isEqualTo(idBefore);
230316

231317
// When
@@ -235,30 +321,28 @@ private void should_never_update_statement_id_for_conditional_updates(Session se
235321
// Then
236322
// Failed conditional update => regular metadata that should also contain the new column
237323
assertThat(rs.wasApplied()).isFalse();
238-
assertThat(rs.getColumnDefinitions()).hasSize(5);
324+
if (isScylla && !supportsUseMetadataId) {
325+
assertThat(rs.getColumnDefinitions()).hasSize(4);
326+
} else {
327+
assertThat(rs.getColumnDefinitions()).hasSize(5);
328+
}
239329
row = rs.one();
240330
assertThat(row.getBool("[applied]")).isFalse();
241331
assertThat(row.getInt("a")).isEqualTo(5);
242332
assertThat(row.getInt("b")).isEqualTo(5);
243333
assertThat(row.getInt("c")).isEqualTo(5);
244-
assertThat(row.isNull("d")).isTrue();
245-
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
246-
assertThat(ps.getPreparedId().resultSetMetadata.id).isEqualTo(idBefore);
247-
}
248-
249-
@CassandraVersion("4.0")
250-
@Test(groups = "short")
251-
public void should_never_update_statement_for_conditional_updates_in_legacy_protocols() {
252-
// Given
253-
Cluster cluster =
254-
register(
255-
Cluster.builder()
256-
.addContactPoints(getContactPoints())
257-
.withPort(ccm().getBinaryPort())
258-
.withProtocolVersion(ccm().getProtocolVersion(V4))
259-
.build());
260-
Session session = cluster.connect(keyspace);
261-
should_never_update_statement_id_for_conditional_updates(session);
334+
if (!isScylla) {
335+
assertThat(row.isNull("d")).isTrue();
336+
assertThat(ps.getPreparedId().resultSetMetadata.variables).isNull();
337+
assertThat(ps.getPreparedId().resultSetMetadata.id).isEqualTo(idBefore);
338+
} else if (supportsUseMetadataId) {
339+
assertThat(row.isNull("d")).isTrue();
340+
assertThat(ps.getPreparedId().resultSetMetadata.variables).hasSize(5);
341+
assertThat(ps.getPreparedId().resultSetMetadata.id).isNotEqualTo(idBefore);
342+
} else {
343+
assertThat(row.getColumnDefinitions()).doesNotContainVariable("d");
344+
assertThat(ps.getPreparedId().resultSetMetadata.variables).hasSize(4);
345+
}
262346
}
263347

264348
@DataProvider(name = "resolverName")
@@ -402,4 +486,9 @@ private Session sessionWithSkipCQL4MetadataResolveMethod(
402486
session.execute("USE cql4_loopholes_test");
403487
return session;
404488
}
489+
490+
private boolean isSupportsUseMetadataId() {
491+
return Objects.isNull(ccm().getScyllaVersion())
492+
|| ccm().getScyllaVersion().compareTo(SCYLLA_METADATA_ID_SUPPORT_VERSION) > 0;
493+
}
405494
}

0 commit comments

Comments
 (0)