Skip to content

Commit

Permalink
Support for setting credentials and vhost in rabbit addresses
Browse files Browse the repository at this point in the history
User can now add credentials, vhost and protocol prefix (amqp://)
to any or all of the addresses, extending the format beyond that accepted
bu the rabbitmq client, but making it cloud friendly. Only one of
the addresses needs those properties and all are optional. Port
also defaults to 5672 in an address.
  • Loading branch information
Dave Syer committed Jul 7, 2014
1 parent 148e32d commit aa38d33
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
Expand Up @@ -70,13 +70,46 @@ public int getPort() {
}

public void setAddresses(String addresses) {
this.addresses = addresses;
this.addresses = parseAddresses(addresses);
}

public String getAddresses() {
return (this.addresses == null ? this.host + ":" + this.port : this.addresses);
}

private String parseAddresses(String addresses) {
StringBuilder result = new StringBuilder();
for (String address : StringUtils.commaDelimitedListToSet(addresses)) {

This comment has been minimized.

Copy link
@garyrussell

garyrussell Jul 15, 2014

Contributor

@dsyer I think this is a problem.

People will typically connect to a cluster in "preferred" order in fact they might use a different order on each server to spread the load.

Since commaDelimitedListToSet uses a TreeSet, the addresses will always be sorted.

Also, in the code below, the user might anticipate different creds are allowed per address, which is not the case.

This comment has been minimized.

Copy link
@dsyer

dsyer Jul 15, 2014

Member

TreeSet is kind of crazy. We should change that.

On the credentials, I looked at the native Rabbit library and I couldn't see any way to specify credentials per server. What would it look like?

This comment has been minimized.

Copy link
@garyrussell

garyrussell Jul 15, 2014

Contributor

Yeah, it should be a LinkedHashSet to maintain order.

No; there is no way for credentials per server, but with the code below, "last one wins" with no error/warning.

address = address.trim();
if (address.startsWith("amqp://")) {
address = address.substring("amqp://".length());
}
if (address.contains("@")) {
String[] split = StringUtils.split(address, "@");
String creds = split[0];
address = split[1];
split = StringUtils.split(creds, ":");
this.username = split[0];
if (split.length > 0) {
this.password = split[1];
}
}
int index = address.indexOf("/");
if (index >= 0 && index < address.length()) {
this.virtualHost = address.substring(index + 1);
address = address.substring(0, index);
}
if (result.length() > 0) {
result.append(",");
}
if (!address.contains(":")) {
address = address + ":" + this.port;
}
result.append(address);
}
return result.length() > 0 ? result.toString() : null;
}

public void setPort(int port) {
this.port = port;
}
Expand Down
Expand Up @@ -50,6 +50,34 @@ public void addressesDoubleValued() {
assertEquals(9999, this.properties.getPort());
}

@Test
public void addressesDoubleValuedWithCredentials() {
this.properties.setAddresses("myhost:9999,root:password@otherhost:1111/host");
assertNull(this.properties.getHost());
assertEquals(9999, this.properties.getPort());
assertEquals("root", this.properties.getUsername());
assertEquals("host", this.properties.getVirtualHost());
}

@Test
public void addressesSingleValuedWithCredentials() {
this.properties.setAddresses("amqp://root:password@otherhost:1111/host");
assertEquals("otherhost", this.properties.getHost());
assertEquals(1111, this.properties.getPort());
assertEquals("root", this.properties.getUsername());
assertEquals("host", this.properties.getVirtualHost());
}

@Test
public void addressesSingleValuedWithCredentialsDefaultPort() {
this.properties.setAddresses("amqp://root:password@lemur.cloudamqp.com/host");
assertEquals("lemur.cloudamqp.com", this.properties.getHost());
assertEquals(5672, this.properties.getPort());
assertEquals("root", this.properties.getUsername());
assertEquals("host", this.properties.getVirtualHost());
assertEquals("lemur.cloudamqp.com:5672", this.properties.getAddresses());
}

@Test
public void testDefaultVirtualHost() {
this.properties.setVirtualHost("/");
Expand Down

1 comment on commit aa38d33

@jhoeller
Copy link

Choose a reason for hiding this comment

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

Guys, I'm afraid we can't easily change StringUtils.commaDelimitedListToSet since it is even documented to return a TreeSet; I'm rather considering to deprecate it altogether. Preferably, use StringUtils.commaDelimitedListToStringArray instead which does preserve the original order...

Please sign in to comment.