[jira] [Updated] (THRIFT-2556) TUnion copy constructor throws in some unexpected cases

["Jeremiah Menétrey (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/THRIFT-2556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeremiah Menétrey updated THRIFT-2556:
--------------------------------------

    Description: 
The copy constructor of {{TUnion}}
{code}protected TUnion(TUnion<T, F> other) { ... {code}
throws when the instantiated class and the copied one are not exactly the same.

This prevents using the copy constructor pattern in some cases, in particular when used for wrapping thrift struct objects. E.g.:

Let's say the following {{union}} class is generated:
{code}public class MyUnion extends TUnion<MyUnion, MyUnion._Fields> { ... {code}

Then the following wrapper class (that redefines some behavior) will compile but throw upon instantiation:
{code}
public class MyWrapper extends MyUnion {
  public MyWrapper(MyUnion wrapped) {
    super(wrapper); // Throws a ClassCastException
  }

  @Override
  public String toString() {
    // Generates a better String representation for the use-case
    return "foobar";
  }
}
{code}

Also, the idiomatic constructor of the copy constructor pattern:
{code}public MyWrapper(MyWrapper other) { ...{code}
won't work if the provided instance is a subclass of {{MyWrapper}}.

I don't think that the goal was to prevent subclassing, as generated {{TUnion}} subclasses are not {{final}}.

Maybe using {{Class.isInstance(Object)}} or {{Class.isAssignableFrom(Class)}} in the copy constructor of {{TUnion}} instead of checking for strict equality would be a solution, but I am not sure of all the implications of doing so for the rest of the framework.

  was:
The copy constructor of {{TUnion}}
{code}protected TUnion(TUnion<T, F> other) { ... {code}
throws when the instantiated class and the copied one are not exactly the same.

This prevents using the copy constructor pattern in some cases, in particular when used for wrapping thrift struct objects. E.g.:

Let's say the following {{union}} class is generated:
{code}public class MyUnion extends TUnion<MyUnion, MyUnion._Fields> { ... {code}

Then the following wrapper class (that redefines some behavior) will compile but throw upon instantiation:
{code}
public class MyWrapper extends MyUnion {
  public MyWrapper(MyUnion wrapper) {
    super(wrapper); // Throws a ClassCastException
  }

  @Override
  public String toString() {
    // Generates a better String representation for the use-case
    return "foobar";
  }
}
{code}

Also, the idiomatic constructor of the copy constructor pattern:
{code}public MyWrapper(MyWrapper other) { ...{code}
won't work if the provided instance is a subclass of {{MyWrapper}}.

I don't think that the goal was to prevent subclassing, as generated {{TUnion}} subclasses are not {{final}}.

Maybe using {{Class.isInstance(Object)}} or {{Class.isAssignableFrom(Class)}} in the copy constructor of {{TUnion}} instead of checking for strict equality would be a solution, but I am not sure of all the implications of doing so for the rest of the framework.


> TUnion copy constructor throws in some unexpected cases
> -------------------------------------------------------
>
>                 Key: THRIFT-2556
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2556
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Library
>    Affects Versions: 0.7
>         Environment: Java 1.7
>            Reporter: Jeremiah Menétrey
>
> The copy constructor of {{TUnion}}
> {code}protected TUnion(TUnion<T, F> other) { ... {code}
> throws when the instantiated class and the copied one are not exactly the same.
> This prevents using the copy constructor pattern in some cases, in particular when used for wrapping thrift struct objects. E.g.:
> Let's say the following {{union}} class is generated:
> {code}public class MyUnion extends TUnion<MyUnion, MyUnion._Fields> { ... {code}
> Then the following wrapper class (that redefines some behavior) will compile but throw upon instantiation:
> {code}
> public class MyWrapper extends MyUnion {
>   public MyWrapper(MyUnion wrapped) {
>     super(wrapper); // Throws a ClassCastException
>   }
>   @Override
>   public String toString() {
>     // Generates a better String representation for the use-case
>     return "foobar";
>   }
> }
> {code}
> Also, the idiomatic constructor of the copy constructor pattern:
> {code}public MyWrapper(MyWrapper other) { ...{code}
> won't work if the provided instance is a subclass of {{MyWrapper}}.
> I don't think that the goal was to prevent subclassing, as generated {{TUnion}} subclasses are not {{final}}.
> Maybe using {{Class.isInstance(Object)}} or {{Class.isAssignableFrom(Class)}} in the copy constructor of {{TUnion}} instead of checking for strict equality would be a solution, but I am not sure of all the implications of doing so for the rest of the framework.



--
This message was sent by Atlassian JIRA
(v6.2#6252)