Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not build exception instances in advance (#3983)

Browse Source 
* Do not build exception instances in advance

When no `cause` and no clean context of the exception thrown,
the code place where an exception has been created is confusing.

* Fix `SimpleMessageStore` to create an `out of capacity` exception in the
`addMessagesToGroup()` exactly at the point where it is thrown
* Fix `FunctionExpression` and `SupplierExpression` do not pre-create `readOnlyException`:
it is unlikely these kind of expressions are going to be used in the SpEL `write` context,
so we save some time and memory not creating extra object and the plain call `throw new`
gives us a clean context what method call has ended up with such an exception

* * Extract `outOfCapacityException` to the method in the `SimpleMessageStore`
to avoid duplication.
Even if we got in the end extra line in th stack trace, it is still clear from
where it is thrown:

```
org.springframework.messaging.MessagingException: SimpleMessageStore was out of capacity (1) for group 'foo', try constructing it with a larger number.

	at org.springframework.integration.store.SimpleMessageStore.outOfCapacityException(SimpleMessageStore.java:324)
	at org.springframework.integration.store.SimpleMessageStore.addMessagesToGroup(SimpleMessageStore.java:302)
	at org.springframework.integration.store.AbstractMessageGroupStore.addMessageToGroup(AbstractMessageGroupStore.java:189)
	at org.springframework.integration.store.SimpleMessageStoreTests.shouldNotHoldMoreThanGroupCapacity(SimpleMessageStoreTests.java:128)
```
This commit is contained in:
Artem Bilan
2023-01-05 11:35:24 -05:00
committed by GitHub
parent cf5f218449
commit 308f3fadeb
4 changed files with 93 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
spring-integration-core/src/main/java/org/springframework/integration/expression/FunctionExpression.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2019 the original author or authors.
* Copyright 2014-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.
@@ -57,13 +57,9 @@ public class FunctionExpression<S> implements Expression {
private final EvaluationContext defaultContext = new StandardEvaluationContext();
private final EvaluationException readOnlyException;
public FunctionExpression(Function<S, ?> function) {
Assert.notNull(function, "'function' must not be null.");
this.function = function;
this.readOnlyException = new EvaluationException(getExpressionString(),
"FunctionExpression is a 'read only' Expression implementation");
}
@Override
@@ -123,60 +119,65 @@ public class FunctionExpression<S> implements Expression {
@Override
public Class<?> getValueType() throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(@Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(EvaluationContext context) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(EvaluationContext context, @Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor() throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(@Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context, @Nullable Object rootObject)
throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(EvaluationContext context, @Nullable Object value) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(@Nullable Object rootObject, @Nullable Object value) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(EvaluationContext context, @Nullable Object rootObject, @Nullable Object value)
throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
private EvaluationException readOnlyException() {
return new EvaluationException(getExpressionString(),
"FunctionExpression is a 'read only' Expression implementation");
}
@Override

34
spring-integration-core/src/main/java/org/springframework/integration/expression/SupplierExpression.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2019 the original author or authors.
* Copyright 2014-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.
@@ -47,6 +47,7 @@ import org.springframework.util.Assert;
*
* @author Artem Bilan
* @author Gary Russell
*
* @since 5.0
*/
public class SupplierExpression<T> implements Expression {
@@ -55,13 +56,9 @@ public class SupplierExpression<T> implements Expression {
private final EvaluationContext defaultContext = new StandardEvaluationContext();
private final EvaluationException readOnlyException;
public SupplierExpression(Supplier<T> supplier) {
Assert.notNull(supplier, "'function' must not be null.");
this.supplier = supplier;
this.readOnlyException = new EvaluationException(getExpressionString(),
"SupplierExpression is a 'read only' Expression implementation");
}
@Override
@@ -108,58 +105,63 @@ public class SupplierExpression<T> implements Expression {
@Override
public Class<?> getValueType() throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(EvaluationContext context) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public Class<?> getValueType(EvaluationContext context, Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor() throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(Object rootObject) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context, Object rootObject)
throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(EvaluationContext context, Object value) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(Object rootObject, Object value) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
@Override
public void setValue(EvaluationContext context, Object rootObject, Object value) throws EvaluationException {
throw this.readOnlyException;
throw readOnlyException();
}
private EvaluationException readOnlyException() {
return new EvaluationException(getExpressionString(),
"SupplierExpression is a 'read only' Expression implementation");
}
@Override

17
spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageStore.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 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.
@@ -127,6 +127,7 @@ public class SimpleMessageStore extends AbstractMessageGroupStore
*/
public SimpleMessageStore(int individualCapacity, int groupCapacity, long upperBoundTimeout,
LockRegistry lockRegistry) {
super(false);
Assert.notNull(lockRegistry, "The LockRegistry cannot be null");
this.individualUpperBound = new UpperBound(individualCapacity);
@@ -278,13 +279,9 @@ public class SimpleMessageStore extends AbstractMessageGroupStore
try {
UpperBound upperBound;
MessageGroup group = this.groupIdToMessageGroup.get(groupId);
MessagingException outOfCapacityException =
new MessagingException(getClass().getSimpleName() +
" was out of capacity (" + this.groupCapacity + ") for group '" + groupId +
"', try constructing it with a larger capacity.");
if (group == null) {
if (this.groupCapacity > 0 && messages.length > this.groupCapacity) {
throw outOfCapacityException;
throw outOfCapacityException(groupId);
}
group = getMessageGroupFactory().create(groupId);
this.groupIdToMessageGroup.put(groupId, group);
@@ -302,7 +299,7 @@ public class SimpleMessageStore extends AbstractMessageGroupStore
lock.unlock();
if (!upperBound.tryAcquire(this.upperBoundTimeout)) {
unlocked = true;
throw outOfCapacityException;
throw outOfCapacityException(groupId);
}
lock.lockInterruptibly();
group.add(message);
@@ -323,6 +320,12 @@ public class SimpleMessageStore extends AbstractMessageGroupStore
}
}
private MessagingException outOfCapacityException(Object groupId) {
return new MessagingException(getClass().getSimpleName() +
" was out of capacity (" + this.groupCapacity + ") for group '" + groupId +
"', try constructing it with a larger number.");
}
@Override
public void removeMessageGroup(Object groupId) {
Lock lock = this.lockRegistry.obtain(groupId);

110
spring-integration-core/src/test/java/org/springframework/integration/store/SimpleMessageStoreTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 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.
@@ -17,7 +17,6 @@
package org.springframework.integration.store;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.CountDownLatch;
@@ -25,9 +24,8 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.springframework.integration.store.MessageGroupStore.MessageGroupCallback;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessagingException;
@@ -35,7 +33,7 @@ import org.springframework.messaging.support.GenericMessage;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
/**
* @author Iwein Fuld
@@ -55,13 +53,14 @@ public class SimpleMessageStoreTests {
assertThat(store.getMessage(testMessage1.getHeaders().getId())).isEqualTo(testMessage1);
}
@Test(expected = MessagingException.class)
@Test
public void shouldNotHoldMoreThanCapacity() {
SimpleMessageStore store = new SimpleMessageStore(1);
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
Message<String> testMessage2 = MessageBuilder.withPayload("bar").build();
store.addMessage(testMessage1);
store.addMessage(testMessage2);
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessage(testMessage2));
}
@Test
@@ -70,23 +69,16 @@ public class SimpleMessageStoreTests {
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
Message<String> testMessage2 = MessageBuilder.withPayload("bar").build();
store.addMessage(testMessage1);
try {
store.addMessage(testMessage2);
fail("Should have thrown");
}
catch (Exception e) {
assertThat(e).isInstanceOf(MessagingException.class);
assertThat(e.getMessage()).contains("was out of capacity (1)");
}
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessage(testMessage2))
.withMessageContaining("was out of capacity (1)");
store.removeMessage(testMessage2.getHeaders().getId());
try {
store.addMessage(testMessage2);
fail("Should have thrown");
}
catch (Exception e) {
assertThat(e).isInstanceOf(MessagingException.class);
assertThat(e.getMessage()).contains("was out of capacity (1)");
}
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessage(testMessage2))
.withMessageContaining("was out of capacity (1)");
store.removeMessage(testMessage1.getHeaders().getId());
store.addMessage(testMessage2);
@@ -118,23 +110,23 @@ public class SimpleMessageStoreTests {
exec.shutdownNow();
}
@Test(expected = MessagingException.class)
public void shouldTimeoutAfterWaitIfCapacity() throws InterruptedException {
@Test
public void shouldTimeoutAfterWaitIfCapacity() {
SimpleMessageStore store2 = new SimpleMessageStore(1, 1, 10);
store2.addMessage(new GenericMessage<Object>("foo"));
// This should throw
store2.addMessage(new GenericMessage<Object>("foo"));
fail("Should have thrown already");
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store2.addMessage(new GenericMessage<Object>("foo")));
}
@Test(expected = MessagingException.class)
@Test
public void shouldNotHoldMoreThanGroupCapacity() {
SimpleMessageStore store = new SimpleMessageStore(0, 1);
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
Message<String> testMessage2 = MessageBuilder.withPayload("bar").build();
store.addMessageToGroup("foo", testMessage1);
store.addMessageToGroup("foo", testMessage2);
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessageToGroup("foo", testMessage2));
}
@Test
@@ -158,17 +150,17 @@ public class SimpleMessageStoreTests {
assertThat(message2Latch.await(10, TimeUnit.SECONDS)).isTrue();
MessageGroup messageGroup = store2.getMessageGroup("foo");
messageGroup.getMessages().contains(testMessage2);
assertThat(messageGroup.getMessages()).contains(testMessage2);
exec.shutdownNow();
}
@Test(expected = MessagingException.class)
@Test
public void shouldTimeoutAfterWaitIfGroupCapacity() {
SimpleMessageStore store2 = new SimpleMessageStore(1, 1, 1);
store2.addMessageToGroup("foo", MessageBuilder.withPayload("foo").build());
// This should throw
store2.addMessageToGroup("foo", MessageBuilder.withPayload("bar").build());
fail("Should have thrown already");
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store2.addMessageToGroup("foo", MessageBuilder.withPayload("bar").build()));
}
@@ -187,30 +179,24 @@ public class SimpleMessageStoreTests {
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
Message<String> testMessage2 = MessageBuilder.withPayload("bar").build();
store.addMessageToGroup("foo", testMessage1);
try {
store.addMessageToGroup("foo", testMessage2);
fail("Should have thrown");
}
catch (Exception e) {
assertThat(e).isInstanceOf(MessagingException.class);
assertThat(e.getMessage()).contains("was out of capacity (1) for group 'foo'");
}
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessageToGroup("foo", testMessage2))
.withMessageContaining("was out of capacity (1) for group 'foo'");
store.removeMessagesFromGroup("foo", testMessage2);
try {
store.addMessageToGroup("foo", testMessage2);
fail("Should have thrown");
}
catch (Exception e) {
assertThat(e).isInstanceOf(MessagingException.class);
assertThat(e.getMessage()).contains("was out of capacity (1) for group 'foo'");
}
assertThatExceptionOfType(MessagingException.class)
.isThrownBy(() -> store.addMessageToGroup("foo", testMessage2))
.withMessageContaining("was out of capacity (1) for group 'foo'");
store.removeMessagesFromGroup("foo", testMessage1);
store.addMessageToGroup("foo", testMessage2);
}
@Test
public void shouldListByCorrelation() throws Exception {
public void shouldListByCorrelation() {
SimpleMessageStore store = new SimpleMessageStore();
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
store.addMessageToGroup("bar", testMessage1);
@@ -218,7 +204,7 @@ public class SimpleMessageStoreTests {
}
@Test
public void shouldRemoveFromGroup() throws Exception {
public void shouldRemoveFromGroup() {
SimpleMessageStore store = new SimpleMessageStore();
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
store.addMessageToGroup("bar", testMessage1);
@@ -230,7 +216,7 @@ public class SimpleMessageStoreTests {
}
@Test
public void testRepeatedAddAndRemoveGroup() throws Exception {
public void testRepeatedAddAndRemoveGroup() {
SimpleMessageStore store = new SimpleMessageStore(10, 10);
for (int i = 0; i < 10; i++) {
store.addMessageToGroup("bar", MessageBuilder.withPayload("foo").build());
@@ -242,7 +228,7 @@ public class SimpleMessageStoreTests {
}
@Test
public void shouldCopyMessageGroup() throws Exception {
public void shouldCopyMessageGroup() {
SimpleMessageStore store = new SimpleMessageStore();
store.setCopyOnGet(true);
Message<String> testMessage1 = MessageBuilder.withPayload("foo").build();
@@ -251,18 +237,18 @@ public class SimpleMessageStoreTests {
}
@Test
public void shouldRegisterCallbacks() throws Exception {
public void shouldRegisterCallbacks() {
SimpleMessageStore store = new SimpleMessageStore();
store.setExpiryCallbacks(Arrays.<MessageGroupCallback>asList((messageGroupStore, group) -> {
store.setExpiryCallbacks(List.of((messageGroupStore, group) -> {
}));
assertThat(((Collection<?>) ReflectionTestUtils.getField(store, "expiryCallbacks")).size()).isEqualTo(1);
assertThat(((Collection<?>) ReflectionTestUtils.getField(store, "expiryCallbacks"))).hasSize(1);
}
@Test
public void shouldExpireMessageGroup() throws Exception {
public void shouldExpireMessageGroup() {
SimpleMessageStore store = new SimpleMessageStore();
final List<String> list = new ArrayList<String>();
final List<String> list = new ArrayList<>();
store.registerMessageGroupExpiryCallback((messageGroupStore, group) -> {
list.add(group.getOne().getPayload().toString());
messageGroupStore.removeMessageGroup(group.getGroupId());
@@ -279,10 +265,10 @@ public class SimpleMessageStoreTests {
}
@Test
public void testAddAndRemoveMessagesFromMessageGroup() throws Exception {
public void testAddAndRemoveMessagesFromMessageGroup() {
SimpleMessageStore messageStore = new SimpleMessageStore();
String groupId = "X";
List<Message<?>> messages = new ArrayList<Message<?>>();
List<Message<?>> messages = new ArrayList<>();
for (int i = 0; i < 25; i++) {
Message<String> message = MessageBuilder.withPayload("foo").setCorrelationId(groupId).build();
messageStore.addMessageToGroup(groupId, message);