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

Separate exported instance type and internal dispatch/combine type #107

Open
vpavkin opened this issue Jun 15, 2018 · 13 comments · Fixed by pureconfig/pureconfig#703
Open

Comments

@vpavkin
Copy link
Contributor

vpavkin commented Jun 15, 2018

This is a more principled version of #89. I found at least two cases when the typeclass intance being derived would have a type different from the typeclass used for dispatch and combine.

1) ObjectEncoder[A]/Encoder[A]

In circe, generic derivation provides not just an Encoder[A], but a more specific ObjectEncoder[A], which means that the serialization result is always going to be a json object. But for all the inner steps of the derivation, a more general Encoder[A] typeclass is looked for and used. This makes sense - object internals don't have to be objects themselves.

2) Exported[Decoder[A]]/Decoder[A]

This is how prioritization of derived instances works in circe. auto derives an instance of Exported[Decoder[A]] (code). And there's a low-priority implicit conversion Exported[Decoder[A]] => Decoder[A], which allows the default instances to be selected before the derived ones.

But of course, under the hood derivation of Exported[Decoder[A]] combines and dispatches on just Decoder[A] (otherwise default instances would not be picked up).

Possible Solution

I feel that this is not specific to circe and similar concerns might arise in other contexts.

I understand this would complicate things quite a bit, but ideally for proper circe auto derivation we'd need something like this:

object MagnoliaDecoder {

  type ExportedTypeclass[T] = Exported[Decoder[T]]
  type InternalLookup[T] = Decoder[T]

  // run outer level combine for the exported instance
  // ExportedCaseClass is the same as CaseClass, but parametersArray 
  // will already have InternalLookup instances 
  def combineExported[T](
    caseClass: ExportedCaseClass[ExportedTypeclass, InternalLookup, T]): ExportedTypeclass[T] = ???

  // same idea for the dispatch on the outer level instance
  def dispatchExported[T](
    sealedTrait: ExportedSealedTrait[ExportedTypeclass, InternalLookup, T]): ExportedTypeclass[T] = ???

  // all the internal derivation works in the same way as before - using InternalLookup

  def combine[T](caseClass:    CaseClass[InternalLookup, T]): InternalLookup[T] = ???
  def dispatch[T](sealedTrait: SealedTrait[InternalLookup, T]): InternalLookup[T] = ???

  implicit def magnoliaDecoder[T]: ExportedTypeclass[T] = macro Magnolia.gen[T]

WDYT, how hard is that gonna be?

P.S. Thanks for 0.8.0, semiauto seems to be working perfectly! 👍

@vpavkin
Copy link
Contributor Author

vpavkin commented Jun 15, 2018

I still know zero about internals of magnolia, but maybe a simpler approach would be to just ask end-user for a mapping: Internal => Exported, and magnolia would just run the conversion after following regular derivation steps.

Hope that helps :)

@joroKr21
Copy link
Contributor

joroKr21 commented Jun 16, 2018

I think this would be really useful and can be generalized - typeclass derivation doesn't have to be regular in many ways:

  1. It might not require an instance for all subparts (see Derivation with "exists" semantics #61)
  2. It might not require the same type of instances (this issue)
  3. It might or might not work for recursive types (see Is my data recursive? #55)

Another simple example would be a CSV parser. It can handle only products of primitives by definition.

It's not clear how to implement this though.

@jtjeferreira
Copy link

Just wanted to say that the first point would be helpful for a similar library but for reactivemongo bson encoder/decoder (https://github.com/rethab/magnolia-bson) because Reactivemongo uses a BSONDocumentWriter and a BSONWriter

@fommil
Copy link
Contributor

fommil commented Aug 7, 2018

semiauto is actually working in 0.9.0 😄 It was doing full auto in 0.8.0, oops!

A foldLeft like thing would be 💯 for performance and probably makes my constructEither redundant if it can do a .flatMap step.

@vpavkin
Copy link
Contributor Author

vpavkin commented Aug 7, 2018

@fommil yeah, it was not a semiauto in a usual sense, but nor it was a proper full auto to be precise :) it was something in the middle.
regardless, thanks for pushing this forward! 👍

@fommil
Copy link
Contributor

fommil commented Aug 7, 2018

this semiauto is also somewhere in the middle... if you derive the top level ADT eg.

@deriving(Show, Equal, Arbitrary)
sealed abstract class JsValue { def widen: JsValue = this }
final case object JsNull                                    extends JsValue
final case class JsObject(fields: IList[(String, JsValue)]) extends JsValue
final case class JsArray(elements: IList[JsValue])          extends JsValue
final case class JsBoolean(value: Boolean)                  extends JsValue
final case class JsString(value: String)                    extends JsValue
final case class JsDouble(value: Double)                    extends JsValue
final case class JsInteger(value: Long)                     extends JsValue

it will continue to derive for all the fields in the coproduct, but not for the fields in the products.

Which I feel is a very practical, yet safe, trade-off.

@jtjeferreira
Copy link

As far as I understand things, nothing in the new release (0.9.1) changed so that having different type classes for the exported and internal types... You see any problems with the suggested solution?

@Krever
Copy link

Krever commented Apr 7, 2019

I first encountered this problem a few months ago and just fell back to shapeless. However recently I got two new use cases for this so it becomes more and more painful to not be able to use magnolia in this way.
Right now my use case are:

  • encode case class as a map of variables used by workflow engine (and the other way around)
  • render form based on case class
  • decode case class from http URL-form-encoded request body

All of that fits into the described issue.
I tried looking into the implementation but 500 lines of macro code are above my level it seems.

An alternative solution to the one suggested by @vpavkin is to make output type of gen dynamic, so its not Typeclass[T] but rather the type returned by combined/dispatch. Something like that:

trait QueryParamPayload[T] {
  def decode(map: Map[String, Chain[String]]): T
}

object QueryParamPayload {

  type Typeclass[T] = QueryParamDecoder[T]

  def combine[T](ctx: CaseClass[QueryParamDecoder, T]): QueryParamPayload[T] = ???
  def gen[T]: QueryParamPayload[T] = macro Magnolia.gen[T]

}

So we derive from QueryParamDecoder but return QueryParamPayload. I'm willing to contribute time into it but unfortunately, I don't have the required macro expertise.

neko-kai added a commit to 7mind/pureconfig that referenced this issue Feb 20, 2020
…to mode, when deriving collection a instance

* Also forces all magnolia derivations to run in `blackbox.Context` (after typer) for performance. Seems magnolia doesn't use whitebox capabilities anyway
* Fixes softwaremill/magnolia#107 for pureconfig-magnolia
* Ported from `izumi` - 7mind/izumi#915
@neko-kai
Copy link
Contributor

neko-kai commented Feb 20, 2020

Seems like this actually works in a direct way, just wrap the output of Magnolia.gen[A] in your library's Export type with a wrapper macro:

  object exportedReader {
    type Typeclass[A] = ConfigReader[A]
    def combine[A: ProductHint](ctx: CaseClass[ConfigReader, A]): ConfigReader[A] = MagnoliaConfigReader.combine(ctx)
    def dispatch[A: CoproductHint](ctx: SealedTrait[ConfigReader, A]): ConfigReader[A] = MagnoliaConfigReader.dispatch(ctx)

    implicit def exportReader[A]: Exported[ConfigReader[A]] = macro ExportedMagnolia.materializeImpl[A]
    // Wrap the output of Magnolia in an Exported to force it to a lower priority.
    // This seems to work, despite magnolia hardcode checks for `macroApplication` symbol
    // and relying on getting a diverging implicit expansion error for auto-mode.
    // Thankfully at least it doesn't check the output type of its `macroApplication`
    object ExportedMagnolia {
      def materializeImpl[A](c: whitebox.Context)(implicit t: c.WeakTypeTag[A]): c.Expr[Exported[ConfigReader[A]]] = {
        val magnoliaTree = c.Expr[ConfigReader[A]](Magnolia.gen[A](c))
        c.universe.reify(Exported(magnoliaTree.splice))
      }
    }
  }

See:
distage-extension-config - 7mind/izumi#915
pureconfig-magnolia - pureconfig/pureconfig#703

@neko-kai
Copy link
Contributor

neko-kai commented Feb 21, 2020

You can also stick a shapeless.LowPriority in front of Magnolia if you don't have an Export type, at the cost of additional macro calls to materialize that evidence:

implicit def gen[A](implicit lp: shapeless.LowPriority): ConfigReader[A] = macro lpgen[A]

def lpgen[A: c.WeakTypeTag](c: whitebox.Context)(lp: c.Tree): c.Tree = Magnolia.gen[A]

@jtjeferreira
Copy link

@neko-kai thanks for this nice trick...

ruippeixotog pushed a commit to pureconfig/pureconfig that referenced this issue Feb 27, 2020
* Fix pureconfig-magnolia not using existing pureconfig instances in auto mode, when deriving collection a instance

* Also forces all magnolia derivations to run in `blackbox.Context` (after typer) for performance. Seems magnolia doesn't use whitebox capabilities anyway
* Fixes softwaremill/magnolia#107 for pureconfig-magnolia
* Ported from `izumi` - 7mind/izumi#915

* Remove blackboxification, copy test to `generic`

* unused import

* remove useless diffs
jtjeferreira added a commit to jtjeferreira/magnolia-bson that referenced this issue Mar 1, 2020
@kubukoz
Copy link
Contributor

kubukoz commented Apr 6, 2020

After #221 it seems like it's possible to return a more precise type from gen, if gen is called explicitly (no auto derivation).

neko-kai added a commit to 7mind/pureconfig that referenced this issue Apr 18, 2020
…econfig#703)

* Fix pureconfig-magnolia not using existing pureconfig instances in auto mode, when deriving collection a instance

* Also forces all magnolia derivations to run in `blackbox.Context` (after typer) for performance. Seems magnolia doesn't use whitebox capabilities anyway
* Fixes softwaremill/magnolia#107 for pureconfig-magnolia
* Ported from `izumi` - 7mind/izumi#915

* Remove blackboxification, copy test to `generic`

* unused import

* remove useless diffs
@OlegYch
Copy link
Contributor

OlegYch commented Apr 27, 2020

@kubukoz yes, with one little fix #240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants