ISPN-7906 Infinispan Query DSL does not handle inheritance of propert…

…ies/fields correctly

object-filter/src/main/java/org/infinispan/objectfilter/impl/util/ReflectionHelper.java

@@ -63,6 +63,7 @@ private static class FieldPropertyAccessor extends BasePropertyAccessor {
 
       FieldPropertyAccessor(Field field) {
          this.field = field;
+         field.setAccessible(true);
       }
 
       @Override
@@ -87,7 +88,7 @@ public Iterator<Object> getValueIterator(Object instance) {
       }
    }
 
-   private static class ArrayFieldPropertyAccessor extends FieldPropertyAccessor {
+   private static final class ArrayFieldPropertyAccessor extends FieldPropertyAccessor {
 
       ArrayFieldPropertyAccessor(Field field) {
          super(field);
@@ -108,7 +109,7 @@ public Class<?> getPropertyType() {
       }
    }
 
-   private static class CollectionFieldPropertyAccessor extends FieldPropertyAccessor {
+   private static final class CollectionFieldPropertyAccessor extends FieldPropertyAccessor {
 
       CollectionFieldPropertyAccessor(Field field) {
          super(field);
@@ -129,7 +130,7 @@ public Class<?> getPropertyType() {
       }
    }
 
-   private static class MapFieldPropertyAccessor extends FieldPropertyAccessor {
+   private static final class MapFieldPropertyAccessor extends FieldPropertyAccessor {
 
       MapFieldPropertyAccessor(Field field) {
          super(field);
@@ -156,6 +157,7 @@ private static class MethodPropertyAccessor extends BasePropertyAccessor {
 
       MethodPropertyAccessor(Method method) {
          this.method = method;
+         method.setAccessible(true);
       }
 
       @Override
@@ -180,7 +182,7 @@ public Iterator<Object> getValueIterator(Object instance) {
       }
    }
 
-   private static class ArrayMethodPropertyAccessor extends MethodPropertyAccessor {
+   private static final class ArrayMethodPropertyAccessor extends MethodPropertyAccessor {
 
       ArrayMethodPropertyAccessor(Method method) {
          super(method);
@@ -201,7 +203,7 @@ public Class<?> getPropertyType() {
       }
    }
 
-   private static class CollectionMethodPropertyAccessor extends MethodPropertyAccessor {
+   private static final class CollectionMethodPropertyAccessor extends MethodPropertyAccessor {
 
       CollectionMethodPropertyAccessor(Method method) {
          super(method);
@@ -222,7 +224,7 @@ public Class<?> getPropertyType() {
       }
    }
 
-   private static class MapMethodPropertyAccessor extends MethodPropertyAccessor {
+   private static final class MapMethodPropertyAccessor extends MethodPropertyAccessor {
 
       MapMethodPropertyAccessor(Method method) {
          super(method);
@@ -250,13 +252,26 @@ public static PropertyAccessor getAccessor(Class<?> clazz, String propertyName)
       if (propertyName == null || propertyName.length() == 0) {
          throw new IllegalArgumentException("Property name cannot be null or empty");
       }
-      if (propertyName.contains(".")) {
+      if (propertyName.indexOf('.') != -1) {
          throw new IllegalArgumentException("The argument cannot be a nested property name");
       }
+      String propertyNameSuffix = Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1);
+
+      Class<?> c = clazz;
+      while (c != null) {
+         PropertyAccessor m = getAccessor(c, propertyName, propertyNameSuffix);
+         if (m != null) {
+            return m;
+         }
+         c = c.getSuperclass();
+      }
+
+      throw new IntrospectionException("Property not found: " + propertyName);
+   }
 
+   private static PropertyAccessor getAccessor(Class<?> clazz, String propertyName, String propertyNameSuffix) {
       // try getter method access
       // we need to find a no-arg public "getXyz" or "isXyz" method which has a suitable return type
-      String propertyNameSuffix = Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1);
       try {
          Method m = clazz.getDeclaredMethod("get" + propertyNameSuffix);
          if (Modifier.isPublic(m.getModifiers()) && !m.getReturnType().equals(Void.class)) {
@@ -283,7 +298,7 @@ public static PropertyAccessor getAccessor(Class<?> clazz, String propertyName)
          // ignored, continue
       }
 
-      throw new IntrospectionException("Property not found: " + propertyName);
+      return null;
    }
 
    private static PropertyAccessor getFieldAccessor(Field f) {

query/src/test/java/org/infinispan/query/dsl/embedded/NonIndexedSingleClassDSLQueryTest.java

@@ -0,0 +1,20 @@
+package org.infinispan.query.dsl.embedded;
+
+import org.infinispan.configuration.cache.ConfigurationBuilder;
+import org.testng.annotations.Test;
+
+/**
+ * Test entities defined as inner classes and inheritance of fields using non-indexed query. Just a simple field equals
+ * is tested. The purpose of this test is just to check class and property lookup correctness.
+ *
+ * @author anistor@redhat.com
+ * @since 9.1
+ */
+@Test(groups = "functional", testName = "query.dsl.embedded.NonIndexedSingleClassDSLQueryTest")
+public class NonIndexedSingleClassDSLQueryTest extends SingleClassDSLQueryTest {
+
+   @Override
+   protected void configureCache(ConfigurationBuilder builder) {
+      // do nothing
+   }
+}

query/src/test/java/org/infinispan/query/dsl/embedded/SingleClassDSLQueryTest.java

@@ -7,8 +7,6 @@
 import org.hibernate.search.annotations.Analyze;
 import org.hibernate.search.annotations.Field;
 import org.hibernate.search.annotations.Indexed;
-import org.hibernate.search.annotations.Store;
-import org.infinispan.Cache;
 import org.infinispan.configuration.cache.ConfigurationBuilder;
 import org.infinispan.configuration.cache.Index;
 import org.infinispan.manager.EmbeddedCacheManager;
@@ -17,11 +15,16 @@
 import org.infinispan.query.dsl.QueryFactory;
 import org.infinispan.test.SingleCacheManagerTest;
 import org.infinispan.test.fwk.TestCacheManagerFactory;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 /**
+ * Test entities defined as inner classes and inheritance of fields using indexed query. Just a simple field equals is
+ * tested. The purpose of this test is just to check class and property lookup correctness.
+ *
  * @author gustavonalle
  * @author Tristan Tarrant
+ * @author anistor@redhat.com
  * @since 8.0
  */
 @Test(groups = "functional", testName = "query.dsl.embedded.SingleClassDSLQueryTest")
@@ -30,37 +33,155 @@ public class SingleClassDSLQueryTest extends SingleCacheManagerTest {
    @Override
    protected EmbeddedCacheManager createCacheManager() throws Exception {
       ConfigurationBuilder builder = new ConfigurationBuilder();
-      builder.indexing().index(Index.ALL)
-              .addIndexedEntity(Person.class)
-              .addProperty("default.directory_provider", "ram")
-              .addProperty("lucene_version", "LUCENE_CURRENT");
+      configureCache(builder);
       return TestCacheManagerFactory.createCacheManager(builder);
    }
 
+   protected void configureCache(ConfigurationBuilder builder) {
+      builder.indexing().index(Index.ALL)
+            .addIndexedEntity(Person.class)
+            .addProperty("default.directory_provider", "ram")
+            .addProperty("lucene_version", "LUCENE_CURRENT");
+   }
+
+   @BeforeClass(alwaysRun = true)
+   protected void populateCache() throws Exception {
+      cache.put("person1", new Person("William", "Shakespeare", 50, "ZZ3141592", "M"));
+   }
+
+   @Override
+   protected void clearContent() {
+      // Don't clear, this is destroying the index
+   }
+
    /**
     * Test querying for entities defined as inner classes.
     */
-   public void testQuery() throws Exception {
-      Cache<String, Person> cache = cacheManager.getCache();
-      cache.put("person1", new Person("William", "Shakespeare"));
+   public void testQueryInnerClass() throws Exception {
       QueryFactory queryFactory = Search.getQueryFactory(cache);
-      Query query = queryFactory.from(Person.class).having("name").eq("William").build();
+      Query query = queryFactory.from(Person.class).build();
+
       List<Person> matches = query.list();
       assertEquals(1, matches.size());
    }
 
-   @Indexed
-   static class Person {
-      @Field(store = Store.YES, analyze = Analyze.NO)
+   /**
+    * Test querying for a field - direct access to field.
+    */
+   public void testField() throws Exception {
+      QueryFactory queryFactory = Search.getQueryFactory(cache);
+      Query query = queryFactory.from(Person.class)
+            .having("driverLicenseId").eq("ZZ3141592")
+            .build();
+
+      List<Person> matches = query.list();
+      assertEquals(1, matches.size());
+   }
+
+   /**
+    * Test querying for an inherited indexed field - direct inherited field access.
+    */
+   public void testInheritedField() throws Exception {
+      QueryFactory queryFactory = Search.getQueryFactory(cache);
+      Query query = queryFactory.from(Person.class)
+            .having("age").lte(52)
+            .build();
+
+      List<Person> matches = query.list();
+      assertEquals(1, matches.size());
+   }
+
+   /**
+    * Test querying for an inherited indexed field - interface method with inherited implementation.
+    */
+   public void testInheritedField2() throws Exception {
+      QueryFactory queryFactory = Search.getQueryFactory(cache);
+      Query query = queryFactory.from(Person.class)
+            .having("name").eq("William")
+            .build();
+
+      List<Person> matches = query.list();
+      assertEquals(1, matches.size());
+   }
+
+   /**
+    * Test querying for an inherited indexed field - interface method implemented in class.
+    */
+   public void testInheritedField3() throws Exception {
+      QueryFactory queryFactory = Search.getQueryFactory(cache);
+      Query query = queryFactory.from(Person.class)
+            .having("gender").eq("M")
+            .build();
+
+      List<Person> matches = query.list();
+      assertEquals(1, matches.size());
+   }
+
+   /**
+    * Test querying for an inherited indexed field - method inherited from superclass.
+    */
+   public void testInheritedField4() throws Exception {
+      QueryFactory queryFactory = Search.getQueryFactory(cache);
+      Query query = queryFactory.from(Person.class)
+            .having("surname").eq("Shakespeare")
+            .build();
+
+      List<Person> matches = query.list();
+      assertEquals(1, matches.size());
+   }
+
+   interface PersonInterface {
+
+      String getName();
+
+      String getGender();
+   }
+
+   static abstract class PersonBase implements PersonInterface {
+
       String name;
 
-      @Field(store = Store.YES, analyze = Analyze.NO, indexNullAs = Field.DEFAULT_NULL_TOKEN)
       String surname;
 
-      public Person(String name, String surname) {
+      @Field(analyze = Analyze.NO)
+      int age;
+
+      PersonBase(String name, String surname, int age) {
          this.name = name;
          this.surname = surname;
+         this.age = age;
       }
 
+      @Field(analyze = Analyze.NO, indexNullAs = Field.DEFAULT_NULL_TOKEN)
+      public String getSurname() {
+         return surname;
+      }
+
+      @Field(analyze = Analyze.NO)
+      @Override
+      public String getName() {
+         return name;
+      }
+   }
+
+   @Indexed
+   static class Person extends PersonBase {
+
+      @Field(analyze = Analyze.NO)
+      String driverLicenseId;
+
+      String gender;
+
+      public Person(String name, String surname, int age, String driverLicenseId, String gender) {
+         super(name, surname, age);
+         this.driverLicenseId = driverLicenseId;
+         this.gender = gender;
+      }
+
+      @Field(analyze = Analyze.NO)
+      @Override
+      public String getGender() {
+         return gender;
+      }
    }
 }