HipChat mention_name/email only available on Contact

Issue #76 closed
Andrew Potter created an issue

HipChat isn't a very good XMPP citizen, and instead of using extensions made the decision to pass extra info on the Contact object, specifically the mention_name and email fields.

I was able to access these values by adding the following fields and annotations to rocks.xmpp.im.roster.model.Contact:

    @XmlAttribute(name = "mention_name")
    private final String mentionName;

    @XmlAttribute
    private final String email;

This doesn't seem like a great solution, since it's not in the spec and it's tailored to one xmpp provider. Is there a better way of doing this? Access to the mention name is very important to me, so it would be great if there were a way to override which class JAXB uses to deserialize Contacts so we could add our own subclass at runtime, or somehow hook into the deserialization to maintain a map of mention names somewhere else.

Unfortunately I don't know nearly enough about JAXB to know if any of this is possible or which would be the best approach.

Comments (20)

  1. Christian Schudt repo owner

    I've never tested it, but you could try this:

    XmppSessionConfiguration configuration = XmppSessionConfiguration.builder()
                            .extensions(Extension.of(YourRosterClass.class))
                            .build();
    

    But I am not sure, how JAXB behaves, when it suddenly finds multiple classes for the same XML element.

  2. Andrew Potter reporter

    This doesn't seem to work - the IQ's extension is still a rocks.xmpp.im.roster.model.Roster. I also tried removed Roster.class from CoreModule:76 with the same result.

  3. Christian Schudt repo owner

    I guess, you missed something. I've also tested it and I get: com.sun.xml.internal.bind.v2.runtime.IllegalAnnotationsException as soon there are two classes with the same namespace/element.

    If you remove Roster.class from CoreModule, I get a javax.xml.bind.JAXBException

    I'll see what we can do.

  4. Christian Schudt repo owner

    Allow custom extensions to override predefined extensions.

    This will only work for some classes now (those which are registered with a namespace, e.g. the Roster).

    It's achieved by first removing all extensions, which are redefined, then re-add them, which basically overwrites predefined extensions.

    Also only request roster during login, if the manager is enabled.

    Fixes issue #76

    → <<cset 75eca0c11f98>>

  5. Christian Schudt repo owner

    It will still be a bit cumbersome, but you can now define your own Roster classes: Create or copy the roster package, which contains the current roster classes, modify them, then overwrite the default roster classes:

    XmppSessionConfiguration configuration = XmppSessionConfiguration.builder()
        .extensions(Extension.of(Roster.NAMESPACE, true, MyRoster.class))
        .build();
    

    Roster-related classes are idenfitied by the Roster.NAMESPACE, so the new extension replaces the default one. Note that the login method uses the default RosterManager to retrieve the Roster during login. That no longer works and you would have to do it manually.

    Maybe not the best solution (I also thought about introducing a roster interface), but I don't want to stretch the code, just for some proprietary implementation.

  6. Christian Schudt repo owner

    Btw.: If you are able + willing to test a snapshot build, I can upload it to Maven Snapshot repository. Let me know. Alternatively you could build yourself.

  7. Christian Schudt repo owner

    yes, I fear, you have to write your own RosterManager as well. It uses Roster.class, which is now overwritten by yours.

  8. Christian Schudt repo owner

    I've uploaded latest snapshot to Maven Central Snapshot repo. It also includes the fix for issue #75.

  9. Andrew Potter reporter

    I tested this last night by copying Contact, Roster, ContactGroup, RosterEvent and RosterManager, and it almost worked, but it was pretty buggy (I couldn't resolve the roster - it would take a very long time and only return one user?) and very error prone, and I realized that swapping out the RosterManager would make other parts of the library break (e.g. ContactExchangeManager, which calls session.getManager(RosterManager)). Unfortunately the only other options I can see are adding the fields to the base class or providing a way to pass the Contact implementation to the session config.

  10. Andrew Potter reporter

    For now, I'm just maintaining a fork with those two fields added - not sure how you want to proceed.

  11. Christian Schudt repo owner

    Could you find out (and tell me), what exactly was "pretty buggy", "very error prone", why it took a very long time, and why it only returned one user? Maybe a stacktrace? You could addSessionStatusListener and print the e.getThrowable() of the SessionStatusEvent. In my testing it worked (and I also only copied the roster classes). Did you copy the package-info.java as well? It defines the jabber:iq:roster namespace.

    General options:

    1. Rebind the jabber:iq:roster to a new roster class. This is what we've tried here.
    2. Make Roster, Contact, ContactGroup class an interface, with a DefaultRoster, DefaultContact, DefaultContactGroup class as default implementations. Then similarly rebind the default implementation to yours. The only advantage is, that you could reuse RosterManager (working with the interfaces)
    3. Make Roster classes non-final and try to inherit your own classes. But not sure how JAXB likes this. In any case, you would need to remove the old Roster class from JAXBContext and rebind the namespace to your inheritied one, similarly as in 1).

    I still like the first option best, especially because it doesn't warp the library for proprietary needs.

    About the other parts, you are right, but it's really only used by ContactExchangeManager, which you probably don't need.

  12. Andrew Potter reporter

    Ah, I think I found my issue. I was using a message listener to get the roster, and I didn't realize that the listeners were all fired synchronously. This cause my .get() to always time out.

    So it looks like this would work for me, but it would still be cool if I didn't have to copy as much code. Overriding just the Contact would be ideal, and much more future-proof.

  13. Christian Schudt repo owner

    A message listener for roster IQs? That can't work. Messages are processed synchronously in the same thread. So if you block that thread, which actually processes new messages, you get the timeout, because the new messages can't be process by the thread. IQ results are processed in different threads, so that you could query from the message thread.

  14. Andrew Potter reporter

    Just for kicks I tried implementing (2) and (3), and it seems like JAXB is unable to unmarshall interfaces or abstract classes that the interface or abstract class doesn't have access to. In other words, JAXB can't unmarshall an interface in the babbler library to a class in a project that includes babbler as a dependency. For it to be able to unmarshall interfaces or abstract classes, the implementations have to be added to the class/field via annotations.

    Once I figured out the issue with getting the roster, (1) has been working perfectly.

  15. Christian Schudt repo owner

    Yes, JAXB doesn't like interfaces. I've been through that already... I hope solution 1) is acceptable.

  16. Log in to comment