From dc010d5dddec5d5ad30f6f406737f398ca5be48b Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 18 Oct 2024 10:06:22 +0200 Subject: [PATCH 1/4] HV-2057 Provide name/description explicitly when running Sonar analysis --- Jenkinsfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index 84d20fbf0c..43e1f5c099 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -367,6 +367,8 @@ stage('Sonar analysis') { -Dsonar.organization=\${SONARCLOUD_ORGANIZATION} \ -Dsonar.host.url=https://sonarcloud.io \ -Dsonar.projectKey=hibernate_hibernate-validator \ + -Dsonar.projectName="Hibernate Validator" \ + -Dsonar.projectDescription="Hibernate Validator, declare and validate application constraints" \ ${helper.scmSource.pullRequest ? """ \ -Dsonar.pullrequest.branch=${helper.scmSource.branch.name} \ -Dsonar.pullrequest.key=${helper.scmSource.pullRequest.id} \ From 054aca8888845e2ba73b31e507b00e8201514c72 Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 18 Oct 2024 10:48:32 +0200 Subject: [PATCH 2/4] HV-2057 Do not use optional.get() for required xml attributes those should be already validated and must be present in the xml --- .../internal/xml/AbstractStaxBuilder.java | 17 +++++++++++++---- .../xml/config/ValidationConfigStaxBuilder.java | 2 +- .../AbstractConstrainedElementStaxBuilder.java | 2 +- ...ConstrainedExecutableElementStaxBuilder.java | 2 +- .../internal/xml/mapping/BeanStaxBuilder.java | 2 +- .../ConstraintDefinitionStaxBuilder.java | 2 +- .../xml/mapping/ConstraintTypeStaxBuilder.java | 6 +++--- .../xml/mapping/GroupConversionStaxBuilder.java | 2 +- 8 files changed, 22 insertions(+), 13 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/AbstractStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/AbstractStaxBuilder.java index bbe5f22108..fd664797f7 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/AbstractStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/AbstractStaxBuilder.java @@ -6,6 +6,7 @@ */ package org.hibernate.validator.internal.xml; +import java.util.Locale; import java.util.Optional; import javax.xml.namespace.QName; @@ -31,9 +32,8 @@ public abstract class AbstractStaxBuilder { * corresponding xml tag can be processed based on a tag name. * * @param xmlEvent an event to check - * * @return {@code true} if corresponding event can be processed by current builder, - * {@code false} otherwise + * {@code false} otherwise */ protected boolean accept(XMLEvent xmlEvent) { return xmlEvent.isStartElement() && xmlEvent.asStartElement().getName().getLocalPart().equals( getAcceptableQName() ); @@ -59,7 +59,6 @@ public boolean process(XMLEventReader xmlEventReader, XMLEvent xmlEvent) { * return {@code some-value} as a string. * * @param xmlEventReader a current {@link XMLEventReader} - * * @return a value of a current xml tag as a string */ protected String readSingleElement(XMLEventReader xmlEventReader) throws XMLStreamException { @@ -78,11 +77,21 @@ protected String readSingleElement(XMLEventReader xmlEventReader) throws XMLStre * * @param startElement an element to get an attribute from * @param qName a {@link QName} of an attribute to read - * * @return a value of an attribute if it is present, {@link Optional#empty()} otherwise */ protected Optional readAttribute(StartElement startElement, QName qName) { Attribute attribute = startElement.getAttributeByName( qName ); return Optional.ofNullable( attribute ).map( Attribute::getValue ); } + + protected String readRequiredAttribute(StartElement startElement, QName qName) { + Attribute attribute = startElement.getAttributeByName( qName ); + if ( attribute == null ) { + // NOTE: we run schema validation before we try reading the xml, + // hence at this point if we want to get a required attribute, it should be there ... + // and if not: then there's a bigger issue >_< + throw new IllegalStateException( String.format( Locale.ROOT, "Required attribute %s not found within %s", qName, startElement ) ); + } + return attribute.getValue(); + } } diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/config/ValidationConfigStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/config/ValidationConfigStaxBuilder.java index 33ba712c39..6f5d435c66 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/config/ValidationConfigStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/config/ValidationConfigStaxBuilder.java @@ -193,7 +193,7 @@ protected String getAcceptableQName() { @Override protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { StartElement startElement = xmlEvent.asStartElement(); - String name = readAttribute( startElement, NAME_QNAME ).get(); + String name = readRequiredAttribute( startElement, NAME_QNAME ); String value = readSingleElement( xmlEventReader ); if ( LOG.isDebugEnabled() ) { LOG.debugf( diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedElementStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedElementStaxBuilder.java index c1b33da4f7..0fdd0cc465 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedElementStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedElementStaxBuilder.java @@ -66,7 +66,7 @@ abstract class AbstractConstrainedElementStaxBuilder extends AbstractStaxBuilder protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { Optional mainAttributeValueQname = getMainAttributeValueQname(); if ( mainAttributeValueQname.isPresent() ) { - mainAttributeValue = readAttribute( xmlEvent.asStartElement(), mainAttributeValueQname.get() ).get(); + mainAttributeValue = readRequiredAttribute( xmlEvent.asStartElement(), mainAttributeValueQname.get() ); } ignoreAnnotations = readAttribute( xmlEvent.asStartElement(), IGNORE_ANNOTATIONS_QNAME ).map( Boolean::parseBoolean ); ConstraintTypeStaxBuilder constraintTypeStaxBuilder = getNewConstraintTypeStaxBuilder(); diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedExecutableElementStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedExecutableElementStaxBuilder.java index 91c87f4d6c..677f22b65e 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedExecutableElementStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/AbstractConstrainedExecutableElementStaxBuilder.java @@ -57,7 +57,7 @@ abstract class AbstractConstrainedExecutableElementStaxBuilder extends AbstractS protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { Optional mainAttributeValueQname = getMainAttributeValueQname(); if ( mainAttributeValueQname.isPresent() ) { - mainAttributeValue = readAttribute( xmlEvent.asStartElement(), mainAttributeValueQname.get() ).get(); + mainAttributeValue = readRequiredAttribute( xmlEvent.asStartElement(), mainAttributeValueQname.get() ); } ignoreAnnotations = readAttribute( xmlEvent.asStartElement(), IGNORE_ANNOTATIONS_QNAME ).map( Boolean::parseBoolean ); ConstrainedParameterStaxBuilder constrainedParameterStaxBuilder = getNewConstrainedParameterStaxBuilder(); diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/BeanStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/BeanStaxBuilder.java index 35b8cd20c8..21dc9b26e6 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/BeanStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/BeanStaxBuilder.java @@ -83,7 +83,7 @@ protected String getAcceptableQName() { @Override protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { - this.className = readAttribute( xmlEvent.asStartElement(), CLASS_QNAME ).get(); + this.className = readRequiredAttribute( xmlEvent.asStartElement(), CLASS_QNAME ); this.ignoreAnnotations = readAttribute( xmlEvent.asStartElement(), IGNORE_ANNOTATIONS_QNAME ).map( Boolean::parseBoolean ); ConstrainedFieldStaxBuilder fieldStaxBuilder = getNewConstrainedFieldStaxBuilder(); ConstrainedGetterStaxBuilder getterStaxBuilder = getNewConstrainedGetterStaxBuilder(); diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintDefinitionStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintDefinitionStaxBuilder.java index 73c00d4347..09c6cf75e9 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintDefinitionStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintDefinitionStaxBuilder.java @@ -60,7 +60,7 @@ protected String getAcceptableQName() { @Override protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { - annotation = readAttribute( xmlEvent.asStartElement(), ANNOTATION_QNAME ).get(); + annotation = readRequiredAttribute( xmlEvent.asStartElement(), ANNOTATION_QNAME ); while ( !( xmlEvent.isEndElement() && xmlEvent.asEndElement().getName().getLocalPart().equals( getAcceptableQName() ) ) ) { validatedByStaxBuilder.process( xmlEventReader, xmlEvent ); xmlEvent = xmlEventReader.nextEvent(); diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintTypeStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintTypeStaxBuilder.java index 10d6b21ff8..005b165a96 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintTypeStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/ConstraintTypeStaxBuilder.java @@ -97,7 +97,7 @@ protected String getAcceptableQName() { @Override protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { StartElement startElement = xmlEvent.asStartElement(); - constraintAnnotation = readAttribute( startElement, CONSTRAINT_ANNOTATION_QNAME ).get(); + constraintAnnotation = readRequiredAttribute( startElement, CONSTRAINT_ANNOTATION_QNAME ); while ( !( xmlEvent.isEndElement() && xmlEvent.asEndElement().getName().getLocalPart().equals( CONSTRAINT_QNAME_LOCAL_PART ) ) ) { XMLEvent currentEvent = xmlEvent; builders.forEach( builder -> builder.process( xmlEventReader, currentEvent ) ); @@ -177,7 +177,7 @@ protected String getAcceptableQName() { @Override protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLStreamException { - String name = readAttribute( xmlEvent.asStartElement(), NAME_QNAME ).get(); + String name = readRequiredAttribute( xmlEvent.asStartElement(), NAME_QNAME ); while ( !( xmlEvent.isEndElement() && xmlEvent.asEndElement().getName().getLocalPart().equals( ELEMENT_QNAME_LOCAL_PART ) ) ) { xmlEvent = xmlEventReader.nextEvent(); readElement( xmlEventReader, xmlEvent, name ); @@ -244,7 +244,7 @@ protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) throws XMLS if ( xmlEvent.isStartElement() ) { StartElement startElement = xmlEvent.asStartElement(); if ( startElement.getName().getLocalPart().equals( ELEMENT_QNAME_LOCAL_PART ) ) { - String name = readAttribute( xmlEvent.asStartElement(), NAME_QNAME ).get(); + String name = readRequiredAttribute( xmlEvent.asStartElement(), NAME_QNAME ); // we put empty collection here in case the corresponding string element in xml is empty // if there will be a value it will get merged in this#addParameterValue() diff --git a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/GroupConversionStaxBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/GroupConversionStaxBuilder.java index 7cad3b8abf..dd6f3ca730 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/GroupConversionStaxBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/GroupConversionStaxBuilder.java @@ -60,7 +60,7 @@ protected String getAcceptableQName() { protected void add(XMLEventReader xmlEventReader, XMLEvent xmlEvent) { StartElement startElement = xmlEvent.asStartElement(); String from = readAttribute( startElement, FROM_QNAME ).orElse( DEFAULT_GROUP_NAME ); - String to = readAttribute( startElement, TO_QNAME ).get(); + String to = readRequiredAttribute( startElement, TO_QNAME ); groupConversionRules.merge( from, Collections.singletonList( to ), From 1ed5cb281b76509302d55c00a43f5313e18ca294 Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 18 Oct 2024 10:57:35 +0200 Subject: [PATCH 3/4] HV-2057 Adjust constraint message regex check --- .../checks/annotationparameters/AnnotationMessageCheck.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/annotation-processor/src/main/java/org/hibernate/validator/ap/internal/checks/annotationparameters/AnnotationMessageCheck.java b/annotation-processor/src/main/java/org/hibernate/validator/ap/internal/checks/annotationparameters/AnnotationMessageCheck.java index 6142123249..fd3d95daea 100644 --- a/annotation-processor/src/main/java/org/hibernate/validator/ap/internal/checks/annotationparameters/AnnotationMessageCheck.java +++ b/annotation-processor/src/main/java/org/hibernate/validator/ap/internal/checks/annotationparameters/AnnotationMessageCheck.java @@ -21,7 +21,7 @@ */ public abstract class AnnotationMessageCheck extends AnnotationParametersAbstractCheck { - private static final String WORDS_SEPARATED_WITH_DOTS = "(\\w)+(\\.(\\w)+)*"; + private static final String WORDS_SEPARATED_WITH_DOTS = "\\w+(?:\\.\\w+)*+"; // for dots and no {} around, or one of the {} is missing private static final Pattern MESSAGE_PATTERN = Pattern.compile( WORDS_SEPARATED_WITH_DOTS + "|\\{" + WORDS_SEPARATED_WITH_DOTS + "|" + WORDS_SEPARATED_WITH_DOTS + "\\}" ); From 38376877d8c5a18c60446874bdd7307628bf7ffa Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 18 Oct 2024 11:39:43 +0200 Subject: [PATCH 4/4] HV-2057 Address a few more Sonar-reported issues --- .../hibernate/validator/internal/engine/groups/Group.java | 8 +++++++- .../internal/engine/valuecontext/ValueContext.java | 5 ++--- .../internal/metadata/aggregated/BeanMetaDataImpl.java | 3 ++- .../org/hibernate/validator/internal/util/TypeHelper.java | 1 + .../privilegedactions/GetAnnotationsParameterTest.java | 4 ++-- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/groups/Group.java b/engine/src/main/java/org/hibernate/validator/internal/engine/groups/Group.java index b52389857a..1075cbbc4f 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/groups/Group.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/groups/Group.java @@ -15,6 +15,7 @@ */ public class Group { public static final Group DEFAULT_GROUP = new Group( Default.class ); + private static final String DEFAULT_GROUP_NAME = Default.class.getName(); /** * The actual group. @@ -47,7 +48,12 @@ public boolean equals(Object o) { } public boolean isDefaultGroup() { - return getDefiningClass().getName().equals( Default.class.getName() ); + return isDefaultGroup( group ); + } + + + public static boolean isDefaultGroup(Class group) { + return DEFAULT_GROUP_NAME.equals( group.getName() ); } @Override diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java index e6bcd6f176..f6ed15f220 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java @@ -8,8 +8,7 @@ import java.lang.reflect.TypeVariable; -import jakarta.validation.groups.Default; - +import org.hibernate.validator.internal.engine.groups.Group; import org.hibernate.validator.internal.engine.path.PathImpl; import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject; import org.hibernate.validator.internal.engine.valueextraction.ArrayElement; @@ -150,7 +149,7 @@ public final void setCurrentValidatedValue(V currentValue) { } public final boolean validatingDefault() { - return getCurrentGroup() != null && getCurrentGroup().getName().equals( Default.class.getName() ); + return getCurrentGroup() != null && Group.isDefaultGroup( getCurrentGroup() ); } public final ConstraintLocationKind getConstraintLocationKind() { diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java index 976e633b06..56d7ced23e 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java @@ -27,6 +27,7 @@ import jakarta.validation.metadata.ConstructorDescriptor; import jakarta.validation.metadata.PropertyDescriptor; +import org.hibernate.validator.internal.engine.groups.Group; import org.hibernate.validator.internal.engine.groups.Sequence; import org.hibernate.validator.internal.engine.groups.ValidationOrder; import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator; @@ -579,7 +580,7 @@ private static List> getValidDefaultGroupSequence(Class beanClass, L validDefaultGroupSequence.add( Default.class ); groupSequenceContainsDefault = true; } - else if ( group.getName().equals( Default.class.getName() ) ) { + else if ( Group.isDefaultGroup( group ) ) { throw LOG.getNoDefaultGroupInGroupSequenceException(); } else { diff --git a/engine/src/main/java/org/hibernate/validator/internal/util/TypeHelper.java b/engine/src/main/java/org/hibernate/validator/internal/util/TypeHelper.java index 001c83f6d3..2f0560df60 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/util/TypeHelper.java +++ b/engine/src/main/java/org/hibernate/validator/internal/util/TypeHelper.java @@ -340,6 +340,7 @@ public static Type extractConstraintType(Class> validator, int typeArgumentIndex) { + Contracts.assertNotNull( validator, "validator cannot be null" ); Map resolvedTypes = new HashMap<>(); Type constraintValidatorType = resolveTypes( resolvedTypes, validator ); diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/util/privilegedactions/GetAnnotationsParameterTest.java b/engine/src/test/java/org/hibernate/validator/test/internal/util/privilegedactions/GetAnnotationsParameterTest.java index 09012cfe3e..663d1737a4 100644 --- a/engine/src/test/java/org/hibernate/validator/test/internal/util/privilegedactions/GetAnnotationsParameterTest.java +++ b/engine/src/test/java/org/hibernate/validator/test/internal/util/privilegedactions/GetAnnotationsParameterTest.java @@ -65,7 +65,7 @@ public Class annotationType() { fail(); } catch (ValidationException e) { - assertThat( e.getMessage() ).startsWith( "HV000082" ).as( "Wrong exception message" ); + assertThat( e.getMessage() ).as( "Wrong exception message" ).startsWith( "HV000082" ); } try { @@ -73,7 +73,7 @@ public Class annotationType() { fail(); } catch (ValidationException e) { - assertThat( e.getMessage() ).startsWith( "HV000083" ).as( "Wrong exception message" ); + assertThat( e.getMessage() ).as( "Wrong exception message" ).startsWith( "HV000083" ); } } }