Skip to content

Commit 08551ca

Browse files
committed
add a parser option to validate oneofs
The exception looks like: ``` scalapb.json4s.JsonFormatException: Overlapping keys in oneof: primitive, wrapper ```
1 parent ad73426 commit 08551ca

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

src/main/scala/scalapb/json4s/JsonFormat.scala

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ class Printer private (config: Printer.PrinterConfig) {
461461
object Parser {
462462
private final case class ParserConfig(
463463
isIgnoringUnknownFields: Boolean,
464+
failOnOverlappingOneofKeys: Boolean,
464465
mapEntriesAsKeyValuePairs: Boolean,
465466
formatRegistry: FormatRegistry,
466467
typeRegistry: TypeRegistry
@@ -472,6 +473,7 @@ class Parser private (config: Parser.ParserConfig) {
472473
this(
473474
Parser.ParserConfig(
474475
isIgnoringUnknownFields = false,
476+
failOnOverlappingOneofKeys = false,
475477
mapEntriesAsKeyValuePairs = false,
476478
JsonFormat.DefaultRegistry,
477479
TypeRegistry.empty
@@ -490,6 +492,7 @@ class Parser private (config: Parser.ParserConfig) {
490492
this(
491493
Parser.ParserConfig(
492494
isIgnoringUnknownFields = false,
495+
failOnOverlappingOneofKeys = false,
493496
mapEntriesAsKeyValuePairs = false,
494497
formatRegistry,
495498
typeRegistry
@@ -499,6 +502,9 @@ class Parser private (config: Parser.ParserConfig) {
499502
def ignoringUnknownFields: Parser =
500503
new Parser(config.copy(isIgnoringUnknownFields = true))
501504

505+
def failOnOverlappingOneofKeys: Parser =
506+
new Parser(config.copy(failOnOverlappingOneofKeys = true))
507+
502508
def mapEntriesAsKeyValuePairs: Parser =
503509
new Parser(config.copy(mapEntriesAsKeyValuePairs = true))
504510

@@ -527,7 +533,24 @@ class Parser private (config: Parser.ParserConfig) {
527533
value: JValue,
528534
skipTypeUrl: Boolean
529535
)(implicit cmp: GeneratedMessageCompanion[A]): A = {
530-
cmp.messageReads.read(fromJsonToPMessage(cmp, value, skipTypeUrl))
536+
val message = fromJsonToPMessage(cmp, value, skipTypeUrl)
537+
if (config.failOnOverlappingOneofKeys) {
538+
validateOneofs(message)
539+
}
540+
cmp.messageReads.read(message)
541+
}
542+
543+
private def validateOneofs[A <: GeneratedMessage](message: PMessage) = {
544+
message.value.keys
545+
.groupBy(_.containingOneof)
546+
.filter(x => x._1.isDefined && x._2.size != 1)
547+
.values
548+
.headOption
549+
.foreach(keys =>
550+
throw new JsonFormatException(
551+
s"Overlapping keys in oneof: ${keys.map(_.name).mkString(", ")}"
552+
)
553+
)
531554
}
532555

533556
private def fromJsonToPMessage(

src/test/scala/scalapb/json4s/JavaAssertions.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ class IgnoringUnknownParserContext {
3131
)
3232
}
3333

34+
class StrictOneofParserContext {
35+
implicit val pc: ParserContext = ParserContext(
36+
new Parser().failOnOverlappingOneofKeys,
37+
JavaJsonFormat.parser // by default the java parser fails on overlapping keys
38+
)
39+
}
40+
3441
trait JavaAssertions {
3542
self: AnyFlatSpec with Matchers =>
3643

src/test/scala/scalapb/json4s/JsonFormatSpec.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.google.protobuf.any.{Any => PBAny}
77
import com.google.protobuf.util.JsonFormat.{TypeRegistry => JavaTypeRegistry}
88
import com.google.protobuf.util.{JsonFormat => JavaJsonFormat}
99
import jsontest.custom_collection.{Guitar, Studio}
10+
import jsontest.oneof.OneOf
1011
import jsontest.test._
1112
import jsontest.test3._
1213
import org.json4s.JsonDSL._
@@ -674,6 +675,19 @@ class JsonFormatSpec
674675
JsonFormat.parser.fromJsonString[TestAllTypes](javaJson) must be(obj)
675676
}
676677

678+
"oneofs" should "fail for overlapping keys if failOnOverlappingOneofKeys" in new StrictOneofParserContext {
679+
val extraKey = """{"primitive": "", "wrapper": ""}"""
680+
assertFails(extraKey, OneOf)
681+
}
682+
683+
"oneofs" should "not fail for overlapping keys if failOnOverlappingOneofKeys with a scala parser" in new DefaultParserContext {
684+
val extraKey = """{"primitive": "", "wrapper": ""}"""
685+
val parsedScala = pc.scalaParser.fromJsonString[OneOf](extraKey)(OneOf)
686+
parsedScala must be(
687+
OneOf(field = OneOf.Field.Primitive(""))
688+
)
689+
}
690+
677691
"TestProto" should "be TestJsonWithMapEntriesAsKeyValuePairs when converted to Proto with mapEntriesAsKeyValuePairs setting" in {
678692
JsonFormat.printer.mapEntriesAsKeyValuePairs.toJson(TestProto) must be(
679693
parse(TestJsonWithMapEntriesAsKeyValuePairs)

0 commit comments

Comments
 (0)