-
Notifications
You must be signed in to change notification settings - Fork 3k
Vertx CDI integration - introduce ConsumeEvent annotation #753
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
Conversation
It might open us up to unintended typos, but what about |
I'm afraid this would not help much. We should try to avoid problems like CDI |
I would use For the rest, it looks very good! |
Well, I can see the users asking why it does not work and then realizing they use |
You are right! Forgot about
...naming is hard.... |
@ConsumeEvent("foo.address")
String send(String message) {
return message.toUpperCase();
} |
+1 for |
Thinking loudly... A few years back (in 2005), I've read a paper about using the class name to identify event recipient. Obviously, it was making the assumption that consumer would be "unique". Now let's imagine a |
Like this? package foo;
class SimpleBean {
@ConsumeMessage // -> EventBus.consumer("foo.SimpleBean").handler(m -> m.reply(m.body().toUpperCase()))
String send(String message) { return message.toUpperCase(); }
} I think this would work for any scope. |
Exactly. |
@cescoffier FQCN as the default address should work now. |
- resolves #757
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments inline.
extensions/vertx/runtime/src/main/java/org/jboss/shamrock/vertx/runtime/VertxProducer.java
Outdated
Show resolved
Hide resolved
extensions/vertx/runtime/src/main/java/org/jboss/shamrock/vertx/runtime/VertxProducer.java
Show resolved
Hide resolved
extensions/vertx/runtime/src/main/java/org/jboss/shamrock/vertx/runtime/VertxProducer.java
Show resolved
Hide resolved
extensions/vertx/runtime/src/main/java/org/jboss/shamrock/vertx/runtime/VertxProducer.java
Show resolved
Hide resolved
extensions/vertx/runtime/src/main/java/org/jboss/shamrock/vertx/runtime/VertxProducer.java
Show resolved
Hide resolved
...sions/vertx/deployment/src/test/java/org/jboss/shamrock/vertx/MessageConsumerMethodTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good!
Fixes quarkusio#753 Signed-off-by: Jeff MAURY <[email protected]>
@cescoffier @FroMage This is what I have so far. Let me know what you think about the API...
WRT the annotation name - we cannot use
@Consumes
because of JAX-RS. I don't like@Address
. And I find@ConsumeMessage
quite self-explanatory.