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 case class by cypher string interpolator #201

Merged
merged 4 commits into from Nov 16, 2020

Conversation

iRevive
Copy link
Contributor

@iRevive iRevive commented Nov 13, 2020

It works the next way:

case class User(name: String, age: Int)
val user = User("my name", 33)

val query = c"""CREATE (u: User { $user }) RETURN u"""
// CREATE (u: User { name: "my name", age: 33 }) RETURN u

Copy link
Contributor

@BalmungSan BalmungSan left a comment

Choose a reason for hiding this comment

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

Amazing work! and I love that the tests ensure we do not generate meaningless queries like a nested case class.

I have an idea but the more I think about it the less I like it.
Making the disclaimer that I really do not know too much about shapeless, I wonder if it would be possible to include the case class name in the query.
Thus instead of: CREATE (u: User { $user }) one could just CREATE (u: $user }).

However, even if possible, that should be optional because:

  • I may want / need a different name for the Node / Relationship - BTW, it would be good to have some test using this on a Relationship; In theory, it should just work the same but just in case 🙂.
  • I may need to add extra properties to the Node / Relationship that are not (and should not be) part of the case class. - BTW, this would also be a good additional test.

So, I wonder that, even if possible and even if optional, if it would be of any use. What do you think @iRevive ?

@dimafeng what do you think in general?

@synedu
Copy link

synedu commented Nov 13, 2020

Amazing work! and I love that the tests ensure we do not generate meaningless queries like a nested case class.

I have an idea but the more I think about it the less I like it.
Making the disclaimer that I really do not know too much about shapeless, I wonder if it would be possible to include the case class name in the query.
Thus instead of: CREATE (u: User { $user }) one could just CREATE (u: $user }).

However, even if possible, that should be optional because:

  • I may want / need a different name for the Node / Relationship - BTW, it would be good to have some test using this on a Relationship; In theory, it should just work the same but just in case 🙂.
  • I may need to add extra properties to the Node / Relationship that are not (and should not be) part of the case class. - BTW, this would also be a good additional test.

So, I wonder that, even if possible and even if optional, if it would be of any use. What do you think @iRevive ?

@dimafeng what do you think in general?

We do something similar to what you want, @BalmungSan in my project using Typeable[T] to get the case class name to use as the node label, and it works well (and gives a bit extra safety). I think doing something like what you suggest could be a bit confusing: "CREATE (u: $user })" because it isn't obvious that the label will be interpolated. Maybe it would be better to have a separate function to generate this kind of query? Something like this:

object Ex {
  import cats._
  import cats.syntax.all._
  import shapeless._, labelled._
  import shapeless.ops.hlist.MapFolder

  trait QueryBuilder[T] {
    def build(t: T, name: String): String
  }
  object QueryBuilder {
    def build[T](t: T, name: String)(implicit ev: QueryBuilder[T]): String =
      ev.build(t, name)

    implicit def all[T <: Product, R <: HList](
        implicit
        ev: LabelledGeneric.Aux[T, R],
        folder: MapFolder[R, Map[String, String], prettyPrint.type]
    ): QueryBuilder[T] =
      new QueryBuilder[T] {
        def build(t: T, name: String) = {
          val pairs =
            folder
              .apply(ev.to(t), Map.empty, _ ++ _)
              .toList
              .map { case (k, v) => s"""$k: "$v"""" }
              .mkString(", ")

          s"CREATE ($name:${t.productPrefix} { $pairs }) RETURN $name"
        }
      }

    trait LowPrio extends Poly1 {
      implicit def all[K <: Symbol, V: Show](
          implicit key: Witness.Aux[K]
      ): Case.Aux[FieldType[K, V], Map[String, String]] =
        at[FieldType[K, V]] { (v: V) =>
          Map(key.value.name -> v.show)
        }
    }
    object prettyPrint extends LowPrio {
      implicit def optional[K <: Symbol, V: Show](
          implicit key: Witness.Aux[K]
      ): Case.Aux[FieldType[K, Option[V]], Map[String, String]] =
        at[FieldType[K, Option[V]]] { v =>
          (v: Option[V])
            .foldMap(v => Map(key.value.name -> v.show))
        }
    }
  }

  final case class Foo(id: String, name: String, otherThing: Option[Double])

  val f = Foo("xxx", "Bar", None)
  val ff = QueryBuilder.build(f, name = "f")
  // res0: String = CREATE (f:Foo {id: "xxx", name: "Bar"}) RETURN f

  val g = Foo("yyy", "Baz", Option(3.0))
  val gg = QueryBuilder.build(g, name = "g")
  // res1: String = CREATE (g:Foo {id: "yyy", name: "Baz", otherThing: "3.0"}) RETURN g
}

Credit to @SystemFw for this magic.

@BalmungSan
Copy link
Contributor

BalmungSan commented Nov 13, 2020

val f = Foo("xxx", "Bar", None)
 val ff = QueryBuilder.build(f, name = "f")
// res0: String = CREATE (f:Foo {id: "xxx", name: "Bar"}) RETURN f

@bengraygh That looks quite interesting. However, I would prefer not to generate all that. For me, it would be more than great to have just (f: Foo {id: "xxx", name: "Bar"}) I sometimes need to create more complex Paths than just a single Node and not always I need to return the created Node.

So, I personally would prefer if this kind of things to be outside of the core module (but I am open to having an ORM-like module in the repo).

@iRevive
Copy link
Contributor Author

iRevive commented Nov 14, 2020

I have an idea but the more I think about it the less I like it.
Making the disclaimer that I really do not know too much about shapeless, I wonder if it would be possible to include the case class name in the query.
Thus instead of: CREATE (u: User { $user }) one could just CREATE (u: $user }).

In the core library I would prefer having CREATE (u: User { $user } ) over Create (u: $user).
Meanwhile, I can add neotypes-extras module where we can have a more orm-like way of a query generator.

@iRevive
Copy link
Contributor Author

iRevive commented Nov 14, 2020

It would be good to have some test using this on a Relationship; In theory, it should just work the same but just in case

Could you give me a hint or an example of a query, please?

@synedu
Copy link

synedu commented Nov 14, 2020

Could you give me a hint or an example of a query, please?

Something like:

val query = c"CREATE ( u: User { $user } ) -[r:HAS_CAT { $hasCatProps }]->(c:Cat{ $cat } ) RETURN r"

@iRevive
Copy link
Contributor Author

iRevive commented Nov 14, 2020

Could you give me a hint or an example of a query, please?

Something like:

val query = c"CREATE ( u: User { $user } ) -[r:HAS_CAT { $hasCatProps }]->(c:Cat{ $cat } ) RETURN r"

thank you

@iRevive iRevive changed the title WIP: Support case class by cypher string interpolator Support case class by cypher string interpolator Nov 14, 2020
@iRevive
Copy link
Contributor Author

iRevive commented Nov 14, 2020

Meanwhile, I can add neotypes-extras module where we can have a more orm-like way of a query generator.

Done. The PR is #204

@dimafeng
Copy link
Contributor

@iRevive thank you for this contribution!

@iRevive @BalmungSan just a quick thought - since the string interpolation is not part of Cypher standard and is Neotypes's opinionated implementation, would it make sense to extract it into a separate module? So with #204, we would have two distinct modules with two implementations of interpolation that are not part of the core.

@iRevive
Copy link
Contributor Author

iRevive commented Nov 14, 2020

#204 uses interpolation under the hood but does not expose it.

You can do only the following things:

val userQuery: DeferredQuery[User] = CreateQuery[User].query[User](User(...))
val unitQuery: DeferredQuery[Unit] = CreateQuery[User].query[Unit](User(...))

If you do want to split the core into submodules, I believe it could be:

  1. core - the wrapper around neo4j (including ResultMapper, ValueMapper, etc)
  2. generic - the module with shapeless-based derivation neotypes.generic.auto._ and neotypes.generic.semiauto._
  3. cypher - basic cypher interpolation neotypes.implicits.syntax.cypher._
  4. queries - currently, only CreateQuery

From my point of view, it requires quite a lot of work. I would prefer keeping core module as is (including a case class
interpolation). And move everything else to the neotypes-extras submodule.

@BalmungSan
Copy link
Contributor

BalmungSan commented Nov 14, 2020

Actually, after 0.16.0 I wanted to propose a discussion about splitting the repo into more modules (like generic and maybe even take the streaming into another one).

So, I would prefer to merge this right now as it is; and start a discussion in gitter to see if we all think is a good idea or not. If we all agree it is a good idea then the best IMHO would be to open a new issue to continue the discussion there.

@dimafeng
Copy link
Contributor

@iRevive @BalmungSan thank you for weighing in. I agree

@BalmungSan
Copy link
Contributor

BTW @iRevive could you please add a short section about this to the Parameterized Queries page of the docs?

@iRevive
Copy link
Contributor Author

iRevive commented Nov 16, 2020

BTW @iRevive could you please add a short section about this to the Parameterized Queries page of the docs?

done

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

Successfully merging this pull request may close these issues.

None yet

4 participants