From 9c7642495b0fa7d9bb3c96a278dffb121063f542 Mon Sep 17 00:00:00 2001
From: Guillaume Poirier-Morency <poirigui@msl.ubc.ca>
Date: Mon, 25 Sep 2023 14:20:46 -0700
Subject: [PATCH] Break-up filters so that individual clauses can be cached
 (fix #870)

---
 .../AbstractQueryFilteringVoEnabledDao.java   | 40 ++++++++++++++-----
 .../ubic/gemma/persistence/util/Filters.java  |  9 +++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/AbstractQueryFilteringVoEnabledDao.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/AbstractQueryFilteringVoEnabledDao.java
index 2276f3be65..a965113728 100644
--- a/gemma-core/src/main/java/ubic/gemma/persistence/service/AbstractQueryFilteringVoEnabledDao.java
+++ b/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 ) {
diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/Filters.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/Filters.java
index 1f0d9809ef..1f0e071450 100644
--- a/gemma-core/src/main/java/ubic/gemma/persistence/util/Filters.java
+++ b/gemma-core/src/main/java/ubic/gemma/persistence/util/Filters.java
@@ -96,6 +96,10 @@ public static Filters by( Filter... subClauses ) {
         return empty().and( subClauses );
     }
 
+    public static Filters by( Collection<Filter> subClauses ) {
+        return empty().and( subClauses );
+    }
+
     /**
      * Create a singleton {@link Filters} from an explicit clause.
      */
@@ -181,6 +185,11 @@ public Filters and( Filter... filters ) {
         return this;
     }
 
+    public Filters and( Collection<Filter> filters ) {
+        clauses.add( new TreeSet<>( filters ) );
+        return this;
+    }
+
     /**
      * Add a clause of one explicit clause to the conjunction.
      */