vert-x3 / vertx-config

Vert.x Configuration Service
Apache License 2.0
54 stars 64 forks source link

Adding multiple file stores generates a wrong JsonObject #47

Closed ufoscout closed 6 years ago

ufoscout commented 6 years ago

In my project I have two config files (one for application configuration and the other for tests):

config.json:

{
  "server": {
    "port": 8080
  },
  "jwt": {
    "secret": "mySecret",
    "signatureAlgorithm": "HS512",
    "tokenValidityMinutes": 600
  }
}

test-config.json:

{
  "server": {
    "port": 0
  }
}

They are loaded this way (kotlin code):

val fileStore = ConfigStoreOptions()
         .setType("file")
         .setOptional(false)
         .setConfig(JsonObject().put("path", "conf/config.json"))

val testFileStore = ConfigStoreOptions()
        .setType("file")
        .setOptional(true)
        .setConfig(JsonObject().put("path", "conf/test-config.json"))

var retriever = ConfigRetriever.create(vertx, ConfigRetrieverOptions().addStore(fileStore).addStore(testFileStore))

If I do not add the testFileStore to the ConfigRetrieverOptions(), everything works fine and the generated JsonObject is:

{"server":{"port":8080},"jwt":{"secret":"mySecret","signatureAlgorithm":"HS512","tokenValidityMinutes":600}}

But, when I add the testFileStore then the JsonObject is wrong:

{"server":{"map":{"port":0},"empty":false},"jwt":{"secret":"mySecret","signatureAlgorithm":"HS512","tokenValidityMinutes":600}}

Please note that the error can be reproduced by simply adding two times the same file. For example this code:

val fileStore = ConfigStoreOptions()
        .setType("file")
        .setOptional(false)
        .setConfig(JsonObject().put("path", "conf/config.json"))

var retriever = ConfigRetriever.create(vertx, ConfigRetrieverOptions().addStore(fileStore).addStore(fileStore))

generates this wrong JsonObject:

{"server":{"map":{"port":8080},"empty":false},"jwt":{"map":{"secret":"mySecret","signatureAlgorithm":"HS512","tokenValidityMinutes":600},"empty":false}}
cescoffier commented 6 years ago

Looking into it.

cescoffier commented 6 years ago

@ufoscout I've tried to reproduce it in Java and in Kotlin but failed. Can you provide a reproducer?

ufoscout commented 6 years ago

@cescoffier After some investigations, I found that the issue happens only when the default Json.mapper is set to the Jackson mapper for Kotlin. E.g., this causes the bug:

       Json.mapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper()
       Json.prettyMapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper()

At this point I am not sure what/where is the issue since the mapper works fine everywhere else.

vietj commented 6 years ago

@ufoscout would you mind try this on your mappers:

    // custom types
    module.addSerializer(JsonObject.class, new JsonObjectSerializer());
    module.addSerializer(JsonArray.class, new JsonArraySerializer());
    // he have 2 extensions: RFC-7493
    module.addSerializer(Instant.class, new InstantSerializer());
    module.addSerializer(byte[].class, new ByteArraySerializer());

and see if that fixes the problem ?

ufoscout commented 6 years ago

@vietj Yes, indeed adding those serializers fixes the issue. However, they are declared as private into the Json class, so there is no way to reuse them. Nevertheless, I found a better solution. In fact, instead of overriding the Json mappers, I can extend them by adding the KotlinModule:

        Json.mapper.registerModule(KotlinModule())
        Json.prettyMapper.registerModule(KotlinModule())

This is more maintainable and works as expected. Probably this should be reported somewhere in the kotlin support documentation.

vietj commented 6 years ago

@ufoscout thanks for reporting, I think actually we will add documentation in vertx-core as this is a not only kotlin issue.

ufoscout commented 6 years ago

@vietj Why do you have a custom serializer for the Instant class? Shouldn't you instead register the JavaTimeModule provided by Jackson?

vietj commented 6 years ago

@ufoscout I think @pmlopes can elaborate on it

cescoffier commented 6 years ago

Closing this issue, not related to vert.x config.