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

Custom deserializer shouldn't instantiate their own ObjectMapper #128

Closed
guillaumelamirand opened this issue Feb 15, 2023 · 9 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@guillaumelamirand
Copy link
Contributor

guillaumelamirand commented Feb 15, 2023

Describe the bug

While trying to deserialize AsyncAPI with a custom object mapper which disables FAIL_ON_UNKNOWN_PROPERTIES, stack exceptions are logged anyway and if any extensions are present under Component, Schema, or Message the result AsyncAPI object is half empty.

I will try to work on a PR.

How to Reproduce

Steps to reproduce the issue. Attach all resources that can help us understand the issue:

  • Using the following unit test with the attached AsyncAPI will fail
public class ImportAsyncTest {

    @Test
    void should_parse_asyncapi() throws IOException {
        ClassLoader classloader = Thread.currentThread().getContextClassLoader();
        try (InputStream myStream = classloader.getResourceAsStream("solace.yaml")) {
            String asyncapi = IOUtils.toString(myStream, Charset.defaultCharset()).trim();
            ObjectMapper yamlMapper = new ObjectMapper(
                YAMLFactory
                    .builder()
                    .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER)
                    .enable(StreamReadFeature.STRICT_DUPLICATE_DETECTION)
                    .build()
            )
                .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
                .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)
                .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
                .setSerializationInclusion(JsonInclude.Include.NON_NULL)
                .findAndRegisterModules();
            var asyncApi = yamlMapper.readValue(asyncapi, AsyncAPI.class);

            assertThat(asyncApi.getServers()).hasSize(2);
            assertThat(asyncApi.getChannels()).hasSize(2);
            assertThat(asyncApi.getComponents()).isNotNull();
            assertThat(asyncApi.getComponents().getMessages()).hasSize(2);
            assertThat(asyncApi.getComponents().getSchemas()).hasSize(2);
        }
    }
components:
  schemas:
    UserSignedUpSchema:
      x-ep-schema-version: "0.1.0"
      x-ep-schema-state-id: "2"
      x-ep-schema-version-id: "5rlnuswe64g"
      $schema: "http://json-schema.org/draft-06/schema#"
      x-ep-schema-id: "hcq88rogdaj"
      x-ep-schema-state-name: "RELEASED"
      x-ep-schema-name: "UserSignedUpSchema"
      definitions:
        UserSignedUp:
          additionalProperties: false
          type: "object"
          title: "UserSignedUp"
          properties:
            displayName:
              type: "string"
            email:
              type: "string"
          required:
            - "displayName"
            - "email"
      x-ep-schema-version-displayname: ""
    UserSignedInSchema:
      x-ep-schema-version: "0.1.0"
      x-ep-schema-state-id: "2"
      x-ep-schema-version-id: "fxnate3zvf6"
      $schema: "http://json-schema.org/draft-06/schema#"
      x-ep-schema-id: "iigkr9ji3kg"
      x-ep-schema-state-name: "RELEASED"
      x-ep-schema-name: "UserSignedInSchema"
      definitions:
        UserSignedIn:
          additionalProperties: false
          type: "object"
          title: "UserSignedn"
          properties:
            displayName:
              type: "string"
            email:
              type: "string"
          required:
            - "displayName"
            - "email"
      x-ep-schema-version-displayname: ""
asyncapi: "2.5.0"
info:
  x-ep-event-api-product-version-id: "4ndau725iz0"
  x-ep-event-api-id: "jcc06dwq1bh"
  x-ep-event-api-product-version: "0.1.0"
  x-ep-state-name: "RELEASED"
  title: "User Event Api"
  x-ep-application-domain-id: "4auuz4amd31"
  version: "0.1.0"
  x-ep-event-api-product-id: "tcpugx95gc0"
  x-ep-event-api-version-id: "t8hkbn8lit9"
  x-ep-application-domain-name: "gravitee_demo"
  x-ep-event-api-version: "0.1.0"
  x-ep-event-api-product-name: "User Event Api Product"
  x-ep-state-id: "2"

Expected behavior

All ignore properties (x-*) are ignored.

@guillaumelamirand guillaumelamirand added the bug Something isn't working label Feb 15, 2023
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@Pakisan
Copy link
Member

Pakisan commented Feb 15, 2023

#126

@Pakisan Pakisan closed this as completed Feb 20, 2023
@Pakisan
Copy link
Member

Pakisan commented Feb 21, 2023

@guillaumelamirand hi!

Can you tell me more about #131 (comment)?

You crafted schema by hands or by objectMapper?

upd:
When I wrote response to you I didn't check changes from @dennis-brinley. He did all right, by using @JsonAnyGetter, @JsonAnySetter

On deserialization:

{
  "description": "Id of the user.",
  "schema": {
    "type": "string"
  },
  "location": "$message.payload#/user/id",
  "x-ep-event-api-product-version-id": "4ndau725iz0",
  "x-ep-event-api-id": "jcc06dwq1bh",
  "x-ep-event-api-product-version": "0.1.0",
  "x-ep-state-name": "RELEASED",
  "x-ep-application-domain-id": "4auuz4amd31",
  "x-ep-event-api-product-id": "tcpugx95gc0",
  "x-ep-event-api-version-id": "t8hkbn8lit9",
  "x-ep-application-domain-name": "gravitee_demo",
  "x-ep-event-api-version": "0.1.0",
  "x-ep-event-api-product-name": "User Event Api Product",
  "x-ep-state-id": "2"
}

On serialization:

Parameter(
  description = "Id of the user.", 
  ...
  location = "$message.payload#/user/id", 
  extensionFields = {
    x - ep - event - api - version = 0.1 .0,
    x - ep - event - api - product - version - id = 4n dau725iz0,
    x - ep - event - api - id = jcc06dwq1bh,
    x - ep - event - api - product - name = User Event Api Product,
    x - ep - event - api - product - version = 0.1 .0,
    x - ep - state - name = RELEASED,
    x - ep - application - domain - id = 4 auuz4amd31,
    x - ep - event - api - product - id = tcpugx95gc0,
    x - ep - event - api - version - id = t8hkbn8lit9,
    x - ep - state - id = 2,
    x - ep - application - domain - name = gravitee_demo
})

Here is only one type in value - String instead of Object

@Pakisan Pakisan reopened this Feb 21, 2023
@guillaumelamirand
Copy link
Contributor Author

guillaumelamirand commented Feb 22, 2023

My comment regarding extensionFields is not related to this PR, but it is due to a change made with aec9541a4a469b1b62dd58f3728203f41e755957 the extra extensionFields get written back when you serialized the model.

I tested by reading AsyncAPI from file to model with x-xxx attributes which creates extra extensionFields maps, then I write it back and the result is the one I attached above.

@Pakisan
Copy link
Member

Pakisan commented Feb 22, 2023

Great, looks like everything works as expected. Will close this issue tomorrow

upd:
extensionFields are for internal use only and must not be present in specification

@guillaumelamirand
Copy link
Contributor Author

Great, looks like everything works as expected. Will close this issue tomorrow

upd: extensionFields are for internal use only and must not be present in specification

Yes that is exactly the meaning of my comment. The field got generated when I serialized the model to the file and it shouldn’t.

@Pakisan
Copy link
Member

Pakisan commented Feb 22, 2023

I'm a little bit confused, is here unexpected behavior or not😅

If you think that yes, tell how to reproduce.

I checked once more, on reading of spec with 'x-*' fields, they have been moved to 'extensionFuelds', on writing they have been put to object body directly

@guillaumelamirand
Copy link
Contributor Author

I will check it again, let you know.

@Pakisan
Copy link
Member

Pakisan commented Feb 25, 2023

Check on new develop. I merged changes - #133

@Pakisan Pakisan closed this as completed May 24, 2023
@Pakisan Pakisan assigned Pakisan and unassigned Pakisan May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants