Rework Roster IQ and DirectoryRosterStore

remove (set|get)ItemStatus. This was always the ask status, which can
only be set to 'subscribe' or is non-existent.

Use the standard (de-)serialization facilities for DirectoryRosterStore.

Fixes Smack-657.

smack-core/src/main/java/org/jivesoftware/smack/util/FileUtils.java

@@ -132,13 +132,16 @@ public final class FileUtils {
         return null;
     }
 
-    public static void writeFileOrThrow(File file, String content) throws IOException {
+    public static void writeFileOrThrow(File file, CharSequence content) throws IOException {
         FileWriter writer = new FileWriter(file, false);
-        writer.write(content);
+        try {
+            writer.write(content.toString());
+        } finally {
             writer.close();
         }
+    }
 
-    public static boolean writeFile(File file, String content) {
+    public static boolean writeFile(File file, CharSequence content) {
         try {
             writeFileOrThrow(file, content);
             return true;

smack-core/src/main/java/org/jivesoftware/smack/util/ParserUtils.java

@@ -43,6 +43,11 @@ public class ParserUtils {
         assert(parser.getEventType() == XmlPullParser.START_TAG);
     }
 
+    public static void assertAtStartTag(XmlPullParser parser, String name) throws XmlPullParserException {
+        assertAtStartTag(parser);
+        assert name.equals(parser.getName());
+    }
+
     public static void assertAtEndTag(XmlPullParser parser) throws XmlPullParserException {
         assert(parser.getEventType() == XmlPullParser.END_TAG);
     }

smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java

@@ -1396,8 +1396,7 @@ public final class Roster extends Manager {
                 }
 
                 for (RosterPacket.Item item : validItems) {
-                    RosterEntry entry = new RosterEntry(item.getJid(), item.getName(),
+                    RosterEntry entry = new RosterEntry(item, Roster.this, connection);
-                            item.getItemType(), item.getItemStatus(), item.isApproved(), Roster.this, connection);
                     addUpdateEntry(addedEntries, updatedEntries, unchangedEntries, item, entry);
                 }
 
@@ -1426,8 +1425,7 @@ public final class Roster extends Manager {
                 // version we presented the server. So we simply load the roster from the store and
                 // await possible further roster pushes.
                 for (RosterPacket.Item item : rosterStore.getEntries()) {
-                    RosterEntry entry = new RosterEntry(item.getJid(), item.getName(),
+                    RosterEntry entry = new RosterEntry(item, Roster.this, connection);
-                            item.getItemType(), item.getItemStatus(), item.isApproved(), Roster.this, connection);
                     addUpdateEntry(addedEntries, updatedEntries, unchangedEntries, item, entry);
                 }
             }
@@ -1496,8 +1494,7 @@ public final class Roster extends Manager {
             // We assured above that the size of items is exaclty 1, therefore we are able to
             // safely retrieve this single item here.
             Item item = items.iterator().next();
-            RosterEntry entry = new RosterEntry(item.getJid(), item.getName(),
+            RosterEntry entry = new RosterEntry(item, Roster.this, connection);
-                            item.getItemType(), item.getItemStatus(), item.isApproved(), Roster.this, connection);
             String version = rosterPacket.getVersion();
 
             if (item.getItemType().equals(RosterPacket.ItemType.remove)) {

smack-im/src/main/java/org/jivesoftware/smack/roster/RosterEntry.java

@@ -37,38 +37,23 @@ import org.jxmpp.jid.BareJid;
  * JID and a name or nickname you assign.
  *
  * @author Matt Tucker
+ * @author Florian Schmaus
  */
 public final class RosterEntry extends Manager {
 
-    /**
+    private final RosterPacket.Item item;
-     * The JID of the entity/user.
-     */
-    private final BareJid jid;
-
-    private String name;
-    private RosterPacket.ItemType type;
-    private RosterPacket.ItemStatus status;
-    private final boolean approved;
     final private Roster roster;
 
     /**
      * Creates a new roster entry.
      *
-     * @param user the user.
+     * @param item the Roster Stanza's Item entry.
-     * @param name the nickname for the entry.
+     * @param roster The Roster managing this entry.
-     * @param type the subscription type.
-     * @param status the subscription status (related to subscriptions pending to be approbed).
-     * @param approved the pre-approval flag.
      * @param connection a connection to the XMPP server.
      */
-    RosterEntry(BareJid user, String name, RosterPacket.ItemType type,
+    RosterEntry(RosterPacket.Item item, Roster roster, XMPPConnection connection) {
-                RosterPacket.ItemStatus status, boolean approved, Roster roster, XMPPConnection connection) {
         super(connection);
-        this.jid = user;
+        this.item = item;
-        this.name = name;
-        this.type = type;
-        this.status = status;
-        this.approved = approved;
         this.roster = roster;
     }
 
@@ -80,7 +65,7 @@ public final class RosterEntry extends Manager {
      */
     @Deprecated
     public String getUser() {
-        return jid.toString();
+        return getJid().toString();
     }
 
     /**
@@ -89,7 +74,7 @@ public final class RosterEntry extends Manager {
      * @return the user associated with this entry.
      */
     public BareJid getJid() {
-        return jid;
+        return item.getJid();
     }
 
     /**
@@ -98,7 +83,7 @@ public final class RosterEntry extends Manager {
      * @return the name.
      */
     public String getName() {
-        return name;
+        return item.getName();
     }
 
     /**
@@ -112,7 +97,7 @@ public final class RosterEntry extends Manager {
      */
     public synchronized void setName(String name) throws NotConnectedException, NoResponseException, XMPPErrorException, InterruptedException {
         // Do nothing if the name hasn't changed.
-        if (name != null && name.equals(this.name)) {
+        if (name != null && name.equals(getName())) {
             return;
         }
 
@@ -125,7 +110,7 @@ public final class RosterEntry extends Manager {
         connection().createPacketCollectorAndSend(packet).nextResultOrThrow();
 
         // We have received a result response to the IQ set, the name was successfully changed
-        this.name = name;
+        item.setName(name);
     }
 
     /**
@@ -133,12 +118,12 @@ public final class RosterEntry extends Manager {
      *
      * @param name the nickname for the entry.
      * @param type the subscription type.
-     * @param status the subscription status (related to subscriptions pending to be approbed).
+     * @param subscriptionPending TODO
      */
-    void updateState(String name, RosterPacket.ItemType type, RosterPacket.ItemStatus status) {
+    void updateState(String name, RosterPacket.ItemType type, boolean subscriptionPending) {
-        this.name = name;
+        item.setName(name);
-        this.type = type;
+        item.setItemType(type);
-        this.status = status;
+        item.setSubscriptionPending(subscriptionPending);
     }
 
     /**
@@ -147,7 +132,7 @@ public final class RosterEntry extends Manager {
      * @return the pre-approval state.
      */
     public boolean isApproved() {
-        return approved;
+        return item.isApproved();
     }
 
     /**
@@ -176,26 +161,27 @@ public final class RosterEntry extends Manager {
      * @return the type.
      */
     public RosterPacket.ItemType getType() {
-        return type;
+        return item.getItemType();
     }
 
     /**
-     * Returns the roster subscription status of the entry. When the status is
+     * Returns the roster subscription request status of the entry. If
-     * RosterPacket.ItemStatus.SUBSCRIPTION_PENDING, the contact has to answer the
+     * {@code true}, then the contact did not answer the subscription request
-     * subscription request.
+     * yet.
      *
      * @return the status.
+     * @since 4.2
      */
-    public RosterPacket.ItemStatus getStatus() {
+    public boolean isSubscriptionPending() {
-        return status;
+        return item.isSubscriptionPending();
     }
 
     public String toString() {
         StringBuilder buf = new StringBuilder();
-        if (name != null) {
+        if (getName() != null) {
-            buf.append(name).append(": ");
+            buf.append(getName()).append(": ");
         }
-        buf.append(jid);
+        buf.append(getJid());
         Collection<RosterGroup> groups = getGroups();
         if (!groups.isEmpty()) {
             buf.append(" [");
@@ -214,7 +200,7 @@ public final class RosterEntry extends Manager {
 
     @Override
     public int hashCode() {
-        return (jid == null ? 0 : jid.hashCode());
+        return getJid().hashCode();
     }
 
     public boolean equals(Object object) {
@@ -222,7 +208,7 @@ public final class RosterEntry extends Manager {
             return true;
         }
         if (object != null && object instanceof RosterEntry) {
-            return jid.equals(((RosterEntry)object).getUser());
+            return getJid().equals(((RosterEntry)object).getJid());
         }
         else {
             return false;
@@ -246,33 +232,7 @@ public final class RosterEntry extends Manager {
         if (getClass() != obj.getClass())
             return false;
         RosterEntry other = (RosterEntry) obj;
-        if (name == null) {
+        return other.item.equals(this.item);
-            if (other.name != null)
-                return false;
-        }
-        else if (!name.equals(other.name))
-            return false;
-        if (status == null) {
-            if (other.status != null)
-                return false;
-        }
-        else if (!status.equals(other.status))
-            return false;
-        if (type == null) {
-            if (other.type != null)
-                return false;
-        }
-        else if (!type.equals(other.type))
-            return false;
-        if (jid == null) {
-            if (other.jid != null)
-                return false;
-        }
-        else if (!jid.equals(other.jid))
-            return false;
-        if (approved != other.approved)
-            return false;
-        return true;
     }
 
     static RosterPacket.Item toRosterItem(RosterEntry entry) {
@@ -282,7 +242,7 @@ public final class RosterEntry extends Manager {
     private static RosterPacket.Item toRosterItem(RosterEntry entry, String name) {
         RosterPacket.Item item = new RosterPacket.Item(entry.getJid(), name);
         item.setItemType(entry.getType());
-        item.setItemStatus(entry.getStatus());
+        item.setSubscriptionPending(entry.isSubscriptionPending());
         item.setApproved(entry.isApproved());
         // Set the correct group names for the item.
         for (RosterGroup group : entry.getGroups()) {

smack-im/src/main/java/org/jivesoftware/smack/roster/packet/RosterPacket.java

@@ -18,14 +18,17 @@
 package org.jivesoftware.smack.roster.packet;
 
 import org.jivesoftware.smack.packet.IQ;
+import org.jivesoftware.smack.packet.NamedElement;
 import org.jivesoftware.smack.packet.Stanza;
 import org.jivesoftware.smack.util.Objects;
+import org.jivesoftware.smack.util.StringUtils;
 import org.jivesoftware.smack.util.XmlStringBuilder;
 import org.jxmpp.jid.BareJid;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArraySet;
 
@@ -33,6 +36,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
  * Represents XMPP roster packets.
  *
  * @author Matt Tucker
+ * @author Florian Schmaus
  */
 public class RosterPacket extends IQ {
 
@@ -104,31 +108,56 @@ public class RosterPacket extends IQ {
      * A roster item, which consists of a JID, their name, the type of subscription, and
      * the groups the roster item belongs to.
      */
-    public static class Item {
+    public static class Item implements NamedElement {
+
+        /**
+         * The constant value "{@value}".
+         */
+        public static final String ELEMENT = Stanza.ITEM;
 
         public static final String GROUP = "group";
 
         private final BareJid jid;
+
+        /**
+         * TODO describe me. With link to the RFC. Is ask= attribute.
+         */
+        private boolean subscriptionPending;
+
         private String name;
-        private ItemType itemType;
+        private ItemType itemType = ItemType.none;
-        private ItemStatus itemStatus;
         private boolean approved;
         private final Set<String> groupNames;
 
         /**
          * Creates a new roster item.
          *
-         * @param jid the jid.
+         * @param jid
-         * @param name the user's name.
+         * @param name
          */
         public Item(BareJid jid, String name) {
+            this(jid, name, false);
+        }
+
+        /**
+         * Creates a new roster item.
+         *
+         * @param jid the jid.
+         * @param name the user's name.
+         * @param subscriptionPending
+         */
+        public Item(BareJid jid, String name, boolean subscriptionPending) {
             this.jid = Objects.requireNonNull(jid);
             this.name = name;
-            itemType = null;
+            this.subscriptionPending = subscriptionPending;
-            itemStatus = null;
             groupNames = new CopyOnWriteArraySet<String>();
         }
 
+        @Override
+        public String getElementName() {
+            return ELEMENT;
+        }
+
         /**
          * Returns the user.
          *
@@ -182,28 +211,17 @@ public class RosterPacket extends IQ {
          * @param itemType the roster item type.
          */
         public void setItemType(ItemType itemType) {
-            this.itemType = itemType;
+            this.itemType = Objects.requireNonNull(itemType, "itemType must not be null");
         }
 
-        /**
+        public void setSubscriptionPending(boolean subscriptionPending) {
-         * Returns the roster item status.
+            this.subscriptionPending = subscriptionPending;
-         *
-         * @return the roster item status.
-         */
-        public ItemStatus getItemStatus() {
-            return itemStatus;
         }
 
-        /**
+        public boolean isSubscriptionPending() {
-         * Sets the roster item status.
+            return subscriptionPending;
-         *
-         * @param itemStatus the roster item status.
-         */
-        public void setItemStatus(ItemStatus itemStatus) {
-            this.itemStatus = itemStatus;
         }
 
-
         /**
          * Returns the roster item pre-approval state.
          *
@@ -251,18 +269,20 @@ public class RosterPacket extends IQ {
         }
 
         public XmlStringBuilder toXML() {
-            XmlStringBuilder xml = new XmlStringBuilder();
+            XmlStringBuilder xml = new XmlStringBuilder(this);
-            xml.halfOpenElement(Stanza.ITEM).attribute("jid", jid);
+            xml.attribute("jid", jid);
             xml.optAttribute("name", name);
             xml.optAttribute("subscription", itemType);
-            xml.optAttribute("ask", itemStatus);
+            if (subscriptionPending) {
+                xml.append(" ask='subscribe'");
+            }
             xml.optBooleanAttribute("approved", approved);
             xml.rightAngleBracket();
 
             for (String groupName : groupNames) {
                 xml.openElement(GROUP).escape(groupName).closeElement(GROUP);
             }
-            xml.closeElement(Stanza.ITEM);
+            xml.closeElement(this);
             return xml;
         }
 
@@ -271,7 +291,7 @@ public class RosterPacket extends IQ {
             final int prime = 31;
             int result = 1;
             result = prime * result + ((groupNames == null) ? 0 : groupNames.hashCode());
-            result = prime * result + ((itemStatus == null) ? 0 : itemStatus.hashCode());
+            result = prime * result + ((subscriptionPending) ? 0 : 1);
             result = prime * result + ((itemType == null) ? 0 : itemType.hashCode());
             result = prime * result + ((name == null) ? 0 : name.hashCode());
             result = prime * result + ((jid == null) ? 0 : jid.hashCode());
@@ -294,7 +314,7 @@ public class RosterPacket extends IQ {
             }
             else if (!groupNames.equals(other.groupNames))
                 return false;
-            if (itemStatus != other.itemStatus)
+            if (subscriptionPending != other.subscriptionPending)
                 return false;
             if (itemType != other.itemType)
                 return false;
@@ -317,37 +337,6 @@ public class RosterPacket extends IQ {
 
     }
 
-    /**
-     * The subscription status of a roster item. An optional element that indicates
-     * the subscription status if a change request is pending.
-     */
-    public static enum ItemStatus {
-        /**
-         * Request to subscribe.
-         */
-        subscribe,
-
-        /**
-         * Request to unsubscribe.
-         */
-        unsubscribe;
-
-        public static final ItemStatus SUBSCRIPTION_PENDING = subscribe;
-        public static final ItemStatus UNSUBSCRIPTION_PENDING = unsubscribe;
-
-        public static ItemStatus fromString(String s) {
-            if (s == null) {
-                return null;
-            }
-            try {
-                return ItemStatus.valueOf(s);
-            }
-            catch (IllegalArgumentException e) {
-                return null;
-            }
-        }
-    }
-
     public static enum ItemType {
 
         /**
@@ -378,6 +367,14 @@ public class RosterPacket extends IQ {
         /**
          * The user wishes to stop receiving presence updates from the subscriber.
          */
-        remove
+        remove,
+        ;
+
+        public static ItemType fromString(String string) {
+            if (StringUtils.isNullOrEmpty(string)) {
+                return none;
+            }
+            return ItemType.valueOf(string.toLowerCase(Locale.US));
+        }
     }
 }

smack-im/src/main/java/org/jivesoftware/smack/roster/provider/RosterPacketProvider.java

@@ -19,6 +19,7 @@ package org.jivesoftware.smack.roster.provider;
 import java.io.IOException;
 
 import org.jivesoftware.smack.SmackException;
+import org.jivesoftware.smack.packet.IQ;
 import org.jivesoftware.smack.provider.IQProvider;
 import org.jivesoftware.smack.roster.packet.RosterPacket;
 import org.jivesoftware.smack.util.ParserUtils;
@@ -35,8 +36,6 @@ public class RosterPacketProvider extends IQProvider<RosterPacket> {
     public RosterPacket parse(XmlPullParser parser, int initialDepth) throws XmlPullParserException, IOException,
                     SmackException {
         RosterPacket roster = new RosterPacket();
-        RosterPacket.Item item = null;
-
         String version = parser.getAttributeValue("", "ver");
         roster.setVersion(version);
 
@@ -47,27 +46,51 @@ public class RosterPacketProvider extends IQProvider<RosterPacket> {
                 String startTag = parser.getName();
                 switch (startTag) {
                 case "item":
-                    String jidString = parser.getAttributeValue("", "jid");
+                    RosterPacket.Item item = parseItem(parser);
-                    String name = parser.getAttributeValue("", "name");
+                    roster.addRosterItem(item);
+                    break;
+                }
+                break;
+            case XmlPullParser.END_TAG:
+                String endTag = parser.getName();
+                switch(endTag) {
+                case IQ.QUERY_ELEMENT:
+                    if (parser.getDepth() == initialDepth) {
+                        break outerloop;
+                    }
+                }
+            }
+        }
+        return roster;
+    }
 
+    public static RosterPacket.Item parseItem(XmlPullParser parser) throws XmlPullParserException, IOException {
+        ParserUtils.assertAtStartTag(parser, RosterPacket.Item.ELEMENT);
+        final int initialDepth = parser.getDepth();
+        String jidString = parser.getAttributeValue("", "jid");
+        String itemName = parser.getAttributeValue("", "name");
         BareJid jid = JidCreate.bareFrom(jidString);
 
-                    // Create packet.
+        // Create item.
-                    item = new RosterPacket.Item(jid, name);
+        RosterPacket.Item item = new RosterPacket.Item(jid, itemName);
         // Set status.
         String ask = parser.getAttributeValue("", "ask");
-                    RosterPacket.ItemStatus status = RosterPacket.ItemStatus.fromString(ask);
+        item.setSubscriptionPending("subscribe".equals(ask));
-                    item.setItemStatus(status);
         // Set type.
         String subscription = parser.getAttributeValue("", "subscription");
-                    RosterPacket.ItemType type = RosterPacket.ItemType.valueOf(subscription != null ? subscription : "none");
+        RosterPacket.ItemType type = RosterPacket.ItemType.fromString(subscription);
         item.setItemType(type);
         // Set approval status.
         boolean approved = ParserUtils.getBooleanAttribute(parser, "approved", false);
         item.setApproved(approved);
-                    break;
+
-                case "group":
+        outerloop: while(true) {
-                    // TODO item!= null
+            int eventType = parser.next();
+            switch (eventType) {
+            case XmlPullParser.START_TAG:
+                String name = parser.getName();
+                switch (name) {
+                case RosterPacket.Item.GROUP:
                     final String groupName = parser.nextText();
                     if (groupName != null && groupName.trim().length() > 0) {
                         item.addGroupName(groupName);
@@ -76,17 +99,14 @@ public class RosterPacketProvider extends IQProvider<RosterPacket> {
                 }
                 break;
             case XmlPullParser.END_TAG:
-                String endTag = parser.getName();
+                if (parser.getDepth() == initialDepth) {
-                switch(endTag) {
-                case "item":
-                    roster.addRosterItem(item);
-                    break;
-                case "query":
                     break outerloop;
                 }
+                break;
             }
         }
-        return roster;
+        ParserUtils.assertAtEndTag(parser);
+        assert(item != null);
+        return item;
     }
-
 }

smack-im/src/main/java/org/jivesoftware/smack/roster/rosterstore/DirectoryRosterStore.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 the original author or authors
+ * Copyright 2013-2015 the original author or authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,8 +18,10 @@ package org.jivesoftware.smack.roster.rosterstore;
 
 import java.io.File;
 import java.io.FileFilter;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
 import java.io.IOException;
-import java.io.StringReader;
+import java.io.Reader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -28,13 +30,11 @@ import java.util.logging.Logger;
 
 import org.jivesoftware.smack.roster.packet.RosterPacket;
 import org.jivesoftware.smack.roster.packet.RosterPacket.Item;
+import org.jivesoftware.smack.roster.provider.RosterPacketProvider;
 import org.jivesoftware.smack.util.FileUtils;
-import org.jivesoftware.smack.util.XmlStringBuilder;
+import org.jivesoftware.smack.util.PacketParserUtils;
 import org.jivesoftware.smack.util.stringencoder.Base32;
-import org.jxmpp.jid.BareJid;
 import org.jxmpp.jid.Jid;
-import org.jxmpp.jid.impl.JidCreate;
-import org.xmlpull.v1.XmlPullParserFactory;
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
 
@@ -44,6 +44,7 @@ import org.xmlpull.v1.XmlPullParserException;
  *
  * @author Lars Noschinski
  * @author Fabian Schuetz
+ * @author Florian Schmaus
  */
 public final class DirectoryRosterStore implements RosterStore {
 
@@ -179,136 +180,33 @@ public final class DirectoryRosterStore implements RosterStore {
     }
 
     private static Item readEntry(File file) {
-        String s = FileUtils.readFile(file);
+        Reader reader;
-        if (s == null) {
+        try {
+            reader = new FileReader(file);
+        } catch (FileNotFoundException e) {
+            LOGGER.log(Level.FINE, "Roster entry file not found", e);
             return null;
         }
 
-        String parserName;
-        BareJid user = null;
-        String name = null;
-        String type = null;
-        String status = null;
-        String approved = null;
-
-        List<String> groupNames = new ArrayList<String>();
-
         try {
-            XmlPullParser parser = XmlPullParserFactory.newInstance().newPullParser();
+            XmlPullParser parser = PacketParserUtils.getParserFor(reader);
-            parser.setInput(new StringReader(s));
+            Item item = RosterPacketProvider.parseItem(parser);
-
+            reader.close();
-            boolean done = false;
-            while (!done) {
-                int eventType = parser.next();
-                parserName = parser.getName();
-                if (eventType == XmlPullParser.START_TAG) {
-                    if (parserName.equals("item")) {
-                        user = null;
-                        name = type = status = null;
-                    }
-                    else if (parserName.equals("user")) {
-                        parser.next();
-
-                        String jidString = parser.getText();
-                        user = JidCreate.bareFrom(jidString);
-                    }
-                    else if (parserName.equals("name")) {
-                        parser.next();
-                        name = parser.getText();
-                    }
-                    else if (parserName.equals("type")) {
-                        parser.next();
-                        type = parser.getText();
-                    }
-                    else if (parserName.equals("status")) {
-                        parser.next();
-                        status = parser.getText();
-                    }
-                    else if (parserName.equals("approved")) {
-                        parser.next();
-                        approved = parser.getText();
-                    }
-                    else if (parserName.equals("group")) {
-                        parser.next();
-                        parser.next();
-                        String group = parser.getText();
-                        if (group != null) {
-                            groupNames.add(group);
-                        }
-                        else {
-                            LOGGER.severe("Invalid group entry in store entry file "
-                                    + file);
-                        }
-                    }
-                }
-                else if (eventType == XmlPullParser.END_TAG) {
-                    if (parserName.equals("item")) {
-                        done = true;
-                    }
-                }
-            }
-        }
-        catch (IOException e) {
-            LOGGER.log(Level.SEVERE, "readEntry()", e);
-            return null;
-        }
-        catch (XmlPullParserException e) {
-            LOGGER.log(Level.SEVERE, "Invalid group entry in store entry file", e);
-            return null;
-        }
-
-        if (user == null) {
-            return null;
-        }
-        RosterPacket.Item item = new RosterPacket.Item(user, name);
-        for (String groupName : groupNames) {
-            item.addGroupName(groupName);
-        }
-
-        if (type != null) {
-            try {
-                item.setItemType(RosterPacket.ItemType.valueOf(type));
-            }
-            catch (IllegalArgumentException e) {
-                LOGGER.log(Level.SEVERE, "Invalid type in store entry file " + file, e);
-                return null;
-            }
-            if (status != null) {
-                RosterPacket.ItemStatus itemStatus = RosterPacket.ItemStatus
-                        .fromString(status);
-                if (itemStatus == null) {
-                    LOGGER.severe("Invalid status in store entry file " + file);
-                    return null;
-                }
-                item.setItemStatus(itemStatus);
-            }
-        }
-        if (approved != null) {
-            item.setApproved(Boolean.parseBoolean(approved));
-        }
-
             return item;
+        } catch (XmlPullParserException | IOException e) {
+            boolean deleted = file.delete();
+            String message = "Exception while parsing roster entry.";
+            if (deleted) {
+                message += " File was deleted.";
+            }
+            LOGGER.log(Level.SEVERE, message, e);
+            return null;
+        }
     }
-
 
     private boolean addEntryRaw (Item item) {
-        XmlStringBuilder xml = new XmlStringBuilder();
+        return FileUtils.writeFile(getBareJidFile(item.getJid()), item.toXML());
-        xml.openElement("item");
-        xml.element("user", item.getJid());
-        xml.optElement("name", item.getName());
-        xml.optElement("type", item.getItemType());
-        xml.optElement("status", item.getItemStatus());
-        xml.optElement("approved", Boolean.toString(item.isApproved()));
-        for (String groupName : item.getGroupNames()) {
-            xml.openElement("group");
-            xml.element("groupName", groupName);
-            xml.closeElement("group");
     }
-        xml.closeElement("item");
-
-        return FileUtils.writeFile(getBareJidFile(item.getJid()), xml.toString());
-    }
-
 
     private File getBareJidFile(Jid bareJid) {
         String encodedJid = Base32.encode(bareJid.toString());

smack-im/src/test/java/org/jivesoftware/smack/roster/rosterstore/DirectoryRosterStoreTest.java

@@ -27,7 +27,6 @@ import java.util.List;
 
 import org.jivesoftware.smack.roster.packet.RosterPacket;
 import org.jivesoftware.smack.roster.packet.RosterPacket.Item;
-import org.jivesoftware.smack.roster.packet.RosterPacket.ItemStatus;
 import org.jivesoftware.smack.roster.packet.RosterPacket.ItemType;
 import org.junit.After;
 import org.junit.Before;
@@ -106,7 +105,7 @@ public class DirectoryRosterStoreTest {
         assertEquals("ItemType of added entry",
                 item1.getItemType(), storedItem.getItemType());
         assertEquals("ItemStatus of added entry",
-                item1.getItemStatus(), storedItem.getItemStatus());
+                item1.isSubscriptionPending(), storedItem.isSubscriptionPending());
         assertEquals("Approved of added entry",
                 item1.isApproved(), storedItem.isApproved());
 
@@ -115,7 +114,7 @@ public class DirectoryRosterStoreTest {
         final RosterPacket.Item item2 = new Item(userName, "Ursula Example");
         item2.addGroupName("users");
         item2.addGroupName("examples");
-        item2.setItemStatus(ItemStatus.subscribe);
+        item2.setSubscriptionPending(true);
         item2.setItemType(ItemType.none);
         item2.setApproved(true);
         store.addEntry(item2,version2);
@@ -130,7 +129,7 @@ public class DirectoryRosterStoreTest {
         assertEquals("ItemType of added entry",
                 item2.getItemType(), storedItem.getItemType());
         assertEquals("ItemStatus of added entry",
-                item2.getItemStatus(), storedItem.getItemStatus());
+                item2.isSubscriptionPending(), storedItem.isSubscriptionPending());
         assertEquals("Approved of added entry",
                 item2.isApproved(), storedItem.isApproved());
 
@@ -140,13 +139,13 @@ public class DirectoryRosterStoreTest {
         final RosterPacket.Item item3 = new Item(JidTestUtil.BARE_JID_1, "Foo Bar");
         item3.addGroupName("The Foo Fighters");
         item3.addGroupName("Bar Friends");
-        item3.setItemStatus(ItemStatus.unsubscribe);
+        item3.setSubscriptionPending(true);
         item3.setItemType(ItemType.both);
 
         final RosterPacket.Item item4 = new Item(JidTestUtil.BARE_JID_2, "Baba Baz");
         item4.addGroupName("The Foo Fighters");
         item4.addGroupName("Bar Friends");
-        item4.setItemStatus(ItemStatus.subscribe);
+        item4.setSubscriptionPending(false);
         item4.setItemType(ItemType.both);
         item4.setApproved(true);
 
@@ -167,7 +166,7 @@ public class DirectoryRosterStoreTest {
         assertEquals("ItemType of added entry",
                 item3.getItemType(), storedItem.getItemType());
         assertEquals("ItemStatus of added entry",
-                item3.getItemStatus(), storedItem.getItemStatus());
+                item3.isSubscriptionPending(), storedItem.isSubscriptionPending());
         assertEquals("Approved of added entry",
                 item3.isApproved(), storedItem.isApproved());
 
@@ -182,7 +181,7 @@ public class DirectoryRosterStoreTest {
         assertEquals("ItemType of added entry",
                 item4.getItemType(), storedItem.getItemType());
         assertEquals("ItemStatus of added entry",
-                item4.getItemStatus(), storedItem.getItemStatus());
+                item4.isSubscriptionPending(), storedItem.isSubscriptionPending());
         assertEquals("Approved of added entry",
                 item4.isApproved(), storedItem.isApproved());