From 7fb834981ab6e987d1d0c784aebfbe642677e9a9 Mon Sep 17 00:00:00 2001 From: Eric Bottard Date: Wed, 27 Jan 2016 14:05:23 +0100 Subject: [PATCH] GH-291 Provide an option to create a mutable Tuple This fixes spring-cloud/spring-cloud-stream#290 --- .../MessageChannelBinderSupportTests.java | 7 +- .../stream/tuple/DefaultMutableTuple.java | 63 +++++++ .../cloud/stream/tuple/DefaultTuple.java | 33 ++-- .../tuple/JsonStringToTupleConverter.java | 6 +- .../cloud/stream/tuple/MutableTuple.java | 38 ++++ .../cloud/stream/tuple/Tuple.java | 7 +- .../cloud/stream/tuple/TupleBuilder.java | 31 +++- .../tuple/spel/TuplePropertyAccessor.java | 18 +- .../spel/TuplePropertyAccessorTests.java | 171 ++++++++++++++++++ 9 files changed, 333 insertions(+), 41 deletions(-) create mode 100644 spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultMutableTuple.java create mode 100644 spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/MutableTuple.java create mode 100644 spring-cloud-stream-tuple/src/test/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessorTests.java diff --git a/spring-cloud-stream-binders/spring-cloud-stream-binder-test/src/test/java/org/springframework/cloud/stream/binder/MessageChannelBinderSupportTests.java b/spring-cloud-stream-binders/spring-cloud-stream-binder-test/src/test/java/org/springframework/cloud/stream/binder/MessageChannelBinderSupportTests.java index 907483c0c..a57075376 100644 --- a/spring-cloud-stream-binders/spring-cloud-stream-binder-test/src/test/java/org/springframework/cloud/stream/binder/MessageChannelBinderSupportTests.java +++ b/spring-cloud-stream-binders/spring-cloud-stream-binder-test/src/test/java/org/springframework/cloud/stream/binder/MessageChannelBinderSupportTests.java @@ -25,6 +25,8 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import com.esotericsoftware.kryo.Kryo; +import com.esotericsoftware.kryo.Registration; import org.junit.Before; import org.junit.Test; @@ -44,9 +46,6 @@ import org.springframework.messaging.support.GenericMessage; import org.springframework.util.MimeType; import org.springframework.util.MimeTypeUtils; -import com.esotericsoftware.kryo.Kryo; -import com.esotericsoftware.kryo.Registration; - /** * @author Gary Russell * @author David Turanski @@ -177,7 +176,7 @@ public class MessageChannelBinderSupportTests { @Test public void testTupleSerialization() { Tuple payload = TupleBuilder.tuple().of("foo", "bar"); - MessageValues convertedValues = binder.serializePayloadIfNecessary(new GenericMessage(payload)); + MessageValues convertedValues = binder.serializePayloadIfNecessary(new GenericMessage<>(payload)); Message converted = convertedValues.toMessage(); MimeType mimeType = contentTypeResolver.resolve(converted.getHeaders()); assertEquals("application", mimeType.getType()); diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultMutableTuple.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultMutableTuple.java new file mode 100644 index 000000000..69389a703 --- /dev/null +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultMutableTuple.java @@ -0,0 +1,63 @@ +/* + * Copyright 2016 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 + * + * http://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.cloud.stream.tuple; + +import java.util.List; + +import org.springframework.core.convert.support.ConfigurableConversionService; + +/** + * Extension of {@link DefaultTuple} that also allows tuple {@link MutableTuple mutation}. + * + *

Note that this implementation is not threadsafe.

+ * + * @author Eric Bottard + */ +public class DefaultMutableTuple extends DefaultTuple implements MutableTuple { + + /** + * Construct a new DefaultMutableTuple, given a list of names and object values along + * with a conversion service. + * @param names The list of String names to associate with the list of Object values + * @param values The list of Object values to associate with the list of String names + * @param configurableConversionService A conversion service instance that can Converting strings to Java types. + */ + public DefaultMutableTuple(List names, List values, ConfigurableConversionService configurableConversionService) { + super(names, values, configurableConversionService); + } + + @Override + public void setValue(int index, Object value) { + if (index < 0 || index >= size()) { + throw new IndexOutOfBoundsException(); + } + values.set(index, value); + } + + @Override + public void setValue(String name, Object value) { + int index = indexOf(name); + if (index != -1) { + setValue(index, value); + } else { + names.add(name); + values.add(value); + } + } + + +} diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultTuple.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultTuple.java index 1fb9342ef..8b479a76b 100644 --- a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultTuple.java +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/DefaultTuple.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -36,22 +36,31 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** - * Default implementation of Tuple interface + * Default implementation of Tuple interface. + * * @author Mark Pollack * @author David Turanski * @author Michael Minella */ public class DefaultTuple implements Tuple { - private List names; + protected List names; - private List values; + protected List values; - private transient ConfigurableConversionService configurableConversionService; + protected transient ConfigurableConversionService configurableConversionService; - private transient Converter tupleToStringConverter = new TupleToJsonStringConverter(); + protected transient Converter tupleToStringConverter = new TupleToJsonStringConverter(); - public DefaultTuple(List names, List values, ConfigurableConversionService + /** + * Construct a new DefaultMutableTuple, given a list of names and object values along + * with a conversion service. + * + * @param names The list of names to associate with the list of values + * @param values The list of values to associate with the list of names + * @param configurableConversionService A conversion service instance that can Converting strings to Java types. + */ + public DefaultTuple(List names, List values, ConfigurableConversionService configurableConversionService) { Assert.notNull(names); Assert.notNull(values); @@ -521,22 +530,12 @@ public class DefaultTuple implements Tuple { } } - /* - * (non-Javadoc) - * - * @see org.springframework.cloud.stream.tuple.Tuple#getValue(java.lang.String, java.lang.Class) - */ @Override public T getValue(String name, Class valueClass) { Object value = values.get(indexOf(name)); return convert(value, valueClass); } - /* - * (non-Javadoc) - * - * @see org.springframework.cloud.stream.tuple.Tuple#getValue(int, java.lang.Class) - */ @Override public T getValue(int index, Class valueClass) { return convert(values.get(index), valueClass); diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/JsonStringToTupleConverter.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/JsonStringToTupleConverter.java index 5e6710ab9..0e7af4fc3 100644 --- a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/JsonStringToTupleConverter.java +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/JsonStringToTupleConverter.java @@ -18,13 +18,13 @@ import java.util.Iterator; import java.util.List; import java.util.Map.Entry; -import com.fasterxml.jackson.core.JsonProcessingException; -import org.springframework.core.convert.converter.Converter; - import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import org.springframework.core.convert.converter.Converter; + /** * @author David Turanski */ diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/MutableTuple.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/MutableTuple.java new file mode 100644 index 000000000..eacfcfff2 --- /dev/null +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/MutableTuple.java @@ -0,0 +1,38 @@ +/* + * Copyright 2016 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 + * + * http://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.cloud.stream.tuple; + +/** + * Extension of {@link Tuple} that allows mutating and addition of fields. + * + * @author Eric Bottard + */ +public interface MutableTuple extends Tuple { + + /** + * Sets the value of an already existing property, given its index. + */ + void setValue(int index, Object value); + + /** + * Sets the value of a property, by name. If this Tuple does not currently hold + * a property under that name, a new mapping is added at the end and the size of this + * Tuple is extended by 1. + */ + void setValue(String name, Object value); + +} diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/Tuple.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/Tuple.java index a0c4c5d75..cd019b550 100644 --- a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/Tuple.java +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/Tuple.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2016 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. @@ -26,11 +26,12 @@ import org.springframework.core.convert.ConversionFailedException; * Data structure that stores a fixed number of ordered key-value pairs and adds convenience methods to access values in * a type-safe manner. * - * The structure is immutable once created and values to not need to be of the same type. When accessing values, + * Values do not need to be of the same type. When accessing values, * Spring's type conversion system is used to convert the value type to the requested type. The type conversion system * is extensible. * - * Tuples are created using the TupleBuilder class. + * This interface only allows querying properties. Concrete Tuples can also implement {@link MutableTuple} to allow + * mutation. Tuples are created using the TupleBuilder class. * * @author Mark Pollack * diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/TupleBuilder.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/TupleBuilder.java index 1881b0d62..cc0aa573f 100644 --- a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/TupleBuilder.java +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/TupleBuilder.java @@ -47,7 +47,7 @@ public class TupleBuilder { private List values = new ArrayList<>(); - private static final ConfigurableConversionService defaultConversionService; + private final boolean mutable; private ConfigurableConversionService customConversionService = null; @@ -55,6 +55,8 @@ public class TupleBuilder { private final static Locale DEFAULT_LOCALE = Locale.US; + private static final ConfigurableConversionService defaultConversionService; + private static Converter tupleToStringConverter = new TupleToJsonStringConverter(); private static Converter stringToTupleConverter = new JsonStringToTupleConverter(); @@ -68,8 +70,22 @@ public class TupleBuilder { defaultConversionService.addConverter(new StringToDateConverter(dateFormat)); } + /** + * Return a new builder that will create immutable Tuples. + */ + public static TupleBuilder mutableTuple() { + return new TupleBuilder(true); + } + + /** + * Return a new builder that will create {@link MutableTuple}. + */ public static TupleBuilder tuple() { - return new TupleBuilder(); + return new TupleBuilder(false); + } + + private TupleBuilder(boolean mutable) { + this.mutable = mutable; } public Tuple of(String k1, Object v1) { @@ -161,13 +177,10 @@ public class TupleBuilder { protected Tuple newTuple(List names, List values) { DefaultTuple tuple; - if (customConversionService != null) { - tuple = new DefaultTuple(names, values, customConversionService); - } - else { - tuple = new DefaultTuple(names, values, defaultConversionService); - } - + ConfigurableConversionService conversionService = customConversionService != null ? customConversionService : defaultConversionService; + tuple = mutable + ? new DefaultMutableTuple(names, values, conversionService) + : new DefaultTuple(names, values, conversionService); tuple.setTupleToStringConverter(tupleToStringConverter); return tuple; } diff --git a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessor.java b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessor.java index 6a934c836..43e6c9650 100644 --- a/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessor.java +++ b/spring-cloud-stream-tuple/src/main/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2016 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. @@ -16,6 +16,7 @@ package org.springframework.cloud.stream.tuple.spel; +import org.springframework.cloud.stream.tuple.MutableTuple; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; import org.springframework.expression.PropertyAccessor; @@ -24,15 +25,16 @@ import org.springframework.cloud.stream.tuple.Tuple; /** * A {@link PropertyAccessor} implementation that enables reading of {@link Tuple} values using dot notation within SpEL - * expressions. Writing is not supported since {@link Tuple}s are immutable. + * expressions. * * @author Mark Fisher + * @author Eric Bottard */ public class TuplePropertyAccessor implements PropertyAccessor { @Override public Class[] getSpecificTargetClasses() { - return new Class[] { Tuple.class }; + return new Class[] { Tuple.class, MutableTuple.class}; } @Override @@ -85,12 +87,18 @@ public class TuplePropertyAccessor implements PropertyAccessor { @Override public boolean canWrite(EvaluationContext context, Object target, String name) throws AccessException { - return false; + return target instanceof MutableTuple; } @Override public void write(EvaluationContext context, Object target, String name, Object newValue) throws AccessException { - throw new UnsupportedOperationException("Tuple is immutable"); + MutableTuple tuple = (MutableTuple) target; + Integer index = maybeIndex(name, tuple); + if (index != null) { + tuple.setValue(index, newValue); + } else { + tuple.setValue(name, newValue); + } } /** diff --git a/spring-cloud-stream-tuple/src/test/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessorTests.java b/spring-cloud-stream-tuple/src/test/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessorTests.java new file mode 100644 index 000000000..2c0570b42 --- /dev/null +++ b/spring-cloud-stream-tuple/src/test/java/org/springframework/cloud/stream/tuple/spel/TuplePropertyAccessorTests.java @@ -0,0 +1,171 @@ +/* + * Copyright 2016 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 + * + * http://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.cloud.stream.tuple.spel; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import org.hamcrest.CoreMatchers; +import org.junit.Before; +import org.junit.Test; + +import org.springframework.cloud.stream.tuple.Tuple; +import org.springframework.cloud.stream.tuple.TupleBuilder; +import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.StandardEvaluationContext; + +/** + * Unit tests for {@link TuplePropertyAccessor}. + * + * @author Eric Bottard + * @author Mark Fisher + */ +public class TuplePropertyAccessorTests { + + private final SpelExpressionParser parser = new SpelExpressionParser(); + + private final StandardEvaluationContext context = new StandardEvaluationContext(); + + @Before + public void setup() { + this.context.addPropertyAccessor(new TuplePropertyAccessor()); + } + + @Test + public void testSimplePropertyByFieldName() { + Tuple tuple = TupleBuilder.tuple().of("foo", "bar"); + String result = evaluate("foo", tuple, String.class); + assertThat(result, is("bar")); + } + + @Test + public void testSimplePropertyByIndex() { + Tuple tuple = TupleBuilder.tuple().of("foo", "bar"); + String result = evaluate("['0']", tuple, String.class); + assertThat(result, is("bar")); + } + + @Test(expected = SpelEvaluationException.class) + public void failOnNegativeIndex() { + Tuple tuple = TupleBuilder.tuple().of("foo", "bar"); + evaluate("['-3']", tuple, String.class); + } + + @Test + public void testNestedPropertyByFieldName() { + Tuple child = TupleBuilder.tuple().of("b", 123); + Tuple tuple = TupleBuilder.tuple().of("a", child); + int result = evaluate("a.b", tuple, Integer.class); + assertThat(result, is(123)); + } + + @Test + public void testNestedPropertyByIndex() { + Tuple child = TupleBuilder.tuple().of("b", 123); + Tuple tuple = TupleBuilder.tuple().of("a", child); + int result = evaluate("a['0']", tuple, Integer.class); + assertThat(result, is(123)); + } + + @Test + public void testNestedPropertyByIndexOnly() { + Tuple child = TupleBuilder.tuple().of("b", 123); + Tuple tuple = TupleBuilder.tuple().of("a", child); + int result = evaluate("['0']['0']", tuple, Integer.class); + assertThat(result, is(123)); + } + + @Test + public void testArrayPropertyByFieldName() { + Tuple tuple = TupleBuilder.tuple().of("numbers", new Integer[] { 1, 2, 3 }); + int result = evaluate("numbers[1]", tuple, Integer.class); + assertThat(result, is(2)); + } + + @Test + public void testArrayPropertyByIndex() { + Tuple tuple = TupleBuilder.tuple().of("numbers", new Integer[] { 1, 2, 3 }); + int result = evaluate("['0'][0]", tuple, Integer.class); + assertThat(result, is(1)); + } + + @Test + public void testNestedArrayPropertyByFieldName() { + Tuple child = TupleBuilder.tuple().of("numbers", new Integer[] { 7, 8, 9 }); + Tuple tuple = TupleBuilder.tuple().of("child", child); + int result = evaluate("child.numbers[1]", tuple, Integer.class); + assertThat(result, is(8)); + } + + @Test + public void testNestedArrayPropertyByIndex() { + Tuple child = TupleBuilder.tuple().of("numbers", new Integer[] { 7, 8, 9 }); + Tuple tuple = TupleBuilder.tuple().of("child", child); + int result = evaluate("child['0'][2]", tuple, Integer.class); + assertThat(result, is(9)); + } + + @Test + public void testNestedArrayPropertyByIndexOnly() { + Tuple child = TupleBuilder.tuple().of("numbers", new Integer[] { 7, 8, 9 }); + Tuple tuple = TupleBuilder.tuple().of("child", child); + int result = evaluate("['0']['0'][1]", tuple, Integer.class); + assertThat(result, is(8)); + } + + // Write + + @Test + public void testSimpleWriteOnExistingProperty() { + Tuple tuple = TupleBuilder.mutableTuple().of("foo", "bar"); + write("foo", tuple, 123); + assertThat(tuple.getInt("foo"), is(123)); + } + + @Test + public void testSimpleWriteOnNewProperty() { + Tuple tuple = TupleBuilder.mutableTuple().of("foo", "bar"); + write("other", tuple, 123); + assertThat(tuple.getInt("other"), is(123)); + assertThat(tuple.getValue("foo"), CoreMatchers.is("bar")); + } + + @Test + public void testSimpleWriteOnExistingPropertyByIndex() { + Tuple tuple = TupleBuilder.mutableTuple().of("foo", "bar"); + write("['0']", tuple, 123); + assertThat(tuple.getInt("foo"), is(123)); + } + + @Test + public void testSimpleWriteOnNewPropertyByIndex() { + Tuple tuple = TupleBuilder.mutableTuple().of("foo", "bar"); + write("['12']", tuple, 123); + assertThat(tuple.getInt("12"), is(123)); + assertThat(tuple.getValue("foo"), CoreMatchers.is("bar")); + } + + private T evaluate(String expression, Tuple tuple, Class expectedType) { + return parser.parseExpression(expression).getValue(this.context, tuple, expectedType); + } + + private void write(String expression, Tuple tuple, Object value) { + parser.parseExpression(expression).setValue(this.context, tuple, value); + } + +}