-
Notifications
You must be signed in to change notification settings - Fork 74
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
Migrate Blazegraph module to Cats Effect #4478
Merged
Merged
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a261e9e
Migrate Blazegraph client to Cats Effect
dantb 95b91e7
Merge branch 'master' into migrate-bg-client
dantb 790c6a4
Fix after merge
dantb adb59a3
Mock clock in slow query tests
dantb ca41924
Fix BG module wiring
dantb 6e0e723
Merge branch 'master' into migrate-bg-client
dantb 248ef8d
Migrate BlazegraphDecoderConfiguration
dantb ce21bb3
Merge branch 'migrate-bg-client' of github.com:dantb/nexus into migra…
dantb af84486
Simplify test clock, refactor
dantb c173bb0
Merge branch 'master' into migrate-bg-client
dantb a808e9f
Fix after merge
dantb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 5 additions & 4 deletions
9
...ala/ch/epfl/bluebrain/nexus/delta/plugins/blazegraph/BlazegraphDecoderConfiguration.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 0 additions & 1 deletion
1
.../scala/ch/epfl/bluebrain/nexus/delta/plugins/blazegraph/BlazegraphServiceDependency.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,19 +7,20 @@ import akka.http.scaladsl.model.headers.{BasicHttpCredentials, HttpCredentials, | |
import akka.http.scaladsl.model.{HttpEntity, HttpHeader, Uri} | ||
import akka.http.scaladsl.unmarshalling.FromEntityUnmarshaller | ||
import akka.http.scaladsl.unmarshalling.PredefinedFromEntityUnmarshallers.stringUnmarshaller | ||
import ch.epfl.bluebrain.nexus.delta.kernel.utils.ClasspathResourceUtils | ||
import cats.effect.IO | ||
import cats.syntax.all._ | ||
import ch.epfl.bluebrain.nexus.delta.kernel.effect.migration.toCatsIOOps | ||
import ch.epfl.bluebrain.nexus.delta.plugins.blazegraph.client.BlazegraphClient.timeoutHeader | ||
import ch.epfl.bluebrain.nexus.delta.plugins.blazegraph.client.SparqlClientError.{InvalidCountRequest, WrappedHttpClientError} | ||
import ch.epfl.bluebrain.nexus.delta.plugins.blazegraph.client.SparqlQueryResponseType.{Aux, SparqlResultsJson} | ||
import ch.epfl.bluebrain.nexus.delta.plugins.blazegraph.config.BlazegraphViewsConfig.Credentials | ||
import ch.epfl.bluebrain.nexus.delta.rdf.query.SparqlQuery | ||
import ch.epfl.bluebrain.nexus.delta.rdf.query.SparqlQuery.SparqlConstructQuery | ||
import ch.epfl.bluebrain.nexus.delta.sdk.http.HttpClient | ||
import ch.epfl.bluebrain.nexus.delta.sdk.http.{HttpClient, HttpClientError} | ||
import ch.epfl.bluebrain.nexus.delta.sdk.model.ComponentDescription.ServiceDescription | ||
import ch.epfl.bluebrain.nexus.delta.sdk.model.ComponentDescription.ServiceDescription.ResolvedServiceDescription | ||
import ch.epfl.bluebrain.nexus.delta.sdk.model.Name | ||
import ch.epfl.bluebrain.nexus.delta.sdk.syntax._ | ||
import monix.bio.{IO, UIO} | ||
|
||
import scala.concurrent.duration._ | ||
|
||
|
@@ -29,24 +30,20 @@ import scala.concurrent.duration._ | |
class BlazegraphClient( | ||
client: HttpClient, | ||
endpoint: Uri, | ||
queryTimeout: Duration | ||
queryTimeout: Duration, | ||
defaultProperties: Map[String, String] | ||
)(implicit credentials: Option[HttpCredentials], as: ActorSystem) | ||
extends SparqlClient(client, SparqlQueryEndpoint.blazegraph(endpoint)) { | ||
|
||
implicit private val cl: ClassLoader = getClass.getClassLoader | ||
|
||
private val serviceVersion = """(buildVersion">)([^<]*)""".r | ||
private val serviceName = Name.unsafe("blazegraph") | ||
|
||
private val defaultProperties = | ||
ClasspathResourceUtils.bioPropertiesOf("blazegraph/index.properties").hideErrors.memoizeOnSuccess | ||
|
||
override def query[R <: SparqlQueryResponse]( | ||
indices: Iterable[String], | ||
q: SparqlQuery, | ||
responseType: Aux[R], | ||
additionalHeaders: Seq[HttpHeader] | ||
): IO[SparqlClientError, R] = { | ||
): IO[R] = { | ||
val headers = queryTimeout match { | ||
case finite: FiniteDuration => additionalHeaders :+ RawHeader(timeoutHeader, finite.toMillis.toString) | ||
case _ => additionalHeaders | ||
|
@@ -57,10 +54,11 @@ class BlazegraphClient( | |
/** | ||
* Fetches the service description information (name and version) | ||
*/ | ||
def serviceDescription: UIO[ServiceDescription] = | ||
def serviceDescription: IO[ServiceDescription] = | ||
client | ||
.fromEntityTo[ResolvedServiceDescription](Get(endpoint / "status")) | ||
.timeout(5.seconds) | ||
.toCatsIO | ||
.redeem( | ||
_ => ServiceDescription.unresolved(serviceName), | ||
_.map(_.copy(name = serviceName)).getOrElse(ServiceDescription.unresolved(serviceName)) | ||
|
@@ -69,11 +67,13 @@ class BlazegraphClient( | |
/** | ||
* Check whether the passed namespace ''namespace'' exists. | ||
*/ | ||
def existsNamespace(namespace: String): IO[SparqlClientError, Boolean] = | ||
client(Get(endpoint / "namespace" / namespace)) { | ||
case resp if resp.status == OK => UIO.delay(resp.discardEntityBytes()) >> IO.pure(true) | ||
case resp if resp.status == NotFound => UIO.delay(resp.discardEntityBytes()) >> IO.pure(false) | ||
}.mapError(WrappedHttpClientError) | ||
def existsNamespace(namespace: String): IO[Boolean] = | ||
client | ||
.run(Get(endpoint / "namespace" / namespace)) { | ||
case resp if resp.status == OK => IO.delay(resp.discardEntityBytes()).as(true) | ||
case resp if resp.status == NotFound => IO.delay(resp.discardEntityBytes()).as(false) | ||
} | ||
.adaptError { case e: HttpClientError => WrappedHttpClientError(e) } | ||
|
||
/** | ||
* Attempts to create a namespace (if it doesn't exist) recovering gracefully when the namespace already exists. | ||
|
@@ -85,17 +85,19 @@ class BlazegraphClient( | |
* @return | ||
* ''true'' wrapped on an IO when namespace has been created and ''false'' wrapped on an IO when it already existed | ||
*/ | ||
def createNamespace(namespace: String, properties: Map[String, String]): IO[SparqlClientError, Boolean] = | ||
def createNamespace(namespace: String, properties: Map[String, String]): IO[Boolean] = | ||
existsNamespace(namespace).flatMap { | ||
case true => IO.pure(false) | ||
case false => | ||
val updated = properties + ("com.bigdata.rdf.sail.namespace" -> namespace) | ||
val payload = updated.map { case (key, value) => s"$key=$value" }.mkString("\n") | ||
val req = Post(endpoint / "namespace", HttpEntity(payload)) | ||
client(req) { | ||
case resp if resp.status.isSuccess() => UIO.delay(resp.discardEntityBytes()) >> IO.pure(true) | ||
case resp if resp.status == Conflict => UIO.delay(resp.discardEntityBytes()) >> IO.pure(false) | ||
}.mapError(WrappedHttpClientError) | ||
client | ||
.run(req) { | ||
case resp if resp.status.isSuccess() => IO.delay(resp.discardEntityBytes()).as(true) | ||
case resp if resp.status == Conflict => IO.delay(resp.discardEntityBytes()).as(false) | ||
} | ||
.adaptError { case e: HttpClientError => WrappedHttpClientError(e) } | ||
} | ||
|
||
/** | ||
|
@@ -107,25 +109,26 @@ class BlazegraphClient( | |
* @return | ||
* ''true'' wrapped on an IO when namespace has been created and ''false'' wrapped on an IO when it already existed | ||
*/ | ||
def createNamespace(namespace: String): IO[SparqlClientError, Boolean] = | ||
defaultProperties.flatMap(createNamespace(namespace, _)) | ||
def createNamespace(namespace: String): IO[Boolean] = createNamespace(namespace, defaultProperties) | ||
|
||
/** | ||
* Attempts to delete a namespace recovering gracefully when the namespace does not exists. | ||
* | ||
* @return | ||
* ''true'' wrapped in ''F'' when namespace has been deleted and ''false'' wrapped in ''F'' when it does not existe | ||
*/ | ||
def deleteNamespace(namespace: String): IO[SparqlClientError, Boolean] = | ||
client(Delete(endpoint / "namespace" / namespace)) { | ||
case resp if resp.status == OK => UIO.delay(resp.discardEntityBytes()) >> IO.pure(true) | ||
case resp if resp.status == NotFound => UIO.delay(resp.discardEntityBytes()) >> IO.pure(false) | ||
}.mapError(WrappedHttpClientError) | ||
def deleteNamespace(namespace: String): IO[Boolean] = | ||
client | ||
.run(Delete(endpoint / "namespace" / namespace)) { | ||
case resp if resp.status == OK => IO.delay(resp.discardEntityBytes()).as(true) | ||
case resp if resp.status == NotFound => IO.delay(resp.discardEntityBytes()).as(false) | ||
} | ||
.adaptError { case e: HttpClientError => WrappedHttpClientError(e) } | ||
|
||
/** | ||
* Count all the triples on an index | ||
*/ | ||
def count(index: String): IO[SparqlClientError, Long] = { | ||
def count(index: String): IO[Long] = { | ||
val sparqlQuery = SparqlConstructQuery.unsafe("SELECT (COUNT(?s) AS ?count) WHERE { ?s ?p ?o }") | ||
query(Set(index), sparqlQuery, SparqlResultsJson) | ||
.flatMap { response => | ||
|
@@ -135,7 +138,7 @@ class BlazegraphClient( | |
count <- countAsString.value.toLongOption | ||
} yield count | ||
|
||
IO.fromOption(count, InvalidCountRequest(index, sparqlQuery.value)) | ||
IO.fromOption(count)(InvalidCountRequest(index, sparqlQuery.value)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will never forgive the Cats authors for this method signature |
||
} | ||
} | ||
|
||
|
@@ -163,10 +166,11 @@ object BlazegraphClient { | |
client: HttpClient, | ||
endpoint: Uri, | ||
credentials: Option[Credentials], | ||
queryTimeout: Duration | ||
queryTimeout: Duration, | ||
defaultProperties: Map[String, String] | ||
)(implicit as: ActorSystem): BlazegraphClient = { | ||
implicit val cred: Option[BasicHttpCredentials] = | ||
credentials.map { cred => BasicHttpCredentials(cred.username, cred.password.value) } | ||
new BlazegraphClient(client, endpoint, queryTimeout) | ||
new BlazegraphClient(client, endpoint, queryTimeout, defaultProperties) | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth a test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm reluctant since it's essentially copied stdlib code which you wouldn't normally test... Kinda covered in those slow query tests as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think copied code is still worth testing if we rely on it. Tests are there to make sure it works going forward as well as now. It should be super easy to write