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

Make ArC intercept default methods #9472

Merged
merged 1 commit into from
May 21, 2020
Merged

Make ArC intercept default methods #9472

merged 1 commit into from
May 21, 2020

Conversation

stuartwdouglas
Copy link
Member

Fixes #7188

@boring-cyborg boring-cyborg bot added area/arc Issue related to ARC (dependency injection) area/hibernate-orm Hibernate ORM labels May 20, 2020
@stuartwdouglas stuartwdouglas requested review from mkouba and FroMage May 20, 2020 01:34
@stuartwdouglas stuartwdouglas mentioned this pull request May 20, 2020
Closed
@mkouba
Copy link
Contributor

mkouba commented May 20, 2020

This should resolve #3533 too.

@mkouba mkouba requested a review from manovotn May 20, 2020 07:55
FroMage
FroMage approved these changes May 20, 2020
View reviewed changes
independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java
@@ -186,6 +186,13 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
classLevelBindings, bytecodeTransformerConsumer, transformUnproxyableClasses));
Copy link
Member

Choose a reason for hiding this comment

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

OK, my bad, I didn't know interceptors also applied to superclass methods, otherwise I'd have suggested to handle interface default methods too. Great move.

@FroMage
Copy link
Member

FroMage commented May 20, 2020

AMQP test failed, but I don't really know if it could be related to this change. @cescoffier are you using ArC interceptors in AMQP? Could they be affected with interface default methods suddenly being intercepted? I'll restart the test to see if it changes.

mkouba
mkouba approved these changes May 20, 2020
View reviewed changes
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

Looks good but I'd like to wait for the review from @manovotn too...

manovotn
manovotn requested changes May 20, 2020
View reviewed changes
Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

Just one minor comment, otherwise looks good.

@stuartwdouglas
Copy link
Member Author

Fixed.

@stuartwdouglas stuartwdouglas requested a review from manovotn May 21, 2020 01:12
@stuartwdouglas stuartwdouglas merged commit 9ef7784 into quarkusio:master May 21, 2020
@stuartwdouglas stuartwdouglas added this to the 1.6.0 milestone May 21, 2020
@gsmet gsmet modified the milestones: 1.6.0 - master, 1.5.0.Final May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FroMage FroMage FroMage approved these changes

@mkouba mkouba mkouba approved these changes

@manovotn manovotn manovotn approved these changes

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) area/hibernate-orm Hibernate ORM
Projects
None yet
Milestone
1.5.0.Final
Development

Successfully merging this pull request may close these issues.

PanacheRepository injected methods cannot be made transactional
5 participants
@stuartwdouglas @mkouba @FroMage @manovotn @gsmet