diff --git a/CHANGELOG.md b/CHANGELOG.md index 437ab8c2..c2072ba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased +### Fixed + +- Issue [#196](https://github.com/42BV/beanmapper/issues/196) **Calls to the logger should not contain a call to String.formatted(Object...)**; Fixed by removing any formatting from performance logging. Trace-logging should keep the level of detail provided by the formatting. +- Issue [#197](https://github.com/42BV/beanmapper/issues/197) **Implement caching for Unproxy results.**; Created UnproxyResultStore, allowing for thread-safe caching and retrieval of unproxied classes. + ## [4.1.3] - 2024-03-27 ### Fixed diff --git a/src/main/java/io/beanmapper/config/CollectionHandlerStore.java b/src/main/java/io/beanmapper/config/CollectionHandlerStore.java index cbe3e247..5d51b090 100644 --- a/src/main/java/io/beanmapper/config/CollectionHandlerStore.java +++ b/src/main/java/io/beanmapper/config/CollectionHandlerStore.java @@ -5,6 +5,7 @@ import io.beanmapper.core.collections.CollectionHandler; import io.beanmapper.core.unproxy.BeanUnproxy; +import io.beanmapper.core.unproxy.UnproxyResultStore; public class CollectionHandlerStore { @@ -28,7 +29,8 @@ public CollectionHandler getCollectionHandlerFor(Class clazz, BeanUnproxy bea return collectionHandler; } // Unproxy the collection class in case it was anonymous and try again - return getCollectionHandlerFor(beanUnproxy.unproxy(clazz)); + Class unproxiedClass = UnproxyResultStore.getInstance().getOrComputeUnproxyResult(clazz, beanUnproxy); + return getCollectionHandlerFor(unproxiedClass); } private CollectionHandler getCollectionHandlerFor(Class clazz) { diff --git a/src/main/java/io/beanmapper/config/OverrideField.java b/src/main/java/io/beanmapper/config/OverrideField.java index 502d641f..0196b839 100644 --- a/src/main/java/io/beanmapper/config/OverrideField.java +++ b/src/main/java/io/beanmapper/config/OverrideField.java @@ -6,6 +6,7 @@ public class OverrideField { + private static final String LOGGING_STRING = "OverrideField#get(void) -> OverrideField#get(void)"; private final Supplier supplier; private boolean block = false; @@ -34,7 +35,7 @@ public T get() { return null; } if (this.value == null) { - this.value = BeanMapperPerformanceLogger.runTimed("%s#%s -> %s#%s".formatted(this.getClass().getSimpleName(), "get(void)", this.getClass().getSimpleName(), "get(void)"), this.supplier); + this.value = BeanMapperPerformanceLogger.runTimed(LOGGING_STRING, this.supplier); } return value; } diff --git a/src/main/java/io/beanmapper/config/StrictMappingProperties.java b/src/main/java/io/beanmapper/config/StrictMappingProperties.java index d6014960..d264b5c4 100644 --- a/src/main/java/io/beanmapper/config/StrictMappingProperties.java +++ b/src/main/java/io/beanmapper/config/StrictMappingProperties.java @@ -2,12 +2,15 @@ import io.beanmapper.core.unproxy.BeanUnproxy; import io.beanmapper.core.unproxy.SkippingBeanUnproxy; +import io.beanmapper.core.unproxy.UnproxyResultStore; import io.beanmapper.utils.BeanMapperPerformanceLogger; import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName; public class StrictMappingProperties { + private static final String LOGGING_STRING = "StrictMappingProperties#createBeanPair(Class, Class) -> BeanUnproxy#unproxy(Class)"; + private BeanUnproxy beanUnproxy; /** @@ -82,9 +85,10 @@ public void setApplyStrictMappingConvention(boolean applyStrictMappingConvention } public BeanPair createBeanPair(Class sourceClass, Class targetClass) { - BeanPair beanPair = BeanMapperPerformanceLogger.runTimed("%s#%s".formatted(this.getClass().getSimpleName(), "createBeanPair(Class, Class) -> BeanUnproxy#unproxy(Class)"), () -> { - Class unproxiedSource = beanUnproxy.unproxy(sourceClass); - Class unproxiedTarget = beanUnproxy.unproxy(targetClass); + UnproxyResultStore unproxyResultStore = UnproxyResultStore.getInstance(); + BeanPair beanPair = BeanMapperPerformanceLogger.runTimed(LOGGING_STRING, () -> { + Class unproxiedSource = unproxyResultStore.getOrComputeUnproxyResult(sourceClass, beanUnproxy); + Class unproxiedTarget = unproxyResultStore.getOrComputeUnproxyResult(targetClass, beanUnproxy); return new BeanPair(unproxiedSource, unproxiedTarget); }); if (!isApplyStrictMappingConvention()) { diff --git a/src/main/java/io/beanmapper/core/collections/AbstractCollectionHandler.java b/src/main/java/io/beanmapper/core/collections/AbstractCollectionHandler.java index 56b6bc64..54076258 100644 --- a/src/main/java/io/beanmapper/core/collections/AbstractCollectionHandler.java +++ b/src/main/java/io/beanmapper/core/collections/AbstractCollectionHandler.java @@ -10,6 +10,8 @@ public abstract class AbstractCollectionHandler implements CollectionHandler { + private static final String LOGGING_STRING = "%s#mapItem(BeanMapper, Class, Object) -> BeanMapper#map(Object)"; + private final Class type; private final DefaultBeanInitializer beanInitializer = new DefaultBeanInitializer(); @@ -33,14 +35,12 @@ public Object mapItem( BeanMapper beanMapper, Class collectionElementClass, Object source) { - return BeanMapperPerformanceLogger.runTimed("%s#%s" - .formatted(this.getClass().getSimpleName(), "mapItem(BeanMapper, Class, Object) -> BeanMapper#map(Object)"), - () -> beanMapper.wrap() + return BeanMapperPerformanceLogger.runTimed(() -> beanMapper.wrap() .setTargetClass(collectionElementClass) .setCollectionClass(null) .setConverterChoosable(true) .build() - .map(source)); + .map(source), LOGGING_STRING, getClass().getSimpleName()); } @Override @@ -91,4 +91,4 @@ public int getGenericParameterIndex() { return 0; } -} \ No newline at end of file +} diff --git a/src/main/java/io/beanmapper/core/converter/collections/CollectionConverter.java b/src/main/java/io/beanmapper/core/converter/collections/CollectionConverter.java index a8609d54..efd2cda4 100644 --- a/src/main/java/io/beanmapper/core/converter/collections/CollectionConverter.java +++ b/src/main/java/io/beanmapper/core/converter/collections/CollectionConverter.java @@ -8,6 +8,8 @@ public class CollectionConverter implements BeanConverter { + private static final String LOGGING_STRING = "%s#convert(BeanMapper, Object, Class, BeanPropertyMatch) -> BeanMapper#map(Object)"; + private final CollectionHandler collectionHandler; public CollectionConverter(CollectionHandler collectionHandler) { @@ -25,9 +27,7 @@ public U convert( return targetClass.cast(source); } - return BeanMapperPerformanceLogger.runTimed("%s#%s" - .formatted(this.getClass().getSimpleName(), "convert(BeanMapper, Object, Class, BeanPropertyMatch) -> BeanMapper#map(Object)"), - () -> beanMapper.wrap() + return BeanMapperPerformanceLogger.runTimed(() -> beanMapper.wrap() .setCollectionClass(collectionHandler.getType()) .setCollectionUsage(beanPropertyMatch.getCollectionInstructions().getBeanCollectionUsage()) .setPreferredCollectionClass(beanPropertyMatch.getCollectionInstructions().getPreferredCollectionClass().getAnnotationClass()) @@ -36,7 +36,7 @@ public U convert( .setTarget(beanPropertyMatch.getTargetObject()) .setUseNullValue() .build() - .map(source)); + .map(source), LOGGING_STRING, getClass().getSimpleName()); } @Override diff --git a/src/main/java/io/beanmapper/core/unproxy/UnproxyResultStore.java b/src/main/java/io/beanmapper/core/unproxy/UnproxyResultStore.java new file mode 100644 index 00000000..5aa0bd51 --- /dev/null +++ b/src/main/java/io/beanmapper/core/unproxy/UnproxyResultStore.java @@ -0,0 +1,37 @@ +package io.beanmapper.core.unproxy; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Singleton responsible for storing the results of unproxy-results. Allows for fast and thread-safe retrieval of results. + */ +public final class UnproxyResultStore { + + private static UnproxyResultStore INSTANCE; + + private final Map, Class> unproxyResultClassStore; + + private UnproxyResultStore() { + unproxyResultClassStore = new ConcurrentHashMap<>(); + } + + /** + * Gets or computes the result of an unproxy-operation. + * + * @param source The class for which the unproxy-result needs to be retrieved or computed. + * @param unproxy The BeanUnproxy that will be used to compute an unproxy-result if none exist for the given source-class. + * @return The unproxied class. + */ + public Class getOrComputeUnproxyResult(Class source, BeanUnproxy unproxy) { + return unproxyResultClassStore.computeIfAbsent(source, unproxy::unproxy); + } + + public static synchronized UnproxyResultStore getInstance() { + if (INSTANCE == null) { + INSTANCE = new UnproxyResultStore(); + } + return INSTANCE; + } + +} diff --git a/src/main/java/io/beanmapper/strategy/AbstractMapStrategy.java b/src/main/java/io/beanmapper/strategy/AbstractMapStrategy.java index 8b672feb..200ab5ae 100644 --- a/src/main/java/io/beanmapper/strategy/AbstractMapStrategy.java +++ b/src/main/java/io/beanmapper/strategy/AbstractMapStrategy.java @@ -10,6 +10,8 @@ import io.beanmapper.core.BeanMatch; import io.beanmapper.core.BeanPropertyMatch; import io.beanmapper.core.converter.BeanConverter; +import io.beanmapper.core.unproxy.BeanUnproxy; +import io.beanmapper.core.unproxy.UnproxyResultStore; import io.beanmapper.exceptions.BeanConversionException; import io.beanmapper.exceptions.BeanPropertyNoMatchException; import io.beanmapper.utils.BeanMapperTraceLogger; @@ -65,8 +67,9 @@ public ConstructorArguments getConstructorArguments(S source, BeanMatch bean } public BeanMatch getBeanMatch(Class sourceClazz, Class targetClazz) { - Class sourceClass = getConfiguration().getBeanUnproxy().unproxy(sourceClazz); - Class targetClass = getConfiguration().getBeanUnproxy().unproxy(targetClazz); + BeanUnproxy unproxy = getConfiguration().getBeanUnproxy(); + Class sourceClass = UnproxyResultStore.getInstance().getOrComputeUnproxyResult(sourceClazz, unproxy); + Class targetClass = UnproxyResultStore.getInstance().getOrComputeUnproxyResult(targetClazz, unproxy); return getConfiguration().getBeanMatchStore().getBeanMatch( configuration.getStrictMappingProperties().createBeanPair(sourceClass, targetClass) ); @@ -131,7 +134,8 @@ private void dealWithMappableNestedClass(BeanPropertyMatch beanPropertyMatch) { */ public Object convert(Object value, Class targetClass, BeanPropertyMatch beanPropertyMatch) { - Class valueClass = getConfiguration().getBeanUnproxy().unproxy(beanPropertyMatch.getSourceClass()); + BeanUnproxy unproxy = getConfiguration().getBeanUnproxy(); + Class valueClass = UnproxyResultStore.getInstance().getOrComputeUnproxyResult(beanPropertyMatch.getSourceClass(), unproxy); BeanConverter converter = getConverterOptional(valueClass, targetClass); if (converter != null) { diff --git a/src/main/java/io/beanmapper/strategy/MapToClassStrategy.java b/src/main/java/io/beanmapper/strategy/MapToClassStrategy.java index 06f45bd4..b95e3887 100644 --- a/src/main/java/io/beanmapper/strategy/MapToClassStrategy.java +++ b/src/main/java/io/beanmapper/strategy/MapToClassStrategy.java @@ -4,6 +4,8 @@ import io.beanmapper.config.Configuration; import io.beanmapper.core.BeanMatch; import io.beanmapper.core.converter.BeanConverter; +import io.beanmapper.core.unproxy.BeanUnproxy; +import io.beanmapper.core.unproxy.UnproxyResultStore; import io.beanmapper.utils.BeanMapperTraceLogger; public class MapToClassStrategy extends MapToInstanceStrategy { @@ -17,7 +19,8 @@ public T map(S source) { Class targetClass = getConfiguration().getTargetClass(); if (getConfiguration().isConverterChoosable() || source instanceof Record) { - Class valueClass = getConfiguration().getBeanUnproxy().unproxy(source.getClass()); + BeanUnproxy unproxy = getConfiguration().getBeanUnproxy(); + Class valueClass = UnproxyResultStore.getInstance().getOrComputeUnproxyResult(source.getClass(), unproxy); BeanConverter converter = getConverterOptional(valueClass, targetClass); if (converter != null) { BeanMapperTraceLogger.log("Converter called for source of class {}, while mapping to class {}\t{}->", source.getClass(), targetClass, diff --git a/src/main/java/io/beanmapper/strategy/MapToDynamicClassStrategy.java b/src/main/java/io/beanmapper/strategy/MapToDynamicClassStrategy.java index bf7f6ab3..64466061 100644 --- a/src/main/java/io/beanmapper/strategy/MapToDynamicClassStrategy.java +++ b/src/main/java/io/beanmapper/strategy/MapToDynamicClassStrategy.java @@ -9,6 +9,8 @@ public class MapToDynamicClassStrategy extends AbstractMapStrategy { + private static final String LOGGING_STRING = "Recursively calling BeanMapper#map(Object), to map source of type %s, to type %s."; + public MapToDynamicClassStrategy(BeanMapper beanMapper, Configuration configuration) { super(beanMapper, configuration); } @@ -49,25 +51,21 @@ public T downsizeSource(S source, List downsizeSourceFields) { Class targetClass = getConfiguration().getTargetClass(); Object target = getConfiguration().getTarget(); - Object dynSource = BeanMapperPerformanceLogger.runTimed("Recursively calling BeanMapper#map(Object), to map source of type %s, to target of type %s." - .formatted(source.getClass().getCanonicalName(), dynamicClass.getCanonicalName()), - () -> getBeanMapper() + Object dynSource = BeanMapperPerformanceLogger.runTimed(() -> getBeanMapper() .wrap() .downsizeSource(null) .setTarget(target) .setTargetClass(dynamicClass) .build() - .map(source)); + .map(source), LOGGING_STRING, source.getClass().getSimpleName(), targetClass != null ? targetClass.getSimpleName() : null); - return BeanMapperPerformanceLogger.runTimed("Recursively calling BeanMapper#map(Object), to map source of type %s, to type %s." - .formatted(dynSource.getClass().getCanonicalName(), targetClass != null ? targetClass.getCanonicalName() : "null"), - () -> getBeanMapper() + return BeanMapperPerformanceLogger.runTimed(() -> getBeanMapper() .wrap() .downsizeSource(null) .setTarget(target) .setTargetClass(targetClass) .build() - .map(dynSource)); + .map(dynSource), LOGGING_STRING, dynSource.getClass().getSimpleName(), targetClass != null ? targetClass.getSimpleName() : null); } public T downsizeTarget(S source, List downsizeTargetFields) { @@ -76,13 +74,12 @@ public T downsizeTarget(S source, List downsizeTargetFields) { downsizeTargetFields, getConfiguration().getStrictMappingProperties()); Class collectionClass = getBeanMapper().getConfiguration().getCollectionClass(); - return BeanMapperPerformanceLogger.runTimed("Recursively calling BeanMapper#map(Object), to map source of type %s, to type %s." - .formatted(source.getClass().getCanonicalName(), dynamicClass.getCanonicalName()), () -> getBeanMapper() + return BeanMapperPerformanceLogger.runTimed(() -> getBeanMapper() .wrap() .downsizeTarget(null) .setCollectionClass(collectionClass) .setTargetClass(dynamicClass) .build() - .map(source)); + .map(source), LOGGING_STRING, source.getClass().getSimpleName(), dynamicClass.getSimpleName()); } } diff --git a/src/main/java/io/beanmapper/utils/BeanMapperPerformanceLogger.java b/src/main/java/io/beanmapper/utils/BeanMapperPerformanceLogger.java index 9f890229..069086b6 100644 --- a/src/main/java/io/beanmapper/utils/BeanMapperPerformanceLogger.java +++ b/src/main/java/io/beanmapper/utils/BeanMapperPerformanceLogger.java @@ -25,6 +25,14 @@ public static T runTimed(String taskName, Supplier task) { return result; } + public static T runTimed(Supplier task, String unformattedTaskName, Object... messageArguments) { + if (log.isDebugEnabled()) { + String taskName = unformattedTaskName.formatted(messageArguments); + return runTimed(taskName, task); + } + return task.get(); + } + private static class Stopwatch { private final Instant started;