#30 Add dropAlways annotation #31

JasnaMRB · 2020-06-23T18:00:37Z

Lets you annotate a case class field to flag it as something to ignore during serialization.

Resolves #30

 ✘  AWS: care-rally-dev  jasna.blemberg@jasna-blemberg  ~/code/weePickle   issue30  mill __.test
[256/2037] implicits.jvm[2.11.12].compile 
[info] Compiling 1 Scala source to /Users/jasna.blemberg/code/weePickle/out/implicits/jvm/2.11.12/compile/dest/classes ...
[info] Done compiling.
[271/2037] implicits.jvm[2.11.12].reportBinaryIssues 
1 targets failed
implicits.jvm[2.11.12].reportBinaryIssues java.lang.RuntimeException: Compared to artifact: /Users/jasna.blemberg/Library/Caches/Coursier/v1/https/dl.bintray.com/rallyhealth/maven/com/rallyhealth/weepickle-implicits-v1_2.11/1.0.1/weepickle-implicits-v1_2.11-1.0.1.jar
found 3 binary incompatibilities:
method copy(Int,java.lang.String,java.lang.String,scala.reflect.api.Types#TypeApi,Boolean,Boolean,Boolean,scala.reflect.api.Trees#TreeApi,scala.reflect.api.Names#TermNameApi,scala.reflect.api.Names#TermNameApi)com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults#Argument in class com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults#Argument does not have a correspondent in current version
method this(com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults,Int,java.lang.String,java.lang.String,scala.reflect.api.Types#TypeApi,Boolean,Boolean,Boolean,scala.reflect.api.Trees#TreeApi,scala.reflect.api.Names#TermNameApi,scala.reflect.api.Names#TermNameApi)Unit in class com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults#Argument does not have a correspondent in current version
method apply(Int,java.lang.String,java.lang.String,scala.reflect.api.Types#TypeApi,Boolean,Boolean,Boolean,scala.reflect.api.Trees#TreeApi,scala.reflect.api.Names#TermNameApi,scala.reflect.api.Names#TermNameApi)com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults#Argument in object com.rallyhealth.weepickle.v1.implicits.internal.Macros#DeriveDefaults#Argument does not have a correspondent in current version
    scala.sys.package$.error(package.scala:30)
    ammonite.$file.build$MiMa.$anonfun$reportBinaryIssues$2(build.sc:377)
    mill.define.Task$MappedDest.evaluate(Task.scala:337)

russellremple · 2020-06-24T06:25:11Z

implicits/src/com/rallyhealth/weepickle/v1/implicits/internal/Macros.scala

+      aggregate: TermName,
+      omitJson: Boolean


This is definitely the right way to solve it, but you will hit the same binary compatibility issues that I hit the last time I attempted to change the Arguments structure. I have a refactoring in #24 that take all these structures private, but it includes other changes I don't think we want to make now. I'll remove those changes and just include the refactoring.

Thanks, Russ. I'll hold off on begging for stamps for this PR until yours is in. 🙂 Then build off your changes.

plokhotnyuk · 2020-06-24T06:54:41Z

weepickle/test/src/com/rallyhealth/weepickle/v1/ClassDefs.scala

@@ -192,6 +192,10 @@ object Annotated {
  object C {
    implicit def rw: RW[C] = WeePickle.macroFromTo
  }
+  case class D(a: String, @jsonIgnore b: Option[String], c: String)


How the case class instance can be parsed parsed back if @jsonIgnore or @dropAlways is used for a non-optional field without default values?

Yes, good catch. I was wondering about this, whether it should be ignored on both serialization and deserialization, or require a default or Option type. I've previously used Jackson's @JsonIgnore annotation for fields that a team's frontend consumers no longer supported, but another team's direct backend consumers still used (in a monorepo). I'll have to check my current use case: lib-pokitdok-client#PokitdokResponse has a lot of redacted fields with Play Json...

@dropalways, not @JsonIgnore? Note that @dropalways is more generic -- you may be serializing to YAML and not Json.

Yeah, Doug recommended I be more generic since it looks like these macros handle several serialization types. Would make behavior more consistent as well across types. (Though I guess we could get granular on types in the future, if the need arises.)

The macro doesn't have any idea if your case class will become json, yaml, msgpack, cbor, xml, csv, etc.

We already have @dropDefault. Reusing the "drop" term made sense to me here, since it's a similar effect.

russellremple · 2020-06-24T16:20:22Z

weepickle/test/src/com/rallyhealth/weepickle/v1/ClassDefs.scala

@@ -192,6 +192,10 @@ object Annotated {
  object C {
    implicit def rw: RW[C] = WeePickle.macroFromTo
  }
+  case class D(a: String, @jsonIgnore b: Option[String], c: String)


@dropalways, not @JsonIgnore? Note that @dropalways is more generic -- you may be serializing to YAML and not Json.

russellremple · 2020-06-24T16:24:47Z

weepickle/test/src/com/rallyhealth/weepickle/v1/MacroTests.scala

+      test("ignoreAnnotatedFieldsWhenSerializing") {
+        import Annotated.D
+        val w = FromScala(D("a", Some("b"), "c")).transform(ToJson.string)
+        assert(w == """{"a":"a","c":"c"}""")
+      }


Test FromJson too. If b was a String rather than an Option[String] (which has an assumed default of None), how would that work? Seems like a potentially problematic asymmetry. Perhaps we should require that all @dropalways fields have a default (assumed or explicit) to avoid the issue?

Right, definitely thinking along the same vein about requiring a default. #31 (comment)

htmldoug · 2020-11-06T19:11:05Z

Closing due to age. Please reopen if you want to revive it.

JasnaMRB changed the title ~~#30 Add jsonIgnoreannotation~~ #30 Add jsonIgnore annotation Jun 23, 2020

Init jsonIgnore

aff9668

JasnaMRB force-pushed the issue30 branch from 7cb7bf8 to aff9668 Compare June 23, 2020 19:26

russellremple reviewed Jun 24, 2020

View reviewed changes

Something that works, at least for POC before Russ refactors everything.

f6f769f

JasnaMRB changed the title ~~#30 Add jsonIgnore annotation~~ #30 Add dropAlways annotation Jun 24, 2020

plokhotnyuk reviewed Jun 24, 2020

View reviewed changes

Consistent naming

1ebfae3

JasnaMRB force-pushed the issue30 branch from b5cf731 to 1ebfae3 Compare June 24, 2020 06:56

russellremple suggested changes Jun 24, 2020

View reviewed changes

htmldoug closed this Nov 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#30 Add dropAlways annotation #31

#30 Add dropAlways annotation #31

JasnaMRB commented Jun 23, 2020 •

edited

Loading

russellremple Jun 24, 2020

JasnaMRB Jun 24, 2020

plokhotnyuk Jun 24, 2020

JasnaMRB Jun 24, 2020 •

edited

Loading

russellremple Jun 24, 2020

JasnaMRB Jun 24, 2020 •

edited

Loading

htmldoug Jun 25, 2020

russellremple Jun 24, 2020

russellremple Jun 24, 2020

JasnaMRB Jun 24, 2020

htmldoug commented Nov 6, 2020

#30 Add dropAlways annotation #31

#30 Add dropAlways annotation #31

Conversation

JasnaMRB commented Jun 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasnaMRB Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasnaMRB Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htmldoug commented Nov 6, 2020

JasnaMRB commented Jun 23, 2020 •

edited

Loading

JasnaMRB Jun 24, 2020 •

edited

Loading

JasnaMRB Jun 24, 2020 •

edited

Loading