javaee / json-processing-spec

Legacy JSON Processing spec. Please use the link below to find the current JSON P project
https://github.com/jakartaee/jsonp-api
Other
8 stars 3 forks source link

review Json public static JsonObjectBuilder createObjectBuilder(Map<String, Object> object) #86

Closed glassfishrobot closed 7 years ago

glassfishrobot commented 7 years ago

I have quite a few open questions regarding

{source} public static JsonObjectBuilder Json#createObjectBuilder(Map<String, Object> object){source}

1.) Do we really need this method? This is just a convenience method anyway. You can just call the add() methods on a fresh builder as well.

2.) What can you pass in? Of course all types where add methods exist on the JsonObjectBuilder. But what about e.g. Optional? What about a List or even Map<String, Integer> ?

Affected Versions

[1.0-pr]

glassfishrobot commented 7 years ago

Reported by struberg

glassfishrobot commented 7 years ago

struberg said: Side note: this method is almost in the JSON-B area already.

glassfishrobot commented 7 years ago

@lukasj said: re 1) agree this is just a convenience method but it may be useful in scenarios where one obtains a Map from somewhere (ie reads properties file) and wants to transmit it over the wire... not sure how much sense this makes...

re 2) basically all types supported by json-p can be passed in; attached patch adds support for Optionals - if Optional contains a value, it is serialized, if not (= optional.isPresent() == false) then particular key is not serialized. If 'null' value for given key in JsonObject/JsonArray being built is required then it must not be Optional.

WDYT?

glassfishrobot commented 7 years ago

@m0mus said: I don't see a problem with this method. Optionals support is a good idea. javadoc must clearly describe what is supported.

glassfishrobot commented 7 years ago

struberg said: Ok, what if a value in the map is not one of the JsonValue types? Should we use toString() on it and handle it as String? Or shall it blow up?

glassfishrobot commented 7 years ago

struberg said: edit wrong ticket

glassfishrobot commented 7 years ago

@lukasj said: if something in the map is not json supported data type then RI currently throws IllegalArgumentException(String.format("Type %s is not supported.", value.getClass())

glassfishrobot commented 7 years ago

@lukasj said: https://java.net/projects/jsonp/sources/git/revision/7899554

glassfishrobot commented 7 years ago

File: jsonp-86.diff Attached By: @lukasj

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JSON_PROCESSING_SPEC-86

glassfishrobot commented 7 years ago

Marked as fixed on Thursday, March 9th 2017, 9:42:39 am