From 8e3b855deac9dcf475dd2d38aeeea7a3a4dc70f0 Mon Sep 17 00:00:00 2001 From: Neil McErlean Date: Tue, 12 Jul 2011 11:42:16 +0000 Subject: [PATCH] ALF-9151. Added findSites() method which uses Lucene queries to retrieve sites. This method supports a CONTAINS query on cm:name, title, description. All listSites() methods are now immediately consistent - as opposed to eventually. However in moving these method implementations to CannedQueries, they now only support STARTS_WITH_IGNORE_CASE queries on cm:name, title, description. I've highlighted this in the javadoc. ScriptSiteService now uses the eventually consistent findSites() method for searches. Also some fallout in the tests. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@28943 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../public-services-security-context.xml | 1 + .../alfresco/repo/site/SiteServiceImpl.java | 189 ++++++++++-------- .../repo/site/SiteServiceImplTest.java | 57 ++++-- .../repo/site/script/ScriptSiteService.java | 24 ++- .../service/cmr/site/SiteService.java | 50 ++++- 5 files changed, 201 insertions(+), 120 deletions(-) diff --git a/config/alfresco/public-services-security-context.xml b/config/alfresco/public-services-security-context.xml index 8f8d5673ed..ba8390ba1b 100644 --- a/config/alfresco/public-services-security-context.xml +++ b/config/alfresco/public-services-security-context.xml @@ -914,6 +914,7 @@ org.alfresco.service.cmr.site.SiteService.createContainer=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties org.alfresco.service.cmr.site.SiteService.createSite=ACL_ALLOW org.alfresco.service.cmr.site.SiteService.deleteSite=ACL_ALLOW + org.alfresco.service.cmr.site.SiteService.findSites=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties org.alfresco.service.cmr.site.SiteService.getContainer=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties org.alfresco.service.cmr.site.SiteService.getMembersRole=ACL_ALLOW org.alfresco.service.cmr.site.SiteService.getSite=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties diff --git a/source/java/org/alfresco/repo/site/SiteServiceImpl.java b/source/java/org/alfresco/repo/site/SiteServiceImpl.java index ee4dc90b16..e7bddfe05d 100644 --- a/source/java/org/alfresco/repo/site/SiteServiceImpl.java +++ b/source/java/org/alfresco/repo/site/SiteServiceImpl.java @@ -729,7 +729,79 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic } return siteHomeRef; } - + + /* + * (non-Javadoc) + * @see org.alfresco.service.cmr.site.SiteService#findSites(java.lang.String, java.lang.String, int) + */ + @Override + public List findSites(String filter, String sitePresetFilter, int size) + { + List result; + + NodeRef siteRoot = getSiteRoot(); + if (siteRoot == null) + { + result = Collections.emptyList(); + } + else + { + // get the sites that match the specified names + StringBuilder query = new StringBuilder(128); + query.append("+PARENT:\"").append(siteRoot.toString()).append('"'); + + final boolean nameFilterIsPresent = filter != null && filter.length() > 0; + final boolean sitePresetFilterIsPresent = sitePresetFilter != null && sitePresetFilter.length() > 0; + + if (nameFilterIsPresent || sitePresetFilterIsPresent) + { + query.append(" +("); + if (nameFilterIsPresent) + { + String escNameFilter = AbstractLuceneQueryParser.escape(filter.replace('"', ' ')); + + query.append(" @cm\\:name:\"*" + escNameFilter + "*\"") + .append(" @cm\\:title:\"" + escNameFilter + "\"") + .append(" @cm\\:description:\"" + escNameFilter + "\""); + } + if (sitePresetFilterIsPresent) + { + String escPresetFilter = AbstractLuceneQueryParser.escape(sitePresetFilter.replace('"', ' ')); + query.append(" @st\\:sitePreset:\"" + escPresetFilter + "\""); + } + + query.append(")"); + } + + ResultSet results = this.searchService.query( + siteRoot.getStoreRef(), + SearchService.LANGUAGE_LUCENE, + query.toString(), + null); + try + { + result = new ArrayList(results.length()); + for (NodeRef site : results.getNodeRefs()) + { + // Ignore any node type that is not a "site" + QName siteClassName = this.nodeService.getType(site); + if (this.dictionaryService.isSubClass(siteClassName, SiteModel.TYPE_SITE) == true) + { + result.add(createSiteInfo(site)); + // break on max size limit reached + if (result.size() == size) break; + } + } + } + finally + { + results.close(); + } + } + + return result; + } + /** * @see org.alfresco.service.cmr.site.SiteService#listSites(java.lang.String, java.lang.String) */ @@ -741,98 +813,41 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic /** * @see org.alfresco.service.cmr.site.SiteService#listSites(java.lang.String, java.lang.String, int) */ - public List listSites(final String nameFilter, final String sitePresetFilter, int size) + public List listSites(final String filter, final String sitePresetFilter, int size) { - // Implementation note: The behaviour of this method depends on what parameter values are passed in. - // - // If there is a non-null, non-empty nameFilter String, then it is wrapped in *s, e.g. "*foo*" and that expression - // is matched against cm:name in a Lucene query. - // sitePresetFilters are treated as exact matches, i.e. "foo" will search for st:sitePreset.equals("foo"). - // - // Therefore we are able to handle searches: (nameFilter = null, sitePresetFilter = "foo", size = {int}) - // as GetChildrenCannedQueries. - // However GCCQs do not support the 'contains' match that would be required for nameFilter - for performance reasons. - - List result; + List result = Collections.emptyList(); NodeRef siteRoot = getSiteRoot(); - if (siteRoot == null) + if (siteRoot != null) { - result = Collections.emptyList(); - } - else - { - final boolean nameFilterHasValue = nameFilter != null && nameFilter.length() != 0; + final boolean filterHasValue = filter != null && filter.length() != 0; final boolean sitePresetFilterHasValue = sitePresetFilter != null && sitePresetFilter.length() > 0; - // If nameFilter is not specified, we can use a GetChildrenCannedQuery. - if ( !nameFilterHasValue) + List> sortProps = null; + + PagingRequest pagingRequest = new PagingRequest(size == 0 ? Integer.MAX_VALUE : size); + List filterProps = new ArrayList(); + + if (filterHasValue) { - List> sortProps = null; - - PagingRequest pagingRequest = new PagingRequest(size == 0 ? Integer.MAX_VALUE : size); - List filterProps = new ArrayList(1); - if (sitePresetFilterHasValue) - { - filterProps.add(new FilterPropString(SiteModel.PROP_SITE_PRESET, sitePresetFilter, FilterTypeString.EQUALS)); - } - PagingResults allSites = listSites(filterProps, sortProps, pagingRequest); - result = allSites.getPage(); + filterProps.add(new FilterPropString(ContentModel.PROP_NAME, filter, FilterTypeString.STARTSWITH_IGNORECASE)); + filterProps.add(new FilterPropString(ContentModel.PROP_TITLE, filter, FilterTypeString.STARTSWITH_IGNORECASE)); + filterProps.add(new FilterPropString(ContentModel.PROP_DESCRIPTION, filter, FilterTypeString.STARTSWITH_IGNORECASE)); } - // FIXME Otherwise we need a different approach. Currently we search with Lucene, but this may change. - else + if (sitePresetFilterHasValue) { - final NodeRef siteRoot1 = getSiteRoot(); - - StringBuilder query = new StringBuilder(128); - query.append("+PARENT:\"").append(siteRoot1.toString()) - .append("\" +("); - - String escNameFilter = AbstractLuceneQueryParser.escape(nameFilter.replace('"', ' ')); - - query.append(" @cm\\:name:\"*" + escNameFilter + "*\"") - .append(" @cm\\:title:\"" + escNameFilter + "\"") - .append(" @cm\\:description:\"" + escNameFilter + "\""); - - if (sitePresetFilterHasValue) - { - String escPresetFilter = AbstractLuceneQueryParser.escape(sitePresetFilter.replace('"', ' ')); - query.append(" @st\\:sitePreset:\"" + escPresetFilter + "\""); - } - - query.append(")"); - - ResultSet results = this.searchService.query( - siteRoot1.getStoreRef(), - SearchService.LANGUAGE_LUCENE, - query.toString(), - null); - try - { - result = new ArrayList(results.length()); - for (NodeRef site : results.getNodeRefs()) - { - // Ignore any node type that is not a "site" - QName siteClassName = this.directNodeService.getType(site); - if (this.dictionaryService.isSubClass(siteClassName, SiteModel.TYPE_SITE) == true) - { - result.add(createSiteInfo(site)); - // break on max size limit reached - if (result.size() == size) break; - } - } - } - finally - { - results.close(); - } + filterProps.add(new FilterPropString(SiteModel.PROP_SITE_PRESET, sitePresetFilter, FilterTypeString.EQUALS)); } + + PagingResults allSites = listSites(filterProps, sortProps, pagingRequest); + result = allSites.getPage(); } return result; } - - /** + + /* + * (non-Javadoc) * @see org.alfresco.service.cmr.site.SiteService#listSites(java.lang.String) */ public List listSites(final String userName) @@ -868,7 +883,8 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic searchTypeQNames.add(SiteModel.TYPE_SITE); // get canned query - GetChildrenCannedQueryFactory getChildrenCannedQueryFactory = (GetChildrenCannedQueryFactory)cannedQueryRegistry.getNamedObject("siteGetChildrenCannedQueryFactory"); + final String cQBeanName = "siteGetChildrenCannedQueryFactory"; + GetChildrenCannedQueryFactory getChildrenCannedQueryFactory = (GetChildrenCannedQueryFactory)cannedQueryRegistry.getNamedObject(cQBeanName); GetChildrenCannedQuery cq = (GetChildrenCannedQuery)getChildrenCannedQueryFactory.getCannedQuery(getSiteRoot(), searchTypeQNames, filterProps, sortProps, pagingRequest); @@ -925,6 +941,13 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic }; } + /** + * This method returns the {@link SiteInfo siteInfos} for sites to which the specified user has access. + * Note that if the user has access to more than 1000 sites, the list will be truncated to 1000 entries. + * + * @param userName the username + * @return a list of {@link SiteInfo site infos}. + */ private List listSitesImpl(String userName) { List result = null; @@ -932,7 +955,7 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic // get the Groups this user is contained within (at any level) Set groups = this.authorityService.getContainingAuthorities(null, userName, false); Set siteNames = new HashSet(groups.size()); - // purge non Site related Groups and strip the group name down to the site "shortName" it relates too + // purge non Site related Groups and strip the group name down to the site "shortName" it relates to for (String group : groups) { if (group.startsWith(GROUP_SITE_PREFIX)) @@ -956,7 +979,7 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic NodeRef siteRoot = getSiteRoot(); if (siteRoot == null) { - result = new ArrayList(0); + result = Collections.emptyList(); } else { @@ -965,6 +988,8 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic // // Note the implicit assumption here: that the specified user is not a member of > 1000 sites // If the user IS a member of more than 1000 sites, then a truncated list of sites will be returned. + // Also, given that the siteNames are a Set, there is no guarantee about which sites would be + // included in the truncated results and which would be excluded. HashSets are unordered. if (siteList.size() > 1000) { siteList = siteList.subList(0, 1000); diff --git a/source/java/org/alfresco/repo/site/SiteServiceImplTest.java b/source/java/org/alfresco/repo/site/SiteServiceImplTest.java index 9ee944af59..419e312d58 100644 --- a/source/java/org/alfresco/repo/site/SiteServiceImplTest.java +++ b/source/java/org/alfresco/repo/site/SiteServiceImplTest.java @@ -76,8 +76,8 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest { private static final String TEST_SITE_PRESET = "testSitePreset"; private static final String TEST_SITE_PRESET_2 = "testSitePreset2"; - private static final String TEST_TITLE = "This is my title"; - private static final String TEST_DESCRIPTION = "This is my description"; + private static final String TEST_TITLE = "TitleTest This is my title"; + private static final String TEST_DESCRIPTION = "DescriptionTest This is my description"; private static final String USER_ONE = "UserOne_SiteServiceImplTest"; private static final String USER_TWO = "UserTwo_SiteServiceImplTest"; @@ -398,10 +398,18 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest } /** - * Test listSite methods. + * Test listSite and findSites methods. + *

+ * Note that {@link SiteService#findSites(String, int)} offers eventually consistent results and therefore may + * exhibit changed behaviour if Lucene is switched off or is replaced by SOLR. + * {@link SiteService#listSites(List, List, org.alfresco.query.PagingRequest)} and the other listSites methods + * should offer consistent, accurate result sets. */ public void testListSites() throws Exception { + // We'll match against the first few letter of TEST_TITLE in various listSites() tests below. + final String testTitlePrefix = TEST_TITLE.substring(0, 9); + List sites = this.siteService.listSites(null, null); assertNotNull("sites list was null.", sites); final int preexistingSitesCount = sites.size(); @@ -417,23 +425,33 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest sites = this.siteService.listSites(null, null); assertNotNull(sites); assertEquals(preexistingSitesCount + 5, sites.size()); + List sitesFromFind = this.siteService.findSites(null, null, 100); + assertEquals(preexistingSitesCount + 5, sitesFromFind.size()); - // Get sites by matching name - sites = this.siteService.listSites("One", null); + // Get sites by matching name - as of 4.0 listSites only supports STARTS WITH matches + sites = this.siteService.listSites("mySiteO", null); assertNotNull(sites); - assertEquals("Matched wrong number of sites named 'One'", 1, sites.size()); + assertEquals("Matched wrong number of sites named 'mySiteO*'", 1, sites.size()); + // However 'findSites' allows CONTAINS matching. + sitesFromFind = this.siteService.findSites("One", null, 100); + assertEquals("Matched wrong number of sites named 'One'", 1, sitesFromFind.size()); // Get sites by matching title - sites = this.siteService.listSites("title", null); + sites = this.siteService.listSites(testTitlePrefix, null); assertNotNull(sites); - assertEquals("Matched wrong number of sites named 'title'", 5, sites.size()); + assertEquals("Matched wrong number of sites starting with '" + testTitlePrefix + "'", 5, sites.size()); + sitesFromFind = this.siteService.findSites("title", null, 100); + assertEquals("Matched wrong number of sites containing 'title'", 5, sitesFromFind.size()); // Get sites by matching description sites = this.siteService.listSites("description", null); assertNotNull(sites); assertEquals("Matched wrong number of sites named 'description'", 5, sites.size()); + sitesFromFind = this.siteService.findSites("description", null, 100); + assertEquals("Matched wrong number of sites named 'description'", 5, sitesFromFind.size()); // Get sites by matching sitePreset - see ALF-5620 + // SiteService.findSites does not support finding by sitePreset and so is not tested here. sites = this.siteService.listSites(null, TEST_SITE_PRESET); assertNotNull(sites); assertEquals("Matched wrong number of sites with PRESET", 2, sites.size()); @@ -496,16 +514,17 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest /** * Test list sites with a name filter */ - sites = this.siteService.listSites("One", null, 10); + sites = this.siteService.listSites("mySiteOne", null, 10); assertNotNull(sites); assertEquals(1, sites.size()); + sitesFromFind = this.siteService.findSites("One", null, 100); + assertEquals(1, sitesFromFind.size()); /** - * Search for partial match on more titles - matches word "Site" + * Search for partial match on more titles - matches word "Site". */ - sites = this.siteService.listSites("ite", null, 10); - assertNotNull(sites); - assertEquals(5, sites.size()); + sitesFromFind = this.siteService.findSites("ite", null, 100); + assertEquals(5, sitesFromFind.size()); /** * Now Switch to User Two and do the same sort of searching. @@ -516,9 +535,8 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest /** * As User Two Search for partial match on more titles - matches word "Site" - should not find private sites */ - sites = this.siteService.listSites("ite", null, 10); - assertNotNull(sites); - assertEquals(4, sites.size()); + sitesFromFind = this.siteService.findSites("ite", null, 100); + assertEquals(4, sitesFromFind.size()); for (SiteInfo site : sites) { String shortName = site.getShortName(); @@ -555,9 +573,8 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest /** * As User Three Search for partial match on more titles - matches word "Site" - should not find private and moderated sites */ - sites = this.siteService.listSites("ite", null, 10); - assertNotNull(sites); - assertEquals(3, sites.size()); + sitesFromFind = this.siteService.findSites("ite", null, 100); + assertEquals(3, sitesFromFind.size()); for (SiteInfo site : sites) { String shortName = site.getShortName(); @@ -579,7 +596,7 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest } else if (shortName.equals("mySiteFive") == true) { - checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED); + checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED); } else { diff --git a/source/java/org/alfresco/repo/site/script/ScriptSiteService.java b/source/java/org/alfresco/repo/site/script/ScriptSiteService.java index 154fb44d37..32ad06c0e2 100644 --- a/source/java/org/alfresco/repo/site/script/ScriptSiteService.java +++ b/source/java/org/alfresco/repo/site/script/ScriptSiteService.java @@ -160,30 +160,36 @@ public class ScriptSiteService extends BaseScopableProcessorExtension *

* If no filters are specified then all the available sites are returned. * - * @param nameFilter name filter + * @param filter inclusion filter for returned sites. Only sites whose cm:name OR cm:title + * OR cm:description CONTAIN the filter string will be returned. * @param sitePresetFilter site preset filter - * @return Site[] a list of the site filtered as appropriate + * @return Site[] a list of the site filtered as appropriate + * + * @see SiteService#findSites(String, String, int) for a description of the limitations of this method. */ - public Site[] listSites(String nameFilter, String sitePresetFilter) + public Site[] listSites(String filter, String sitePresetFilter) { - return listSites(nameFilter, sitePresetFilter, 0); + return listSites(filter, sitePresetFilter, 0); } /** * List the sites available in the repository. The returned list can optionally be filtered by name and site * preset. - *

+ *

* If no filters are specified then all the available sites are returned. * - * @param nameFilter name filter + * @param filter inclusion filter for returned sites. Only sites whose cm:name OR cm:title + * OR cm:description CONTAIN the filter string will be returned. * @param sitePresetFilter site preset filter * @param size max results size crop if >0 * - * @return Site[] a list of the site filtered as appropriate + * @return Site[] a list of the site filtered as appropriate + * + * @see SiteService#findSites(String, String, int) for a description of the limitations of this method. */ - public Site[] listSites(String nameFilter, String sitePresetFilter, int size) + public Site[] listSites(String filter, String sitePresetFilter, int size) { - List siteInfos = this.siteService.listSites(nameFilter, sitePresetFilter, size); + List siteInfos = this.siteService.findSites(filter, sitePresetFilter, size); List sites = new ArrayList(siteInfos.size()); for (SiteInfo siteInfo : siteInfos) { diff --git a/source/java/org/alfresco/service/cmr/site/SiteService.java b/source/java/org/alfresco/service/cmr/site/SiteService.java index 9e943b0b52..8180409d07 100644 --- a/source/java/org/alfresco/service/cmr/site/SiteService.java +++ b/source/java/org/alfresco/service/cmr/site/SiteService.java @@ -22,13 +22,13 @@ import java.io.Serializable; import java.util.List; import java.util.Map; +import org.alfresco.model.ContentModel; import org.alfresco.query.PagingRequest; import org.alfresco.query.PagingResults; import org.alfresco.repo.node.getchildren.FilterProp; import org.alfresco.repo.security.authority.UnknownAuthorityException; import org.alfresco.service.Auditable; import org.alfresco.service.NotAuditable; -import org.alfresco.service.PublicService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; @@ -95,25 +95,56 @@ public interface SiteService boolean hasCreateSitePermissions(); /** - * List the available sites. This list can optionally be filtered by site name and/or site preset. + * This method will find all {@link SiteInfo sites} available to the currently authenticated user based on + * the specified site filter, site preset filter and result set size. + * The filter parameter will match any sites whose {@link ContentModel#PROP_NAME cm:name}, {@link ContentModel#PROP_TITLE cm:title} + * or {@link ContentModel#PROP_DESCRIPTION cm:description} contain the specified string (ignoring case). + *

+ * Note that this method uses Alfresco Full Text Search to retrieve results + * and depending on server Lucene, SOLR configuration may only offer eventually consistent results. * - * @param nameFilter name filter - * @param sitePresetFilter site preset filter + * @param filter Any supplied filter will be wrapped in asterisks (e.g. '*foo*') and used to match the sites' cm:name, cm:title or cm:description. + * @param sitePresetFilter a site preset filter name to match against. + * @param size this parameter specifies a maximum result set size. + * @return Site objects for all matching sites up to the maximum result size. + * + * @since 4.0 + */ + @NotAuditable + List findSites(String filter, String sitePresetFilter, int size); + + /** + * List the available sites. This list can optionally be filtered by site name/title/description and/or site preset. + *

+ * Note: Starting with Alfresco 4.0, the filter parameter will only match sites whose {@link ContentModel#PROP_NAME cm:name} or + * {@link ContentModel#PROP_TITLE cm:title} or {@link ContentModel#PROP_DESCRIPTION cm:description} start with + * the specified string (ignoring case). The listing of sites whose cm:names (or titles or descriptions) contain the + * specified string is no longer supported. To retrieve sites whose cm:names etc contain a substring, {@link SiteService#findSites(String, String, int)} + * should be used instead. + * + * @param filter filter (sites whose cm:name, cm:title or cm:description START WITH filter) + * @param sitePresetFilter site preset filter (sites whose preset EQUALS sitePresetFilter) * @param size list maximum size or zero for all * @return List list of site information */ @NotAuditable - List listSites(String nameFilter, String sitePresetFilter, int size); + List listSites(String filter, String sitePresetFilter, int size); /** - * List the available sites. This list can optionally be filtered by site name and/or site preset. + * List the available sites. This list can optionally be filtered by site name/title/description and/or site preset. + *

+ * Note: Starting with Alfresco 4.0, the filter parameter will only match sites whose {@link ContentModel#PROP_NAME cm:name} or + * {@link ContentModel#PROP_TITLE cm:title} or {@link ContentModel#PROP_DESCRIPTION cm:description} start with + * the specified string (ignoring case). The listing of sites whose cm:names (or titles or descriptions) contain the + * specified string is no longer supported. To retrieve sites whose cm:names etc contain a substring, {@link SiteService#findSites(String, String, int)} + * should be used instead. * - * @param nameFilter name filter + * @param filter filter * @param sitePresetFilter site preset filter * @return List list of site information */ @NotAuditable - List listSites(String nameFilter, String sitePresetFilter); + List listSites(String filter, String sitePresetFilter); /** * List all the sites that the specified user has a explicit membership to. @@ -126,7 +157,8 @@ public interface SiteService /** * This method returns {@link PagingResults paged result sets} of {@link SiteInfo} objects, which should be - * more efficient than the unpaged methods also available on this interface. + * more efficient than the unpaged methods also available on this interface. It is also guaranteed to return + * fully consistent results. * * @param filterProps property filters * @param sortProps sorting options