From 5cd10438c3010fe09ac0dd14e986271eca3bd65d Mon Sep 17 00:00:00 2001 From: Ed Gomoliako Date: Tue, 5 Nov 2024 22:19:49 +0100 Subject: [PATCH 1/4] fix: Make JsonGenerator::writeTypePrefix method to not make a WRAPPER_ARRAY when `typeIdDef.id == null` --- src/main/java/com/fasterxml/jackson/core/JsonGenerator.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java index 42ff2a9d82..1c8c05e2af 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java @@ -10,6 +10,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.Objects; import com.fasterxml.jackson.core.JsonParser.NumberType; import com.fasterxml.jackson.core.exc.StreamWriteException; @@ -1978,12 +1979,13 @@ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOExcepti } else { // No native type id; write wrappers // Normally we only support String type ids (non-String reserved for native type ids) - String idStr = (id instanceof String) ? (String) id : String.valueOf(id); + String idStr = (id instanceof String) ? (String) id : Objects.toString(id, null); typeIdDef.wrapperWritten = true; Inclusion incl = typeIdDef.include; // first: can not output "as property" if value not Object; if so, must do "as array" if ((valueShape != JsonToken.START_OBJECT) + && idStr != null && incl.requiresObjectContext()) { typeIdDef.include = incl = WritableTypeId.Inclusion.WRAPPER_ARRAY; } From c6bc9365ac241a5a996946ba8c952a6508e3d7d8 Mon Sep 17 00:00:00 2001 From: Eduard Gomoliako Date: Thu, 7 Nov 2024 14:39:17 +0100 Subject: [PATCH 2/4] feat: Skip writing type prefix wrapper in case typeDef.is is null --- .../fasterxml/jackson/core/JsonGenerator.java | 123 +++++++++++------- 1 file changed, 73 insertions(+), 50 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java index 1c8c05e2af..d71ee9aa04 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java @@ -1969,64 +1969,87 @@ public void writeTypeId(Object id) throws IOException { */ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOException { - Object id = typeIdDef.id; + final boolean hasStartObjectWritten = canWriteTypeId() + // just rely on native type output method (sub-classes likely to override) + ? writeTypePrefixWithTypeId(typeIdDef) + // No native type id; write wrappers + : writeTypePrefixWrapper(typeIdDef); - final JsonToken valueShape = typeIdDef.valueShape; - if (canWriteTypeId()) { - typeIdDef.wrapperWritten = false; - // just rely on native type output method (sub-classes likely to override) - writeTypeId(id); - } else { - // No native type id; write wrappers - // Normally we only support String type ids (non-String reserved for native type ids) - String idStr = (id instanceof String) ? (String) id : Objects.toString(id, null); - typeIdDef.wrapperWritten = true; - - Inclusion incl = typeIdDef.include; - // first: can not output "as property" if value not Object; if so, must do "as array" - if ((valueShape != JsonToken.START_OBJECT) - && idStr != null - && incl.requiresObjectContext()) { - typeIdDef.include = incl = WritableTypeId.Inclusion.WRAPPER_ARRAY; - } - - switch (incl) { - case PARENT_PROPERTY: - // nothing to do here, as it has to be written in suffix... - break; - case PAYLOAD_PROPERTY: - // only output as native type id; otherwise caller must handle using some - // other mechanism, so... - break; - case METADATA_PROPERTY: - // must have Object context by now, so simply write as field name - // Note, too, that it's bit tricky, since we must print START_OBJECT that is part - // of value first -- and then NOT output it later on: hence return "early" - writeStartObject(typeIdDef.forValue); - writeStringField(typeIdDef.asProperty, idStr); - return typeIdDef; - - case WRAPPER_OBJECT: - // NOTE: this is wrapper, not directly related to value to output, so don't pass - writeStartObject(); - writeFieldName(idStr); - break; - case WRAPPER_ARRAY: - default: // should never occur but translate as "as-array" - writeStartArray(); // wrapper, not actual array object to write - writeString(idStr); - } - } // and finally possible start marker for value itself: - if (valueShape == JsonToken.START_OBJECT) { - writeStartObject(typeIdDef.forValue); - } else if (valueShape == JsonToken.START_ARRAY) { + switch (typeIdDef.valueShape) { + case START_OBJECT: + if (!hasStartObjectWritten) + writeStartObject(typeIdDef.forValue); + break; + case START_ARRAY: // should we now set the current object? writeStartArray(); + break; } + return typeIdDef; } + private boolean writeTypePrefixWithTypeId(WritableTypeId typeIdDef) throws IOException { + typeIdDef.wrapperWritten = false; + writeTypeId(typeIdDef.id); + + return false; + } + + /** + * Writes a wrapper for the type id; if necessary. + * @return True if start of an object has been written, False otherwise. + */ + private boolean writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOException { + // Normally we only support String type ids (non-String reserved for native type ids) + final String id = Objects.toString(typeIdDef.id, null); + + // If we don't have Type ID we don't write a wrapper. + if (id == null) { + return false; + } + + Inclusion incl = typeIdDef.include; + + // first: can not output "as property" if value not Object; if so, must do "as array" + if ((typeIdDef.valueShape != JsonToken.START_OBJECT) && incl.requiresObjectContext()) { + typeIdDef.include = incl = WritableTypeId.Inclusion.WRAPPER_ARRAY; + } + + + typeIdDef.wrapperWritten = true; + + switch (incl) { + case PARENT_PROPERTY: + // nothing to do here, as it has to be written in suffix... + break; + case PAYLOAD_PROPERTY: + // only output as native type id; otherwise caller must handle using some + // other mechanism, so... + break; + case METADATA_PROPERTY: + // must have Object context by now, so simply write as field name + // Note, too, that it's bit tricky, since we must print START_OBJECT that is part + // of value first -- and then NOT output it later on: hence return "early" + writeStartObject(typeIdDef.forValue); + writeStringField(typeIdDef.asProperty, id); + return true; + + case WRAPPER_OBJECT: + // NOTE: this is wrapper, not directly related to value to output, so don't pass + writeStartObject(); + writeFieldName(id); + break; + case WRAPPER_ARRAY: + default: // should never occur but translate as "as-array" + writeStartArray(); // wrapper, not actual array object to write + writeString(id); + } + + return false; + } + /** * Method to call along with {@link #writeTypePrefix}, but after actual value * that has type id has been completely written. This allows post-processing From 201ed2bcf446d310d6568c089ee9324ab4c5467b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 8 Nov 2024 15:49:51 -0800 Subject: [PATCH 3/4] Add release notes; streamline code; minor logic changes (since I'm familiar with intent) --- release-notes/CREDITS-2.x | 5 ++++ release-notes/VERSION-2.x | 3 ++ .../fasterxml/jackson/core/JsonGenerator.java | 29 ++++++++++++------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 400c0cec40..a89312826b 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -456,3 +456,8 @@ Jared Stehler (@jaredstehler) Zhanghao (@zhangOranges) * Contributed #1305: Make helper methods of `WriterBasedJsonGenerator` non-final to allow overriding (2.18.0) + +Eduard Gomoliako (@Gems) + * Contributed #1356: Make `JsonGenerator::writeTypePrefix` method to not write a + `WRAPPER_ARRAY` when `typeIdDef.id == null` + (2.19.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index ad4b7179f1..d7062e8df2 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -17,6 +17,9 @@ a pure JSON library. 2.19.0 (not yet released) #1328: Optimize handling of `JsonPointer.head()` +#1356: Make `JsonGenerator::writeTypePrefix` method to not write a + `WRAPPER_ARRAY` when `typeIdDef.id == null` + (contributed by Eduard G) 2.18.1 (28-Oct-2024) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java index d71ee9aa04..58437fadda 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java @@ -1969,39 +1969,47 @@ public void writeTypeId(Object id) throws IOException { */ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOException { - final boolean hasStartObjectWritten = canWriteTypeId() - // just rely on native type output method (sub-classes likely to override) - ? writeTypePrefixWithTypeId(typeIdDef) - // No native type id; write wrappers - : writeTypePrefixWrapper(typeIdDef); + // First: is this a native type id? If so, just use write method, be done + if (canWriteTypeId()) { + // just rely on native type output method (sub-classes likely to override) + return _writeNativeTypePrefix(typeIdDef); + } + + // No native type id; write wrappers + final boolean wasStartObjectWritten = _writeTypePrefixWrapper(typeIdDef); // and finally possible start marker for value itself: switch (typeIdDef.valueShape) { case START_OBJECT: - if (!hasStartObjectWritten) + if (!wasStartObjectWritten) { writeStartObject(typeIdDef.forValue); + } break; case START_ARRAY: // should we now set the current object? writeStartArray(); break; + default: // otherwise: no start marker } return typeIdDef; } - private boolean writeTypePrefixWithTypeId(WritableTypeId typeIdDef) throws IOException { + // @since 2.19 + protected WritableTypeId _writeNativeTypePrefix(WritableTypeId typeIdDef) throws IOException { typeIdDef.wrapperWritten = false; writeTypeId(typeIdDef.id); - - return false; + return typeIdDef; } /** * Writes a wrapper for the type id; if necessary. + * * @return True if start of an object has been written, False otherwise. + * + * @since 2.19 */ - private boolean writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOException { + protected boolean _writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOException { // Normally we only support String type ids (non-String reserved for native type ids) final String id = Objects.toString(typeIdDef.id, null); @@ -2017,7 +2025,6 @@ private boolean writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOExcept typeIdDef.include = incl = WritableTypeId.Inclusion.WRAPPER_ARRAY; } - typeIdDef.wrapperWritten = true; switch (incl) { From a5f17c329369dd1748db33b56986b5f0862e08fe Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 8 Nov 2024 16:22:16 -0800 Subject: [PATCH 4/4] Revert my earlier misguided simplification (need START_OBJECT/ARRAY for values, with native Type Ids too) --- .../fasterxml/jackson/core/JsonGenerator.java | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java index 58437fadda..cee202f465 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java @@ -1969,16 +1969,12 @@ public void writeTypeId(Object id) throws IOException { */ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOException { - // First: is this a native type id? If so, just use write method, be done - if (canWriteTypeId()) { - // just rely on native type output method (sub-classes likely to override) - return _writeNativeTypePrefix(typeIdDef); - } - - // No native type id; write wrappers - final boolean wasStartObjectWritten = _writeTypePrefixWrapper(typeIdDef); + // Are native type ids allowed? If so, use them; otherwise, use wrappers + final boolean wasStartObjectWritten = canWriteTypeId() + ? _writeTypePrefixUsingNative(typeIdDef) + : _writeTypePrefixUsingWrapper(typeIdDef); - // and finally possible start marker for value itself: + // And then possible start marker for value itself: switch (typeIdDef.valueShape) { case START_OBJECT: if (!wasStartObjectWritten) { @@ -1986,8 +1982,7 @@ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOExcepti } break; case START_ARRAY: - // should we now set the current object? - writeStartArray(); + writeStartArray(typeIdDef.forValue); break; default: // otherwise: no start marker } @@ -1995,21 +1990,27 @@ public WritableTypeId writeTypePrefix(WritableTypeId typeIdDef) throws IOExcepti return typeIdDef; } - // @since 2.19 - protected WritableTypeId _writeNativeTypePrefix(WritableTypeId typeIdDef) throws IOException { + /** + * Writes a native type id (when supported by format). + * + * @return True if start of an object has been written, False otherwise. + * + * @since 2.19 + */ + protected boolean _writeTypePrefixUsingNative(WritableTypeId typeIdDef) throws IOException { typeIdDef.wrapperWritten = false; writeTypeId(typeIdDef.id); - return typeIdDef; + return false; } /** - * Writes a wrapper for the type id; if necessary. + * Writes a wrapper for the type id if necessary. * - * @return True if start of an object has been written, False otherwise. + * @return True if start of an object has been written, false otherwise. * * @since 2.19 */ - protected boolean _writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOException { + protected boolean _writeTypePrefixUsingWrapper(WritableTypeId typeIdDef) throws IOException { // Normally we only support String type ids (non-String reserved for native type ids) final String id = Objects.toString(typeIdDef.id, null); @@ -2044,7 +2045,8 @@ protected boolean _writeTypePrefixWrapper(WritableTypeId typeIdDef) throws IOExc return true; case WRAPPER_OBJECT: - // NOTE: this is wrapper, not directly related to value to output, so don't pass + // NOTE: this is wrapper, not directly related to value to output, so + // do NOT pass "typeIdDef.forValue" writeStartObject(); writeFieldName(id); break;