Repository: hibernate/hibernate-orm (language: java)

Issue:
HHH-18414 Remove duplicated call of `callAttributeBindersInSecondPass(property)`
<!--
If this is your first time contributing to the project, please consider reviewing https://github.com/hibernate/hibernate-orm/blob/main/CONTRIBUTING.md
-->

Prior to this commit, `AttributeBinder` will be called twice, it will produce duplicated tenant filter.

<!--
Please read and do not remove the following lines:
-->
----------------------
By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license](https://www.apache.org/licenses/LICENSE-2.0.txt)
and can be relicensed under the terms of the [LGPL v2.1 license](https://www.gnu.org/licenses/old-licenses/lgpl-2.1.txt) in the future at the maintainers' discretion.
For more information on licensing, please check [here](https://github.com/hibernate/hibernate-orm/blob/main/CONTRIBUTING.md#legal).

----------------------

<!-- Hibernate GitHub Bot issue links start -->
<!-- THIS SECTION IS AUTOMATICALLY GENERATED, ANY MANUAL CHANGES WILL BE LOST -->
https://hibernate.atlassian.net/browse/HHH-18414
<!-- Hibernate GitHub Bot issue links end -->


Discussion / hints:
@gavinking It seems the regression is introduced by your [commit](https://github.com/hibernate/hibernate-orm/commit/95ee026cb24ef54b8bfb24447ca133ca0f11326f), please review.
I agree with your analysis here, but I don't think the fix is quite perfect. I think it would be better to delete the call to `callAttributeBindersInSecondPass( property )` from `makeProperty()`, and ensure that all callers of `makeProperty()` also call `callAttributeBindersInSecondPass()`, *after* the property is added to its container.

Fix patch:
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java
index 44fe405313ff..8ddecb6a140d 100644
--- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java
+++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java
@@ -128,5 +128,6 @@ private static void bindAny(
 		prop.setOptional( optional && value.isNullable() );
 		//composite FK columns are in the same table, so it's OK
 		propertyHolder.addProperty( prop, columns, inferredData.getDeclaringClass() );
+		binder.callAttributeBindersInSecondPass( prop );
 	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java
index 2d8f5c0938de..786e6dc7f875 100644
--- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java
+++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java
@@ -1321,6 +1321,7 @@ private void bindProperty() {
 			throw new AssertionFailure( "DeclaringClass is not set in CollectionBinder while binding" );
 		}
 		propertyHolder.addProperty( prop, declaringClass );
+		binder.callAttributeBindersInSecondPass( prop );
 	}
 
 	@SuppressWarnings("deprecation")
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java
index c868220f3c62..fc04221faaa1 100644
--- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java
+++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java
@@ -127,6 +127,7 @@ public void doSecondPass(Map<String, PersistentClass> persistentClasses) throws
 		else {
 			bindUnowned( persistentClasses, value, result );
 		}
+		binder.callAttributeBindersInSecondPass( result );
 		value.sortProperties();
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java
index 8295a9ff56c7..5c98e401d0ba 100644
--- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java
+++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java
@@ -325,7 +325,7 @@ private Property bind(Property property) {
 		return property;
 	}
 
-	private void callAttributeBindersInSecondPass(Property property) {
+	void callAttributeBindersInSecondPass(Property property) {
 		final InFlightMetadataCollector metadataCollector = buildingContext.getMetadataCollector();
 		if ( metadataCollector.isInSecondPass() ) {
 			callAttributeBinders( property, metadataCollector.getEntityBindingMap() );
@@ -426,7 +426,6 @@ public Property makeProperty() {
 		handleOptional( property );
 		inferOptimisticLocking( property );
 		LOG.tracev( "Cascading {0} with {1}", name, cascade );
-		callAttributeBindersInSecondPass( property );
 		return property;
 	}
 


Test patch:
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/Foo.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/Foo.java
new file mode 100644
index 000000000000..81c08b14d552
--- /dev/null
+++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/Foo.java
@@ -0,0 +1,27 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.mapping.attributebinder;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import org.hibernate.annotations.AttributeBinderType;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+/**
+ * Dummy annotation to verify binders are called only once.
+ *
+ * @author Yanming Zhou
+ */
+@Target({METHOD,FIELD})
+@Retention(RUNTIME)
+@AttributeBinderType( binder = FooBinder.class )
+public @interface Foo {
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/FooBinder.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/FooBinder.java
new file mode 100644
index 000000000000..394a7b7b7ea2
--- /dev/null
+++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/FooBinder.java
@@ -0,0 +1,38 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.mapping.attributebinder;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.hibernate.binder.AttributeBinder;
+import org.hibernate.boot.spi.MetadataBuildingContext;
+import org.hibernate.mapping.PersistentClass;
+import org.hibernate.mapping.Property;
+
+/**
+ * The binder to verify binders are called only once.
+ *
+ * @author Yanming Zhou
+ */
+public class FooBinder implements AttributeBinder<Foo> {
+
+	private static final Map<String, Foo> map = new ConcurrentHashMap<>();
+
+	@Override
+	public void bind(
+			Foo annotation,
+			MetadataBuildingContext buildingContext,
+			PersistentClass persistentClass,
+			Property property) {
+			String key = persistentClass.getClassName() + "." + property.getName();
+			Foo existing = map.putIfAbsent( key, annotation );
+			if ( existing == annotation ) {
+				throw new IllegalStateException( "AttributeBinder is called twice" );
+			}
+	}
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/System.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/System.java
index 60dd253fcd42..a7b7f63bcf2b 100644
--- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/System.java
+++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/attributebinder/System.java
@@ -19,6 +19,7 @@ public class System {
 	@Basic
 	public String name;
 	@YesNo
+	@Foo
 	public boolean active;
 
 	private System() {