Changeset 2387

Timestamp:

05/20/08 14:33:21 (7 months ago)

Author:

jmoore

Message:

OmeroSearch improvements for ticket:975 and ticket:976

• Added concept of union, intersection, complement
  
• Re-enablded byHqlQuery() (More documentation on IQuery)
  
• Further specified the logic of AnnotatedWith with a null value
  
• Added specific TextAnnotation subclasses for now
  
• Fixed serialization of FullText before hasNext() is called
  
• SecurityViolation still being thrown in multiuser tests
  

Location:

trunk/components

Files:

• blitz/resources/omero/API.ice (modified) (3 diffs)
• common/src/ome/api/IQuery.java (modified) (1 diff)
• common/src/ome/api/Search.java (modified) (4 diffs)
• server/src/ome/services/SearchBean.java (modified) (7 diffs)
• server/src/ome/services/search/AnnotatedWith.java (modified) (6 diffs)
• server/src/ome/services/search/Complement.java (added)
• server/src/ome/services/search/FullText.java (modified) (9 diffs)
• server/src/ome/services/search/HqlQuery.java (modified) (2 diffs)
• server/src/ome/services/search/Intersection.java (added)
• server/src/ome/services/search/SearchAction.java (modified) (3 diffs)
• server/src/ome/services/search/Union.java (added)
• server/test/ome/server/itests/search/SearchTest.java (modified) (11 diffs)

trunk/components/blitz/resources/omero/API.ice

@@ -437 +437 @@
         bool isAllowLeadingWildcard() throws ServerError;
 
+
         // Filters ~~~~~~~~~~~~~~~~~~~~~~
 
@@ -452 +453 @@
         void onlyAnnotatedWith(StringSet classes) throws ServerError;
 
+
         // Fetches, order, counts, etc ~~~~~~~~~~~~~~~~~~~~~~
 
@@ -460 +462 @@
         void fetchAlso(StringSet fetches) throws ServerError;
 
+
         // Reset ~~~~~~~~~~~~~~~~~~~~~~~~~
 
         void resetDefaults() throws ServerError;
 
+
         // Query state  ~~~~~~~~~~~~~~~~~~~~~~~~~
+
         void byGroupForTags(string group) throws ServerError;
         void byTagForGroups(string tag) throws ServerError;
         void byFullText(string query) throws ServerError;
+        void byHqlQuery(string query, omero::sys::Parameters params) throws ServerError;
         void bySomeMustNone(StringSet some, StringSet must, StringSet none) throws ServerError;
         void byAnnotatedWith(AnnotationList examples) throws ServerError;
         void clearQueries() throws ServerError;
 
+        void union() throws ServerError;
+        void intersection() throws ServerError;
+        void complement() throws ServerError;
+
+
         // Retrieval  ~~~~~~~~~~~~~~~~~~~~~~~~~
+
         bool hasNext() throws ServerError;
         omero::model::IObject next() throws ServerError;
         IObjectList results() throws ServerError;
-        
+
         // Currently unused
         SearchMetadata currentMetadata() throws ServerError;
         SearchMetadataList currentMetadataList() throws ServerError;
-        
+
         // Unused; Part of Java Iterator interface
         void remove() throws ServerError;

trunk/components/common/src/ome/api/IQuery.java

@@ -214 +214 @@
      * query has proper permissions.
      * 
+     * Queries can only return lists of {@link IObject} instances. This means
+     * all must be of the form:
+     * 
+     * <pre>
+     * select this from SomeModelClass this ...
+     * </pre>
+     * 
+     * though the alias "this" is unimportant. Do not try to return multiple
+     * classes in one call like:
+     * 
+     * <pre>
+     * select this, that from SomeClass this, SomeOtherClass that ...
+     * </pre>
+     * 
+     * nor to project values out of an object:
+     * 
+     * <pre>
+     * select this.name from SomeClass this ...
+     * </pre>
+     * 
      * If a {@link Page} is desired, add it to the query parameters.
      * 

trunk/components/common/src/ome/api/Search.java

@@ -457 +457 @@
 
     /**
-     * Provides the main {@link IQuery} method here to take advantage of the
-     * various filters which are provided. See
-     * {@link IQuery#findAllByQuery(String, Parameters)} for the semantics.
+     * Delegates to {@link IQuery#findAllByQuery(String, Parameters)} method to
+     * take advantage of the {@link #intersection()}, {@link #union()}, and
+     * {@link #complement()} methods, or queue-semantics.
      * 
      * @param query
@@ -467 +467 @@
      * @see IQuery#findAllByQuery(String, Parameters)
      */
-    // Disabling. This needs to be more intelligent before it can
-    // be provided here. Something of the form byHql(String[] join, String[]
-    // where) etc.
-    // Quite possibly note worth it.
-    // void byHqlQuery(String query, Parameters p);
+    void byHqlQuery(String query, Parameters p);
+
     /**
      * Finds entities annotated with an {@link Annotation} similar to the
@@ -481 +478 @@
      * {@link OriginalFile#getId()}, etc.</li>
      * </ul>
+     * <p>
+     * If the main content is <em>null</em> it is assumed to be a wildcard
+     * searched, and only the type of the annotation is searched. Currently,
+     * ListAnnotations are not supported.
+     * <p>
      * 
      * @param examples
@@ -486 +488 @@
      */
     void byAnnotatedWith(Annotation... examples);
+
+    /**
+     * Applies the next by* method to the previous by* method, so that a call
+     * {@link #hasNext()}, {@link #next()}, or {@link #results()} sees only
+     * the union of the two calls.
+     * 
+     * For example,
+     * 
+     * <pre>
+     * service.onlyType(Image.class);
+     * service.byFullText(&quot;foo&quot;);
+     * service.union();
+     * service.onlyType(Dataset.class);
+     * service.byFullText(&quot;foo&quot;);
+     * </pre>
+     * 
+     * will return both Images and Datasets together.
+     * 
+     * Calling this method overrides a previous setting of
+     * {@link #intersection()} or {@link #complement()}. If there is no active
+     * queries (i.e. {@link #activeQueries()} > 0), then an
+     * {@link ApiUsageException} will be thrown.
+     */
+    void union();
+
+    /**
+     * Applies the next by* method to the previous by* method, so that a call
+     * {@link #hasNext()}, {@link #next()}, or {@link #results()} sees only
+     * the intersection of the two calls.
+     * 
+     * For example,
+     * 
+     * <pre>
+     * service.onlyType(Image.class);
+     * service.byFullText(&quot;foo&quot;);
+     * service.intersection();
+     * service.byAnnotatedWith(TagAnnotation.class);
+     * </pre>
+     * 
+     * will return only the Images with TagAnnotations.
+     * 
+     * <p>
+     * Calling this method overrides a previous setting of {@link #union()} or
+     * {@link #complement()}. If there is no active queries (i.e.
+     * {@link #activeQueries()} > 0), then an {@link ApiUsageException} will be
+     * thrown.
+     * </p>
+     */
+    void intersection();
+
+    /**
+     * Applies the next by* method to the previous by* method, so that a call
+     * {@link #hasNext()}, {@link #next()}, or {@link #results()} sees only
+     * the intersection of the two calls.
+     * 
+     * For example,
+     * 
+     * <pre>
+     * service.onlyType(Image.class);
+     * service.byFullText(&quot;foo&quot;);
+     * service.complement();
+     * service.byAnnotatedWith(TagAnnotation.class);
+     * </pre>
+     * 
+     * will return all the Images <em>not</em> annotated with TagAnnotation.
+     * <p>
+     * Calling this method overrides a previous setting of {@link #union()} or
+     * {@link #intersection()}. If there is no active queries (i.e.
+     * {@link #activeQueries()} > 0), then an {@link ApiUsageException} will be
+     * thrown.
+     * </p>
+     */
+    void complement();
 
     /**

trunk/components/server/src/ome/services/SearchBean.java

@@ -8 +8 @@
 package ome.services;
 
+import java.io.Serializable;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -31 +31 @@
 import ome.api.ServiceInterface;
 import ome.conditions.ApiUsageException;
+import ome.conditions.InternalException;
 import ome.model.IObject;
 import ome.model.annotations.Annotation;
@@ -36 +37 @@
 import ome.parameters.Parameters;
 import ome.services.search.AnnotatedWith;
+import ome.services.search.Complement;
 import ome.services.search.FullText;
 import ome.services.search.HqlQuery;
+import ome.services.search.Intersection;
 import ome.services.search.SearchAction;
 import ome.services.search.SearchValues;
 import ome.services.search.SomeMustNone;
 import ome.services.search.TagsAndGroups;
+import ome.services.search.Union;
 import ome.services.util.Executor;
 import ome.services.util.OmeroAroundInvoke;
@@ -73 +77 @@
     private final static Log log = LogFactory.getLog(SearchBean.class);
 
-    private final List<SearchAction> actions = Collections
-            .synchronizedList(new ArrayList<SearchAction>());
+    private final ActionList actions = new ActionList();
 
     private final SearchValues values = new SearchValues();
@@ -212 +215 @@
     }
 
+    // LOGICAL COMBINATIONS
+
+    @Transactional
+    @RolesAllowed("user")
+    public void union() {
+        actions.union();
+    }
+
+    @Transactional
+    @RolesAllowed("user")
+    public void intersection() {
+        actions.intersection();
+    }
+
+    @Transactional
+    @RolesAllowed("user")
+    public void complement() {
+        actions.complement();
+    }
+
     //
     // FETCH METHODS
@@ -233 +256 @@
             return false;
         }
-        SearchAction action = actions.remove(0);
+        SearchAction action = actions.popFirst();
         List<IObject> list = (List<IObject>) executor.execute(null, action);
         results.add(list);
@@ -642 +665 @@
         }
     }
+
+    /**
+     * Synchronized helper collection for maintaining {@link SearchAction}
+     * instances. Also knows how to do logical joins (union, etc.)
+     */
+    private static class ActionList implements Serializable {
+
+        private static final long serialVersionUID = 1L;
+
+        enum State {
+            normal, union, intersection, complement;
+        }
+
+        private State state = State.normal;
+
+        final private List<SearchAction> actions = new ArrayList<SearchAction>();
+
+        synchronized void union() {
+            state = State.union;
+        }
+
+        synchronized void intersection() {
+            state = State.intersection;
+        }
+
+        synchronized void complement() {
+            state = State.complement;
+        }
+
+        synchronized void add(SearchAction b) {
+
+            // Any call to "add" reset the state of the ActionList
+            State previousState = state;
+            this.state = State.normal;
+
+            SearchAction a;
+            switch (previousState) {
+            case normal:
+                actions.add(b);
+                break;
+            case union:
+                a = popLast();
+                actions.add(new Union(b.copyOfValues(), a, b));
+                break;
+            case intersection:
+                a = popLast();
+                actions.add(new Intersection(b.copyOfValues(), a, b));
+                break;
+            case complement:
+                a = popLast();
+                actions.add(new Complement(b.copyOfValues(), a, b));
+                break;
+            default:
+                throw new InternalException("Unknown state:" + state);
+            }
+        }
+
+        synchronized int size() {
+            return actions.size();
+        }
+
+        synchronized void clear() {
+            actions.clear();
+        }
+
+        synchronized SearchAction popFirst() {
+            assertNonZero();
+            return actions.remove(0);
+        }
+
+        synchronized SearchAction popLast() {
+            assertNonZero();
+            return actions.remove(actions.size() - 1);
+        }
+
+        synchronized void assertNonZero() {
+            if (actions.size() == 0) {
+                throw new ApiUsageException("There must be at least 1"
+                        + " active query for this operation.");
+            }
+        }
+    }
 }

trunk/components/server/src/ome/services/search/AnnotatedWith.java

@@ -18 +18 @@
 import ome.model.annotations.FileAnnotation;
 import ome.model.annotations.LongAnnotation;
+import ome.model.annotations.QueryAnnotation;
+import ome.model.annotations.TagAnnotation;
 import ome.model.annotations.TextAnnotation;
 import ome.model.annotations.ThumbnailAnnotation;
 import ome.model.annotations.TimestampAnnotation;
+import ome.model.annotations.UrlAnnotation;
+import ome.model.annotations.XmlAnnotation;
 import ome.model.core.OriginalFile;
 import ome.model.display.Thumbnail;
@@ -107 +111 @@
         for (int i = 0; i < annotation.length; i++) {
             if (annotation[i] instanceof TextAnnotation) {
-                annCls[i] = TextAnnotation.class;
+                // FIXME This should be unneccessary. See ticket:976
+                if (annotation[i] instanceof TagAnnotation) {
+                    annCls[i] = TagAnnotation.class;
+                } else if (annotation[i] instanceof QueryAnnotation) {
+                    annCls[i] = QueryAnnotation.class;
+                } else if (annotation[i] instanceof UrlAnnotation) {
+                    annCls[i] = UrlAnnotation.class;
+                } else if (annotation[i] instanceof XmlAnnotation) {
+                    annCls[i] = XmlAnnotation.class;
+                } else {
+                    annCls[i] = TextAnnotation.class;
+                }
                 type[i] = String.class;
                 path[i] = "textValue";
@@ -145 +160 @@
                         + annotation);
             }
-            // fetch annotations
+
+            // If we have an example of the given annoation, then we can
+            // fetch it directly and don't need to use the fetchAnnotationsCopy
+            // collection.
             for (Class ac : values.fetchAnnotations) {
                 if (annCls[i].isAssignableFrom(ac)) {
@@ -152 +170 @@
                 }
             }
+            // On the other hand, if the value is null, then we might as well
+            // treat this as a fetch request and search only on Ann.class =.
+            // See the guards around the Joins section and the the call to
+            // notNullOrLikeOrEqual below.
+            if (value[i] == null) {
+                fetchAnnotationsCopy.add(annCls[i]);
+            }
         }
 
@@ -165 +190 @@
         qb.select("this");
         qb.from(cls.getName(), "this");
+
+        // Joins
         for (int i = 0; i < ann.length; i++) {
-            link[i] = qb.unique_alias("link");
-            ann[i] = link[i] + "_child";
-            qb.join("this.annotationLinks", link[i], false, fetch[i]);
-            qb.join(link[i] + ".child", ann[i], false, fetch[i]);
+            if (value[i] != null) {
+                link[i] = qb.unique_alias("link");
+                ann[i] = link[i] + "_child";
+                qb.join("this.annotationLinks", link[i], false, fetch[i]);
+                qb.join(link[i] + ".child", ann[i], false, fetch[i]);
+            }
         }
 
@@ -191 +220 @@
             // Main criteria
             if (useNamespace) {
-                notNullOrLikeOrEqual(qb, ann + ".name", type[j], annotation[j]
+                notNullOrLikeOrEqual(qb, ann + ".ns", type[j], annotation[j]
                         .getNs(), useLike, values.caseSensitive);
             }
 
-            notNullOrLikeOrEqual(qb, ann[j] + "." + path[j], type[j], value[j],
-                    useLike, values.caseSensitive);
+            // If the value of the annotation is null, we assume that we are not
+            // actually searching for null annotations (whose nullability is
+            // actually a by-product of polymorphism), instead we assume the
+            // null acts like a wildcard, in which case this search has been
+            // added to the fetchAnnotationsCopy collection above.
+            if (value[j] != null) {
+                notNullOrLikeOrEqual(qb, ann[j] + "." + path[j], type[j],
+                        value[j], useLike, values.caseSensitive);
+            }
 
             annotatedBetween(qb, ann[j] + ".");

trunk/components/server/src/ome/services/search/FullText.java

@@ -8 +8 @@
 package ome.services.search;
 
+import java.util.List;
+
 import ome.conditions.ApiUsageException;
+import ome.conditions.SecurityViolation;
 import ome.model.IAnnotated;
 import ome.services.SearchBean;
@@ -19 +22 @@
 import org.apache.lucene.search.SortField;
 import org.hibernate.Criteria;
-import org.hibernate.Query;
-import org.hibernate.ScrollableResults;
 import org.hibernate.Session;
 import org.hibernate.criterion.Restrictions;
@@ -30 +31 @@
 
 /**
- * Query template used by {@link SearchBean} to store user requests.
+ * Search based on Lucene's {@link Query} class. Takes a Google-like search
+ * string and returns fully formed objects via Hibernate Search.
  * 
  * @author Josh Moore, josh at glencoesoftware.com
@@ -37 +39 @@
 public class FullText extends SearchAction {
 
-    private final static QueryParser parser = new QueryParser(
-            "combined_fields", new StandardAnalyzer());
-
     private static final long serialVersionUID = 1L;
 
     private final String queryStr;
 
-    private FullTextSession session;
-
-    private org.apache.lucene.search.Query q;
-
-    private Query query;
-
-    private ScrollableResults scroll;
+    private final org.apache.lucene.search.Query q;
 
     public FullText(SearchValues values, String query) {
@@ -79 +72 @@
         this.queryStr = query;
         try {
+            final QueryParser parser = new QueryParser("combined_fields",
+                    new StandardAnalyzer());
             parser.setAllowLeadingWildcard(values.leadingWildcard);
             q = parser.parse(queryStr);
@@ -88 +83 @@
     }
 
-    public Object doWork(TransactionStatus status, Session session,
-            ServiceFactory sf) {
+    public Object doWork(TransactionStatus status, Session s, ServiceFactory sf) {
 
-        Class cls = values.onlyTypes.get(0);
+        final Class<?> cls = values.onlyTypes.get(0);
 
-        this.session = Search.createFullTextSession(session);
+        FullTextSession session = Search.createFullTextSession(s);
         Criteria criteria = session.createCriteria(cls);
         AnnotationCriteria ann = new AnnotationCriteria(criteria,
@@ -118 +112 @@
                     return null; // EARLY EXIT !
                 } else {
-                    for (Class annCls : values.onlyAnnotatedWith) {
+                    for (Class<?> annCls : values.onlyAnnotatedWith) {
                         SimpleExpression ofType = new TypeEqualityExpression(
                                 "class", annCls);
@@ -130 +124 @@
 
         // Main query
-        FullTextQuery ftQuery = this.session.createFullTextQuery(this.q);
+        FullTextQuery ftQuery = session.createFullTextQuery(this.q, cls);
         ftQuery.setCriteriaQuery(criteria);
 
@@ -151 +145 @@
         }
 
-        query = ftQuery;
-        return query.list();
+        final String ticket975 = "ticket:975 - Wrong return type: %s instead of %s\n"
+                + "Under some circumstances, byFullText and related methods \n"
+                + "like bySomeMustNone can return instances of the wrong \n"
+                + "types. One known case is the use of onlyAnnotatedWith(). \n"
+                + "If you are recieving this error, please try using the \n"
+                + "intersection/union methods to achieve the same results.";
+
+        List<?> check975 = ftQuery.list();
+
+        // WORKAROUND
+        for (Object object : check975) {
+            if (!cls.isAssignableFrom(object.getClass())) {
+                throw new ApiUsageException(String.format(ticket975, object
+                        .getClass(), cls));
+            }
+        }
+        return check975;
     }
-
 }

trunk/components/server/src/ome/services/search/HqlQuery.java

@@ -8 +8 @@
 package ome.services.search;
 
+import ome.api.IQuery;
 import ome.parameters.Parameters;
-import ome.services.SearchBean;
 import ome.system.ServiceFactory;
 
@@ -16 +16 @@
 
 /**
- * Query template used by {@link SearchBean} to store user requests.
+ * Delegate to {@link IQuery#findAllByQuery(String, Parameters)}.
  * 
  * @author Josh Moore, josh at glencoesoftware.com

trunk/components/server/src/ome/services/search/SearchAction.java

@@ -8 +8 @@
 package ome.services.search;
 
+import java.io.Serializable;
 import java.util.List;
 
@@ -36 +37 @@
  * @since 3.0-Beta3
  */
-public abstract class SearchAction implements ome.services.util.Executor.Work {
+public abstract class SearchAction implements Serializable,
+        ome.services.util.Executor.Work {
 
     protected final SearchValues values = new SearchValues();
@@ -46 +48 @@
         }
         this.values.copy(values);
+    }
+