-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jackson 2.16 support #564
jackson 2.16 support #564
Changes from all commits
20d336e
0509754
6f54740
0acec75
75fd3d3
e46f955
9fad47c
ea6fd21
16c16a3
cf2a742
f538da2
5a0e50f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,27 @@ pekko.serialization.jackson { | |
|
||
} | ||
|
||
#//#stream-read-constraints | ||
pekko.serialization.jackson { | ||
read { | ||
# see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.1/com/fasterxml/jackson/core/StreamReadConstraints.html | ||
# these defaults are the same as the defaults in `StreamReadConstraints` | ||
max-nesting-depth = 1000 | ||
max-number-length = 1000 | ||
max-string-length = 20000000 | ||
max-name-length = 50000 | ||
# max-document-length of -1 means unlimited | ||
max-document-length = -1 | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a blank line here |
||
|
||
write { | ||
# see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.1/com/fasterxml/jackson/core/StreamWriteConstraints.html | ||
# these defaults are the same as the defaults in `StreamWriteConstraints` | ||
max-nesting-depth = 1000 | ||
} | ||
} | ||
#//#stream-read-constraints | ||
|
||
#//#features | ||
pekko.serialization.jackson { | ||
# Configuration of the ObjectMapper serialization features. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,40 +17,42 @@ import java.util.Optional | |
import java.util.concurrent.ConcurrentHashMap | ||
import scala.annotation.nowarn | ||
import scala.collection.immutable | ||
import scala.util.Failure | ||
import scala.util.Success | ||
|
||
import com.fasterxml.jackson.annotation.JsonAutoDetect | ||
import com.fasterxml.jackson.annotation.JsonCreator | ||
import com.fasterxml.jackson.annotation.PropertyAccessor | ||
import com.fasterxml.jackson.core.JsonFactory | ||
import com.fasterxml.jackson.core.JsonFactoryBuilder | ||
import com.fasterxml.jackson.core.JsonGenerator | ||
import com.fasterxml.jackson.core.JsonParser | ||
import com.fasterxml.jackson.core.StreamReadFeature | ||
import com.fasterxml.jackson.core.StreamWriteFeature | ||
import com.fasterxml.jackson.core.json.JsonReadFeature | ||
import com.fasterxml.jackson.core.json.JsonWriteFeature | ||
import com.fasterxml.jackson.databind.DeserializationFeature | ||
import com.fasterxml.jackson.databind.MapperFeature | ||
import com.fasterxml.jackson.databind.Module | ||
import com.fasterxml.jackson.databind.ObjectMapper | ||
import com.fasterxml.jackson.databind.SerializationFeature | ||
import scala.util.{ Failure, Success } | ||
import com.fasterxml.jackson.annotation.{ JsonAutoDetect, JsonCreator, PropertyAccessor } | ||
import com.fasterxml.jackson.core.{ | ||
JsonFactory, | ||
JsonFactoryBuilder, | ||
JsonGenerator, | ||
JsonParser, | ||
StreamReadConstraints, | ||
StreamReadFeature, | ||
StreamWriteConstraints, | ||
StreamWriteFeature | ||
} | ||
import com.fasterxml.jackson.core.json.{ JsonReadFeature, JsonWriteFeature } | ||
import com.fasterxml.jackson.databind.{ | ||
DeserializationFeature, | ||
MapperFeature, | ||
Module, | ||
ObjectMapper, | ||
SerializationFeature | ||
} | ||
import com.fasterxml.jackson.databind.json.JsonMapper | ||
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule | ||
import com.typesafe.config.Config | ||
import org.apache.pekko | ||
import pekko.actor.ActorSystem | ||
import pekko.actor.ClassicActorSystemProvider | ||
import pekko.actor.DynamicAccess | ||
import pekko.actor.ExtendedActorSystem | ||
import pekko.actor.Extension | ||
import pekko.actor.ExtensionId | ||
import pekko.actor.ExtensionIdProvider | ||
import pekko.actor.{ | ||
ActorSystem, | ||
ClassicActorSystemProvider, | ||
DynamicAccess, | ||
ExtendedActorSystem, | ||
Extension, | ||
ExtensionId, | ||
ExtensionIdProvider | ||
} | ||
import pekko.actor.setup.Setup | ||
import pekko.annotation.InternalStableApi | ||
import pekko.event.Logging | ||
import pekko.event.LoggingAdapter | ||
import pekko.event.{ Logging, LoggingAdapter } | ||
import pekko.util.unused | ||
import pekko.util.OptionConverters._ | ||
|
||
|
@@ -68,7 +70,7 @@ object JacksonObjectMapperProvider extends ExtensionId[JacksonObjectMapperProvid | |
*/ | ||
def configForBinding(bindingName: String, systemConfig: Config): Config = { | ||
val basePath = "pekko.serialization.jackson" | ||
val baseConf = systemConfig.getConfig("pekko.serialization.jackson") | ||
val baseConf = systemConfig.getConfig(basePath) | ||
if (systemConfig.hasPath(s"$basePath.$bindingName")) | ||
systemConfig.getConfig(s"$basePath.$bindingName").withFallback(baseConf) | ||
else | ||
|
@@ -81,15 +83,31 @@ object JacksonObjectMapperProvider extends ExtensionId[JacksonObjectMapperProvid | |
config: Config, | ||
baseJsonFactory: Option[JsonFactory]): JsonFactory = { | ||
|
||
val streamReadConstraints = StreamReadConstraints.builder() | ||
.maxNestingDepth(config.getInt("read.max-nesting-depth")) | ||
.maxNumberLength(config.getInt("read.max-number-length")) | ||
.maxStringLength(config.getInt("read.max-string-length")) | ||
.maxNameLength(config.getInt("read.max-name-length")) | ||
.maxDocumentLength(config.getLong("read.max-document-length")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do these configurations have default values, or users must specify the configuration? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We ship reference conf with the defaults. Users can override the values by setting them in their own application.conf or application.json. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how Akka/Pekko has always worked. |
||
.build() | ||
|
||
val streamWriteConstraints = StreamWriteConstraints.builder() | ||
.maxNestingDepth(config.getInt("write.max-nesting-depth")) | ||
.build() | ||
|
||
val jsonFactory: JsonFactory = baseJsonFactory match { | ||
case Some(factory) => | ||
// Issue #28918 not possible to use new JsonFactoryBuilder(jsonFactory) here. | ||
// It doesn't preserve the formatParserFeatures and formatGeneratorFeatures in | ||
// CBORFactor. Therefore we use JsonFactory and configure the features with mappedFeature | ||
// instead of using JsonFactoryBuilder (new in Jackson 2.10.0). | ||
factory | ||
factory.setStreamReadConstraints(streamReadConstraints) | ||
factory.setStreamWriteConstraints(streamWriteConstraints) | ||
case None => | ||
new JsonFactoryBuilder().build() | ||
new JsonFactoryBuilder() | ||
.streamReadConstraints(streamReadConstraints) | ||
.streamWriteConstraints(streamWriteConstraints) | ||
.build() | ||
} | ||
|
||
val configuredStreamReadFeatures = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You 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.apache.pekko.serialization.jackson | ||
|
||
import com.typesafe.config.ConfigFactory | ||
import org.scalatest.BeforeAndAfterAll | ||
import org.scalatest.matchers.should.Matchers | ||
import org.scalatest.wordspec.AnyWordSpecLike | ||
import org.apache.pekko | ||
import pekko.actor.{ ActorSystem, ExtendedActorSystem } | ||
import pekko.testkit.TestKit | ||
|
||
class JacksonFactorySpec extends TestKit(ActorSystem("JacksonFactorySpec")) | ||
with AnyWordSpecLike with Matchers with BeforeAndAfterAll { | ||
|
||
private val defaultConfig = ConfigFactory.defaultReference() | ||
private val dynamicAccess = system.asInstanceOf[ExtendedActorSystem].dynamicAccess | ||
private val objectMapperFactory = new JacksonObjectMapperFactory | ||
|
||
override def afterAll(): Unit = { | ||
super.afterAll() | ||
system.terminate() | ||
} | ||
|
||
"Jackson Factory config" must { | ||
"support StreamReadConstraints" in { | ||
val bindingName = "testJackson" | ||
val maxNumLen = 987 | ||
val maxStringLen = 1234567 | ||
val maxDocLen = 123456789L | ||
val maxNestingDepth = 5 | ||
val config = ConfigFactory.parseString( | ||
s"""pekko.serialization.jackson.read.max-number-length=$maxNumLen | ||
|pekko.serialization.jackson.read.max-string-length=$maxStringLen | ||
|pekko.serialization.jackson.read.max-document-length=$maxDocLen | ||
|pekko.serialization.jackson.read.max-nesting-depth=$maxNestingDepth | ||
|""".stripMargin) | ||
.withFallback(defaultConfig) | ||
val jacksonConfig = JacksonObjectMapperProvider.configForBinding(bindingName, config) | ||
val mapper = JacksonObjectMapperProvider.createObjectMapper( | ||
bindingName, None, objectMapperFactory, jacksonConfig, dynamicAccess, None) | ||
val streamReadConstraints = mapper.getFactory.streamReadConstraints() | ||
streamReadConstraints.getMaxNumberLength shouldEqual maxNumLen | ||
streamReadConstraints.getMaxStringLength shouldEqual maxStringLen | ||
streamReadConstraints.getMaxDocumentLength shouldEqual maxDocLen | ||
streamReadConstraints.getMaxNestingDepth shouldEqual maxNestingDepth | ||
} | ||
"support StreamWriteConstraints" in { | ||
val bindingName = "testJackson" | ||
val maxNestingDepth = 54321 | ||
val config = ConfigFactory.parseString( | ||
s"pekko.serialization.jackson.write.max-nesting-depth=$maxNestingDepth") | ||
.withFallback(defaultConfig) | ||
val jacksonConfig = JacksonObjectMapperProvider.configForBinding(bindingName, config) | ||
val mapper = JacksonObjectMapperProvider.createObjectMapper( | ||
bindingName, None, objectMapperFactory, jacksonConfig, dynamicAccess, None) | ||
val streamWriteConstraints = mapper.getFactory.streamWriteConstraints() | ||
streamWriteConstraints.getMaxNestingDepth shouldEqual maxNestingDepth | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems more clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly don't think adding the word
constraints
to the config names here makes them any clearer. Config names need to balance descriptiveness with conciseness.