Modular CachedPreparedStatementCreator [DATACASS-403]

[spring-projects-issues]

James Howe opened DATACASS-403 and commented

The driver version 3.1 changed how query idempotence is determined.

Any BuiltStatements from a QueryBuilder will automatically determine their idempotence, but because they must be converted to String in order to use the cache, this information is lost.

If there were an alternate constructor that took a Statement instead of a String, then the idempotence of the statement could be carried over to the prepared and bound statements

---

Reference URL: http://docs.datastax.com/en/drivers/java/3.1/com/datastax/driver/core/Statement.html#isIdempotent--

Issue Links:

• DATACASS-291 CachedPreparedStatementCreator uses a static field for caching

Referenced from: pull request #105

[spring-projects-issues]

Mark Paluch commented

Thanks for your ticket. CachedPreparedStatementCreator is String-query based and we currently don't plan to ship a feature release on the 1.x line. The caching is broken although it works in most cases. It uses an intrinsic cache that is held static final. Please rather use an own caching implementation for now.

It would make sense for the upcoming 2.0 version, to extract a common ground for cache support so you can build an own implementation

[spring-projects-issues]

James Howe commented

That's fine, I can probably just do it with a subclass for now

[spring-projects-issues]

Dmitro Lisnichenko commented

It would be great if there was an option for a @Query annotation or a separate annotation for query method in Repository for marking this method as idempotent.

[spring-projects-issues]

Mark Paluch commented

I pushed a newly designed CachedPreparedStatementCreator to https://github.com/spring-projects/spring-data-cassandra/tree/issue/DATACASS-403.

Care to give it a try? I'd love to get some feedback and what you think about it.

<dependency>
  <groupId>org.springframework.data</groupId>
  <artifactId>spring-data-cassandra</artifactId>
  <version>2.0.0.DATACASS-403-SNAPSHOT</version>
</dependency>

<repository>
  <id>spring-libs-snapshot</id>
  <name>Spring Snapshot Repository</name>
  <url>http://repo.spring.io/libs-snapshot</url>
</repository>

[spring-projects-issues]

James Howe commented

The idea is you inject a PreparedStatementCache bean and use it with CachedPreparedStatementCreator.of?

Compared to the current version, subclasses cannot read the fields.
I have added a PreparedStatementCreator withConsistency(ConsistencyLevel cl), which looks harder to do with this new one.
Can you add protected getters maybe?

[spring-projects-issues]

Mark Paluch commented

Thanks a lot for your feedback. It's up to you, whether you inject PreparedStatementCache or manage the state yourself. Dependency injection would make sense if Cluster/Session are managed within the same scope to align lifetime duration.

Regarding query options, I'd propose to use QueryOptions/WriteOptions that apply the options to the created statement.

Regarding getters, I'm ok with adding a getter for the cache, as the cache might be of interest for outside users. As a subclass, you can intercept the values you're interested in within the constructor.

Does this make sense?

[spring-projects-issues]

James Howe commented

Regarding query options
My error handling reads a lot better with the fluent interface I added to change the consistency, especially as I never override any of the other options.

you can intercept the values you're interested in within the constructor
Indeed, but that's more work for me, and I'd just be duplicating all the fields it already has

[spring-projects-issues]

Mark Paluch commented

How about decorating CachedPreparedStatementCreator with an own PreparedStatementCreator? You can alter the input/output to your needs without interfering with the state of CachedPreparedStatementCreator itself?

We will cache prepared statements using the CQL-text only without regard to statement flags. The first prepared statements flags will apply to all prepared statements retrieved from CachedPreparedStatementCreator. You will be able to pass in QueryOptions when creating CachedPreparedStatementCreator from plain CQL. Decorations lets you post-process the result

[spring-projects-issues]

James Howe commented

I don't want to change what a single creator returns, I want an entirely new creator derived from the first one.
The first design was fine for that, it's just protected field access would have made it a bit simpler

[spring-projects-issues]

Mark Paluch commented

Exposing the internal state violates encapsulation. We don't want to expose these elements any further. Otherwise, we're constrained to additional API we can't change any more and the API isn't related to the exposed contract of a caching PreparedStatementCreator.

What's the reason for sticking to a subclass of CachedPreparedStatementCreator instead of having a class that implements PreparedStatementCreator and delegates calls to CachedPreparedStatementCreator after pre-processing the input parameters?

[spring-projects-issues]

John Blum commented

Reviewed, polished and merged! Thank you!