From 1a7fd86ee2ea68ee6c8c855492a240135efeffc1 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Thu, 20 Feb 2014 14:37:41 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (Cloud33/4.3) to HEAD (Cloud33/4.3) 62920: Merged PLATFORM1 (Cloud33) to HEAD-BUG-FIX (Cloud33/4.3) << This was a bad merge with conflicts to do with the impl/solr directory not existing >> 62511: ACE-482: Hybrid search can be disabled/enabled and is disabled by default. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@62975 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../Search/common-opencmis-context.xml | 4 + .../Search/common-search-context.xml | 2 + .../Search/lucene/common-search.properties | 2 +- .../Search/noindex/common-search.properties | 2 +- .../Search/solr/common-search.properties | 1 + .../solr/DbOrIndexSwitchingQueryLanguage.java | 24 ++ .../impl/solr/DisabledFeatureException.java | 34 +++ .../repo/search/SearchServiceTest.java | 23 ++ .../DbOrIndexSwitchingQueryLanguageTest.java | 224 ++++++++++++++++++ .../repo/search/impl/solr/TestNode.java | 111 +++++++++ 10 files changed, 425 insertions(+), 2 deletions(-) create mode 100644 source/java/org/alfresco/repo/search/impl/solr/DisabledFeatureException.java create mode 100644 source/test-java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguageTest.java create mode 100644 source/test-java/org/alfresco/repo/search/impl/solr/TestNode.java diff --git a/config/alfresco/subsystems/Search/common-opencmis-context.xml b/config/alfresco/subsystems/Search/common-opencmis-context.xml index bf97c17666..b2a3b19b60 100644 --- a/config/alfresco/subsystems/Search/common-opencmis-context.xml +++ b/config/alfresco/subsystems/Search/common-opencmis-context.xml @@ -22,6 +22,8 @@ ${solr.query.cmis.queryConsistency} + + @@ -42,6 +44,8 @@ ${solr.query.cmis.queryConsistency} + + diff --git a/config/alfresco/subsystems/Search/common-search-context.xml b/config/alfresco/subsystems/Search/common-search-context.xml index 47ef8ceac5..bbb1bd4656 100644 --- a/config/alfresco/subsystems/Search/common-search-context.xml +++ b/config/alfresco/subsystems/Search/common-search-context.xml @@ -84,6 +84,8 @@ ${solr.query.fts.queryConsistency} + + diff --git a/config/alfresco/subsystems/Search/lucene/common-search.properties b/config/alfresco/subsystems/Search/lucene/common-search.properties index 45985a0c65..5b8746fda2 100644 --- a/config/alfresco/subsystems/Search/lucene/common-search.properties +++ b/config/alfresco/subsystems/Search/lucene/common-search.properties @@ -1,4 +1,4 @@ search.solrTrackingSupport.enabled=true solr.query.fts.queryConsistency=TRANSACTIONAL_IF_POSSIBLE solr.query.cmis.queryConsistency=TRANSACTIONAL_IF_POSSIBLE - +solr.query.hybrid.enabled=false diff --git a/config/alfresco/subsystems/Search/noindex/common-search.properties b/config/alfresco/subsystems/Search/noindex/common-search.properties index 5e6e86743b..52268a1c75 100644 --- a/config/alfresco/subsystems/Search/noindex/common-search.properties +++ b/config/alfresco/subsystems/Search/noindex/common-search.properties @@ -1,4 +1,4 @@ search.solrTrackingSupport.enabled=true solr.query.fts.queryConsistency=TRANSACTIONAL_IF_POSSIBLE solr.query.cmis.queryConsistency=TRANSACTIONAL_IF_POSSIBLE - +solr.query.hybrid.enabled=false diff --git a/config/alfresco/subsystems/Search/solr/common-search.properties b/config/alfresco/subsystems/Search/solr/common-search.properties index 5efbff847a..22344e1472 100644 --- a/config/alfresco/subsystems/Search/solr/common-search.properties +++ b/config/alfresco/subsystems/Search/solr/common-search.properties @@ -1,3 +1,4 @@ search.solrTrackingSupport.enabled=true solr.query.fts.queryConsistency=TRANSACTIONAL_IF_POSSIBLE solr.query.cmis.queryConsistency=TRANSACTIONAL_IF_POSSIBLE +solr.query.hybrid.enabled=false \ No newline at end of file diff --git a/source/java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguage.java b/source/java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguage.java index 7e69688081..2f5002ef89 100644 --- a/source/java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguage.java +++ b/source/java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguage.java @@ -41,6 +41,11 @@ public class DbOrIndexSwitchingQueryLanguage extends AbstractLuceneQueryLanguage LuceneQueryLanguageSPI indexQueryLanguage; QueryConsistency queryConsistency = QueryConsistency.DEFAULT; + private NodeService nodeService; + + private SOLRDAO solrDao; + + private boolean hybridEnabled; /** * @param dbQueryLanguage the dbQueryLanguage to set @@ -70,7 +75,20 @@ public class DbOrIndexSwitchingQueryLanguage extends AbstractLuceneQueryLanguage this.queryConsistency = queryConsistency; } + public void setNodeService(NodeService nodeService) + { + this.nodeService = nodeService; + } + public void setSolrDao(SOLRDAO solrDao) + { + this.solrDao = solrDao; + } + + public void setHybridEnabled(boolean hybridEnabled) + { + this.hybridEnabled = hybridEnabled; + } public ResultSet executeQuery(SearchParameters searchParameters, ADMLuceneSearcherImpl admLuceneSearcher) { @@ -108,6 +126,12 @@ public class DbOrIndexSwitchingQueryLanguage extends AbstractLuceneQueryLanguage { throw new QueryModelException("No query language available"); } + case HYBRID: + if (!hybridEnabled) + { + throw new DisabledFeatureException("Hybrid query is disabled."); + } + return executeHybridQuery(searchParameters, admLuceneSearcher); case DEFAULT: case TRANSACTIONAL_IF_POSSIBLE: default: diff --git a/source/java/org/alfresco/repo/search/impl/solr/DisabledFeatureException.java b/source/java/org/alfresco/repo/search/impl/solr/DisabledFeatureException.java new file mode 100644 index 0000000000..4c53f099ba --- /dev/null +++ b/source/java/org/alfresco/repo/search/impl/solr/DisabledFeatureException.java @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.repo.search.impl.solr; + +/** + * Identifies an attempt to use a disabled feature. + * + * @author Matt Ward + */ +public class DisabledFeatureException extends RuntimeException +{ + private static final long serialVersionUID = 1L; + + DisabledFeatureException(String message) + { + super(message); + } +} \ No newline at end of file diff --git a/source/test-java/org/alfresco/repo/search/SearchServiceTest.java b/source/test-java/org/alfresco/repo/search/SearchServiceTest.java index af9d71e8c3..0f186a5d1c 100644 --- a/source/test-java/org/alfresco/repo/search/SearchServiceTest.java +++ b/source/test-java/org/alfresco/repo/search/SearchServiceTest.java @@ -23,6 +23,7 @@ import javax.transaction.UserTransaction; import junit.framework.TestCase; import org.alfresco.model.ContentModel; +import org.alfresco.repo.search.impl.solr.DisabledFeatureException; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.MutableAuthenticationDao; @@ -32,6 +33,7 @@ import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.search.LimitBy; import org.alfresco.service.cmr.search.PermissionEvaluationMode; +import org.alfresco.service.cmr.search.QueryConsistency; import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.search.SearchParameters; import org.alfresco.service.cmr.search.SearchService; @@ -161,6 +163,27 @@ public class SearchServiceTest extends TestCase super.tearDown(); } + public void testHybridDisabledByDefault() + { + try + { + authenticationComponent.setCurrentUser(AuthenticationUtil.getAdminUserName()); + SearchParameters sp = new SearchParameters(); + sp.setQueryConsistency(QueryConsistency.HYBRID); + sp.setLanguage(SearchService.LANGUAGE_CMIS_ALFRESCO); + sp.setQuery("select * from cmis:document where cmis:name like '%alfresco%'"); + sp.addStore(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE); + + pubSearchService.query(sp); + + fail("Hybrid search should be disabled."); + } + catch (DisabledFeatureException e) + { + // Got here, good. + } + } + public void testAdmim() { authenticationComponent.setCurrentUser(AuthenticationUtil.getAdminUserName()); diff --git a/source/test-java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguageTest.java b/source/test-java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguageTest.java new file mode 100644 index 0000000000..ce6e606d48 --- /dev/null +++ b/source/test-java/org/alfresco/repo/search/impl/solr/DbOrIndexSwitchingQueryLanguageTest.java @@ -0,0 +1,224 @@ +package org.alfresco.repo.search.impl.solr; + +import static org.junit.Assert.*; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.alfresco.model.ContentModel; +import org.alfresco.repo.domain.node.Node; +import org.alfresco.repo.domain.solr.SOLRDAO; +import org.alfresco.repo.search.impl.lucene.ADMLuceneSearcherImpl; +import org.alfresco.repo.search.impl.lucene.LuceneQueryLanguageSPI; +import org.alfresco.repo.search.impl.lucene.SolrJSONResultSet; +import org.alfresco.repo.search.impl.querymodel.QueryModelException; +import org.alfresco.repo.solr.NodeParameters; +import org.alfresco.service.cmr.repository.ChildAssociationRef; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.search.QueryConsistency; +import org.alfresco.service.cmr.search.ResultSet; +import org.alfresco.service.cmr.search.SearchParameters; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class DbOrIndexSwitchingQueryLanguageTest +{ + private DbOrIndexSwitchingQueryLanguage queryLang; + private SearchParameters searchParameters; + private ADMLuceneSearcherImpl admLuceneSearcher; + private @Mock LuceneQueryLanguageSPI dbQueryLang; + private @Mock LuceneQueryLanguageSPI indexQueryLang; + private @Mock SolrJSONResultSet indexResults; + private @Mock ResultSet dbResults; + private @Mock SOLRDAO solrDAO; + private List changedNodes; + + @Before + public void setUp() throws Exception + { + queryLang = new DbOrIndexSwitchingQueryLanguage(); + queryLang.setDbQueryLanguage(dbQueryLang); + queryLang.setIndexQueryLanguage(indexQueryLang); + queryLang.setSolrDao(solrDAO); + searchParameters = new SearchParameters(); + changedNodes = new ArrayList<>(); + + // By default, tests will have hybrid enabled. + queryLang.setHybridEnabled(true); + } + + @Test + public void hybridSearch() + { + when(indexQueryLang.executeQuery(argThat(isSearchParamsSinceTxId(null)), eq(admLuceneSearcher))).thenReturn(indexResults); + when(indexResults.getLastIndexedTxId()).thenReturn(80L); + when(dbQueryLang.executeQuery(argThat(isSearchParamsSinceTxId(80L)), eq(admLuceneSearcher))).thenReturn(dbResults); + when(solrDAO.getNodes(argThat(isNodeParamsFromTxnId(81L)))).thenReturn(changedNodes); + + searchParameters.setQueryConsistency(QueryConsistency.HYBRID); + + // These results will come back from the SOLR query. + List indexRefs = new ArrayList<>(); + indexRefs.add(childAssoc("Car1")); + indexRefs.add(childAssoc("Car2")); + indexRefs.add(childAssoc("Car3")); + indexRefs.add(childAssoc("Car4")); + when(indexResults.getChildAssocRefs()).thenReturn(indexRefs); + + // These results will come back from the DB query. + List dbRefs = new ArrayList<>(); + dbRefs.add(childAssoc("Car1")); // Updated node, so also in index + dbRefs.add(childAssoc("Car5")); + dbRefs.add(childAssoc("Car6")); + when(dbResults.getChildAssocRefs()).thenReturn(dbRefs); + + // Nodes that have changed since last SOLR index. + // includes nodes that will come back from the DB query, plus deleted nodes. + changedNodes.add(node("Car1")); + changedNodes.add(node("Car5")); + changedNodes.add(node("Car6")); + changedNodes.add(node("Car4")); // Deleted node - not in the DB query results. + + // Execute the hybrid query. + ResultSet results = queryLang.executeQuery(searchParameters, admLuceneSearcher); + + // Check that the results have come back and that the are merged/de-duped. + assertEquals(5, results.length()); + + // NOTE: No assertion of ordering is currently present. + // TODO: ordering? + assertTrue(results.getChildAssocRefs().contains(childAssoc("Car1"))); + assertTrue(results.getChildAssocRefs().contains(childAssoc("Car2"))); + assertTrue(results.getChildAssocRefs().contains(childAssoc("Car3"))); + assertTrue(results.getChildAssocRefs().contains(childAssoc("Car5"))); + assertTrue(results.getChildAssocRefs().contains(childAssoc("Car6"))); + } + + @Test(expected=QueryModelException.class) + public void hybridSearchWhenNoQueryLanguageAvailable() + { + searchParameters.setQueryConsistency(QueryConsistency.HYBRID); + queryLang.setIndexQueryLanguage(null); + queryLang.setDbQueryLanguage(null); + + queryLang.executeQuery(searchParameters, admLuceneSearcher); + } + + @Test(expected=QueryModelException.class) + public void hybridSearchWhenNoDBLanguageAvailable() + { + searchParameters.setQueryConsistency(QueryConsistency.HYBRID); + queryLang.setDbQueryLanguage(null); + + queryLang.executeQuery(searchParameters, admLuceneSearcher); + } + + @Test(expected=QueryModelException.class) + public void hybridSearchWhenNoIndexLanguageAvailable() + { + searchParameters.setQueryConsistency(QueryConsistency.HYBRID); + queryLang.setIndexQueryLanguage(null); + + queryLang.executeQuery(searchParameters, admLuceneSearcher); + } + + @Test(expected=DisabledFeatureException.class) + public void canDisableHybridSearch() + { + queryLang.setHybridEnabled(false); + searchParameters.setQueryConsistency(QueryConsistency.HYBRID); + queryLang.executeQuery(searchParameters, admLuceneSearcher); + } + + /** + * Custom matcher for SearchParameters having a particular value + * for the property sinceTxId. + * + * @param sinceTxId The value to match, may be null. + * @return Matcher capable of checking for SearchParameters with the specified TX ID parameter. + */ + private Matcher isSearchParamsSinceTxId(final Long sinceTxId) + { + return new BaseMatcher() + { + @Override + public void describeTo(Description description) + { + description.appendText(SearchParameters.class.getSimpleName()+"[sinceTxId="+sinceTxId+"]"); + } + + @Override + public boolean matches(Object item) + { + if (!(item instanceof SearchParameters)) + { + return false; + } + SearchParameters sp = (SearchParameters) item; + if (sinceTxId == null) + { + return sp.getSinceTxId() == null; + } + else + { + return sinceTxId.equals(sp.getSinceTxId()); + } + } + }; + } + + private Matcher isNodeParamsFromTxnId(final Long fromTxnId) + { + return new BaseMatcher() + { + @Override + public void describeTo(Description description) + { + description.appendText(NodeParameters.class.getSimpleName()+"[fromTxId="+fromTxnId+"]"); + } + + @Override + public boolean matches(Object item) + { + if (!(item instanceof NodeParameters)) + { + return false; + } + NodeParameters np = (NodeParameters) item; + if (fromTxnId == null) + { + return np.getFromTxnId() == null; + } + else + { + return fromTxnId.equals(np.getFromTxnId()); + } + } + }; + } + + private ChildAssociationRef childAssoc(String id) + { + return new ChildAssociationRef( + ContentModel.ASSOC_CONTAINS, + new NodeRef("test://store/parentRef"), + ContentModel.TYPE_CONTENT, + new NodeRef("test://store/" + id) + ); + } + + private Node node(String id) + { + return new TestNode(id); + } +} diff --git a/source/test-java/org/alfresco/repo/search/impl/solr/TestNode.java b/source/test-java/org/alfresco/repo/search/impl/solr/TestNode.java new file mode 100644 index 0000000000..03c29c0726 --- /dev/null +++ b/source/test-java/org/alfresco/repo/search/impl/solr/TestNode.java @@ -0,0 +1,111 @@ +package org.alfresco.repo.search.impl.solr; + +import org.alfresco.repo.domain.node.AuditablePropertiesEntity; +import org.alfresco.repo.domain.node.Node; +import org.alfresco.repo.domain.node.NodeVersionKey; +import org.alfresco.repo.domain.node.StoreEntity; +import org.alfresco.repo.domain.node.TransactionEntity; +import org.alfresco.repo.domain.qname.QNameDAO; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeRef.Status; +import org.alfresco.util.Pair; + +class TestNode implements Node +{ + NodeRef nodeRef; + + TestNode(String id) + { + nodeRef = new NodeRef("test://store/" + id); + } + + @Override + public Long getId() + { + return null; + } + + @Override + public Long getAclId() + { + return null; + } + + @Override + public NodeVersionKey getNodeVersionKey() + { + return null; + } + + @Override + public void lock() + { + } + + @Override + public NodeRef getNodeRef() + { + return nodeRef; + } + + @Override + public Status getNodeStatus(QNameDAO qnameDAO) + { + return null; + } + + @Override + public Pair getNodePair() + { + return null; + } + + @Override + public boolean getDeleted(QNameDAO qnameDAO) + { + return false; + } + + @Override + public Long getVersion() + { + return null; + } + + @Override + public StoreEntity getStore() + { + return null; + } + + @Override + public String getUuid() + { + return null; + } + + @Override + public Long getTypeQNameId() + { + return null; + } + + @Override + public Long getLocaleId() + { + return null; + } + + @Override + public TransactionEntity getTransaction() + { + return null; + } + + @Override + public AuditablePropertiesEntity getAuditableProperties() + { + return null; + } + +}