Skip to content

Commit a2d9ee0

Browse files
GH-2277 - Don't log warnings on literal null parameters.
This changes the log level to `debug` for literal null parameters and also improves the wording around it. In addition, a TODO has been removed: - It doesn't make sense to move the conversion of RANGE etc. to the conversion service, as those will be converted to maps etc. which are subject to further conversion anyway This fixes #2277.
1 parent c3efb37 commit a2d9ee0

File tree

4 files changed

+36
-27
lines changed

4 files changed

+36
-27
lines changed

src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java

+1
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ protected QueryFragmentsAndParameters complete(@Nullable Condition condition, So
259259
QueryFragments queryFragments = createQueryFragments(condition, sort);
260260

261261
Map<String, Object> convertedParameters = this.boundedParameters.stream()
262+
.peek(p -> Neo4jQuerySupport.logParameterIfNull(p.nameOrIndex, p.value))
262263
.collect(Collectors.toMap(p -> p.nameOrIndex, p -> parameterConversion.apply(p.value, p.conversionOverride)));
263264
return new QueryFragmentsAndParameters(nodeDescription, queryFragments, convertedParameters);
264265
}

src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java

+17-10
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Map;
2525
import java.util.function.BiFunction;
2626
import java.util.function.Function;
27+
import java.util.function.Supplier;
2728

2829
import org.apache.commons.logging.LogFactory;
2930
import org.neo4j.driver.Value;
@@ -115,6 +116,21 @@ protected final List<String> getInputProperties(final ResultProcessor resultProc
115116
return returnedType.isProjecting() ? returnedType.getInputProperties() : Collections.emptyList();
116117
}
117118

119+
static void logParameterIfNull(String name, Object value) {
120+
121+
if (value != null || !REPOSITORY_QUERY_LOG.isDebugEnabled()) {
122+
return;
123+
}
124+
125+
Supplier<CharSequence> messageSupplier = () -> {
126+
String pointer = name == null || name.trim().isEmpty() ? "An unknown parameter" : "$" + name;
127+
return String.format("%s points to a literal `null` value during a comparison. " +
128+
"The comparisons will always resolve to false and probably lead to an empty result.",
129+
pointer);
130+
};
131+
REPOSITORY_QUERY_LOG.debug(messageSupplier);
132+
}
133+
118134
/**
119135
* Converts parameter as needed by the query generated, which is not covered by standard conversion services.
120136
*
@@ -135,17 +151,8 @@ final Object convertParameter(Object parameter) {
135151
final Object convertParameter(Object parameter, @Nullable Function<Object, Value> conversionOverride) {
136152

137153
if (parameter == null) {
138-
// According to https://neo4j.com/docs/cypher-manual/current/syntax/working-with-null/#cypher-null-intro
139-
// it does not make any sense to continue if a `null` value gets into a comparison
140-
// but we just warn the users and do not throw an exception on `null`.
141-
REPOSITORY_QUERY_LOG.warn("Do not use `null` as a property value for comparison."
142-
+ " It will always be false and return an empty result.");
143-
144154
return Values.NULL;
145-
}
146-
147-
// Maybe move all of those into Neo4jConverter at some point.
148-
if (parameter instanceof Range) {
155+
} else if (parameter instanceof Range) {
149156
return convertRange((Range) parameter);
150157
} else if (parameter instanceof Distance) {
151158
return calculateDistanceInMeter((Distance) parameter);

src/main/java/org/springframework/data/neo4j/repository/query/ReactiveStringBasedNeo4jQuery.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -150,25 +150,25 @@ Map<String, Object> bindParameters(Neo4jParameterAccessor parameterAccessor) {
150150

151151
// Values from the parameter accessor can only get converted after evaluation
152152
for (Map.Entry<String, Object> evaluatedParam : spelEvaluator.evaluate(parameterAccessor.getValues()).entrySet()) {
153-
Object value;
154-
155-
if (evaluatedParam.getValue() instanceof Neo4jSpelSupport.LiteralReplacement) {
156-
value = evaluatedParam.getValue();
157-
} else {
153+
Object value = evaluatedParam.getValue();
154+
if (!(evaluatedParam.getValue() instanceof Neo4jSpelSupport.LiteralReplacement)) {
155+
Neo4jQuerySupport.logParameterIfNull(evaluatedParam.getKey(), value);
158156
value = super.convertParameter(evaluatedParam.getValue());
159157
}
160158
resolvedParameters.put(evaluatedParam.getKey(), value);
161159
}
162160

163161
formalParameters.stream().filter(Parameter::isBindable).forEach(parameter -> {
164162

165-
int parameterIndex = parameter.getIndex();
166-
Object parameterValue = super.convertParameter(parameterAccessor.getBindableValue(parameterIndex));
163+
int index = parameter.getIndex();
164+
Object value = parameterAccessor.getBindableValue(index);
165+
Neo4jQuerySupport.logParameterIfNull(parameter.getName().orElseGet(() -> Integer.toString(index)), value);
166+
Object convertedValue = super.convertParameter(value);
167167

168168
// Add the parameter under its name when possible
169-
parameter.getName().ifPresent(parameterName -> resolvedParameters.put(parameterName, parameterValue));
169+
parameter.getName().ifPresent(parameterName -> resolvedParameters.put(parameterName, convertedValue));
170170
// Always add under its index.
171-
resolvedParameters.put(Integer.toString(parameterIndex), parameterValue);
171+
resolvedParameters.put(Integer.toString(index), convertedValue);
172172
});
173173

174174
return resolvedParameters;

src/main/java/org/springframework/data/neo4j/repository/query/StringBasedNeo4jQuery.java

+9-8
Original file line numberDiff line numberDiff line change
@@ -200,24 +200,25 @@ Map<String, Object> bindParameters(Neo4jParameterAccessor parameterAccessor, boo
200200

201201
// Values from the parameter accessor can only get converted after evaluation
202202
for (Entry<String, Object> evaluatedParam : spelEvaluator.evaluate(parameterAccessor.getValues()).entrySet()) {
203-
Object value;
204-
if (evaluatedParam.getValue() instanceof LiteralReplacement) {
205-
value = evaluatedParam.getValue();
206-
} else {
203+
Object value = evaluatedParam.getValue();
204+
if (!(evaluatedParam.getValue() instanceof LiteralReplacement)) {
205+
Neo4jQuerySupport.logParameterIfNull(evaluatedParam.getKey(), value);
207206
value = super.convertParameter(evaluatedParam.getValue());
208207
}
209208
resolvedParameters.put(evaluatedParam.getKey(), value);
210209
}
211210

212211
formalParameters.getBindableParameters().forEach(parameter -> {
213212

214-
int parameterIndex = parameter.getIndex();
215-
Object parameterValue = super.convertParameter(parameterAccessor.getBindableValue(parameterIndex));
213+
int index = parameter.getIndex();
214+
Object value = parameterAccessor.getBindableValue(index);
215+
Neo4jQuerySupport.logParameterIfNull(parameter.getName().orElseGet(() -> Integer.toString(index)), value);
216+
Object convertedValue = super.convertParameter(value);
216217

217218
// Add the parameter under its name when possible
218-
parameter.getName().ifPresent(parameterName -> resolvedParameters.put(parameterName, parameterValue));
219+
parameter.getName().ifPresent(parameterName -> resolvedParameters.put(parameterName, convertedValue));
219220
// Always add under its index.
220-
resolvedParameters.put(Integer.toString(parameterIndex), parameterValue);
221+
resolvedParameters.put(Integer.toString(index), convertedValue);
221222
});
222223

223224
if (formalParameters.hasPageableParameter() && includePageableParameter) {

0 commit comments

Comments
 (0)