From 57d841b450a9d0fbf3dc86d57d13885f850816be Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Tue, 18 Feb 2025 11:22:43 +0100 Subject: [PATCH] feat: Suppress logging of deprecation warning for `id()` by default. Signed-off-by: Michael Simons --- .../data/neo4j/core/Neo4jClient.java | 7 + .../data/neo4j/core/ResultSummaries.java | 87 +++++++----- .../AbstractElementIdTestBase.java | 1 + .../neo4j/integration/misc/IdLoggingIT.java | 130 ++++++++++++++++++ .../data/neo4j/test/LogbackCapture.java | 2 +- 5 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 src/test/java/org/springframework/data/neo4j/integration/misc/IdLoggingIT.java diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java index db2d7fb2a..c885ae023 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java @@ -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)); diff --git a/src/main/java/org/springframework/data/neo4j/core/ResultSummaries.java b/src/main/java/org/springframework/data/neo4j/core/ResultSummaries.java index 12f9f3720..3fa646205 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ResultSummaries.java +++ b/src/main/java/org/springframework/data/neo4j/core/ResultSummaries.java @@ -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 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 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 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; + }; } /** diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java index 10807e622..18bee6a3a 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java @@ -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. } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/misc/IdLoggingIT.java b/src/test/java/org/springframework/data/neo4j/integration/misc/IdLoggingIT.java new file mode 100644 index 000000000..96e09c130 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/misc/IdLoggingIT.java @@ -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 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); + } + } +} diff --git a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java index 92fb3b845..daccd6fab 100644 --- a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java +++ b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java @@ -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(); }