Issue 458 WithParam: throw exception if parameter starts with '$' (or… (#460)

* Issue 458 WithParam: throw exception if parameter starts with '$' (or any other not allowed character).

Issue 459 WithParam: exeption when using already existing param name outputs "key" instead of param name

* Added WithParams test method to ThrowsExceptionForDuplicateManualKey

* fixes usage of params in ArgumentExceptions

* added output of multiple invalid/duplicate parameters with tests

* missing space

Author: Clooney24

Neo4jClient.Tests/Cypher/CypherFluentQueryWithParamTests.cs

@@ -1,7 +1,9 @@
 using System;
+using FluentAssertions;
 using Neo4jClient.Cypher;
 using Newtonsoft.Json.Serialization;
 using NSubstitute;
+using NSubstitute.ExceptionExtensions;
 using Xunit;
 
 namespace Neo4jClient.Tests.Cypher
@@ -53,14 +55,27 @@ public void ThrowsExceptionForDuplicateManualKey()
             var client = Substitute.For<IRawGraphClient>();
             var query = new CypherFluentQuery(client)
                 .Match("n")
-                .WithParam("foo", 123);
+                .WithParam("foo", 123)
+                .WithParam("bar", 123);
 
             // Assert
             var ex = Assert.Throws<ArgumentException>(
                 () => query.WithParam("foo", 456)
             );
             Assert.Equal("key", ex.ParamName);
-            Assert.Equal("A parameter with the given key is already defined in the query." + Environment.NewLine + "Parameter name: key", ex.Message);
+            Assert.Equal("A parameter with the given key 'foo' is already defined in the query." + Environment.NewLine + "Parameter name: key", ex.Message);
+
+            ex = Assert.Throws<ArgumentException>(
+                () => query.WithParams( new { foo = 456 })
+            );
+            Assert.Equal("parameters", ex.ParamName);
+            Assert.Equal("A parameter with the given key 'foo' is already defined in the query." + Environment.NewLine + "Parameter name: parameters", ex.Message);
+
+            ex = Assert.Throws<ArgumentException>(
+                () => query.WithParams(new { foo = 456, bar = 456 })
+            );
+            Assert.Equal("parameters", ex.ParamName);
+            Assert.Equal("Parameters with the given keys 'foo, bar' are already defined in the query." + Environment.NewLine + "Parameter name: parameters", ex.Message);
         }
 
         [Fact]
@@ -77,7 +92,39 @@ public void ThrowsExceptionForDuplicateOfAutoKey()
                 () => query.WithParam("p0", 456)
             );
             Assert.Equal("key", ex.ParamName);
-            Assert.Equal("A parameter with the given key is already defined in the query." + Environment.NewLine + "Parameter name: key", ex.Message);
+            Assert.Equal("A parameter with the given key 'p0' is already defined in the query." + Environment.NewLine + "Parameter name: key", ex.Message);
+        }
+
+        [Fact]
+        //(Description = https://github.com/DotNet4Neo4j/Neo4jClient/issues/458)
+        public void ThrowsExceptionForInvalidParamName()
+        {
+            // Arrange
+            var client = Substitute.For<IRawGraphClient>();
+
+            // Assert WithParam
+            var ex = Assert.Throws<ArgumentException>(() => new CypherFluentQuery(client).WithParam("$uuid", ""));
+            ex.Should().NotBeNull();
+            ex.Message.Should().Be("The parameter with the given key '$uuid' is not valid. Parameters may consist of letters and numbers, and any combination of these, but cannot start with a number or a currency symbol.\r\nParameter name: key");
+
+            ex = Assert.Throws<ArgumentException>(() => new CypherFluentQuery(client).WithParam("0uuid", ""));
+            ex.Should().NotBeNull();
+            ex.Message.Should().Be("The parameter with the given key '0uuid' is not valid. Parameters may consist of letters and numbers, and any combination of these, but cannot start with a number or a currency symbol.\r\nParameter name: key");
+
+            ex = Assert.Throws<ArgumentException>(() => new CypherFluentQuery(client).WithParam("{uuid}", ""));
+            ex.Should().NotBeNull();
+            ex.Message.Should().Be("The parameter with the given key '{uuid}' is not valid. Parameters may consist of letters and numbers, and any combination of these, but cannot start with a number or a currency symbol.\r\nParameter name: key");
+
+            // no exception for correct usage
+            var ex2 = Record.Exception(() => new CypherFluentQuery(client).WithParam("uuid", ""));
+            Assert.Null(ex2);
+
+            // Assert WithParams
+            //ex = Assert.Throws<ArgumentException>(() => new CypherFluentQuery(client).WithParams(new { $uuid = "" })); // this will not compile anyways
+
+            // no exception for correct usage
+            ex2 = Record.Exception(() => new CypherFluentQuery(client).WithParams(new { uuid = "" }));
+            Assert.Null(ex2);
         }
 
         public class ComplexObjForWithParamTest

Neo4jClient/Cypher/CypherFluentQuery.cs

@@ -148,17 +148,33 @@ protected ICypherFluentQuery<TResult> Mutate<TResult>(Action<QueryWriter> callba
 
         public ICypherFluentQuery WithParam(string key, object value)
         {
+            if (string.IsNullOrWhiteSpace(key) || char.IsDigit(key[0]) || char.IsSymbol(key[0]) || key[0] == '{')
+                throw new ArgumentException($"The parameter with the given key '{key}' is not valid. Parameters may consist of letters and numbers, and any combination of these, but cannot start with a number or a currency symbol.", nameof(key));
+
             if (QueryWriter.ContainsParameterWithKey(key))
-                throw new ArgumentException("A parameter with the given key is already defined in the query.", nameof(key));
+                throw new ArgumentException($"A parameter with the given key '{key}' is already defined in the query.", nameof(key));
+
             return Mutate(w => w.CreateParameter(key, value));
         }
 
         public ICypherFluentQuery WithParams(IDictionary<string, object> parameters)
         {
             if (parameters == null || parameters.Count == 0) return this;
 
-            if (parameters.Keys.Any(key => QueryWriter.ContainsParameterWithKey(key)))
-                throw new ArgumentException("A parameter with the given key is already defined in the query.", nameof(parameters));
+            var invalidParameters = parameters.Keys.Where(key => string.IsNullOrWhiteSpace(key) || char.IsDigit(key[0]) || char.IsSymbol(key[0]) || key[0] == '{').ToList();
+
+            if (invalidParameters.Any())
+                throw new ArgumentException((invalidParameters.Count == 1 
+                    ? $"The parameter with the given key '{invalidParameters.First()}' is not valid." 
+                    : $"The parameters with the given keys '{string.Join(", ", invalidParameters)}' are not valid.") + 
+                    " Parameters may consist of letters and numbers, and any combination of these, but cannot start with a number or a currency symbol.", nameof(parameters));
+
+            var duplicateParameters = parameters.Keys.Where(key => QueryWriter.ContainsParameterWithKey(key)).ToList();
+
+            if (duplicateParameters.Any())
+                throw new ArgumentException(duplicateParameters.Count() == 1
+                    ? $"A parameter with the given key '{duplicateParameters.First()}' is already defined in the query."
+                    : $"Parameters with the given keys '{string.Join(", ", duplicateParameters)}' are already defined in the query.", nameof(parameters));
 
             return Mutate(w => w.CreateParameters(parameters));
         }
@@ -226,7 +242,7 @@ public ICypherFluentQuery Call(string storedProcedureText)
             if (!Client.CypherCapabilities.SupportsStoredProcedures)
                 throw new InvalidOperationException("CALL not supported in Neo4j versions older than 3.0");
 
-           if(string.IsNullOrWhiteSpace(storedProcedureText))
+            if(string.IsNullOrWhiteSpace(storedProcedureText))
                 throw new ArgumentException("The stored procedure to call can't be null or whitespace.", nameof(storedProcedureText));
 
             return Mutate(w =>
@@ -388,18 +404,18 @@ public ICypherFluentQuery LoadCsv(Uri fileUri, string identifier, bool withHeade
             }
 
             string withHeadersEnabledText = string.Empty;
-            string fieldSeperatorEnabledText = string.Empty;
+            string fieldSeparatorEnabledText = string.Empty;
             if (withHeaders)
             {
                 withHeadersEnabledText = " WITH HEADERS";
             }
 
             if (!string.IsNullOrEmpty(fieldTerminator))
             {
-                fieldSeperatorEnabledText = $" FIELDTERMINATOR '{fieldTerminator}'";
+                fieldSeparatorEnabledText = $" FIELDTERMINATOR '{fieldTerminator}'";
             }
 
-            return Mutate(w => w.AppendClause($"{periodicCommitText} LOAD CSV{withHeadersEnabledText} FROM '{fileUri.AbsoluteUri}' AS {identifier}{fieldSeperatorEnabledText}".Trim()));
+            return Mutate(w => w.AppendClause($"{periodicCommitText} LOAD CSV{withHeadersEnabledText} FROM '{fileUri.AbsoluteUri}' AS {identifier}{fieldSeparatorEnabledText}".Trim()));
         }
 
         public ICypherFluentQuery Unwind(string collectionName, string columnName)