WAVE-311 Refactors some search related code.
Project: http://git-wip-us.apache.org/repos/asf/incubator-wave/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-wave/commit/d566b91d Tree: http://git-wip-us.apache.org/repos/asf/incubator-wave/tree/d566b91d Diff: http://git-wip-us.apache.org/repos/asf/incubator-wave/diff/d566b91d Branch: refs/heads/fulltextsearch Commit: d566b91dffd3554af7295b373458a097ad3555cb Parents: b45dc71 Author: Yuri Zelikov <[email protected]> Authored: Tue Apr 15 12:32:08 2014 +0300 Committer: Yuri Zelikov <[email protected]> Committed: Wed Aug 27 19:17:55 2014 +0300 ---------------------------------------------------------------------- .../waveserver/AbstractSearchProviderImpl.java | 55 +++++++++++-- .../waveserver/SimpleSearchProviderImpl.java | 54 +++++-------- .../waveserver/SolrSearchProviderImpl.java | 84 +++++++------------- .../box/server/waveserver/WaveDigester.java | 14 ++-- 4 files changed, 105 insertions(+), 102 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-wave/blob/d566b91d/src/org/waveprotocol/box/server/waveserver/AbstractSearchProviderImpl.java ---------------------------------------------------------------------- diff --git a/src/org/waveprotocol/box/server/waveserver/AbstractSearchProviderImpl.java b/src/org/waveprotocol/box/server/waveserver/AbstractSearchProviderImpl.java index e5544fe..f3d18c2 100644 --- a/src/org/waveprotocol/box/server/waveserver/AbstractSearchProviderImpl.java +++ b/src/org/waveprotocol/box/server/waveserver/AbstractSearchProviderImpl.java @@ -24,11 +24,14 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; +import org.waveprotocol.box.server.util.WaveletDataUtil; +import org.waveprotocol.wave.model.id.IdUtil; import org.waveprotocol.wave.model.id.WaveId; import org.waveprotocol.wave.model.id.WaveletId; import org.waveprotocol.wave.model.id.WaveletName; import org.waveprotocol.wave.model.wave.ParticipantId; import org.waveprotocol.wave.model.wave.ParticipantIdUtil; +import org.waveprotocol.wave.model.wave.data.ObservableWaveletData; import org.waveprotocol.wave.model.wave.data.ReadableWaveletData; import org.waveprotocol.wave.model.wave.data.WaveViewData; import org.waveprotocol.wave.model.wave.data.impl.WaveViewDataImpl; @@ -36,8 +39,8 @@ import org.waveprotocol.wave.util.logging.Log; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; /** * Base implementation of search provider. @@ -57,7 +60,7 @@ public abstract class AbstractSearchProviderImpl implements SearchProvider { this.waveMap = waveMap; } - protected Collection<WaveViewData> computeSearchResult(final ParticipantId user, int startAt, + protected List<WaveViewData> computeSearchResult(final ParticipantId user, int startAt, int numResults, List<WaveViewData> results) { int searchResultSize = results.size(); // Check if we have enough results to return. @@ -69,17 +72,25 @@ public abstract class AbstractSearchProviderImpl implements SearchProvider { } } - protected Map<WaveId, WaveViewData> filterWavesViewBySearchCriteria( + protected LinkedHashMap<WaveId, WaveViewData> filterWavesViewBySearchCriteria( Function<ReadableWaveletData, Boolean> matchesFunction, Multimap<WaveId, WaveletId> currentUserWavesView) { // Must use a map with stable ordering, since indices are meaningful. - Map<WaveId, WaveViewData> results = Maps.newLinkedHashMap(); + LinkedHashMap<WaveId, WaveViewData> results = Maps.newLinkedHashMap(); // Loop over the user waves view. for (WaveId waveId : currentUserWavesView.keySet()) { Collection<WaveletId> waveletIds = currentUserWavesView.get(waveId); WaveViewData view = buildWaveViewData(waveId, waveletIds, matchesFunction, waveMap); - if (view != null) { + Iterable<? extends ObservableWaveletData> wavelets = view.getWavelets(); + boolean hasConversation = false; + for (ObservableWaveletData waveletData : wavelets) { + if (IdUtil.isConversationalId(waveletData.getWaveletId())) { + hasConversation = true; + break; + } + } + if ((view != null) && hasConversation) { results.put(waveId, view); } } @@ -127,7 +138,7 @@ public abstract class AbstractSearchProviderImpl implements SearchProvider { // look at the user's wave view and determine if the view matches the // query. try { - if (waveletContainer == null || !waveletContainer.applyFunction(matchesFunction)) { + if ((waveletContainer == null) || !waveletContainer.applyFunction(matchesFunction)) { LOG.fine("----doesn't match: " + waveletContainer); continue; } @@ -142,4 +153,36 @@ public abstract class AbstractSearchProviderImpl implements SearchProvider { } return view; } + + /** + * Verifies whether the wavelet matches the filter criteria. + * + * @param wavelet the wavelet. + * @param user the logged in user. + * @param sharedDomainParticipantId the shared domain participant id. + * @param isAllQuery true if the search results should include shared for this + * domain waves. + */ + protected boolean isWaveletMatchesCriteria(ReadableWaveletData wavelet, ParticipantId user, + ParticipantId sharedDomainParticipantId, boolean isAllQuery) + throws WaveletStateException { + // If it is user data wavelet for the user - return true. + if (IdUtil.isUserDataWavelet(wavelet.getWaveletId()) && wavelet.getCreator().equals(user)) { + return true; + } + // The wavelet should have logged in user as participant for 'in:inbox' + // query. + if (!isAllQuery && !wavelet.getParticipants().contains(user)) { + return false; + } + // Or if it is an 'all' query - then either logged in user or shared domain + // participant should be present in the wave. + if (isAllQuery + && !WaveletDataUtil.checkAccessPermission(wavelet, user, sharedDomainParticipantId)) { + return false; + } + // If not returned 'false' above - then logged in user is either + // explicit or implicit participant and therefore has access permission. + return true; + } } http://git-wip-us.apache.org/repos/asf/incubator-wave/blob/d566b91d/src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java ---------------------------------------------------------------------- diff --git a/src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java b/src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java index ea75231..917a3b8 100644 --- a/src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java +++ b/src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java @@ -28,9 +28,7 @@ import com.google.inject.name.Named; import com.google.wave.api.SearchResult; import org.waveprotocol.box.server.CoreSettings; -import org.waveprotocol.box.server.util.WaveletDataUtil; import org.waveprotocol.box.server.waveserver.QueryHelper.InvalidQueryException; -import org.waveprotocol.wave.model.id.IdUtil; import org.waveprotocol.wave.model.id.WaveId; import org.waveprotocol.wave.model.id.WaveletId; import org.waveprotocol.wave.model.id.WaveletName; @@ -66,7 +64,7 @@ public class SimpleSearchProviderImpl extends AbstractSearchProviderImpl { @Override public SearchResult search(final ParticipantId user, String query, int startAt, int numResults) { LOG.fine("Search query '" + query + "' from user: " + user + " [" + startAt + ", " - + (startAt + numResults - 1) + "]"); + + ((startAt + numResults) - 1) + "]"); Map<TokenQueryType, Set<String>> queryParams = null; try { queryParams = QueryHelper.parseQuery(query); @@ -139,19 +137,19 @@ public class SimpleSearchProviderImpl extends AbstractSearchProviderImpl { Function<ReadableWaveletData, Boolean> matchesFunction = new Function<ReadableWaveletData, Boolean>() { - @Override - public Boolean apply(ReadableWaveletData wavelet) { - try { - return isWaveletMatchesCriteria(wavelet, user, sharedDomainParticipantId, - withParticipantIds, creatorParticipantIds, isAllQuery); - } catch (WaveletStateException e) { - LOG.warning( - "Failed to access wavelet " - + WaveletName.of(wavelet.getWaveId(), wavelet.getWaveletId()), e); - return false; - } - } - }; + @Override + public Boolean apply(ReadableWaveletData wavelet) { + try { + return isWaveletMatchesCriteria(wavelet, user, sharedDomainParticipantId, + withParticipantIds, creatorParticipantIds, isAllQuery); + } catch (WaveletStateException e) { + LOG.warning( + "Failed to access wavelet " + + WaveletName.of(wavelet.getWaveId(), wavelet.getWaveletId()), e); + return false; + } + } + }; return matchesFunction; } @@ -166,13 +164,9 @@ public class SimpleSearchProviderImpl extends AbstractSearchProviderImpl { * @param isAllQuery true if the search results should include shared for this * domain waves. */ - private boolean isWaveletMatchesCriteria(ReadableWaveletData wavelet, ParticipantId user, + protected boolean isWaveletMatchesCriteria(ReadableWaveletData wavelet, ParticipantId user, ParticipantId sharedDomainParticipantId, List<ParticipantId> withList, List<ParticipantId> creatorList, boolean isAllQuery) throws WaveletStateException { - // If it is user data wavelet for the user - return true. - if (IdUtil.isUserDataWavelet(wavelet.getWaveletId()) && wavelet.getCreator().equals(user)) { - return true; - } // Filter by creator. This is the fastest check so we perform it first. for (ParticipantId creator : creatorList) { if (!creator.equals(wavelet.getCreator())) { @@ -180,20 +174,8 @@ public class SimpleSearchProviderImpl extends AbstractSearchProviderImpl { return false; } } - // The wavelet should have logged in user as participant for 'in:inbox' - // query. - if (!isAllQuery && !wavelet.getParticipants().contains(user)) { - return false; - } - // Or if it is an 'all' query - then either logged in user or shared domain - // participant should be present in the wave. - if (isAllQuery - && !WaveletDataUtil.checkAccessPermission(wavelet, user, sharedDomainParticipantId)) { - return false; - } - // If not returned 'false' above - then logged in user is either - // explicit or implicit participant and therefore has access permission. - + boolean matches = + super.isWaveletMatchesCriteria(wavelet, user, sharedDomainParticipantId, isAllQuery); // Now filter by 'with'. for (ParticipantId otherUser : withList) { if (!wavelet.getParticipants().contains(otherUser)) { @@ -201,7 +183,7 @@ public class SimpleSearchProviderImpl extends AbstractSearchProviderImpl { return false; } } - return true; + return matches; } private List<WaveViewData> sort(Map<TokenQueryType, Set<String>> queryParams, http://git-wip-us.apache.org/repos/asf/incubator-wave/blob/d566b91d/src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java ---------------------------------------------------------------------- diff --git a/src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java b/src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java index 2ed82e3..fb45f3f 100644 --- a/src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java +++ b/src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java @@ -20,7 +20,8 @@ package org.waveprotocol.box.server.waveserver; import com.google.common.base.Function; -import com.google.common.collect.HashMultimap; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.gson.JsonArray; import com.google.gson.JsonElement; @@ -37,6 +38,7 @@ import org.apache.http.HttpStatus; import org.waveprotocol.box.server.CoreSettings; import org.waveprotocol.wave.model.id.WaveId; import org.waveprotocol.wave.model.id.WaveletId; +import org.waveprotocol.wave.model.id.WaveletName; import org.waveprotocol.wave.model.wave.ParticipantId; import org.waveprotocol.wave.model.wave.data.ReadableWaveletData; import org.waveprotocol.wave.model.wave.data.WaveViewData; @@ -44,11 +46,9 @@ import org.waveprotocol.wave.util.logging.Log; import java.io.IOException; import java.io.InputStreamReader; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; -import java.util.List; +import java.util.LinkedHashMap; import java.util.Map; import java.util.regex.Pattern; @@ -57,7 +57,7 @@ import java.util.regex.Pattern; * * @author Frank R. <[email protected]> */ -public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements SearchProvider { +public class SolrSearchProviderImpl extends AbstractSearchProviderImpl { private static final Log LOG = Log.get(SolrSearchProviderImpl.class); @@ -93,7 +93,7 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements + " AND " + LMT + ":[* TO *]" // + " AND " + WITH + ":[* TO *]" // + " AND " + WITH_FUZZY + ":[* TO *]" // - + " AND " + CREATOR + ":[* TO *]" // + + " AND " + CREATOR + ":[* TO *]" /* + " AND " + TEXT + ":[* TO *]" */; /*- @@ -116,16 +116,6 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements private static final String FILTER_QUERY_PREFIX = "{!lucene q.op=AND df=" + TEXT + "}" // + WITH + ":"; - - public static Function<ReadableWaveletData, Boolean> matchesFunction = - new Function<ReadableWaveletData, Boolean>() { - - @Override - public Boolean apply(ReadableWaveletData wavelet) { - return true; - } - }; - public static String buildUserQuery(String query) { return query.replaceAll(WORD_START + TokenQueryType.IN.getToken() + ":", IN + ":") .replaceAll(WORD_START + TokenQueryType.WITH.getToken() + ":", WITH_FUZZY + ":") @@ -135,13 +125,13 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements @Inject public SolrSearchProviderImpl(WaveDigester digester, WaveMap waveMap, @Named(CoreSettings.WAVE_SERVER_DOMAIN) String waveDomain) { - super(waveDomain, digester, waveMap, null); + super(waveDomain, digester, waveMap); } @Override public SearchResult search(final ParticipantId user, String query, int startAt, int numResults) { LOG.fine("Search query '" + query + "' from user: " + user + " [" + startAt + ", " - + (startAt + numResults - 1) + "]"); + + ((startAt + numResults) - 1) + "]"); /*- * see @@ -151,7 +141,7 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements // added. final boolean isAllQuery = isAllQuery(query); - Multimap<WaveId, WaveletId> currentUserWavesView = HashMultimap.create(); + Multimap<WaveId, WaveletId> currentUserWavesView = LinkedHashMultimap.create(); if (numResults > 0) { @@ -168,7 +158,7 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements try { while (true) { getMethod.setURI(new URI(SOLR_BASE_URL + "/select?wt=json" + "&start=" + start + "&rows=" - + rows + "&q=" + Q + "&fq=" + fq, false)); + + rows + "&sort=" + LMT + "+desc" + "&q=" + Q + "&fq=" + fq, false)); HttpClient httpClient = new HttpClient(); int statusCode = httpClient.executeMethod(getMethod); @@ -179,7 +169,7 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements JsonObject json = new JsonParser().parse(new InputStreamReader(getMethod.getResponseBodyAsStream())) - .getAsJsonObject(); + .getAsJsonObject(); JsonObject responseJson = json.getAsJsonObject("response"); JsonArray docsJson = responseJson.getAsJsonArray("docs"); if (docsJson.size() == 0) { @@ -190,23 +180,12 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements while (docJsonIterator.hasNext()) { JsonObject docJson = docJsonIterator.next().getAsJsonObject(); - /* - * TODO (Frank R.) c.f. - * org.waveprotocol.box.server.waveserver.SimpleSearchProviderImpl - * .isWaveletMatchesCriteria(ReadableWaveletData, ParticipantId, - * ParticipantId, List<ParticipantId>, List<ParticipantId>, boolean) - */ - WaveId waveId = WaveId.deserialise(docJson.getAsJsonPrimitive(WAVE_ID).getAsString()); WaveletId waveletId = WaveletId.deserialise(docJson.getAsJsonPrimitive(WAVELET_ID).getAsString()); currentUserWavesView.put(waveId, waveletId); } - /* - * there won't be any more results - stop querying next page of - * results - */ if (docsJson.size() < rows) { break; } @@ -222,7 +201,23 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements } } - Map<WaveId, WaveViewData> results = + Function<ReadableWaveletData, Boolean> matchesFunction = + new Function<ReadableWaveletData, Boolean>() { + + @Override + public Boolean apply(ReadableWaveletData wavelet) { + try { + return isWaveletMatchesCriteria(wavelet, user, sharedDomainParticipantId, isAllQuery); + } catch (WaveletStateException e) { + LOG.warning( + "Failed to access wavelet " + + WaveletName.of(wavelet.getWaveId(), wavelet.getWaveletId()), e); + return false; + } + } + }; + + LinkedHashMap<WaveId, WaveViewData> results = filterWavesViewBySearchCriteria(matchesFunction, currentUserWavesView); if (LOG.isFineLoggable()) { for (Map.Entry<WaveId, WaveViewData> e : results.entrySet()) { @@ -230,7 +225,8 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements } } - Collection<WaveViewData> searchResult = computeSearchResult(user, startAt, numResults, results); + Collection<WaveViewData> searchResult = + computeSearchResult(user, startAt, numResults, Lists.newArrayList(results.values())); LOG.info("Search response to '" + query + "': " + searchResult.size() + " results, user: " + user); return digester.generateSearchResult(user, query, searchResult); @@ -257,24 +253,4 @@ public class SolrSearchProviderImpl extends SimpleSearchProviderImpl implements return fq; } - - /*- - * copied with modification from - * org.waveprotocol.box.server.waveserver.SimpleSearchProviderImpl.computeSearchResult(ParticipantId, int, int, Map<TokenQueryType, Set<String>>, Map<WaveId, WaveViewData>) - * - * removed queryParams - */ - private Collection<WaveViewData> computeSearchResult(final ParticipantId user, int startAt, - int numResults, Map<WaveId, WaveViewData> results) { - List<WaveViewData> searchResultslist = null; - int searchResultSize = results.values().size(); - // Check if we have enough results to return. - if (searchResultSize < startAt) { - searchResultslist = Collections.emptyList(); - } else { - int endAt = Math.min(startAt + numResults, searchResultSize); - searchResultslist = new ArrayList<WaveViewData>(results.values()).subList(startAt, endAt); - } - return searchResultslist; - } } http://git-wip-us.apache.org/repos/asf/incubator-wave/blob/d566b91d/src/org/waveprotocol/box/server/waveserver/WaveDigester.java ---------------------------------------------------------------------- diff --git a/src/org/waveprotocol/box/server/waveserver/WaveDigester.java b/src/org/waveprotocol/box/server/waveserver/WaveDigester.java index 25d19d6..b17b571 100644 --- a/src/org/waveprotocol/box/server/waveserver/WaveDigester.java +++ b/src/org/waveprotocol/box/server/waveserver/WaveDigester.java @@ -107,8 +107,8 @@ public class WaveDigester { root = waveletData; } else if (IdUtil.isConversationalId(waveletId)) { other = waveletData; - } else if (IdUtil.isUserDataWavelet(waveletId)) { - // assume this is the user data wavelet for the right user. + } else if (IdUtil.isUserDataWavelet(waveletId) + && waveletData.getCreator().equals(participant)) { udw = waveletData; } } @@ -148,8 +148,8 @@ public class WaveDigester { WaveletData rawWaveletData) { ObservableConversation rootConversation = conversations.getRoot(); ObservableConversationBlip firstBlip = null; - if (rootConversation != null && rootConversation.getRootThread() != null - && rootConversation.getRootThread().getFirstBlip() != null) { + if ((rootConversation != null) && (rootConversation.getRootThread() != null) + && (rootConversation.getRootThread().getFirstBlip() != null)) { firstBlip = rootConversation.getRootThread().getFirstBlip(); } String title; @@ -176,13 +176,15 @@ public class WaveDigester { } int unreadCount = 0; int blipCount = 0; + long lastModified = -1; for (ConversationBlip blip : BlipIterators.breadthFirst(rootConversation)) { if (supplement.isUnread(blip)) { unreadCount++; } + lastModified = Math.max(blip.getLastModifiedTime(), lastModified); blipCount++; } - return new Digest(title, snippet, waveId, participants, rawWaveletData.getLastModifiedTime(), + return new Digest(title, snippet, waveId, participants, lastModified, rawWaveletData.getCreationTime(), unreadCount, blipCount); } @@ -245,6 +247,6 @@ public class WaveDigester { PrimitiveSupplement udwState = udw != null ? WaveletBasedSupplement.create(OpBasedWavelet.createReadOnly(udw)) : new PrimitiveSupplementImpl(); - return SupplementedWaveImpl.create(udwState, conversations, viewer, DefaultFollow.ALWAYS); + return SupplementedWaveImpl.create(udwState, conversations, viewer, DefaultFollow.ALWAYS); } }
