HHH-5289 remove unnecessary security checks in property accessors

brmeyer · brmeyer · commit 78a62acc2d3e · 2014-01-10T16:06:50.000-05:00
diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java b/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java
@@ -69,9 +69,7 @@ public static void validateFactory(Object object) {
 			final Class activatorClass = BeanValidationIntegrator.class.getClassLoader().loadClass( ACTIVATOR_CLASS_NAME );
 			try {
 				final Method validateMethod = activatorClass.getMethod( VALIDATE_SUPPLIED_FACTORY_METHOD_NAME, Object.class );
-				if ( ! validateMethod.isAccessible() ) {
-					validateMethod.setAccessible( true );
-				}
+				validateMethod.setAccessible( true );
 				try {
 					validateMethod.invoke( null, object );
 				}
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java
@@ -303,9 +303,7 @@ public static <T> Constructor<T> getDefaultConstructor(Class<T> clazz) throws Pr
 
 		try {
 			Constructor<T> constructor = clazz.getDeclaredConstructor( NO_PARAM_SIGNATURE );
-			if ( !isPublic( clazz, constructor ) ) {
-				constructor.setAccessible( true );
-			}
+			constructor.setAccessible( true );
 			return constructor;
 		}
 		catch ( NoSuchMethodException nme ) {
@@ -363,9 +361,7 @@ public static Constructor getConstructor(Class clazz, Type[] types) throws Prope
 					}
 				}
 				if ( found ) {
-					if ( !isPublic( clazz, constructor ) ) {
-						constructor.setAccessible( true );
-					}
+					constructor.setAccessible( true );
 					return constructor;
 				}
 			}
diff --git a/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java b/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java
@@ -260,7 +260,7 @@ private static BasicSetter getSetterOrNull(Class theClass, String propertyName)
 		Method method = setterMethod(theClass, propertyName);
 
 		if (method!=null) {
-			if ( !ReflectHelper.isPublic(theClass, method) ) method.setAccessible(true);
+			method.setAccessible(true);
 			return new BasicSetter(theClass, method, propertyName);
 		}
 		else {
@@ -325,9 +325,7 @@ private static BasicGetter getGetterOrNull(Class theClass, String propertyName)
 		Method method = getterMethod(theClass, propertyName);
 
 		if (method!=null) {
-			if ( !ReflectHelper.isPublic( theClass, method ) ) {
-				method.setAccessible(true);
-			}
+			method.setAccessible(true);
 			return new BasicGetter(theClass, method, propertyName);
 		}
 		else {
diff --git a/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java b/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java
@@ -157,7 +157,7 @@ private static Field getField(Class clazz, String name) throws PropertyNotFoundE
 		catch (NoSuchFieldException nsfe) {
 			field = getField( clazz, clazz.getSuperclass(), name );
 		}
-		if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true);
+		field.setAccessible(true);
 		return field;
 	}
 
@@ -172,7 +172,7 @@ private static Field getField(Class root, Class clazz, String name) throws Prope
 		catch (NoSuchFieldException nsfe) {
 			field = getField( root, clazz.getSuperclass(), name );
 		}
-		if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true);
+		field.setAccessible(true);
 		return field;
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java b/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java
@@ -192,9 +192,7 @@ public Object invoke(
 						returnValue = thisMethod.invoke( target, args );
 					}
 					else {
-						if ( !thisMethod.isAccessible() ) {
-							thisMethod.setAccessible( true );
-						}
+						thisMethod.setAccessible( true );
 						returnValue = thisMethod.invoke( target, args );
 					}
 					
diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java b/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java
@@ -135,14 +135,11 @@ private Constructor<? extends ComponentTuplizer> getProperConstructor(Class<? ex
 		Constructor<? extends ComponentTuplizer> constructor = null;
 		try {
 			constructor = clazz.getDeclaredConstructor( COMPONENT_TUP_CTOR_SIG );
-			if ( ! ReflectHelper.isPublic( constructor ) ) {
-				try {
-					// found a constructor, but it was not publicly accessible so try to request accessibility
-					constructor.setAccessible( true );
-				}
-				catch ( SecurityException e ) {
-					constructor = null;
-				}
+			try {
+				constructor.setAccessible( true );
+			}
+			catch ( SecurityException e ) {
+				constructor = null;
 			}
 		}
 		catch ( NoSuchMethodException ignore ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java
@@ -227,14 +227,11 @@ private Constructor<? extends EntityTuplizer> getProperConstructor(
 		Constructor<? extends EntityTuplizer> constructor = null;
 		try {
 			constructor = clazz.getDeclaredConstructor( constructorArgs );
-			if ( ! ReflectHelper.isPublic( constructor ) ) {
-				try {
-					// found a constructor, but it was not publicly accessible so try to request accessibility
-					constructor.setAccessible( true );
-				}
-				catch ( SecurityException e ) {
-					constructor = null;
-				}
+			try {
+				constructor.setAccessible( true );
+			}
+			catch ( SecurityException e ) {
+				constructor = null;
 			}
 		}
 		catch ( NoSuchMethodException ignore ) {
diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java
@@ -132,9 +132,7 @@ private Callback createListenerCallback(
 			if (argType != Object.class && argType != entityClass) {
 				continue;
 			}
-			if (!method.isAccessible()) {
-				method.setAccessible( true );
-			}
+			method.setAccessible( true );
 
 			return new ListenerCallback( listenerInstance, method );
 		}
@@ -151,7 +149,7 @@ private Callback createBeanCallback( Class<?> callbackClass,
 		for (Method method : callbackClass.getDeclaredMethods()) {
 			if (!method.getName().equals(methodName)) continue;
 			if (method.getParameterTypes().length != 0) continue;
-			if (!method.isAccessible()) method.setAccessible(true);
+			method.setAccessible(true);
 			return new EntityCallback(method);
 		}
 		return null;
diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java
@@ -104,7 +104,7 @@ public Callback[] resolveCallbacks(XClass beanClass, Class annotation, Reflectio
 												.getName() + " - " + xMethod
 								);
 							}
-							if (!method.isAccessible()) method.setAccessible(true);
+							method.setAccessible(true);
 							log.debugf("Adding %s as %s callback for entity %s",
 									   methodName,
 									   annotation.getSimpleName(),
diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java
@@ -428,10 +428,8 @@ private <X> void registerAttribute(Class metamodelClass, Attribute<X, ?> attribu
 					? metamodelClass.getField( name )
 					: metamodelClass.getDeclaredField( name );
 			try {
-				if ( ! field.isAccessible() ) {
-					// should be public anyway, but to be sure...
-					field.setAccessible( true );
-				}
+				// should be public anyway, but to be sure...
+				field.setAccessible( true );
 				field.set( null, attribute );
 			}
 			catch ( IllegalAccessException e ) {
diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java
@@ -83,13 +83,11 @@ private void addBeforeClassOnceCallback(Method method) {
 	}
 
 	private void ensureAccessibility(Method method) {
-		if ( !method.isAccessible() ) {
-			try {
-				method.setAccessible( true );
-			}
-			catch (Exception ignored) {
-				// ignore for now
-			}
+		try {
+			method.setAccessible( true );
+		}
+		catch (Exception ignored) {
+			// ignore for now
 		}
 	}
 
@@ -141,17 +139,15 @@ private void validateCallbackMethod(Method method, CallbackType type, List<Throw
 					)
 			);
 		}
-		if ( !method.isAccessible() ) {
-			try {
-				method.setAccessible( true );
-			}
-			catch (Exception e) {
-				errors.add(
-						new InvalidMethodForAnnotationException(
-								type.buildTypeMarker() + " attached to inaccessible method and unable to make accessible"
-						)
-				);
-			}
+		try {
+			method.setAccessible( true );
+		}
+		catch (Exception e) {
+			errors.add(
+					new InvalidMethodForAnnotationException(
+							type.buildTypeMarker() + " attached to inaccessible method and unable to make accessible"
+					)
+			);
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -303,9 +303,7 @@ public static <T> Constructor<T> getDefaultConstructor(Class<T> clazz) throws Pr`
`303`	`303`
`304`	`304`	`try {`
`305`	`305`	`Constructor<T> constructor = clazz.getDeclaredConstructor( NO_PARAM_SIGNATURE );`
`306`		`- if ( !isPublic( clazz, constructor ) ) {`
`307`		`- constructor.setAccessible( true );`
`308`		`- }`
	`306`	`+ constructor.setAccessible( true );`
`309`	`307`	`return constructor;`
`310`	`308`	`}`
`311`	`309`	`catch ( NoSuchMethodException nme ) {`
`@@ -363,9 +361,7 @@ public static Constructor getConstructor(Class clazz, Type[] types) throws Prope`
`363`	`361`	`}`
`364`	`362`	`}`
`365`	`363`	`if ( found ) {`
`366`		`- if ( !isPublic( clazz, constructor ) ) {`
`367`		`- constructor.setAccessible( true );`
`368`		`- }`
	`364`	`+ constructor.setAccessible( true );`
`369`	`365`	`return constructor;`
`370`	`366`	`}`
`371`	`367`	`}`
Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ private static Field getField(Class clazz, String name) throws PropertyNotFoundE`
`157`	`157`	`catch (NoSuchFieldException nsfe) {`
`158`	`158`	`field = getField( clazz, clazz.getSuperclass(), name );`
`159`	`159`	`}`
`160`		`- if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true);`
	`160`	`+ field.setAccessible(true);`
`161`	`161`	`return field;`
`162`	`162`	`}`
`163`	`163`
`@@ -172,7 +172,7 @@ private static Field getField(Class root, Class clazz, String name) throws Prope`
`172`	`172`	`catch (NoSuchFieldException nsfe) {`
`173`	`173`	`field = getField( root, clazz.getSuperclass(), name );`
`174`	`174`	`}`
`175`		`- if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true);`
	`175`	`+ field.setAccessible(true);`
`176`	`176`	`return field;`
`177`	`177`	`}`
`178`	`178`
Original file line number	Diff line number	Diff line change
`@@ -192,9 +192,7 @@ public Object invoke(`
`192`	`192`	`returnValue = thisMethod.invoke( target, args );`
`193`	`193`	`}`
`194`	`194`	`else {`
`195`		`- if ( !thisMethod.isAccessible() ) {`
`196`		`- thisMethod.setAccessible( true );`
`197`		`- }`
	`195`	`+ thisMethod.setAccessible( true );`
`198`	`196`	`returnValue = thisMethod.invoke( target, args );`
`199`	`197`	`}`
`200`	`198`
Original file line number	Diff line number	Diff line change
`@@ -132,9 +132,7 @@ private Callback createListenerCallback(`
`132`	`132`	`if (argType != Object.class && argType != entityClass) {`
`133`	`133`	`continue;`
`134`	`134`	`}`
`135`		`- if (!method.isAccessible()) {`
`136`		`- method.setAccessible( true );`
`137`		`- }`
	`135`	`+ method.setAccessible( true );`
`138`	`136`
`139`	`137`	`return new ListenerCallback( listenerInstance, method );`
`140`	`138`	`}`
`@@ -151,7 +149,7 @@ private Callback createBeanCallback( Class<?> callbackClass,`
`151`	`149`	`for (Method method : callbackClass.getDeclaredMethods()) {`
`152`	`150`	`if (!method.getName().equals(methodName)) continue;`
`153`	`151`	`if (method.getParameterTypes().length != 0) continue;`
`154`		`- if (!method.isAccessible()) method.setAccessible(true);`
	`152`	`+ method.setAccessible(true);`
`155`	`153`	`return new EntityCallback(method);`
`156`	`154`	`}`
`157`	`155`	`return null;`
Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ public Callback[] resolveCallbacks(XClass beanClass, Class annotation, Reflectio`
`104`	`104`	`.getName() + " - " + xMethod`
`105`	`105`	`);`
`106`	`106`	`}`
`107`		`- if (!method.isAccessible()) method.setAccessible(true);`
	`107`	`+ method.setAccessible(true);`
`108`	`108`	`log.debugf("Adding %s as %s callback for entity %s",`
`109`	`109`	`methodName,`
`110`	`110`	`annotation.getSimpleName(),`