valentiay / phobos

Efficient and expressive XML data-binding library for Scala
Apache License 2.0
20 stars 5 forks source link

Law does not work with `usingElementNamesAsDiscriminator` #14

Closed geny200 closed 4 months ago

geny200 commented 6 months ago

Hi, I found a case where the law x == decode(encode(x)) is not fulfilled.

Example:

import phobos.configured.ElementCodecConfig
import phobos.decoding._
import phobos.derivation.semiauto._
import phobos.encoding._

sealed trait Foo

case class Foo1(a: String) extends Foo

case class Foo2(b: Int) extends Foo

object Foo {
  val config: ElementCodecConfig = ElementCodecConfig.default.usingElementNamesAsDiscriminator

  implicit val foo1Encoder: ElementEncoder[Foo1] = deriveElementEncoder[Foo1]
  implicit val foo2Encoder: ElementEncoder[Foo2] = deriveElementEncoder[Foo2]
  implicit val fooEncoder: ElementEncoder[Foo]   = deriveElementEncoderConfigured[Foo](config)

  implicit val foo1Decoder: ElementDecoder[Foo1] = deriveElementDecoder[Foo1]
  implicit val foo2Decoder: ElementDecoder[Foo2] = deriveElementDecoder[Foo2]
  implicit val fooDecoder: ElementDecoder[Foo]   = deriveElementDecoderConfigured[Foo](config)
}

case class Bar[T](foo: T)

object Bar {
  implicit def barEncoder[T: ElementEncoder]: ElementEncoder[Bar[T]] = deriveElementEncoder[Bar[T]]
  implicit def barDecoder[T: ElementDecoder]: ElementDecoder[Bar[T]] = deriveElementDecoder[Bar[T]]

  implicit def barXmlEncoder[T: ElementEncoder]: XmlEncoder[Bar[T]] = XmlEncoder.fromElementEncoder[Bar[T]]("Bar")
  implicit def barXmlDecoder[T: ElementDecoder]: XmlDecoder[Bar[T]] = XmlDecoder.fromElementDecoder[Bar[T]]("Bar")
}

object App {
  def main(args: Array[String]): Unit = {
    println(XmlEncoder[Bar[Foo]].encode(Bar(Foo1("without"))))
    // <?xml version='1.0' encoding='UTF-8'?><Bar><Foo1><a>without</a></Foo1></Bar>
    // serialization don't have level with `<foo>` (field in case class `Bar`)

    println(XmlEncoder[Bar[String]].encode(Bar("with")))
    // <?xml version='1.0' encoding='UTF-8'?><Bar><foo>with</foo></Bar>
    // serialization contains `<foo>` (field in case class `Bar`)

    val withFooXml = """<?xml version='1.0' encoding='UTF-8'?><Bar><Foo1><a>without</a></Foo1></Bar>"""
    println(XmlDecoder[Bar[Foo]].decode(withFooXml))
    // Ok, if add `@default` in Bar, otherwise it will be failed:
    // {{{
    // phobos.decoding.DecodingError: Error while decoding XML: Element 'foo' is missing or invalid
    //  In element 'Bar'
    // }}}

    val withStrXml = """<?xml version='1.0' encoding='UTF-8'?><Bar><foo>with</foo></Bar>"""
    println(XmlDecoder[Bar[String]].decode(withStrXml)) // Always Ok

    // Check law
    val x: Bar[Foo] = Bar(Foo1("without"))
    assert(
      XmlEncoder[Bar[Foo]]
        .encode(x)
        .toOption
        .flatMap(XmlDecoder[Bar[Foo]].decode(_).toOption)
        .contains(x),
    )
  }
}
valentiay commented 6 months ago

Hi! Thanks for reporting. This is actually the case for all codecs configured with usingElementNamesAsDiscriminator if the @default annotation is not used for parameter with such codec.

Here's a little more minimalistic example ```scala import phobos.configured.ElementCodecConfig import phobos.decoding._ import phobos.derivation.semiauto._ import phobos.encoding._ sealed trait Foo case class Foo1(a: String) extends Foo case class Foo2(b: Int) extends Foo object Foo { val config: ElementCodecConfig = ElementCodecConfig.default.usingElementNamesAsDiscriminator implicit val foo1Encoder: ElementEncoder[Foo1] = deriveElementEncoder[Foo1] implicit val foo2Encoder: ElementEncoder[Foo2] = deriveElementEncoder[Foo2] implicit val fooEncoder: ElementEncoder[Foo] = deriveElementEncoderConfigured[Foo](config) implicit val foo1Decoder: ElementDecoder[Foo1] = deriveElementDecoder[Foo1] implicit val foo2Decoder: ElementDecoder[Foo2] = deriveElementDecoder[Foo2] implicit val fooDecoder: ElementDecoder[Foo] = deriveElementDecoderConfigured[Foo](config) } case class Bar(foo: Foo) object Bar { implicit val barEncoder: ElementEncoder[Bar] = deriveElementEncoder[Bar] implicit val barDecoder: ElementDecoder[Bar] = deriveElementDecoder[Bar] implicit val barXmlEncoder: XmlEncoder[Bar] = XmlEncoder.fromElementEncoder[Bar]("Bar") implicit val barXmlDecoder: XmlDecoder[Bar] = XmlDecoder.fromElementDecoder[Bar]("Bar") } object App { def main(args: Array[String]): Unit = { println(XmlEncoder[Bar].encode(Bar(Foo1("without")))) // without // serialization don't have level with `` (field in case class `Bar`) val withFooXml = """without""" println(XmlDecoder[Bar].decode(withFooXml)) // {{{ // phobos.decoding.DecodingError: Error while decoding XML: Element 'foo' is missing or invalid // In element 'Bar' // }}} // Check law val x: Bar = Bar(Foo1("without")) assert( XmlEncoder[Bar] .encode(x) .toOption .flatMap(XmlDecoder[Bar].decode(_).toOption) .contains(x), ) } } ```

It's certainly a flaw, but it's caused by decoder derivation design. Derivation compares XML element name and case class parameter name to select which decoder to use. This cannot be applied for decoders with usingElementNamesAsDiscriminator, because in this case XML element name does not match the parameter name. The @default annotation is required to solve this problem. Fixing this flaw requires a complete redesign of Phobos interfaces, and so far I don't have ideas how to do it.

I suppose, the best thing we can do for now is enhance usingElementNamesAsDiscriminator documentation. The need for @default annotation must be described there. I can do it in near future, you may also contribute, if you wish

geny200 commented 6 months ago

The @default annotation is required to solve this problem

Actually, in general, this won't help

An example when default doesn't help ```scala import phobos.configured.ElementCodecConfig import phobos.decoding._ import phobos.derivation.semiauto._ import phobos.encoding._ import phobos.syntax.default sealed trait Foo case class Foo1(a: String) extends Foo case class Foo2(b: Int) extends Foo object Foo { val config: ElementCodecConfig = ElementCodecConfig.default.usingElementNamesAsDiscriminator implicit val foo1Encoder: ElementEncoder[Foo1] = deriveElementEncoder[Foo1] implicit val foo2Encoder: ElementEncoder[Foo2] = deriveElementEncoder[Foo2] implicit val fooEncoder: ElementEncoder[Foo] = deriveElementEncoderConfigured[Foo](config) implicit val foo1Decoder: ElementDecoder[Foo1] = deriveElementDecoder[Foo1] implicit val foo2Decoder: ElementDecoder[Foo2] = deriveElementDecoder[Foo2] implicit val fooDecoder: ElementDecoder[Foo] = deriveElementDecoderConfigured[Foo](config) } case class Bar(@default foo: Foo, kek: Foo) object Bar { implicit val barEncoder: ElementEncoder[Bar] = deriveElementEncoder[Bar] implicit val barDecoder: ElementDecoder[Bar] = deriveElementDecoder[Bar] implicit val barXmlEncoder: XmlEncoder[Bar] = XmlEncoder.fromElementEncoder[Bar]("Bar") implicit val barXmlDecoder: XmlDecoder[Bar] = XmlDecoder.fromElementDecoder[Bar]("Bar") } object App { def main(args: Array[String]): Unit = { println(XmlEncoder[Bar].encode(Bar(Foo1("without"), Foo1("cool")))) // coolwithout // serialization don't have level with '' and '' (field in case class 'Bar') val withFooXml = """coolwithout""" println(XmlDecoder[Bar].decode(withFooXml)) // {{{ // phobos.decoding.DecodingError: Error while decoding XML: Element is already decoded (Most likely it occurred more than once) // In element 'Foo1' // in element 'Bar' // }}} // Check law val x: Bar = Bar(Foo1("without"), Foo1("cool")) assert( XmlEncoder[Bar] .encode(x) .toOption .flatMap(XmlDecoder[Bar].decode(_).toOption) .contains(x), ) } } ```
valentiay commented 4 months ago

Hi!

Actually, in general, this won't help

Field kek: Foo in Bar does not have the @default annotation, so still it's the same case. If a case class has two fields using element names as a discriminator, Phobos won't understand which field it should use for decoding. A solution for this case is to use List: @default foo: List[Foo]

valentiay commented 4 months ago

Merged #20 with documentation update, so closing this issue