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

Ecaping array elements is too eager #504

Open
nafg opened this issue Mar 11, 2021 · 5 comments
Open

Ecaping array elements is too eager #504

nafg opened this issue Mar 11, 2021 · 5 comments

Comments

@nafg
Copy link
Contributor

nafg commented Mar 11, 2021

  def arrayAsText[A: ClassTag](fromString: String => Option[A])(toString: A => String): JdbcType[Seq[A]] =
    new SimpleArrayJdbcType[String]("text")
      .mapTo(s => fromString(s).getOrElse(sys.error(s"Could not convert '$s' to a ${classTag[A]}")), toString)

does not work if toString of an A includes an apostrophe, because it gets saved with a double apostrophe.

I assume that without the mapTo line as well, nothing would crash but your strings would get their apostrophes doubled.

Similarly, this broke recently:

  def arrayAsText[A: ClassTag](fromString: String => Option[A])(toString: A => String): JdbcType[Seq[A]] =
    new AdvancedArrayJdbcType[A](
      sqlBaseType = "text",
      fromString = SimpleArrayUtils.fromString(fromString)(_).map(_.flatten).orNull,
      mkString = SimpleArrayUtils.mkString[A](toString)(_)
    )

Changing SimpleArrayUtils.mkString to SimpleArrayUtils.mkStringUnsafe works.

@tminglei
Copy link
Owner

@nafg I didn't reproduce the double apostrophe.
Can you provide full reproducible codes?

@nafg
Copy link
Contributor Author

nafg commented May 7, 2021

I just tried with 0.19.6 and it's still broken.

It's not hard to reproduce. There's nothing complicated here.

In fact, if you can show a working example that includes saving and reloading a TEXT[] containing an apostrophe character that doesn't involve low-level APIs (preferably mapped to a custom type that has a string representation), then I can't claim anything is broken.

That said, I will try to share more code. It would be hard to start making a minimal executable self-contained program, but here is the relevant code, that should give you a clearer picture of what I have.

def arrayAsText[A: ClassTag](fromString: String => Option[A])(toString: A => String): JdbcType[Seq[A]] =
    new SimpleArrayJdbcType[String]("text")
      .mapTo(s => fromString(s).getOrElse(sys.error(s"Could not convert '$s' to a ${classTag[A]}")), toString)
abstract class CustomFieldKind(val id: String, val name: String) {
  def subfieldDefs: List[SubfieldDefBase]
  def allowInNature(@unused nature: Nature): Boolean = true

  override def toString = s"CustomFieldKind($id, $name) { subfieldDefs = $subfieldDefs }"
}
object CustomFieldKind {
  class Simple(id: String, title: String)(defs: SubfieldDefBase*) extends CustomFieldKind(id, title) {
    override def subfieldDefs = defs.toList
  }

  private val ClientAddressFieldKind = new Simple("clientAddress", "Client's address")(
    SubfieldDef(SubfieldKind.ClientAddress, "clientAddress", "Client's address")
  )
  def forName(string: String): Option[CustomFieldKind] = all.find(_.name == string)
  private implicit val fieldKindListMapping: JdbcType[Seq[CustomFieldKind]] =
    SlickColumnMappings.arrayAsText(CustomFieldKind.forName)(_.name)
  def allowedKinds = column[Seq[CustomFieldKind]]("allowed_kinds")

If I then save an array containing a ClientAddressFieldKind into allowedKinds and then try to load it, I get the following error:

java.lang.RuntimeException: Could not convert 'Client''s address' to a chesednow.customfields.shared.CustomFieldKind            
     at scala.sys.package$.error(package.scala:27)                                                                              
     at chesednow.models.common.SlickColumnMappings$.$anonfun$arrayAsText$2(SlickColumnMappings.scala:50)                       
     at scala.Option.getOrElse(Option.scala:201)                                                                                
     at chesednow.models.common.SlickColumnMappings$.$anonfun$arrayAsText$1(SlickColumnMappings.scala:50)                       
     at com.github.tminglei.slickpg.array.PgArrayJdbcTypes$SimpleArrayJdbcType.$anonfun$mapTo$1(PgArrayJdbcTypes.scala:66)      
     at scala.collection.immutable.ArraySeq.$anonfun$map$1(ArraySeq.scala:71)                                                   
     at scala.collection.immutable.ArraySeq.$anonfun$map$1$adapted(ArraySeq.scala:71)                                           
     at scala.collection.immutable.ArraySeq$.tabulate(ArraySeq.scala:286)                                                       
     at scala.collection.immutable.ArraySeq$.tabulate(ArraySeq.scala:265)                                                       
     at scala.collection.ClassTagIterableFactory$AnyIterableDelegate.tabulate(Factory.scala:679)                                

Is it clearer now?

@nafg
Copy link
Contributor Author

nafg commented May 7, 2021

I should mention that querying the database directly shows that indeed, it saved it as Client''s address instead of as Client's address.

@nafg
Copy link
Contributor Author

nafg commented Aug 20, 2023

Hi, was there any update on this?

tminglei added a commit that referenced this issue Sep 2, 2023
@tminglei
Copy link
Owner

tminglei commented Sep 2, 2023

@nafg I can't reproduce it with updated tests at a6836d4.

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

No branches or pull requests

2 participants