Fix custom converter params not being thread-safe

core/src/main/java/com/github/dozermapper/core/ConfigurableCustomConverter.java

@@ -22,7 +22,7 @@
  * class B types. When a custom converter is specified for a class A and class B combination, Dozer will invoke the
  * custom converter to perform the data mapping instead of the standard mapping logic.
  * <p>
- * This interface also gives you the opportunity to send a configuration parameter to it
+ * This interface also gives you the opportunity to send a configuration parameter to it.
  * <p>
  * <a
  * href="https://dozermapper.github.io/gitbook/documentation/customconverter.html">
@@ -31,7 +31,10 @@
 public interface ConfigurableCustomConverter extends CustomConverter {
 
     /**
-     * Setter for converter static parameter. Method is guaranteed to be called before the first execution.
+     * Setter for converter static parameter. Method is guaranteed to be called before each mapping call.
+     * <p>
+     * Dozer may reuse custom converter instances across different threads. The implementing class is responsible for
+     * storing the parameter in a thread-safe manner, for example using a {@link ThreadLocal}.
      *
      * @param parameter - converter instance, which is injected via custom-converter-param attribute
      */

core/src/main/java/com/github/dozermapper/core/DozerConverter.java

@@ -22,7 +22,7 @@
  */
 public abstract class DozerConverter<A, B> implements ConfigurableCustomConverter {
 
-    private String parameter;
+    private ThreadLocal<String> parameter = new ThreadLocal<>();
     private Class<A> prototypeA;
     private Class<B> prototypeB;
 
@@ -117,7 +117,7 @@ public A convertFrom(B source) {
      * @param parameter configured parameter value
      */
     public void setParameter(String parameter) {
-        this.parameter = parameter;
+        this.parameter.set(parameter);
     }
 
     /**
@@ -128,10 +128,11 @@ public void setParameter(String parameter) {
      * @throws IllegalStateException if parameter has not been set yet.
      */
     public String getParameter() {
-        if (parameter == null) {
+        String theParameter = parameter.get();
+        if (theParameter == null) {
             throw new IllegalStateException("Custom Converter Parameter has not yet been set!");
         }
-        return parameter;
+        return theParameter;
     }
 
 }

core/src/main/java/com/github/dozermapper/core/MappingProcessor.java

@@ -65,7 +65,6 @@
 import com.github.dozermapper.core.util.MappingUtils;
 import com.github.dozermapper.core.util.MappingValidator;
 import com.github.dozermapper.core.util.ReflectionUtils;
-
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -1017,34 +1016,23 @@ private Object mapUsingCustomConverterInstance(CustomConverter converterInstance
             ((MapperAware)converterInstance).setMapper(this);
         }
 
-        // TODO Remove code duplication
         Object result;
         if (converterInstance instanceof ConfigurableCustomConverter) {
-            ConfigurableCustomConverter theConverter = (ConfigurableCustomConverter)converterInstance;
+            ConfigurableCustomConverter theConverter = (ConfigurableCustomConverter) converterInstance;
 
             // Converter could be not configured for this particular case
             if (fieldMap != null) {
                 String param = fieldMap.getCustomConverterParam();
                 theConverter.setParameter(param);
             }
-
-            // if this is a top level mapping the destObj is the highest level
-            // mapping...not a recursive mapping
-            if (topLevel) {
-                result = theConverter.convert(existingDestFieldValue, srcFieldValue, destFieldClass, srcFieldClass);
-            } else {
-                Object existingValue = getExistingValue(fieldMap, existingDestFieldValue, destFieldClass);
-                result = theConverter.convert(existingValue, srcFieldValue, destFieldClass, srcFieldClass);
-            }
+        }
+        // if this is a top level mapping the destObj is the highest level
+        // mapping...not a recursive mapping
+        if (topLevel) {
+            result = converterInstance.convert(existingDestFieldValue, srcFieldValue, destFieldClass, srcFieldClass);
         } else {
-            // if this is a top level mapping the destObj is the highest level
-            // mapping...not a recursive mapping
-            if (topLevel) {
-                result = converterInstance.convert(existingDestFieldValue, srcFieldValue, destFieldClass, srcFieldClass);
-            } else {
-                Object existingValue = getExistingValue(fieldMap, existingDestFieldValue, destFieldClass);
-                result = converterInstance.convert(existingValue, srcFieldValue, destFieldClass, srcFieldClass);
-            }
+            Object existingValue = getExistingValue(fieldMap, existingDestFieldValue, destFieldClass);
+            result = converterInstance.convert(existingValue, srcFieldValue, destFieldClass, srcFieldClass);
         }
 
         return result;

core/src/test/java/com/github/dozermapper/core/converters/CustomConverterThreadSafetyTest.java

@@ -0,0 +1,111 @@
+/*
+ * Copyright 2005-2019 Dozer Project
+ *
+ * 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 com.github.dozermapper.core.converters;
+
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.stream.Stream;
+
+import com.github.dozermapper.core.AbstractDozerTest;
+import com.github.dozermapper.core.DozerBeanMapperBuilder;
+import com.github.dozermapper.core.DozerConverter;
+import com.github.dozermapper.core.Mapper;
+import com.github.dozermapper.core.loader.api.BeanMappingBuilder;
+import org.junit.Before;
+import org.junit.Test;
+
+import static com.github.dozermapper.core.loader.api.FieldsMappingOptions.customConverter;
+import static org.junit.Assert.assertEquals;
+
+public class CustomConverterThreadSafetyTest extends AbstractDozerTest {
+
+    private Mapper mapper;
+
+    @Before
+    public void semtUp() {
+        BeanMappingBuilder builder = new BeanMappingBuilder() {
+            protected void configure() {
+                mapping(Bean.class, Bean.class)
+                        .fields("property1", "property1",
+                                customConverter(ConstantConverter.class, "param1")
+                        )
+                        .fields("property2", "property2",
+                                customConverter(ConstantConverter.class, "param2")
+                        );
+            }
+        };
+
+        mapper = DozerBeanMapperBuilder.create()
+                .withCustomConverter(new ConstantConverter())
+                .withMappingBuilder(builder)
+                .build();
+    }
+
+    @Test
+    public void testParameterThreadSafety() throws ExecutionException, InterruptedException {
+        ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2);
+        final Bean input = new Bean();
+        executorService.submit(() -> Stream.generate(() -> mapper.map(input, Bean.class)).limit(100).parallel())
+                .get()
+                .forEach(output -> {
+                    assertEquals("param1", output.property1);
+                    assertEquals("param2", output.property2);
+                });
+    }
+
+    private static class Bean {
+
+        private String property1;
+        private String property2;
+
+        public String getProperty1() {
+            return property1;
+        }
+
+        public void setProperty1(String property1) {
+            this.property1 = property1;
+        }
+
+        public String getProperty2() {
+            return property2;
+        }
+
+        public void setProperty2(String property2) {
+            this.property2 = property2;
+        }
+
+    }
+
+    private static class ConstantConverter extends DozerConverter<String, String> {
+
+
+        ConstantConverter() {
+            super(String.class, String.class);
+        }
+
+        @Override
+        public String convertTo(String s, String s2) {
+            return getParameter();
+        }
+
+        @Override
+        public String convertFrom(String s, String s2) {
+            return getParameter();
+        }
+
+    }
+}