Skip to content
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

support default values for case class fields #566

Merged
merged 8 commits into from
Jun 11, 2024

Conversation

mberndt123
Copy link
Contributor

@mberndt123 mberndt123 commented Jan 13, 2024

It seems like this is all that's required to support defaults for case class fields in Scala 3.

Users need -Yretain-trees in their scalacOptions as per the documentation:
https://github.com/softwaremill/magnolia#limitations

I also bumped sbt as it crashes otherwise on recent OpenJDK versions.

@mberndt123 mberndt123 changed the title support defaults support default values for case class fields Jan 13, 2024
@soujiro32167
Copy link
Contributor

This is great! I can't get my test to pass though...

/*
 * Copyright 2019-2023 OVO Energy Limited
 *
 * SPDX-License-Identifier: Apache-2.0
 */

package vulcan.generic

import vulcan.{AvroError, Codec}

final class AvroFieldDefaultSpec extends CodecBase {

  sealed trait Enum extends Product {
    self =>
    def value: String = self.productPrefix
  }

  object Enum {
    case object A extends Enum

    case object B extends Enum

    implicit val codec: Codec[Enum] = deriveEnum(
      symbols = List(A.value, B.value),
      encode = _.value,
      decode = {
        case "A" => Right(A)
        case "B" => Right(B)
        case other => Left(AvroError(s"Invalid S: $other"))
      }
    )
  }

  sealed trait Union

  object Union {
    case class A(a: Int) extends Union

    case class B(b: String) extends Union

    implicit val codec: Codec[Union] = Codec.derive
  }

  describe("AvroFieldDefault") {
    it("should create a schema with a default for a field") {
      case class Foo(
        a: Int = 1,
        b: String = "foo",
      )

      object Foo {
        implicit val codec: Codec[Foo] = Codec.derive
      }

      assert(Foo.codec.schema.exists(_.getField("a").defaultVal() == 1))
      assert(Foo.codec.schema.exists(_.getField("b").defaultVal() == "foo"))
    }
    
    it("should fail when the default value is not of the correct type") {
      case class InvalidDefault(
        a: Int
      )
      object InvalidDefault {
        implicit val codec: Codec[InvalidDefault] = Codec.derive
      }

      assertSchemaError[InvalidDefault]
    }

   it("should fail when annotating an Option") {
     case class InvalidDefault2(
       a: Option[String] = Some("foo")
     )
     object InvalidDefault2 {
       implicit val codec: Codec[InvalidDefault2] = Codec.derive
     }

     assertSchemaError[InvalidDefault2]
   }

   it("should succeed when annotating an enum first element") {
     case class HasSFirst(
                           s: Enum = Enum.A
                         )
     object HasSFirst {
       implicit val codec: Codec[HasSFirst] = Codec.derive
     }

     assert(HasSFirst.codec.schema.exists(_.getField("s").defaultVal() == "A"))
   }

   it("should succeed when annotating an enum second element") {
     case class HasSSecond(
        s: Enum = Enum.B
      )
     object HasSSecond {
       implicit val codec: Codec[HasSSecond] = Codec.derive
     }

     assert(HasSSecond.codec.schema.exists(_.getField("s").defaultVal() == "B"))
   }

   it("should succeed with the first member of a union"){
     case class HasUnion(
       u: Union = Union.A(1)
     )
     object HasUnion {
       implicit val codec: Codec[HasUnion] = Codec.derive
     }

     case class Empty()
     object Empty {
       implicit val codec: Codec[Empty] = Codec.derive
     }

     assertSchemaIs[HasUnion](
       """{"type":"record","name":"HasUnion","namespace":"vulcan.generic.AvroFieldDefaultSpec.<local AvroFieldDefaultSpec>","fields":[{"name":"u","type":[{"type":"record","name":"A","namespace":"vulcan.generic.AvroFieldDefaultSpec.Union","fields":[{"name":"a","type":"int"}]},{"type":"record","name":"B","namespace":"vulcan.generic.AvroFieldDefaultSpec.Union","fields":[{"name":"b","type":"string"}]}],"default":{"a":1}}]}"""
     )

     val result = unsafeDecode[HasUnion](unsafeEncode[Empty](Empty()))

     assert(result == HasUnion(Union.A(1)))

   }

   it("should fail with the second member of a union"){
     case class HasUnionSecond(
       u: Union = Union.B("foo")
     )
     object HasUnionSecond {
       implicit val codec: Codec[HasUnionSecond] = Codec.derive
     }

     assertSchemaError[HasUnionSecond]
   }
  }
}

@mberndt123
Copy link
Contributor Author

mberndt123 commented Jan 14, 2024

Did you compile with the -Yretain-trees compiler flag?

@mberndt123
Copy link
Contributor Author

mberndt123 commented Jan 14, 2024

Oh, I had added it to Scala 3 only. I've made the same change for Scala 2 now, for the poor sods stuck on the legacy version 😉

@mberndt123
Copy link
Contributor Author

I've stolen one of your tests to add it to your PR. Seems to work if you enable the -Yretain-trees compiler flag!

@soujiro32167
Copy link
Contributor

I was sure it was enabled 🤔
Thanks for checking

@mberndt123
Copy link
Contributor Author

@soujiro32167
So does it work for you now?

@soujiro32167
Copy link
Contributor

I've discovered some fascinating stuff:

   it("should fail when annotating an Option") {
     case class InvalidDefault2(
       a: Option[String] = Some("foo")
     )
     object InvalidDefault2 {
       implicit val codec: Codec[InvalidDefault2] = Codec.derive
     }

     assertSchemaError[InvalidDefault2]
   }

results in a stack overflow error:

An exception or error caused a run to abort. 
java.lang.StackOverflowError
	at vulcan.Codec$OptionCodec.<init>(Codec.scala:1017)
	at vulcan.Codec$.option(Codec.scala:995)
	at vulcan.generic.AvroFieldDefaultSpec$InvalidDefault2$2$.<init>(AvroFieldDefaultSpec.scala:64)
	at vulcan.generic.AvroFieldDefaultSpec.InvalidDefault2$lzycompute$1(AvroFieldDefaultSpec.scala:63)
	at vulcan.generic.AvroFieldDefaultSpec.vulcan$generic$AvroFieldDefaultSpec$$InvalidDefault2$3(AvroFieldDefaultSpec.scala:63)
	at vulcan.generic.AvroFieldDefaultSpec$InvalidDefault2$2$.$anonfun$codec$19(AvroFieldDefaultSpec.scala:64)

while

case class InvalidDefault2(
                            a: Option[String] = Some("foo")
                          )
object InvalidDefault2 {
  implicit val codec: Codec[InvalidDefault2] = Codec.derive
}

final class AvroFieldDefaultSpec extends CodecBase {
  describe("AvroFieldDefault") {
    it("should create a schema with a default for a field") {
      assert(Foo.codec.schema.exists(_.getField("a").defaultVal() == 1))
      assert(Foo.codec.schema.exists(_.getField("b").defaultVal() == "foo"))
    }

   it("should fail when annotating an Option") {

     assertSchemaError[InvalidDefault2]
   }

works just fine 🤯

@soujiro32167
Copy link
Contributor

Added a PR with all the tests mberndt123#1

@mberndt123
Copy link
Contributor Author

@bplommer
Any chance this could be merged?

@mberndt123
Copy link
Contributor Author

Since this project appears to be unmaintained, we've decided to use avro4s instead 🤷🏻‍♂️

@ayoub-benali
Copy link
Contributor

Hi, I just realised that #579 aims at tackling the same problem.
@bplommer Is it possible to give this PR a review ?

@soujiro32167
Copy link
Contributor

@mberndt123 could you merge mberndt123#1 into your repo? In case this PR gets reviewed

Copy link
Contributor

@ayoub-benali ayoub-benali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work @mberndt123 👍
I got write access today so I was able to review and hopefully we can release this fix very soon :)

Comment on lines +11 to +18
case class Foo(
a: Int = 1,
b: String = "foo",
)

object Foo {
implicit val codec: Codec[Foo] = Codec.derive
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please move the class definition to modules/generic/src/test/scala/vulcan/generic/examples just to be consistent with the other tests ?


import vulcan.Codec

case class Foo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a field with type Option and a default value ?
Just to be sure that null is serialized correctly

@ayoub-benali
Copy link
Contributor

@soujiro32167 would you be able to make a PR later to add mberndt123#1 directly here ?

@soujiro32167
Copy link
Contributor

@ayoub-benali here we go #593

@ayoub-benali ayoub-benali merged commit 1e5f699 into fd4s:master Jun 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants