Expose exception handling to AbstractJsonDeserializer API

[maybeec]

Inheriting the AbstractJsonDeserializer, I would like to throw any processing exception resulting in jackson to understand that deserialization did not work our properly. Currently, the API of jackson completely hidden behind the abstract method (the API of AbstractJsonDeserializer) to be implemented.

So what is missing is basically, the ability to throw any IOException, which I consider as a bug. Currently, the AbstractJsonDeserializer is not really useful. Even although it took me a while to find the code snippet for reading JSON as a tree. I would now consider it as not useful to just provide parts of the parameters provided by jackson to the API of AbstractJsonDeserializer.

The helper method throws IllegalStateExceptions, but most probably should throw the intended JsonProcessingException given by Jackson?

[hohwille]

Thanks for the feedback. I do agree that AbstractJsonDeserializer is not designed well. My suggestion for devon4j would be to do the following:

• extract a utility class with getValue method and make it static: https://github.com/oasp/oasp4j/blob/a858d6556cafd30f7226e7928f71549b29b74107/modules/json/src/main/java/io/oasp/module/json/common/base/AbstractJsonDeserializer.java#L69
• The remaining value of AbstractJsonDeserializer is then just this line of code: JsonNode node = jp.getCodec().readTree(jp); Is that worth a class and dependency? IMHO we should instead extend the JSON guide and explain advanced mapping with such examples.

For oasp4j I would tend to do nothing. If you want to have this simple do not extend AbstractJsonDeserializer and add that single line of code to your own class.

[hohwille]

public class JacksonUtil {

  public static void writeValue(JsonGenerator jgen, Object value) throws IOException {
    if (value == null) {
      return;
    }
    if (value instanceof CharSequence) {
      jgen.writeString(value.toString());
    } else if (value instanceof Number) {
      writeNumber(jgen, (Number) value);
    } else if (value instanceof Boolean) {
      jgen.writeBoolean(((Boolean) value).booleanValue());
    } else if (value instanceof Date) {
      Date date = (Date) value;
      jgen.writeString(date.toInstant().toString());
    } else if (value instanceof Calendar) {
      Calendar calendar = (Calendar) value;
      jgen.writeString(calendar.toInstant().toString());
    } else {
      jgen.writeString(value.toString());
    }
  }

  private static void writeNumber(JsonGenerator jgen, Number value) throws IOException {
    if (value instanceof Long) {
      jgen.writeNumber(value.longValue());
    } else if (value instanceof Integer) {
      jgen.writeNumber(value.intValue());
    } else if (value instanceof BigDecimal) {
      jgen.writeNumber((BigDecimal) value);
    } else if (value instanceof BigInteger) {
      jgen.writeNumber((BigInteger) value);
    } else {
      jgen.writeNumber(value.toString());
    }
  }

  public static <V> V readValue(JsonNode node, String fieldName, Class<V> type, boolean required)
      throws RuntimeException {
    JsonNode childNode = node.get(fieldName);
    try {
      return readValue(childNode, type, required);
    } catch (Exception e) {
      throw new IllegalStateException("Error deserializing field '" + fieldName + "'.", e);
    }
  }

  public static <V> V readValue(JsonNode node, Class<V> type, boolean required) throws RuntimeException {
    V value = readValue(node, type);
    if ((value == null) && (required)) {
      throw new IllegalStateException(
          "The value (" + type.getSimpleName() + ") is missing but was required!");
    }
    return value;
  }

  public static <V> V readValue(JsonNode node, Class<V> type) throws RuntimeException {
    V value = null;
    if (node != null) {
      if (!node.isNull()) {
        try {
          value = readAtomicValue(node, type);
        } catch (NumberFormatException e) {
          throw new IllegalArgumentException("Error converting to: " + type.getName(), e);
        }
      }
    }
    return value;
  }

  private static <V> V readAtomicValue(JsonNode node, Class<V> type) {
    Object result;
    if (type == Boolean.class) {
      // types that may be primitive shall be casted explicitly as Class.cast does not work for primitive types.
      result = Boolean.valueOf(node.booleanValue());
    } else if (type == Integer.class) {
      result = Integer.valueOf(node.asText());
    } else if (type == Long.class) {
      result = Long.valueOf(node.asText());
    } else if (type == Double.class) {
      result = Double.valueOf(node.asText());
    } else if (type == Float.class) {
      result = Float.valueOf(node.asText());
    } else if (type == Short.class) {
      result = Short.valueOf(node.asText());
    } else if (type == Byte.class) {
      result = Byte.valueOf(node.asText());
    } else if (type == String.class) {
      result = node.asText();
    } else if (type == BigDecimal.class) {
      result = new BigDecimal(node.asText());
    } else if (type == BigInteger.class) {
      result = new BigInteger(node.asText());
    } else if (type == Date.class) {
      result = Date.from(Instant.parse(node.asText()));
    } else if (type == Calendar.class) {
      Date date = Date.from(Instant.parse(node.asText()));
      Calendar calendar = Calendar.getInstance();
      calendar.setTime(date);      
      result = calendar;
    } else if (type == Instant.class) {
      result = Instant.parse(node.asText());
    } else if (type == LocalDate.class) {
      result = LocalDate.parse(node.asText());
    } else if (type == LocalDateTime.class) {
      result = LocalDateTime.parse(node.asText());
    } else if (type == LocalTime.class) {
      result = LocalTime.parse(node.asText());
    } else if (type == OffsetDateTime.class) {
      result = OffsetDateTime.parse(node.asText());
    } else if (type == ZonedDateTime.class) {
      result = ZonedDateTime.parse(node.asText());
    } else if (type == Year.class) {
      result = Year.parse(node.asText());
    } else if (type == MonthDay.class) {
      result = type.cast(MonthDay.parse(node.asText()));
    } else {
      throw new IllegalArgumentException("Unsupported value type " + type.getName());      
    }
    if (type.isPrimitive()) {
      return (V) result;
    } else {
      return type.cast(result);
    }
  }

}

[hohwille]

Maybe it would be even better to make the static methods non-static, the private methods protected and add a static get method returning a singleton instance. Of course for usage then one would need to write JacksonUtil.get().readValue... instead of just JacksonUtil.readValue... but we would allow to extend the functionality with additional custom datatypes.

Also we could instead include that into archetype rather than JSON module.

[hohwille]

See also #198 - Jackson has some historical design flaws that makes it hard to reuse custom mapping logic and compose higher level object mappings out of that. On the other hand from my experiments JSON-B seems to be quite limited as well.

[maybeec]

I like the static methods. The singleton is even not better in regards to testing, so let us take the static methods as is. Let's not put it into the archetype, although it will foster visibility and usage. The discussions in the last years about the non-needed classes in archetype shows the demands of the users. Let's keep it clean and provide hints in the documentation pointing at our util classes.