r7272 - in dumbhippo/trunk/server/src/com/dumbhippo/server: dm impl

[commits mugshot org]

Author: otaylor
Date: 2008-01-28 14:23:32 -0600 (Mon, 28 Jan 2008)
New Revision: 7272

Modified:
   dumbhippo/trunk/server/src/com/dumbhippo/server/dm/ContactDMO.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/IdentitySpiderBean.java
Log:
* Back out merging UserDMO information into ContactDMO ... I'm not sure it's 
  better this way, but it will confuse me when debugging if nothing else.

* Return ContactDMO.user correctly even when the contact points to a resource
  of the user other than Account.

* Send notifications for ContactDMO.user


Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/dm/ContactDMO.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/dm/ContactDMO.java	2008-01-28 16:19:00 UTC (rev 7271)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/dm/ContactDMO.java	2008-01-28 20:23:32 UTC (rev 7272)
@@ -13,25 +13,28 @@
 import com.dumbhippo.dm.annotations.Inject;
 import com.dumbhippo.identity20.Guid;
 import com.dumbhippo.persistence.Account;
+import com.dumbhippo.persistence.AccountClaim;
 import com.dumbhippo.persistence.AimResource;
 import com.dumbhippo.persistence.Contact;
 import com.dumbhippo.persistence.ContactClaim;
 import com.dumbhippo.persistence.EmailResource;
 import com.dumbhippo.persistence.Resource;
+import com.dumbhippo.persistence.User;
 import com.dumbhippo.persistence.XmppResource;
 import com.dumbhippo.server.NotFoundException;
 
 /** 
- * ContactDMO is not quite a straight wrapper of a Contact in the database, because 
- * we go ahead and merge the info from the User a contact refers to. This is done 
- * on the server side because if we had a web view of contacts, we'd want it to 
- * be consistent with the client.
+ * Currently ContactDMO is pretty much a straight wrapper of Contact and doesn't
+ * try to pull in information from the user, if the Contact can be resolved to
+ * a user. This has two advantages: first, it facilitates creating editing
+ * interfaces for a Contact since the editing interface can distinguish editable
+ * information from not-editable information. It also simplifies our work in
+ * creating the appropriate notifications and cache invalidations.
  * 
- * There are several downsides to this approach, though; the change notification 
- * is busted right now, i.e. changing the User won't update the Contact. In addition 
- * this approach means sending some info to the client twice, once in the User object
- * and once as part of the Contact.
- *
+ * The disadvantage is that every user who wants to display a contact with
+ * information merged in has to do the merge themselves. If we didn't mind
+ * doing the notification/invalidation work, we could duplicate properties
+ * with both merged and unmerged versions.
  */
 @DMO(classId="http://online.gnome.org/p/o/contact";, resourceBase="/o/contact")
 @DMFilter("viewer.canSeeContact(this)")
@@ -58,49 +61,12 @@
 
 	@DMProperty(defaultInclude=true)
 	public String getName() {
-		String nick = contact.getNickname();
-		if (nick == null) {
-			UserDMO user = getUser();
-			if (user != null)
-				nick = user.getName();
-		}
-		
-		// It should be safe to fall back to a full address of 
-		// some kind, since the contact is only visible to 
-		// the person who added the contact
-		
-		if (nick == null) {
-			Set<String> emails = getEmails();
-			if (!emails.isEmpty())
-				nick = emails.iterator().next();
-		}
-		
-		if (nick == null) {
-			Set<String> xmpps = getXmpps();
-			if (!xmpps.isEmpty())
-				nick = xmpps.iterator().next();
-		}		
-		
-		if (nick == null) {
-			Set<String> aims = getAims();
-			if (!aims.isEmpty())
-				nick = aims.iterator().next();
-		}
-		
-		if (nick == null) {
-			nick = "NO NAME"; // should not happen
-		}
-		
-		return nick;
+		return contact.getNickname();
 	}
 	
-	private UserDMO getUserDMOFromAccount(Account account) {
-		return session.findUnchecked(UserDMO.class, account.getOwner().getGuid());
-	}
-	
 	@DMProperty(defaultInclude=true)
 	public UserDMO getOwner() {
-		return getUserDMOFromAccount(contact.getAccount());
+		return session.findUnchecked(UserDMO.class, contact.getAccount().getOwner().getGuid());
 	}
 	
 	@DMProperty(defaultInclude=true)
@@ -118,13 +84,6 @@
 				results.add(((EmailResource)r).getEmail());
 		}
 		
-		// merge in any email from the User if we can see them
-		UserDMO user = getUser();
-		if (user != null) {
-			// user.getEmails() should be filtered to our viewpoint already
-			results.addAll(user.getEmails());
-		}
-		
 		return results;
 	}
 
@@ -138,13 +97,6 @@
 				results.add(((AimResource)r).getScreenName());
 		}
 		
-		// merge in any screen names from the User if we can see them
-		UserDMO user = getUser();
-		if (user != null) {
-			// user.getAims() should be filtered to our viewpoint already
-			results.addAll(user.getAims());
-		}
-		
 		return results;
 	}	
 	
@@ -158,24 +110,27 @@
 				results.add(((XmppResource)r).getJid());
 		}
 		
-		// merge in any xmpps from the User if we can see them
-		UserDMO user = getUser();
-		if (user != null) {
-			// user.getXmpps() should be filtered to our viewpoint already
-			results.addAll(user.getXmpps());
-		}
-		
 		return results;
 	}		
 	
 	@DMProperty(defaultInclude=true, defaultChildren="+")
 	public UserDMO getUser() {
+		User bestUser = null;
+		
 		for (ContactClaim cc : contact.getResources()) {
 			Resource r = cc.getResource();
-			if (r instanceof Account)
-				return getUserDMOFromAccount((Account)r);
+			AccountClaim ac = r.getAccountClaim();
+			
+			if (ac != null) {
+				// Always prefer claims directly on Account resources
+				if (r instanceof Account || bestUser == null);
+					bestUser = ac.getOwner();
+			}
 		}
 		
-		return null;
+		if (bestUser != null)
+			return session.findUnchecked(UserDMO.class, bestUser.getGuid());
+		else
+			return null;
 	}
 }

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/IdentitySpiderBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/IdentitySpiderBean.java	2008-01-28 16:19:00 UTC (rev 7271)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/IdentitySpiderBean.java	2008-01-28 20:23:32 UTC (rev 7272)
@@ -391,6 +391,10 @@
 			new UserClientMatcher(contacterUserId));
 	}
 	
+	public void invalidateContactUser(Contact contact) {
+		DataService.currentSessionRW().changed(ContactDMO.class, contact.getGuid(), "user");
+	}
+	
 	public void addVerifiedOwnershipClaim(User claimedOwner, Resource res) {
 
 		// first be sure it isn't a dup - the db constraints check this too,
@@ -430,20 +434,24 @@
 			DataService.currentSessionRW().changed(UserDMO.class, claimedOwner.getGuid(), "facebook");
 		
 		// People may have listed the newly claimed resource as a contact
-		Collection<Guid> newContacters = findResourceContacters(res);
-		if (!newContacters.isEmpty()) {
+		Collection<Contact> newContacts = findResourceContacts(res);
+		if (!newContacts.isEmpty()) {
 			LiveState.getInstance().invalidateContacters(claimedOwner.getGuid());
 			
-			for (Guid contacter : newContacters) {
-				invalidateContactStatus(contacter, claimedOwner.getGuid());
-				LiveState.getInstance().invalidateContacts(contacter);
+			for (Contact contact : newContacts) {
+				Guid contacterId = contact.getAccount().getOwner().getGuid();
+				
+				invalidateContactStatus(contacterId, claimedOwner.getGuid());
+				LiveState.getInstance().invalidateContacts(contacterId);
+				
+				invalidateContactUser(contact);
 			}
 		}						
 	}
 
 	public void removeVerifiedOwnershipClaim(UserViewpoint viewpoint,
 			User owner, Resource res) {
-		Collection<Guid> oldContacters = findResourceContacters(res);
+		Collection<Contact> oldContacts = findResourceContacts(res);
 		
 		if (!viewpoint.isOfUser(owner)) {
 			throw new RuntimeException(
@@ -473,12 +481,16 @@
 					DataService.currentSessionRW().changed(UserDMO.class, owner.getGuid(), "facebook");
 				
 				// People may have listed resource as a contact
-				if (!oldContacters.isEmpty()) {
+				if (!oldContacts.isEmpty()) {
 					LiveState.getInstance().invalidateContacters(owner.getGuid());
 					
-					for (Guid contacter : oldContacters) {
-						invalidateContactStatus(contacter, owner.getGuid());
-						LiveState.getInstance().invalidateContacts(contacter);
+					for (Contact contact : oldContacts) {
+						Guid contacterId = contact.getAccount().getOwner().getGuid();
+
+						invalidateContactStatus(contacterId, owner.getGuid());
+						LiveState.getInstance().invalidateContacts(contacterId);
+						
+						invalidateContactUser(contact);
 					}
 				}						
 				
@@ -491,22 +503,13 @@
 		claimVerifier.cancelClaimToken(owner, res);
 	}
 
-	private Collection<Guid> findResourceContacters(Resource res) {
-		Query q = em.createQuery("SELECT cc.account.owner.id " +
+	private Collection<Contact> findResourceContacts(Resource res) {
+		Query q = em.createQuery("SELECT cc.contact " +
 		 		                 "  FROM ContactClaim cc " +
 				                 "  WHERE cc.resource = :resource");
 		q.setParameter("resource", res);
 
-		Set<Guid> result = new HashSet<Guid>();
-		for (String id : TypeUtils.castList(String.class, q.getResultList())) {
-			try {
-				result.add(new Guid(id));
-			} catch (ParseException e) {
-				logger.error("Bad GUID in database: {}", id);
-			}
-		}
-		
-		return result;
+		return TypeUtils.castList(Contact.class, q.getResultList());
 	}
 
 	private Contact findContactByUser(User owner, User contactUser) throws NotFoundException {
@@ -607,6 +610,8 @@
 			
 			invalidateContactStatus(contactOwner.getGuid(), resourceOwner.getGuid());			
 			liveState.invalidateContacters(resourceOwner.getGuid());
+			
+			invalidateContactUser(contact);
 		}
 	}
 		
@@ -630,6 +635,8 @@
 		if (removedUser != null) {
 			invalidateContactStatus(contact.getAccount().getOwner().getGuid(), removedUser.getGuid());
 			liveState.invalidateContacters(removedUser.getGuid());
+			
+			invalidateContactUser(contact);
 		}
 		
 		// we could now have a 'bare' Contact with an empty set of resources