Skip to content

Commit 59d8df9

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 1c51f28 commit 59d8df9

File tree

6 files changed

+108
-15
lines changed

6 files changed

+108
-15
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
@@ -91,7 +91,7 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
9191
Observation observation = startObservation(statement, true, "prepare");
9292

9393
try {
94-
return this.delegate.prepare((SimpleStatement) ObservationStatement.createProxy(observation, statement));
94+
return this.delegate.prepare((SimpleStatement) statement);
9595
} catch (RuntimeException e) {
9696

9797
observation.error(e);
@@ -111,7 +111,7 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
111111

112112
Observation observation = startObservation(statement, true, "prepareAsync");
113113

114-
return this.delegate.prepareAsync((SimpleStatement) ObservationStatement.createProxy(observation, statement))
114+
return this.delegate.prepareAsync((SimpleStatement) statement)
115115
.whenComplete((preparedStatement, throwable) -> {
116116

117117
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
@@ -166,7 +166,7 @@ public Mono<PreparedStatement> prepare(SimpleStatement statement) {
166166
return Mono.deferContextual(contextView -> {
167167

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

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

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@
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.springframework.aop.framework.ProxyFactory;
2725
import org.springframework.lang.Nullable;
2826
import org.springframework.util.ClassUtils;
27+
import org.springframework.util.ObjectUtils;
2928

3029
import com.datastax.oss.driver.api.core.cql.Statement;
3130

@@ -77,23 +76,50 @@ public static boolean isObservationStatement(Statement<?> statement) {
7776

7877
@Nullable
7978
@Override
80-
public Object invoke(@Nonnull MethodInvocation invocation) throws Throwable {
79+
public Object invoke(MethodInvocation invocation) throws Throwable {
8180

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

92105
Object result = invocation.proceed();
93106
if (result instanceof Statement<?>) {
94107
this.delegate = (Statement<?>) result;
95108
}
109+
96110
return result;
97111
}
98112

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

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
System.out.println(((SimpleMeterRegistry) meterRegistry).getMetersAsString());
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: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.springframework.test.context.ContextConfiguration;
3838
import org.springframework.test.context.junit.jupiter.SpringExtension;
3939

40+
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
41+
4042
/**
4143
* Collection of tests that log metrics and tracing.
4244
*
@@ -69,7 +71,6 @@ public SampleTestRunnerConsumer yourCode() {
6971

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

72-
7374
Mono<ReactiveResultSet> drop = observableSession.execute("DROP KEYSPACE IF EXISTS ObservationTest");
7475
Mono<ReactiveResultSet> create = observableSession.execute("CREATE KEYSPACE ObservationTest " + "WITH "
7576
+ "REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };");
@@ -88,7 +89,12 @@ public SampleTestRunnerConsumer yourCode() {
8889
.verifyComplete();
8990
});
9091

91-
System.out.println(((SimpleMeterRegistry) meterRegistry).getMetersAsString());
92+
PreparedStatement prepare1 = observableSession
93+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);").block();
94+
PreparedStatement prepare2 = observableSession
95+
.prepare("INSERT INTO person (id, firstName, lastName) VALUES (?, ?, ?);").block();
96+
97+
assertThat(prepare1).isSameAs(prepare2);
9298

9399
assertThat(tracer.getFinishedSpans()).hasSizeGreaterThanOrEqualTo(5);
94100
};

0 commit comments

Comments
 (0)