Improve cacheability of filters

gemma-core/src/main/java/ubic/gemma/core/util/ListUtils.java

@@ -1,9 +1,6 @@
 package ubic.gemma.core.util;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.*;
 
 /**
  * Utilities and algorithms for {@link List}.
@@ -44,4 +41,21 @@ private static <T> void fillMap( Map<T, Integer> element2position, List<T> list
             }
         }
     }
+
+    /**
+     * Pad a list to the next power of 2 with the given element.
+     */
+    public static List<?> padToNextPowerOfTwo( List<?> list, Object elementForPadding ) {
+        int k = Integer.highestOneBit( list.size() );
+        if ( list.size() == k ) {
+            return list; // already a power of 2
+        }
+        k <<= 1;
+        List<Object> paddedList = new ArrayList<>( k );
+        paddedList.addAll( list );
+        for ( int j = list.size(); j < k; j++ ) {
+            paddedList.add( elementForPadding );
+        }
+        return paddedList;
+    }
 }

gemma-core/src/main/java/ubic/gemma/persistence/service/AbstractQueryFilteringVoEnabledDao.java

@@ -1,5 +1,6 @@
 package ubic.gemma.persistence.service;
 
+import org.apache.commons.collections4.IterableUtils;
 import org.apache.commons.lang3.NotImplementedException;
 import org.apache.commons.lang3.time.StopWatch;
 import org.hibernate.Query;
@@ -9,9 +10,7 @@
 import ubic.gemma.persistence.util.*;
 
 import javax.annotation.Nullable;
-import java.util.Collection;
-import java.util.List;
-import java.util.Objects;
+import java.util.*;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
@@ -166,7 +165,7 @@ public List<Long> loadIds( @Nullable Filters filters, @Nullable Sort sort ) {
 
     @Override
     public List<Long> loadIdsWithCache( @Nullable Filters filters, @Nullable Sort sort ) {
-        return doLoadIdsWithCache( filters, sort, true );
+        return doLoadIdsWithCache( getByIdsFiltersIfPossible( filters ), sort, true );
     }
 
     @Override
@@ -176,7 +175,7 @@ public List<O> load( @Nullable Filters filters, @Nullable Sort sort ) {
 
     @Override
     public Slice<O> loadWithCache( @Nullable Filters filters, @Nullable Sort sort, int offset, int limit ) {
-        return doLoadWithCache( filters, sort, offset, limit, true );
+        return doLoadWithCache( getByIdsFiltersIfPossible( filters ), sort, offset, limit, true );
     }
 
     @Override
@@ -186,7 +185,7 @@ public Slice<O> load( @Nullable Filters filters, @Nullable Sort sort, int offset
 
     @Override
     public List<O> loadWithCache( @Nullable Filters filters, @Nullable Sort sort ) {
-        return doLoadWithCache( filters, sort, true );
+        return doLoadWithCache( getByIdsFiltersIfPossible( filters ), sort, true );
     }
 
     @Override
@@ -196,7 +195,7 @@ public Slice<VO> loadValueObjects( @Nullable Filters filters, @Nullable Sort sor
 
     @Override
     public Slice<VO> loadValueObjectsWithCache( @Nullable Filters filters, @Nullable Sort sort, int offset, int limit ) {
-        return doLoadValueObjectsWithCache( filters, sort, offset, limit, true );
+        return doLoadValueObjectsWithCache( getByIdsFiltersIfPossible( filters ), sort, offset, limit, true );
     }
 
     @Override
@@ -206,7 +205,7 @@ public List<VO> loadValueObjects( @Nullable Filters filters, @Nullable Sort sort
 
     @Override
     public List<VO> loadValueObjectsWithCache( @Nullable Filters filters, @Nullable Sort sort ) {
-        return doLoadValueObjectsWithCache( filters, sort, true );
+        return doLoadValueObjectsWithCache( getByIdsFiltersIfPossible( filters ), sort, true );
     }
 
     @Override
@@ -216,7 +215,30 @@ public long count( @Nullable Filters filters ) {
 
     @Override
     public long countWithCache( @Nullable Filters filters ) {
-        return doCountWithCache( filters, true );
+        return doCountWithCache( getByIdsFiltersIfPossible( filters ), true );
+    }
+
+    /**
+     * Optimize a filter by replacing it with a filter over IDs if possible.
+     * <p>
+     * This is taking advantage of the fact that we can cache the IDs of individual filters and intersect those results
+     * instead of running a complex database query.
+     *
+     * @param filters   the filters to be optimized
+     * @return an equivalent filter, optimized
+     */
+    private Filters getByIdsFiltersIfPossible( @Nullable Filters filters ) {
+        if ( filters != null ) {
+            Iterator<Iterable<Filter>> it = filters.iterator();
+            if ( it.hasNext() ) {
+                Set<Long> ids = new HashSet<>( doLoadIdsWithCache( Filters.by( IterableUtils.toList( it.next() ) ), null, true ) );
+                it.forEachRemaining( clause -> {
+                    ids.retainAll( doLoadIdsWithCache( Filters.by( IterableUtils.toList( it.next() ) ), null, true ) );
+                } );
+                return Filters.by( getFilter( "id", Long.class, Filter.Operator.in, ids ) );
+            }
+        }
+        return filters;
     }
 
     private List<Long> doLoadIdsWithCache( @Nullable Filters filters, @Nullable Sort sort, boolean cacheable ) {

gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentServiceImpl.java

@@ -564,7 +564,7 @@ public Filters getFiltersWithInferredAnnotations( Filters f, @Nullable Collectio
         // apply inference to terms
         // collect clauses mentioning terms
         final Map<SubClauseKey, Set<String>> termUrisBySubClause = new HashMap<>();
-        for ( List<Filter> clause : f ) {
+        for ( Iterable<Filter> clause : f ) {
             Filters.FiltersClauseBuilder clauseBuilder = f2.and();
             for ( Filter subClause : clause ) {
                 if ( ArrayUtils.contains( PROPERTIES_USED_FOR_ANNOTATIONS, subClause.getOriginalProperty() ) ) {

gemma-core/src/main/java/ubic/gemma/persistence/util/Filter.java

@@ -38,20 +38,23 @@
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.stream.Collectors;
 
 import static ubic.gemma.persistence.util.PropertyMappingUtils.formProperty;
 
 /**
  * Holds the necessary information to filter an entity with a property, operator and right-hand side value.
- *
+ * <p>
+ * Clauses and sub-clauses are always sorted by objectAlias, propertyName, operator and requiredValue. A clause is
+ * sorted by its first element, if any.
  * @author tesarst
  * @author poirigui
  */
 @Value
-@EqualsAndHashCode(of = { "objectAlias", "operator", "requiredValue" })
-public class Filter implements PropertyMapping {
+@EqualsAndHashCode(of = { "objectAlias", "propertyName", "operator", "requiredValue" })
+public class Filter implements PropertyMapping, Comparable<Filter> {
 
     /**
      * This is only the date part of the ISO 8601 standard.
@@ -63,6 +66,25 @@ public class Filter implements PropertyMapping {
      */
     private static final ConfigurableConversionService conversionService = new GenericConversionService();
 
+    private static final Comparator<Filter> comparator = Comparator
+            .comparing( Filter::getObjectAlias, Comparator.nullsFirst( Comparator.naturalOrder() ) )
+            .thenComparing( Filter::getPropertyName )
+            .thenComparing( Filter::getOperator )
+            .thenComparing( Filter::getComparableRequiredValue, Comparator.nullsLast( Comparator.naturalOrder() ) );
+
+    @Nullable
+    private Comparable<Object> getComparableRequiredValue() {
+        Object requiredValue = this.requiredValue;
+        if ( operator.requiredType != null && Collection.class.isAssignableFrom( operator.requiredType ) ) {
+            // unpack the first element of the collection
+            if ( requiredValue != null ) {
+                requiredValue = ( ( Collection<?> ) requiredValue ).iterator().next();
+            }
+        }
+        //noinspection unchecked
+        return ( Comparable<Object> ) requiredValue;
+    }
+
     private static <T> void addConverter( Class<T> targetClass, Converter<String, T> converter, Converter<T, String> reverseConverter ) {
         conversionService.addConverter( String.class, targetClass, converter );
         conversionService.addConverter( targetClass, String.class, reverseConverter );
@@ -253,6 +275,11 @@ private Filter( @Nullable String objectAlias, String propertyName, Class<?> prop
         this.checkTypeCorrect();
     }
 
+    @Override
+    public int compareTo( Filter filter ) {
+        return comparator.compare( this, filter );
+    }
+
     @Override
     public String toString() {
         return toString( false );

gemma-core/src/main/java/ubic/gemma/persistence/util/FilterCriteriaUtils.java

@@ -6,8 +6,10 @@
 import javax.annotation.Nullable;
 import java.util.Collection;
 import java.util.List;
-import java.util.Objects;
+import java.util.stream.Collectors;
 
+import static java.util.Objects.requireNonNull;
+import static ubic.gemma.core.util.ListUtils.padToNextPowerOfTwo;
 import static ubic.gemma.persistence.util.PropertyMappingUtils.formProperty;
 
 /**
@@ -26,8 +28,8 @@ public static Criterion formRestrictionClause( @Nullable Filters filters ) {
         Conjunction c = Restrictions.conjunction();
         if ( filters == null || filters.isEmpty() )
             return c;
-        for ( List<Filter> clause : filters ) {
-            if ( clause == null || clause.isEmpty() )
+        for ( Iterable<Filter> clause : filters ) {
+            if ( clause == null || !clause.iterator().hasNext() )
                 continue;
             Disjunction d = Restrictions.disjunction();
             for ( Filter subClause : clause ) {
@@ -77,7 +79,7 @@ private static Criterion formRestrictionClause( Filter filter ) {
                     return Restrictions.ne( property, filter.getRequiredValue() );
                 }
             case like:
-                return Restrictions.like( property, escapeLike( ( String ) Objects.requireNonNull( filter.getRequiredValue(), "Required value cannot be null for the like operator." ) ), MatchMode.START );
+                return Restrictions.like( property, escapeLike( ( String ) requireNonNull( filter.getRequiredValue(), "Required value cannot be null for the like operator." ) ), MatchMode.START );
             case lessThan:
                 return Restrictions.lt( property, filter.getRequiredValue() );
             case greaterThan:
@@ -87,8 +89,12 @@ private static Criterion formRestrictionClause( Filter filter ) {
             case greaterOrEq:
                 return Restrictions.ge( property, filter.getRequiredValue() );
             case in:
-                return Restrictions.in( property, ( Collection<?> ) Objects.requireNonNull( filter.getRequiredValue(),
-                        "Required value cannot be null for a collection." ) );
+                List<?> item = requireNonNull( ( Collection<?> ) filter.getRequiredValue(), "Required value cannot be null for a collection." )
+                        .stream().sorted().distinct().collect( Collectors.toList() );
+                if ( item.isEmpty() ) {
+                    throw new IllegalArgumentException( "The right hand size of a in operator cannot be empty." );
+                }
+                return Restrictions.in( property, padToNextPowerOfTwo( item, item.get( item.size() - 1 ) ) );
             default:
                 throw new IllegalStateException( "Unexpected operator for filter: " + filter.getOperator() );
         }

gemma-core/src/main/java/ubic/gemma/persistence/util/FilterQueryUtils.java

@@ -9,6 +9,7 @@
 import java.util.stream.Collectors;
 
 import static java.util.Objects.requireNonNull;
+import static ubic.gemma.core.util.ListUtils.padToNextPowerOfTwo;
 import static ubic.gemma.persistence.util.PropertyMappingUtils.formProperty;
 
 /**
@@ -69,8 +70,8 @@ public static String formRestrictionClause( @Nullable Filters filters ) {
             return "";
         int i = 0;
         StringBuilder conjunction = new StringBuilder();
-        for ( List<Filter> clause : filters ) {
-            if ( clause == null || clause.isEmpty() )
+        for ( Iterable<Filter> clause : filters ) {
+            if ( clause == null || !clause.iterator().hasNext() )
                 continue;
             StringBuilder disjunction = new StringBuilder();
             boolean first = true;
@@ -200,7 +201,7 @@ public static void addRestrictionParameters( Query query, @Nullable Filters filt
     private static void addRestrictionParameters( Query query, @Nullable Filters filters, int i ) {
         if ( filters == null )
             return;
-        for ( List<Filter> clause : filters ) {
+        for ( Iterable<Filter> clause : filters ) {
             if ( clause == null )
                 continue;
             for ( Filter subClause : clause ) {
@@ -212,8 +213,12 @@ private static void addRestrictionParameters( Query query, @Nullable Filters fil
                     addRestrictionParameters( query, Filters.by( s.getFilter() ), i - 1 );
                 } else if ( subClause.getOperator().equals( Filter.Operator.in ) ) {
                     // order is unimportant for this operation, so we can ensure that it is consistent and therefore cacheable
-                    query.setParameterList( paramName, requireNonNull( ( Collection<?> ) subClause.getRequiredValue(), "Required value cannot be null for the 'in' operator." )
-                            .stream().sorted().distinct().collect( Collectors.toList() ) );
+                    List<?> item = requireNonNull( ( Collection<?> ) subClause.getRequiredValue(), "Required value cannot be null for the 'in' operator." )
+                            .stream().sorted().distinct().collect( Collectors.toList() );
+                    if ( item.isEmpty() ) {
+                        throw new IllegalArgumentException( "The right hand size of a in operator cannot be empty." );
+                    }
+                    query.setParameterList( paramName, padToNextPowerOfTwo( item, item.get( item.size() - 1 ) ) );
                 } else if ( subClause.getOperator().equals( Filter.Operator.like ) ) {
                     query.setParameter( paramName, escapeLike( ( String ) requireNonNull( subClause.getRequiredValue(), "Required value cannot be null for the 'like' operator." ) ) + "%" );
                 } else {