diff --git a/config/alfresco/model/solrFacetModel.xml b/config/alfresco/model/solrFacetModel.xml index 663f0a1c01..17d8bea4b9 100644 --- a/config/alfresco/model/solrFacetModel.xml +++ b/config/alfresco/model/solrFacetModel.xml @@ -151,10 +151,24 @@ + Facets cm:folder + + + Ordered sequence of Facet IDs + d:text + false + true + + false + false + false + + + diff --git a/source/java/org/alfresco/repo/search/impl/solr/facet/Exceptions.java b/source/java/org/alfresco/repo/search/impl/solr/facet/Exceptions.java new file mode 100644 index 0000000000..50ad5e1c7e --- /dev/null +++ b/source/java/org/alfresco/repo/search/impl/solr/facet/Exceptions.java @@ -0,0 +1,86 @@ +/* + * 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.facet; + +import org.alfresco.error.AlfrescoRuntimeException; + +/** These exceptions are thrown by the {@link SolrFacetService}. */ +public class Exceptions extends AlfrescoRuntimeException +{ + private static final long serialVersionUID = 1L; + + // Constructors for the basic SolrFacet Exception itself. + public Exceptions() { this("", null); } + public Exceptions(String message) { this(message, null); } + public Exceptions(Throwable cause) { this("", null); } + public Exceptions(String message, Throwable cause) { super(message, cause); } + + /** This exception is used to signal a bad parameter. */ + public static class IllegalArgument extends Exceptions + { + private static final long serialVersionUID = 1L; + + public IllegalArgument() { super(); } + public IllegalArgument(String message) { super(message); } + } + + public static class MissingFacetId extends IllegalArgument + { + private static final long serialVersionUID = 1L; + + public MissingFacetId() { super(); } + public MissingFacetId(String message) { super(message); } + } + + public static class DuplicateFacetId extends IllegalArgument + { + private static final long serialVersionUID = 1L; + private final String facetId; + + public DuplicateFacetId(String facetId) + { + this("", facetId); + } + public DuplicateFacetId(String message, String facetId) + { + super(message); + this.facetId = facetId; + } + + public String getFacetId() { return this.facetId; } + } + + public static class UnrecognisedFacetId extends IllegalArgument + { + private static final long serialVersionUID = 1L; + private final String facetId; + + public UnrecognisedFacetId(String facetId) + { + this("", facetId); + } + public UnrecognisedFacetId(String message, String facetId) + { + super(message); + this.facetId = facetId; + } + + public String getFacetId() { return this.facetId; } + } +} diff --git a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparator.java b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparator.java new file mode 100644 index 0000000000..55f12337cd --- /dev/null +++ b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparator.java @@ -0,0 +1,87 @@ +/* + * 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.facet; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; + +import org.alfresco.util.Pair; + +/** This comparator defines the default sort order for facets. */ +public class SolrFacetComparator implements Comparator +{ + /** A sequence of facet IDs which defines their order, as used in REST API & UI. */ + private final List sortedIDs; + + public SolrFacetComparator(List sortedIDs) + { + this.sortedIDs = new ArrayList<>(); + if (sortedIDs != null) { this.sortedIDs.addAll(sortedIDs); } + } + + @Override public int compare(SolrFacetProperties facet1, SolrFacetProperties facet2) + { + Pair facetIndicesInSortedList = find(facet1, facet2); + + if (bothSorted(facetIndicesInSortedList)) + { + // Sorting is by position in the sortedIDs list. + return facetIndicesInSortedList.getFirst() - facetIndicesInSortedList.getSecond(); + } + else if (neitherSorted(facetIndicesInSortedList)) + { + // Sorting is by the index value defined in the facet itself. + final int indexDifference = facet1.getIndex() - facet2.getIndex(); + + if (indexDifference == 0) + { + // This could happen if an end user defines/overrides indexes to be equal. + // We'll sort based on facet ID if it does happen. + return facet1.getFilterID().compareTo(facet2.getFilterID()); + } + else + { + return indexDifference; + } + } + else + { + // One is in the sortedIDs list and one is not. + // All we want in this case is predictability. The order should be the same. + // We'll (arbitrarily) have facets without an explicit position go at the end. + return facetIndicesInSortedList.getSecond() == -1 ? -1 : 1; + } + } + + /** Get the positional indices of the provided {@link SolrFacetProperties} in the {@link #sortedIDs}. */ + private Pair find(SolrFacetProperties facet1, SolrFacetProperties facet2) + { + return new Pair<>(sortedIDs.indexOf(facet1.getFilterID()), + sortedIDs.indexOf(facet2.getFilterID())); + } + + /** Are both of the provided positional indexes in the {@link #sortedIDs}? */ + private boolean bothSorted(Pair indices) + { return indices.getFirst() != -1 && indices.getSecond() != -1; } + + /** Are neither of the provided positional indexes in the {@link #sortedIDs}? */ + private boolean neitherSorted(Pair indices) + { return indices.getFirst() == -1 && indices.getSecond() == -1; } +} diff --git a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetModel.java b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetModel.java index 2b3f27a4ff..68645ecf31 100644 --- a/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetModel.java +++ b/source/java/org/alfresco/repo/search/impl/solr/facet/SolrFacetModel.java @@ -63,4 +63,9 @@ public interface SolrFacetModel public static final QName PROP_IS_DEFAULT = QName.createQName(SOLR_FACET_MODEL_URL, "isDefault"); public static final QName PROP_EXTRA_INFORMATION = QName.createQName(SOLR_FACET_CUSTOM_PROPERTY_URL, "extraInformation"); + + /** The type of the facet container folder. */ + public static final QName TYPE_FACETS = QName.createQName(SOLR_FACET_MODEL_URL, "facets"); + + public static final QName PROP_FACET_ORDER = QName.createQName(SOLR_FACET_MODEL_URL, "facetOrder"); } 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 c26f23d943..f4a90f179e 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 @@ -21,6 +21,10 @@ package org.alfresco.repo.search.impl.solr.facet; import java.util.List; +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.service.cmr.repository.NodeRef; /** @@ -37,7 +41,7 @@ public interface SolrFacetService * @return List of {@code SolrFacetProperties} or an empty list if none exists */ public List getFacets(); - + /** * Gets the facet by filter Id. * @@ -87,4 +91,15 @@ public interface SolrFacetService public void deleteFacet(String filterID); public int getNextIndex(); + + /** + * Reorders existing facets to the provided order. + * + * @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 a47c5bf935..c0efb4670b 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 @@ -31,9 +31,10 @@ import java.util.Map; import java.util.Map.Entry; import java.util.NavigableMap; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.concurrent.ConcurrentSkipListMap; - import org.alfresco.model.ContentModel; import org.alfresco.repo.cache.SimpleCache; import org.alfresco.repo.node.NodeServicePolicies; @@ -44,6 +45,10 @@ import org.alfresco.repo.node.NodeServicePolicies.OnUpdateNodePolicy; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.policy.JavaBehaviour; 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; @@ -210,7 +215,22 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF @Override public List getFacets() { - return new ArrayList<>(facetsMap.values()); + // Sort the facets into display order + final SolrFacetComparator comparator = new SolrFacetComparator(getFacetOrder()); + + SortedSet result = new TreeSet<>(comparator); + result.addAll(facetsMap.values()); + + return new ArrayList<>(result); + } + + public List getFacetOrder() + { + final NodeRef facetContainer = getFacetsRoot(); + + @SuppressWarnings("unchecked") + final List facetOrder = (List) nodeService.getProperty(facetContainer, SolrFacetModel.PROP_FACET_ORDER); + return facetOrder; } @Override @@ -435,6 +455,8 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF { logger.debug("Deleted [" + filterID + "] facet."); } + + // TODO Remove the matching filterID from the property list on the container. } private Map createNodeProperties(SolrFacetProperties facetProperties) @@ -544,7 +566,8 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF mergedMap.putAll(persistedProperties); // Sort the merged maps - Map sortedMap = CollectionUtils.sortMapByValue(mergedMap, getIndextComparator()); + Comparator> entryComparator = CollectionUtils.toEntryComparator(new SolrFacetComparator(getFacetOrder())); + Map sortedMap = CollectionUtils.sortMapByValue(mergedMap, entryComparator); LinkedList orderedFacets = new LinkedList<>(sortedMap.values()); // Get the last index, as the map is sorted by the FP's index value @@ -641,24 +664,6 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF this.facetNodeRefCache.remove(filterID); } - /** - * Note: this comparator imposes orderings that are inconsistent with equals - * method of the {@link SolrFacetProperties}." - * - * @return - */ - private Comparator> getIndextComparator() - { - return new Comparator>() - { - public int compare(Entry facet1, - Entry facet2) - { - return Integer.compare(facet1.getValue().getIndex(), facet2.getValue().getIndex()); - } - }; - } - @Override public int getNextIndex() { @@ -737,4 +742,47 @@ public class SolrFacetServiceImpl extends AbstractLifecycleBean implements SolrF } } } + + @Override public void reorderFacets(List facetIds) + { + // We need to validate the provided facet IDs + if (facetIds == null) { throw new NullPointerException("Illegal null facetIds"); } + else if (facetIds.isEmpty()) { throw new MissingFacetId("Illegal empty facetIds"); } + else + { + final List existingFacets = getFacets(); + + final Map sortedFacets = new LinkedHashMap<>(); // maintains insertion order + for (String facetId : facetIds) + { + SolrFacetProperties facet = getFacet(facetId); + + if (facet == null) + { + throw new UnrecognisedFacetId("Cannot reorder facets as ID not recognised:", facetId); + } + else if (sortedFacets.containsKey(facetId)) + { + throw new DuplicateFacetId("Cannot reorder facets as sequence contains duplicate entry for ID:", facetId); + } + else + { + sortedFacets.put(facetId, facet); + } + } + if (existingFacets.size() != sortedFacets.size()) + { + throw new IllegalArgument("Cannot reorder facets. Expected " + existingFacets.size() + + " IDs but only received " + sortedFacets.size()); + } + + // We can now safely apply the updates to the facet ID sequence. + // + // Put them in an ArrayList to ensure the collection is Serializable. + // The alternative is changing the service API to look like > + // which is a bit verbose for an API. + ArrayList serializableProp = new ArrayList<>(facetIds); + nodeService.setProperty(getFacetsRoot(), SolrFacetModel.PROP_FACET_ORDER, serializableProp); + } + } } diff --git a/source/test-java/org/alfresco/AllUnitTestsSuite.java b/source/test-java/org/alfresco/AllUnitTestsSuite.java index 564050ecfb..461b3d02d1 100644 --- a/source/test-java/org/alfresco/AllUnitTestsSuite.java +++ b/source/test-java/org/alfresco/AllUnitTestsSuite.java @@ -96,5 +96,6 @@ public class AllUnitTestsSuite extends TestSuite suite.addTest(new JUnit4TestAdapter(org.alfresco.util.test.junitrules.TemporaryMockOverrideTest.class)); suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.search.impl.solr.SolrQueryHTTPClientTest.class)); suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.search.impl.solr.SolrStatsResultTest.class)); + suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.search.impl.solr.facet.SolrFacetComparatorTest.class)); } } diff --git a/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparatorTest.java b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparatorTest.java new file mode 100644 index 0000000000..eaf0cdeeeb --- /dev/null +++ b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetComparatorTest.java @@ -0,0 +1,62 @@ +/* + * 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.facet; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.alfresco.util.collections.CollectionUtils; +import org.alfresco.util.collections.Function; +import org.junit.Test; + +/**Some Unit tests for {@link SolrFacetComparator}. */ +public class SolrFacetComparatorTest +{ + @Test public void simpleSortOfSortedFacets() throws Exception + { + List expectedIds = Arrays.asList(new String[] { "a", "b", "c"}); + + SolrFacetProperties.Builder builder = new SolrFacetProperties.Builder(); + + List facets = Arrays.asList(new SolrFacetProperties[] + { + builder.filterID("c").index(1).build(), + builder.filterID("b").index(2).build(), + builder.filterID("a").index(3).build(), + }); + Collections.sort(facets, new SolrFacetComparator(expectedIds)); + + assertEquals(expectedIds, toFacetIds(facets)); + } + + private List toFacetIds(List facets) + { + return CollectionUtils.transform(facets, new Function() + { + @Override public String apply(SolrFacetProperties value) + { + return value.getFilterID(); + } + }); + } +} 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 new file mode 100644 index 0000000000..4b4846064a --- /dev/null +++ b/source/test-java/org/alfresco/repo/search/impl/solr/facet/SolrFacetServiceImplTest.java @@ -0,0 +1,158 @@ +/* + * 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.facet; + +import static org.junit.Assert.assertEquals; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +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.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.util.collections.CollectionUtils; +import org.alfresco.util.collections.Function; +import org.alfresco.util.test.junitrules.ApplicationContextInit; +import org.alfresco.util.test.junitrules.RunAsFullyAuthenticatedRule; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; + +/** + * Integration tests for {@link SolrFacetServiceImpl}. + */ +public class SolrFacetServiceImplTest +{ + // Rule to initialise the default Alfresco spring configuration + @ClassRule public static ApplicationContextInit APP_CONTEXT_INIT = new ApplicationContextInit(); + + @Rule public RunAsFullyAuthenticatedRule runAsRule = new RunAsFullyAuthenticatedRule(AuthenticationUtil.getAdminUserName()); + + // Various services + private static SolrFacetService SOLR_FACET_SERVICE; + private static RetryingTransactionHelper TRANSACTION_HELPER; + + @BeforeClass public static void initStaticData() throws Exception + { + SOLR_FACET_SERVICE = APP_CONTEXT_INIT.getApplicationContext().getBean("solrFacetService", SolrFacetService.class); + TRANSACTION_HELPER = APP_CONTEXT_INIT.getApplicationContext().getBean("retryingTransactionHelper", RetryingTransactionHelper.class); + } + + // TODO Ensure non-admin, non-search-admin user cannot access SolrFacetService + + @Test public void getFacetsAndReorderThem() throws Exception + { + TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() + { + @Override public Void execute() throws Throwable + { + final List facetIds = getExistingFacetIds(); + final List reorderedFacetIds = new ArrayList<>(facetIds); + Collections.reverse(reorderedFacetIds); + + SOLR_FACET_SERVICE.reorderFacets(reorderedFacetIds); + + final List newfacetIds = getExistingFacetIds(); + + assertEquals(reorderedFacetIds, newfacetIds); + + return null; + } + }); + } + + @Test(expected=NullPointerException.class) + public void reorderNullFacetIdsShouldFail() throws Exception + { + TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() + { + @Override public Void execute() throws Throwable + { + SOLR_FACET_SERVICE.reorderFacets(null); + return null; + } + }); + } + + @Test(expected=MissingFacetId.class) + public void reorderEmptyFacetIdsShouldFail() throws Exception + { + TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() + { + @Override public Void execute() throws Throwable + { + SOLR_FACET_SERVICE.reorderFacets(Collections.emptyList()); + return null; + } + }); + } + + @Test(expected=DuplicateFacetId.class) + public void reorderDuplicateFacetIdsShouldFail() throws Exception + { + TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() + { + @Override public Void execute() throws Throwable + { + final List facetIds = getExistingFacetIds(); + facetIds.add(facetIds.get(0)); + + SOLR_FACET_SERVICE.reorderFacets(facetIds); + return null; + } + }); + } + + @Test(expected=UnrecognisedFacetId.class) + public void reorderUnrecognisedFacetIdsShouldFail() throws Exception + { + TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback() + { + @Override public Void execute() throws Throwable + { + final List facetIds = getExistingFacetIds(); + facetIds.add("unrecognisedID"); + + SOLR_FACET_SERVICE.reorderFacets(facetIds); + return null; + } + + }); + } + + private List getExistingFacetIds() + { + final List facetProps = SOLR_FACET_SERVICE.getFacets(); + final List facetIds = CollectionUtils.transform(facetProps, + new Function() + { + @Override public String apply(SolrFacetProperties value) + { + return value.getFilterID(); + } + }); + return facetIds; + } +}