Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@ConfigurationProperties JSON deserialization drops records with null values #11420

Open
kostanovych opened this issue Dec 7, 2024 · 3 comments

Comments

@kostanovych
Copy link

Expected Behavior

When using @ConfigurationProperties to deserialize JSON configuration, all records, including those with null values for properties, should be correctly deserialized into the corresponding Java objects.

Actual Behaviour

Starting from Micronaut 4.2.0, records with null values for properties (e.g., expirationDate in the provided example) are dropped during deserialization.

Steps To Reproduce

  1. Define a @ConfigurationProperties annotated interface to read a JSON configuration file.
  2. Use a configuration file containing a list of objects, some of which have fields with null values.
  3. When loaded, records with null values will be dropped.

Minimal reproducer is available here.

We've the next Config class:

@ConfigurationProperties("common")
public interface Config {
    List<SupportedVersion> getSupportedVersions();
}

SupportedVersion class:

public record SupportedVersion(Long version, OffsetDateTime expirationDate) {
}

Controller:

@Controller
public class SupportedVersionsController {
    @Inject
    private Config config;

    @Get(value = "/supported-versions", produces = MediaType.APPLICATION_JSON)
    public SupportedVersionsResponse getSupportedVersions() {
        return new SupportedVersionsResponse(config.getSupportedVersions());
    }
}

Unit test:

@MicronautTest(propertySources = "classpath:config.json")
public class MicronautDeserializationTest {
    @Inject
    @Client("/")
    HttpClient client;

    @Test
    void testVersions() {
        HttpRequest<?> request = HttpRequest.GET("/supported-versions");

        SupportedVersionsResponse response = client.toBlocking().retrieve(request, SupportedVersionsResponse.class);

        assertEquals(
            List.of(new SupportedVersion(0L, OffsetDateTime.of(2024, 12, 1, 0, 0, 0, 0, ZoneOffset.UTC)),
                    new SupportedVersion(1L, OffsetDateTime.of(2024, 12, 2, 0, 0, 0, 0, ZoneOffset.UTC)),
                    new SupportedVersion(2L, null)),
            response.supportedVersions());
    }
}

And JSON configuration:

{
  "common": {
    "supportedVersions": [
      {
        "version": 0,
        "expirationDate": "2024-12-01T00:00:00.000Z"
      },
      {
        "version": 1,
        "expirationDate": "2024-12-02T00:00:00.000Z"
      },
      {
        "version": 2,
        "expirationDate": null
      }
    ]
  }
}

When you set in build.gradle.kts:

val micronautVersion = "4.1.12"
val micronautTestVersion = "4.1.1"

and run MicronautDeserializationTest, the test passes.

When you set:

val micronautVersion = "4.2.0"
val micronautTestVersion = "4.2.0"

the test fails with the next reason:

Expected :[SupportedVersion[version=0, expirationDate=2024-12-01T00:00Z], SupportedVersion[version=1, expirationDate=2024-12-02T00:00Z], SupportedVersion[version=2, expirationDate=null]]
Actual :[SupportedVersion[version=0, expirationDate=2024-12-01T00:00Z], SupportedVersion[version=1, expirationDate=2024-12-02T00:00Z]]

It seems, breaking change is in this MR: #10070. The structure of the built tree has changed; an exception occurs when deserializing a null value, causing the record to be dropped.

Adding for visibility: @dstepanov, @yawkat, @graemerocher.

Environment Information

  • JDK Version: 17

Example Application

https://github.com/kostanovych/micronaut-deserialization-issue

Version

4.7.7

@glorrian
Copy link
Contributor

glorrian commented Dec 8, 2024

You're right about pr that you mentioned, there's no problems into pr itself, but this problem appears because Jackson has separated logic to parsing arrays

com.fasterxml.jackson.databind.deser.std.StdDeserializer#_deserializeFromArray

 protected T _deserializeFromArray(JsonParser p, DeserializationContext ctxt) throws IOException
    {
        final CoercionAction act = _findCoercionFromEmptyArray(ctxt);
        final boolean unwrap = ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS);

        if (unwrap || (act != CoercionAction.Fail)) {
            JsonToken t = p.nextToken();
            if (t == JsonToken.END_ARRAY) {
                switch (act) {
                case AsEmpty:
                    return (T) getEmptyValue(ctxt);
                case AsNull:
                case TryConvert:
                    return getNullValue(ctxt);
                default:
                }
            } else if (unwrap) {
                final T parsed = _deserializeWrappedValue(p, ctxt);
                if (p.nextToken() != JsonToken.END_ARRAY) {
                    handleMissingEndArrayForSingle(p, ctxt);
                }
                return parsed;
            }
        }
        return (T) ctxt.handleUnexpectedToken(getValueType(ctxt), JsonToken.START_ARRAY, p, null);
    }

so, before 4.2.0 you single null element was just null element and parsing with logic like this

com.fasterxml.jackson.databind.ObjectReader#_bind

protected Object _bind(JsonParser p, Object valueToUpdate) throws IOException {
        DefaultDeserializationContext ctxt = this.createDeserializationContext(p);
        JsonToken t = this._initForReading(ctxt, p);
        Object result;
        if (t == JsonToken.VALUE_NULL) {
            if (valueToUpdate == null) {
                result = this._findRootDeserializer(ctxt).getNullValue(ctxt);
            } else {
                result = valueToUpdate;
            }
        } else if (t != JsonToken.END_ARRAY && t != JsonToken.END_OBJECT) {
            result = ctxt.readRootValue(p, this._valueType, this._findRootDeserializer(ctxt), this._valueToUpdate);
        } else {
            result = valueToUpdate;
        }

        p.clearCurrentToken();
        if (this._config.isEnabled(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)) {
            this._verifyNoTrailingTokens(p, ctxt, this._valueType);
        }

        return result;
    }

but after your object not NULL element it ARRAY element that wrapped NULL, and jackson hasn't logic for this case( so it throw ex and this ex catch into

io.micronaut.json.bind.JsonBeanPropertyBinder#bind(io.micronaut.core.convert.ArgumentConversionContext<java.lang.Object>, java.util.Map<java.lang.CharSequence,? super java.lang.Object>)

public ArgumentBinder.BindingResult<Object> bind(final ArgumentConversionContext<Object> context, Map<CharSequence, ? super Object> source) {
        try {
            JsonNode objectNode = this.buildSourceObjectNode(source.entrySet());
            Object result = this.jsonMapper.readValueFromTree(objectNode, context.getArgument());
            return () -> {
                return Optional.of(result);
            };
        } catch (Exception var5) {
            Exception e = var5;
            context.reject(e);
            return new ArgumentBinder.BindingResult<Object>() {
                public List<ConversionError> getConversionErrors() {
                    return CollectionUtils.iterableToList(context);
                }

                public boolean isSatisfied() {
                    return false;
                }

                public Optional<Object> getValue() {
                    return Optional.empty();
                }
            };
        }
    }

that's why you cannot see element with null

this is a bug, i will think what we can do for fix this

@glorrian
Copy link
Contributor

@kostanovych it was a bug on a jackson side, i sent pr, it will be merge and release, so when MK updates to jackson-databind 2.18 it bug will disappear. so you can close this issue. thanks for this report and great description!

@kostanovych
Copy link
Author

@glorrian nice, thanks for fast reaction and fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants