Skip to content

Configure ConnectionFactory instances using fluent API pattern #1038

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

Merged

Conversation

enniokerber
Copy link

@enniokerber enniokerber commented May 20, 2023

Proposed Changes

This pull request adds the ability to configure ConnectionFactory instances using the fluent API pattern as proposed in #330.
The aim of this is to make configuration of new connections easier and require less code.

Before (extracted from SslContextFactoryTest)

        doTestSetSslContextFactory(() -> {
            ConnectionFactory connectionFactory = new ConnectionFactory();
            connectionFactory.useBlockingIo();
            connectionFactory.setAutomaticRecoveryEnabled(true);
            return connectionFactory;
        });

After

        doTestSetSslContextFactory(() -> new ConnectionFactory()
                .useBlockingIo()
                .setAutomaticRecoveryEnabled(true)
        );

As you can see, using this pattern increases readability and reduces boilerplate code.

Types of Changes

  • Bugfix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance)

As proposed in #330 already, I also encourange deploying this with the new major version 6 as the API of ConnectionFactory changes due to this feature.

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

Closes #330.

@michaelklishin
Copy link
Contributor

michaelklishin commented May 21, 2023

I don't think it really "reduces boilerplate code" in your example, it's basically one fewer line, and usually with the Builder pattern you have an explicit termination method (.build() or similar).

@michaelklishin
Copy link
Contributor

That said, thanks for taking the time to contribute. I don't have any objections to having
an API like this, assuming the existing code does not have to be modified (recompiling is OK for the next minor release).

@michaelklishin
Copy link
Contributor

To confirm: some instantiations of ConnectionFactory in tests still use the original API,
therefore proving (source-level) backwards compatibility.

@michaelklishin michaelklishin added this to the 5.18.0 milestone May 21, 2023
Copy link
Contributor

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

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

Thank you!

@michaelklishin michaelklishin merged commit 9c31a5c into rabbitmq:main May 21, 2023
@michaelklishin michaelklishin removed this from the 5.18.0 milestone May 21, 2023
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.

ConnectionFactory: fluent style
2 participants