soabase / soabase-halva

Idiomatic Scala ... in Java
https://github.com/soabase/soabase-halva/blob/master/README.md
Apache License 2.0
81 stars 5 forks source link

Support custom JsonProperty annotation in case class interfaces #19

Closed azell closed 8 years ago

azell commented 8 years ago

I would like to define a case class such as:

@CaseClass(json = true)
public interface JsonTest
{
    @JsonProperty("first_name")
    String firstName();
    @JsonProperty("last_name")
    String lastName();
    int age();
}

Not sure how much halva wants to expose a dependency on Jackson, in terms of the actual annotation used by the interface.

Randgalt commented 8 years ago

I guess the json attribute could be an enum that controls naming. e.g. NONE, STANDARD, LOWERCASE. But I worry about supporting a ton of different naming conventions. I think though that you can add the annotations manually. I'll do a test when I get a chance.

azell commented 8 years ago

Great, and I am guessing that the JsonIgnore / JsonIgnoreProperties annotations would also be useful.

Randgalt commented 8 years ago

I'd appreciate a PR or at least a straw-man design

azell commented 8 years ago

Fair enough, let me start reading the case class code.

Randgalt commented 8 years ago

I did some experimenting and it looks like Jackson will use @JsonProperty annotations set in the parent interface. So, we could have an option to @CaseClass(json = true) that sets all Jackson annotations except @JsonPropertys so that users can override them. Or maybe, the annotation processor could just not set @JsonProperty if the parent interface already has it.

azell commented 8 years ago

I see a couple of phases:

  1. When generating the JsonProperty annotation in Templates.addField, check if the method already has a JsonProperty annotation attached to it. If so, either skip the annotation in the generated code, or copy it from the interface.
  2. Support other Jackson annotations such as JsonIgnore by checking for other annotations on the method by name, or by the JacksonAnnotation type.
  3. Support JSR 303 annotations

For number 1, I need to check if all of the required metadata is passed to addField. The metadata would include the class and method.

azell commented 8 years ago

One thing I have been bitten by before is depending on key order in the generated JSON. Ex:

String json = new ObjectMapper().writeValueAsString(object);
Assert.assertEquals("{\"age\":32,\"type\":\"large\",\"nombre\":\"me\"}", json);

A way around this is:

ObjectMapper mapper = new ObjectMapper();
Assert.assertEquals(mapper.readTree("{\"age\":32,\"type\":\"large\",\"nombre\":\"me\"}"), mapper.valueToTree(object));

Lastly ObjectMappers are immutable and fairly expensive to create. Adding a private final ObjectMapper mapper = new ObjectMapper(); to the class could be useful.

Randgalt commented 8 years ago

I don't use a single ObjectMapper because it's just a test. FYI

azell commented 8 years ago

I know, just my OCD / DRY kicking in.