Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

DATACASS-529 - Allow slice queries using reactive repositories. #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hleb-albau wants to merge 1 commit into spring-projects:2.0.x from hleb-albau:2.0.x
Closed

Conversation

@hleb-albau
Copy link
Contributor

@hleb-albau hleb-albau commented May 14, 2018
edited
Loading

Reactive repositories now support methods returning Mono<Slice<T>>.

Mono<Slice<User>> findByLastname(String firstname, Pageable pageable);

Related ticket: DATACASS-529.

arturalbov, lukasz-gosiewski, and eximius313 reacted with thumbs up emoji
@hleb-albau hleb-albau changed the title (削除) 2.0.x (削除ここまで) (追記) DATACASS-529 - Allow slice queries using reactive repositories. (追記ここまで) May 14, 2018
Copy link
Member

mp911de commented May 19, 2018

Thanks a lot, we'll take a look as soon as we have time.

hleb-albau reacted with hooray emoji

Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few comments to this pull request. Do you want to address these issues from your side or should we take your pull request from here and apply the changes ourselves?

* @param rowMapper must not be {@literal null}.
* @return the resulting {@link Slice}.
*/
static <T> Mono<Slice<T>> readSlice(ReactiveResultSet resultSet, Integer fetchSize, Function<Row, T> rowMapper) {
Copy link
Member

@mp911de mp911de Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't introduce dependencies to reactive types here as we want the module to run without reactive dependencies as well.

We also need to be careful about fetching/reading progress. If the fetch size differs from the actual number of rows returned for a page, we might request the next page without intending to do so. I think we need another method on ReactiveResultSet that gives us a Flux without transparent next page fetching.

*
* @param statement the CQL statement, must not be {@literal null}.
* @param entityClass The entity type must not be {@literal null}.
* @return the result object returned by the action or {@link Mono#empty()}
Copy link
Member

@mp911de mp911de Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, we're mixing paradigms. We should return an empty Slice (see Page.empty()) as the query terminates without rows and the query result is the container for rows, not the rows themselves.

Copy link
Contributor Author

Hi @mp911de,
I have no free time in near future, so if you can finish this task, it would be nice. Thanks

Copy link
Member

mp911de commented Jun 7, 2018

Cool, thanks for your timely update. We'll take care of this one.

Copy link

@mp911de do you have any plans when you could handle this?

lukasz-gosiewski reacted with thumbs up emoji

Copy link
Member

mp911de commented Jun 12, 2018

This pull request needs some polishing before we can merge it. We intend to ship the change with the next release candidate.

Copy link

So 2.1.0.RC1?

Copy link

@mp911de
That's some nice news, but there's one problem with this approach. CassandraPageRequest returned by Slice object has no option of serialization. Therefore we cannot use it easily in web layer. Underlying PagingState, however, is easily serializable with toString method. Shouldn't CassandraPageRequst expose this functionality as well?

Copy link
Member

mp911de commented Jun 12, 2018

@lukasz-gosiewski Please file a new ticket in Spring Data Cassandra. We'll see what we can do for you.

lukasz-gosiewski reacted with thumbs up emoji

mp911de pushed a commit that referenced this pull request Jun 18, 2018
We now support Slice queries using reactive Cassandra repositories. Repository query methods can declare Mono<Slice<T>> as their return type to query for slices without applying transparent paging. The resulting Mono always completes with a value. An empty query result returns a Mono emitting an empty Slice.
interface UserRepository extends CrudRepository<User, String> {
 Mono<Slice<User>> findByLastname(String firstname, Pageable pageable);
}
Mono<Slice<User>> result =repository.findByLastname("White", CassandraPageRequest.first(1));
Original pull request: #128.
mp911de added a commit that referenced this pull request Jun 18, 2018
Introduce ReactiveResultSet.availableRows() to fetch rows without transparent paging. Refactor QueryUtils to extract a Slice from an Iterable. Add slice(Query, Class) to ReactiveCassandraOperations to expose a consistent API. Convert space indentation to tab indentation. Add tests. Add since tags. Add documentation. Reformat code.
Original pull request: #128.
Copy link
Member

mp911de commented Jun 18, 2018

That's merged and polished now. Thanks a lot!

hleb-albau, eximius313, and lukasz-gosiewski reacted with hooray emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mp911de mp911de mp911de left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /