This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 507b8f8771319e2054755d177e5d639716a06c1a Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> AuthorDate: Thu Dec 23 11:16:29 2021 +0000 better serialization for guava inner classes when persisted and deserialization for legacy guava transformed-value maps --- .../brooklyn/util/core/xstream/XmlSerializer.java | 58 ++++++++++- .../util/core/xstream/ConverterTestFixture.java | 38 +++++++- .../core/xstream/TransformedMapConverterTest.java | 106 +++++++++++++++++++++ .../util/core/xstream/XmlSerializerTest.java | 31 ++++++ 4 files changed, 227 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/util/core/xstream/XmlSerializer.java b/core/src/main/java/org/apache/brooklyn/util/core/xstream/XmlSerializer.java index bcd608a..c54a98e 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/xstream/XmlSerializer.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/xstream/XmlSerializer.java @@ -18,6 +18,9 @@ */ package org.apache.brooklyn.util.core.xstream; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import java.io.Reader; import java.io.StringReader; import java.io.StringWriter; @@ -40,9 +43,16 @@ import com.thoughtworks.xstream.converters.extended.JavaClassConverter; import com.thoughtworks.xstream.mapper.DefaultMapper; import com.thoughtworks.xstream.mapper.Mapper; import com.thoughtworks.xstream.mapper.MapperWrapper; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.guava.Maybe; +import org.apache.brooklyn.util.javalang.Reflections; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class XmlSerializer<T> { + private static final Logger LOG = LoggerFactory.getLogger(XmlSerializer.class); + private final Map<String, String> deserializingClassRenames; protected final XStream xstream; @@ -64,7 +74,7 @@ public class XmlSerializer<T> { @Override protected MapperWrapper wrapMapper(MapperWrapper next) { MapperWrapper result = XmlSerializer.this.wrapMapperForNormalUsage(super.wrapMapper(next)); - if (mapperCustomizer!=null) { + if (mapperCustomizer != null) { result = mapperCustomizer.apply(result); } return result; @@ -73,12 +83,17 @@ public class XmlSerializer<T> { allowAllTypes(xstream); - if (loader!=null) { + if (loader != null) { xstream.setClassLoader(loader); } - + xstream.registerConverter(newCustomJavaClassConverter(), XStream.PRIORITY_NORMAL); - + addStandardHelpers(xstream); + } + + @VisibleForTesting + public static void addStandardHelpers(XStream xstream) { + // list as array list is default xstream.alias("map", Map.class, LinkedHashMap.class); xstream.alias("set", Set.class, LinkedHashSet.class); @@ -87,7 +102,7 @@ public class XmlSerializer<T> { xstream.alias("MutableMap", MutableMap.class); xstream.alias("MutableSet", MutableSet.class); xstream.alias("MutableList", MutableList.class); - + // Needs an explicit MutableSet converter! // Without it, the alias for "set" seems to interfere with the MutableSet.map field, so it gets // a null field on deserialization. @@ -103,12 +118,45 @@ public class XmlSerializer<T> { xstream.registerConverter(new EnumCaseForgivingConverter()); xstream.registerConverter(new Inet4AddressConverter()); + addStandardInnerClassHelpers(xstream); + // See ObjectWithDefaultStringImplConverter (and its usage) for why we want to auto-detect // annotations (usages of this is in the camp project, so we can't just list it statically // here unfortunately). xstream.autodetectAnnotations(true); } + @VisibleForTesting + public static void addStandardInnerClassHelpers(XStream xstream) { + Maybe<Object> valueTransformer = Reflections.getFieldValueMaybe(Maps.transformValues(MutableMap.of(), x -> x), "transformer"); + // add old aliases first, these will be deserialized but not serialized! + // not ideal that we map both 7 and 9 to the value tansformer, but okay as 7 is not used for other serialized things + // (fortunately, as otherwise hard to deserialize!) + addAliasForInnerClass(xstream, "com.google.common.collect.Maps$7", valueTransformer); + // preferred alias + addAliasForInnerClass(xstream, "com.google.common.collect.Maps._inners.valueTransformer", valueTransformer); + + Maybe<Iterable<Object>> iterableTransformer = Maybe.of(Iterables.transform(MutableSet.of(), x -> x)); + // we don't seem to serialize iterables, not surprisingly; but if we do this is useful, and if legacy are found they can be added here + // preferred alias + addAliasForInnerClass(xstream, "com.google.common.collect.Iterables._inners.transform", iterableTransformer); + } + + private static Set<String> LOGGED_ALIASES = MutableSet.of(); + + private static <T> void addAliasForInnerClass(XStream xstream, String alias, Maybe<T> object) { + if (object.isAbsent()) { + if (LOGGED_ALIASES.add(alias)) { + LOG.warn("No object found to register serialization alias for " + alias + "; ignoring"); + } + } else { + xstream.alias(alias, object.get().getClass()); + if (LOGGED_ALIASES.add(alias)) { + LOG.debug("XStream alias for "+object.get().getClass()+": "+alias+" ("+object+")"); + } + } + } + public static void allowAllTypes(final XStream xstream) { xstream.allowTypesByWildcard(new String[] { "**" diff --git a/core/src/test/java/org/apache/brooklyn/util/core/xstream/ConverterTestFixture.java b/core/src/test/java/org/apache/brooklyn/util/core/xstream/ConverterTestFixture.java index 4e4f29f..d794cba 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/xstream/ConverterTestFixture.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/xstream/ConverterTestFixture.java @@ -18,16 +18,52 @@ */ package org.apache.brooklyn.util.core.xstream; +import java.util.function.BiPredicate; +import org.apache.brooklyn.test.Asserts; import org.testng.Assert; import com.thoughtworks.xstream.XStream; public class ConverterTestFixture { - protected Object assertX(Object obj, String fmt) { + protected XStream cachedXstream = null; + + protected XStream newXstream() { XStream xstream = new XStream(); XmlSerializer.allowAllTypes(xstream); registerConverters(xstream); + return xstream; + } + + protected void useNewCachedXstream() { + cachedXstream = newXstream(); + } + + protected Object assertX(Object obj, String fmt, String ...others) { + return assertX(obj, (a,b)->{ Asserts.assertEquals(a,b); return true; }, fmt, others); + } + + protected <T> T assertX(T obj, BiPredicate<T,T> equals, String fmt, String ...others) { + XStream xstream = cachedXstream!=null ? cachedXstream : newXstream(); + + String s1 = xstream.toXML(obj); + Assert.assertEquals(s1, fmt); + T out1 = (T) xstream.fromXML(s1); + if (!equals.test(out1, obj)) Asserts.fail("Objects not equal:\n"+out1+"\n---\n"+obj); + for (String other: others) { + try { + T outO = (T)xstream.fromXML(other); + if (!equals.test(outO, obj)) Asserts.fail("Objects not equal:\n"+outO+"\n---\n"+obj); + } catch (Throwable e) { + Assert.fail("Deserializable output does not produce identical result:\n"+other, e); + } + } + return out1; + } + + protected Object assertX(Object obj, String fmt) { + XStream xstream = cachedXstream!=null ? cachedXstream : newXstream(); + String s1 = xstream.toXML(obj); Assert.assertEquals(s1, fmt); Object out = xstream.fromXML(s1); diff --git a/core/src/test/java/org/apache/brooklyn/util/core/xstream/TransformedMapConverterTest.java b/core/src/test/java/org/apache/brooklyn/util/core/xstream/TransformedMapConverterTest.java new file mode 100644 index 0000000..f47b36c --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/util/core/xstream/TransformedMapConverterTest.java @@ -0,0 +1,106 @@ +/* + * 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.brooklyn.util.core.xstream; + +import com.google.common.base.Function; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; +import com.thoughtworks.xstream.XStream; +import java.util.Map; +import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.collections.MutableSet; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.testng.annotations.Test; + +public class TransformedMapConverterTest extends ConverterTestFixture { + + @Override + protected void registerConverters(XStream xstream) { + super.registerConverters(xstream); + + xstream.alias("MutableMap", MutableMap.class); + xstream.registerConverter(new StringKeyMapConverter(xstream.getMapper()), /* priority */ 10); + + xstream.alias("MutableSet", MutableSet.class); + xstream.registerConverter(new MutableSetConverter(xstream.getMapper())); + } + + private static class AddOne implements Function<Integer, Integer> { + @Override + public @Nullable Integer apply(@Nullable Integer input) { + return input+1; + } + } + + + @Test + public void testGuavaPortableSerialization_MapTranformValues() { + useNewCachedXstream(); + XmlSerializer.addStandardInnerClassHelpers(cachedXstream); + + Function<String,String> xml = clazz -> "<com.google.common.collect.Maps_-TransformedEntriesMap>\n" + + " <fromMap class=\"MutableMap\">\n" + + " <a type=\"int\">1</a>\n" + + " </fromMap>\n" + + " <transformer class=\""+clazz+"\">\n" + + " <val_-function class=\"org.apache.brooklyn.util.core.xstream.TransformedMapConverterTest$AddOne\"/>\n" + + " </transformer>\n" + + "</com.google.common.collect.Maps_-TransformedEntriesMap>"; + + Map<String, Integer> out = Maps.transformValues(MutableMap.of("a", 1), new AddOne()); + String preferred = xml.apply("com.google.common.collect.Maps._inners.valueTransformer"); + + // IMPORTANT! for backwards compatibility + String guava18 = xml.apply("com.google.common.collect.Maps$7"); + + // NOT IMPORTANT - for reference; should not be written, so if guava changes this can be changed + String guava27 = xml.apply("com.google.common.collect.Maps$9"); + + assertX(out, preferred, guava18, guava27); + } + + @Test + public void testGuavaPortableSerialization_IterablesTranform() { + useNewCachedXstream(); + XmlSerializer.addStandardInnerClassHelpers(cachedXstream); + + Iterable<Integer> out = Iterables.transform(MutableSet.of(1), new AddOne()); + Function<String,String> xml = clazz -> "<"+clazz+">\n" + + " <iterableDelegate class=\"com.google.common.base.Absent\"/>\n" + + " <val_-fromIterable class=\"MutableSet\">\n" + + " <int>1</int>\n" + + " </val_-fromIterable>\n" + + " <val_-function class=\"org.apache.brooklyn.util.core.xstream.TransformedMapConverterTest$AddOne\"/>\n" + + "</"+clazz+">"; + + String preferred = xml.apply("com.google.common.collect.Iterables.__inners.transform"); + + // NOT IMPORTANT - for reference; should not be written, so if guava changes this can be changed + // (note $ becomes_- when used as tag, since $ not allowed there) + String guava27 = xml.apply("com.google.common.collect.Iterables_-5"); + + assertX(out, (i1,i2) -> { + Asserts.assertEquals(MutableList.copyOf(i1), MutableList.copyOf(i2)); + return true; + }, preferred, guava27); + } + +} diff --git a/core/src/test/java/org/apache/brooklyn/util/core/xstream/XmlSerializerTest.java b/core/src/test/java/org/apache/brooklyn/util/core/xstream/XmlSerializerTest.java index 098f6ee..52dbd14 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/xstream/XmlSerializerTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/xstream/XmlSerializerTest.java @@ -27,8 +27,13 @@ import java.io.Serializable; import java.util.Arrays; import java.util.function.Supplier; import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.xstream.LambdaPreventionMapper.LambdaPersistenceMode; +import org.omg.CORBA.ObjectHolder; +import org.omg.CORBA.StringHolder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import static org.testng.Assert.assertEquals; @@ -59,6 +64,32 @@ public class XmlSerializerTest { assertSerializeAndDeserialize(new IntHolder(123)); assertSerializeAndDeserialize(new IntegerHolder(123)); } + + @Test + public void testXmlOutput() throws Exception { + Asserts.assertEquals(serializer.toString(MutableMap.of("a", 1)), + "<MutableMap>\n" + + " <a type=\"int\">1</a>\n" + + "</MutableMap>"); + + Asserts.assertEquals(serializer.toString(MutableSet.of(1)), + "<MutableSet>\n" + + " <int>1</int>\n" + + "</MutableSet>"); + + // no nice serializer for this yet + Asserts.assertEquals(serializer.toString(MutableList.of(1)), + "<MutableList serialization=\"custom\">\n" + + " <unserializable-parents/>\n" + + " <list>\n" + + " <default>\n" + + " <size>1</size>\n" + + " </default>\n" + + " <int>1</int>\n" + + " <int>1</int>\n" + + " </list>\n" + + "</MutableList>"); + } @Test public void testAnnotationsExample() throws Exception {