eclipse / microprofile-health

microprofile-health
Apache License 2.0
105 stars 61 forks source link

HealthCheckResponse deserialization issues #243

Closed fabiobrz closed 4 years ago

fabiobrz commented 4 years ago

Hi all, while testing the latest spec release (2.2) on Wildfly, the following Arquillian test case failed:

        try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
            HttpUriRequest request = new HttpGet(
                    String.format("http://%s:%d/health", managementClient.getMgmtAddress(), managementClient.getMgmtPort()));
            try (CloseableHttpResponse response = client.execute(request)) {
                Assert.assertEquals(HttpServletResponse.SC_OK, response.getStatusLine().getStatusCode());

                String responseContent = EntityUtils.toString(response.getEntity());
                System.out.println(responseContent);

                //  this fails with the exception reported below
                HealthCheckResponse typedResponse = new ObjectMapper().readValue(
                        responseContent, HealthCheckResponse.class);
            }
        }

because of the following serialization issue:

[ERROR] test(org.wildfly.test.integration.microprofile.health.ConcreteHealthCheckResponseTestCase)  Time elapsed: 32.048 s  <<< ERROR!
com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: 
Unrecognized field "status" (class org.eclipse.microprofile.health.HealthCheckResponse), not marked as ignorable (3 known properties: "state", "data", "name"])
 at [Source: (String)"{"status":"UP","checks":[{"name":"both","status":"UP","data":{"key":"value"}},{"name":"both","status":"UP","data":{"key":"value"}}]}"; line: 1, column: 12] (through reference chain: org.eclipse.microprofile.health.HealthCheckResponse["status"])

So it looks like HealthCheckResponse definition [1] is not matching the spec payload schema [2].

[1] https://github.com/eclipse/microprofile-health/blob/2.2/api/src/main/java/org/eclipse/microprofile/health/HealthCheckResponse.java#L155

[2] https://download.eclipse.org/microprofile/microprofile-health-2.2/microprofile-health-spec.html#_json_schema

xstefank commented 4 years ago

thanks @fabiobrz for the report. This basically renders HealthCheckResponse unusable in this kind of scenario. We will fix it in the next release.

xstefank commented 4 years ago

@pgunapal volunteered to take this. Thanks!

xstefank commented 4 years ago

@pgunapal mentioned that the State enum should also be renamed to Status, so I am marking this issue as a breaking change.

fabiobrz commented 4 years ago

@pgunapal mentioned that the State enum should also be renamed to Status, so I am marking this issue as a breaking change.

+1 see links reported here for comparison between schema definition and actual response, too: https://github.com/eclipse/microprofile-health/issues/243#issue-566439370