From 534fa030c34d70799d1170b6c61f67360b109f01 Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Sat, 18 Jan 2025 08:07:39 -0600 Subject: [PATCH 1/8] Add test demonstrating field id case --- .../dataformat/MessagePackGenerator.java | 7 ++ .../jackson/dataformat/MessagePackParser.java | 5 + .../MessagePackDataformatForFieldIdTest.java | 115 ++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java index 11aba6e5..dbda27d3 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java @@ -513,6 +513,13 @@ private void writeByteArrayTextKey(byte[] text, int offset, int len) throws IOEx addValueNode(new String(text, offset, len, DEFAULT_CHARSET)); } + // TODO: Uncomment + //@Override + //public void writeFieldId(long id) throws IOException + //{ + // addKeyToStackTop(id); + //} + @Override public void writeFieldName(String name) { diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java index 4876c797..0fddefb6 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java @@ -621,6 +621,11 @@ public String currentName() return streamReadContext.getCurrentName(); } + // TODO: Uncomment + //public boolean isCurrentFieldId() { + // return this.type == Type.INT || this.type == Type.LONG; + //} + @Override public String getCurrentName() throws IOException diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java new file mode 100644 index 00000000..97548320 --- /dev/null +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -0,0 +1,115 @@ +// +// MessagePack for Java +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +package org.msgpack.jackson.dataformat; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.KeyDeserializer; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.deser.NullValueProvider; +import com.fasterxml.jackson.databind.deser.impl.JDKValueInstantiators; +import com.fasterxml.jackson.databind.deser.std.MapDeserializer; +import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.databind.type.TypeFactory; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.Collections; +import java.util.LinkedHashMap; + +import static org.junit.Assert.assertEquals; + +public class MessagePackDataformatForFieldIdTest +{ + + static class MessagePackMapDeserializer extends MapDeserializer + { + + public static KeyDeserializer keyDeserializer = new KeyDeserializer() + { + + @Override + public Object deserializeKey(String s, DeserializationContext deserializationContext) + throws IOException + { + JsonParser parser = deserializationContext.getParser(); + if (parser instanceof MessagePackParser p) { + // TODO: Uncomment + //if (p.isCurrentFieldId()) { + // return Integer.valueOf(s); + //} + } + return s; + } + }; + + public MessagePackMapDeserializer() + { + super( + TypeFactory.defaultInstance().constructMapType(Map.class, Object.class, Object.class), + JDKValueInstantiators.findStdValueInstantiator(null, LinkedHashMap.class), + keyDeserializer, null, null); + } + + public MessagePackMapDeserializer(MapDeserializer src, KeyDeserializer keyDeser, + JsonDeserializer valueDeser, TypeDeserializer valueTypeDeser, NullValueProvider nuller, + Set ignorable, Set includable) + { + super(src, keyDeser, valueDeser, valueTypeDeser, nuller, ignorable, includable); + } + + @Override + protected MapDeserializer withResolved(KeyDeserializer keyDeser, TypeDeserializer valueTypeDeser, + JsonDeserializer valueDeser, NullValueProvider nuller, Set ignorable, + Set includable) + { + return new MessagePackMapDeserializer(this, keyDeser, (JsonDeserializer) valueDeser, valueTypeDeser, + nuller, ignorable, includable); + } + } + + @Test + public void mixed() + { + ObjectMapper mapper = new ObjectMapper(new MessagePackFactory()) + .registerModule(new SimpleModule() + .addDeserializer(Map.class, new MessagePackMapDeserializer())); + + Map map = new HashMap<>(); + map.put(1, "one"); + map.put("2", "two"); + + byte[] bytes = mapper.writeValueAsBytes(map); + Map deserialized = mapper.readValue(bytes, new TypeReference>() {}); + + assertEquals(map, deserialized); + } +} From 481c22365fa6efc052b2941c27fdc6793c1baf72 Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Sun, 19 Jan 2025 11:01:22 -0600 Subject: [PATCH 2/8] Fix java 8 error --- .../dataformat/MessagePackDataformatForFieldIdTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 97548320..20ca4d1b 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -61,7 +61,8 @@ public Object deserializeKey(String s, DeserializationContext deserializationCon throws IOException { JsonParser parser = deserializationContext.getParser(); - if (parser instanceof MessagePackParser p) { + if (parser instanceof MessagePackParser) { + MessagePackParser p = (MessagePackParser) parser; // TODO: Uncomment //if (p.isCurrentFieldId()) { // return Integer.valueOf(s); From d8a371671c29c4b57170e0a82ce327a8d62fb49a Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Sun, 19 Jan 2025 11:07:39 -0600 Subject: [PATCH 3/8] Add missing import --- .../jackson/dataformat/MessagePackDataformatForFieldIdTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 20ca4d1b..1383f5df 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -44,6 +44,7 @@ import java.util.Set; import java.util.Collections; import java.util.LinkedHashMap; +import org.junit.Test; import static org.junit.Assert.assertEquals; From 757b6f9e7591be2a9400d658f363161ce3aa7830 Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Sun, 19 Jan 2025 11:15:18 -0600 Subject: [PATCH 4/8] Add missing throws --- .../dataformat/MessagePackDataformatForFieldIdTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 1383f5df..692f2a62 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -99,7 +99,8 @@ protected MapDeserializer withResolved(KeyDeserializer keyDeser, TypeDeserialize } @Test - public void mixed() + public void testMixedKeys() + throws IOException { ObjectMapper mapper = new ObjectMapper(new MessagePackFactory()) .registerModule(new SimpleModule() From 546d48b3c2c97e0fda40fe8991c33ae3ab02c424 Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Mon, 20 Jan 2025 09:31:41 -0600 Subject: [PATCH 5/8] Cleanup unused imports --- .../MessagePackDataformatForFieldIdTest.java | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 692f2a62..39742d55 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -15,7 +15,6 @@ // package org.msgpack.jackson.dataformat; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.DeserializationContext; @@ -29,20 +28,10 @@ import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.type.TypeFactory; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.OutputStream; -import java.math.BigInteger; -import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Collections; import java.util.LinkedHashMap; import org.junit.Test; @@ -50,13 +39,10 @@ public class MessagePackDataformatForFieldIdTest { - static class MessagePackMapDeserializer extends MapDeserializer { - public static KeyDeserializer keyDeserializer = new KeyDeserializer() { - @Override public Object deserializeKey(String s, DeserializationContext deserializationContext) throws IOException @@ -96,7 +82,7 @@ protected MapDeserializer withResolved(KeyDeserializer keyDeser, TypeDeserialize return new MessagePackMapDeserializer(this, keyDeser, (JsonDeserializer) valueDeser, valueTypeDeser, nuller, ignorable, includable); } - } + } @Test public void testMixedKeys() From 3c08b90cf3b6f66e13b9d614cc989fe91ab1889f Mon Sep 17 00:00:00 2001 From: Brendan Bartels <12563144+brenbar@users.noreply.github.com> Date: Mon, 20 Jan 2025 09:48:04 -0600 Subject: [PATCH 6/8] Cleanup test --- .../dataformat/MessagePackDataformatForFieldIdTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 39742d55..72770adb 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -97,8 +97,11 @@ public void testMixedKeys() map.put("2", "two"); byte[] bytes = mapper.writeValueAsBytes(map); - Map deserialized = mapper.readValue(bytes, new TypeReference>() {}); + Map deserializedInit = mapper.readValue(bytes, new TypeReference>() {}); - assertEquals(map, deserialized); + Map expected = new HashMap<>(map); + Map actual = new HashMap<>(deserializedInit); + + assertEquals(expected, actual); } } From 84cbc794bfee093b07f002432ad4d860fb56cc86 Mon Sep 17 00:00:00 2001 From: brenbar <12563144+brenbar@users.noreply.github.com> Date: Mon, 20 Jan 2025 16:35:54 +0000 Subject: [PATCH 7/8] Implement jackson field ids --- .../jackson/dataformat/MessagePackGenerator.java | 11 +++++------ .../msgpack/jackson/dataformat/MessagePackParser.java | 8 ++++---- .../MessagePackDataformatForFieldIdTest.java | 7 +++---- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java index dbda27d3..4748fb1b 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java @@ -513,12 +513,11 @@ private void writeByteArrayTextKey(byte[] text, int offset, int len) throws IOEx addValueNode(new String(text, offset, len, DEFAULT_CHARSET)); } - // TODO: Uncomment - //@Override - //public void writeFieldId(long id) throws IOException - //{ - // addKeyToStackTop(id); - //} + @Override + public void writeFieldId(long id) throws IOException + { + addKeyNode(id); + } @Override public void writeFieldName(String name) diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java index 0fddefb6..e59b7f57 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java @@ -621,10 +621,10 @@ public String currentName() return streamReadContext.getCurrentName(); } - // TODO: Uncomment - //public boolean isCurrentFieldId() { - // return this.type == Type.INT || this.type == Type.LONG; - //} + public boolean isCurrentFieldId() + { + return this.type == Type.INT || this.type == Type.LONG; + } @Override public String getCurrentName() diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 72770adb..73113a29 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -50,10 +50,9 @@ public Object deserializeKey(String s, DeserializationContext deserializationCon JsonParser parser = deserializationContext.getParser(); if (parser instanceof MessagePackParser) { MessagePackParser p = (MessagePackParser) parser; - // TODO: Uncomment - //if (p.isCurrentFieldId()) { - // return Integer.valueOf(s); - //} + if (p.isCurrentFieldId()) { + return Integer.valueOf(s); + } } return s; } From 8247f243c4c9141b237c815caf4c4838e7a6bb64 Mon Sep 17 00:00:00 2001 From: brenbar <12563144+brenbar@users.noreply.github.com> Date: Mon, 3 Feb 2025 06:23:58 +0000 Subject: [PATCH 8/8] Address feedback from @komamitsu --- .../dataformat/MessagePackFactory.java | 9 +++++- .../dataformat/MessagePackGenerator.java | 27 +++++++++++++++++- .../MessagePackDataformatForFieldIdTest.java | 28 ++++++++++++++++++- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java index bb5064f1..e7066dcd 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java @@ -39,6 +39,7 @@ public class MessagePackFactory private final MessagePack.PackerConfig packerConfig; private boolean reuseResourceInGenerator = true; private boolean reuseResourceInParser = true; + private boolean writeIntegerKeysAsStringKeys = true; private ExtensionTypeCustomDeserializers extTypeCustomDesers; public MessagePackFactory() @@ -74,6 +75,12 @@ public MessagePackFactory setReuseResourceInParser(boolean reuseResourceInParser return this; } + public MessagePackFactory setWriteIntegerKeysAsStringKeys(boolean writeIntegerKeysAsStringKeys) + { + this.writeIntegerKeysAsStringKeys = writeIntegerKeysAsStringKeys; + return this; + } + public MessagePackFactory setExtTypeCustomDesers(ExtensionTypeCustomDeserializers extTypeCustomDesers) { this.extTypeCustomDesers = extTypeCustomDesers; @@ -84,7 +91,7 @@ public MessagePackFactory setExtTypeCustomDesers(ExtensionTypeCustomDeserializer public JsonGenerator createGenerator(OutputStream out, JsonEncoding enc) throws IOException { - return new MessagePackGenerator(_generatorFeatures, _objectCodec, out, packerConfig, reuseResourceInGenerator); + return new MessagePackGenerator(_generatorFeatures, _objectCodec, out, packerConfig, reuseResourceInGenerator, writeIntegerKeysAsStringKeys); } @Override diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java index 4748fb1b..6bf02cb4 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java @@ -50,6 +50,7 @@ public class MessagePackGenerator private static final ThreadLocal messageBufferOutputHolder = new ThreadLocal<>(); private final OutputStream output; private final MessagePack.PackerConfig packerConfig; + private final boolean writeIntegerKeysAsStringKeys; private int currentParentElementIndex = -1; private int currentState = IN_ROOT; @@ -195,6 +196,7 @@ private MessagePackGenerator( this.messagePacker = packerConfig.newPacker(out); this.packerConfig = packerConfig; this.nodes = new ArrayList<>(); + this.writeIntegerKeysAsStringKeys = true; } public MessagePackGenerator( @@ -210,6 +212,24 @@ public MessagePackGenerator( this.messagePacker = packerConfig.newPacker(getMessageBufferOutputForOutputStream(out, reuseResourceInGenerator)); this.packerConfig = packerConfig; this.nodes = new ArrayList<>(); + this.writeIntegerKeysAsStringKeys = true; + } + + public MessagePackGenerator( + int features, + ObjectCodec codec, + OutputStream out, + MessagePack.PackerConfig packerConfig, + boolean reuseResourceInGenerator, + boolean writeIntegerKeysAsStringKeys) + throws IOException + { + super(features, codec); + this.output = out; + this.messagePacker = packerConfig.newPacker(getMessageBufferOutputForOutputStream(out, reuseResourceInGenerator)); + this.packerConfig = packerConfig; + this.nodes = new ArrayList<>(); + this.writeIntegerKeysAsStringKeys = writeIntegerKeysAsStringKeys; } private MessageBufferOutput getMessageBufferOutputForOutputStream( @@ -516,7 +536,12 @@ private void writeByteArrayTextKey(byte[] text, int offset, int len) throws IOEx @Override public void writeFieldId(long id) throws IOException { - addKeyNode(id); + if (this.writeIntegerKeysAsStringKeys) { + super.writeFieldId(id); + } + else { + addKeyNode(id); + } } @Override diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java index 73113a29..f5a08b39 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackDataformatForFieldIdTest.java @@ -87,7 +87,10 @@ protected MapDeserializer withResolved(KeyDeserializer keyDeser, TypeDeserialize public void testMixedKeys() throws IOException { - ObjectMapper mapper = new ObjectMapper(new MessagePackFactory()) + ObjectMapper mapper = new ObjectMapper( + new MessagePackFactory() + .setWriteIntegerKeysAsStringKeys(false) + ) .registerModule(new SimpleModule() .addDeserializer(Map.class, new MessagePackMapDeserializer())); @@ -103,4 +106,27 @@ public void testMixedKeys() assertEquals(expected, actual); } + + @Test + public void testMixedKeysBackwardsCompatiable() + throws IOException + { + ObjectMapper mapper = new ObjectMapper(new MessagePackFactory()) + .registerModule(new SimpleModule() + .addDeserializer(Map.class, new MessagePackMapDeserializer())); + + Map map = new HashMap<>(); + map.put(1, "one"); + map.put("2", "two"); + + byte[] bytes = mapper.writeValueAsBytes(map); + Map deserializedInit = mapper.readValue(bytes, new TypeReference>() {}); + + Map expected = new HashMap<>(); + expected.put("1", "one"); + expected.put("2", "two"); + Map actual = new HashMap<>(deserializedInit); + + assertEquals(expected, actual); + } }