diff --git a/annotation-processor/src/test/java/org/hibernate/validator/ap/testutil/CompilerTestHelper.java b/annotation-processor/src/test/java/org/hibernate/validator/ap/testutil/CompilerTestHelper.java index 2288a7d14b..dc77175dc6 100644 --- a/annotation-processor/src/test/java/org/hibernate/validator/ap/testutil/CompilerTestHelper.java +++ b/annotation-processor/src/test/java/org/hibernate/validator/ap/testutil/CompilerTestHelper.java @@ -10,8 +10,8 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.EnumSet; import java.util.HashSet; import java.util.List; @@ -57,7 +57,7 @@ public enum Library { private final String name; - private Library(String name) { + Library(String name) { this.name = name; } @@ -82,7 +82,7 @@ public String getName() { PROCESSOR_OUT_DIR = new File( TARGET_DIR, "processor-generated-test-classes" ); if ( !PROCESSOR_OUT_DIR.exists() ) { if ( !PROCESSOR_OUT_DIR.mkdirs() ) { - fail( "Unable to create test output directory " + PROCESSOR_OUT_DIR.toString() ); + fail( "Unable to create test output directory " + PROCESSOR_OUT_DIR ); } } } @@ -163,7 +163,23 @@ public boolean compile(Processor annotationProcessor, DiagnosticCollector dependencies, File... sourceFiles) { StandardJavaFileManager fileManager = compiler.getStandardFileManager( null, null, null ); Iterable compilationUnits = fileManager.getJavaFileObjects( sourceFiles ); - List options = new ArrayList(); + try { + fileManager.setLocation( StandardLocation.CLASS_PATH, getDependenciesAsFiles( dependencies ) ); + fileManager.setLocation( StandardLocation.CLASS_OUTPUT, Collections.singletonList( PROCESSOR_OUT_DIR ) ); + } + catch (IOException e) { + throw new RuntimeException( e ); + } + + final List options = extractOptions( diagnosticKind, verbose, allowMethodConstraints ); + CompilationTask task = compiler.getTask( null, fileManager, diagnostics, options, null, compilationUnits ); + task.setProcessors( Collections.singletonList( annotationProcessor ) ); + + return task.call(); + } + + private static List extractOptions(Kind diagnosticKind, Boolean verbose, Boolean allowMethodConstraints) { + final List options = new ArrayList<>(); if ( diagnosticKind != null ) { options.add( StringHelper.format( "-A%s=%s", Configuration.DIAGNOSTIC_KIND_PROCESSOR_OPTION, diagnosticKind ) ); @@ -174,27 +190,9 @@ public boolean compile(Processor annotationProcessor, DiagnosticCollector asExpectations(Collection getDependenciesAsFiles(EnumSet dependencies) { - Set files = new HashSet(); + Set files = new HashSet<>(); for ( Library oneDependency : dependencies ) { files.add( new File( testLibraryDir + File.separator + oneDependency.getName() ) ); diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java index 90907484a8..4fc34f63ec 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java @@ -224,6 +224,9 @@ public CascadingMetaData build(ValueExtractorManager valueExtractorManager, Obje Set containerDetectionValueExtractorCandidates = valueExtractorManager.getResolver() .getValueExtractorCandidatesForContainerDetectionOfGlobalCascadedValidation( enclosingType ); if ( !containerDetectionValueExtractorCandidates.isEmpty() ) { + // Using @Valid on a container is deprecated at the moment. You are supposed to apply the annotation on the type argument(s). + LOG.deprecatedUseOfValidOnContainer( ReflectionHelper.getClassFromType( enclosingType ), context ); + if ( containerDetectionValueExtractorCandidates.size() > 1 ) { throw LOG.getUnableToGetMostSpecificValueExtractorDueToSeveralMaximallySpecificValueExtractorsDeclaredException( ReflectionHelper.getClassFromType( enclosingType ), diff --git a/engine/src/main/java/org/hibernate/validator/internal/util/logging/Log.java b/engine/src/main/java/org/hibernate/validator/internal/util/logging/Log.java index 590bd2af1c..684e1f8a96 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/util/logging/Log.java +++ b/engine/src/main/java/org/hibernate/validator/internal/util/logging/Log.java @@ -946,4 +946,8 @@ ConstraintDefinitionException getConstraintValidatorDefinitionConstraintMismatch @LogMessage(level = DEBUG) @Message(id = 269, value = "Unable to enable secure XML feature processing when loading %1$s: %2$s") void unableToEnableSecureFeatureProcessingSchemaXml(String fileName, String message); + + @LogMessage(level = WARN) + @Message(id = 270, value = "Using `@Valid` on a container (%1$s) is deprecated. You should apply the annotation on the type argument(s). Field name: %2$s") + void deprecatedUseOfValidOnContainer(@FormatWith(ClassObjectFormatter.class) Class valueType, Object context); } diff --git a/engine/src/test/java/org/hibernate/validator/test/constraints/annotations/hv/ValidAnnotationTest.java b/engine/src/test/java/org/hibernate/validator/test/constraints/annotations/hv/ValidAnnotationTest.java new file mode 100644 index 0000000000..0b3992e603 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/constraints/annotations/hv/ValidAnnotationTest.java @@ -0,0 +1,207 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.validator.test.constraints.annotations.hv; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import jakarta.validation.Valid; + +import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder; +import org.hibernate.validator.test.constraints.annotations.AbstractConstrainedTest; +import org.hibernate.validator.test.internal.engine.serialization.Email; +import org.hibernate.validator.testutils.ListAppender; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configurator; +import org.testng.annotations.AfterTest; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +public class ValidAnnotationTest extends AbstractConstrainedTest { + + private ListAppender logAppender; + private Level originalLogLevel; + private Logger targetLogger; + + /** + * @return true if the string matches the expected log message for deprecated use of @Value. + */ + private static boolean deprecatedUsedOfValueCode(String s) { + return s.startsWith( "HV000270" ); + } + + @BeforeTest + public void setUpLogger() { + logAppender = new ListAppender( ValidAnnotationTest.class.getSimpleName() ); + logAppender.start(); + + LoggerContext context = LoggerContext.getContext( false ); + targetLogger = context.getLogger( CascadingMetaDataBuilder.class.getName() ); + targetLogger.addAppender( logAppender ); + + // Set level of log messages to WARN (if they are not already enabled) + if ( targetLogger.getLevel().isMoreSpecificThan( Level.WARN ) ) { + // Store the original log level to restore it later + originalLogLevel = targetLogger.getLevel(); + + // Override the log level for this test class only + // Default tests will only print the error messages, we need to override it + // so that we can capture the deprecated warning message + Configurator.setLevel( CascadingMetaDataBuilder.class.getName(), Level.WARN ); + context.updateLoggers(); + } + } + + @BeforeMethod + public void cleanLogger() { + logAppender.clear(); + } + + @AfterTest + public void tearDownLogger() { + targetLogger.removeAppender( logAppender ); + logAppender.stop(); + + // Restore the original log level + if ( originalLogLevel != null ) { + Configurator.setLevel( CascadingMetaDataBuilder.class.getName(), originalLogLevel ); + // Update the logger context to apply changes + LoggerContext context = LoggerContext.getContext( false ); + context.updateLoggers(); + } + } + + @Test + public void twiceWithList() { + class Foo { + + @Valid + private List<@Valid String> prop; + + public Foo(List prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( List.of( "K1" ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + @Test + public void onTheContainerWithList() { + class Foo { + + @Valid + private List prop; + + public Foo(List prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( List.of( new MyBean( "entry.list@email.com" ) ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + @Test + public void twiceWithSet() { + class Foo { + + @Valid + private Set<@Valid String> prop; + + public Foo(Set prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( Set.of( "K1" ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + @Test + public void onTheContainerWithSet() { + class Foo { + + @Valid + private Set prop; + + public Foo(Set prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( Set.of( new MyBean( "entry.set@email.email" ) ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + @Test + public void twiceWithMap() { + class Foo { + + @Valid + private Map prop; + + public Foo(Map prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( Map.of( "K1", "V1" ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + @Test + public void onContainerWithMap() { + class Foo { + + @Valid + private Map prop; + + public Foo(Map prop) { + this.prop = prop; + } + } + + Foo foo = new Foo( Map.of( "K1", new MyBean( "value1@email.email" ) ) ); + validator.validate( foo ); + + assertThat( logAppender.getMessages() ).hasSize( 1 ).allMatch( ValidAnnotationTest::deprecatedUsedOfValueCode ); + } + + private static class MyBean { + @Email + private String email; + + public MyBean(String email) { + this.email = email; + } + + public String getEmail() { + return email; + } + + public void setEmail(String email) { + this.email = email; + } + } +} diff --git a/engine/src/test/java/org/hibernate/validator/testutils/ListAppender.java b/engine/src/test/java/org/hibernate/validator/testutils/ListAppender.java index 940485deda..94a5b8200d 100644 --- a/engine/src/test/java/org/hibernate/validator/testutils/ListAppender.java +++ b/engine/src/test/java/org/hibernate/validator/testutils/ListAppender.java @@ -4,14 +4,19 @@ */ package org.hibernate.validator.testutils; +import static java.util.Collections.unmodifiableList; + import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Predicate; +import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.config.Property; import org.apache.logging.log4j.core.impl.MutableLogEvent; +import org.apache.logging.log4j.message.Message; /** * A simple log appender for tests. @@ -39,6 +44,25 @@ public void clear() { } public List getEvents() { - return Collections.unmodifiableList( this.events ); + return unmodifiableList( this.events ); + } + + /** + * Returns all logged messages + */ + public List getMessages() { + return getMessages( logEvent -> true ); + } + + /** + * Returns logged messages matching the specified {@link Level} + */ + public List getMessages(Level level) { + return getMessages( logEvent -> logEvent.getLevel().equals( level ) ); + } + + private List getMessages(Predicate filter) { + return events.stream().filter( filter ).map( LogEvent::getMessage ) + .map( Message::getFormattedMessage ).toList(); } } diff --git a/engine/src/test/resources/log4j2.properties b/engine/src/test/resources/log4j2.properties index d4a2ce8010..69039b50a8 100644 --- a/engine/src/test/resources/log4j2.properties +++ b/engine/src/test/resources/log4j2.properties @@ -1,7 +1,8 @@ # License: Apache License, Version 2.0 # See the LICENSE file in the root directory or . - -status = error +rootLogger.level = error +rootLogger.appenderRefs = console +rootLogger.appenderRef.console.ref = console # Direct log messages to stdout appender.console.type = Console