From eece9fef5efae023fea36e8f6d6cd5c582260db3 Mon Sep 17 00:00:00 2001 From: Neil McErlean Date: Tue, 13 Jul 2010 13:31:07 +0000 Subject: [PATCH] Rating Service. Allow fractional ratings. I've changed the ratings 'score' from an integer to a float as fractional ratings seems like a reasonable idea to me. This had impact all through the Java layer, the REST layer, the model and the test code. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@21128 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/model/contentModel.xml | 2 +- .../repo/rating/RatingNodeProperties.java | 28 +++++++++---------- .../repo/rating/RatingNodePropertiesTest.java | 22 +++++++-------- .../repo/rating/RatingSchemeImpl.java | 10 +++---- .../repo/rating/RatingServiceImpl.java | 22 ++++++--------- .../rating/RatingServiceIntegrationTest.java | 28 +++++++++---------- .../alfresco/service/cmr/rating/Rating.java | 6 ++-- .../service/cmr/rating/RatingScheme.java | 10 +++++-- .../service/cmr/rating/RatingService.java | 8 +++--- 9 files changed, 68 insertions(+), 68 deletions(-) diff --git a/config/alfresco/model/contentModel.xml b/config/alfresco/model/contentModel.xml index 8ed0447374..52d56a91fe 100644 --- a/config/alfresco/model/contentModel.xml +++ b/config/alfresco/model/contentModel.xml @@ -427,7 +427,7 @@ Rating - d:int + d:float true true diff --git a/source/java/org/alfresco/repo/rating/RatingNodeProperties.java b/source/java/org/alfresco/repo/rating/RatingNodeProperties.java index 8519921276..ccf2077374 100644 --- a/source/java/org/alfresco/repo/rating/RatingNodeProperties.java +++ b/source/java/org/alfresco/repo/rating/RatingNodeProperties.java @@ -47,14 +47,14 @@ public class RatingNodeProperties { // These lists are declared as the concrete type 'ArrayList' as they must implement Serializable private final ArrayList schemes; - private final ArrayList scores; + private final ArrayList scores; private final ArrayList dates; - public RatingNodeProperties(List schemes, List scores, List dates) + public RatingNodeProperties(List schemes, List scores, List dates) { // No null lists. if (schemes == null) schemes = new ArrayList(); - if (scores == null) scores = new ArrayList(); + if (scores == null) scores = new ArrayList(); if (dates == null) dates = new ArrayList(); // All lists must be the same length. @@ -69,8 +69,8 @@ public class RatingNodeProperties for (String s : schemes) this.schemes.add(s); - this.scores = new ArrayList(scores.size()); - for (Integer i : scores) + this.scores = new ArrayList(scores.size()); + for (Float i : scores) this.scores.add(i); this.dates = new ArrayList(dates.size()); @@ -89,7 +89,7 @@ public class RatingNodeProperties public static RatingNodeProperties createFrom(Map props) { List schemes = (List) props.get(ContentModel.PROP_RATING_SCHEME); - List scores = (List)props.get(ContentModel.PROP_RATING_SCORE); + List scores = (List)props.get(ContentModel.PROP_RATING_SCORE); List dates = (List) props.get(ContentModel.PROP_RATED_AT); return new RatingNodeProperties(schemes, scores, dates); @@ -112,7 +112,7 @@ public class RatingNodeProperties public RatingStruct getRatingAt(int index) { String scheme = this.schemes.get(index); - int score = this.scores.get(index); + float score = this.scores.get(index); Date d = this.dates.get(index); return new RatingStruct(scheme, score, d); } @@ -124,7 +124,7 @@ public class RatingNodeProperties * @param schemeName the scheme name. * @param score the score. */ - public void appendRating(String schemeName, int score) + public void appendRating(String schemeName, float score) { this.schemes.add(schemeName); this.scores.add(score); @@ -142,7 +142,7 @@ public class RatingNodeProperties * @param scheme the new rating scheme name. * @param score the new rating score. */ - public void setRatingAt(int index, String scheme, int score) + public void setRatingAt(int index, String scheme, float score) { this.schemes.set(index, scheme); this.scores.set(index, score); @@ -157,7 +157,7 @@ public class RatingNodeProperties public RatingStruct removeRatingAt(int index) { String removedScheme = this.schemes.remove(index); - int removedScore = this.scores.remove(index); + float removedScore = this.scores.remove(index); Date removedDate = this.dates.remove(index); return new RatingStruct(removedScheme, removedScore, removedDate); @@ -237,10 +237,10 @@ public class RatingNodeProperties public class RatingStruct { private String scheme; - private int score; + private float score; private Date date; - public RatingStruct(String scheme, int score, Date d) + public RatingStruct(String scheme, float score, Date d) { RatingStruct.this.scheme = scheme; RatingStruct.this.score = score; @@ -252,7 +252,7 @@ public class RatingNodeProperties return scheme; } - public int getScore() + public float getScore() { return score; } @@ -278,7 +278,7 @@ public class RatingNodeProperties @Override public int hashCode() { - return scheme.hashCode() + 7 * score + 11 * date.hashCode(); + return scheme.hashCode() + (int)(7 * score) + 11 * date.hashCode(); } @Override diff --git a/source/java/org/alfresco/repo/rating/RatingNodePropertiesTest.java b/source/java/org/alfresco/repo/rating/RatingNodePropertiesTest.java index ff1203a3a7..efb8030079 100644 --- a/source/java/org/alfresco/repo/rating/RatingNodePropertiesTest.java +++ b/source/java/org/alfresco/repo/rating/RatingNodePropertiesTest.java @@ -50,14 +50,14 @@ public class RatingNodePropertiesTest // These are declared as ArrayLists in order to be Serializable. private ArrayList testSchemes; - private ArrayList testScores; + private ArrayList testScores; private ArrayList testDates; private RatingNodeProperties testProps; @Before public void initTestData() { testSchemes = new ArrayList(); - testScores = new ArrayList(); + testScores = new ArrayList(); testDates = new ArrayList(); // These correspond to two ratings: // '1' in the 'like' scheme at 'now'. @@ -65,8 +65,8 @@ public class RatingNodePropertiesTest testSchemes.add(LIKE); testSchemes.add(FIVESTAR); - testScores.add(1); - testScores.add(5); + testScores.add(1.0f); + testScores.add(5.0f); testDates.add(new Date()); testDates.add(new Date()); @@ -87,7 +87,7 @@ public class RatingNodePropertiesTest @Test public void noNullPropertyLists() throws Exception { List schemes = null; - List scores = null; + List scores = null; List dates = null; RatingNodeProperties nullProps = new RatingNodeProperties(schemes, scores, dates); @@ -104,20 +104,20 @@ public class RatingNodePropertiesTest final RatingStruct firstRating = testProps.getRatingAt(0); assertEquals(LIKE, firstRating.getScheme()); - assertEquals(1, firstRating.getScore()); + assertEquals(1.0f, firstRating.getScore(), 0.1f); final Date recoveredLikeDate = firstRating.getDate(); assertNotNull(recoveredLikeDate); final RatingStruct secondRating = testProps.getRatingAt(1); assertEquals(FIVESTAR, secondRating.getScheme()); - assertEquals(5, secondRating.getScore()); + assertEquals(5, secondRating.getScore(), 0.1f); final Date recoveredSecondDate = secondRating.getDate(); assertNotNull(recoveredSecondDate); RatingStruct l = testProps.getRating(LIKE); assertNotNull(l); assertEquals(LIKE, l.getScheme()); - assertEquals(1, l.getScore()); + assertEquals(1, l.getScore(), 0.1f); assertEquals(recoveredLikeDate, l.getDate()); } @@ -165,7 +165,7 @@ public class RatingNodePropertiesTest assertNotNull(testProps.getRating(FIVESTAR)); final RatingStruct fooRating = testProps.getRating("foo"); assertNotNull(fooRating); - assertEquals(42, fooRating.getScore()); + assertEquals(42, fooRating.getScore(), 0.1f); } @SuppressWarnings("unchecked") @@ -176,14 +176,14 @@ public class RatingNodePropertiesTest final int numberOfProperties = 3; assertEquals(numberOfProperties, alfProps.size()); final List ratingSchemes = (List)alfProps.get(ContentModel.PROP_RATING_SCHEME); - final List ratingScores = (List)alfProps.get(ContentModel.PROP_RATING_SCORE); + final List ratingScores = (List)alfProps.get(ContentModel.PROP_RATING_SCORE); final List ratingDates = (List)alfProps.get(ContentModel.PROP_RATED_AT); final int numberOfRatings = 2; assertEquals(numberOfRatings, ratingSchemes.size()); assertEquals(numberOfRatings, ratingScores.size()); assertEquals(numberOfRatings, ratingDates.size()); assertEquals(Arrays.asList(new String[]{LIKE, FIVESTAR}), ratingSchemes); - assertEquals(Arrays.asList(new Integer[]{1, 5}), ratingScores); + assertEquals(Arrays.asList(new Float[]{1.0f, 5.0f}), ratingScores); // No Date checks } } diff --git a/source/java/org/alfresco/repo/rating/RatingSchemeImpl.java b/source/java/org/alfresco/repo/rating/RatingSchemeImpl.java index 3d8d966197..8af8ed8366 100644 --- a/source/java/org/alfresco/repo/rating/RatingSchemeImpl.java +++ b/source/java/org/alfresco/repo/rating/RatingSchemeImpl.java @@ -33,7 +33,7 @@ public class RatingSchemeImpl implements RatingScheme, BeanNameAware, Initializi private final RatingSchemeRegistry ratingSchemeRegistry; private String name; - private int minRating, maxRating; + private float minRating, maxRating; public RatingSchemeImpl(RatingSchemeRegistry registry) { @@ -54,12 +54,12 @@ public class RatingSchemeImpl implements RatingScheme, BeanNameAware, Initializi this.name = name; } - public void setMinRating(int minRating) + public void setMinRating(float minRating) { this.minRating = minRating; } - public void setMaxRating(int maxRating) + public void setMaxRating(float maxRating) { this.maxRating = maxRating; } @@ -84,7 +84,7 @@ public class RatingSchemeImpl implements RatingScheme, BeanNameAware, Initializi * (non-Javadoc) * @see org.alfresco.service.cmr.rating.RatingScheme#getMaxRating() */ - public int getMaxRating() + public float getMaxRating() { return this.maxRating; } @@ -93,7 +93,7 @@ public class RatingSchemeImpl implements RatingScheme, BeanNameAware, Initializi * (non-Javadoc) * @see org.alfresco.service.cmr.rating.RatingScheme#getMinRating() */ - public int getMinRating() + public float getMinRating() { return this.minRating; } diff --git a/source/java/org/alfresco/repo/rating/RatingServiceImpl.java b/source/java/org/alfresco/repo/rating/RatingServiceImpl.java index 6d856dbc09..9f4b48573a 100644 --- a/source/java/org/alfresco/repo/rating/RatingServiceImpl.java +++ b/source/java/org/alfresco/repo/rating/RatingServiceImpl.java @@ -81,9 +81,9 @@ public class RatingServiceImpl implements RatingService /* * (non-Javadoc) - * @see org.alfresco.service.cmr.rating.RatingService#applyRating(org.alfresco.service.cmr.repository.NodeRef, int, java.lang.String) + * @see org.alfresco.service.cmr.rating.RatingService#applyRating(org.alfresco.service.cmr.repository.NodeRef, float, java.lang.String) */ - public void applyRating(final NodeRef targetNode, final int rating, + public void applyRating(final NodeRef targetNode, final float rating, final String ratingSchemeName) throws RatingServiceException { final String currentUser = AuthenticationUtil.getFullyAuthenticatedUser(); @@ -110,7 +110,7 @@ public class RatingServiceImpl implements RatingService return currentUser.equals(creator); } - private void applyRating(NodeRef targetNode, int rating, + private void applyRating(NodeRef targetNode, float rating, String ratingSchemeName, final String userName) throws RatingServiceException { if (log.isDebugEnabled()) @@ -150,7 +150,7 @@ public class RatingServiceImpl implements RatingService // These are multivalued properties. Map ratingProps = new HashMap(); - ratingProps.put(ContentModel.PROP_RATING_SCORE, toSerializableList(new Integer[]{rating})); + ratingProps.put(ContentModel.PROP_RATING_SCORE, toSerializableList(new Float[]{rating})); ratingProps.put(ContentModel.PROP_RATED_AT, toSerializableList(new Date[]{new Date()})); ratingProps.put(ContentModel.PROP_RATING_SCHEME, toSerializableList(new String[]{ratingSchemeName})); @@ -230,8 +230,6 @@ public class RatingServiceImpl implements RatingService ratingStruct.getDate()); return result; } - - //TODO Don't forget that it is possible on read to have out-of-range ratings. } /* @@ -280,16 +278,17 @@ public class RatingServiceImpl implements RatingService * (non-Javadoc) * @see org.alfresco.service.cmr.rating.RatingService#getTotalRating(org.alfresco.service.cmr.repository.NodeRef, java.lang.String) */ - public int getTotalRating(NodeRef targetNode, String ratingSchemeName) + public float getTotalRating(NodeRef targetNode, String ratingSchemeName) { - //TODO Put these in the db as properties? + //TODO Performance improvement? : put node rating total/count/average into a multi-valued + // property in the db. List ratingsNodes = this.getRatingNodeChildren(targetNode, null); // It's one node per user so the size of this list is the number of ratings applied. // However not all of these users' ratings need be in the specified scheme. // So we need to go through and check that the rating node contains a rating for the // specified scheme. - int result = 0; + float result = 0; for (ChildAssociationRef ratingsNode : ratingsNodes) { List ratings = getRatingsFrom(ratingsNode.getChildRef()); @@ -304,10 +303,8 @@ public class RatingServiceImpl implements RatingService return result; } - // TODO We can at least amalgamate these into one looping call. public float getAverageRating(NodeRef targetNode, String ratingSchemeName) { - //TODO Put these in the db as properties? List ratingsNodes = this.getRatingNodeChildren(targetNode, null); // It's one node per user so the size of this list is the number of ratings applied. @@ -315,7 +312,7 @@ public class RatingServiceImpl implements RatingService // So we need to go through and check that the rating node contains a rating for the // specified scheme. int ratingCount = 0; - int ratingTotal = 0; + float ratingTotal = 0; for (ChildAssociationRef ratingsNode : ratingsNodes) { List ratings = getRatingsFrom(ratingsNode.getChildRef()); @@ -340,7 +337,6 @@ public class RatingServiceImpl implements RatingService public int getRatingsCount(NodeRef targetNode, String ratingSchemeName) { - //TODO Put these in the db as properties? List ratingsNodes = this.getRatingNodeChildren(targetNode, null); // It's one node per user so the size of this list is the number of ratings applied. diff --git a/source/java/org/alfresco/repo/rating/RatingServiceIntegrationTest.java b/source/java/org/alfresco/repo/rating/RatingServiceIntegrationTest.java index 5f7a701a24..631644d7cc 100644 --- a/source/java/org/alfresco/repo/rating/RatingServiceIntegrationTest.java +++ b/source/java/org/alfresco/repo/rating/RatingServiceIntegrationTest.java @@ -134,14 +134,14 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest RatingScheme likesRS = schemes.get(LIKES_SCHEME_NAME); assertNotNull("'likes' rating scheme was missing.", likesRS); assertEquals("'likes' rating scheme had wrong name.", LIKES_SCHEME_NAME, likesRS.getName()); - assertEquals("'likes' rating scheme had wrong min.", 1, likesRS.getMinRating()); - assertEquals("'likes' rating scheme had wrong max.", 1, likesRS.getMaxRating()); + assertEquals("'likes' rating scheme had wrong min.", 1.0f, likesRS.getMinRating()); + assertEquals("'likes' rating scheme had wrong max.", 1.0f, likesRS.getMaxRating()); RatingScheme fiveStarRS = schemes.get(FIVE_STAR_SCHEME_NAME); assertNotNull("'5*' rating scheme was missing.", fiveStarRS); assertEquals("'5*' rating scheme had wrong name.", FIVE_STAR_SCHEME_NAME, fiveStarRS.getName()); - assertEquals("'5*' rating scheme had wrong min.", 1, fiveStarRS.getMinRating()); - assertEquals("'5*' rating scheme had wrong max.", 5, fiveStarRS.getMaxRating()); + assertEquals("'5*' rating scheme had wrong min.", 1.0f, fiveStarRS.getMinRating()); + assertEquals("'5*' rating scheme had wrong max.", 5.0f, fiveStarRS.getMaxRating()); } /** @@ -151,14 +151,14 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest public void testApplyIllegalRatings() throws Exception { // See rating-services-context.xml for definitions of these rating schemes. - int[] illegalRatings = new int[]{0, 2}; - for (int illegalRating : illegalRatings) + float[] illegalRatings = new float[]{0.0f, 2.0f}; + for (float illegalRating : illegalRatings) { applyIllegalRating(testDoc_Admin, illegalRating, LIKES_SCHEME_NAME); } } - private void applyIllegalRating(NodeRef nodeRef, int illegalRating, String schemeName) + private void applyIllegalRating(NodeRef nodeRef, float illegalRating, String schemeName) { try { @@ -180,8 +180,8 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest assertNull("Expected a null rating,", nullRating); assertNull("Expected a null remove result.", ratingService.removeRatingByCurrentUser(testDoc_Admin, LIKES_SCHEME_NAME)); - final int likesScore = 1; - final int fiveStarScore = 5; + final float likesScore = 1; + final float fiveStarScore = 5; ratingService.applyRating(testDoc_Admin, likesScore, LIKES_SCHEME_NAME); ratingService.applyRating(testDoc_Admin, fiveStarScore, FIVE_STAR_SCHEME_NAME); @@ -221,7 +221,7 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest assertDateIsCloseToNow(fiveStarRatingAppliedAt); // Now we'll update a rating - final int updatedFiveStarScore = 3; + final float updatedFiveStarScore = 3; ratingService.applyRating(testDoc_Admin, updatedFiveStarScore, FIVE_STAR_SCHEME_NAME); // Some basic node structure tests. @@ -290,16 +290,16 @@ public class RatingServiceIntegrationTest extends BaseAlfrescoSpringTest { // 2 different users rating the same piece of content in the same rating scheme AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE); - ratingService.applyRating(testDoc_Admin, 4, FIVE_STAR_SCHEME_NAME); + ratingService.applyRating(testDoc_Admin, 4.0f, FIVE_STAR_SCHEME_NAME); AuthenticationUtil.setFullyAuthenticatedUser(USER_USERTWO); - ratingService.applyRating(testDoc_Admin, 2, FIVE_STAR_SCHEME_NAME); + ratingService.applyRating(testDoc_Admin, 2.0f, FIVE_STAR_SCHEME_NAME); float meanRating = ratingService.getAverageRating(testDoc_Admin, FIVE_STAR_SCHEME_NAME); assertEquals("Document had wrong mean rating.", 3f, meanRating); - int totalRating = ratingService.getTotalRating(testDoc_Admin, FIVE_STAR_SCHEME_NAME); - assertEquals("Document had wrong total rating.", 6, totalRating); + float totalRating = ratingService.getTotalRating(testDoc_Admin, FIVE_STAR_SCHEME_NAME); + assertEquals("Document had wrong total rating.", 6.0f, totalRating); int ratingsCount = ratingService.getRatingsCount(testDoc_Admin, FIVE_STAR_SCHEME_NAME); assertEquals("Document had wrong ratings count.", 2, ratingsCount); diff --git a/source/java/org/alfresco/service/cmr/rating/Rating.java b/source/java/org/alfresco/service/cmr/rating/Rating.java index a5db35125c..705a5e0a9a 100644 --- a/source/java/org/alfresco/service/cmr/rating/Rating.java +++ b/source/java/org/alfresco/service/cmr/rating/Rating.java @@ -29,12 +29,12 @@ import java.util.Date; */ public class Rating { - private final int ratingScore; + private final float ratingScore; private final String ratingAppliedBy; private final Date ratingAppliedAt; private final RatingScheme ratingScheme; - public Rating(RatingScheme scheme, int score, String appliedBy, Date appliedAt) + public Rating(RatingScheme scheme, float score, String appliedBy, Date appliedAt) { this.ratingScheme = scheme; this.ratingScore = score; @@ -48,7 +48,7 @@ public class Rating * * @return the score. */ - public int getScore() + public float getScore() { return ratingScore; } diff --git a/source/java/org/alfresco/service/cmr/rating/RatingScheme.java b/source/java/org/alfresco/service/cmr/rating/RatingScheme.java index f6cf4838e2..0a9481a605 100644 --- a/source/java/org/alfresco/service/cmr/rating/RatingScheme.java +++ b/source/java/org/alfresco/service/cmr/rating/RatingScheme.java @@ -19,8 +19,12 @@ package org.alfresco.service.cmr.rating; +import org.alfresco.repo.rating.RatingSchemeRegistry; + /** - * TODO + * This data type represents a rating scheme as used in the {@link RatingService}. + * These schemes are defined within spring context files and injected into the + * {@link RatingSchemeRegistry} at startup. * * @author Neil McErlean * @since 3.4 @@ -39,12 +43,12 @@ public interface RatingScheme * * @return the minimum rating. */ - public int getMinRating(); + public float getMinRating(); /** * This method returns the maximum rating defined for this scheme. * * @return the maximum rating. */ - public int getMaxRating(); + public float getMaxRating(); } diff --git a/source/java/org/alfresco/service/cmr/rating/RatingService.java b/source/java/org/alfresco/service/cmr/rating/RatingService.java index 47432cd700..e06b56586f 100644 --- a/source/java/org/alfresco/service/cmr/rating/RatingService.java +++ b/source/java/org/alfresco/service/cmr/rating/RatingService.java @@ -33,8 +33,8 @@ import org.alfresco.service.cmr.repository.NodeRef; * which are injected via spring (see rating-service-context.xml). The rating * schemes define a minimum and a maximum score value for that scheme. *

- * Ratings can be {@link RatingService#applyRating(NodeRef, int, String) applied}, - * {@link RatingService#applyRating(NodeRef, int, String) updated} and + * Ratings can be {@link RatingService#applyRating(NodeRef, float, String) applied}, + * {@link RatingService#applyRating(NodeRef, float, String) updated} and * {@link RatingService#removeRatingByCurrentUser(NodeRef, RatingScheme) removed}. * * TODO Get average/total @@ -77,7 +77,7 @@ public interface RatingService * @see RatingScheme */ @NotAuditable - void applyRating(NodeRef targetNode, int rating, String ratingSchemeName) throws RatingServiceException; + void applyRating(NodeRef targetNode, float rating, String ratingSchemeName) throws RatingServiceException; /** * This method gets the number of individual ratings which have been applied to @@ -93,7 +93,7 @@ public interface RatingService @NotAuditable int getRatingsCount(NodeRef targetNode, String ratingSchemeName); - int getTotalRating(NodeRef targetNode, String ratingSchemeName); + float getTotalRating(NodeRef targetNode, String ratingSchemeName); /** * This method returns the average (mean) rating in the specified scheme for the