Skip to content

Commit

Permalink
GH-2439: Fix Bindings with Broker Gen Queue Names
Browse files Browse the repository at this point in the history
Resolves #2439

To configure a broker-named queue, the `Queue` name is set to an empty
String; the name is later populated when the queue is declared.

However, if such a queue is used in a `BindingBuilder`, the builder accesses
the name before it is populated.

When such a queue is used in a binding, retain a copy of the queue object and
retrieve its actual name when the binding is declared.

Add a unit test for all binding types; also tested with the following, with
both queues being bound properly to the exchange.

```java
@bean
Queue q1() {
	return QueueBuilder.nonDurable("").build();
}

@bean
Queue q2() {
	return QueueBuilder.nonDurable("").build();
}

@bean
Binding b1(Queue q1, DirectExchange ex) {
	return BindingBuilder.bind(q1).to(ex).with("foo");
}

@bean
Binding b2(Queue q2, DirectExchange ex) {
	return BindingBuilder.bind(q2).to(ex).with("bar");
}

@bean
DirectExchange ex() {
	return new DirectExchange("ex");
}
```

**cherry-pick to 2.4.x**
  • Loading branch information
garyrussell authored and artembilan committed Mar 23, 2023
1 parent 61f535b commit 6ed749e
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
import java.util.Map;

import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
* Simple container collecting information to describe a binding. Takes String destination and exchange names as
Expand Down Expand Up @@ -50,26 +51,46 @@ public enum DestinationType {
EXCHANGE;
}

@Nullable
private final String destination;

private final String exchange;

@Nullable
private final String routingKey;

private final DestinationType destinationType;

@Nullable
private final Queue lazyQueue;

public Binding(String destination, DestinationType destinationType, String exchange, String routingKey,
@Nullable Map<String, Object> arguments) {

this(null, destination, destinationType, exchange, routingKey, arguments);
}

public Binding(@Nullable Queue lazyQueue, @Nullable String destination, DestinationType destinationType,
String exchange, @Nullable String routingKey, @Nullable Map<String, Object> arguments) {

super(arguments);
Assert.isTrue(lazyQueue == null || destinationType.equals(DestinationType.QUEUE),
"'lazyQueue' must be null for destination type " + destinationType);
Assert.isTrue(lazyQueue != null || destination != null, "`destination` cannot be null");
this.lazyQueue = lazyQueue;
this.destination = destination;
this.destinationType = destinationType;
this.exchange = exchange;
this.routingKey = routingKey;
}

public String getDestination() {
return this.destination;
if (this.lazyQueue != null) {
return this.lazyQueue.getActualName();
}
else {
return this.destination;
}
}

public DestinationType getDestinationType() {
Expand All @@ -81,6 +102,9 @@ public String getExchange() {
}

public String getRoutingKey() {
if (this.routingKey == null && this.lazyQueue != null) {
return this.lazyQueue.getActualName();
}
return this.routingKey;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,7 +37,12 @@ private BindingBuilder() {
}

public static DestinationConfigurer bind(Queue queue) {
return new DestinationConfigurer(queue.getName(), DestinationType.QUEUE);
if ("".equals(queue.getName())) {
return new DestinationConfigurer(queue, DestinationType.QUEUE);
}
else {
return new DestinationConfigurer(queue.getName(), DestinationType.QUEUE);
}
}

public static DestinationConfigurer bind(Exchange exchange) {
Expand All @@ -61,13 +66,22 @@ public static final class DestinationConfigurer {

protected final DestinationType type; // NOSONAR

protected final Queue queue; // NOSONAR

DestinationConfigurer(String name, DestinationType type) {
this.queue = null;
this.name = name;
this.type = type;
}

DestinationConfigurer(Queue queue, DestinationType type) {
this.queue = queue;
this.name = null;
this.type = type;
}

public Binding to(FanoutExchange exchange) {
return new Binding(this.name, this.type, exchange.getName(), "", new HashMap<String, Object>());
return new Binding(this.queue, this.name, this.type, exchange.getName(), "", new HashMap<String, Object>());
}

public HeadersExchangeMapConfigurer to(HeadersExchange exchange) {
Expand Down Expand Up @@ -134,15 +148,17 @@ public final class HeadersExchangeSingleValueBindingCreator {
}

public Binding exists() {
return new Binding(HeadersExchangeMapConfigurer.this.destination.name,
return new Binding(HeadersExchangeMapConfigurer.this.destination.queue,
HeadersExchangeMapConfigurer.this.destination.name,
HeadersExchangeMapConfigurer.this.destination.type,
HeadersExchangeMapConfigurer.this.exchange.getName(), "", createMapForKeys(this.key));
}

public Binding matches(Object value) {
Map<String, Object> map = new HashMap<String, Object>();
map.put(this.key, value);
return new Binding(HeadersExchangeMapConfigurer.this.destination.name,
return new Binding(HeadersExchangeMapConfigurer.this.destination.queue,
HeadersExchangeMapConfigurer.this.destination.name,
HeadersExchangeMapConfigurer.this.destination.type,
HeadersExchangeMapConfigurer.this.exchange.getName(), "", map);
}
Expand All @@ -162,7 +178,8 @@ public final class HeadersExchangeKeysBindingCreator {
}

public Binding exist() {
return new Binding(HeadersExchangeMapConfigurer.this.destination.name,
return new Binding(HeadersExchangeMapConfigurer.this.destination.queue,
HeadersExchangeMapConfigurer.this.destination.name,
HeadersExchangeMapConfigurer.this.destination.type,
HeadersExchangeMapConfigurer.this.exchange.getName(), "", this.headerMap);
}
Expand All @@ -182,7 +199,8 @@ public final class HeadersExchangeMapBindingCreator {
}

public Binding match() {
return new Binding(HeadersExchangeMapConfigurer.this.destination.name,
return new Binding(HeadersExchangeMapConfigurer.this.destination.queue,
HeadersExchangeMapConfigurer.this.destination.name,
HeadersExchangeMapConfigurer.this.destination.type,
HeadersExchangeMapConfigurer.this.exchange.getName(), "", this.headerMap);
}
Expand Down Expand Up @@ -211,13 +229,13 @@ public static final class TopicExchangeRoutingKeyConfigurer extends AbstractRout
}

public Binding with(String routingKey) {
return new Binding(destination.name, destination.type, exchange, routingKey,
return new Binding(destination.queue, destination.name, destination.type, exchange, routingKey,
Collections.<String, Object>emptyMap());
}

public Binding with(Enum<?> routingKeyEnum) {
return new Binding(destination.name, destination.type, exchange, routingKeyEnum.toString(),
Collections.<String, Object>emptyMap());
return new Binding(destination.queue, destination.name, destination.type, exchange,
routingKeyEnum.toString(), Collections.<String, Object>emptyMap());
}
}

Expand Down Expand Up @@ -255,12 +273,14 @@ public GenericArgumentsConfigurer(GenericExchangeRoutingKeyConfigurer configurer
}

public Binding and(Map<String, Object> map) {
return new Binding(this.configurer.destination.name, this.configurer.destination.type, this.configurer.exchange,
return new Binding(this.configurer.destination.queue,
this.configurer.destination.name, this.configurer.destination.type, this.configurer.exchange,
this.routingKey, map);
}

public Binding noargs() {
return new Binding(this.configurer.destination.name, this.configurer.destination.type, this.configurer.exchange,
return new Binding(this.configurer.destination.queue,
this.configurer.destination.name, this.configurer.destination.type, this.configurer.exchange,
this.routingKey, Collections.<String, Object>emptyMap());
}

Expand All @@ -276,19 +296,20 @@ public static final class DirectExchangeRoutingKeyConfigurer extends AbstractRou
}

public Binding with(String routingKey) {
return new Binding(destination.name, destination.type, exchange, routingKey,
return new Binding(destination.queue, destination.name, destination.type, exchange, routingKey,
Collections.<String, Object>emptyMap());
}

public Binding with(Enum<?> routingKeyEnum) {
return new Binding(destination.name, destination.type, exchange, routingKeyEnum.toString(),
Collections.<String, Object>emptyMap());
return new Binding(destination.queue, destination.name, destination.type, exchange,
routingKeyEnum.toString(), Collections.<String, Object>emptyMap());
}

public Binding withQueueName() {
return new Binding(destination.name, destination.type, exchange, destination.name,
return new Binding(destination.queue, destination.name, destination.type, exchange, destination.name,
Collections.<String, Object>emptyMap());
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.amqp.core;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.Collections;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

/**
* Copy of {@link BindingBuilderTests} but using a queue with a lazy name.
*
* @author Mark Fisher
* @author Artem Yakshin
* @author Gary Russell
*/
public class BindingBuilderWithLazyQueueNameTests {

private static Queue queue;

@BeforeAll
public static void setUp() {
queue = new Queue("");
queue.setActualName("actual");
}

@Test
public void fanoutBinding() {
FanoutExchange fanoutExchange = new FanoutExchange("f");
Binding binding = BindingBuilder.bind(queue).to(fanoutExchange);
assertThat(binding).isNotNull();
assertThat(binding.getExchange()).isEqualTo(fanoutExchange.getName());
assertThat(binding.getRoutingKey()).isEqualTo("");
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
}

@Test
public void directBinding() {
DirectExchange directExchange = new DirectExchange("d");
String routingKey = "r";
Binding binding = BindingBuilder.bind(queue).to(directExchange).with(routingKey);
assertThat(binding).isNotNull();
assertThat(binding.getExchange()).isEqualTo(directExchange.getName());
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
assertThat(binding.getRoutingKey()).isEqualTo(routingKey);
}

@Test
public void directBindingWithQueueName() {
DirectExchange directExchange = new DirectExchange("d");
Binding binding = BindingBuilder.bind(queue).to(directExchange).withQueueName();
assertThat(binding).isNotNull();
assertThat(binding.getExchange()).isEqualTo(directExchange.getName());
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
assertThat(binding.getRoutingKey()).isEqualTo(queue.getActualName());
}

@Test
public void topicBinding() {
TopicExchange topicExchange = new TopicExchange("t");
String routingKey = "r";
Binding binding = BindingBuilder.bind(queue).to(topicExchange).with(routingKey);
assertThat(binding).isNotNull();
assertThat(binding.getExchange()).isEqualTo(topicExchange.getName());
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
assertThat(binding.getRoutingKey()).isEqualTo(routingKey);
}

@Test
public void headerBinding() {
HeadersExchange headersExchange = new HeadersExchange("h");
String headerKey = "headerKey";
Binding binding = BindingBuilder.bind(queue).to(headersExchange).where(headerKey).exists();
assertThat(binding).isNotNull();
assertThat(binding.getExchange()).isEqualTo(headersExchange.getName());
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
assertThat(binding.getRoutingKey()).isEqualTo("");
}

@Test
public void customBinding() {
class CustomExchange extends AbstractExchange {
CustomExchange(String name) {
super(name);
}

@Override
public String getType() {
return "x-custom";
}
}
Object argumentObject = new Object();
CustomExchange customExchange = new CustomExchange("c");
String routingKey = "r";
Binding binding = BindingBuilder.//
bind(queue).//
to(customExchange).//
with(routingKey).//
and(Collections.<String, Object>singletonMap("k", argumentObject));
assertThat(binding).isNotNull();
assertThat(binding.getArguments().get("k")).isEqualTo(argumentObject);
assertThat(binding.getExchange()).isEqualTo(customExchange.getName());
assertThat(binding.getDestinationType()).isEqualTo(Binding.DestinationType.QUEUE);
assertThat(binding.getDestination()).isEqualTo(queue.getActualName());
assertThat(binding.getRoutingKey()).isEqualTo(routingKey);
}

}

0 comments on commit 6ed749e

Please sign in to comment.