sam-tsai / activejdbc

Automatically exported from code.google.com/p/activejdbc
0 stars 0 forks source link

Eliminate casting for cases when a static method on model returns a single instance of that model type #156

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This is annoying:
Person p = (Person)Person.findById(1);

needs to be:
Person p = Person.findById(1);

Affected methods:
findById()
findFirst()
create()
createIt()

Original issue reported on code.google.com by ipolevoy@gmail.com on 25 Apr 2012 at 6:27

GoogleCodeExporter commented 9 years ago

Original comment by ipolevoy@gmail.com on 25 Apr 2012 at 6:28

GoogleCodeExporter commented 9 years ago
deferred till better times, Java driving me insane

Original comment by ipolevoy@gmail.com on 25 Apr 2012 at 11:02

GoogleCodeExporter commented 9 years ago
Why is this "won't fix"?  The problem is with the delegator is created in 
ModelInstrumentation#addDelegates.  It's using javassists method:
  CtNewMethod.delegator(method, target)

The problem is with the generic methods like:
    public static <T extends Model> T findById(Object id);

The created CtNewMethod is based on the type erased method signature.  At the 
byte code:
  // Method descriptor #29 (Ljava/lang/Object;)Lorg/javalite/activejdbc/Model;
  // Stack: 2, Locals: 2
  public static org.javalite.activejdbc.Model findById(java.lang.Object arg0);
    0  aload_0 [arg0]
    1  invokestatic org.javalite.activejdbc.Model.findById(java.lang.Object) : org.javalite.activejdbc.Model [308]
    4  areturn

This is not correctly carrying over the meta generic signature.  For more info 
see:
  http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.3

I've attached a patch that copies over the SignatureAttribute.  This correctly 
generates:
  // Method descriptor #29 (Ljava/lang/Object;)Lorg/javalite/activejdbc/Model;
  // Signature: <T:Lorg/javalite/activejdbc/Model;>(Ljava/lang/Object;)TT;
  // Stack: 2, Locals: 2
  public static org.javalite.activejdbc.Model findById(java.lang.Object arg0);
    0  aload_0 [arg0]
    1  invokestatic org.javalite.activejdbc.Model.findById(java.lang.Object) : org.javalite.activejdbc.Model [320]
    4  areturn

NOTE: Make sure to do a *clean* build.  Trying to instrument the code that was 
already instrumented the old way will create invalid byte code.

Original comment by marc.har...@gmail.com on 12 May 2012 at 4:09

Attachments:

GoogleCodeExporter commented 9 years ago
Marc, you are a hero if this is working, I will give it a spin this weekend, 
and if all checks out, will bake it into a new release!

thanks,
Igor

Original comment by ipolevoy@gmail.com on 12 May 2012 at 4:25

GoogleCodeExporter commented 9 years ago

Original comment by ipolevoy@gmail.com on 12 May 2012 at 4:25

GoogleCodeExporter commented 9 years ago
I should have added - locally I'm using javassist-3.12.1.GA.jar.  I haven't 
tried other versions of javassist.  

Original comment by marc.har...@gmail.com on 12 May 2012 at 6:46

GoogleCodeExporter commented 9 years ago
this is fixed, deployed as a new snapshot. 
Kudos to Marc for the patch!

Original comment by ipolevoy@gmail.com on 13 May 2012 at 8:32