From 11a8f2804410a19913aac22c3026e0b1fa32e31f Mon Sep 17 00:00:00 2001 From: Neil McErlean Date: Mon, 29 Dec 2014 21:23:13 +0000 Subject: [PATCH] This is a tentative fix for ACE-3671. It's being checked in in order to push one build through bamboo. The fix was to remove the bootstrap patch for the solrFacetsRootFolder created within Data Dictionary. We are lucky with this feature in that any time this folder is accessed, it either happens: 1. within a write transaction and so we can lazy-create the folder 2. within a read transaction where the data being returned by the SolrFacetService can be easily simulated. So we now lazily create the folder when it is first used - reusing the importer bootstrap/view data that the patch had used. AFAICT, this will create the folder (in the default tenant) during startup and also when any search admin tries to create a new SOLR filter within a non-default tenant. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@92799 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/import-export-context.xml | 6 - .../alfresco/patch/patch-services-context.xml | 28 +-- config/alfresco/solr-facets-context.xml | 14 +- .../patch/impl/GenericBootstrapPatch.java | 2 +- .../impl/solr/facet/SolrFacetServiceImpl.java | 209 ++++++++++++++---- .../impl/solr/facet/SolrFacetTestSuite.java | 1 + 6 files changed, 181 insertions(+), 79 deletions(-) diff --git a/config/alfresco/import-export-context.xml b/config/alfresco/import-export-context.xml index 992ec8616b..3f80e4bb93 100644 --- a/config/alfresco/import-export-context.xml +++ b/config/alfresco/import-export-context.xml @@ -669,12 +669,6 @@ alfresco/bootstrap/downloadsSpace.xml alfresco/messages/bootstrap-spaces - - - ${solr_facets.root.path} - alfresco/bootstrap/solrFacetsRootFolder.xml - alfresco/messages/bootstrap-spaces - diff --git a/config/alfresco/patch/patch-services-context.xml b/config/alfresco/patch/patch-services-context.xml index c23d382a29..94f7d4aabf 100644 --- a/config/alfresco/patch/patch-services-context.xml +++ b/config/alfresco/patch/patch-services-context.xml @@ -1020,33 +1020,7 @@ - - - patch.solrFacets.root - patch.solrFacets.root.description - 0 - 8002 - 8003 - - - - - - - - - - ${solr_facets.root} - - - - ${solr_facets.root.path} - alfresco/bootstrap/solrFacetsRootFolder.xml - alfresco/messages/bootstrap-spaces - - - - + diff --git a/config/alfresco/solr-facets-context.xml b/config/alfresco/solr-facets-context.xml index 8b99db418c..a1304381d5 100644 --- a/config/alfresco/solr-facets-context.xml +++ b/config/alfresco/solr-facets-context.xml @@ -29,11 +29,21 @@ + - - ${solr_facets.root} + + + + + + + + ${solr_facets.root.path} + alfresco/bootstrap/solrFacetsRootFolder.xml + alfresco/messages/bootstrap-spaces + diff --git a/source/java/org/alfresco/repo/admin/patch/impl/GenericBootstrapPatch.java b/source/java/org/alfresco/repo/admin/patch/impl/GenericBootstrapPatch.java index 4e83d7a262..fcd561dfa8 100644 --- a/source/java/org/alfresco/repo/admin/patch/impl/GenericBootstrapPatch.java +++ b/source/java/org/alfresco/repo/admin/patch/impl/GenericBootstrapPatch.java @@ -108,7 +108,7 @@ public class GenericBootstrapPatch extends AbstractPatch } else if (results.size() == 1) { - // nothing to do - it exsists + // nothing to do - it exists return I18NUtil.getMessage(MSG_EXISTS, checkPath); } diff --git a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImpl.java b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImpl.java index 1d35632a5c..b933074d36 100644 --- a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImpl.java +++ b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImpl.java @@ -19,6 +19,8 @@ package org.alfresco.repo.search.impl.solr.facet; +import static org.alfresco.repo.security.authentication.AuthenticationUtil.getSystemUserName; + import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; @@ -30,6 +32,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Properties; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -39,6 +42,8 @@ import java.util.concurrent.ConcurrentMap; import org.alfresco.model.ContentModel; import org.alfresco.repo.cache.SimpleCache; import org.alfresco.repo.dictionary.Facetable; +import org.alfresco.repo.importer.ImporterBootstrap; +import org.alfresco.repo.model.Repository; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.node.NodeServicePolicies.BeforeDeleteNodePolicy; import org.alfresco.repo.node.NodeServicePolicies.OnCreateNodePolicy; @@ -65,6 +70,7 @@ import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; +import org.alfresco.service.namespace.RegexQNamePattern; import org.alfresco.util.ParameterCheck; import org.alfresco.util.collections.CollectionUtils; import org.apache.commons.logging.Log; @@ -78,9 +84,10 @@ import org.springframework.extensions.surf.util.AbstractLifecycleBean; * @author Jamal Kaabi-Mofrad * @since 5.0 */ -public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrFacetService, - NodeServicePolicies.OnCreateNodePolicy, - NodeServicePolicies.BeforeDeleteNodePolicy +public class SolrFacetServiceImpl extends AbstractLifecycleBean + implements SolrFacetService, + NodeServicePolicies.OnCreateNodePolicy, + NodeServicePolicies.BeforeDeleteNodePolicy { private static final Log logger = LogFactory.getLog(SolrFacetServiceImpl.class); /** @@ -102,11 +109,17 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF private BehaviourFilter behaviourFilter; private PolicyComponent policyComponent; private SolrFacetConfig facetConfig; + private Repository repositoryHelper; private String facetsRootXPath; + private String facetsRootChildName; + + private ImporterBootstrap importerBootstrap; + private Properties bootstrapView; + private SimpleCache singletonCache; // eg. for facetsHomeNodeRef private final String KEY_FACETS_HOME_NODEREF = "key.facetshome.noderef"; private SimpleCache facetNodeRefCache; // for filterID to nodeRef lookup - private ConcurrentMap defaultFacetsMap = new ConcurrentHashMap(10); + private ConcurrentMap defaultFacetsMap = new ConcurrentHashMap<>(10); /** * @param authorityService the authorityService to set @@ -172,6 +185,11 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF this.policyComponent = policyComponent; } + public void setRepositoryHelper(Repository repository) + { + this.repositoryHelper = repository; + } + /** * @param facetConfig the facetConfig to set */ @@ -188,6 +206,18 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF this.facetsRootXPath = facetsRootXPath; } + public void setFacetsRootChildName(String facetsRootChildName) + { + this.facetsRootChildName = facetsRootChildName; + } + + public void setImporterBootstrap(ImporterBootstrap importer) { this.importerBootstrap = importer; } + + public void setBootstrapView(Properties bootstrapView) + { + this.bootstrapView = bootstrapView; + } + /** * @param singletonCache the singletonCache to set */ @@ -222,25 +252,30 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF final SolrFacetComparator comparator = new SolrFacetComparator(getFacetOrder()); SortedSet result = new TreeSet<>(comparator); - List children = nodeService.getChildAssocs(getFacetsRoot()); - for (ChildAssociationRef ref : children) + final NodeRef facetsRoot = getFacetsRoot(); + if (facetsRoot != null) { - result.add(getFacetProperties(ref.getChildRef())); + for (ChildAssociationRef ref : nodeService.getChildAssocs(facetsRoot)) + { + result.add(getFacetProperties(ref.getChildRef())); + } } + // add the default filters result.addAll(defaultFacetsMap.values()); return new ArrayList<>(result); } + /** Gets the filter IDs in display order. Will not return {@code null}. */ public List getFacetOrder() { - final NodeRef facetContainer = getFacetsRoot(); + final NodeRef facetsRoot = getFacetsRoot(); - @SuppressWarnings("unchecked") - final List facetOrder = (List) nodeService.getProperty(facetContainer, SolrFacetModel.PROP_FACET_ORDER); - return facetOrder; + return facetsRoot == null ? + new ArrayList<>(facetConfig.getDefaultFacets().keySet()) : + (List)nodeService.getProperty(facetsRoot, SolrFacetModel.PROP_FACET_ORDER); } @Override @@ -274,25 +309,33 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF { // not in cache - find and store final NodeRef facetRoot = getFacetsRoot(); - facetNodeRef = AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork() + + if (facetRoot != null) { - public NodeRef doWork() throws Exception + facetNodeRef = AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork() { - // the filterID directly maps to the cm:name property - NodeRef nodeRef = nodeService.getChildByName(facetRoot, ContentModel.ASSOC_CONTAINS, filterID); - // cache the result if found - if (nodeRef != null) + public NodeRef doWork() throws Exception { - facetNodeRefCache.put(filterID, nodeRef); + // the filterID directly maps to the cm:name property + NodeRef nodeRef = nodeService.getChildByName(facetRoot, ContentModel.ASSOC_CONTAINS, filterID); + // cache the result if found + if (nodeRef != null) + { + facetNodeRefCache.put(filterID, nodeRef); + } + return nodeRef; } - return nodeRef; - } - }, AuthenticationUtil.getSystemUserName()); + }, AuthenticationUtil.getSystemUserName()); + } } return facetNodeRef; } + /** + * Gets the {@link SolrFacetProperties} stored on the specified {@link NodeRef}. + * @throws org.alfresco.service.cmr.repository.InvalidNodeRefException if the nodeRef does not exist. + */ private SolrFacetProperties getFacetProperties(NodeRef nodeRef) { Map properties = nodeService.getProperties(nodeRef); @@ -383,10 +426,10 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF final NodeRef facetRoot = getFacetsRoot(); if (facetRoot == null) { - throw new SolrFacetConfigException("Facets root folder does not exist."); + createFacetsRootFolder(); } - return facetNodeRef = AuthenticationUtil.runAs(new RunAsWork() + return AuthenticationUtil.runAs(new RunAsWork() { @Override public NodeRef doWork() throws Exception @@ -430,17 +473,13 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF if (facetNodeRef == null) { SolrFacetProperties fp = defaultFacetsMap.get(filterID); - if (fp != null) - { - // As we don't create nodes for the bootstrapped FP on server - // startup, we need to create a node here, when a user tries to - // update the default properties for the first time. - createFacetNodeImpl(facetProperties, false); - } - else - { - throw new SolrFacetConfigException("Cannot update facet [" + filterID + "] as it does not exist."); + if (fp == null) { + createFacetsRootFolder(); } + // As we don't create nodes for the bootstrapped FP on server + // startup, we need to create a node here, when a user tries to + // update the default properties for the first time. + createFacetNodeImpl(facetProperties, false); } else { @@ -486,7 +525,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF } boolean isDefaultFP = defaultFacetsMap.containsKey(facetProperties.getFilterID()); - Map properties = new HashMap(15); + Map properties = new HashMap<>(15); properties.put(ContentModel.PROP_NAME, facetProperties.getFilterID()); properties.put(SolrFacetModel.PROP_IS_DEFAULT, isDefaultFP); @@ -529,6 +568,10 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF properties.put(qname, propValue); } + /** + * Gets the {@link NodeRef} of the {@code srft:facets} folder, if it exists. + * @return the {@link NodeRef} if it exists, else {@code null}. + */ public NodeRef getFacetsRoot() { NodeRef facetHomeRef = (NodeRef) singletonCache.get(KEY_FACETS_HOME_NODEREF); @@ -559,7 +602,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF } }, AuthenticationUtil.getSystemUserName()); - singletonCache.put(KEY_FACETS_HOME_NODEREF, facetHomeRef); + if (facetHomeRef != null) { singletonCache.put(KEY_FACETS_HOME_NODEREF, facetHomeRef); } } return facetHomeRef; } @@ -626,7 +669,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF } } - List facetOrder = getFacetOrder(); + final List facetOrder = getFacetOrder(); // Sort the merged maps Comparator> entryComparator = CollectionUtils.toEntryComparator(new SolrFacetComparator(facetOrder)); Map sortedMap = CollectionUtils.sortMapByValue(mergedMap, entryComparator); @@ -636,7 +679,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF logger.debug("The facets [" + persistedProperties + "] have overridden their matched default facets."); } - final Set newFacetOrder = (facetOrder == null) ? new LinkedHashSet(sortedMap.size()) : new LinkedHashSet(facetOrder); + final Set newFacetOrder = (facetOrder == null) ? new LinkedHashSet(sortedMap.size()) : new LinkedHashSet<>(facetOrder); for (SolrFacetProperties fp : sortedMap.values()) { @@ -652,7 +695,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF { public Void execute() throws Exception { - reorderFacets(new ArrayList(newFacetOrder)); + reorderFacets(new ArrayList<>(newFacetOrder)); return null; } }, false); @@ -665,11 +708,16 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF } } + /** Gets the persisted {@link SolrFacetProperties} if there are any, else an empty map. */ private Map getPersistedFacetProperties() { - List list = nodeService.getChildAssocs(getFacetsRoot()); + final NodeRef facetsRoot = getFacetsRoot(); + + Map facets = new HashMap<>(); + + final List list = facetsRoot == null ? + new ArrayList() : nodeService.getChildAssocs(facetsRoot); - Map facets = new HashMap<>(list.size()); for (ChildAssociationRef associationRef : list) { SolrFacetProperties fp = getFacetProperties(associationRef.getChildRef()); @@ -735,7 +783,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF */ private Map getFacetCustomProperties(Map properties) { - Map customProperties = new HashMap(5); + Map customProperties = new HashMap<>(5); for (Map.Entry entry : properties.entrySet()) { @@ -774,7 +822,7 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF final List removedFacetIds = new ArrayList<>(); for (String facetId : facetIds) { - SolrFacetProperties facet = getFacet(facetId); + final SolrFacetProperties facet = getFacet(facetId); if (facet == null) { @@ -811,9 +859,84 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF logger.info("Removed " + removedFacetIds + " from the facets' ordering list."); } } - nodeService.setProperty(getFacetsRoot(), SolrFacetModel.PROP_FACET_ORDER, serializableProp); + NodeRef facetsRoot = getFacetsRoot(); + + if (facetsRoot == null) + { + facetsRoot = createFacetsRootFolder(); + } + nodeService.setProperty(facetsRoot, SolrFacetModel.PROP_FACET_ORDER, serializableProp); } } + + private NodeRef createFacetsRootFolder() + { + return AuthenticationUtil.runAs(new RunAsWork() + { + @Override public NodeRef doWork() throws Exception + { + final NodeRef companyHome = repositoryHelper.getCompanyHome(); + final QName appModel = QName.createQName("http://www.alfresco.org/model/application/1.0", "dictionary"); + final NodeRef dataDict = getSingleChildNodeRef(companyHome, appModel); + + // The name of the child-assoc to the facets root folder. + final QName facetsRootAssocQName = QName.createQName(facetsRootChildName, namespaceService); + + NodeRef result = getSingleChildNodeRef(dataDict, facetsRootAssocQName); + + if (result == null) + { + List singletonList = new ArrayList<>(); + singletonList.add(bootstrapView); + importerBootstrap.setBootstrapViews(singletonList); + importerBootstrap.setUseExistingStore(true); + importerBootstrap.bootstrap(); + + // Now to get the NodeRef we just imported. (Not using SOLR to avoid consistency effects.) + result = getSingleChildNodeRef(dataDict, facetsRootAssocQName); + } + + return result; + } + }, getSystemUserName()); + } + + /** + * Gets a child NodeRef under the specified parent NodeRef linked by a child-assoc of the specified name. + * + * @param parent the parent whose child is sought. + * @param assocName the name of the child-association. + * @return the NodeRef of the requested child, if it exists. null if there is no match. + */ + private NodeRef getSingleChildNodeRef(NodeRef parent, QName assocName) + { + final List assocs = nodeService.getChildAssocs(parent, + RegexQNamePattern.MATCH_ALL, + assocName, true); + final NodeRef result; + + if (assocs == null || assocs.isEmpty()) + { + result = null; + } + else if (assocs.size() > 1) + { + final StringBuilder msg = new StringBuilder(); + msg.append("Expected exactly one child node at: ") + .append(parent).append("/").append(assocName) + .append(" but found ") + .append(assocs == null ? "" : assocs.size()); + if (logger.isErrorEnabled()) { logger.error(msg.toString()); } + + result = assocs.get(0).getChildRef(); + } + else + { + result = assocs.get(0).getChildRef(); + } + + return result; + } @Override public List getFacetableProperties() { diff --git a/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetTestSuite.java b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetTestSuite.java index 32f7a6b21a..cd41570580 100644 --- a/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetTestSuite.java +++ b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetTestSuite.java @@ -40,6 +40,7 @@ public class SolrFacetTestSuite extends TestSuite suite.addTest(new JUnit4TestAdapter(SolrFacetQueriesDisplayHandlersTest.class)); suite.addTest(new JUnit4TestAdapter(SolrFacetServiceImplTest.class)); suite.addTest(new JUnit4TestAdapter(SolrFacetConfigTest.class)); + suite.addTest(new JUnit4TestAdapter(SolrFacetComparatorTest.class)); return suite; }