javax.json.JsonObject.isNull(String) contract broken

[eleumik]

Hi,
while testing some json parsing libraries I noticed that genson returns true when passing an unexisting field name to javax.json.JsonObject.isNull(String) while javadoc says that

throws NullPointerException if the specified name doesn't have any mapping

Digging in code I found

  @Override
   public boolean isNull(String name) {
           JsonValue value = values.get(name);
           return (JsonValue.NULL.equals(value) || value == null);
    }

|| value == null should be removed however isNull is used also by impl of other methods of same class: getString, getBoolean, getInt, for them the current implementation is ok.

My proposed change is the following, with getString, getBoolean, getInt calling isNullOrAbsent

      @Override
  public boolean isNull(String name) {
    JsonValue value = values.get(name);
    if (value==null) throw new NullPointerException("no mapping for " + name);
    return (JsonValue.NULL.equals(value));
  }
  
  private boolean isNullOrAbsent(String name) {
    JsonValue value = values.get(name);
    return (JsonValue.NULL.equals(value) || value == null);
  }

[EugenCepoi]

The annoying part with failing if the property is missing is that many JSON ser/de libs allow to exclude null values at serialization time. The result is that during deser all those keys will have no value in the json document. Throwing every time an NPE and catching it on the other side wouldn't be efficient at all from a performance point of view. Ideally we probably would want to have to public methods (the second one would be like the isNullOrAbsent that you propose). Also it feels surprising to have a method called isNull that does throw NPE... What is your opinion on this? Do you believe that it is a good call to throw an NPE from isNull? 2017-03-23 10:14 GMT-07:00 Michele Vivoda <notifications@github.com>:

…

Hi, while testing some json parsing libraries I noticed that genson returns true when passing an unexisting field name to javax.json.JsonObject.isNull(String) while javadoc <https://docs.oracle.com/javaee/7/api/javax/json/JsonObject.html#isNull-java.lang.String-> says that throws NullPointerException if the specified name doesn't have any mapping Digging in code I found @OverRide public boolean isNull(String name) { JsonValue value = values.get(name); return (JsonValue.NULL.equals(value) || value == null); } || value == null should be removed however isNull is used also by impl of other methods of same class: getString, getBoolean, getInt, for them the current implementation is ok. My proposed change is the following, with getString, getBoolean, getInt calling isNullOrAbsent @OverRide public boolean isNull(String name) { JsonValue value = values.get(name); if (value==null) throw new NullPointerException("no mapping for " + name); return (JsonValue.NULL.equals(value)); } private boolean isNullOrAbsent(String name) { JsonValue value = values.get(name); return (JsonValue.NULL.equals(value) || value == null); } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#110>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADS_0Xq2VcrTa1xAvXLSs6fG7irG4Tw3ks5roqiTgaJpZM4MnFAx> .

[eleumik]

I think that if a json api is meant for high level clients that often don't need to distinguish between the two cases then a isNullOrAbsent is convenient and isNull is almost useless. For a low level client like a generic binding app it doesnt' make any difference as it will normally use only has()/containsKey() and get().

So yes maybe having isNull behave as isNullOrAbsent would be more convenient, however this behavior is specified and all javax.json implementations I tested behave as from the spec.