Skip to content

Commit 0ec97e3

Browse files
committed
Fix prepared statement caching using observability.
We now no longer create a statement proxy when preparing CQL statements as prepared statements do not use RequestTracker for completion callbacks. Also, ObservationStatement now implements equals and hashCode to report equality for its underlying statement in case the statement was used as cache key. Previously, we created a proxy without implementing equals and hashCode resulting in re-preparation as the prepared statement cache kept growing because the input statement did not provide means to serve as cache key. Closes #1601
1 parent 3570e6a commit 0ec97e3

File tree

6 files changed

+111
-14
lines changed

6 files changed

+111
-14
lines changed

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/observability/CqlSessionObservationInterceptor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ final class CqlSessionObservationInterceptor implements MethodInterceptor {
9393
Observation observation = startObservation(statement, true, "prepare");
9494

9595
try {
96-
return this.delegate.prepare((SimpleStatement) ObservationStatement.createProxy(observation, statement));
96+
return this.delegate.prepare((SimpleStatement) statement);
9797
} catch (RuntimeException e) {
9898

9999
observation.error(e);
@@ -113,7 +113,7 @@ final class CqlSessionObservationInterceptor implements MethodInterceptor {
113113

114114
Observation observation = startObservation(statement, true, "prepareAsync");
115115

116-
return this.delegate.prepareAsync((SimpleStatement) ObservationStatement.createProxy(observation, statement))
116+
return this.delegate.prepareAsync((SimpleStatement) statement)
117117
.whenComplete((preparedStatement, throwable) -> {
118118

119119
if (throwable != null) {

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/observability/ObservableReactiveSession.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public Mono<PreparedStatement> prepare(SimpleStatement statement) {
167167
return Mono.deferContextual(contextView -> {
168168

169169
Observation observation = startObservation(getParentObservation(contextView), statement, true, "prepare");
170-
return this.delegate.prepare(ObservationStatement.createProxy(observation, statement)) //
170+
return this.delegate.prepare(statement) //
171171
.doOnError(observation::error) //
172172
.doFinally(ignore -> observation.stop());
173173
});

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/observability/ObservationStatement.java

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@
1919

2020
import java.lang.reflect.Method;
2121

22-
import javax.annotation.Nonnull;
23-
2422
import org.aopalliance.intercept.MethodInterceptor;
2523
import org.aopalliance.intercept.MethodInvocation;
2624
import org.jspecify.annotations.Nullable;
25+
2726
import org.springframework.aop.framework.ProxyFactory;
2827
import org.springframework.util.ClassUtils;
28+
import org.springframework.util.ObjectUtils;
2929

3030
import com.datastax.oss.driver.api.core.cql.Statement;
3131

@@ -76,23 +76,51 @@ public static boolean isObservationStatement(Statement<?> statement) {
7676
}
7777

7878
@Override
79-
public @Nullable Object invoke(@Nonnull MethodInvocation invocation) throws Throwable {
79+
public @Nullable Object invoke(MethodInvocation invocation) throws Throwable {
8080

8181
Method method = invocation.getMethod();
82-
83-
if (method.getName().equals("getTargetClass")) {
84-
return this.delegate.getClass();
85-
}
86-
87-
if (method.getName().equals("getObservation")) {
88-
return this.observation;
82+
@Nullable
83+
Object[] args = invocation.getArguments();
84+
85+
String name = method.getName();
86+
87+
switch (name) {
88+
case "equals" -> {
89+
if (args.length == 1) {
90+
return equals(args[0]);
91+
}
92+
}
93+
case "hashCode" -> {
94+
return hashCode();
95+
}
96+
97+
case "getTargetClass" -> {
98+
return this.delegate.getClass();
99+
}
100+
101+
case "getObservation" -> {
102+
return this.observation;
103+
}
89104
}
90105

91106
Object result = invocation.proceed();
92107
if (result instanceof Statement<?>) {
93108
this.delegate = (Statement<?>) result;
94109
}
110+
95111
return result;
96112
}
97113

114+
@Override
115+
public boolean equals(Object o) {
116+
if (!(o instanceof ObservationStatement that)) {
117+
return false;
118+
}
119+
return ObjectUtils.nullSafeEquals(delegate, that.delegate);
120+
}
121+
122+
@Override
123+
public int hashCode() {
124+
return ObjectUtils.nullSafeHash(delegate);
125+
}
98126
}

spring-data-cassandra/src/test/java/org/springframework/data/cassandra/observability/ImperativeIntegrationTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.springframework.test.context.junit.jupiter.SpringExtension;
3232

3333
import com.datastax.oss.driver.api.core.CqlSession;
34+
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
3435

3536
/**
3637
* Collection of tests that log metrics and tracing.
@@ -74,6 +75,13 @@ public SampleTestRunnerConsumer yourCode() {
7475

7576
CqlTemplate template = new CqlTemplate(observableSession);
7677

78+
PreparedStatement prepare1 = observableSession
79+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);");
80+
PreparedStatement prepare2 = observableSession
81+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);");
82+
83+
assertThat(prepare1).isSameAs(prepare2);
84+
7785
template.execute("INSERT INTO person (id,firstName,lastName) VALUES(?,?,?)", 1, "Walter", "White");
7886

7987
assertThat(tracer.getFinishedSpans()).hasSizeGreaterThanOrEqualTo(5);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.cassandra.observability;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import io.micrometer.observation.Observation;
21+
import io.micrometer.observation.tck.TestObservationRegistry;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
import com.datastax.oss.driver.api.core.cql.SimpleStatement;
26+
27+
/**
28+
* Unit test for {@link ObservationStatement}
29+
*
30+
* @author Mark Paluch
31+
*/
32+
class ObservationStatementUnitTests {
33+
34+
@Test // GH-1601
35+
void equalsAndHashCodeShouldBeEqual() {
36+
37+
TestObservationRegistry registry = TestObservationRegistry.create();
38+
39+
Observation observation1 = Observation.start("foo", registry);
40+
Observation observation2 = Observation.start("bar", registry);
41+
42+
SimpleStatement statement1 = ObservationStatement.createProxy(observation1,
43+
SimpleStatement.newInstance("SELECT * FROM foo"));
44+
SimpleStatement statement2 = ObservationStatement.createProxy(observation2,
45+
SimpleStatement.newInstance("SELECT * FROM foo"));
46+
SimpleStatement statement3 = ObservationStatement.createProxy(observation2,
47+
SimpleStatement.newInstance("SELECT * FROM bar"));
48+
49+
assertThat(statement1).isEqualTo(statement2).hasSameHashCodeAs(statement2);
50+
assertThat(statement1).isNotEqualTo(statement3);
51+
assertThat(statement1.hashCode()).isNotEqualTo(statement3.hashCode());
52+
}
53+
}

spring-data-cassandra/src/test/java/org/springframework/data/cassandra/observability/ReactiveIntegrationTests.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import org.springframework.test.context.ContextConfiguration;
3737
import org.springframework.test.context.junit.jupiter.SpringExtension;
3838

39+
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
40+
3941
/**
4042
* Collection of tests that log metrics and tracing.
4143
*
@@ -68,7 +70,6 @@ public SampleTestRunnerConsumer yourCode() {
6870

6971
Observation intermediate = Observation.start("intermediate", createObservationRegistry());
7072

71-
7273
Mono<ReactiveResultSet> drop = observableSession.execute("DROP KEYSPACE IF EXISTS ObservationTest");
7374
Mono<ReactiveResultSet> create = observableSession.execute("CREATE KEYSPACE ObservationTest " + "WITH "
7475
+ "REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };");
@@ -87,6 +88,13 @@ public SampleTestRunnerConsumer yourCode() {
8788
.verifyComplete();
8889
});
8990

91+
PreparedStatement prepare1 = observableSession
92+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);").block();
93+
PreparedStatement prepare2 = observableSession
94+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);").block();
95+
96+
assertThat(prepare1).isSameAs(prepare2);
97+
9098
assertThat(tracer.getFinishedSpans()).hasSizeGreaterThanOrEqualTo(5);
9199
};
92100
}

0 commit comments

Comments
 (0)