diff --git a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetService.java b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetService.java index f38f6f81c2..4648866288 100644 --- a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetService.java +++ b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetService.java @@ -24,7 +24,6 @@ import java.util.List; import org.alfresco.repo.dictionary.Facetable; import org.alfresco.repo.search.impl.solr.facet.Exceptions.DuplicateFacetId; import org.alfresco.repo.search.impl.solr.facet.Exceptions.MissingFacetId; -import org.alfresco.repo.search.impl.solr.facet.Exceptions.UnrecognisedFacetId; import org.alfresco.service.cmr.dictionary.PropertyDefinition; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; @@ -99,7 +98,6 @@ public interface SolrFacetService * @param filterIds an ordered sequence of filter IDs. * @throws NullPointerException if filterIds is {@code null}. * @throws MissingFacetId if the list is empty. - * @throws UnrecognisedFacetId if any of the provided filter IDs are not recognised. * @throws DuplicateFacetId if there is a duplicate filter ID in the list. */ public void reorderFacets(List filterIds); 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 6dc1210fac..1d35632a5c 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 @@ -48,7 +48,6 @@ import org.alfresco.repo.policy.PolicyComponent; import org.alfresco.repo.search.impl.solr.facet.Exceptions.DuplicateFacetId; import org.alfresco.repo.search.impl.solr.facet.Exceptions.IllegalArgument; import org.alfresco.repo.search.impl.solr.facet.Exceptions.MissingFacetId; -import org.alfresco.repo.search.impl.solr.facet.Exceptions.UnrecognisedFacetId; import org.alfresco.repo.search.impl.solr.facet.SolrFacetProperties.CustomProperties; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; @@ -588,8 +587,44 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF // Persisted facets Map persistedProperties = getPersistedFacetProperties(); - // The persisted facets will override the default facets - mergedMap.putAll(persistedProperties); + for(Entry entry : persistedProperties.entrySet()) + { + final String facetId = entry.getKey(); + /* + * If the default facet has been removed from the config file and + * the facet was persisted as its property was modified, then, the + * persisted node needs to be deleted. This should be done to avoid + * errors when loading the facets. Also, as all the properties of + * the facet may not have been persisted and the default facet + * doesn't exist anymore, there is no way of merging the + * non-persisted properties. + */ + if(entry.getValue().isDefault() && !defaultFP.containsKey(facetId)) + { + AuthenticationUtil.runAs(new RunAsWork() + { + @Override + public Void doWork() throws Exception + { + return retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback() + { + public Void execute() throws Exception + { + deleteFacet(facetId); + + logger.info("Deleted [" + facetId + "] node, as the filter has been removed from the config file!"); + return null; + } + }, false); + } + }, AuthenticationUtil.getSystemUserName()); + } + else + { + // The persisted facets will override the default facets + mergedMap.put(facetId, entry.getValue()); + } + } List facetOrder = getFacetOrder(); // Sort the merged maps @@ -736,13 +771,16 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF final List existingFacets = getFacets(); final Map sortedFacets = new LinkedHashMap<>(); // maintains insertion order + final List removedFacetIds = new ArrayList<>(); for (String facetId : facetIds) { SolrFacetProperties facet = getFacet(facetId); if (facet == null) { - throw new UnrecognisedFacetId("Cannot reorder facets as ID not recognised:", facetId); + // ACE-3083 + logger.warn("Facet with [" + facetId + "] ID does not exist. Removing it from the facets' ordering list"); + removedFacetIds.add(facetId); } else if (sortedFacets.containsKey(facetId)) { @@ -765,6 +803,14 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF // The alternative is changing the service API to look like > // which is a bit verbose for an API. ArrayList serializableProp = new ArrayList<>(facetIds); + if (removedFacetIds.size() > 0) + { + boolean result = serializableProp.removeAll(removedFacetIds); + if (result) + { + logger.info("Removed " + removedFacetIds + " from the facets' ordering list."); + } + } nodeService.setProperty(getFacetsRoot(), SolrFacetModel.PROP_FACET_ORDER, serializableProp); } } diff --git a/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImplTest.java b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImplTest.java index 0808bee27b..5a4ea5a0a6 100644 --- a/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImplTest.java @@ -163,17 +163,23 @@ public class SolrFacetServiceImplTest }); } - @Test(expected=UnrecognisedFacetId.class) + @Test public void reorderUnrecognisedFacetIdsShouldFail() throws Exception { TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() { @Override public Void execute() throws Throwable { - final List facetIds = getExistingFacetIds(); + final List existingFacetIds = getExistingFacetIds(); + + final List facetIds = new ArrayList<>(existingFacetIds); facetIds.add("unrecognisedID"); - + SOLR_FACET_SERVICE.reorderFacets(facetIds); + + final List newfacetIds = getExistingFacetIds(); + + assertEquals(existingFacetIds, newfacetIds); return null; }