Skip to content

Commit

Permalink
Bug #190 Enum with abstract method
Browse files Browse the repository at this point in the history
Since a later version of Java, when you call getCanonicalName on
an enum with abstract methods, it will return a null value. This
messes up several routines in BeanMapper.

The method to get the canonical name has been abstracted into a
utility class that can deal with the null value by calling a
fallback function that looks up the canonical name of the superclass.
This solves the problem at hand.

All methods that directly call getCanonicalName now call the utility
class instead.
  • Loading branch information
robert-bor authored and marcus-talbot42 committed Mar 27, 2024
1 parent 806b09d commit eaa8105
Show file tree
Hide file tree
Showing 15 changed files with 95 additions and 36 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,4 @@ Find the rest of the documentation on [beanmapper.io](http://beanmapper.io).
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.

Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.beanmapper.annotations;

import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName;

/**
* Determines how to deal with the target collection. If the value is set to CONSTRUCT, it
* will also be recreated, even if it already exists. If REUSE is set, it will attempt to
Expand Down Expand Up @@ -34,7 +36,7 @@ public boolean mustClear() {

public boolean mustConstruct(Object targetCollection) {
if (targetCollection != null &&
targetCollection.getClass().getCanonicalName().startsWith("java.util.Collections.")) {
determineCanonicalClassName(targetCollection.getClass()).startsWith("java.util.Collections.")) {
return true;
}
return this.construct || targetCollection == null;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/io/beanmapper/config/OverrideField.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,4 @@ public T get() {
}
return value;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import io.beanmapper.core.unproxy.SkippingBeanUnproxy;
import io.beanmapper.utils.BeanMapperPerformanceLogger;

import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName;

public class StrictMappingProperties {

private BeanUnproxy beanUnproxy;
Expand Down Expand Up @@ -89,11 +91,11 @@ public BeanPair createBeanPair(Class<?> sourceClass, Class<?> targetClass) {
return beanPair;
}
if (strictSourceSuffix != null &&
beanPair.getSourceClass().getCanonicalName().endsWith(strictSourceSuffix)) {
determineCanonicalClassName(sourceClass).endsWith(strictSourceSuffix)) {
beanPair = beanPair.withStrictSource();
}
if (strictTargetSuffix != null &&
beanPair.getTargetClass().getCanonicalName().endsWith(strictTargetSuffix)) {
determineCanonicalClassName(targetClass).endsWith(strictTargetSuffix)) {
beanPair = beanPair.withStrictTarget();
}
return beanPair;
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/io/beanmapper/core/BeanMatch.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName;

public class BeanMatch {

private static final Logger log = LoggerFactory.getLogger(BeanMatch.class);
Expand Down Expand Up @@ -91,8 +93,8 @@ private List<BeanProperty> validateMappingRequirements(Map<String, BeanProperty>
BeanProperty targetBeanProperty = matchedField.targetBeanProperty();
if (sourceBeanProperty == null || targetBeanProperty == null) {
missingMatches.add(sourceBeanProperty == null ?
targetBeanProperty :
sourceBeanProperty);
targetBeanProperty :
sourceBeanProperty);
}
}
return missingMatches;
Expand All @@ -111,8 +113,9 @@ private void checkForMandatoryUnmatchedNodes(String side, Class<?> containingCla
for (Map.Entry<String, BeanProperty> entry : nodes.entrySet()) {
BeanProperty currentField = entry.getValue();
if (currentField.isUnmatched()) {
log.error("{} {} has no match for property {}", side, containingClass.getCanonicalName() != null ? containingClass.getCanonicalName() : containingClass.getDeclaringClass().getCanonicalName(), entry.getKey());
throw new BeanNoSuchPropertyException(side + " " + containingClass.getCanonicalName() + " has no match for property " + entry.getKey());
String canonicalClassName = determineCanonicalClassName(containingClass);
log.error("{} {} has no match for property {}", side, canonicalClassName, entry.getKey());
throw new BeanNoSuchPropertyException(side + " " + canonicalClassName + " has no match for property " + entry.getKey());
}
}
}
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/io/beanmapper/core/BeanMatchStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static io.beanmapper.core.converter.collections.CollectionElementType.EMPTY_COLLECTION_ELEMENT_TYPE;
import static io.beanmapper.core.converter.collections.CollectionElementType.derived;
import static io.beanmapper.core.converter.collections.CollectionElementType.set;
import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName;

import java.beans.IntrospectionException;
import java.beans.Introspector;
Expand Down Expand Up @@ -74,29 +75,29 @@ public BeanMatch getBeanMatch(BeanPair beanPair) {
}

private boolean containsKeyForTargetClass(Map<String, BeanMatch> targetsForSource, BeanPair beanPair) {
return targetsForSource.containsKey(beanPair.getTargetClass().getCanonicalName());
return targetsForSource.containsKey(determineCanonicalClassName(beanPair.getTargetClass()));
}

public BeanMatch addBeanMatch(BeanMatch beanMatch) {
Map<String, BeanMatch> targetsForSource = getTargetsForSource(beanMatch.getSourceClass());
if (targetsForSource == null) {
targetsForSource = new TreeMap<>();
store.put(beanMatch.getSourceClass().getCanonicalName(), targetsForSource);
store.put(determineCanonicalClassName(beanMatch.getSourceClass()), targetsForSource);
}
storeTarget(targetsForSource, beanMatch.getTargetClass(), beanMatch);
return beanMatch;
}

private Map<String, BeanMatch> getTargetsForSource(Class<?> sourceClass) {
return store.get(sourceClass.getCanonicalName());
return store.get(determineCanonicalClassName(sourceClass));
}

private BeanMatch getTarget(Map<String, BeanMatch> targetsForSource, Class<?> target) {
return targetsForSource.get(target.getCanonicalName());
return targetsForSource.get(determineCanonicalClassName(target));
}

private void storeTarget(Map<String, BeanMatch> targetsForSource, Class<?> target, BeanMatch beanMatch) {
targetsForSource.put(target.getCanonicalName(), beanMatch);
targetsForSource.put(determineCanonicalClassName(target), beanMatch);
}

private BeanMatch determineBeanMatch(BeanPair beanPair) {
Expand Down Expand Up @@ -283,11 +284,9 @@ private BeanPropertyWrapper dealWithBeanProperty(BeanPropertyMatchupDirection ma
new BeanPropertyCreator(matchupDirection.getInverse(), otherType, wrapper.getName())
.determineNodesForPath());
} catch (BeanNoSuchPropertyException err) {

BeanMapperTraceLogger.log("""
BeanNoSuchPropertyException thrown by BeanMatchStore#dealWithBeanProperty(BeanPropertyMatchupDirection, Map<String, BeanProperty>, Class, PropertyAccessor), for {}.
{}""", wrapper.getName(), err.getMessage());

}
}
return wrapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static io.beanmapper.utils.CanonicalClassName.determineCanonicalClassName;

public class BeanStrictMappingRequirementsException extends RuntimeException {

private static final Logger log = LoggerFactory.getLogger(BeanStrictMappingRequirementsException.class);
Expand All @@ -30,9 +32,9 @@ private void logErrors(List<BeanMatchValidationMessage> validationMessages) {
log.error("""
Missing matching properties for source [{}] {} > target [{}] {} for fields:
""",
validationMessage.getSourceClass().getCanonicalName(),
determineCanonicalClassName(validationMessage.getSourceClass()),
(validationMessage.isSourceStrict() ? "*" : ""),
validationMessage.getTargetClass().getCanonicalName(),
determineCanonicalClassName(validationMessage.getTargetClass()),
(validationMessage.isTargetStrict() ? "*" : ""));

for (BeanProperty field : validationMessage.getFields()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ public int getGenericParameterIndex() {
return 0;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,4 @@ private Object[] mapParameterizedArguments(Type[] constructorParameterTypes, Obj
}
return mappedArguments;
}

}
8 changes: 4 additions & 4 deletions src/main/java/io/beanmapper/strategy/MapToRecordStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public <S, T> T map(final S source) {
* present.
*
* @param sourceClass The class of the source-object.
* @param <S> The type of the sourceClass.
* @return A Map containing the fields of the source-class, mapped by the name of the field, or the value of an available BeanAlias.
* @param <S> The type of the sourceClass.
*/
private <S> Map<String, Field> getFieldsOfClass(final Class<S> sourceClass) {
return Arrays.stream(sourceClass.getDeclaredFields())
Expand Down Expand Up @@ -176,7 +176,7 @@ private Object[] getConstructorArgumentsMappedToCorrectTargetType(final Paramete
} else {
var parameterType = parameters[i].getType();
if (Collection.class.isAssignableFrom(parameterType) || Optional.class.isAssignableFrom(parameterType)
|| Map.class.isAssignableFrom(parameterType)) {
|| Map.class.isAssignableFrom(parameterType)) {
arguments[i] = this.getBeanMapper().map(values.get(i), (ParameterizedType) parameters[i].getParameterizedType());
} else {
arguments[i] = values.get(i) != null && parameters[i].getType().equals(values.get(i).getClass()) ?
Expand Down Expand Up @@ -232,8 +232,8 @@ private <T> Constructor<?> getSuitableConstructor(final Map<String, Field> sourc
var recordConstruct = (BeanRecordConstruct) canonicalConstructor.getAnnotation(BeanRecordConstruct.class);
if (recordConstruct.constructMode() == BeanRecordConstructMode.EXCLUDE)
throw new RecordNoAvailableConstructorsExceptions((Class<? extends Record>) targetClass, "All available constructors have been "
+ "annotated with @RecordConstruct(constructMode = RecordConstructMode.EXCLUDE). To enable mapping to the target record, please make at "
+ "least one constructor available.");
+ "annotated with @RecordConstruct(constructMode = RecordConstructMode.EXCLUDE). To enable mapping to the target record, please make at "
+ "least one constructor available.");
}
return canonicalConstructor;
}
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/io/beanmapper/utils/CanonicalClassName.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.beanmapper.utils;

public class CanonicalClassName {

Check warning on line 3 in src/main/java/io/beanmapper/utils/CanonicalClassName.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/io/beanmapper/utils/CanonicalClassName.java#L3

Added line #L3 was not covered by tests
public static String determineCanonicalClassName(Class<?> clazz) {
return clazz.getCanonicalName() == null ?
fallbackWhenCanonicalNameIsNull(clazz) :
clazz.getCanonicalName();
}

// See bug 190, https://github.com/42BV/beanmapper/issues/190
// When an enum has abstract methods, calling getCanonicalName() on its class will return a null value.
// This fallback method will allow for the expected canonical name value to be returned. getCanonicalName
// must no longer be called directly.
private static String fallbackWhenCanonicalNameIsNull(Class<?> clazz) {
return clazz.getSuperclass().getCanonicalName();
}
}
21 changes: 8 additions & 13 deletions src/test/java/io/beanmapper/BeanMapperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,7 @@
import io.beanmapper.testmodel.encapsulate.source_annotated.Car;
import io.beanmapper.testmodel.encapsulate.source_annotated.CarDriver;
import io.beanmapper.testmodel.encapsulate.source_annotated.Driver;
import io.beanmapper.testmodel.enums.ColorEntity;
import io.beanmapper.testmodel.enums.ColorResult;
import io.beanmapper.testmodel.enums.ColorStringResult;
import io.beanmapper.testmodel.enums.Day;
import io.beanmapper.testmodel.enums.DayEnumSourceArraysAsList;
import io.beanmapper.testmodel.enums.EnumSourceArraysAsList;
import io.beanmapper.testmodel.enums.EnumTargetList;
import io.beanmapper.testmodel.enums.RGB;
import io.beanmapper.testmodel.enums.UserRole;
import io.beanmapper.testmodel.enums.UserRoleResult;
import io.beanmapper.testmodel.enums.WeekEntity;
import io.beanmapper.testmodel.enums.WeekResult;
import io.beanmapper.testmodel.enums.WeekStringResult;
import io.beanmapper.testmodel.enums.*;
import io.beanmapper.testmodel.ignore.IgnoreSource;
import io.beanmapper.testmodel.ignore.IgnoreTarget;
import io.beanmapper.testmodel.initially_unmatched_source.SourceWithUnmatchedField;
Expand Down Expand Up @@ -1977,6 +1965,13 @@ void testMapEntityWithMapToEntityResultWithMap() {
assertNull(result.children.get("Klaas").children);
}

@Test
void testEnumWithAbstractMethod() {
WithAbstractMethod enumValue = WithAbstractMethod.class.getEnumConstants()[0];
ComplexEnumResult result = beanMapper.map(enumValue, ComplexEnumResult.class);
assertEquals(enumValue.name(), result.name);
}

private MyEntity createMyEntity() {
MyEntity child = new MyEntity();
child.value = "Piet";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package io.beanmapper.testmodel.enums;

public class ComplexEnumResult {

public String name;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.beanmapper.testmodel.enums;

public enum WithAbstractMethod {
ONE_VALUE() {
@Override
public void someAbstractMethod() {}

Check notice on line 6 in src/test/java/io/beanmapper/testmodel/enums/WithAbstractMethod.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/test/java/io/beanmapper/testmodel/enums/WithAbstractMethod.java#L6

Document empty method body
};
public abstract void someAbstractMethod();

public String getName() {
return name();
}
}
21 changes: 21 additions & 0 deletions src/test/java/io/beanmapper/utils/CanonicalClassNameTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.beanmapper.utils;

import io.beanmapper.testmodel.enums.Day;
import io.beanmapper.testmodel.enums.WithAbstractMethod;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class CanonicalClassNameTest {
@Test
public void determineCanonicalClassNameOfOrdinaryEnum() {
assertEquals("io.beanmapper.testmodel.enums.Day", CanonicalClassName.determineCanonicalClassName(Day.MONDAY.getClass()));
}

@Test
public void determineCanonicalClassNameOfEnumWithAbstractMethod() {
assertEquals("io.beanmapper.testmodel.enums.WithAbstractMethod",
CanonicalClassName.determineCanonicalClassName(WithAbstractMethod.ONE_VALUE.getClass()));
}
}

0 comments on commit eaa8105

Please sign in to comment.