HHH-12786 Improve the basic proxy interceptor

Apart from cosmetic changes, we were testing in the equals() method that the
instance == the proxied object which will always be true.

We should use the argument of the equals() method instead to do the
comparison.

And we can do the comparison on the instance, instead of requiring
passing the proxiedObject into the interceptor.
This commit is contained in:
Guillaume Smet 2018-07-12 18:51:06 +02:00
parent 67698b8bdb
commit 1688c3ff8d
3 changed files with 122 additions and 27 deletions

View File

@ -26,6 +26,7 @@ public class BasicProxyFactoryImpl implements BasicProxyFactory {
private static final String PROXY_NAMING_SUFFIX = Environment.useLegacyProxyClassnames() ? "HibernateBasicProxy$" : "HibernateBasicProxy"; private static final String PROXY_NAMING_SUFFIX = Environment.useLegacyProxyClassnames() ? "HibernateBasicProxy$" : "HibernateBasicProxy";
private final Class proxyClass; private final Class proxyClass;
private final ProxyConfiguration.Interceptor interceptor;
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public BasicProxyFactoryImpl(Class superClass, Class[] interfaces, ByteBuddyState bytebuddy) { public BasicProxyFactoryImpl(Class superClass, Class[] interfaces, ByteBuddyState bytebuddy) {
@ -47,12 +48,13 @@ public class BasicProxyFactoryImpl implements BasicProxyFactory {
.make() .make()
.load( superClassOrMainInterface.getClassLoader(), ByteBuddyState.resolveClassLoadingStrategy( superClassOrMainInterface ) ) .load( superClassOrMainInterface.getClassLoader(), ByteBuddyState.resolveClassLoadingStrategy( superClassOrMainInterface ) )
.getLoaded(); .getLoaded();
this.interceptor = new PassThroughInterceptor( proxyClass.getName() );
} }
public Object getProxy() { public Object getProxy() {
try { try {
final ProxyConfiguration proxy = (ProxyConfiguration) proxyClass.newInstance(); final ProxyConfiguration proxy = (ProxyConfiguration) proxyClass.newInstance();
proxy.$$_hibernate_set_interceptor( new PassThroughInterceptor( proxy, proxyClass.getName() ) ); proxy.$$_hibernate_set_interceptor( this.interceptor );
return proxy; return proxy;
} }
catch (Throwable t) { catch (Throwable t) {

View File

@ -11,57 +11,58 @@ import java.util.HashMap;
import org.hibernate.proxy.ProxyConfiguration; import org.hibernate.proxy.ProxyConfiguration;
import net.bytebuddy.implementation.bind.annotation.AllArguments;
import net.bytebuddy.implementation.bind.annotation.Origin;
import net.bytebuddy.implementation.bind.annotation.RuntimeType;
import net.bytebuddy.implementation.bind.annotation.This;
public class PassThroughInterceptor implements ProxyConfiguration.Interceptor { public class PassThroughInterceptor implements ProxyConfiguration.Interceptor {
private HashMap data = new HashMap(); private HashMap<Object, Object> data = new HashMap<>();
private final Object proxiedObject;
private final String proxiedClassName; private final String proxiedClassName;
public PassThroughInterceptor(Object proxiedObject, String proxiedClassName) { public PassThroughInterceptor(String proxiedClassName) {
this.proxiedObject = proxiedObject;
this.proxiedClassName = proxiedClassName; this.proxiedClassName = proxiedClassName;
} }
@SuppressWarnings("unchecked")
@Override @Override
public Object intercept(Object instance, Method method, Object[] arguments) throws Exception { public Object intercept(Object instance, Method method, Object[] arguments) throws Exception {
final String name = method.getName(); final String name = method.getName();
if ( "toString".equals( name ) ) {
if ( "toString".equals( name ) && arguments.length == 0 ) {
return proxiedClassName + "@" + System.identityHashCode( instance ); return proxiedClassName + "@" + System.identityHashCode( instance );
} }
else if ( "equals".equals( name ) ) {
return proxiedObject == instance; if ( "equals".equals( name ) && arguments.length == 1 ) {
return instance == arguments[0];
} }
else if ( "hashCode".equals( name ) ) {
if ( "hashCode".equals( name ) && arguments.length == 0 ) {
return System.identityHashCode( instance ); return System.identityHashCode( instance );
} }
final boolean hasGetterSignature = method.getParameterCount() == 0 if ( name.startsWith( "get" ) && hasGetterSignature( method ) ) {
&& method.getReturnType() != null;
final boolean hasSetterSignature = method.getParameterCount() == 1
&& ( method.getReturnType() == null || method.getReturnType() == void.class );
if ( name.startsWith( "get" ) && hasGetterSignature ) {
final String propName = name.substring( 3 ); final String propName = name.substring( 3 );
return data.get( propName ); return data.get( propName );
} }
else if ( name.startsWith( "is" ) && hasGetterSignature ) {
if ( name.startsWith( "is" ) && hasGetterSignature( method ) ) {
final String propName = name.substring( 2 ); final String propName = name.substring( 2 );
return data.get( propName ); return data.get( propName );
} }
else if ( name.startsWith( "set" ) && hasSetterSignature ) {
if ( name.startsWith( "set" ) && hasSetterSignature( method ) ) {
final String propName = name.substring( 3 ); final String propName = name.substring( 3 );
data.put( propName, arguments[0] ); data.put( propName, arguments[0] );
return null; return null;
} }
else {
// todo : what else to do here? // todo : what else to do here?
return null; return null;
} }
private boolean hasGetterSignature(Method method) {
return method.getParameterCount() == 0
&& method.getReturnType() != null;
}
private boolean hasSetterSignature(Method method) {
return method.getParameterCount() == 1
&& ( method.getReturnType() == null || method.getReturnType() == void.class );
} }
} }

View File

@ -0,0 +1,92 @@
/*
* 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.test.proxy.bytebuddy;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import org.hibernate.bytecode.internal.bytebuddy.BasicProxyFactoryImpl;
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
import org.hibernate.testing.TestForIssue;
import org.junit.Test;
@TestForIssue(jiraKey = "HHH-12786")
public class ByteBuddyBasicProxyFactoryTest {
private static final BasicProxyFactoryImpl BASIC_PROXY_FACTORY = new BasicProxyFactoryImpl( Entity.class, new Class[0], new ByteBuddyState() );
@Test
public void testEqualsHashCode() {
Object entityProxy = BASIC_PROXY_FACTORY.getProxy();
assertTrue( entityProxy.equals( entityProxy ) );
assertNotNull( entityProxy.hashCode() );
Object otherEntityProxy = BASIC_PROXY_FACTORY.getProxy();
assertFalse( entityProxy.equals( otherEntityProxy ) );
}
@Test
public void testToString() {
Object entityProxy = BASIC_PROXY_FACTORY.getProxy();
assertTrue( entityProxy.toString().contains( "HibernateBasicProxy" ) );
}
@Test
public void testGetterSetter() {
Entity entityProxy = (Entity) BASIC_PROXY_FACTORY.getProxy();
entityProxy.setBool( true );
assertTrue( entityProxy.isBool() );
entityProxy.setBool( false );
assertFalse( entityProxy.isBool() );
entityProxy.setString( "John Irving" );
assertEquals( "John Irving", entityProxy.getString() );
}
@Test
public void testNonGetterSetterMethod() {
Entity entityProxy = (Entity) BASIC_PROXY_FACTORY.getProxy();
assertNull( entityProxy.otherMethod() );
}
public static class Entity {
private String string;
private boolean bool;
public Entity() {
}
public boolean isBool() {
return bool;
}
public void setBool(boolean bool) {
this.bool = bool;
}
public String getString() {
return string;
}
public void setString(String string) {
this.string = string;
}
public String otherMethod() {
return "a string";
}
}
}