From 36f8cf2bd9f49e209cba6b78d7e06a05e670db07 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Thu, 12 Jun 2014 07:21:37 +0000 Subject: QPID-5721: Improve exception message used when String operand cannot be interpreted as JSON * Also made AVC consistently return unmodifiable when converting from a JSON string git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1602078 13f79535-47bb-0310-9956-ffa450edef68 --- .../qpid/server/model/AttributeValueConverter.java | 70 +++----- .../server/model/AbstractConfiguredObjectTest.java | 2 +- .../server/model/AttributeValueConverterTest.java | 196 +++++++++++++++++++++ 3 files changed, 224 insertions(+), 44 deletions(-) create mode 100644 qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AttributeValueConverterTest.java (limited to 'qpid/java') diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java index d6940655a3..b7b56db15c 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java @@ -196,16 +196,7 @@ abstract class AttributeValueConverter } else if(value instanceof String) { - String interpolated = AbstractConfiguredObject.interpolate(object, (String) value); - ObjectMapper objectMapper = new ObjectMapper(); - try - { - return objectMapper.readValue(interpolated, List.class); - } - catch (IOException e) - { - throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a List"); - } + return Collections.unmodifiableList(convertFromJson((String) value, object, List.class)); } else if(value == null) { @@ -217,6 +208,7 @@ abstract class AttributeValueConverter } } }; + static final AttributeValueConverter SET_CONVERTER = new AttributeValueConverter() { @Override @@ -233,16 +225,7 @@ abstract class AttributeValueConverter } else if(value instanceof String) { - String interpolated = AbstractConfiguredObject.interpolate(object, (String) value); - ObjectMapper objectMapper = new ObjectMapper(); - try - { - return objectMapper.readValue(interpolated, Set.class); - } - catch (IOException e) - { - throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a List"); - } + return Collections.unmodifiableSet(convertFromJson((String) value, object, Set.class)); } else if(value == null) { @@ -250,7 +233,7 @@ abstract class AttributeValueConverter } else { - throw new IllegalArgumentException("Cannot convert type " + value.getClass() + " to a List"); + throw new IllegalArgumentException("Cannot convert type " + value.getClass() + " to a Set"); } } }; @@ -270,16 +253,7 @@ abstract class AttributeValueConverter } else if(value instanceof String) { - String interpolated = AbstractConfiguredObject.interpolate(object, (String) value); - ObjectMapper objectMapper = new ObjectMapper(); - try - { - return objectMapper.readValue(interpolated, List.class); - } - catch (IOException e) - { - throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a Collection"); - } + return Collections.unmodifiableCollection(convertFromJson((String) value, object, Collection.class)); } else if(value == null) { @@ -287,7 +261,7 @@ abstract class AttributeValueConverter } else { - throw new IllegalArgumentException("Cannot convert type " + value.getClass() + " to a List"); + throw new IllegalArgumentException("Cannot convert type " + value.getClass() + " to a Collection"); } } }; @@ -316,24 +290,34 @@ abstract class AttributeValueConverter } else if(value instanceof String) { - String interpolated = AbstractConfiguredObject.interpolate(object, (String) value); - ObjectMapper objectMapper = new ObjectMapper(); - try - { - return objectMapper.readValue(interpolated, Map.class); - } - catch (IOException e) - { - throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a Map"); - } + return Collections.unmodifiableMap(convertFromJson((String) value, object, Map.class)); } else { throw new IllegalArgumentException("Cannot convert type " + value.getClass() + " to a Map"); } } + }; + private static T convertFromJson(final String value, final ConfiguredObject object, final Class valueType) + { + String interpolated = AbstractConfiguredObject.interpolate(object, value); + ObjectMapper objectMapper = new ObjectMapper(); + try + { + return objectMapper.readValue(interpolated, valueType); + } + catch (IOException e) + { + throw new IllegalArgumentException("Cannot convert String '" + + value + "'" + + (value.equals(interpolated) + ? "" : (" (interpolated to '" + interpolated + "')")) + + " to a " + valueType.getSimpleName()); + } + } + static AttributeValueConverter getConverter(final Class type, final Type returnType) { if(type == String.class) @@ -408,7 +392,7 @@ abstract class AttributeValueConverter { return (AttributeValueConverter) new ConfiguredObjectConverter(type); } - throw new IllegalArgumentException("Cannot create attributes of type " + type.getName()); + throw new IllegalArgumentException("Cannot create attribute converter of type " + type.getName()); } abstract T convert(Object value, final ConfiguredObject object); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java index 7382a20022..e147abd170 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java @@ -174,7 +174,7 @@ public class AbstractConfiguredObjectTest extends TestCase Map attributes = new HashMap<>(); attributes.put(ConfiguredObject.NAME, objectName); - attributes.put("context", Collections.singletonMap("myReplacement", "myValue")); + attributes.put(ConfiguredObject.CONTEXT, Collections.singletonMap("myReplacement", "myValue")); attributes.put(TestRootCategory.STRING_VALUE, contextToken); TestRootCategory object1 = _model.getObjectFactory().create(TestRootCategory.class, diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AttributeValueConverterTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AttributeValueConverterTest.java new file mode 100644 index 0000000000..8ddcb179cc --- /dev/null +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AttributeValueConverterTest.java @@ -0,0 +1,196 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.qpid.server.model; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; +import static java.util.Collections.unmodifiableSet; +import static java.util.Collections.unmodifiableList; + +import static org.apache.qpid.server.model.AttributeValueConverter.getConverter; + +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import junit.framework.TestCase; + +import org.apache.qpid.server.model.testmodel.TestModel; +import org.apache.qpid.server.model.testmodel.TestRootCategory; + +public class AttributeValueConverterTest extends TestCase +{ + private final ConfiguredObjectFactory _objectFactory = TestModel.getInstance().getObjectFactory(); + private final Map _attributes = new HashMap<>(); + private final Map _context = new HashMap<>(); + + @Override + protected void setUp() throws Exception + { + super.setUp(); + + _attributes.put(ConfiguredObject.NAME, "objectName"); + _attributes.put(ConfiguredObject.CONTEXT, _context); + } + + public void testMapConverter() + { + _context.put("simpleMap", "{\"a\" : \"b\"}"); + _context.put("mapWithInterpolatedContents", "{\"${mykey}\" : \"b\"}"); + _context.put("mykey", "mykey1"); + + ConfiguredObject object = _objectFactory.create(TestRootCategory.class, _attributes); + + AttributeValueConverter mapConverter = getConverter(Map.class, Map.class); + + Map nullMap = mapConverter.convert(null, object); + assertNull(nullMap); + + Map emptyMap = mapConverter.convert("{ }", object); + assertEquals(emptyMap(), emptyMap); + + Map map = mapConverter.convert("{\"a\" : \"b\"}", object); + assertEquals(singletonMap("a", "b"), map); + + Map mapFromInterpolatedVar = mapConverter.convert("${simpleMap}", object); + assertEquals(singletonMap("a", "b"), mapFromInterpolatedVar); + + Map mapFromInterpolatedVarWithInterpolatedContents = + mapConverter.convert("${mapWithInterpolatedContents}", object); + assertEquals(singletonMap("mykey1", "b"), mapFromInterpolatedVarWithInterpolatedContents); + + try + { + mapConverter.convert("not a map", object); + fail("Exception not thrown"); + } + catch (IllegalArgumentException e) + { + // PASS + } + } + + public void testNonGenericCollectionConverter() + { + _context.put("simpleCollection", "[\"a\", \"b\"]"); + + ConfiguredObject object = _objectFactory.create(TestRootCategory.class, _attributes); + + AttributeValueConverter collectionConverter = getConverter(Collection.class, Collection.class); + + Collection nullCollection = collectionConverter.convert(null, object); + assertNull(nullCollection); + + Collection emptyCollection = collectionConverter.convert("[ ]", object); + assertTrue(emptyCollection.isEmpty()); + + Collection collection = collectionConverter.convert("[\"a\", \"b\"]", object); + assertEquals(2, collection.size()); + assertTrue(collection.contains("a")); + assertTrue(collection.contains("b")); + + Collection collectionFromInterpolatedVar = collectionConverter.convert("${simpleCollection}", object); + assertEquals(2, collectionFromInterpolatedVar.size()); + assertTrue(collectionFromInterpolatedVar.contains("a")); + assertTrue(collectionFromInterpolatedVar.contains("b")); + + try + { + collectionConverter.convert("not a collection", object); + fail("Exception not thrown"); + } + catch (IllegalArgumentException e) + { + // PASS + } + } + + public void testNonGenericListConverter() + { + _context.put("simpleList", "[\"a\", \"b\"]"); + + ConfiguredObject object = _objectFactory.create(TestRootCategory.class, _attributes); + + AttributeValueConverter listConverter = getConverter(List.class, List.class); + + List nullList = listConverter.convert(null, object); + assertNull(nullList); + + List emptyList = listConverter.convert("[ ]", object); + assertTrue(emptyList.isEmpty()); + + List expectedList = unmodifiableList(asList("a", "b")); + + List list = listConverter.convert("[\"a\", \"b\"]", object); + assertEquals(expectedList, list); + + List listFromInterpolatedVar = listConverter.convert("${simpleList}", object); + assertEquals(expectedList, listFromInterpolatedVar); + + try + { + listConverter.convert("not a list", object); + fail("Exception not thrown"); + } + catch (IllegalArgumentException e) + { + // PASS + } + } + + public void testNonGenericSetConverter() + { + _context.put("simpleSet", "[\"a\", \"b\"]"); + + ConfiguredObject object = _objectFactory.create(TestRootCategory.class, _attributes); + + AttributeValueConverter setConverter = getConverter(Set.class, Set.class);; + + Set nullSet = setConverter.convert(null, object); + assertNull(nullSet); + + Set emptySet = setConverter.convert("[ ]", object); + assertTrue(emptySet.isEmpty()); + + Set expectedSet = unmodifiableSet(new HashSet<>(asList("a", "b"))); + + Set set = setConverter.convert("[\"a\", \"b\"]", object); + assertEquals(expectedSet, set); + + Set setFromInterpolatedVar = setConverter.convert("${simpleSet}", object); + assertEquals(expectedSet, setFromInterpolatedVar); + + try + { + setConverter.convert("not a set", object); + fail("Exception not thrown"); + } + catch (IllegalArgumentException e) + { + // PASS + } + } + +} \ No newline at end of file -- cgit v1.2.1