Various fixes for ubic.gemma.persistence relating to null handling

PavlidisLab · Nov 29, 2022 · 43e1e6c · 43e1e6c
1 parent 27f8e03
commit 43e1e6c
Show file tree

Hide file tree

Showing 17 changed files with 67 additions and 133 deletions.
diff --git a/...ic/gemma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetDao.java b/...ic/gemma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetDao.java
@@ -84,5 +84,5 @@ public interface ExpressionAnalysisResultSetDao extends AnalysisResultSetDao<Dif
      * @param sort field and direction by which the collection is ordered
      * @return
      */
-    Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> databaseEntries, Filters objectFilters, int offset, int limit, Sort sort );
+    Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> databaseEntries, @Nullable Filters objectFilters, int offset, int limit, @Nullable Sort sort );
 }
diff --git a/...emma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetDaoImpl.java b/...emma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetDaoImpl.java
@@ -174,7 +174,7 @@ public void remove( ExpressionAnalysisResultSet resultSet ) {
     }
 
     @Override
-    public Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> databaseEntries, Filters objectFilters, int offset, int limit, Sort sort ) {
+    public Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> databaseEntries, @Nullable Filters objectFilters, int offset, int limit, @Nullable Sort sort ) {
         Criteria query = getLoadValueObjectsCriteria( objectFilters );
         Criteria totalElementsQuery = getLoadValueObjectsCriteria( objectFilters );
 

diff --git a/...emma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetService.java b/...emma/persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetService.java
@@ -12,6 +12,7 @@
 import ubic.gemma.persistence.util.Slice;
 import ubic.gemma.persistence.util.Sort;
 
+import javax.annotation.Nullable;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -28,5 +29,5 @@ public interface ExpressionAnalysisResultSetService extends AnalysisResultSetSer
 
     Map<DifferentialExpressionAnalysisResult, List<Gene>> loadResultToGenesMap( ExpressionAnalysisResultSet ears );
 
-    Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( Collection<BioAssaySet> bioAssaySets, Collection<DatabaseEntry> externalIds, Filters objectFilters, int offset, int limit, Sort sort );
+    Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> externalIds, @Nullable Filters objectFilters, int offset, int limit, @Nullable Sort sort );
 }
diff --git a/.../persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetServiceImpl.java b/.../persistence/service/analysis/expression/diff/ExpressionAnalysisResultSetServiceImpl.java
@@ -14,6 +14,7 @@
 import ubic.gemma.persistence.util.Slice;
 import ubic.gemma.persistence.util.Sort;
 
+import javax.annotation.Nullable;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -65,7 +66,7 @@ public Map<DifferentialExpressionAnalysisResult, List<Gene>> loadResultToGenesMa
 
     @Override
     @Transactional(readOnly = true)
-    public Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( Collection<BioAssaySet> bioAssaySets, Collection<DatabaseEntry> externalIds, Filters objectFilters, int offset, int limit, Sort sort ) {
+    public Slice<DifferentialExpressionAnalysisResultSetValueObject> findByBioAssaySetInAndDatabaseEntryInLimit( @Nullable Collection<BioAssaySet> bioAssaySets, @Nullable Collection<DatabaseEntry> externalIds, @Nullable Filters objectFilters, int offset, int limit, @Nullable Sort sort ) {
         return voDao.findByBioAssaySetInAndDatabaseEntryInLimit( bioAssaySets, externalIds,
                 objectFilters,
                 offset,

diff --git a/...ubic/gemma/persistence/service/common/auditAndSecurity/curation/AbstractCuratableDao.java b/...ubic/gemma/persistence/service/common/auditAndSecurity/curation/AbstractCuratableDao.java
@@ -3,16 +3,15 @@
 import gemma.gsec.util.SecurityUtil;
 import org.hibernate.SessionFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.transaction.annotation.Transactional;
 import ubic.gemma.model.common.auditAndSecurity.AuditEvent;
 import ubic.gemma.model.common.auditAndSecurity.curation.AbstractCuratableValueObject;
 import ubic.gemma.model.common.auditAndSecurity.curation.Curatable;
 import ubic.gemma.persistence.service.AbstractQueryFilteringVoEnabledDao;
 import ubic.gemma.persistence.service.common.auditAndSecurity.CurationDetailsDao;
-import ubic.gemma.persistence.service.common.auditAndSecurity.CurationDetailsDaoImpl;
 import ubic.gemma.persistence.util.Filters;
 import ubic.gemma.persistence.util.ObjectFilter;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
@@ -78,7 +77,7 @@ protected void addEventsToMap( Map<Long, Collection<AuditEvent>> eventMap, Long
     /**
      * Restrict results to non-troubled curatable entities for non-administrators
      */
-    protected void addNonTroubledFilter( Filters filters, String objectAlias ) {
+    protected void addNonTroubledFilter( Filters filters, @Nullable String objectAlias ) {
         if ( !SecurityUtil.isUserAdmin() ) {
             filters.add( ObjectFilter.parseObjectFilter( objectAlias, "curationDetails.troubled", Boolean.class, ObjectFilter.Operator.eq, "false" ) );
         }

diff --git a/...ava/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java b/...ava/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java
@@ -1131,7 +1131,7 @@ public Slice<ExpressionExperimentDetailsValueObject> loadDetailsValueObjectsById
 
         if ( ids != null ) {
             if ( ids.isEmpty() )
-                return new Slice<>();
+                Slice.empty();
             List<Long> idList = new ArrayList<>( ids );
             Collections.sort( idList );
             filters.add( new ObjectFilter( getObjectAlias(), "id", Long.class, ObjectFilter.Operator.in, idList ) );

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/AclCriteriaUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/AclCriteriaUtils.java
@@ -21,8 +21,8 @@ public class AclCriteriaUtils {
      * @see AclQueryUtils#formAclRestrictionClause()
      */
     public static Criterion formAclRestrictionClause( String alias, Class<? extends Securable> aoiType ) {
-        if ( StringUtils.isBlank( alias ) || aoiType == null )
-            throw new IllegalArgumentException( "Alias and aoiType can not be empty." );
+        if ( StringUtils.isBlank( alias ) )
+            throw new IllegalArgumentException( "Alias cannot be empty." );
 
         DetachedCriteria dc = DetachedCriteria.forClass( AclObjectIdentity.class, "aoi" )
                 .setProjection( Projections.property( "aoi.identifier" ) )

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
@@ -1,7 +1,5 @@
 package ubic.gemma.persistence.util;
 
-import lombok.NonNull;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
@@ -13,11 +11,7 @@
  */
 public class Filters implements Iterable<ObjectFilter[]> {
 
-    private final ArrayList<ObjectFilter[]> internalFilters;
-
-    public Filters() {
-        internalFilters = new ArrayList<>();
-    }
+    private final ArrayList<ObjectFilter[]> internalFilters = new ArrayList<>();
 
     /**
      * Create a singleton {@link Filters} from a {@link ObjectFilter}.
@@ -34,7 +28,7 @@ public static Filters singleFilter( ObjectFilter filter ) {
     /**
      * Add a disjunction of one or more {@link ObjectFilter} clauses.
      */
-    public void add( @NonNull ObjectFilter... filters ) {
+    public void add( ObjectFilter... filters ) {
         internalFilters.add( filters );
     }
 

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/FiltersUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/FiltersUtils.java
@@ -2,6 +2,8 @@
 
 import org.apache.commons.lang3.ArrayUtils;
 
+import javax.annotation.Nullable;
+
 /**
  * Utilities for working with {@link Filters} and {@link ObjectFilter}.
  */
@@ -16,7 +18,7 @@ public class FiltersUtils {
      * @param aliases
      * @return true if any provided alias is mentioned anywhere in the set of filters
      */
-    public static boolean containsAnyAlias( Filters filters, String... aliases ) {
+    public static boolean containsAnyAlias( @Nullable Filters filters, String... aliases ) {
         if ( filters == null )
             return false;
         for ( ObjectFilter[] filter : filters ) {

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectAlias.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectAlias.java
diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilter.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilter.java
@@ -30,13 +30,16 @@
 import org.springframework.core.convert.support.ConfigurableConversionService;
 import org.springframework.core.convert.support.GenericConversionService;
 
+import javax.annotation.Nullable;
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Date;
 import java.util.Optional;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Holds the necessary information to filter an entity with a property, operator and right-hand side value.
@@ -88,7 +91,7 @@ private static <T> void addConverter( Class<?> targetClass, Converter<String, T>
      *                                  desired propertyType.
      * @see #ObjectFilter(String, String, Class, Operator, Object)
      */
-    public static ObjectFilter parseObjectFilter( String alias, String propertyName, Class<?> propertyType, Operator operator, String requiredValue ) throws IllegalArgumentException {
+    public static ObjectFilter parseObjectFilter( @Nullable String alias, String propertyName, Class<?> propertyType, Operator operator, String requiredValue ) throws IllegalArgumentException {
         return new ObjectFilter( alias, propertyName, propertyType, operator, parseRequiredValue( requiredValue, propertyType ) );
     }
 
@@ -105,14 +108,15 @@ public static ObjectFilter parseObjectFilter( String alias, String propertyName,
      *                                  desired propertyType.
      * @see #ObjectFilter(String, String, Class, Operator, Object)
      */
-    public static ObjectFilter parseObjectFilter( String alias, String propertyName, Class<?> propertyType, Operator operator, Collection<String> requiredValues ) throws IllegalArgumentException {
+    public static ObjectFilter parseObjectFilter( @Nullable String alias, String propertyName, Class<?> propertyType, Operator operator, Collection<String> requiredValues ) throws IllegalArgumentException {
         return new ObjectFilter( alias, propertyName, propertyType, operator, parseRequiredValues( requiredValues, propertyType ) );
     }
 
+    @Getter
     public enum Operator {
         /**
-         * Note that in the case of a null requiredValue, the {@link #sqlToken} of this operator must be ignored and 'is'
-         * must be used instead.
+         * Note that in the case of a null requiredValue, the {@link #getSqlToken()} of this operator must be ignored
+         * and 'is' must be used instead.
          */
         eq( "=", false, null ),
         /**
@@ -146,11 +150,6 @@ public static Optional<Operator> fromToken( String token ) {
          */
         private final String token;
 
-        /**
-         * Token used in SQL/HQL query.
-         */
-        private final String sqlToken;
-
         /**
          * THe required value must not be null.
          */
@@ -159,39 +158,31 @@ public static Optional<Operator> fromToken( String token ) {
         /**
          * The required value must satisfy this type.
          */
+        @Nullable
         private final Class<?> requiredType;
 
-        Operator( String operator, boolean isNonNullRequired, Class<?> requiredType ) {
+        Operator( String operator, boolean isNonNullRequired, @Nullable Class<?> requiredType ) {
             this.token = operator;
-            this.sqlToken = operator;
             this.nonNullRequired = isNonNullRequired;
             this.requiredType = requiredType;
         }
 
-        public String getToken() {
-            return token;
-        }
-
         /**
+         * Token used in SQL/HQL query.
+         * <p>
          * This is package-private on purpose and is only meant for{@link ObjectFilterQueryUtils#formRestrictionClause(Filters)}.
          */
         String getSqlToken() {
-            return sqlToken;
-        }
-
-        public boolean isNonNullRequired() {
-            return nonNullRequired;
-        }
-
-        public Class<?> getRequiredType() {
-            return requiredType;
+            return token;
         }
     }
 
+    @Nullable
     private final String objectAlias;
     private final String propertyName;
     private final Class<?> propertyType;
     private final Operator operator;
+    @Nullable
     private final Object requiredValue;
 
     /**
@@ -207,7 +198,7 @@ public Class<?> getRequiredType() {
      * @param requiredValue a required value, or null to perform a null-check (i.e. <code>objectAlias.propertyName is null</code>)
      * @throws IllegalArgumentException if the type of the requiredValue does not match the propertyType
      */
-    public ObjectFilter( String objectAlias, String propertyName, Class<?> propertyType, Operator operator, Object requiredValue ) throws IllegalArgumentException {
+    public ObjectFilter( @Nullable String objectAlias, String propertyName, Class<?> propertyType, Operator operator, @Nullable Object requiredValue ) throws IllegalArgumentException {
         this.objectAlias = objectAlias;
         this.propertyName = propertyName;
         this.propertyType = propertyType;
@@ -255,7 +246,7 @@ private void checkTypeCorrect() throws IllegalArgumentException {
     private static Object parseRequiredValue( String rv, Class<?> pt ) throws IllegalArgumentException {
         if ( isCollection( rv ) ) {
             // convert individual elements
-            return parseCollection( rv ).stream()
+            return parseCollection( rv )
                     .map( item -> parseItem( item, pt ) )
                     .collect( Collectors.toList() );
         } else {
@@ -269,8 +260,12 @@ private static Object parseRequiredValues( Collection<String> requiredValues, Cl
                 .collect( Collectors.toList() );
     }
 
+    private static final Pattern
+            COLLECTION_PATTERN = Pattern.compile( "^\\((.+,)*.+\\)$" ),
+            COLLECTION_DELIMITER_PATTERN = Pattern.compile( "\\s*,\\s*" );
+
     private static boolean isCollection( String value ) {
-        return value.trim().matches( "^\\((.+,)*.+\\)$" );
+        return COLLECTION_PATTERN.matcher( value ).matches();
     }
 
     /**
@@ -279,11 +274,9 @@ private static boolean isCollection( String value ) {
      *              of strings.
      * @return a collection of strings.
      */
-    private static Collection<String> parseCollection( String value ) {
-        return Arrays.asList( value
-                .trim()
-                .substring( 1, value.length() - 1 ) // these are the parenthesis
-                .split( "\\s*,\\s*" ) );
+    private static Stream<String> parseCollection( String value ) {
+        // these are the parenthesis, thus we skip the first and last non-blank character
+        return COLLECTION_DELIMITER_PATTERN.splitAsStream( value.trim().substring( 1, value.length() - 1 ) );
     }
 
     private static Object parseItem( String rv, Class<?> pt ) throws IllegalArgumentException {

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilterCriteriaUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilterCriteriaUtils.java
@@ -3,7 +3,9 @@
 import org.hibernate.Criteria;
 import org.hibernate.criterion.*;
 
+import javax.annotation.Nullable;
 import java.util.Collection;
+import java.util.Objects;
 
 /**
  * Utilities for integrating {@link ObjectFilter} with Hibernate {@link Criteria} API.
@@ -17,7 +19,7 @@ public class ObjectFilterCriteriaUtils {
      * @param objectFilters the filters to use to create the clause
      * @return a restriction clause that can be appended to a {@link Criteria} using {@link Criteria#add(Criterion)}
      */
-    public static Criterion formRestrictionClause( Filters objectFilters ) {
+    public static Criterion formRestrictionClause( @Nullable Filters objectFilters ) {
         Conjunction c = Restrictions.conjunction();
         if ( objectFilters == null || objectFilters.isEmpty() )
             return c;
@@ -64,7 +66,8 @@ private static Criterion formRestrictionClause( ObjectFilter filter ) {
             case greaterOrEq:
                 return Restrictions.ge( propertyName, filter.getRequiredValue() );
             case in:
-                return Restrictions.in( propertyName, ( Collection<?> ) filter.getRequiredValue() );
+                return Restrictions.in( propertyName, ( Collection<?> ) Objects.requireNonNull( filter.getRequiredValue(),
+                        "Required value cannot be null for a collection." ) );
             default:
                 throw new IllegalStateException( "Unexpected operator for filter: " + filter.getOperator() );
         }

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilterQueryUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/ObjectFilterQueryUtils.java
@@ -173,7 +173,8 @@ public static void addRestrictionParameters( Query query, Filters filters ) {
                 String paramName = formParamName( filter.getObjectAlias(), filter.getPropertyName() ) + ( ++i );
                 if ( filter.getOperator().equals( ObjectFilter.Operator.in ) ) {
                     // order is unimportant for this operation, so we can ensure that it is consistent and therefore cacheable
-                    query.setParameterList( paramName, ( ( Collection<?> ) filter.getRequiredValue() ).stream().sorted().distinct().collect( Collectors.toList() ) );
+                    query.setParameterList( paramName, Objects.requireNonNull( ( Collection<?> ) filter.getRequiredValue(), "Required value cannot be null for a collection.." )
+                            .stream().sorted().distinct().collect( Collectors.toList() ) );
                 } else if ( filter.getOperator().equals( ObjectFilter.Operator.like ) ) {
                     query.setParameter( paramName, "%" + filter.getRequiredValue() + "%" );
                 } else {

diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/ReflectionUtil.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/ReflectionUtil.java