XMPPFramework
XMPPFramework copied to clipboard
"MUC light" implementation
We (Erlang Solutions, makers of the MongooseIM XMPP server) would like to contribute "MUC light".
Proposed version: http://xmpp.org/extensions/inbox/muc-light.html
Latest version: https://github.com/fenek/xeps/blob/muc_light/inbox/muc-light.xml
Doc: http://mongooseim.readthedocs.io/en/latest/developers-guide/mod_muc_light_developers_guide/ http://mongooseim.readthedocs.io/en/latest/modules/mod_muc_light/
Here is preliminary implementation, covering only the basic use cases: https://github.com/robbiehanson/XMPPFramework/compare/master...esl:andres.XMPPMUCLight?expand=1 We are expanding this implementation to cover all the use cases.
What do you think of this?
So I created XMPPRoomLight
and XMPPMUCLight
.
-
XMPPRoomLight
handles sending/receiving messages, creating a room, fetching the members list, leaving a room and adding users to the room. All the basic actions. -
XMPPMUCLight
monitors the active XMPPRoomLight instances, discover rooms, and listens for MUC light affiliation changes.
Also I added a the same persistent layer as the one implemented for XMPPRoom, I created a XMPPRoomLightCoreDataStorage
that knows how to handle incoming and outgoing messages. Also I created a custom XMPPRoomLight
core data model to store the messages.
A lot of the code from XMPPRoomLightCoreDataStorage
is similar/the same as the one in XMPPRoomCoreDataStorage
but I thought that creating a different class was better because MUC Light is way simpler so I thought that having a shorter class that just encapsulates the behaviour of MUC Light was better than trying to reuse XMPPRoomCoreDataStorage
and start mixing concepts. But let me know what's your thought on this...
tl;dr: I didn't touch XMPPMUC, XMPPRoom and all the other classes that currently implement XEP-0045
Overall it's looking pretty good to me! Some small typos I noticed on the first pass:
-
+- (void)xmppRoomLight:(XMPPRoomLight *)sender didCreatRoomLight:(XMPPIQ *)iq;
-
+#import "XMPPRoomLightCoreDataSTorage.h"
Some other improvements:
- I would rename
jid
toroomJID
to be consistent with the method below it:+- (id)initWithJID:(XMPPJID *)jid roomname:(NSString *) roomname;
- Add some general header documentation about how this differs from traditional MUC, server support, links to the proto XEP, etc.
- Bonus: Use instancetype, nullability annotations, and Obj-C generics where possible.
-
+@property(nonatomic, strong, readonly) NSMutableSet *rooms;
there could be threading issues by exposing this as a nonatomic mutable set. It looks like this should never be mutated by a consumer of the class, and it may be mutated by multiple threads internally by- (void)xmppStream:(XMPPStream *)sender didRegisterModule:(id)module {
and unregister. These mutations should probably be done on the moduleQueue, and replacing the public property with an accessor method that returns an NSSet copy that synchronizes access to the object on the moduleQueue.
@chrisballinger I worked on the typos and on the other improvements, I'm not a 100% sure if my changes are doing what you meant in the last bullet, if you can confirm that's what you were thinking of would be great.
Is rooms
meant to be publicly accessible and mutable to other modules or classes that may be operating on different threads? Although it would be an edge case, if two threads mutate the object at the same time, or one is reading while another is mutating, it may cause a crash.
The idea of rooms
is to be publicly accesible but not mutable, it can only be mutated by didRegisterModule
and willUnregisterModule
.
`if two threads mutate the object at the same time, or one is reading while another is mutating, it may cause a crash.
Do you think this is still possible? As now adding and removing objects from rooms
is now dispatched dispatch_async(moduleQueue, block);
, so it would be only changed in the same thread right?
Looks good! I think your changes should prevent any issues and makes the intention of the API more clear.
Ready for a PR?
hey thanks for checking it out so fast!! I'm working on adding the missing functionalities to XMPPRoom like destroyRoom, changeSubject and more... do you think is better to wait for a BIG PR with all the goodies implemented or just merge this and PR again when I finish adding stuff?
Probably best to finish it all and submit a big PR to avoid potential confusion about a partial implementation
Is Muc Light implementation complete?