Fix for ALF-4830 Rating Service needs to suspend auditing behaviour during rating updated.

Added the necessary boilerplate to disable auditing on the rated node during rating.
  Added some asserts to the RatingService JUnit test code to test same.

Also some trivial doc changes.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22708 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
Neil McErlean
2010-09-27 09:48:14 +00:00
parent b4b432b21b
commit 7323502141
4 changed files with 63 additions and 25 deletions

View File

@@ -39,25 +39,14 @@
class="org.alfresco.repo.security.permissions.impl.AlwaysProceedMethodInterceptor" /> class="org.alfresco.repo.security.permissions.impl.AlwaysProceedMethodInterceptor" />
<!-- Rating Service base bean --> <!-- Rating Service base bean -->
<bean id="ratingService" class="org.alfresco.repo.rating.RatingServiceImpl" > <bean id="ratingService" class="org.alfresco.repo.rating.RatingServiceImpl">
<property name="ratingSchemeRegistry"> <property name="ratingSchemeRegistry">
<ref local="ratingSchemeRegistry"/> <ref local="ratingSchemeRegistry"/>
</property> </property>
<property name="nodeService" ref="nodeService"/> <property name="behaviourFilter" ref="policyBehaviourFilter" />
<property name="nodeService" ref="nodeService"/>
</bean> </bean>
<!-- i18n TODO Needed? -->
<!--
<bean id="ratingResourceBundles"
class="org.springframework.extensions.surf.util.ResourceBundleBootstrapComponent">
<property name="resourceBundles">
<list>
<value>alfresco.messages.rating-config</value>
</list>
</property>
</bean>
-->
<!-- The rating scheme registry maintains the collection of registered rating schemes. --> <!-- The rating scheme registry maintains the collection of registered rating schemes. -->
<bean id="ratingSchemeRegistry" class="org.alfresco.repo.rating.RatingSchemeRegistry" /> <bean id="ratingSchemeRegistry" class="org.alfresco.repo.rating.RatingSchemeRegistry" />

View File

@@ -26,6 +26,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import org.alfresco.model.ContentModel; import org.alfresco.model.ContentModel;
import org.alfresco.repo.policy.BehaviourFilter;
import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil;
import org.alfresco.service.cmr.rating.Rating; import org.alfresco.service.cmr.rating.Rating;
import org.alfresco.service.cmr.rating.RatingScheme; import org.alfresco.service.cmr.rating.RatingScheme;
@@ -47,14 +48,13 @@ import org.apache.commons.logging.LogFactory;
*/ */
public class RatingServiceImpl implements RatingService public class RatingServiceImpl implements RatingService
{ {
//TODO Add links to ActivityService. Straight calls? Behaviours?
private static final Log log = LogFactory.getLog(RatingServiceImpl.class); private static final Log log = LogFactory.getLog(RatingServiceImpl.class);
private RatingSchemeRegistry schemeRegistry; private RatingSchemeRegistry schemeRegistry;
// Injected services // Injected services
private NodeService nodeService; private NodeService nodeService;
private BehaviourFilter behaviourFilter;
public void setRatingSchemeRegistry(RatingSchemeRegistry schemeRegistry) public void setRatingSchemeRegistry(RatingSchemeRegistry schemeRegistry)
{ {
this.schemeRegistry = schemeRegistry; this.schemeRegistry = schemeRegistry;
@@ -65,6 +65,11 @@ public class RatingServiceImpl implements RatingService
this.nodeService = nodeService; this.nodeService = nodeService;
} }
public void setBehaviourFilter(BehaviourFilter behaviourFilter)
{
this.behaviourFilter = behaviourFilter;
}
public Map<String, RatingScheme> getRatingSchemes() public Map<String, RatingScheme> getRatingSchemes()
{ {
// This is already an unmodifiable Map. // This is already an unmodifiable Map.
@@ -131,10 +136,21 @@ public class RatingServiceImpl implements RatingService
throw new RatingServiceException("Rating " + rating + " violates range for " + ratingScheme); throw new RatingServiceException("Rating " + rating + " violates range for " + ratingScheme);
} }
// Add the cm:rateable aspect if it's not there already.
// Ensure that the application of a rating does not cause updates
// to the modified, modifier properties on the rated node.
if (nodeService.hasAspect(targetNode, ContentModel.ASPECT_RATEABLE) == false) if (nodeService.hasAspect(targetNode, ContentModel.ASPECT_RATEABLE) == false)
{ {
nodeService.addAspect(targetNode, ContentModel.ASPECT_RATEABLE, null); behaviourFilter.disableBehaviour(targetNode, ContentModel.ASPECT_AUDITABLE);
try
{
// Add the cm:rateable aspect if it's not there already.
nodeService.addAspect(targetNode, ContentModel.ASPECT_RATEABLE, null);
}
finally
{
behaviourFilter.enableBehaviour(targetNode, ContentModel.ASPECT_AUDITABLE);
}
} }
// We're looking for child cm:rating nodes whose qname matches the current user. // We're looking for child cm:rating nodes whose qname matches the current user.
@@ -150,7 +166,15 @@ public class RatingServiceImpl implements RatingService
ratingProps.put(ContentModel.PROP_RATED_AT, new Date()); ratingProps.put(ContentModel.PROP_RATED_AT, new Date());
ratingProps.put(ContentModel.PROP_RATING_SCHEME, ratingSchemeName); ratingProps.put(ContentModel.PROP_RATING_SCHEME, ratingSchemeName);
nodeService.createNode(targetNode, ContentModel.ASSOC_RATINGS, assocQName, ContentModel.TYPE_RATING, ratingProps); behaviourFilter.disableBehaviour(targetNode, ContentModel.ASPECT_AUDITABLE);
try
{
nodeService.createNode(targetNode, ContentModel.ASSOC_RATINGS, assocQName, ContentModel.TYPE_RATING, ratingProps);
}
finally
{
behaviourFilter.enableBehaviour(targetNode, ContentModel.ASPECT_AUDITABLE);
}
} }
else else
{ {
@@ -251,8 +275,8 @@ public class RatingServiceImpl implements RatingService
{ {
return null; return null;
} }
ChildAssociationRef lastChild = ratingChildren.get(0); ChildAssociationRef child = ratingChildren.get(0);
Map<QName, Serializable> properties = nodeService.getProperties(lastChild.getChildRef()); Map<QName, Serializable> properties = nodeService.getProperties(child.getChildRef());
Rating result = null; Rating result = null;
// If the rating is for the specified scheme delete it. // If the rating is for the specified scheme delete it.
@@ -262,7 +286,7 @@ public class RatingServiceImpl implements RatingService
Float score = (Float) properties.get(ContentModel.PROP_RATING_SCORE); Float score = (Float) properties.get(ContentModel.PROP_RATING_SCORE);
Date date = (Date)properties.get(ContentModel.PROP_RATED_AT); Date date = (Date)properties.get(ContentModel.PROP_RATED_AT);
nodeService.deleteNode(lastChild.getChildRef()); nodeService.deleteNode(child.getChildRef());
result = new Rating(getRatingScheme(ratingSchemeName), score, user, date); result = new Rating(getRatingScheme(ratingSchemeName), score, user, date);
} }

View File

@@ -89,7 +89,6 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
companyHome = this.repositoryHelper.getCompanyHome(); companyHome = this.repositoryHelper.getCompanyHome();
//TODO These could be created @BeforeClass
testFolder = createNode(companyHome, "testFolder", ContentModel.TYPE_FOLDER); testFolder = createNode(companyHome, "testFolder", ContentModel.TYPE_FOLDER);
testFolderCopyDest = createNode(companyHome, "testFolderCopyDest", ContentModel.TYPE_FOLDER); testFolderCopyDest = createNode(companyHome, "testFolderCopyDest", ContentModel.TYPE_FOLDER);
testDoc_Admin = createNode(testFolder, "testDocInFolder", ContentModel.TYPE_CONTENT); testDoc_Admin = createNode(testFolder, "testDocInFolder", ContentModel.TYPE_CONTENT);
@@ -191,6 +190,7 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
final float fiveStarScore = 5; final float fiveStarScore = 5;
ratingService.applyRating(testDoc_Admin, fiveStarScore, FIVE_STAR_SCHEME_NAME); ratingService.applyRating(testDoc_Admin, fiveStarScore, FIVE_STAR_SCHEME_NAME);
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
// Some basic node structure tests. // Some basic node structure tests.
assertTrue(ContentModel.ASPECT_RATEABLE + " aspect missing.", assertTrue(ContentModel.ASPECT_RATEABLE + " aspect missing.",
@@ -221,6 +221,7 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
// Now we'll update a rating // Now we'll update a rating
final float updatedFiveStarScore = 3; final float updatedFiveStarScore = 3;
ratingService.applyRating(testDoc_Admin, updatedFiveStarScore, FIVE_STAR_SCHEME_NAME); ratingService.applyRating(testDoc_Admin, updatedFiveStarScore, FIVE_STAR_SCHEME_NAME);
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
// Some basic node structure tests. // Some basic node structure tests.
allChildren = nodeService.getChildAssocs(testDoc_Admin, allChildren = nodeService.getChildAssocs(testDoc_Admin,
@@ -246,6 +247,7 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
// And delete the 'five star' rating. // And delete the 'five star' rating.
Rating deletedStarRating = ratingService.removeRatingByCurrentUser(testDoc_Admin, FIVE_STAR_SCHEME_NAME); Rating deletedStarRating = ratingService.removeRatingByCurrentUser(testDoc_Admin, FIVE_STAR_SCHEME_NAME);
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
// 'five star' rating data should be unchanged. // 'five star' rating data should be unchanged.
assertNotNull("'5*' rating was null.", deletedStarRating); assertNotNull("'5*' rating was null.", deletedStarRating);
assertEquals("Wrong score for rating", updatedFiveStarScore, deletedStarRating.getScore()); assertEquals("Wrong score for rating", updatedFiveStarScore, deletedStarRating.getScore());
@@ -331,14 +333,23 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
assertEquals("Wrong number of child nodes", 1 , nodeService.getChildAssocs(testDoc_Admin).size()); assertEquals("Wrong number of child nodes", 1 , nodeService.getChildAssocs(testDoc_Admin).size());
} }
/**
* This test method applies ratings to a single node as a number of different users.
* It checks that the ratings are applied correctly and that the cm:modifier is not
* updated by these changes.
*/
public void testApplyRating_MultipleUsers() throws Exception public void testApplyRating_MultipleUsers() throws Exception
{ {
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
// 2 different users rating the same piece of content in the same rating scheme // 2 different users rating the same piece of content in the same rating scheme
AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE); AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE);
ratingService.applyRating(testDoc_Admin, 4.0f, FIVE_STAR_SCHEME_NAME); ratingService.applyRating(testDoc_Admin, 4.0f, FIVE_STAR_SCHEME_NAME);
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
AuthenticationUtil.setFullyAuthenticatedUser(USER_TWO); AuthenticationUtil.setFullyAuthenticatedUser(USER_TWO);
ratingService.applyRating(testDoc_Admin, 2.0f, FIVE_STAR_SCHEME_NAME); ratingService.applyRating(testDoc_Admin, 2.0f, FIVE_STAR_SCHEME_NAME);
assertModifierIs(testDoc_Admin, AuthenticationUtil.getAdminUserName());
float meanRating = ratingService.getAverageRating(testDoc_Admin, FIVE_STAR_SCHEME_NAME); float meanRating = ratingService.getAverageRating(testDoc_Admin, FIVE_STAR_SCHEME_NAME);
assertEquals("Document had wrong mean rating.", 3f, meanRating); assertEquals("Document had wrong mean rating.", 3f, meanRating);
@@ -349,6 +360,18 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest
int ratingsCount = ratingService.getRatingsCount(testDoc_Admin, FIVE_STAR_SCHEME_NAME); int ratingsCount = ratingService.getRatingsCount(testDoc_Admin, FIVE_STAR_SCHEME_NAME);
assertEquals("Document had wrong ratings count.", 2, ratingsCount); assertEquals("Document had wrong ratings count.", 2, ratingsCount);
} }
/**
* This method asserts that the modifier of the specified node is equal to the
* provided modifier name.
* @param nodeRef the nodeRef to check.
* @param expectedModifier the expected modifier e.g. "admin".
*/
private void assertModifierIs(NodeRef nodeRef, final String expectedModifier)
{
String actualModifier = (String)nodeService.getProperty(nodeRef, ContentModel.PROP_MODIFIER);
assertEquals("Incorrect cm:modifier", expectedModifier, actualModifier);
}
public void testUsersCantRateTheirOwnContent() throws Exception public void testUsersCantRateTheirOwnContent() throws Exception
{ {

View File

@@ -22,7 +22,9 @@ package org.alfresco.service.cmr.rating;
import org.alfresco.repo.rating.RatingSchemeRegistry; import org.alfresco.repo.rating.RatingSchemeRegistry;
/** /**
* This data type represents a rating scheme as used in the {@link RatingService}. * This interface defines a Rating Scheme, which is a named scheme for user-supplied
* ratings with a defined minimum value and a defined maximum value. The minimum must
* not be greater than the maximum but the two values can be equal.
* These schemes are defined within spring context files and injected into the * These schemes are defined within spring context files and injected into the
* {@link RatingSchemeRegistry} at startup. * {@link RatingSchemeRegistry} at startup.
* *