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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

manojsharma27
Copy link
Contributor

@manojsharma27 manojsharma27 commented Nov 24, 2023

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:

  actors:
    TEST_ACTOR:
      exchange: test-exchg
      delayed: false
      prefix: testService
      concurrency: 1
      prefetchCount: 1
      retryConfig:
        type: COUNT_LIMITED_EXPONENTIAL_BACKOFF
        maxAttempts: 6
        multipier: 50
        maxTimeBetweenRetries: 30s
      messageMetaGeneratorClasses:
        - io.appform.dropwizard.actors.actor.metadata.MessageReDeliveredMetaGenerator
        - io.appform.dropwizard.actors.actor.metadata.MessageDelayMetaGenerator
        - io.appform.dropwizard.actors.actor.metadata.MessageExpiredMetaGenerator

With this change, devs are open to create their custom metadata generators and use them as per need.

@manojsharma27 manojsharma27 force-pushed the expose_message_properties_to_actors branch from 5f4b972 to da06344 Compare December 4, 2023 06:20
@manojsharma27 manojsharma27 force-pushed the expose_message_properties_to_actors branch from da06344 to 62e52af Compare December 4, 2023 06:37
@manojsharma27 manojsharma27 force-pushed the expose_message_properties_to_actors branch from bd33791 to ebf0c38 Compare December 10, 2023 14:53
@@ -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));
Copy link
Owner

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());
Copy link
Owner

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);
Copy link
Owner

Choose a reason for hiding this comment

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

val

@santanusinha
Copy link
Owner

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 @MetaGenerator("REDELIVERY_META_GENERATOR") which can be annotated on the meta generator class. At the initialization of the bundle we can use Reflections to discover all such generators and validate them against what has been provided in config.
Your config also becomes like this:

  actors:
    TEST_ACTOR:
      exchange: test-exchg
      delayed: false
      prefix: testService
      concurrency: 1
      prefetchCount: 1
      retryConfig:
        type: COUNT_LIMITED_EXPONENTIAL_BACKOFF
        maxAttempts: 6
        multipier: 50
        maxTimeBetweenRetries: 30s
      messageMetaGeneratorClasses:
        - REDELIVERY_META_GENERATOR
        - DELAY_META_GENERATOR
        - EXPIRED_META_GENERATOR

Pros:

  • Simplifies config
  • Makes config immune to code refactors later on
    Cons:
  • Potentially slower startup due to classpath scanning (Can be mitigated by taking a reflections object in as param somehere in bundle init, if the client software is already creating one for some reason)

Implementer Note

  • Must throw if two meta-gen classes are named the same

@santanusinha
Copy link
Owner

@r0goyal this needs more reviews

@santanusinha santanusinha requested a review from r0goyal December 11, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants