Skip to content
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

Be clearer about "reactive" repositories #17

Open
keilw opened this issue Sep 19, 2022 · 6 comments
Open

Be clearer about "reactive" repositories #17

keilw opened this issue Sep 19, 2022 · 6 comments
Labels
design Improvements or additions to design

Comments

@keilw
Copy link
Member

keilw commented Sep 19, 2022

Description

The current ReactiveRepository is meaningless and confusing, because it creates a fake impression of being a "Generic Reactive" repository type which does not work.

RxJavaReactiveStreams highlights, the two different incompatible concepts of these two (RxJava and Reactive Streams), add the JDK Async Flow and you have a possible combination between 3 incompatible types.

java.util.concurrent also says nothing about "reactive", hence calling it along the lines of Micronaut AsyncCrudRepository seems more sensitive.
@graemerocher said in #8 (comment), something like FlowRepository would be OK. If Micronaut's AsyncCrudRepository (working with CompletableFuture) was also worth supporting, then both
AsyncCrudRepository or say FutureCrudRepository might exist side-by-side with a FlowRepository or FlowCrudRepository (if others like pageable variants also apply) while the generic "reactive" term sounds meaningless like a source for buzzword bingo where no common denominator could be found between all the mentioned concurrent/async APIs the JDK provides as Flow and Future are not interpoerable or build on top of each other either.

@keilw keilw changed the title Be clearer about "Reactive" repositories Be clearer about "reactive" repositories Sep 19, 2022
@keilw keilw added the design Improvements or additions to design label Sep 19, 2022
@otaviojava
Copy link
Contributor

Great point @keilw when we talk about this topic, we can work with FlowRepository

@hantsy
Copy link

hantsy commented Oct 22, 2022

Like Spring Data JPA, it allows you to use a @Async and CompletableFuture/CompletionStage together in the generic blocking API, no need extra Repository for them.

Aligned with Java/Jakarta, I would like used Java 9 Flow by default for the ReactiveRepository. SmallRye Mutiny is switched to Java 9 Flow by default, which can be used as backend implementations.

@hantsy
Copy link

hantsy commented Oct 22, 2022

In Spring framework, it used Reactor by default, and used a RactiveAdaterRegistry to register other Reactive Streams lib, and convert between them.

Here we can focus on Java 9 FLow, and other reactive streams, we can also used a similar mechanism to handle it.

@gavinking
Copy link
Contributor

gavinking commented Feb 22, 2024

I would just like to point out that the current revision of the spec, as it exists today already fully supports reactive repositories! (They're just not portable between providers.)

The following code is a perfectly legal and reasonable Jakarta Data repository, though of course the Jakarta Data provider must be written to allow it:

@Repository
public interface ReactiveBooks {
    @Insert
    Uni<Void> persist(Book book);

    @Find
    Uni<Book> getBook(String isbn);

    @Query("from Book where title like :title")
    Uni<List<Book>> booksByTitle(String title)
}

Now, I just noticed that the current javadoc of @Insert and @Find are slightly too prescriptive and could be read to disallow the code above (we should fix that). But the spec itself is written to allow the code above.

@mswatosh
Copy link
Member

The Javadoc for those methods seems pretty restrictive, how are you thinking of modifying it to allow Uni?

    @Query("from Book where title like :title")
    Uni<List<Book>> booksByTitle(String title)

Wouldn't it be preferable to return a Multi<Book> here? That should be possible if we allowed the spec to return a Flow.Publisher from a query, which would also cover Spring's Mono and Flux. Though by using Multi you would lose portability.

Also, would the spec need to clarify that vendor-specific subtypes are allowed, with the understanding that their use would prevent portability?

@gavinking
Copy link
Contributor

how are you thinking of modifying it to allow Uni?

I mean, just by adding weasel-words, for example:

This method must have a single parameter whose type is usually one of the following

or:

This method must have a single parameter whose type should be one of the following

Something like that. Just leaving the door slightly ajar should be enough for now.

Wouldn't it be preferable to return a Multi<Book> here?

Haha, that's what everyone asks the first time the see Hibernate Reactive.

The answer is "no", at least as far as today's relational database protocols go. But I'm not sure about NoSQL protocols. For those Multi might indeed make sense.

Also, would the spec need to clarify that vendor-specific subtypes are allowed, with the understanding that their use would prevent portability?

We could add a note to that effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Improvements or additions to design
Projects
None yet
Development

No branches or pull requests

5 participants