From cc3de07772db318fc1b4791ea829520f7780a66c Mon Sep 17 00:00:00 2001 From: Jeff May Date: Thu, 17 Mar 2016 15:47:24 -0700 Subject: [PATCH 1/3] Cleanup deprecated method usage. --- .../neo4j/client/cypher/CypherStringContextSpec.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala index 4ccb2e8..8f79ff8 100644 --- a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala +++ b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala @@ -78,7 +78,7 @@ class CypherStringContextSpec extends WordSpec "allow inserting an object for any known cypher props" in { forAll() { (props: CypherProps) => - val obj = Cypher.obj("props", props) + val obj = Cypher.obj(Cypher.param("props", props)) val stmt = cypher"MATCH (n $obj)" assertResult(s"MATCH (n { ${obj.namespace} })", "template did not render properly") { stmt.template @@ -91,8 +91,8 @@ class CypherStringContextSpec extends WordSpec "throw an exception when given two different objects for the same namespace" in { val conflictingNamespace = "props" - val obj1 = Cypher.obj(conflictingNamespace, Cypher.props("id" -> 1, "name" -> "x")) - val obj2 = Cypher.obj(conflictingNamespace, Cypher.props("id" -> 2, "name" -> "y")) + val obj1 = Cypher.obj(Cypher.param(conflictingNamespace, Cypher.props("id" -> 1, "name" -> "x"))) + val obj2 = Cypher.obj(Cypher.param(conflictingNamespace, Cypher.props("id" -> 2, "name" -> "y"))) val ex = intercept[ConflictingParameterObjectsException] { cypher"MATCH (n $obj1) --> (m $obj2)" } @@ -108,7 +108,7 @@ class CypherStringContextSpec extends WordSpec "throw an exception mixing mutable params with an object that has the same namespace in the same statement" in { val conflictingNamespace = "props" - val obj = Cypher.obj(conflictingNamespace, Cypher.props("id" -> 1, "name" -> "x")) + val obj = Cypher.obj(Cypher.param(conflictingNamespace, Cypher.props("id" -> 1, "name" -> "x"))) val conflicting = Cypher.param(conflictingNamespace) val conflictingKey = "anything" val conflictingValue = "doesn't matter" From 89f8c2d67a9ae146f17c71fab0ad7ef5aceb428d Mon Sep 17 00:00:00 2001 From: Jeff May Date: Fri, 18 Mar 2016 16:59:16 -0700 Subject: [PATCH 2/3] Updated tests. - Added WSDefaultShows - Split basic specs into separate test classes - Added CreateNodeWithPropsObject query - Added InvalidCypherQueriesSpec to test expected failures --- ws/src/main/resources/logback.xml | 26 +++ .../neo4j/client/ws/WSDefaultShows.scala | 11 ++ .../neo4j/client/ws/json/DebugFormats.scala | 2 +- .../neo4j/client/ws/json/ShowAsJson.scala | 2 +- .../client/ws/json/rest/RestFormats.scala | 2 +- .../jeffmay/neo4j/client/FixtureQueries.scala | 47 +++-- .../client/ws/CommonWSNeo4jClientSpec.scala | 66 +++++++ .../client/ws/InvalidCypherQueriesSpec.scala | 21 +++ .../client/ws/WSFixtureQueriesSpec.scala | 86 +++++++++ .../client/ws/WSNeo4jClientBasicSpecs.scala | 165 ------------------ .../neo4j/client/ws/WSNeo4jClientSpec.scala | 46 +++++ 11 files changed, 289 insertions(+), 185 deletions(-) create mode 100644 ws/src/main/resources/logback.xml create mode 100644 ws/src/main/scala/me/jeffmay/neo4j/client/ws/WSDefaultShows.scala create mode 100644 ws/src/test/scala/me/jeffmay/neo4j/client/ws/CommonWSNeo4jClientSpec.scala create mode 100644 ws/src/test/scala/me/jeffmay/neo4j/client/ws/InvalidCypherQueriesSpec.scala create mode 100644 ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSFixtureQueriesSpec.scala delete mode 100644 ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientBasicSpecs.scala create mode 100644 ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientSpec.scala diff --git a/ws/src/main/resources/logback.xml b/ws/src/main/resources/logback.xml new file mode 100644 index 0000000..4f9da63 --- /dev/null +++ b/ws/src/main/resources/logback.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + ${FORMAT_LOGGER} - ${FORMAT_MESSAGE} + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/WSDefaultShows.scala b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/WSDefaultShows.scala new file mode 100644 index 0000000..7874635 --- /dev/null +++ b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/WSDefaultShows.scala @@ -0,0 +1,11 @@ +package me.jeffmay.neo4j.client.ws + +import me.jeffmay.neo4j.client.Show +import me.jeffmay.neo4j.client.ws.json.rest.RestFormats +import me.jeffmay.neo4j.client.ws.json.{DebugFormats, ShowAsJson} + +/** + * Import from this object to get the standard [[Show]] instances for all the rest models as Json. + */ +object WSDefaultShows extends WSDefaultShows +private[ws] trait WSDefaultShows extends DebugFormats with RestFormats with ShowAsJson diff --git a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/DebugFormats.scala b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/DebugFormats.scala index cd2ebe6..bd51303 100644 --- a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/DebugFormats.scala +++ b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/DebugFormats.scala @@ -6,7 +6,7 @@ import me.jeffmay.neo4j.client.ws.json.rest._ import play.api.libs.json._ object DebugFormats extends DebugFormats -private[json] trait DebugFormats extends SharedFormats { +private[ws] trait DebugFormats extends SharedFormats { /* * Core model formats diff --git a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/ShowAsJson.scala b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/ShowAsJson.scala index 23a55e3..141ab53 100644 --- a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/ShowAsJson.scala +++ b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/ShowAsJson.scala @@ -4,7 +4,7 @@ import me.jeffmay.neo4j.client.Show import play.api.libs.json.{Json, Writes} object ShowAsJson extends ShowAsJson -private[json] trait ShowAsJson { +private[ws] trait ShowAsJson { implicit def showAsJson[T](implicit writer: Writes[T]): Show[T] = Show.show(it => Json.prettyPrint(writer writes it)) } diff --git a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/rest/RestFormats.scala b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/rest/RestFormats.scala index 4941c37..307fb5b 100644 --- a/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/rest/RestFormats.scala +++ b/ws/src/main/scala/me/jeffmay/neo4j/client/ws/json/rest/RestFormats.scala @@ -4,7 +4,7 @@ import me.jeffmay.neo4j.client.ws.json.SharedFormats import play.api.libs.json.{Reads, Json, Writes} object RestFormats extends RestFormats -private[json] trait RestFormats extends SharedFormats { +private[ws] trait RestFormats extends SharedFormats { implicit lazy val writesRawRequestStatement: Writes[RawRequestStatement] = Json.writes[RawRequestStatement] implicit lazy val writesRawStatementTransactionRequest: Writes[RawStatementTransactionRequest] = Json.writes[RawStatementTransactionRequest] diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/FixtureQueries.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/FixtureQueries.scala index 57b8fd7..bbad522 100644 --- a/ws/src/test/scala/me/jeffmay/neo4j/client/FixtureQueries.scala +++ b/ws/src/test/scala/me/jeffmay/neo4j/client/FixtureQueries.scala @@ -1,6 +1,6 @@ package me.jeffmay.neo4j.client -import me.jeffmay.neo4j.client.cypher.{Cypher, CypherStatement} +import me.jeffmay.neo4j.client.cypher.{CypherProps, Cypher, CypherStatement} import me.jeffmay.util.Namespace object FixtureQueries extends FixtureQueries @@ -19,15 +19,14 @@ trait FixtureQueries { * Each query has some pre-conditions / post-conditions for the expected success result stats to work. */ - object CreateNode { - def query(id: String, includeStats: Boolean = true)(implicit ns: Namespace): CypherStatement = { + case object CreateNode { + def query(id: String)(implicit ns: Namespace): CypherStatement = { CypherStatement( s"CREATE (n { ns: {props}.ns, ${PropKeys.Id}: {props}.id })", Map("props" -> Cypher.props( "id" -> id, "ns" -> ns.value - )), - includeStats = includeStats + )) ) } val successResultStats: StatementResultStats = { @@ -35,16 +34,32 @@ trait FixtureQueries { } } - object FindNode { + case object CreateNodeWithPropsObject { + def cleanup()(implicit ns: Namespace): CypherStatement = { + CypherStatement( + s"MATCH (n :${ns.value}) DELETE n" + ) + } + def query(props: CypherProps)(implicit ns: Namespace): CypherStatement = { + CypherStatement( + s"CREATE (n :${ns.value} { props })", + Map("props" -> props) + ) + } + val successResultStats: StatementResultStats = { + StatementResultStats.empty.copy(containsUpdates = true, nodesCreated = 1, propertiesSet = 2) + } + } + + case object FindNode { val NodeName = "n" - def query(id: String, includeStats: Boolean = false)(implicit ns: Namespace): CypherStatement = { + def query(id: String)(implicit ns: Namespace): CypherStatement = { CypherStatement( s"MATCH ($NodeName { ns: {props}.ns, ${PropKeys.Id}: {props}.id }) RETURN $NodeName", Map("props" -> Cypher.props( "id" -> id, "ns" -> ns.value - )), - includeStats = includeStats + )) ) } val successResultStats: StatementResultStats = { @@ -52,8 +67,8 @@ trait FixtureQueries { } } - object RenameNode { - def query(id: String, newId: String, includeStats: Boolean = true)(implicit ns: Namespace): CypherStatement = { + case object RenameNode { + def query(id: String, newId: String)(implicit ns: Namespace): CypherStatement = { CypherStatement( s"MATCH (n { ns: {props}.ns, ${PropKeys.Id}: {props}.id }) SET n.${PropKeys.Id} = {new}.id", Map( @@ -64,8 +79,7 @@ trait FixtureQueries { "new" -> Cypher.props( "id" -> newId ) - ), - includeStats = includeStats + ) ) } val successResultStats: StatementResultStats = { @@ -73,8 +87,8 @@ trait FixtureQueries { } } - object AddLabel { - def query(id: String, label: String, includeStats: Boolean = true)(implicit ns: Namespace): CypherStatement = { + case object AddLabel { + def query(id: String, label: String)(implicit ns: Namespace): CypherStatement = { CypherStatement( s"MATCH (n { ns: {props}.ns, ${PropKeys.Id}: {props}.id }) SET n ${Cypher.label(label).getOrThrow.template}", Map( @@ -82,8 +96,7 @@ trait FixtureQueries { "id" -> id, "ns" -> ns.value ) - ), - includeStats = includeStats + ) ) } val successResultStats: StatementResultStats = { diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/CommonWSNeo4jClientSpec.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/CommonWSNeo4jClientSpec.scala new file mode 100644 index 0000000..8ce816b --- /dev/null +++ b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/CommonWSNeo4jClientSpec.scala @@ -0,0 +1,66 @@ +package me.jeffmay.neo4j.client.ws + +import me.jeffmay.neo4j.client._ +import me.jeffmay.neo4j.client.cypher.{Cypher, CypherStatement} +import me.jeffmay.util.UniquePerClassNamespace +import me.jeffmay.util.akka.TestGlobalAkka +import org.scalatest.mock.MockitoSugar +import org.scalatest.{Outcome, Safety, fixture} +import org.slf4j.Logger + +import scala.concurrent.{ExecutionContext, Future} + +abstract class CommonWSNeo4jClientSpec extends fixture.AsyncFreeSpec + with Safety + with MockitoSugar + with FixtureQueries + with WSDefaultShows + with AssertResultStats + with UniquePerClassNamespace { + + override implicit def executionContext: ExecutionContext = ExecutionContext.global + + class FixtureParam extends UniqueNamespace { + val logger: Logger = mock[Logger] + val client: Neo4jClient = new WSNeo4jClient( + TestWSClient.TestWS, + Neo4jClientConfig.fromConfig(TestGlobal.config), + logger, + TestGlobalAkka.scheduler, + executionContext + ) + } + + override def withAsyncFixture(test: OneArgAsyncTest): Future[Outcome] = { + val fixture = new FixtureParam + val futureOutcome = test(fixture) + Neo4jTestNamespace.cleanupAfter(futureOutcome, fixture.namespace, fixture.client) + } + + val ClientName = "CommonWSNeo4jClient" + + s"$ClientName should not require a password change on boot" in { fixture => + import fixture._ + for { + passwordChangeRequired <- client.passwordChangeRequired() + } yield { + assert(!passwordChangeRequired, "password change should not be required after initializing") + } + } + + s"$ClientName should start with an empty unique namespace" in { fixture => + import fixture._ + val getAllNodes = CypherStatement( + "MATCH (n { ns: {props}.ns }) RETURN n", + Map("props" -> Cypher.props("ns" -> namespace.value)) + ) + for { + rsp <- client.openAndCommitTxn(getAllNodes) + } yield { + assert(rsp.errors.isEmpty, "should not encounter errors") + assertResult(Seq(), "namespace should be empty") { + rsp.results.head.data + } + } + } +} diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/InvalidCypherQueriesSpec.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/InvalidCypherQueriesSpec.scala new file mode 100644 index 0000000..bfb0b32 --- /dev/null +++ b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/InvalidCypherQueriesSpec.scala @@ -0,0 +1,21 @@ +package me.jeffmay.neo4j.client.ws + +import me.jeffmay.neo4j.client.cypher.CypherStatement +import me.jeffmay.neo4j.client.{StatusCodeException, StatusCodes} + +class InvalidCypherQueriesSpec extends CommonWSNeo4jClientSpec { + + s"$ClientName should fail to execute a MATCH query with a parameter object" in { fixture => + import fixture._ + client.openAndCommitTxn(CypherStatement( + s"MATCH (n { props })" + )).map { rsp => + fail("Props object should not be allowed in MATCH statement") + }.recover { + case ex: StatusCodeException => + assertResult(StatusCodes.Neo.ClientError.Statement.InvalidSyntax)(ex.errors.head.status) + case ex => + fail("Unexpected exception", ex) + } + } +} diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSFixtureQueriesSpec.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSFixtureQueriesSpec.scala new file mode 100644 index 0000000..22cbe2b --- /dev/null +++ b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSFixtureQueriesSpec.scala @@ -0,0 +1,86 @@ +package me.jeffmay.neo4j.client.ws + +import me.jeffmay.neo4j.client.cypher._ +import me.jeffmay.neo4j.client.{Neo4jError, Show} + +class WSFixtureQueriesSpec extends CommonWSNeo4jClientSpec { + + s"$CreateNode should add a single node to the graph" in { fixture => + import fixture._ + for { + rsp <- client.openAndCommitTxn(CreateNode.query("createSingleNode")) + } yield { + assert(rsp.errors.isEmpty, "should not encounter errors on create") + } + } + + s"$CreateNode should return the correct stats for a single create statement" in { fixture => + import fixture._ + for { + rsp <- client.openAndCommitTxn(CreateNode.query("singleWithStats").withStats) + } yield { + assertResult(1, "should have the correct stats included") { + rsp.results.head.stats.get.nodesCreated + } + } + } + + s"$CreateNodeWithPropsObject should create node with empty props object" in { fixture => + import fixture._ + for { + resp <- client.openAndCommitTxn(CreateNodeWithPropsObject.query(Cypher.props()).withStats) + _ <- client.openAndCommitTxn(CreateNodeWithPropsObject.cleanup()) + } yield { + resp.result match { + case Right(result) => + assertResult(Some(1)) { + result.stats.map(_.nodesCreated) + } + case Left(errors) => + fail(s"Unexpected errors: ${Show[Seq[Neo4jError]].show(errors)}") + } + } + } + + s"$CreateNodeWithPropsObject should create node with multiple properties" in { fixture => + import fixture._ + for { + resp <- client.openAndCommitTxn(CreateNodeWithPropsObject.query(Cypher.props("id" -> 1, "name" -> "multipleProps")).withStats) + _ <- client.openAndCommitTxn(CreateNodeWithPropsObject.cleanup()) + } yield { + resp.result match { + case Right(result) => + assertResult(Some(1)) { + result.stats.map(_.nodesCreated) + } + case Left(errors) => + fail(s"Unexpected errors: ${Show[Seq[Neo4jError]].show(errors)}") + } + } + } + + s"$FindNode should extract a node's properties" in { fixture => + import fixture._ + val nodeId = "extractNodeProps" + for { + insert <- client.openAndCommitTxn(CreateNode.query(nodeId)) + fetch <- client.openAndCommitTxn(FindNode.query(nodeId)) + } yield { + assert(insert.errors.isEmpty, "should not encounter errors on create") + fetch.result match { + case Right(rs) => + rs.rows.headOption match { + case Some(row) => + assertResult(nodeId) { + (row.col(FindNode.NodeName) \ PropKeys.Id).as[String] + } + assertResult(namespace.value) { + (row.col(FindNode.NodeName) \ PropKeys.Namespace).as[String] + } + case None => fail("Found 0 rows") + } + case Left(errors) => fail(s"Encountered errors: [${errors.map(Show[Neo4jError].show).mkString(", ")}]") + } + } + } +} diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientBasicSpecs.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientBasicSpecs.scala deleted file mode 100644 index 3a39c7a..0000000 --- a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientBasicSpecs.scala +++ /dev/null @@ -1,165 +0,0 @@ -package me.jeffmay.neo4j.client.ws - -import me.jeffmay.neo4j.client._ -import me.jeffmay.neo4j.client.cypher.{Cypher, CypherStatement} -import me.jeffmay.neo4j.client.ws.json.{DebugFormats, ShowAsJson} -import me.jeffmay.util.UniquePerClassNamespace -import me.jeffmay.util.akka.TestGlobalAkka -import org.mockito.Matchers._ -import org.mockito.Mockito._ -import org.scalatest._ -import org.scalatest.mock.MockitoSugar -import org.slf4j.Logger - -import scala.concurrent.{ExecutionContext, Future} - -class WSNeo4jClientBasicSpecs extends fixture.AsyncWordSpec - with Safety - with MockitoSugar - with AssertResultStats - with UniquePerClassNamespace { - - import DebugFormats._ - import ShowAsJson._ - - override implicit def executionContext: ExecutionContext = ExecutionContext.global - - class FixtureParam extends UniqueNamespace with FixtureQueries { - val logger: Logger = mock[Logger] - val client: Neo4jClient = new WSNeo4jClient( - TestWSClient.TestWS, - Neo4jClientConfig.fromConfig(TestGlobal.config), - logger, - TestGlobalAkka.scheduler, - executionContext - ) - } - - override def withAsyncFixture(test: OneArgAsyncTest): Future[Outcome] = { - val fixture = new FixtureParam - val futureOutcome = test(fixture) - Neo4jTestNamespace.cleanupAfter(futureOutcome, fixture.namespace, fixture.client) - } - - "WSNeo4jClient" when { - - "providing authorization" should { - - "not require a password change on boot" in { fixture => - import fixture._ - for { - passwordChangeRequired <- client.passwordChangeRequired() - } yield { - assertResult(false, "password change should not be required after initializing") { - passwordChangeRequired - } - } - } - } - - "calling openAndCloseTxn" should { - - "start with an empty unique namespace" in { fixture => - import fixture._ - val getAllNodes = CypherStatement( - "MATCH (n { ns: {props}.ns }) RETURN n", - Map("props" -> Cypher.props("ns" -> namespace.value)) - ) - for { - rsp <- client.openAndCommitTxn(getAllNodes) - } yield { - assertResult(true, "should not encounter errors") { - rsp.errors.isEmpty - } - assertResult(Seq(), "namespace should be empty") { - rsp.results.head.data - } - } - } - - "add a single node to the graph" in { fixture => - import fixture._ - for { - rsp <- client.openAndCommitTxn(CreateNode.query("createSingleNode", includeStats = false)) - } yield { - assert(rsp.errors.isEmpty, "should not encounter errors on create") - } - } - - "extract a node's properties" in { fixture => - import fixture._ - val nodeId = "extractNodeProps" - for { - insert <- client.openAndCommitTxn(CreateNode.query(nodeId)) - fetch <- client.openAndCommitTxn(FindNode.query(nodeId)) - } yield { - assert(insert.errors.isEmpty, "should not encounter errors on create") - fetch.result match { - case Right(rs) => - rs.rows.headOption match { - case Some(row) => - assertResult(nodeId) { - (row.col(FindNode.NodeName) \ PropKeys.Id).as[String] - } - assertResult(namespace.value) { - (row.col(FindNode.NodeName) \ PropKeys.Namespace).as[String] - } - case None => fail("Found 0 rows") - } - case Left(errors) => fail(s"Encountered errors: [${errors.map(Show[Neo4jError].show).mkString(", ")}]") - } - } - } - - "log queries at debug level" in { fixture => - import fixture._ - for { - rsp <- client.openAndCommitTxn(CreateNode.query("logAtDebug", includeStats = false)) - } yield { - verify(logger, times(1)).debug(any()) - Succeeded - } - } - - "return the correct stats for a single create statement" in { fixture => - import fixture._ - for { - rsp <- client.openAndCommitTxn(CreateNode.query("singleWithStats")) - } yield { - assertResult(1, "should have the correct stats included") { - rsp.results.head.stats.get.nodesCreated - } - } - } - - "return the correct stats for sequence of statements" in { fixture => - import fixture._ - val queries = Seq( - CreateNode.query("a"), - RenameNode.query("a", "b"), - AddLabel.query("b", "B") - ) - val expectedResultStats = Seq( - CreateNode.successResultStats, - RenameNode.successResultStats, - AddLabel.successResultStats - ) - for { - rsp <- client.openAndCommitTxn(queries) - } yield { - assert(rsp.results.size === expectedResultStats.size) - val tests = (expectedResultStats zip rsp.results).zipWithIndex - for (((expected, actual), n) <- tests) { - assertResultStats( - expected, - s"stats for query #${n + 1} '${queries(n).template}' did not have the expected stats") { - actual.stats.get - } - } - Succeeded - } - } - } - - } -} diff --git a/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientSpec.scala b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientSpec.scala new file mode 100644 index 0000000..d5054fc --- /dev/null +++ b/ws/src/test/scala/me/jeffmay/neo4j/client/ws/WSNeo4jClientSpec.scala @@ -0,0 +1,46 @@ +package me.jeffmay.neo4j.client.ws + +import org.mockito.Matchers._ +import org.mockito.Mockito._ +import org.scalatest._ + +class WSNeo4jClientSpec extends CommonWSNeo4jClientSpec { + + s"$ClientName should log queries at debug level" in { fixture => + import fixture._ + for { + rsp <- client.openAndCommitTxn(CreateNode.query("logAtDebug")) + } yield { + verify(logger, times(1)).debug(any()) + Succeeded + } + } + + s"$ClientName return the correct stats for sequence of statements" in { fixture => + import fixture._ + val queries = Seq( + CreateNode.query("a").withStats, + RenameNode.query("a", "b").withStats, + AddLabel.query("b", "B").withStats + ) + val expectedResultStats = Seq( + CreateNode.successResultStats, + RenameNode.successResultStats, + AddLabel.successResultStats + ) + for { + rsp <- client.openAndCommitTxn(queries) + } yield { + assert(rsp.results.size === expectedResultStats.size) + val tests = (expectedResultStats zip rsp.results).zipWithIndex + for (((expected, actual), n) <- tests) { + assertResultStats( + expected, + s"stats for query #${n + 1} '${queries(n).template}' did not have the expected stats") { + actual.stats.get + } + } + Succeeded + } + } +} From bc81ccd4477e80a8b941498fae85ef4631a9491d Mon Sep 17 00:00:00 2001 From: Jeff May Date: Wed, 16 Mar 2016 18:41:43 -0700 Subject: [PATCH 3/3] Implemented Cypher.expand and simplified role of CypherStringContext - Added CypherStatementFragment - Added CypherStatement.validate() - Added Cypher.expand() - Updated tests - Unified exceptions - Deprecated old exceptions --- .../jeffmay/neo4j/client/cypher/Cypher.scala | 28 ++- .../neo4j/client/cypher/CypherArg.scala | 41 ++++- .../client/cypher/CypherInterpolation.scala | 95 +++++----- .../neo4j/client/cypher/CypherResult.scala | 9 +- .../neo4j/client/cypher/CypherStatement.scala | 115 ++++++++++-- .../client/cypher/DefaultCypherShows.scala | 25 ++- .../neo4j/client/cypher/exceptions.scala | 174 ++++++++++++++---- .../jeffmay/neo4j/client/cypher/package.scala | 6 + .../neo4j/client/cypher/CypherArgSpec.scala | 39 ---- .../neo4j/client/cypher/CypherSpec.scala | 113 ++++++++++++ .../client/cypher/CypherStatementSpec.scala | 75 +++++++- .../cypher/CypherStringContextSpec.scala | 10 +- 12 files changed, 571 insertions(+), 159 deletions(-) delete mode 100644 core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherArgSpec.scala create mode 100644 core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherSpec.scala diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/Cypher.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/Cypher.scala index d47ddb1..56e6d94 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/Cypher.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/Cypher.scala @@ -12,7 +12,30 @@ object Cypher { def toProps[T](obj: T)(implicit writer: CypherWrites.AsProps[T]): CypherProps = writer writes obj /** - * Creates a [[CypherLabel]] to insert into the cypher query string. + * Creates a [[CypherStatementFragment]] to insert into a cypher query string for matching on the properties + * of a node. + * + * @param param the param object to expand into properties + * @param chopAndIndent whether to chop down and indent the properties with the given number of spaces + * + * @return a fragment of a [[CypherStatement]] that can be embedded into the properties selector of a node + */ + def expand(param: ImmutableParam, chopAndIndent: Option[Int] = None): CypherStatementFragment = { + val propTemplates = param.props.map { + case (k, v) => s"$k: {${param.namespace}}.$k" + } + val template = chopAndIndent match { + case Some(indentWidth) => + val shim = new String(Array.fill(indentWidth)(' ')) + propTemplates.mkString(s",\n$shim") + case None => + propTemplates.mkString(", ") + } + CypherStatementFragment(CypherStatement(template, Map(param.namespace -> param.props))) + } + + /** + * Creates a [[CypherLabel]] to insert into a cypher query string. * * @return A [[CypherResult]] which can either contain the label or an error. */ @@ -30,6 +53,7 @@ object Cypher { * * @return always successfully returns a [[CypherParamObject]] */ + @deprecated("Use Cypher.obj(Cypher.params(namespace, props))", "0.8.0") def obj(namespace: String, props: CypherProps): CypherParamObject = CypherParamObject(namespace, props) def obj(params: ImmutableParam): CypherParamObject = CypherParamObject(params.namespace, params.props) @@ -49,6 +73,7 @@ object Cypher { * @param namespace the namespace to which all parameters inserted by this class share */ sealed abstract class Param(val namespace: String) { + require(!namespace.isEmpty, "Cypher.param() namespace cannot be empty string") def isMutable: Boolean protected def __clsName: String override def toString: String = s"${__clsName}(namespace = $namespace)" @@ -107,6 +132,7 @@ object Cypher { */ sealed abstract class ImmutableParam(namespace: String, val props: CypherProps)(implicit showProps: Show[CypherProps]) extends Param(namespace) with Proxy { + def toParams: CypherParams = Map(namespace -> props) final override def isMutable: Boolean = false override def self: Any = (namespace, props) override def toString: String = s"${__clsName}(namespace = $namespace, props = ${showProps show props})" diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherArg.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherArg.scala index c1575fc..aa46dad 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherArg.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherArg.scala @@ -1,5 +1,7 @@ package me.jeffmay.neo4j.client.cypher +import scala.util.matching.Regex + /** * A valid argument to pass into the [[CypherStringContext]] used to insert some literal string or parameter(s) * into a Cypher [[CypherStatement]]. @@ -47,7 +49,8 @@ sealed abstract class CypherTemplatePart(override val template: String) extends final class CypherIdentifier private (name: String) extends CypherTemplatePart(name) object CypherIdentifier { - val Valid = "^[a-zA-Z][a-zA-Z0-9_]*$".r + private[cypher] val ValidChars: String = "[a-zA-Z][a-zA-Z0-9_]*" + private[cypher] val Valid: Regex = s"^$ValidChars$$".r def isValid(literal: String): Boolean = { Valid.findFirstMatchIn(literal).isDefined @@ -71,7 +74,8 @@ object CypherIdentifier { final class CypherLabel private (name: String) extends CypherTemplatePart(s":$name") object CypherLabel { - val Valid = "^[a-zA-Z0-9_]+$".r + private[cypher] val ValidChars: String = "[a-zA-Z0-9_]+" + private[cypher] val Valid: Regex = s"^$ValidChars$$".r private[this] var validated: Map[String, CypherLabel] = Map.empty @@ -92,6 +96,22 @@ object CypherLabel { } } +/** + * Marker trait for all [[CypherArg]]s that add [[CypherProps]] to a namespace. + */ +sealed trait CypherParamArg extends CypherArg { + + /** + * The namespace in which the [[CypherProps]] live. + */ + def namespace: String + + /** + * Extract the properties provided by this parameter. + */ + def toProps: CypherProps +} + /** * Holds a single parameter field within one of the [[CypherStatement.parameters]] namespaces. * @@ -100,8 +120,11 @@ object CypherLabel { * @param id the field name within the namespace * @param value the value of the parameter object's field */ -case class CypherParamField private[cypher] (namespace: String, id: String, value: CypherValue) extends CypherArg { +case class CypherParamField private[cypher] (namespace: String, id: String, value: CypherValue) extends CypherParamArg { + override val template: String = s"{$namespace}.$id" + + override def toProps: CypherProps = Map(id -> value) } /** @@ -141,6 +164,16 @@ case class CypherParamField private[cypher] (namespace: String, id: String, valu * @param namespace the key of the [[CypherProps]] within which field names are unique * @param props the map of fields to unfold as properties in place */ -case class CypherParamObject private[cypher] (namespace: String, props: CypherProps) extends CypherArg { +case class CypherParamObject private[cypher] (namespace: String, props: CypherProps) extends CypherParamArg { override val template: String = s"{ $namespace }" + override def toProps: CypherProps = props } + +/** + * Represents a fragment of cypher query to embed into another [[CypherStatement]]. + * + * @param statement the fragment of template and any embedded [[CypherParams]]. + */ +case class CypherStatementFragment private[cypher] (statement: CypherStatement) extends CypherArg { + override def template: String = statement.template +} \ No newline at end of file diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherInterpolation.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherInterpolation.scala index 54c27cb..9010f06 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherInterpolation.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherInterpolation.scala @@ -16,11 +16,12 @@ class CypherStringContext(val sc: StringContext) extends AnyVal { * * @return a [[CypherStatement]] with the concatenated template and the the template and parameters * of the [[CypherStatement]]. - * @throws InvalidCypherException if any of the args are [[CypherResultInvalid]] + * @throws CypherResultException if any of the args are [[CypherResultInvalid]] * @throws ConflictingParameterFieldsException if two [[CypherParamField]]s share the same namespace and property name * and the given values are different. */ def cypher(args: CypherResult[CypherArg]*): CypherStatement = { + // Build the literal query string var count: Int = 0 val tmplParts: Seq[String] = args.map { @@ -35,66 +36,56 @@ class CypherStringContext(val sc: StringContext) extends AnyVal { case invalid: CypherResultInvalid => invalid } if (invalidArgs.nonEmpty) { - throw new InvalidCypherException( + throw new CypherResultException( "Encountered errors at the |INVALID[#]| location markers in query.", Some(template), invalidArgs ) } - // Separate the dynamic props from the static props - var dynamicFields = Seq.empty[CypherParamField] - var staticObjects = Seq.empty[CypherParamObject] - args foreach { - case CypherResultValid(p: CypherParamField) => - dynamicFields :+= p - case CypherResultValid(p: CypherParamObject) => - staticObjects :+= p - case _ => - } - val objectsByNamespace = staticObjects.groupBy(_.namespace) - val fieldsByNamespace = dynamicFields.groupBy(_.namespace) - - // Collect all the static parameter objects or throw an exception - // if any dynamic properties share the same namespace - val immutableParamObjects = objectsByNamespace - .map { case (namespace, objects) => - if (fieldsByNamespace contains namespace) { - val conflictingParams = args.collect { - case CypherResultValid(p: CypherParamField) if p.namespace == namespace => Cypher.props(p.id -> p.value) - case CypherResultValid(p: CypherParamObject) if p.namespace == namespace => p.props - } - throw new MutatedParameterObjectException(namespace, conflictingParams, template) - } - else if (objects.toSet.size > 1) { - throw new ConflictingParameterObjectsException(namespace, objects.map(_.props), template) + // Collect all the field references + var foundParamFields = Map.empty[String, Set[String]] + var foundParamObjs = Map.empty[String, CypherProps] + val params = args.foldLeft(Map.empty.withDefaultValue(Map.empty: CypherProps): CypherParams) { + case (accParams, CypherResultValid(param: CypherParamArg)) => + val ns = param.namespace + param match { + case obj: CypherParamObject => + foundParamObjs.get(obj.namespace) match { + case Some(conflictingProps) if conflictingProps != obj.props => + throw new ConflictingParameterObjectsException(ns, Seq(foundParamObjs(obj.namespace), obj.props), template) + case Some(duplicates) => // nothing to do, duplicate props already found + case _ => + foundParamObjs += obj.namespace -> obj.props + } + case field: CypherParamField => + foundParamFields.get(field.namespace) match { + case Some(props) => + foundParamFields += field.namespace -> (props + field.id) + case None => + foundParamFields += field.namespace -> Set(field.id) + } } - else { - namespace -> objects.head.props + val nextProps = param.toProps + accParams.get(ns) match { + case Some(prevProps) => + // Merge non-conflicting properties / duplicates into same namespace + accParams + (ns -> CypherStatement.mergeNamespace(template, ns, prevProps, nextProps)) + case None => + // Add non-conflicting parameter namespace + accParams + (ns -> nextProps) } - } + case (accParams, _) => accParams + } - // Collect the dynamic parameter fields into properties objects - val mutableParamObjects = fieldsByNamespace - .map { case (namespace, fields) => - val props: CypherProps = fields.groupBy(_.id).map { case (name, values) => - // Allow duplicate values if they are equal - val conflictingValues = values.map(_.value) - if (conflictingValues.toSet.size > 1) { - throw new ConflictingParameterFieldsException(namespace, name, conflictingValues, template) - } - name -> values.head.value - } - namespace -> props - } + // Throw the first exception of any object references that conflict with field name references in the same namespace + for (conflictingNs <- foundParamObjs.keySet intersect foundParamFields.keySet) { + throw new MixedParamReferenceException(conflictingNs, foundParamFields(conflictingNs), template) + } - // We should not have any conflicts of namespace between static and dynamic properties - val params = immutableParamObjects ++ mutableParamObjects - assert( - params.size == immutableParamObjects.size + mutableParamObjects.size, - "Mutable and immutable param objects should never merge as " + - "combining the two should always throw a MutatedParameterObjectException" - ) - CypherStatement(template, params) + // Return a statement that has been validated for missing or conflicting parameter values + val stmt = CypherStatement(template, params) + stmt.validate() + stmt } } diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherResult.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherResult.scala index 52a075d..2e123bd 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherResult.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherResult.scala @@ -33,6 +33,13 @@ object CypherResult { implicit def paramIdentResult(arg: Param): CypherResult[CypherIdentifier] = { CypherIdentifier(arg.namespace) } + + /** + * All [[CypherStatement]]s can be embedded into other statements as [[CypherStatementFragment]]s. + */ + implicit def embedCypherStatement(stmt: CypherStatement): CypherResultValid[CypherStatementFragment] = { + CypherResultValid(CypherStatementFragment(stmt)) + } } /** @@ -53,7 +60,7 @@ case class CypherResultValid[+T <: CypherArg](result: T) extends CypherResult[T] */ case class CypherResultInvalid(result: InvalidCypherArg) extends CypherResult[Nothing] { // Construct the exception on instantiation to insure that the stack-trace captures where the failure occurs. - val exception: Throwable = new InvalidCypherException(result.message, result.template) + val exception: Throwable = new CypherResultException(result.message, result.template) final override def isValid: Boolean = false final override def getOrThrow: Nothing = throw exception } diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherStatement.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherStatement.scala index 53eb0d0..b6ec594 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherStatement.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/CypherStatement.scala @@ -1,13 +1,15 @@ package me.jeffmay.neo4j.client.cypher +import me.jeffmay.neo4j.client.Show + +import scala.util.matching.Regex + /** * Represents a cypher statement. * * @note You should always provide the parameters instead of inserting values directly into the query * to prevent cypher injection attacks. - * * @see Transaction Documentation - * * @param template the literal Cypher string with parameter placeholders * @param parameters a map of namespaces -> props objects containing the keys to substitute in the * template associated with the values. These parameters are safe from cypher @@ -20,6 +22,41 @@ case class CypherStatement( includeStats: Boolean = false ) { + /** + * Validates that the parameters referenced in the template have associated entries in the [[parameters]], + * and that object references and parameter field references are not mixed within the same query. + * + * @note This does NOT validate that the query is a valid Cypher query string. + */ + def validate(): Unit = { + // Get the referenced namespaces from the template and their referenced fields + val namespaceReferences = CypherStatement.parseParams(template) + var nsObjRefs = Set.empty[String] + var nsFieldRefs = Map.empty[String, Set[String]].withDefaultValue(Set.empty) + // Build a map of all the param fields referenced and a set of all the param object namespace references + for ((ns, fieldReferenced) <- namespaceReferences) { + fieldReferenced match { + case Some(k) => + nsFieldRefs += ns -> (nsFieldRefs(ns) + k) + case None => + nsObjRefs += ns + } + } + // Throw an exception if any references are not present in the parameters + val referencedNamespaces = nsObjRefs ++ nsFieldRefs.keySet + for (referencedNs <- referencedNamespaces) { + if (!(parameters contains referencedNs)) { + throw new MissingParamReferenceException(this, referencedNs, Set.empty) + } + val existingProps = parameters(referencedNs) + val expectedfields = nsFieldRefs(referencedNs) + val missingProps = expectedfields -- existingProps.keySet + if (missingProps.nonEmpty) { + throw new MissingParamReferenceException(this, referencedNs, missingProps) + } + } + } + /** * Request that this statement include the stats */ @@ -40,7 +77,6 @@ case class CypherStatement( * Prepends the left-hand-side statement's template to this template and merges the properties. * * @note `:+:` is right applicative, so `a :+: b` is actually `b.:+:(a)` and thus it prepends `a` to `b` - * * @see [[concat]] */ @inline final def :+:(that: CypherStatement): CypherStatement = that concat this @@ -57,25 +93,70 @@ case class CypherStatement( val mergedTemplate = this.template + that.template val conflictingNamespaces = this.parameters.keySet intersect that.parameters.keySet var mergedOverrides = Seq.empty[(String, CypherProps)] + // For every namespace that is in conflict for (ns <- conflictingNamespaces) { - val thisParams = this.parameters(ns) - val thatParams = that.parameters(ns) - val conflictingParamKeys = thisParams.keySet intersect thatParams.keySet - for (key <- conflictingParamKeys) { - val duplicates = Set(thisParams(key), thatParams(key)) - if (duplicates.size > 1) { - throw new ConflictingParameterFieldsException(ns, key, duplicates.toSeq, mergedTemplate) - } - } - mergedOverrides :+= ns -> (thisParams ++ thatParams) + // Merge the properties in the conflicting namespace + mergedOverrides :+= ns -> CypherStatement.mergeNamespace(template, ns, this.parameters(ns), that.parameters(ns)) } // Merge the parameters by iterating over the non-conflicting namespaces and appending the merged props - val mergedParams = ( + val mergedParams = { (this.parameters.view ++ that.parameters.view).filterNot { case (ns, _) => conflictingNamespaces(ns) - } ++ - mergedOverrides.view - ).toMap + } ++ mergedOverrides + }.toMap + // Build the CypherStatement by concatenating the templates and merging the properties CypherStatement(mergedTemplate, mergedParams) } } + +object CypherStatement { + + implicit def showStatement(implicit showParams: Show[CypherParams]): Show[CypherStatement] = Show.show[CypherStatement] { stmt => + "CypherStatement(" + + s" template = ${(if (stmt.template.length > DefaultCypherShows.defaultMaxLineWidth) "\n" else "") + stmt.template},\n" + + s" parameters = ${showParams.show(stmt.parameters)},\n" + + s" includeStats = ${stmt.includeStats},\n" + + ")" + } + + private[cypher] val ParamRegex: Regex = """(\{\s*%s\s*\}\s*(\.%s)?)""" + .format(CypherIdentifier.ValidChars, CypherIdentifier.ValidChars).r + + private[cypher] def parseParams(template: String): Seq[(String, Option[String])] = { + ParamRegex.findAllIn(template).foldLeft(Seq.empty[(String, Option[String])]) { (params, nextMatch) => + // Convert all "{ props }" and "{props}.id" into "props" and "props.id" respectively + val clean = nextMatch.replaceAll("[{ }]", "") + val parts = clean.split("\\.").filterNot(_.isEmpty) + if (parts.length == 1) params :+ clean -> None + else if (parts.length == 2) params :+ parts(0) -> Some(parts(1)) + else throw new IllegalArgumentException( + s"Invalid template param token, '$nextMatch' extracted from Cypher:\n" + + s"$template\n" + + s"Split on '.' created ${parts.length} non-empty parts." + ) + } + } + + /** + * Merges all non-conflicting or duplicate properties otherwise throws exception. + * + * @param template the template of this query for debugging purposes. + * @param namespace the namespace to merge for debugging purposes. + * @param first the first set of properties + * @param second the second set of properties + * @return the merged set of first and second properties + */ + @throws[ConflictingParameterFieldsException]("If the given properties contains any conflicting values for a shared key") + def mergeNamespace(template: String, namespace: String, first: CypherProps, second: CypherProps): CypherProps = { + val overlappingKeys = first.keySet intersect second.keySet + val allConflicts = overlappingKeys.toSeq.map { k => + (k, Set(first(k), second(k)).toSeq) // Small Set's retain their order + }.filter { + case (k, conflicts) => conflicts.size >= 2 + }.toMap + if (allConflicts.nonEmpty) { + throw new ConflictingParameterFieldsException(namespace, allConflicts, template) + } + first ++ second + } +} diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/DefaultCypherShows.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/DefaultCypherShows.scala index d4e6aa4..5f93ca0 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/DefaultCypherShows.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/DefaultCypherShows.scala @@ -43,9 +43,9 @@ trait DefaultCypherShows { case v: CypherByte => Show[CypherByte].show(v) } - implicit def showCypherArray[T <: CypherPrimitive](implicit showCypherPrimitive: Show[T]): Show[CypherArray[T]] = { - Show.show { case CypherArray(values) => - val asStrings = values.map(showCypherPrimitive.show) + implicit def showCypherSeq[T <: CypherValue](implicit showCypherValue: Show[CypherValue]): Show[Seq[T]] = { + Show.show { values => + val asStrings = values.map(showCypherValue.show) if (separateWithNewLines(asStrings)) { "[\n" + asStrings.map(" " + _).mkString(",\n") + "\n]" } @@ -55,6 +55,13 @@ trait DefaultCypherShows { } } + implicit def showCypherArray[T <: CypherPrimitive](implicit showCypherPrimitive: Show[CypherPrimitive]): Show[CypherArray[T]] = { + val showSeq = showCypherSeq[T] + Show.show { case CypherArray(values) => + showSeq.show(values) + } + } + implicit def showCypherValue(implicit showCypherPrimitive: Show[CypherPrimitive]): Show[CypherValue] = { Show.show { case prim: CypherPrimitive => @@ -75,4 +82,16 @@ trait DefaultCypherShows { } } } + + implicit def showCypherParams(implicit showProps: Show[CypherProps]): Show[CypherParams] = { + Show.show { params => + val asStrings = params.map { case (k, props) => "\"" + k + "\": " + showProps.show(props) } + if (separateWithNewLines(asStrings)) { + s"CypherParams {\n${asStrings.map(" " + _).mkString(",\n")}\n}" + } + else { + s"CypherParams { ${asStrings.mkString(", ")} }" + } + } + } } diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/exceptions.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/exceptions.scala index 21fae01..6bf088d 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/exceptions.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/exceptions.scala @@ -1,5 +1,7 @@ package me.jeffmay.neo4j.client.cypher +import me.jeffmay.neo4j.client.Show + /** * An exception thrown when there is an error constructing a [[CypherArg]] or [[CypherStatement]]. */ @@ -10,11 +12,10 @@ sealed abstract class CypherException(message: String) extends Exception(message * * @note this will combine multiple exceptions from multiple [[CypherResultInvalid]]s using * suppressed exceptions. - * * @param message the message detailing why the cypher statement could not be constructed * @param causes the underlying issues detailing why the individual [[CypherArg]]s are invalid */ -class InvalidCypherException( +class CypherResultException( val message: String, val template: Option[String], val causes: Seq[CypherResultInvalid] = Seq.empty @@ -35,52 +36,155 @@ class InvalidCypherException( } } +sealed abstract class ConflictingParameterException( + val namespace: String, + val conflictingFields: Map[String, Seq[CypherValue]], + val template: String, + message: String +)(implicit showConflictingFields: Show[Map[String, Seq[CypherValue]]]) extends CypherException(message) { + assert( + conflictingFields.forall { case (k, values) => values.size >= 2 }, + { + val printConflicts = showConflictingFields show conflictingFields + "This exception should not be thrown when there is less than 2 distinct conflicting fields.\n" + + s"Conflicts: $printConflicts" + } + ) +} + /** * An exception thrown when a property is defined twice on the same namespace in the same template. * * @param namespace the namespace within which the [[CypherProps]] live - * @param property the name of the property within the [[CypherProps]] + * @param conflictingFields the properties and values that are in conflict (each entry must have >= 2 values) * @param template the raw cypher template for debugging purposes */ class ConflictingParameterFieldsException( - val namespace: String, - val property: String, - val conflictingValues: Seq[CypherValue], - val template: String -) extends CypherException( - s"The parameter namespace '$namespace' has been assigned conflicting values for property '$property' in:\n" + - s"$template\n" + - s"Conflicting values are: ${conflictingValues.mkString(", ")}" - ) { - private[this] val conflictingValueSet = conflictingValues.toSet - assert( - conflictingValueSet.size >= 2, - "This exception should not be thrown when there is less than 2 distinct conflicting fields.\n" + - s"Given: $conflictingValueSet" - ) + namespace: String, + conflictingFields: Map[String, Seq[CypherValue]], + template: String +)(implicit + showConflictingFields: Show[Map[String, Seq[CypherValue]]] = ConflictingParameterFieldsException.showConflicts +) extends ConflictingParameterException( + namespace, + conflictingFields, + template, { + val printConflicts = showConflictingFields show conflictingFields + s"The parameter namespace '$namespace' has been assigned conflicting values for the following properties: $printConflicts\n" + + "In the following template:\n" + + template + } +) + +object ConflictingParameterFieldsException { + val showConflicts: Show[Map[String, Seq[CypherValue]]] = { + val showValues = Show[Seq[CypherValue]] + Show.show { conflicts => + conflicts.map { + case (k, values) => "\"" + k + "\": " + showValues.show(values) + }.mkString("{\n", ",\n ", "\n}") + } + } } +/** + * An exception thrown when a param is defined with the same namespace but two different sets of properties. + * + * @param namespace the namespace within which the [[CypherProps]] live + * @param conflictingProps the properties and values that are in conflict (each entry must have >= 2 values) + * @param template the raw cypher template for debugging purposes + */ class ConflictingParameterObjectsException( + namespace: String, + val conflictingProps: Seq[CypherProps], + template: String +)(implicit + showConflictingFields: Show[Map[String, Seq[CypherValue]]] = ConflictingParameterFieldsException.showConflicts, + showConflicts: Show[Seq[CypherProps]] = ConflictingParameterObjectsException.showConflicts +) extends ConflictingParameterException( + namespace, + { + conflictingProps + .flatMap(_.toSeq) // flatten properties into sequence of namespace -> value pairs + .groupBy { case (k, v) => k } // group by namespace + .mapValues(_.map(_._2).toSet) // group conflicting namespaces to sets of values + .filter(_._2.size >= 2) // filter out identical value sets + .mapValues(_.toSeq) // convert values back to seq keeping order for small sets + }, + template, { + val printConflicts = showConflicts show conflictingProps + s"The parameter namespace '$namespace' has been assigned conflicting values for the following properties: $printConflicts\n" + + "In the following template:\n" + + template + } +) + +object ConflictingParameterObjectsException { + val showConflicts: Show[Seq[CypherProps]] = { + val showValues = Show[CypherProps] + Show.show { conflicts => + conflicts.map { + case props => showValues.show(props).split("\n").mkString("\n ") + }.mkString("[\n", ",\n ", "\n]") + } + } +} + +/** + * Thrown when a [[CypherStatement]]'s template contains a reference to a parameter that is not present in the + * statement's parameters map. + * + * @param statement + * @param namespace + * @param missingFieldReferences + * @param showStatement + */ +class MissingParamReferenceException( + val statement: CypherStatement, val namespace: String, - val conflictingParams: Seq[CypherProps], - val template: String -) extends CypherException( - s"The parameter namespace '$namespace' has been assigned conflicting objects in:\n" + - s"$template\n" + - s"Conflicting values are: ${conflictingParams.map(" " + _).mkString("[\n", ",\n", "\n]")}" -) { - private[this] val conflictingValueSet = conflictingParams.toSet - assert( - conflictingValueSet.size >= 2, - "This exception should not be thrown when there is less than 2 distinct conflicting objects.\n" + - s"Given: $conflictingValueSet" - ) + val missingFieldReferences: Set[String] +)(implicit + showStatement: Show[CypherStatement] +) extends CypherException({ + val fieldMessage = + if (missingFieldReferences.isEmpty) "" + else s" fields ${missingFieldReferences.mkString("'", "', '", "'")}" + s"Missing parameter namespace '$namespace'$fieldMessage from cypher template:\n" + + showStatement.show(statement) + }) { + def template: String = statement.template } -class MutatedParameterObjectException( +/** + * Thrown when creating a Cypher statement that contains a reference to a parameter using both the `{ param }` object + * notation and per-field `{param}.field` notation. + * + * Mixing these styles is dangerous if the parameter field is added after the parameter object is added mutates the + * effect of the query. This can manifest an issue in 2 ways: + * + * 1. A field can be added that was not present when the properties object was first inserted into the query. + * This can lead to the unexpected behavior of having unexpected properties on a created node. + * + * 2. A field value can be mutated since the "immutable" parameter object was referenced. This violates the + * contract of immutability and is probably not what you want. + * + * @note While this is strictly forbidden when using [[CypherInterpolation]], it is allowed when constructing + * a [[CypherStatement]] directly, as it assumes you know what you are doing when constructing by hand. + * + * @param namespace the namespace of the parameter that has conflicting object and field references + * @param conflictingFieldReferences all field names that are referenced in the cypher template + * @param template the cypher string template with conflicting reference types + */ +class MixedParamReferenceException( val namespace: String, - val conflictingParams: Seq[CypherProps], + val conflictingFieldReferences: Set[String], val template: String +)(implicit + showStatement: Show[CypherStatement] ) extends CypherException({ - "ERROR" -}) + val printParamFields = conflictingFieldReferences.map(k => s"{$namespace}.$k").mkString("', '") + s"Mixed use of param object and fields is not allowed. The namespace '$namespace' contains both a param object " + + s"'{ $namespace }' as well as the following param fields '$printParamFields' in the following cypher template:\n" + + template + }) { +} diff --git a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/package.scala b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/package.scala index 698b433..9c88e79 100644 --- a/core/src/main/scala/me/jeffmay/neo4j/client/cypher/package.scala +++ b/core/src/main/scala/me/jeffmay/neo4j/client/cypher/package.scala @@ -16,5 +16,11 @@ package object cypher extends CypherInterpolation with DefaultCypherShows { type Statement = CypherStatement @deprecated("Use CypherStatement instead", "0.4.0") val Statement = CypherStatement + + @deprecated("Use CypherResultException instead", "0.7.0") + type InvalidCypherException = CypherResultException + + @deprecated("Use MixedParamReferenceException instead", "0.7.0") + type MutatedParameterObjectException = MixedParamReferenceException } diff --git a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherArgSpec.scala b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherArgSpec.scala deleted file mode 100644 index fb398c7..0000000 --- a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherArgSpec.scala +++ /dev/null @@ -1,39 +0,0 @@ -package me.jeffmay.neo4j.client.cypher - -import org.scalacheck.Gen -import org.scalatest.WordSpec -import org.scalatest.prop.GeneratorDrivenPropertyChecks - -class CypherArgSpec extends WordSpec - with GeneratorDrivenPropertyChecks { - - val genLabelLike: Gen[String] = Gen.containerOf[Array, Char]( - Gen.oneOf(Gen.alphaChar, Gen.numChar) - ).map(new String(_)) - - "CypherLabel" should { - - "Not allow empty string" in { - assert(Cypher.label("").isInvalid) - } - - "Not allow back ticks" in { - assert(Cypher.label("`a`").isInvalid) - } - - "Not allow colon" in { - assert(Cypher.label(":a").isInvalid) - } - - "Allow all alpha-numeric characters" in { - forAll(genLabelLike) { (s: String) => - whenever(!s.isEmpty) { - assertResult(s":$s") { - Cypher.label(s).getOrThrow.template - } - } - } - } - } - -} diff --git a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherSpec.scala b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherSpec.scala new file mode 100644 index 0000000..9210227 --- /dev/null +++ b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherSpec.scala @@ -0,0 +1,113 @@ +package me.jeffmay.neo4j.client.cypher + +import org.scalacheck.Gen +import org.scalatest.WordSpec +import org.scalatest.prop.GeneratorDrivenPropertyChecks + +class CypherSpec extends WordSpec + with GeneratorDrivenPropertyChecks { + + val genLabelLike: Gen[String] = Gen.containerOf[Array, Char]( + Gen.oneOf(Gen.alphaChar, Gen.numChar) + ).map(new String(_)) + + "Cypher.label" should { + + "Not allow empty string" in { + assert(Cypher.label("").isInvalid) + } + + "Not allow back ticks" in { + assert(Cypher.label("`a`").isInvalid) + } + + "Not allow colon" in { + assert(Cypher.label(":a").isInvalid) + } + + "Allow all alpha-numeric characters and look like the given string prefixed with colon" in { + forAll(genLabelLike) { (s: String) => + whenever(!s.isEmpty) { + assertResult(s":$s") { + Cypher.label(s).getOrThrow.template + } + } + } + } + } + + "Cypher.ident" should { + + "Not allow empty string" in { + assert(Cypher.ident("").isInvalid) + } + + "Not allow back ticks" in { + assert(Cypher.ident("`a`").isInvalid) + } + + "Not allow colon" in { + assert(Cypher.ident(":a").isInvalid) + } + + "Allow all alpha-numeric characters and look like the given string" in { + forAll(genLabelLike) { (s: String) => + whenever(!s.isEmpty && s.charAt(0).isLetter) { + assertResult(s) { + Cypher.ident(s).getOrThrow.template + } + } + } + } + } + + "Cypher.param" should { + + "Not allow empty string" in { + val ex = intercept[IllegalArgumentException] { + Cypher.expand(Cypher.param("", Map.empty)) + } + assert(ex.getMessage contains "empty") + } + } + + "Cypher.expand" should { + + "Construct the expected fragment for empty props" in { + val fragmentParam = Cypher.param("ns", Cypher.props()) + val fragment = Cypher.expand(fragmentParam) + assertResult("")(fragment.template) + assertResult(fragmentParam.toParams) { + fragment.statement.parameters + } + } + + "Construct the expected fragment for a single prop" in { + val fragmentParam = Cypher.param("ns", Cypher.props("id" -> 1)) + val fragment = Cypher.expand(fragmentParam) + assertResult("id: {ns}.id")(fragment.template) + assertResult(fragmentParam.toParams) { + fragment.statement.parameters + } + } + + "Construct the expected fragment for a multiple props" in { + val fragmentParam = Cypher.param("ns", Cypher.props("id" -> 1, "name" -> "example")) + val fragment = Cypher.expand(fragmentParam) + assertResult("id: {ns}.id, name: {ns}.name")(fragment.template) + assertResult(fragmentParam.toParams) { + fragment.statement.parameters + } + } + + "Construct the expected fragment for a multiple props with indent" in { + val fragmentParam = Cypher.param("ns", Cypher.props("id" -> 1, "name" -> "example")) + val fragment = Cypher.expand(fragmentParam, chopAndIndent = Some(2)) + assertResult("id: {ns}.id,\n name: {ns}.name")(fragment.template) + assertResult(fragmentParam.toParams) { + fragment.statement.parameters + } + } + } + +} diff --git a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStatementSpec.scala b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStatementSpec.scala index 316ac63..6629661 100644 --- a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStatementSpec.scala +++ b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStatementSpec.scala @@ -26,7 +26,7 @@ class CypherStatementSpec extends WordSpec { val ex = intercept[ConflictingParameterFieldsException] { cypher"MATCH (n { ref: ${props.ref(1)} }) --> (m { ref: ${props.ref(2)} })" } - assertResult(Seq(CypherInt(1), CypherInt(2)))(ex.conflictingValues) + assertResult(Seq(CypherInt(1), CypherInt(2)))(ex.conflictingFields.values.head) } } @@ -60,7 +60,7 @@ class CypherStatementSpec extends WordSpec { val ex = intercept[ConflictingParameterFieldsException] { cypher"MATCH (n { ref: ${props.ref(1)} })" :+: cypher" --> (m { ref: ${props.ref(2)} })" } - assertResult(Seq(CypherInt(1), CypherInt(2)))(ex.conflictingValues) + assertResult(Seq(CypherInt(1), CypherInt(2)))(ex.conflictingFields.values.head) } "merge all properties given" in { @@ -77,5 +77,76 @@ class CypherStatementSpec extends WordSpec { } } } + + "parseParams" should { + + "parse a template with an embedded field param" in { + assertResult(Seq("props" -> Some("name"))) { + CypherStatement.parseParams("MATCH (n { id: 1, name: {props}.name })") + } + } + + "parse a template with an embedded object param" in { + assertResult(Seq("props" -> None)) { + CypherStatement.parseParams("CREATE (m { props })") + } + } + + "parse a template with both embedded field and object params" in { + assertResult(Seq("props" -> Some("name"), "props" -> None)) { + CypherStatement.parseParams("MATCH (n { id: 1, name: {props}.name }); CREATE (m { props })") + } + } + + "parse a template empty property selectors" in { + assertResult(Seq()) { + CypherStatement.parseParams("MATCH (n {})") + } + } + + "parse a template with only the start of a param" in { + assertResult(Seq()) { + CypherStatement.parseParams("MATCH (n { id: 1, name: {props") + } + } + + "parse a template with only the end of a param" in { + assertResult(Seq()) { + CypherStatement.parseParams("}.name })") + } + } + } + + "validate()" should { + + "NOT throw an exception when parameter field is present" in { + val template = "MATCH (n { id: 1, name: {props}.name })" + CypherStatement(template, Map("props" -> Cypher.props("name" -> "field"))).validate() + } + + "throw an exception for missing parameter by field" in { + val template = "MATCH (n { id: {props}.id, name: {props}.name })" + val exception = intercept[MissingParamReferenceException] { + CypherStatement(template, Map("props" -> Cypher.props("id" -> 1))).validate() + } + assertResult("props")(exception.namespace) + assertResult(Set("name"))(exception.missingFieldReferences) + assertResult(template)(exception.statement.template) + } + + "NOT throw an exception for parameter object is present" in { + val template = "CREATE (m { mProps })" + CypherStatement(template, Map("mProps" -> Cypher.props())).validate() + } + + "throw an exception for missing parameter object" in { + val template = "CREATE (m { mProps }); CREATE (n { nProps })" + val exception = intercept[MissingParamReferenceException] { + CypherStatement(template, Map("nProps" -> Cypher.props("id" -> 1))).validate() + } + assertResult("mProps")(exception.namespace) + assertResult(template)(exception.statement.template) + } + } } } diff --git a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala index 8f79ff8..cf84c58 100644 --- a/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala +++ b/core/src/test/scala/me/jeffmay/neo4j/client/cypher/CypherStringContextSpec.scala @@ -102,7 +102,7 @@ class CypherStringContextSpec extends WordSpec ex.template } assertResult(Seq(obj1.props, obj2.props)) { - ex.conflictingParams + ex.conflictingProps } } @@ -112,7 +112,7 @@ class CypherStringContextSpec extends WordSpec val conflicting = Cypher.param(conflictingNamespace) val conflictingKey = "anything" val conflictingValue = "doesn't matter" - val ex = intercept[MutatedParameterObjectException] { + val ex = intercept[MixedParamReferenceException] { cypher"MATCH (n $obj) --> (m { id: ${conflicting.applyDynamic(conflictingKey)(conflictingValue)} })" } assertResult( @@ -120,8 +120,8 @@ class CypherStringContextSpec extends WordSpec "template did not render properly") { ex.template } - assertResult(Seq(obj.props, Cypher.props(conflictingKey -> conflictingValue))) { - ex.conflictingParams + assertResult(Set(conflictingKey)) { + ex.conflictingFieldReferences } } @@ -163,7 +163,7 @@ class CypherStringContextSpec extends WordSpec "throw an exception with all invalid cypher arguments included as suppressed exceptions" in { val invalidLabelName = ":INVALID" val invalid = Cypher.label(invalidLabelName) - val ex = intercept[InvalidCypherException](cypher"MATCH (n $invalid) RETURN n") + val ex = intercept[CypherResultException](cypher"MATCH (n $invalid) RETURN n") assertResult(Some("MATCH (n |INVALID[1]|) RETURN n")) { ex.template }