jakartaee / data

Data-API
Apache License 2.0
107 stars 30 forks source link

Be clearer about "reactive" repositories #17

Open keilw opened 2 years ago

keilw commented 2 years ago

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 https://github.com/jakartaee/data/issues/8#issuecomment-1247785287, 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.

otaviojava commented 2 years ago

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

hantsy commented 2 years ago

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 commented 2 years ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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.