ALFCOM-2981 Cannot find moderated site if I am a 'non-owner' using search field

- Moderated sites now have CONSUMER role for everyone.
 - Note that I havn't fixed the permissions of existing Moderates Sites,  This will only work for new and updated sites.

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@14793 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/search/QueryParameterDefImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2007 Alfresco Software Limited.
+ * Copyright (C) 2005-2009 Alfresco Software Limited.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -58,6 +58,14 @@ public class QueryParameterDefImpl implements QueryParameterDefinition
     
     private String defaultValue;
 
+    /**
+     * QueryParameterDefImpl
+     * 
+     * @param qName
+     * @param propertyDefinition
+     * @param hasDefaultValue
+     * @param defaultValue
+     */
     public QueryParameterDefImpl(QName qName, PropertyDefinition propertyDefinition, boolean hasDefaultValue, String defaultValue)
     {
         this(qName, hasDefaultValue, defaultValue);
@@ -65,6 +73,12 @@ public class QueryParameterDefImpl implements QueryParameterDefinition
         this.dataTypeDefintion = propertyDefinition.getDataType();
     }
 
+    /**
+     * 
+     * @param qName
+     * @param hasDefaultValue
+     * @param defaultValue
+     */
     private QueryParameterDefImpl(QName qName, boolean hasDefaultValue, String defaultValue)
     {
         super();
@@ -73,6 +87,13 @@ public class QueryParameterDefImpl implements QueryParameterDefinition
         this.defaultValue = defaultValue;
     }
     
+    /**
+     * 
+     * @param qName
+     * @param dataTypeDefintion
+     * @param hasDefaultValue
+     * @param defaultValue
+     */
     public QueryParameterDefImpl(QName qName, DataTypeDefinition dataTypeDefintion, boolean hasDefaultValue, String defaultValue)
     {
         this(qName, hasDefaultValue, defaultValue);

source/java/org/alfresco/repo/site/SiteServiceImpl.java

@@ -396,7 +396,8 @@ public class SiteServiceImpl implements SiteService, SiteModel
                 }
                 else if (SiteVisibility.MODERATED.equals(visibility) == true)
                 {
-                    permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES, true);
+                	permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, SITE_CONSUMER, true);
+                    //permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES, true);
                 }
                 permissionService.setPermission(siteNodeRef,
                         PermissionService.ALL_AUTHORITIES,
@@ -589,7 +590,7 @@ public class SiteServiceImpl implements SiteService, SiteModel
         {
             String escNameFilter = LuceneQueryParser.escape(nameFilter.replace('"', ' '));
             // Perform a Lucene search under the Site parent node using *name*, title and description search query
-            QueryParameterDefinition[] params = new QueryParameterDefinition[3];
+            final QueryParameterDefinition[] params = new QueryParameterDefinition[3];
             params[0] = new QueryParameterDefImpl(
                     ContentModel.PROP_NAME,
                     dictionaryService.getDataType(
@@ -621,7 +622,8 @@ public class SiteServiceImpl implements SiteService, SiteModel
                     siteRoot.getStoreRef(),
                     SearchService.LANGUAGE_LUCENE,
                     query.toString(),
-                    params);
+                    params);           
+             
             result = new ArrayList<SiteInfo>(results.length());
             try
             {
@@ -895,7 +897,8 @@ public class SiteServiceImpl implements SiteService, SiteModel
             }
             else if (SiteVisibility.MODERATED.equals(currentVisibility) == true)
             {
-                this.permissionService.deletePermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES);
+                this.permissionService.deletePermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, SITE_CONSUMER);
+                //this.permissionService.deletePermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES);
                 // TODO update all child folders ?? ...
             }
             
@@ -906,7 +909,8 @@ public class SiteServiceImpl implements SiteService, SiteModel
             }
             else if (SiteVisibility.MODERATED.equals(updatedVisibility) == true)
             {
-                this.permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES, true);
+                this.permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, SITE_CONSUMER, true);
+                //this.permissionService.setPermission(siteNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.READ_PROPERTIES, true);
                 // TODO update all child folders ?? ...
             }
             

source/java/org/alfresco/repo/site/SiteServiceImplTest.java

@@ -54,7 +54,7 @@ import org.alfresco.util.BaseAlfrescoSpringTest;
 import org.alfresco.util.PropertyMap;
 
 /**
- * Thumbnail service implementation unit test
+ * Site service implementation unit test
  * 
  * @author Roy Wetherall
  */
@@ -232,27 +232,31 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
         assertTrue(this.taggingService.isTagScope(siteInfo.getNodeRef()));
     }
     
+    /**
+     * Test listSite methods.
+     * 
+     * @throws Exception
+     */
     public void testListSites() throws Exception
-    {
+    {    	
-        // TODO
+        /**
-        // - check filters
+         *  Check for no pre-existing sites before we start the test
-        // - check private excluded when not owner (or admin)
+         */
-        
-        // Check for no sites
         List<SiteInfo> sites = this.siteService.listSites(null, null);
-        assertNotNull(sites);
+        assertNotNull("sites already exist prior to starting test", sites);
-        assertTrue(sites.isEmpty());
+        assertTrue("sites already exist prior to starting test", sites.isEmpty());
         
         // Create some sites
         this.siteService.createSite(TEST_SITE_PRESET, "mySiteOne", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
         this.siteService.createSite(TEST_SITE_PRESET, "mySiteTwo", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PRIVATE);
         this.siteService.createSite(TEST_SITE_PRESET_2, "mySiteThree", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
         this.siteService.createSite(TEST_SITE_PRESET_2, "mySiteFour", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PRIVATE);
+        this.siteService.createSite(TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);
         
         // Get all the sites
         sites = this.siteService.listSites(null, null);
         assertNotNull(sites);
-        assertEquals(4, sites.size());
+        assertEquals(5, sites.size());
         
         // Get sites by matching name
         sites = this.siteService.listSites("One", null);
@@ -262,12 +266,12 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
         // Get sites by matching title
         sites = this.siteService.listSites("title", null);
         assertNotNull(sites);
-        assertEquals(4, sites.size());
+        assertEquals(5, sites.size());
 
         // Get sites by matching description
         sites = this.siteService.listSites("description", null);
         assertNotNull(sites);
-        assertEquals(4, sites.size());
+        assertEquals(5, sites.size());
 
         // Do detailed check of the site info objects
         for (SiteInfo site : sites)
@@ -289,12 +293,19 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
             {
                 checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFour", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PRIVATE);                
             }
+            else if (shortName.equals("mySiteFive") == true)
+            {
+                checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);                
+            }
             else
             {
                 fail("The shortname " + shortName + " is not recognised");
             }
         }
         
+        /**
+         * Test list sites for a user
+         */
         sites = this.siteService.listSites(USER_TWO);
         assertNotNull(sites);
         assertEquals(0, sites.size());
@@ -302,14 +313,110 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
         this.siteService.setMembership("mySiteOne", USER_TWO, SiteModel.SITE_CONSUMER);
         this.siteService.setMembership("mySiteTwo", USER_TWO, SiteModel.SITE_CONSUMER);
         
-        sites = this.siteService.listSites(USER_TWO);
+		sites = this.siteService.listSites(USER_TWO);
         assertNotNull(sites);
         assertEquals(2, sites.size());
         
+        /**
+         * User One is the creator of all the sites.
+         */
         sites = this.siteService.listSites(USER_ONE);
         assertNotNull(sites);
-        assertEquals(4, sites.size());
+        assertEquals(5, sites.size());
         
+        /**
+         * Test list sites with a name filter
+         */
+        sites = this.siteService.listSites("One", null, 10);
+        assertNotNull(sites);
+        assertEquals(1, sites.size());
+        
+        /**
+         * Search for partial match on more titles - matches word "Site"
+         */
+        sites = this.siteService.listSites("ite", null, 10);
+        assertNotNull(sites);
+        assertEquals(5, sites.size());    
+        
+        /**
+         * Now Switch to User Two and do the same sort of searching.
+         */
+        // Set the current authentication
+        this.authenticationComponent.setCurrentUser(USER_TWO);
+        
+        /**
+         * 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()); 
+        for (SiteInfo site : sites)
+        {
+        	String shortName = site.getShortName();
+        	if (shortName.equals("mySiteOne") == true)
+        	{
+        		checkSiteInfo(site, TEST_SITE_PRESET, "mySiteOne", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
+        	}
+        	else if (shortName.equals("mySiteTwo") == true)
+        	{
+        		// User Two is a member of this private site
+        		checkSiteInfo(site, TEST_SITE_PRESET, "mySiteTwo", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PRIVATE);
+        	}
+        	else if (shortName.equals("mySiteThree") == true)
+        	{
+        		checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteThree", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
+        	}
+        	else if (shortName.equals("mySiteFour") == true)
+        	{
+        		// User two is not a member of this site
+        		fail("Can see private site mySiteFour");             
+        	}
+        	else if (shortName.equals("mySiteFive") == true)
+        	{
+        		// User Two should be able to see this moderated site.
+        		checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);                
+        	}
+        	else
+        	{
+        		fail("The shortname " + shortName + " is not recognised");
+        	}
+        }
+        
+        authenticationComponent.setCurrentUser(USER_THREE);
+        /**
+         * 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()); 
+        for (SiteInfo site : sites)
+        {
+        	String shortName = site.getShortName();
+        	if (shortName.equals("mySiteOne") == true)
+        	{
+        		checkSiteInfo(site, TEST_SITE_PRESET, "mySiteOne", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
+        	}
+        	else if (shortName.equals("mySiteTwo") == true)
+        	{
+        		fail("Can see private site mySiteTwo");
+        	}
+        	else if (shortName.equals("mySiteThree") == true)
+        	{
+        		checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteThree", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.PUBLIC);
+        	}
+        	else if (shortName.equals("mySiteFour") == true)
+        	{
+        		fail("Can see private site mySiteFour");             
+        	}
+        	else if (shortName.equals("mySiteFive") == true)
+        	{
+        		checkSiteInfo(site, TEST_SITE_PRESET_2, "mySiteFive", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);                
+        	}
+        	else
+        	{
+        		fail("The shortname " + shortName + " is not recognised");
+        	}
+        }        
     }
     
     public void testGetSite()
@@ -860,7 +967,7 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
         siteInfo = this.siteService.getSite("testSiteVisibilityModeratedSite");
         checkSiteInfo(siteInfo, TEST_SITE_PRESET, "testSiteVisibilityModeratedSite", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);
         //  - are the permissions correct for non-members?
-        testVisibilityPermissions("Testing visibility of moderated site", USER_TWO, siteInfo, true, false);
+        testVisibilityPermissions("Testing visibility of moderated site", USER_TWO, siteInfo, true, true);
         
         // Create a private site
         siteInfo = createTestSiteWithContent("testSiteVisibilityPrivateSite", "testComp", SiteVisibility.PRIVATE);
@@ -880,7 +987,7 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
         //  - check the updated sites visibility
         siteInfo = this.siteService.getSite("testSiteVisibilityChangeSite");
         checkSiteInfo(siteInfo, TEST_SITE_PRESET, "testSiteVisibilityChangeSite", TEST_TITLE, TEST_DESCRIPTION, SiteVisibility.MODERATED);
-        testVisibilityPermissions("Testing visibility of moderated site", USER_TWO, siteInfo, true, false);
+        testVisibilityPermissions("Testing visibility of moderated site", USER_TWO, siteInfo, true, true);
         
         // Switch from moderated -> private
         changeSite.setVisibility(SiteVisibility.PRIVATE);