Changeset 1917

Timestamp:

Jun 8, 2011, 11:24:23 AM (12 years ago)

Author:

robert@…

Message:

Solved bugs in searching:

• exception when searching for [any field in any object] equals "" (#480)
• the authentication in searching works properly again

Location:

trunk/src/groovy/dbnp/query

Files:

• Criterion.groovy (modified) (7 diffs)
• Search.groovy (modified) (4 diffs)

trunk/src/groovy/dbnp/query/Criterion.groovy

@@ -194 +194 @@
                         case 'Template':
                         case 'Module':
+                        case 'File':
                                 return value?.toString();
-                        case 'File':
-                                return null; // Never search in filenames, since they are not very descriptive
                         case 'Date':
                                 // The comparison with date values should only be performed iff the value
@@ -296 +295 @@
                 
                 // If no value is given, don't do anything
-                if( !value )
+                if( value == null )
                         return emptyCriterion;
                 
@@ -398 +397 @@
                 }
 
-                def where = whereClause?.findAll { it } ? "( " + whereClause.join( " OR " ) + " )" : ""
+                def where = whereClause?.findAll { it } ? "( " + whereClause.findAll { it }.join( " OR " ) + " )" : ""
                 
                 return [ "join": joinClause, "where": where , "parameters": parameters ];
@@ -423 +422 @@
                         
                         def fieldName = field.name;
+                        def condition
                         
+                        // Searching in (complex) domain fields with value NULL will result in the whole
+                        // where clause to fail. e.g.:
+                        //
+                        // SELECT * FROM Sample sample WHERE sample.material LIKE '%m%' OR 1 = 1
+                        // should result in all samples. However, it only results in those samples
+                        // with sample.material =! NULL.
+                        //
+                        // For that reason, we use a subquery to select those  
                         if(     ( objectToSearchIn.toLowerCase() == "subject" && fieldName.toLowerCase() == "species" ) ||
                                 ( objectToSearchIn.toLowerCase() == "sample" && fieldName.toLowerCase() == "material" ) ||
                                 ( objectToSearchIn.toLowerCase() == "assay" && fieldName.toLowerCase() == "module" ) ||
                                 ( objectToSearchIn.toLowerCase() == "samplingevent" && fieldName.toLowerCase() == "sampletemplate" ) ) {
+                                
+                                condition = manyToManyWhereCondition( objectToSearchIn, fieldName, prefix, "name", value );
                                 fieldName += ".name"
-                        }
-                        
-                        // Search in template name
-                        def condition = extendWhereClause( "( %s )", objectToSearchIn + "." + fieldName, prefix, criterionType, castValue( criterionType ) );
+                        } else {
+                                condition = extendWhereClause( "( %s )", objectToSearchIn + "." + fieldName, prefix, criterionType, castValue( criterionType ) );
+                        }
+
+                        // Search in fields
                         whereClause += condition[ "where" ];
         
@@ -468 +479 @@
                 switch( this.operator ) {
                         case Operator.contains:
+                                // Every field contains an empty string, so we don't search on it
+                                if( !fieldValue )
+                                        return [ "where": "", "parameters": [:] ]
+                                        
                                 // Text fields should be handled case insensitive
+                                def clause
                                 if( textFieldTypes.contains( fieldType ) ) {
-                                        hql = sprintf( hql, "lower( " + fieldName + ") like lower( :" + uniquePrefix + "ValueLike )" );
+                                        clause = "lower( " + fieldName + ") like lower( :" + uniquePrefix + "ValueLike )"
                                 } else {
-                                        hql = sprintf( hql, fieldName + " like :" + uniquePrefix + "ValueLike" );
+                                        clause = fieldName + " like :" + uniquePrefix + "ValueLike";
                                 }
+                                
+                                hql = sprintf( hql, clause );
                                 parameters[ uniquePrefix + "ValueLike" ] = "%" + fieldValue + "%"
                                 break;
@@ -481 +499 @@
                         case Operator.lte:
                         case Operator.lt:
+                                // Text fields should be handled case insensitive
+                                def clause;
                                 if( textFieldTypes.contains( fieldType ) ) {
-                                        hql = sprintf( hql, "lower( " + fieldName + " ) "  + this.operator.name + " lower( :" + uniquePrefix + "Value" + fieldType + ")" );
+                                        clause = "lower( " + fieldName + " ) "  + this.operator.name + " lower( :" + uniquePrefix + "Value" + fieldType + ")"
                                 } else {
-                                        hql = sprintf( hql, fieldName + " "  + this.operator.name + " :" + uniquePrefix + "Value" + fieldType );
+                                        clause = fieldName + " "  + this.operator.name + " :" + uniquePrefix + "Value" + fieldType
                                 }
+                                
+                                // If the user searches for an empty string, it should also match the database NULL value
+                                if( operator == Operator.equals && !fieldValue ) {
+                                        clause += " OR " + fieldName + " IS NULL"
+                                }
+                                
+                                hql = sprintf( hql, clause );
                                 parameters[ uniquePrefix + "Value" + fieldType ] = fieldValue
                                 break;
@@ -509 +536 @@
                 def parameters = [:]
 
+                // If looking for a specific field, that field should also be mentioned in the where clause
                 if( this.field != '*' ) {
                         condition = "( %s AND index(" + fieldName + ") = :" + uniquePrefix + "Field )"

trunk/src/groovy/dbnp/query/Search.groovy

@@ -168 +168 @@
                 }
                 
+                // Generate where clause
+                def whereClause = " WHERE ";
+                if( fullHQL.where ) {
+                        whereClause += " ( " + fullHQL.where.join( " " + searchMode.toString() + " "  ) + " ) "
+                        whereClause += " AND ";
+                } 
+                
+                // Add a filter such that only readable studies are returned
+                def studyName = elementName( "Study" );
+                if( this.user == null ) {
+                        // Anonymous readers are only given access when published and public
+                        whereClause +=  " ( " + studyName + ".publicstudy = true AND " + studyName + ".published = true )"
+                } else if( !this.user.hasAdminRights() ) {
+                        // Administrators are allowed to read every study
+
+                        // Owners and writers are allowed to read this study
+                        // Readers are allowed to read this study when it is published
+                        whereClause += " ( " + studyName + ".owner = :sessionUser OR :sessionUser member of " + studyName + ".writers OR ( :sessionUser member of " + studyName + ".readers AND " + studyName + ".published = true ) )"
+                        fullHQL.parameters[ "sessionUser" ] = this.user
+                }
+                
                 // Combine all parts to generate a full HQL query
-                def hqlQuery = selectClause + " " + fullHQL.from + ( fullHQL.where ? "  WHERE " + fullHQL.where.join( " " + searchMode.toString() + " "  ) : "" );
-                
+                def hqlQuery = selectClause + " " + fullHQL.from + whereClause;
                 
                 // Find all objects 
@@ -387 +407 @@
                         
                 // Wildcards should be checked within each entity
-                def wildcardHQL = createHQLForEntity( this.entity, null, false );
+                def wildcardHQL = createHQLForEntity( this.entity, entityCriteria, false );
                 
                 // Create SQL for other entities, by executing a subquery first, and
@@ -411 +431 @@
                 if( whereClauses ) {
                         fullHQL.from += wildcardHQL.from
-                        fullHQL.where << whereClauses.join( " OR " )
+                        fullHQL.where << whereClauses.findAll { it }.join( " OR " )
                          
                         wildcardHQL[ "parameters" ].each {
@@ -440 +460 @@
                 entityCriteria.each {
                         def criteriaHQL = it.toHQL( "criterion" +entityName + criterionNum++, entityName.toLowerCase() );
-                        fromClause += " " + criteriaHQL[ "join" ]
-                        whereClause << criteriaHQL[ "where" ]
+                        
+                        if( criteriaHQL[ "join" ] )
+                                fromClause += " " + criteriaHQL[ "join" ]
+                                
+                        if( criteriaHQL[ "where" ] )
+                                whereClause << criteriaHQL[ "where" ]
+                                
                         criteriaHQL[ "parameters" ].each {
                                 parameters[ it.key ] = it.value
-                        }
-                }
-                
-                // Add a filter such that only readable studies are returned
-                if( entityName == "Study" ) {
-                        
-                        if( this.user == null ) {
-                                // Anonymous readers are only given access when published and public
-                                whereClause << "( study.publicstudy = true AND study.published = true )"
-                        } else if( !this.user.hasAdminRights() ) {
-                                // Administrators are allowed to read every study
-
-                                // Owners and writers are allowed to read this study
-                                // Readers are allowed to read this study when it is published
-                                whereClause << "( study.owner = :sessionUser OR :sessionUser member of study.writers OR ( :sessionUser member of study.readers AND study.published = true ) )"
-                                parameters[ "sessionUser" ] = this.user
                         }
                 }