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.

Sign up for GitHub

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

Jump to bottom

Use Mutiny event-based API in the Hibernate Reactive quickstart #612

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Use Mutiny event-based API in the Hibernate Reactive quickstart #612

merged 1 commit into from
Jul 15, 2020

Conversation

DavideD
Copy link
Contributor

@DavideD DavideD commented Jul 2, 2020 •
edited
Loading

WARNING: It requires this PR in Quarkus: quarkusio/quarkus#10433

Update the Hibernate Reactive quickstart so that it uses the Mutiny event-based API.
It also inject the session directly making the code simpler.

@Sanne, @gavinking Would you like to have a look?

@gsmet gsmet changed the title Use Mutiny event-based API in the Hibernate Reactive quickstart [Don't merge yet] Use Mutiny event-based API in the Hibernate Reactive quickstart Jul 2, 2020
gsmet
gsmet previously requested changes Jul 2, 2020
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Please don't merge yet as I need the master branch of the quickstarts to stay compatible with 1.6 until next week.

@DavideD DavideD force-pushed the quickstart-refact branch 2 times, most recently from e47d85d to ce83683 Compare July 3, 2020 08:50
@DavideD
Copy link
Contributor Author

DavideD commented Jul 3, 2020

I pushed force some changes:

  • get returns Uni<List<Fruit>> instead of Uni<Fruit[]>
  • Refactoring the methods using IfNotNull, I think it's more readable now
  • Added the exception mapping
  • Additional test cases

I think we should also update the ORM quickstart and return a List instead of an array when we do a find: https://github.com/quarkusio/quarkus-quickstarts/blob/development/hibernate-orm-quickstart/src/main/java/org/acme/hibernate/orm/FruitResource.java#L37

I can do it but do you want me to create a new issue or I can do it in this one?

Thanls

@DavideD DavideD force-pushed the quickstart-refact branch from ce83683 to a2b2c72 Compare July 3, 2020 09:02
@Sanne
Copy link
Member

Sanne commented Jul 3, 2020

very nice @DavideD ! Let's wait for @gsmet 's green light.

@Sanne
Copy link
Member

Sanne commented Jul 3, 2020

I think we should also update the ORM quickstart and return a List instead of an array when we do a find: https://github.com/quarkusio/quarkus-quickstarts/blob/development/hibernate-orm-quickstart/src/main/java/org/acme/hibernate/orm/FruitResource.java#L37

I can do it but do you want me to create a new issue or I can do it in this one?

No need to create an issue, but send a separate PR?

@DavideD
Copy link
Contributor Author

DavideD commented Jul 3, 2020 •
edited
Loading

No need to create an issue, but send a separate PR?

#613

@DavideD
Copy link
Contributor Author

DavideD commented Jul 9, 2020

Updated the return value of the getAll and rebased

@DavideD DavideD force-pushed the quickstart-refact branch from a2b2c72 to 5e5fa03 Compare July 9, 2020 13:19
@DavideD
Use Mutiny event-based API in the Hibernate Reactive quickstart
f2abc7d 
  * Inject the Session directly, unwrap from Uni
  * Additional tests for update and not found entities
  * Return Multi<Fruit> instead of Uni<Fruit[]>
  * Add example on how to map exceptions in the HTTP response
@DavideD DavideD force-pushed the quickstart-refact branch from 5e5fa03 to f2abc7d Compare July 14, 2020 11:44
@DavideD DavideD mentioned this pull request Jul 14, 2020
Merged
@gsmet gsmet changed the title [Don't merge yet] Use Mutiny event-based API in the Hibernate Reactive quickstart Use Mutiny event-based API in the Hibernate Reactive quickstart Jul 15, 2020
@gsmet gsmet dismissed their stale review July 15, 2020 08:31

1.6 has been branched

@gsmet
Copy link
Member

gsmet commented Jul 15, 2020

@DavideD if this is ready it can be merged. Do you confirm?

@DavideD
Copy link
Contributor Author

DavideD commented Jul 15, 2020

I confirm

@gsmet gsmet merged commit 06ef27c into quarkusio:development Jul 15, 2020
@gsmet
Copy link
Member

gsmet commented Jul 15, 2020

Merged, thanks!

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

@burmanm burmanm burmanm left review comments

@Sanne Sanne Sanne left review comments

@gsmet gsmet gsmet 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.
4 participants
@DavideD @Sanne @gsmet @burmanm