-
Notifications
You must be signed in to change notification settings - Fork 47
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
Making message properties accessible while handling/consuming the message #69
base: master
Are you sure you want to change the base?
Making message properties accessible while handling/consuming the message #69
Conversation
f63e3c2
to
5f4b972
Compare
5f4b972
to
da06344
Compare
src/main/java/io/appform/dropwizard/actors/actor/metadata/MessageMetadataProvider.java
Outdated
Show resolved
Hide resolved
da06344
to
62e52af
Compare
src/main/java/io/appform/dropwizard/actors/base/utils/MessageMetaUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/io/appform/dropwizard/actors/base/utils/MessageMetaUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/io/appform/dropwizard/actors/base/utils/MessageMetaUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/io/appform/dropwizard/actors/actor/metadata/MessageDelayMetaGenerator.java
Outdated
Show resolved
Hide resolved
src/main/java/io/appform/dropwizard/actors/actor/metadata/MessageDelayMetaGenerator.java
Outdated
Show resolved
Hide resolved
src/test/java/io/appform/dropwizard/actors/actor/metadata/MessageMetadataProviderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/io/appform/dropwizard/actors/actor/metadata/MessageMetadataProviderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/io/appform/dropwizard/actors/actor/metadata/MessageMetadataProviderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/io/appform/dropwizard/actors/base/utils/MessageMetaUtilsTest.java
Outdated
Show resolved
Hide resolved
...st/java/io/appform/dropwizard/actors/connectivity/actor/MessageHeadersAccessibilityTest.java
Outdated
Show resolved
Hide resolved
bd33791
to
ebf0c38
Compare
src/main/java/io/appform/dropwizard/actors/actor/metadata/MessageMetadataProvider.java
Show resolved
Hide resolved
@@ -59,14 +59,17 @@ public UnmanagedConsumer(final String name, | |||
this.queueName = NamingUtils.queueName(config.getPrefix(), name); | |||
this.retryStrategy = retryStrategyFactory.create(config.getRetryConfig()); | |||
this.exceptionHandler = exceptionHandlingFactory.create(config.getExceptionHandlerConfig()); | |||
this.messageMetadataProvider = new MessageMetadataProvider(getMessageMetaGeneratorClasses(config)); |
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.
This makes testing difficult. Do not create objects using constructors in other constructors if possible
void setUp() { | ||
MockitoAnnotations.openMocks(this); | ||
|
||
List<String> messageMetaGeneratorClasses = Arrays.asList(CustomMessageMetaHeaderGenerator.class.getName()); |
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.
val or remove variable
|
||
@Test | ||
void shouldCreateBlankMessageMetadataWhenMessageMetaGeneratorClassesIsNull() { | ||
MessageMetadataProvider messageMetadataProvider = new MessageMetadataProvider(null); |
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.
val
I do not like this naming of classes in config. If we ever refactor code, every single config will break in existing clients. If a discovery is needed, it is better to create an annotation like
Pros:
Implementer Note
|
@r0goyal this needs more reviews |
The existing implementation does provide a way of pushing meta-info as part of headers via message properties, but there is no support of accessing these headers while consuming and processing the message in the application's actor.
Moreover, it is not just headers, several useful meta-info like correlation Id, appId, clusterId, etc which are present in AMQP.BasicProperties are possible to publish with the message but can't be leveraged during processing the message.
This PR fixes the same thing and exposes message properties to the actor's handle method via MessageMetadata.
Sample Usage:
With this change, devs are open to create their custom metadata generators and use them as per need.