From 681e38beb58af3258cb1f75891835dc3b35eefc1 Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Wed, 20 Nov 2019 15:04:35 +0200 Subject: [PATCH 1/4] added params support --- README.md | 12 ++++-- .../com/redislabs/redisgraph/RedisGraph.java | 18 +++++++++ .../com/redislabs/redisgraph/impl/Utils.java | 39 ++++++++++++++++--- .../impl/api/AbstractRedisGraph.java | 23 +++++++++++ .../redisgraph/RedisGraphAPITest.java | 33 +++++++++++++++- .../redislabs/redisgraph/impl/UtilsTest.java | 29 +++++++++++--- 6 files changed, 138 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index c2d3332..3bda295 100644 --- a/README.md +++ b/README.md @@ -100,9 +100,13 @@ public class RedisGraphExample { // general context api. Not bound to graph key or connection RedisGraph graph = new RedisGraph(); - // send queries to a specific graph called "social" - graph.query("social","CREATE (:person{name:'roi',age:32})"); - graph.query("social","CREATE (:person{name:%s,age:%d})", "amit", 30); + Map params = new HashMap<>(); + params.put("age", 30); + params.put("name", "amit"); + + // send queries to a specific graph called "social" + graph.query("social","CREATE (:person{name:'roi',age:32})"); + graph.query("social","CREATE (:person{name:$name,age:$age})", params); graph.query("social","MATCH (a:person), (b:person) WHERE (a.name = 'roi' AND b.name='amit') CREATE (a)-[:knows]->(b)"); ResultSet resultSet = graph.query("social", "MATCH (a:person)-[r:knows]->(b:person) RETURN a, r, b"); @@ -131,7 +135,7 @@ public class RedisGraphExample { // get connection context - closable object try(RedisGraphContext context = graph.getContext()) { context.query("contextSocial","CREATE (:person{name:'roi',age:32})"); - context.query("contextSocial","CREATE (:person{name:%s,age:%d})", "amit", 30); + context.query("social","CREATE (:person{name:$name,age:$age})", params); context.query("contextSocial", "MATCH (a:person), (b:person) WHERE (a.name = 'roi' AND b.name='amit') CREATE (a)-[:knows]->(b)"); // WATCH/MULTI/EXEC context.watch("contextSocial"); diff --git a/src/main/java/com/redislabs/redisgraph/RedisGraph.java b/src/main/java/com/redislabs/redisgraph/RedisGraph.java index 25c4718..d279978 100644 --- a/src/main/java/com/redislabs/redisgraph/RedisGraph.java +++ b/src/main/java/com/redislabs/redisgraph/RedisGraph.java @@ -6,6 +6,14 @@ public interface RedisGraph extends Closeable { + /** + * Execute a Cypher query with arguments + * @param graphId a graph to perform the query on + * @param query Cypher query + * @return a result set + */ + ResultSet query(String graphId, String query); + /** * Execute a Cypher query with arguments * @param graphId a graph to perform the query on @@ -13,8 +21,18 @@ public interface RedisGraph extends Closeable { * @param args * @return a result set */ + @Deprecated ResultSet query(String graphId, String query, Object ...args); + /** + * Executes a parameterized cypher query. + * @param graphId a graph to perform the query on. + * @param query Cypher query. + * @param params parameters map. + * @return a result set. + */ + ResultSet query(String graphId, String query, Map params); + /** * Invokes stored procedures without arguments * @param graphId a graph to perform the query on diff --git a/src/main/java/com/redislabs/redisgraph/impl/Utils.java b/src/main/java/com/redislabs/redisgraph/impl/Utils.java index bb39c5b..28561e9 100644 --- a/src/main/java/com/redislabs/redisgraph/impl/Utils.java +++ b/src/main/java/com/redislabs/redisgraph/impl/Utils.java @@ -4,11 +4,7 @@ import org.apache.commons.text.translate.CharSequenceTranslator; import org.apache.commons.text.translate.LookupTranslator; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.stream.Collectors; /** @@ -56,6 +52,7 @@ private static String quoteString(String str){ * @param args - query arguments * @return formatted query */ + @Deprecated public static String prepareQuery(String query, Object ...args){ if(args.length > 0) { for(int i=0; i params){ + StringBuilder sb = new StringBuilder("CYPHER "); + for(Map.Entry entry : params.entrySet()) { + String key = entry.getKey(); + Object value = entry.getValue(); + sb.append(key).append("="); + sb.append(valueToString(value)); + sb.append(" "); + } + sb.append(query); + return sb.toString(); + } + + private static String valueToString(Object value) { + if(value == null) + return "null"; + Class valueClass = value.getClass(); + if(valueClass == String.class){ + return quoteString((String) value); + } + if(valueClass.isArray()){ + return Arrays.toString((Object[]) value); + } + return value.toString(); + } + /** * Prepare and format a procedure call and its arguments * @param procedure - procedure to invoke diff --git a/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java b/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java index d659b22..f8ce854 100644 --- a/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java +++ b/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java @@ -29,6 +29,16 @@ public abstract class AbstractRedisGraph implements RedisGraph { */ protected abstract ResultSet sendQuery(String graphId, String preparedQuery); + /** + * Execute a Cypher query with arguments + * @param graphId a graph to perform the query on + * @param query Cypher query + * @return a result set + */ + public ResultSet query(String graphId, String query) { + return sendQuery(graphId, query); + } + /** * Execute a Cypher query with arguments * @param graphId a graph to perform the query on @@ -36,11 +46,24 @@ public abstract class AbstractRedisGraph implements RedisGraph { * @param args * @return a result set */ + @Deprecated public ResultSet query(String graphId, String query, Object ...args) { String preparedQuery = Utils.prepareQuery(query, args); return sendQuery(graphId, preparedQuery); } + /** + * Executes a parameterized cypher query. + * @param graphId a graph to perform the query on. + * @param query Cypher query. + * @param params parameters map. + * @return a result set. + */ + public ResultSet query(String graphId, String query, Map params) { + String preparedQuery = Utils.prepareQuery(query, params); + return sendQuery(graphId, preparedQuery); + } + public ResultSet callProcedure(String graphId, String procedure){ return callProcedure(graphId, procedure, Utils.DUMMY_LIST, Utils.DUMMY_MAP); diff --git a/src/test/java/com/redislabs/redisgraph/RedisGraphAPITest.java b/src/test/java/com/redislabs/redisgraph/RedisGraphAPITest.java index abb78a9..0e8a2b6 100644 --- a/src/test/java/com/redislabs/redisgraph/RedisGraphAPITest.java +++ b/src/test/java/com/redislabs/redisgraph/RedisGraphAPITest.java @@ -245,7 +245,13 @@ public void testRecord(){ + "nullValue=Property{name='nullValue', value=null}, " + "since=Property{name='since', value=2000}}}", expectedEdge.toString()); - Assert.assertNotNull(api.query("social", "CREATE (:person{name:%s,age:%d, doubleValue:%f, boolValue:%b, nullValue:null})", name, age, doubleValue, boolValue)); + Map params = new HashMap<>(); + params.put("name", name); + params.put("age", age); + params.put("boolValue", boolValue); + params.put("doubleValue", doubleValue); + + Assert.assertNotNull(api.query("social", "CREATE (:person{name:$name,age:$age, doubleValue:$doubleValue, boolValue:$boolValue, nullValue:null})", params)); Assert.assertNotNull(api.query("social", "CREATE (:person{name:'amit',age:30})")); Assert.assertNotNull(api.query("social", "MATCH (a:person), (b:person) WHERE (a.name = 'roi' AND b.name='amit') " + "CREATE (a)-[:knows{place:'TLV', since:2000,doubleValue:3.14, boolValue:false, nullValue:null}]->(b)")); @@ -654,8 +660,13 @@ public void testContextedAPI() { expectedEdge.addProperty(falseBooleanProperty); expectedEdge.addProperty(nullProperty); + Map params = new HashMap<>(); + params.put("name", name); + params.put("age", age); + params.put("boolValue", boolValue); + params.put("doubleValue", doubleValue); try (RedisGraphContext c = api.getContext()) { - Assert.assertNotNull(c.query("social", "CREATE (:person{name:%s,age:%d, doubleValue:%f, boolValue:%b, nullValue:null})", name, age, doubleValue, boolValue)); + Assert.assertNotNull(c.query("social", "CREATE (:person{name:$name, age:$age, doubleValue:$doubleValue, boolValue:$boolValue, nullValue:null})", params)); Assert.assertNotNull(c.query("social", "CREATE (:person{name:'amit',age:30})")); Assert.assertNotNull(c.query("social", "MATCH (a:person), (b:person) WHERE (a.name = 'roi' AND b.name='amit') " + "CREATE (a)-[:knows{place:'TLV', since:2000,doubleValue:3.14, boolValue:false, nullValue:null}]->(b)")); @@ -920,4 +931,22 @@ public void testPath(){ } + @Test + public void testParameters(){ + Object[] parameters = {1, 2.3, true, false, null, "str", Arrays.asList(1,2,3), new Integer[]{1,2,3}}; + Map param = new HashMap<>(); + for (int i=0; i < parameters.length; i++) { + Object expected = parameters[i]; + param.put("param", expected); + ResultSet resultSet = api.query("social", "RETURN $param", param); + Assert.assertEquals(1, resultSet.size()); + Record r = resultSet.next(); + Object o = r.getValue(0); + if(i == parameters.length-1) { + expected = Arrays.asList((Object[])expected); + } + Assert.assertEquals(expected, o); + } + } + } diff --git a/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java b/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java index e6b8228..6ab3828 100644 --- a/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java +++ b/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java @@ -1,10 +1,6 @@ package com.redislabs.redisgraph.impl; -import java.util.Arrays; -import java.util.HashMap; -import java.util.IllegalFormatConversionException; -import java.util.List; -import java.util.Map; +import java.util.*; import org.junit.Assert; import org.junit.Rule; @@ -41,4 +37,27 @@ public void prepareQuery() { Assert.assertEquals("CAL prc(\"a\",\"b\")ka,kb", Utils.prepareQuery("query %s %d end of query", "a", "b")); } + @Test + public void testParamsPrep(){ + Map params = new HashMap<>(); + params.put("param", 1); + Assert.assertEquals("CYPHER param=1 RETURN $param", Utils.prepareQuery("RETURN $param", params)); + params.put("param", 2.3); + Assert.assertEquals("CYPHER param=2.3 RETURN $param", Utils.prepareQuery("RETURN $param", params)); + params.put("param", true); + Assert.assertEquals("CYPHER param=true RETURN $param", Utils.prepareQuery("RETURN $param", params)); + params.put("param", false); + Assert.assertEquals("CYPHER param=false RETURN $param", Utils.prepareQuery("RETURN $param", params)); + params.put("param", null); + Assert.assertEquals("CYPHER param=null RETURN $param", Utils.prepareQuery("RETURN $param", params)); + params.put("param", "str"); + Assert.assertEquals("CYPHER param=\"str\" RETURN $param", Utils.prepareQuery("RETURN $param", params)); + Integer arr[] = {1,2,3}; + params.put("param", arr); + Assert.assertEquals("CYPHER param=[1, 2, 3] RETURN $param", Utils.prepareQuery("RETURN $param", params)); + List list = Arrays.asList(1,2,3); + params.put("param", list); + Assert.assertEquals("CYPHER param=[1, 2, 3] RETURN $param", Utils.prepareQuery("RETURN $param", params)); + } + } From e9ea3cc079e23c6752abaa121b580678da91428a Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Mon, 25 Nov 2019 07:49:51 +0200 Subject: [PATCH 2/4] fixed PR comments --- .../com/redislabs/redisgraph/RedisGraph.java | 8 ++++-- .../com/redislabs/redisgraph/impl/Utils.java | 28 ++++++++++++++----- .../impl/api/AbstractRedisGraph.java | 4 +-- .../redislabs/redisgraph/impl/UtilsTest.java | 6 ++++ 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/redislabs/redisgraph/RedisGraph.java b/src/main/java/com/redislabs/redisgraph/RedisGraph.java index d279978..fe3b19d 100644 --- a/src/main/java/com/redislabs/redisgraph/RedisGraph.java +++ b/src/main/java/com/redislabs/redisgraph/RedisGraph.java @@ -7,25 +7,27 @@ public interface RedisGraph extends Closeable { /** - * Execute a Cypher query with arguments + * Execute a Cypher query. * @param graphId a graph to perform the query on * @param query Cypher query * @return a result set */ ResultSet query(String graphId, String query); + @Deprecated /** + * This function is deprecated and will be removed soon. Instead use + * query(String graphId, String query, Map params) * Execute a Cypher query with arguments * @param graphId a graph to perform the query on * @param query Cypher query * @param args * @return a result set */ - @Deprecated ResultSet query(String graphId, String query, Object ...args); /** - * Executes a parameterized cypher query. + * Executes a cypher query with parameters. * @param graphId a graph to perform the query on. * @param query Cypher query. * @param params parameters map. diff --git a/src/main/java/com/redislabs/redisgraph/impl/Utils.java b/src/main/java/com/redislabs/redisgraph/impl/Utils.java index 28561e9..ddd3c34 100644 --- a/src/main/java/com/redislabs/redisgraph/impl/Utils.java +++ b/src/main/java/com/redislabs/redisgraph/impl/Utils.java @@ -46,13 +46,15 @@ private static String quoteString(String str){ return sb.toString(); } + @Deprecated /** + * This function is deprecated and will be removed soon. Instead use + * String prepareQuery(String query, Map params). * Prepare and formats a query and query arguments * @param query - query * @param args - query arguments * @return formatted query */ - @Deprecated public static String prepareQuery(String query, Object ...args){ if(args.length > 0) { for(int i=0; i params){ for(Map.Entry entry : params.entrySet()) { String key = entry.getKey(); Object value = entry.getValue(); - sb.append(key).append("="); + sb.append(key).append('='); sb.append(valueToString(value)); - sb.append(" "); + sb.append(' '); } sb.append(query); return sb.toString(); } + private static String arrayToString(Object[] arr) { + StringBuilder sb = new StringBuilder().append('['); + sb.append(String.join(", ", Arrays.stream(arr).map(obj->valueToString(obj)).collect(Collectors.toList()))); + sb.append(']'); + return sb.toString(); + } + private static String valueToString(Object value) { if(value == null) return "null"; - Class valueClass = value.getClass(); - if(valueClass == String.class){ + if(String.class.isInstance(value)){ return quoteString((String) value); } - if(valueClass.isArray()){ - return Arrays.toString((Object[]) value); + + if(value.getClass().isArray()){ + return arrayToString((Object[]) value); + + } + if(List.class.isInstance(value)){ + List list = (List)value; + return arrayToString(list.toArray()); } return value.toString(); } diff --git a/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java b/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java index f8ce854..d8e0d6f 100644 --- a/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java +++ b/src/main/java/com/redislabs/redisgraph/impl/api/AbstractRedisGraph.java @@ -30,7 +30,7 @@ public abstract class AbstractRedisGraph implements RedisGraph { protected abstract ResultSet sendQuery(String graphId, String preparedQuery); /** - * Execute a Cypher query with arguments + * Execute a Cypher query. * @param graphId a graph to perform the query on * @param query Cypher query * @return a result set @@ -53,7 +53,7 @@ public ResultSet query(String graphId, String query, Object ...args) { } /** - * Executes a parameterized cypher query. + * Executes a cypher query with parameters. * @param graphId a graph to perform the query on. * @param query Cypher query. * @param params parameters map. diff --git a/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java b/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java index 6ab3828..9caed94 100644 --- a/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java +++ b/src/test/java/com/redislabs/redisgraph/impl/UtilsTest.java @@ -58,6 +58,12 @@ public void testParamsPrep(){ List list = Arrays.asList(1,2,3); params.put("param", list); Assert.assertEquals("CYPHER param=[1, 2, 3] RETURN $param", Utils.prepareQuery("RETURN $param", params)); + String strArr[] = {"1", "2", "3"}; + params.put("param", strArr); + Assert.assertEquals("CYPHER param=[\"1\", \"2\", \"3\"] RETURN $param", Utils.prepareQuery("RETURN $param", params)); + List stringList = Arrays.asList("1", "2", "3"); + params.put("param", stringList); + Assert.assertEquals("CYPHER param=[\"1\", \"2\", \"3\"] RETURN $param", Utils.prepareQuery("RETURN $param", params)); } } From 290aba3b7e52705b4fc03bdaf6e17b35cf75cd5f Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Mon, 25 Nov 2019 08:01:00 +0200 Subject: [PATCH 3/4] added missing parameters exception validation --- .../exceptions/JRedisGraphErrorTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/java/com/redislabs/redisgraph/exceptions/JRedisGraphErrorTest.java b/src/test/java/com/redislabs/redisgraph/exceptions/JRedisGraphErrorTest.java index 5b0be9e..c7a6862 100644 --- a/src/test/java/com/redislabs/redisgraph/exceptions/JRedisGraphErrorTest.java +++ b/src/test/java/com/redislabs/redisgraph/exceptions/JRedisGraphErrorTest.java @@ -10,6 +10,8 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import java.util.HashMap; + public class JRedisGraphErrorTest { @@ -86,6 +88,20 @@ public void testContextSyntaxErrorReporting() { } + @Test + public void testMissingParametersSyntaxErrorReporting(){ + exceptionRule.expect(JRedisGraphCompileTimeException.class); + exceptionRule.expectMessage("Missing parameters"); + api.query("social","RETURN $param"); + } + + @Test + public void testMissingParametersSyntaxErrorReporting2(){ + exceptionRule.expect(JRedisGraphCompileTimeException.class); + exceptionRule.expectMessage("Missing parameters"); + api.query("social","RETURN $param", new HashMap<>()); + } + @Test public void testContextRuntimeErrorReporting() { exceptionRule.expect(JRedisGraphRunTimeException.class); From 59a6a987107a6d93c3ff05501242ea1b65a45faa Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Mon, 25 Nov 2019 09:36:27 +0200 Subject: [PATCH 4/4] added deprecated annotations --- src/main/java/com/redislabs/redisgraph/RedisGraph.java | 6 +++--- src/main/java/com/redislabs/redisgraph/impl/Utils.java | 6 +++--- .../redislabs/redisgraph/impl/api/AbstractRedisGraph.java | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/redislabs/redisgraph/RedisGraph.java b/src/main/java/com/redislabs/redisgraph/RedisGraph.java index fe3b19d..e672666 100644 --- a/src/main/java/com/redislabs/redisgraph/RedisGraph.java +++ b/src/main/java/com/redislabs/redisgraph/RedisGraph.java @@ -14,16 +14,16 @@ public interface RedisGraph extends Closeable { */ ResultSet query(String graphId, String query); - @Deprecated + /** - * This function is deprecated and will be removed soon. Instead use - * query(String graphId, String query, Map params) * Execute a Cypher query with arguments * @param graphId a graph to perform the query on * @param query Cypher query * @param args * @return a result set + * @deprecated use {@link #query(String, String, Map)} instead. */ + @Deprecated ResultSet query(String graphId, String query, Object ...args); /** diff --git a/src/main/java/com/redislabs/redisgraph/impl/Utils.java b/src/main/java/com/redislabs/redisgraph/impl/Utils.java index ddd3c34..dbc746a 100644 --- a/src/main/java/com/redislabs/redisgraph/impl/Utils.java +++ b/src/main/java/com/redislabs/redisgraph/impl/Utils.java @@ -46,15 +46,15 @@ private static String quoteString(String str){ return sb.toString(); } - @Deprecated + /** - * This function is deprecated and will be removed soon. Instead use - * String prepareQuery(String query, Map params). * Prepare and formats a query and query arguments * @param query - query * @param args - query arguments * @return formatted query + * @deprecated use {@link #prepareQuery(String, Map)} instead. */ + @Deprecated public static String prepareQuery(String query, Object ...args){ if(args.length > 0) { for(int i=0; i