AbstractMappingContext with JDK 17 crashing with InaccessibleObjectException

[amidukr]

Hi, have you seen the issue with AbstractMappingContext and latest JDK?

Here are few details:

• https://stackoverflow.com/questions/69352207/jdk-17-spring-boot-unable-to-make-private-java-time-localdatetime
• [BUG] java.lang.reflect.InaccessibleObjectException - Defining a CosmosDB entity bean with BigDecimal Azure/azure-sdk-for-java#35345

Environment:

• JDK 20
• Spring Boot version: 2.7.12
• Use java.math.BigDecimal as part of pojo being scanned by AbstractMappingContext.

Looking forward for your help.

[mp911de]

BigDecimal is not a common simple type. If your Spring Data module doesn't support BigDecimal natively, then you need to provide a converter to not treat that type as entity. Entities are inspected deeply via reflection for constructors and other reflective elements.

[amidukr]

BigDecimal is not a common simple type. If your Spring Data module doesn't support BigDecimal natively, then you need to provide a converter to not treat that type as entity. Entities are inspected deeply via reflection for constructors and other reflective elements.

The problem here is that AbstractMappingContext is dump, it does deep scanning across all packages and does Field.setAccessible(true) for every field. Any reason it do for classes of java.* package? That's not a support to request to support BigDecimal.

[Xiphoseer]

I believe this bug should be reopened. java.math.BigInteger and java.math.BigDecimal are the canonical Java Mapping for XML Schema Built-in Types of JAXB which categorize xsd:integer and xsd:decimal as atomic types, and have been for years.

Jackson has no issue dealing with them in ObjectMapper and considering them as beans in Spring Data via opening up their fields to reflection seems seriously flawed.

The following is a standalone reproducer of the issue, using just org.springframework.data:spring-data-commons:3.2.4:

package com.example.spring;

import org.junit.jupiter.api.Test;
import org.springframework.data.mapping.Association;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.mapping.context.AbstractMappingContext;
import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty;
import org.springframework.data.mapping.model.BasicPersistentEntity;
import org.springframework.data.mapping.model.Property;
import org.springframework.data.mapping.model.SimpleTypeHolder;
import org.springframework.data.util.TypeInformation;
import org.springframework.lang.NonNull;

import java.math.BigDecimal;

/**
 * Reproducer for <a href="https://github.com/spring-projects/spring-data-commons/issues/2844">spring-data-commons #2844</a>
 */
public class AbstractMappingContextTest {

    public static class TestProperty extends AnnotationBasedPersistentProperty<TestProperty> {
        public TestProperty(Property property, PersistentEntity<?, TestProperty> owner, SimpleTypeHolder simpleTypeHolder) {
            super(property, owner, simpleTypeHolder);
        }

        @Override
        @NonNull
        protected Association<TestProperty> createAssociation() {
            throw new UnsupportedOperationException();
        }
    }


    public static class TestMappingContext extends AbstractMappingContext<BasicPersistentEntity<?, TestProperty>, TestProperty> {
        @Override
        @NonNull
        protected <T> BasicPersistentEntity<T, TestProperty> createPersistentEntity(@NonNull TypeInformation<T> typeInformation) {
            return new BasicPersistentEntity<>(typeInformation);
        }

        @Override
        @NonNull
        protected TestProperty createPersistentProperty(@NonNull Property property, @NonNull BasicPersistentEntity<?, TestProperty> owner, @NonNull SimpleTypeHolder simpleTypeHolder) {
            return new TestProperty(property, owner, simpleTypeHolder);
        }

        public <T> void addEntityClass(Class<T> entityClass) {
            addPersistentEntity(entityClass);
        }
    }


    public static class MyEntity {
        private BigDecimal value;
    }

    @Test
    public void testAMC() {
        TestMappingContext tmc = new TestMappingContext();
        tmc.addEntityClass(MyEntity.class);
    }
}

[mp911de]

You're right, many XML and JSON libraries handle java.math.* types well. MappingContext is to manage persistent entities, that aren't related to XML or JSON in the first places. Many databases do not support BigInteger or BigDecimal natively. Even until recent, MongoDB didn't have a way to represent BigDecimal until they introduced their decimal128 type. The only other option was storing the decimal value as String which is all but convenient.

That being said, we cannot pretend that BigDecimal are simple types natively supported by the database of your choice. When not natively supported, such types must be converted through an associated Converter that avoids PersistentEntity creation.

In theory, you could ask MappingContext to create and introspect entities for all sorts of protected Java types that one could want to store as simple type (such as Charset, UUID and many more).

[Xiphoseer]

That being said, we cannot pretend that BigDecimal are simple types natively supported by the database of your choice. When not natively supported, such types must be converted through an associated Converter that avoids PersistentEntity creation.

Right, I agree that not all Spring Data implementations will natively support BigDecimal. But that's not the question here:

1. "not natively supported" is not what the error message says. The error message is a raw java.lang.reflect.InaccessibleObjectException, which provides little context on what actually went wrong. At the very least, it could be wrapped into some Spring Data exceptions talking about a failure to find the properties of an entity.
2. Secondly, according to

   spring-data-commons/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

   Lines 214 to 226 in 19e2a1c

   	/**
   	* Configures the {@link SimpleTypeHolder} to be used by the {@link MappingContext}. Allows customization of what
   	* types will be regarded as simple types and thus not recursively analyzed.
   	*
   	* @param simpleTypes must not be {@literal null}.
   	*/
   	public void setSimpleTypeHolder(SimpleTypeHolder simpleTypes) {
   	Assert.notNull(simpleTypes, "SimpleTypeHolder must not be null");
   	this.simpleTypeHolder = simpleTypes;
   	}

   
   a "simple type" is one that is not recursively analyzed. In my understanding, this is much more about the type being atomic i.e. not constructed from simpler values. I would argue this is the case for BigDecimal and BigInteger.
3. Both spring-data-jdbc at here and spring-data-r2dbc here consider BigDecimal and BigInteger as simple. This matches the fact that it exists in the JDBC API / datamodel
4. spring-data-jpa doesn't, but it overrides shouldCreatePersistentEntityFor, which defaults to checking for simple types, but JPA uses the Metamodel instead, to check which fields should be recursed into as entities. Again, this leads to not checking BigDecimal for fields via reflection.
5. All the NoSQL datastores that use Jackson already have support for BigDecimal in there. It may not round-trip perfectly if the database isn't set up to keep the full precision of the field, but that's a limitation of the JSON spec and nothing spring-data-commons can really influence. Note that both serializing as a JSON number and a JSON string ends up as a simple value.
6. I don't have much experience with spring-data-mongodb, but from my understanding and limited testing is that its BigDecimalToStringConverter would still be recognized as applicable for the handling a BigDecimal field and turn that into a string unless otherwise specified in the @Field annotation.

In theory, you could ask MappingContext to create and introspect entities for all sorts of protected Java types that one could want to store as simple type (such as Charset, UUID and many more).

Yes, you could do so, but I find this exceedingly unlikely after the analysis above. I would argue that most if not all classes in java.* should indeed be treated as "simple" (in the sense of not reflecting them for properties) if their fields are not part of the public API. To some extent, this is prepared in

spring-data-commons/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java

Line 158 in 19e2a1c

if (typeName.startsWith("java.lang") || type.getName().startsWith("java.time") || typeName.equals("kotlin.Unit")) {

but that line would be missing type.getName().startsWith("java.math") in that case.