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

HealthCheckResponse deserialization issues #243

Closed
fabiobrz opened this issue Feb 17, 2020 · 4 comments · Fixed by #257
Closed

HealthCheckResponse deserialization issues #243

fabiobrz opened this issue Feb 17, 2020 · 4 comments · Fixed by #257

Comments

@fabiobrz
Copy link

fabiobrz commented Feb 17, 2020

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
Copy link
Member

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

@xstefank xstefank added this to the 3.0 milestone Feb 17, 2020
@xstefank xstefank added the bug label Feb 17, 2020
@xstefank xstefank assigned xstefank and unassigned xstefank May 19, 2020
@xstefank
Copy link
Member

@pgunapal volunteered to take this. Thanks!

@xstefank xstefank assigned xstefank and unassigned xstefank May 19, 2020
@xstefank
Copy link
Member

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

@fabiobrz
Copy link
Author

@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: #243 (comment)

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

Successfully merging a pull request may close this issue.

2 participants