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

json array implicits for doobie #132

Merged
merged 13 commits into from
Jul 12, 2024
4 changes: 2 additions & 2 deletions .github/workflows/jacoco_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
min-coverage-overall: ${{ env.coverage-overall }}
min-coverage-changed-files: ${{ env.coverage-changed-files }}
title: JaCoCo `model` module code coverage report - scala ${{ env.scalaLong }}
title: JaCoCo `core` module code coverage report - scala ${{ env.scalaLong }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this fix, good boy scout 👍

update-comment: true
- name: Add coverage to PR (doobie)
if: steps.jacocorun.outcome == 'success'
Expand All @@ -80,7 +80,7 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
min-coverage-overall: ${{ env.coverage-overall }}
min-coverage-changed-files: ${{ env.coverage-changed-files }}
title: JaCoCo `agent` module code coverage report - scala ${{ env.scalaLong }}
title: JaCoCo `doobie` module code coverage report - scala ${{ env.scalaLong }}
update-comment: true
- name: Add coverage to PR (slick)
if: steps.jacocorun.outcome == 'success'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE TABLE IF NOT EXISTS integration.actors_json_seq (
id SERIAL PRIMARY KEY,
actors_json JSON[],
actors_jsonb JSONB[]
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE OR REPLACE FUNCTION integration.insert_actors_json(actorsJson JSON[], actorsJsonb JSONB[])
RETURNS void AS $$
BEGIN
INSERT INTO integration.actors_json_seq (actors_json, actors_jsonb)
VALUES (actorsJson, actorsJsonb);
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE OR REPLACE FUNCTION integration.retrieve_actors_json(idUntil INT)
RETURNS TABLE(actors_json JSON[]) AS $$
BEGIN
RETURN QUERY SELECT a.actors_json FROM integration.actors_json_seq AS a WHERE id <= idUntil;
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE OR REPLACE FUNCTION integration.retrieve_actors_jsonb(idUntil INT)
RETURNS TABLE(actors_jsonb JSONB[]) AS $$
BEGIN
RETURN QUERY SELECT a.actors_jsonb FROM integration.actors_json_seq AS a WHERE id <= idUntil;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the <= idea :)

END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.db.fadb.doobie.postgres.circe

import cats.Show
import cats.data.NonEmptyList
import doobie.{Get, Put}
import io.circe.Json
import org.postgresql.jdbc.PgArray
import org.postgresql.util.PGobject
import io.circe.parser._

import scala.util.{Failure, Success, Try}

package object implicits {
Copy link
Collaborator

Choose a reason for hiding this comment

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

also, what about JSON/JSONB put and get? (these here are List of JSON/JSONB, so I'm talking about the non-array variant)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm okay! Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made it available also in the implicits object so we can import everything from the same place.


private implicit val showPgArray: Show[PgArray] = Show.fromToString

implicit val jsonPut: Put[Json] = doobie.postgres.circe.json.implicits.jsonPut
implicit val jsonbPut: Put[Json] = doobie.postgres.circe.jsonb.implicits.jsonbPut

implicit val jsonGet: Get[Json] = doobie.postgres.circe.json.implicits.jsonGet
implicit val jsonbGet: Get[Json] = doobie.postgres.circe.jsonb.implicits.jsonbGet

implicit val jsonArrayPut: Put[List[Json]] = {
Put.Advanced
.other[PGobject](
NonEmptyList.of("json[]")
)
.tcontramap { a =>
val o = new PGobject
o.setType("json[]")
o.setValue(jsonListToPGJsonArrayString(a))
o
}
}

implicit val jsonbArrayPut: Put[List[Json]] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about JSONB get utils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to work for both :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

really?

because the GET implements json[]:

  implicit val jsonArrayGet: Get[List[Json]] = {
    Get.Advanced
      .other[PgArray](
        NonEmptyList.of("json[]")
      )
      .temap(pgArray => pgArrayToListOfJson(pgArray))
  }

perhaps for the sake of consistency in the naming we should either: implement the second one with jsonb[] or name this one somehow so that it's understandable that it's also for JSONB GET?

Copy link
Collaborator

Choose a reason for hiding this comment

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

or just something like this :D

implicit val jsonbArrayGet: Get[List[Json]] = jsonArrayGet

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just mention it in the comments (documentation) of the classes. Which btw, is missing somewhat 👼

Copy link
Collaborator

Choose a reason for hiding this comment

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

discussed on a call

Copy link
Contributor Author

@salamonpavel salamonpavel Jul 11, 2024

Choose a reason for hiding this comment

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

implemented like this after discussion on a call

// to be used for both json[] and jsonb[] as it handles well both and we want to avoid collision
// when resolving implicits
implicit val jsonOrJsonbArrayGet: Get[List[Json]] = {
    Get.Advanced
      .other[PgArray](
        NonEmptyList.of("json[]")
      )
      .temap(pgArray => pgArrayToListOfJson(pgArray))
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

Somwhat longer but acceptable, IMHO.

Put.Advanced
.other[PGobject](
NonEmptyList.of("jsonb[]")
)
.tcontramap { a =>
val o = new PGobject
o.setType("jsonb[]")
o.setValue(jsonListToPGJsonArrayString(a))
o
}
}

// to be used for both json[] and jsonb[] as it handles well both
// and we want to avoid collision when resolving implicits
implicit val jsonOrJsonbArrayGet: Get[List[Json]] = {
Get.Advanced
.other[PgArray](
NonEmptyList.of("json[]")
)
.temap(pgArray => pgArrayToListOfJson(pgArray))
}

private def jsonListToPGJsonArrayString(jsonList: List[Json]): String = {
val arrayElements = jsonList.map { x =>
// Convert to compact JSON string and escape inner quotes
val escapedJsonString = x.noSpaces.replace("\"", "\\\"")
// Wrap in double quotes for the array element
s""""$escapedJsonString""""
}

arrayElements.mkString("{", ",", "}")
}

private def pgArrayToListOfJson(pgArray: PgArray): Either[String, List[Json]] = {
Try(Option(pgArray.getArray)) match {
case Success(Some(array: Array[_])) =>
val results = array.toList.map {
case str: String => parse(str).left.map(_.getMessage)
case other => parse(other.toString).left.map(_.getMessage)
}
results.partition(_.isLeft) match {
case (Nil, rights) => Right(rights.collect { case Right(json) => json })
case (lefts, _) => Left("Failed to parse JSON: " + lefts.collect { case Left(err) => err }.mkString(", "))
}
case Success(Some(_)) => Left("Unexpected type encountered. Expected an Array.")
case Success(None) => Right(Nil)
case Failure(exception) => Left(exception.getMessage)
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.db.fadb.doobie

import cats.effect.IO
import cats.effect.unsafe.implicits.global
import doobie.implicits.toSqlInterpolator
import io.circe.Json
import io.circe.syntax.EncoderOps
import org.scalatest.funsuite.AnyFunSuite
import za.co.absa.db.fadb.DBSchema
import za.co.absa.db.fadb.doobie.DoobieFunction.{DoobieMultipleResultFunction, DoobieSingleResultFunction}
import za.co.absa.db.fadb.testing.classes.DoobieTest
import io.circe.generic.auto._

import za.co.absa.db.fadb.doobie.postgres.circe.implicits.jsonOrJsonbArrayGet

class JsonArrayIntegrationTests extends AnyFunSuite with DoobieTest {

class InsertActorsJson(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
extends DoobieSingleResultFunction[List[Actor], Unit, IO] (
values => {
val actorsAsJsonList = values.map(_.asJson)
Seq(
{
// has to be imported inside separate scope to avoid conflicts with the import below
// as both implicits are of the same type and this would cause ambiguity
import za.co.absa.db.fadb.doobie.postgres.circe.implicits.jsonArrayPut
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it enough now, to import once and on the top level?

Copy link
Contributor Author

@salamonpavel salamonpavel Jul 11, 2024

Choose a reason for hiding this comment

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

It would be enough to import it once on the top of the file alongside other imports had we had only json[] or jsonb[]. In this case though we have both and given the way implicits are resolved by their types the compiler is unable to figure out which one to use. That's why in this particular case the imports have to be scope using the curly braces for individual query fragments.

Maybe that's also the reason why doobie doesn't support the array variants by default...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, I missed that they a different import (the one letter difference). 😊
Tiny thing, if you would want to add an explanatory comment why the import is done like this

fr"$actorsAsJsonList"
},
{
// has to be imported inside separate scope to avoid conflicts with the import above
// as both implicits are of the same type and this would cause ambiguity
import za.co.absa.db.fadb.doobie.postgres.circe.implicits.jsonbArrayPut
fr"$actorsAsJsonList"
}
)
}
)

class RetrieveActorsJson(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
extends DoobieMultipleResultFunction[Int, List[Json], IO] (
values => Seq(fr"$values")
)

class RetrieveActorsJsonb(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
extends DoobieMultipleResultFunction[Int, List[Json], IO] (
values => Seq(fr"$values")
)

private val insertActorsJson = new InsertActorsJson()(Integration, new DoobieEngine(transactor))

test("Retrieve Actors from json[] and jsonb[] columns"){
val expectedActors = List(Actor(1, "John", "Doe"), Actor(2, "Jane", "Doe"))
insertActorsJson(expectedActors).unsafeRunSync()

val retrieveActorsJson = new RetrieveActorsJson()(Integration, new DoobieEngine(transactor))
val actualActorsJson = retrieveActorsJson(2).unsafeRunSync()
assert(expectedActors == actualActorsJson.head.map(_.as[Actor]).map(_.toTry.get))

val retrieveActorsJsonb = new RetrieveActorsJsonb()(Integration, new DoobieEngine(transactor))
val actualActorsJsonb = retrieveActorsJsonb(2).unsafeRunSync()
assert(expectedActors == actualActorsJsonb.head.map(_.as[Actor]).map(_.toTry.get))
}

}
7 changes: 5 additions & 2 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import sbt._
import sbt.*

object Dependencies {

Expand Down Expand Up @@ -47,7 +47,9 @@ object Dependencies {
commonDependencies(scalaVersion) ++ Seq(
"org.tpolecat" %% "doobie-core" % "1.0.0-RC2",
"org.tpolecat" %% "doobie-hikari" % "1.0.0-RC2",
"org.tpolecat" %% "doobie-postgres" % "1.0.0-RC2"
"org.tpolecat" %% "doobie-postgres" % "1.0.0-RC2",
"org.tpolecat" %% "doobie-postgres-circe" % "1.0.0-RC2",
"io.circe" %% "circe-generic" % "0.14.9" % Test
)
}

Expand All @@ -56,4 +58,5 @@ object Dependencies {

Seq(postgresql)
}

}
Loading