feat: Suppress logging of deprecation warning for id() by default.

Signed-off-by: Michael Simons <michael@simons.ac>
This commit is contained in:
Michael Simons
2025-02-18 11:22:43 +01:00
parent 341fc7daf0
commit 57d841b450
5 changed files with 189 additions and 38 deletions

View File

@@ -18,6 +18,7 @@ package org.springframework.data.neo4j.core;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -44,6 +45,12 @@ import org.springframework.lang.Nullable;
@API(status = API.Status.STABLE, since = "6.0")
public interface Neo4jClient {
/**
* This is a public API introduced to turn the logging of the infamous warning back on.
* {@code The query used a deprecated function: `id`.}
*/
AtomicBoolean SUPPRESS_ID_DEPRECATIONS = new AtomicBoolean(true);
LogAccessor cypherLog = new LogAccessor(LogFactory.getLog("org.springframework.data.neo4j.cypher"));
LogAccessor log = new LogAccessor(LogFactory.getLog(Neo4jClient.class));

View File

@@ -16,16 +16,20 @@
package org.springframework.data.neo4j.core;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.logging.LogFactory;
import org.neo4j.driver.NotificationCategory;
import org.neo4j.driver.NotificationClassification;
import org.neo4j.driver.NotificationSeverity;
import org.neo4j.driver.summary.InputPosition;
import org.neo4j.driver.summary.Notification;
import org.neo4j.driver.summary.Plan;
import org.neo4j.driver.summary.ResultSummary;
import org.springframework.core.log.LogAccessor;
import org.springframework.lang.Nullable;
/**
* Utility class for dealing with result summaries.
@@ -46,6 +50,8 @@ final class ResultSummaries {
private static final LogAccessor cypherSecurityNotificationLog = new LogAccessor(LogFactory.getLog("org.springframework.data.neo4j.cypher.security"));
private static final LogAccessor cypherTopologyNotificationLog = new LogAccessor(LogFactory.getLog("org.springframework.data.neo4j.cypher.topology"));
private static final Pattern DEPRECATED_ID_PATTERN = Pattern.compile("(?im)The query used a deprecated function: `id`\\.");
/**
* Does some post-processing on the giving result summary, especially logging all notifications
* and potentially query plans.
@@ -65,48 +71,55 @@ final class ResultSummaries {
return;
}
boolean supressIdDeprecations = Neo4jClient.SUPPRESS_ID_DEPRECATIONS.getAcquire();
Predicate<Notification> isDeprecationWarningForId;
try {
isDeprecationWarningForId = notification -> supressIdDeprecations
&& notification.classification().orElse(NotificationClassification.UNRECOGNIZED)
== NotificationClassification.DEPRECATION && DEPRECATED_ID_PATTERN.matcher(notification.description())
.matches();
} finally {
Neo4jClient.SUPPRESS_ID_DEPRECATIONS.setRelease(supressIdDeprecations);
}
String query = resultSummary.query().text();
resultSummary.notifications()
.forEach(notification -> {
LogAccessor log = notification.category()
.map(ResultSummaries::getLogAccessor)
.orElse(Neo4jClient.cypherLog);
Consumer<String> logFunction =
switch (notification.severity()) {
case "WARNING" -> log::warn;
case "INFORMATION" -> log::info;
default -> log::debug;
};
.stream().filter(Predicate.not(isDeprecationWarningForId))
.forEach(notification -> notification.severityLevel().ifPresent(severityLevel -> {
var category = notification.classification().orElse(null);
var logger = getLogAccessor(category);
Consumer<String> logFunction;
if (severityLevel == NotificationSeverity.WARNING) {
logFunction = logger::warn;
} else if (severityLevel == NotificationSeverity.INFORMATION) {
logFunction = logger::info;
} else if (severityLevel == NotificationSeverity.OFF) {
logFunction = (String message) -> {
};
} else {
logFunction = logger::debug;
}
logFunction.accept(ResultSummaries.format(notification, query));
});
}));
}
private static LogAccessor getLogAccessor(NotificationCategory category) {
if (category == NotificationCategory.HINT) {
return cypherHintNotificationLog;
private static LogAccessor getLogAccessor(@Nullable NotificationClassification category) {
if (category == null) {
return Neo4jClient.cypherLog;
}
if (category == NotificationCategory.DEPRECATION) {
return cypherDeprecationNotificationLog;
}
if (category == NotificationCategory.PERFORMANCE) {
return cypherPerformanceNotificationLog;
}
if (category == NotificationCategory.GENERIC) {
return cypherGenericNotificationLog;
}
if (category == NotificationCategory.UNSUPPORTED) {
return cypherUnsupportedNotificationLog;
}
if (category == NotificationCategory.UNRECOGNIZED) {
return cypherUnrecognizedNotificationLog;
}
if (category == NotificationCategory.SECURITY) {
return cypherSecurityNotificationLog;
}
if (category == NotificationCategory.TOPOLOGY) {
return cypherTopologyNotificationLog;
}
return Neo4jClient.cypherLog;
return switch (category) {
case HINT -> cypherHintNotificationLog;
case DEPRECATION -> cypherDeprecationNotificationLog;
case PERFORMANCE -> cypherPerformanceNotificationLog;
case GENERIC -> cypherGenericNotificationLog;
case UNSUPPORTED -> cypherUnsupportedNotificationLog;
case UNRECOGNIZED -> cypherUnrecognizedNotificationLog;
case SECURITY -> cypherSecurityNotificationLog;
case TOPOLOGY -> cypherTopologyNotificationLog;
default -> Neo4jClient.cypherLog;
};
}
/**

View File

@@ -80,6 +80,7 @@ abstract class AbstractElementIdTestBase {
assertThat(formattedMessages)
.noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") ||
s.contains("The query used a deprecated function. ('id' is no longer supported)") ||
s.contains("The query used a deprecated function: `id`.") ||
s.matches("(?s).*toString\\(id\\(.*")); // No deprecations are logged when deprecated function call is nested. Anzeige ist raus.
}
}

View File

@@ -0,0 +1,130 @@
/*
* Copyright 2011-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.neo4j.integration.misc;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import java.util.function.Predicate;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledIf;
import org.junit.jupiter.api.extension.ExtendWith;
import org.neo4j.driver.Driver;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.neo4j.core.Neo4jClient;
import org.springframework.data.neo4j.integration.bookmarks.DatabaseInitializer;
import org.springframework.data.neo4j.test.LogbackCapture;
import org.springframework.data.neo4j.test.LogbackCapturingExtension;
import org.springframework.data.neo4j.test.Neo4jExtension;
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.data.neo4j.test.ServerVersion;
import org.springframework.transaction.annotation.EnableTransactionManagement;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
@Neo4jIntegrationTest
@ExtendWith(LogbackCapturingExtension.class)
class IdLoggingIT {
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
@Configuration
@EnableTransactionManagement
@ComponentScan
static class Config extends Neo4jImperativeTestConfiguration {
@Bean
DatabaseInitializer databaseInitializer(Driver driver) {
return new DatabaseInitializer(driver);
}
@Bean
public Driver driver() {
return neo4jConnectionSupport.getDriver();
}
@Override
public boolean isCypher5Compatible() {
return neo4jConnectionSupport.isCypher5SyntaxCompatible();
}
}
static boolean isGreaterThanOrEqualNeo4j5() {
return neo4jConnectionSupport.getServerVersion().greaterThanOrEqual(ServerVersion.v5_0_0);
}
@EnabledIf("isGreaterThanOrEqualNeo4j5")
@Test
void idWarningShouldBeSuppressed(LogbackCapture logbackCapture, @Autowired Neo4jClient neo4jClient) {
// Was not able to combine the autowiring of capture and the client here
for (Boolean enabled : new Boolean[] {true, false, null}) {
Logger logger = (Logger) org.slf4j.LoggerFactory.getLogger("org.springframework.data.neo4j.cypher.deprecation");
Level originalLevel = logger.getLevel();
logger.setLevel(Level.DEBUG);
Boolean oldValue = null;
if (enabled != null) {
oldValue = Neo4jClient.SUPPRESS_ID_DEPRECATIONS.getAndSet(enabled);
}
try {
assertThatCode(() -> neo4jClient.query(
"CREATE (n:XXXIdTest) RETURN id(n)").fetch().all()).doesNotThrowAnyException();
Predicate<String> stringPredicate = msg -> msg.contains(
"Neo.ClientNotification.Statement.FeatureDeprecationWarning");
if (enabled == null || enabled) {
assertThat(logbackCapture.getFormattedMessages()).noneMatch(stringPredicate);
} else {
assertThat(logbackCapture.getFormattedMessages()).anyMatch(stringPredicate);
}
} finally {
logbackCapture.clear();
logger.setLevel(originalLevel);
if (oldValue != null) {
Neo4jClient.SUPPRESS_ID_DEPRECATIONS.set(oldValue);
}
}
}
}
@EnabledIf("isGreaterThanOrEqualNeo4j5")
@Test
void otherDeprecationsWarningsShouldNotBeSuppressed(LogbackCapture logbackCapture, @Autowired Neo4jClient neo4jClient) {
Logger logger = (Logger) org.slf4j.LoggerFactory.getLogger("org.springframework.data.neo4j.cypher.deprecation");
Level originalLevel = logger.getLevel();
logger.setLevel(Level.DEBUG);
try {
assertThatCode(() -> neo4jClient.query(
"MATCH (n) CALL {WITH n RETURN count(n) AS cnt} RETURN *").fetch().all()).doesNotThrowAnyException();
assertThat(logbackCapture.getFormattedMessages())
.anyMatch(msg -> msg.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning"))
.anyMatch(msg -> msg.contains("CALL subquery without a variable scope clause is now deprecated. Use CALL (n) { ... }"));
} finally {
logger.setLevel(originalLevel);
}
}
}

View File

@@ -61,7 +61,7 @@ public final class LogbackCapture implements ExtensionContext.Store.CloseableRes
this.listAppender.start();
}
void clear() {
public void clear() {
this.resetLogLevel();
this.listAppender.list.clear();
}