r7250 - in dumbhippo/trunk/server/src/com/dumbhippo: server server/impl services

[commits mugshot org]

Author: marinaz
Date: 2008-01-23 12:16:17 -0600 (Wed, 23 Jan 2008)
New Revision: 7250

Modified:
   dumbhippo/trunk/server/src/com/dumbhippo/server/MusicSystem.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/LastFmUpdaterBean.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/MusicSystemBean.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/StackerBean.java
   dumbhippo/trunk/server/src/com/dumbhippo/services/AmazonWebServices.java
   dumbhippo/trunk/server/src/com/dumbhippo/services/FacebookWebServices.java
Log:
Don't re-stack the music block for each track in a batch of Last.fm tracks,
only do it when processing the newest of those tracks. This should ease the 
load on the system, but also prevent us from publishing multiple user actions
about playing the same track on Facebook.

Produce a warning instead of an error when we exceed the maximum number of user
actions that we can publish for a particular user. (We are limited to 10 actions
in a rolling 48-hour window.)

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/MusicSystem.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/MusicSystem.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/MusicSystem.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -174,9 +174,12 @@
 	 * @param user who listened
 	 * @param properties props of the track
 	 * @param listenDate milliseconds since the epoch
+	 * @param isNative indicates if we got the track natively through the client or from some feed/web services
+	 * @param isNewest indicates if the track is newest or if we know that we are processing a newer track in
+	 *                 the same batch, so should not stack an update based in the one
 	 * @throws RetryException 
 	 */	
-	public void addHistoricalTrack(User user, Map<String,String> properties, long listenDate, boolean isNative) throws RetryException;
+	public void addHistoricalTrack(User user, Map<String,String> properties, long listenDate, boolean isNative, boolean isNewest) throws RetryException;
 	
 	/**
 	 * 

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/LastFmUpdaterBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/LastFmUpdaterBean.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/LastFmUpdaterBean.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -99,7 +99,7 @@
 				long trackListenTime = lastFmTrack.getListenTime() * 1000;
 				long listenTime = trackListenTime <= now ? trackListenTime : now;
 				
-				musicSystem.addHistoricalTrack(user, props, listenTime, false);
+				musicSystem.addHistoricalTrack(user, props, listenTime, false, mostRecentListenTimeSeconds == lastFmTrack.getListenTime());
 			}
 		}
 	}

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/MusicSystemBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/MusicSystemBean.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/MusicSystemBean.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -207,7 +207,7 @@
 		});
 	}
 
-	private void addTrackHistory(final User user, final Track track, final Date now, final boolean isNative) throws RetryException {
+	private void addTrackHistory(final User user, final Track track, final Date playedDate, final boolean isNative, final boolean isNewest) throws RetryException {
 		TxUtils.runNeedsRetry(new TxRunnable() {				
 			public void run() {
 				User u = em.find(User.class, user.getId());					
@@ -229,11 +229,11 @@
 				TrackHistory res;
 				try {
 					res = (TrackHistory) q.getSingleResult();
-					res.setLastUpdated(now);
+					res.setLastUpdated(playedDate);
 					res.setTimesPlayed(res.getTimesPlayed() + 1);
 				} catch (NoResultException e) {
 					res = new TrackHistory(user, track);
-					res.setLastUpdated(now);
+					res.setLastUpdated(playedDate);
 					res.setTimesPlayed(1);
 					em.persist(res);
 				}
@@ -241,16 +241,16 @@
 		});
 		
 		// update the stack with this new listen event
-		if (track != null) {
+		if (track != null && isNewest) {
 			DataService.currentSessionRW().changed(UserDMO.class, user.getGuid(), "currentTrack");
 			DataService.currentSessionRW().changed(UserDMO.class, user.getGuid(), "currentTrackPlayTime");
-			notifier.onTrackPlayed(user, track, now);
+			notifier.onTrackPlayed(user, track, playedDate);
 		}
 	}
 	
 	@TransactionAttribute(TransactionAttributeType.SUPPORTS)	
 	public void setCurrentTrack(final User user, final Track track, final boolean isNative) throws RetryException {
-		addTrackHistory(user, track, new Date(), isNative);
+		addTrackHistory(user, track, new Date(), isNative, true);
 	}
 	 
 	// Although this is marked as SUPPORTS - you should never invoke this
@@ -276,8 +276,8 @@
 	// method from any code holding a transaction that could have potentially
 	// modified a Track		
 	@TransactionAttribute(TransactionAttributeType.SUPPORTS)	
-	public void addHistoricalTrack(final User user, final Map<String,String> properties, final long listenDate, final boolean isNative) throws RetryException {
-		addTrackHistory(user, getTrack(properties), new Date(listenDate), isNative);
+	public void addHistoricalTrack(final User user, final Map<String,String> properties, final long listenDate, final boolean isNative, final boolean isNewest) throws RetryException {
+		addTrackHistory(user, getTrack(properties), new Date(listenDate), isNative, isNewest);
 	}
 	
 	public TrackHistory getCurrentTrack(Viewpoint viewpoint, User user) throws NotFoundException {
@@ -1295,7 +1295,14 @@
 			public void run() throws RetryException {
 				User attached = em.find(User.class, userId);
 				DataService.getModel().initializeReadWriteSession(new UserViewpoint(attached, Site.NONE));
-				addTrackHistory(attached, getTrack(properties), new Date(virtualPlayTime), false);
+				// We could use entryPosition == 0 to determine if the track is actually newest.
+				// However, since we add track histories in separate transactions, we need to make sure all 
+				// track histories are added when we last stack the block that corresponds to this update,
+				// so for now, let's pass true for isNewest, which will result in the stacking of the block
+				// each time. Hopefully, because the track histories are added in separate transactions, the 
+				// block timestamp will be updated before we need to decide if we want to publish multiple
+				// user actions for each new track.
+				addTrackHistory(attached, getTrack(properties), new Date(virtualPlayTime), false, true);
 			}
 		});
 	}

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/StackerBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/StackerBean.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/StackerBean.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -733,7 +733,7 @@
 			public void run() throws RetryException {
 				DataService.getModel().initializeReadWriteSession(SystemViewpoint.getInstance());
 				
-				Pair<Long, StackReason> previousParticipation = new Pair<Long, StackReason>(Long.valueOf(-1), null);;
+				Pair<Long, StackReason> previousParticipation = new Pair<Long, StackReason>(Long.valueOf(-1), null);
 				
 				Block attached = em.find(Block.class, block.getId());
 				if (updateAllUserBlockDatas) {

Modified: dumbhippo/trunk/server/src/com/dumbhippo/services/AmazonWebServices.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/services/AmazonWebServices.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/services/AmazonWebServices.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -227,7 +227,7 @@
 		return amazonList;	
 	}
 	
-	// This methos will return null of the list is no longer accessible through
+	// This method will return null of the list is no longer accessible through
 	// ECS (perhaps it was made private).
 	public AmazonList getListDetails(String listId) {
 	    String wsUrl = createListRequest(listId, false, -1);	

Modified: dumbhippo/trunk/server/src/com/dumbhippo/services/FacebookWebServices.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/services/FacebookWebServices.java	2008-01-22 19:37:37 UTC (rev 7249)
+++ dumbhippo/trunk/server/src/com/dumbhippo/services/FacebookWebServices.java	2008-01-23 18:16:17 UTC (rev 7250)
@@ -389,8 +389,18 @@
 	        facebookClient.feed_publishTemplatizedAction(Integer.valueOf(facebookAccount.getFacebookUserId()), titleTemplate, 
 	        		                                     titleData, null, null, null, null, null);
 		} catch (FacebookException e) {
-			logger.error("FacebookException when publishing user action for {}: {}",
-					     facebookAccount.getFacebookUserId(), e.toString());
+			if (e.getCode() == FacebookSaxHandler.FacebookErrorCode.API_EC_TOO_MANY_CALLS.getCode()) {
+				// We are allowed to publish up to 10 user actions in a rolling 48-hours window on Facebook.
+				// Trying to publish additional actions should be harmless, but if necessary, we can add two
+				// more fields to FacebookAccount: publishTime and publishCount. We would update the publishTime
+				// to now and reset the count if publishTime <= now - 48 hours. If publishCount = 10 and 
+				// publishTime > now - 48 hours, we would not publish the user action.
+				logger.warn("The maximum number of actions allowed has been published for {} on Facebook",
+					        facebookAccount.getFacebookUserId());
+		    } else {
+			    logger.error("FacebookException when publishing user action for {}: {}",
+					         facebookAccount.getFacebookUserId(), e.toString());
+		    }
 		} catch (IOException e) {
 			logger.error("IOException when converting {} to an integer", facebookAccount.getFacebookUserId());
 		}