Clean up roster handling

src/XMPPConnection.h

@@ -52,14 +52,14 @@
 - (void)connection: (XMPPConnection*)conn
      wasBoundToJID: (XMPPJID*)jid;
 - (void)connectionDidReceiveRoster: (XMPPConnection*)conn;
--     (void)connection: (XMPPConnection*)conn
-  didReceiveRosterItem: (XMPPRosterItem*)rosterItem;
 - (BOOL)connection: (XMPPConnection*)conn
       didReceiveIQ: (XMPPIQ*)iq;
 -   (void)connection: (XMPPConnection*)conn
   didReceivePresence: (XMPPPresence*)pres;
 -  (void)connection: (XMPPConnection*)conn
   didReceiveMessage: (XMPPMessage*)msg;
+-     (void)connection: (XMPPConnection*)conn
+  didReceiveRosterItem: (XMPPRosterItem*)rosterItem;
 - (void)connectionWasClosed: (XMPPConnection*)conn;
 - (void)connectionWillUpgradeToTLS: (XMPPConnection*)conn;
 - (void)connectionDidUpgradeToTLS: (XMPPConnection*)conn;

src/XMPPConnection.m

@@ -650,19 +650,21 @@
 {
 	OFXMLElement *rosterElem;
 	OFXMLElement *elem;
+	XMPPRosterItem *rosterItem = nil;
 	OFString *subscription;
+	OFEnumerator *enumerator;
+	BOOL isPush = ![[iq ID] isEqual: rosterID];
 
 	rosterElem = [iq elementForName: @"query"
 			      namespace: XMPP_NS_ROSTER];
 
-	if ([[iq ID] isEqual: rosterID]) {
-		OFEnumerator *enumerator;
-
+	if (isPush)
+		assert([[iq type] isEqual: @"set"]);
+	else
 		assert([[iq type] isEqual: @"result"]);
 
 	enumerator = [[rosterElem children] objectEnumerator];
 	while ((elem = [enumerator nextObject]) != nil) {
-			XMPPRosterItem *rosterItem;
 		OFMutableArray *groups = [OFMutableArray array];
 		OFEnumerator *groupEnumerator;
 		OFXMLElement *groupElem;
@@ -679,10 +681,12 @@
 
 		subscription = [[elem attributeForName:
 			@"subscription"] stringValue];
-			if ([subscription isEqual: @"none"] ||
-			    [subscription isEqual: @"to"] ||
-			    [subscription isEqual: @"from"] ||
-			    [subscription isEqual: @"both"])
+		if (![subscription isEqual: @"none"] &&
+		    ![subscription isEqual: @"to"] &&
+		    ![subscription isEqual: @"from"] &&
+		    ![subscription isEqual: @"both"] &&
+		    (![subscription isEqual: @"remove"] || !isPush))
+			subscription = @"none";
 		[rosterItem setSubscription: subscription];
 
 		groupEnumerator =
@@ -696,60 +700,29 @@
 		if ([groups count] > 0)
 			[rosterItem setGroups: groups];
 
+		if ([subscription isEqual: @"remove"])
+			[roster XMPP_deleteRosterItem: rosterItem];
+		else
 			[roster XMPP_addRosterItem: rosterItem];
+
+		if (isPush && [delegate respondsToSelector:
+		    @selector(connection:didReceiveRosterItem:)])
+			[delegate connection:self
+			didReceiveRosterItem: rosterItem];
 	}
 
+	if (isPush) {
+		XMPPIQ *response = [XMPPIQ IQWithType: @"result"
+						   ID: [iq ID]];
+		[response setTo: [iq from]];
+		[self sendStanza: response];
+	} else {
 		if ([delegate respondsToSelector:
 		    @selector(connectionDidReceiveRoster:)])
 			[delegate connectionDidReceiveRoster: self];
 
 		[rosterID release];
 		rosterID = nil;
-	} else {
-		XMPPRosterItem *rosterItem;
-		OFMutableArray *groups = [OFMutableArray array];
-		OFEnumerator *groupEnumerator;
-		OFXMLElement *groupElem;
-		XMPPIQ *response;
-
-		assert([[iq type] isEqual: @"set"]);
-
-		elem = [[rosterElem children] firstObject];
-		rosterItem = [XMPPRosterItem rosterItem];
-		[rosterItem setJID: [XMPPJID JIDWithString:
-		    [[elem attributeForName: @"jid"] stringValue]]];
-		[rosterItem setName:
-		    [[elem attributeForName: @"name"] stringValue]];
-
-		subscription = [[elem attributeForName:
-			@"subscription"] stringValue];
-		if ([subscription isEqual: @"none"] ||
-		    [subscription isEqual: @"to"] ||
-		    [subscription isEqual: @"from"] ||
-		    [subscription isEqual: @"both"] ||
-		    [subscription isEqual: @"remove"])
-			[rosterItem setSubscription: subscription];
-
-		groupEnumerator =
-		    [[elem elementsForName: @"group"
-				 namespace: XMPP_NS_ROSTER] objectEnumerator];
-		while ((groupElem = [groupEnumerator nextObject]) != nil)
-			[groups addObject: [groupElem stringValue]];
-
-		if ([groups count] > 0)
-			[rosterItem setGroups: groups];
-
-		[roster XMPP_updateRosterItem: rosterItem];
-
-		response = [XMPPIQ IQWithType: @"result"
-					   ID: [iq ID]];
-		[response setTo: [iq from]];
-		[self sendStanza: response];
-
-		if ([delegate respondsToSelector:
-		    @selector(connection:didReceiveRosterItem:)])
-			[delegate connection:self
-			didReceiveRosterItem: rosterItem];
 	}
 }
 
@@ -816,6 +789,11 @@
 {
 }
 
+-     (void)connection: (XMPPConnection*)conn
+  didReceiveRosterItem: (XMPPRosterItem*)rosterItem
+{
+}
+
 - (void)connectionWasClosed: (XMPPConnection*)conn
 {
 }

tests/test.m

@@ -123,7 +123,7 @@ OF_APPLICATION_DELEGATE(AppDelegate)
 	[conn requestRoster];
 }
 
-- (void)connectionDidReceiveRoster :(XMPPConnection*)conn
+- (void)connectionDidReceiveRoster: (XMPPConnection*)conn
 {
 	XMPPPresence *pres;