Skip to content
This repository was archived by the owner on Sep 13, 2024. It is now read-only.

Simplify Connection Configuration#1

Merged
wegenmic merged 13 commits into
developfrom
feature/rabbitmq-builder
Feb 16, 2017
Merged

Simplify Connection Configuration#1
wegenmic merged 13 commits into
developfrom
feature/rabbitmq-builder

Conversation

@tunikum
Copy link
Copy Markdown
Contributor

@tunikum tunikum commented Feb 13, 2017

This PR simplifies the RabbitMQ and JMS strategies by the following changes:

  • Builder pattern introduced, which allows building connection by calling builder methods or by just passing a Properties instance.
  • RabbitMQ: Required specification of a queue name removed. RabbitMQ will create a random temporary queue per default.

@tunikum tunikum changed the title Simplify RabbitMQ Configuration [WIP] Simplify RabbitMQ Configuration Feb 13, 2017
@tunikum tunikum added the WIP label Feb 13, 2017
@tunikum tunikum changed the title [WIP] Simplify RabbitMQ Configuration Simplify RabbitMQ Configuration Feb 13, 2017
@tunikum tunikum changed the title Simplify RabbitMQ Configuration Simplify Connection Configuration Feb 15, 2017
@tunikum tunikum removed the WIP label Feb 15, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+6.2%) to 75.342% when pulling 29a132e on feature/rabbitmq-builder into da271ee on develop.

*/
public ContextProperties(Properties properties, String context) {
super(properties);
this.context = context + '.';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we really manipulate a context we get as an input param?

Copy link
Copy Markdown
Contributor Author

@tunikum tunikum Feb 16, 2017

Choose a reason for hiding this comment

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

The idea is that you don't need to suffix your context by ., which would most likely always be the case as . is the default delimiter for property names (at least in our libs). How about a setContextDelimiter()? Removed the ..

/**
* Created by tgdpaad2 on 15/02/17.
*/
public class PropertyUtil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add missing javadoc for PropertyUtil class

*/
public JmsBuilder connectionFactory(ConnectionFactory connectionFactory) {
this.connectionFactory = connectionFactory;
return (JmsBuilder) this;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should try to avoid unchecked casts when returning 'this'.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same in class QueueConnection

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only solution I found is this one: http://stackoverflow.com/questions/17164375/subclassing-a-java-builder-class/34741836#34741836. I wasn't really happy with this either.
What do you vote for, getThis() or unchecked casts?

@wegenmic
Copy link
Copy Markdown
Contributor

In general we should use the modifiers 'static final' for constants (normally written in capital letters)

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 16, 2017

Coverage Status

Coverage increased (+6.1%) to 75.229% when pulling 27458cb on feature/rabbitmq-builder into da271ee on develop.

@wegenmic wegenmic merged commit 94aef87 into develop Feb 16, 2017
@wegenmic wegenmic deleted the feature/rabbitmq-builder branch February 16, 2017 10:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants