Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic properties are not supported in version 4.0.0.RC1 #354

Closed
brunneng opened this issue Apr 29, 2019 · 5 comments
Closed

Generic properties are not supported in version 4.0.0.RC1 #354

brunneng opened this issue Apr 29, 2019 · 5 comments

Comments

@brunneng
Copy link

brunneng commented Apr 29, 2019

Code to reproduce an issue:

public class EasyRandomTest {

   public static void main(String[] args) {
      EasyRandom easyRandom = new EasyRandom();
      System.out.println(easyRandom.nextObject(B.class).id);
   }

   class B extends A<Long> {

   }

   class A<T extends Serializable> {
      T id;
   }
}

Expection is:
Caused by: java.lang.InstantiationError: java.io.Serializable
at sun.reflect.GeneratedSerializationConstructorAccessor2.newInstance(Unknown Source)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at org.objenesis.instantiator.sun.SunReflectionFactoryInstantiator.newInstance(SunReflectionFactoryInstantiator.java:48)
at org.objenesis.ObjenesisBase.newInstance(ObjenesisBase.java:73)
at org.jeasy.random.ObjenesisObjectFactory.createNewInstance(ObjenesisObjectFactory.java:73)
at org.jeasy.random.ObjenesisObjectFactory.createInstance(ObjenesisObjectFactory.java:58)
... 8 more

Expected behaviour: random id of type Long should be generated.

@fmbenhassine
Copy link
Member

@brunneng Thank you for reporting this issue. I believe it is a duplicate of #313 . Due to type erasure, some generic infos about types/fields cannot be retrieved at runtime. This is a known limitation of the library.

You can always register a custom randomizer for the generic field/type.

I'm closing this issue for now but please do not hesitate to add your questions/comments here.

@brunneng
Copy link
Author

brunneng commented May 4, 2019

Hi. Can't agree with you because it's possible to retrieve generic information of fields in runtime.
Please check this example:

package com.greentea.ma.utils.randomizer;

import java.io.Serializable;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.*;

public class GetGenericFieldTest {

   static List<Method> getAllDeclaredMethods(Class clazz) {
      List<Method> res = new ArrayList<>();
      Class currentClass = clazz;
      while (currentClass != null && currentClass != Object.class) {
         res.addAll(Arrays.asList(currentClass.getDeclaredMethods()));
         currentClass = currentClass.getSuperclass();
      }
      return res;
   }

   /**
    * Calculates mapping of type variables to actual classes. For example, if we have
    * class A<T> and class B extends A<Integer> then getActualTypeVariablesTypes(B.class) will return
    * map with entry: T -> Integer.
    * @param clazz class to search type variable mapping for it
    * @return map of type variables to actual classes
    */
   static Map<TypeVariable, Type> getActualTypeVariablesTypes(Class clazz) {
      Map<TypeVariable, Type> res = new HashMap<>();

      Class currentClass = clazz;
      while (currentClass != null) {
         Type genericSuperclass = currentClass.getGenericSuperclass();
         if (genericSuperclass instanceof ParameterizedType) {
            final ParameterizedType pt = (ParameterizedType) genericSuperclass;
            Type rawType = pt.getRawType();
            if (rawType instanceof Class) {
               int i = 0;
               for (TypeVariable tv : ((Class) rawType).getTypeParameters()) {
                  res.put(tv, pt.getActualTypeArguments()[i]);
               }
            }
         }

         currentClass = currentClass.getSuperclass();
      }

      return res;
   }

   public static void main(String[] args) throws NoSuchMethodException {
      Method setter = getAllDeclaredMethods(B.class).stream().filter(m -> m.getName().equals("setId")).findFirst().get();
      Type setterTypeVariable = setter.getGenericParameterTypes()[0];

      System.out.println(getActualTypeVariablesTypes(B.class).get(setterTypeVariable));
   }

   static class B extends A<Long> {

   }

   static class A<T extends Serializable> {
      T id;

      public T getId() {
         return id;
      }

      public void setId(T id) {
         this.id = id;
      }
   }
}

It prints:
class java.lang.Long

@fmbenhassine
Copy link
Member

Thank you for the example. I deliberately used the world some in "some generic infos .. cannot be retrieved at runtime" because some other info can be indeed retrieved as you showed. I'm aware of this kind of tricks and some of them are already used in the library (to populate generic collections for example). However, the method getActualTypeVariablesTypes (which, as mentioned in its javadoc, calculates the generic type) is exactly the kind of complexity I'm not ready to introduce in the library and maintain in the long term. I would wait for a clean way from the Java Reflection API to get this generic information instead of calculating it (probably with reified generics?).

I invite you to read the thread #242 where we addressed the same use case but for enums. I came up with a similar code that you suggested (See this code mentioned in the comment here), but I said in the aforementioned comment, I gave up with this kind of code because of its complexity even though it does the trick.

Proving that we can get the generic info of a field or type with a simple use case involving 2 classes A and B could look simple (even though getActualTypeVariablesTypes is already not that simple..). However, implementing the feature in a generic way is another story (think of types like class Foo<K, V extends Map<K, List<V>>> { .. } or fields like private Map<? extends Number, List<String>> myMap; which can be everywhere in the object graph). It is always a trade-off between introducing more features and controlling the complexity of the framework/library, and since I want to keep things easy, I acknowledge this limitation in the library and I suggest to use a custom randomizer for your generic type.

@brunneng
Copy link
Author

brunneng commented May 4, 2019

Ok. got it, thank you for the detailed explanation. Understand that it can be complex to cover all possible cases, but as for me better to cover at least most widespread cases like this one in issue and at least detection of generic types of collections and maps. Agree that in java reflection API this all looks like some trick, but me, as a user, don't care much about which tricks are used inside. If the library is just working as expected - then I will use it. If not, if it fails on simple cases and forces me to search workarounds - then it's not a user-friendly library, and I better to find something better. Also not sure I understand what is the complexity of "maintain in the long term" if everything is covered by unit tests.

@fmbenhassine
Copy link
Member

fmbenhassine commented May 5, 2019

I would agree with you if this was not documented. But it is not the case, we clearly document that this is a known issue and that generic types are not supported.

If the library is just working as expected - then I will use it. If not, if it fails on simple cases and forces me to search workarounds - then it's not a user-friendly library, and I better to find something better

If you look at Java's know issues, you can see for example:

"Currently it is not possible to programmatically invoke default interface methods using the JDI API."

If you programmatically invoke default interface methods using the JDI API and it would not work as you expect, does this make Java not user friendly? I don't think so.

So to back up, I'm not against adding this feature in Easy Random. However, I'm against introducing hacky code to implement it. The day the Java reflection API allows to get the generic field type at runtime in a clean way, I would consider adding this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants