New MUC module

Hi all,

Apologies for the lack of contribution to the group for a while, it has been super busy at work. I’m still actively involved at work with Openfire but just haven’t had that much of a chance to commit community wise.

A proposal I had regarding MUC…I really want to contribute but personally find working with the code kind of hard in terms of its modularity and coupling, static methods etc (don’t take this as a criticism!). On the other hand I find Tinder really easy to work with and I use it a lot. What do people think about slowly pulling out some of the MUC stuff into it’s own library just like Tinder is? You could have a really concise, well tested and easy to work with module (by which I just mean a jar) that Openfire then pulls in with its build system. The project could be in Git or SVN, and use Maven or Gradle or whatever anyone else wanted as a build tool?

Michael

Just as an aside of what we’re up to here in case you are interested…

We’ve recently upgraded our Openfire to 3.7.1 and are seeing average daily MUC activity of ~5K concurrents and even had that rise to ~12K the other day! We have seen some performance problems but they all seem to be at our end, and I’ve been rewriting many plugins as components using Guus’s helpful bit of documentation (the Achilles Heel post) as a great guide. We’ve looked into PEP but proved with a Tsung load test that there is definitely a memory leak in there, so at one stage we might get some bandwith to investigate that.

It would make sense to me that MUC was made a component (I wouldn’t call it a library as it is very much a standalone peice of code). Then it could by default work as part of Openfire, but decoupled from it. It could also be installed on its own hardware to help with scalability.

Personally, I think it would make sense to keep it in svn along with the other projects. I think this would be simpler from a management perspective, as well as keeping all the related projects colocated. My vote would also be for Maven, as it is well adopted and understood.

I was thinking that pubsub needs to have the same thing done to it, but that particular case has some other issues (due to PEP).

By the way, there has been some refactoring of the pubsub module with regards to memory consumption that will also affect PEP. I would suggest that you upgrade to 3.7.2 (alpha right now) before bothering to do any testing against it.

I see, I think other servers that we looked at do something similar to this and it seemed a good idea. Would it be an addressable component?

I’m happy to start, even if it’s just one bug. I actually have one in mind which has annoyed me and I’d like to look at.

The only reason I was thinking the library approach was because it could be started small - say one helpful class, and then hopefully it’ll pick up momentum. The bug I had in mind was the one where room admins can’t get into a room that has hit capacity - my idea was to have a RoomAccessDecisionManager(Impl?) in this library that whatever the Openfire code is would call when making a decision on whether or not to let people into a room.

I think it’d be a good way of starting small and actually getting the code into the trunk quickly, but if the whole thing doesn’t work out you’ve got an odd small jar with some MUC code there and some in Openfire itself.

Thanks for the PEP tip, much appreciated!

Making MUC more of a standalone component is on my wishlist too. Because of what Gato once told me, I’d consider a different approach first though. Instead of a complete rewrite (which will probably be cleaner, but might introduce a lot of backwards compatibility issues), we might want to restore a bit of flexibility that the code once had. Gato told me that not to long ago, the MUC code could be wrapped in Whack and be turned into an external component. That way, the load is easily offloaded to a different host.

I’m game - how do we start?!

Another wishlist one, have the Openfire build publish its sources so IntelliJ et al can pick them up automatically like it can with Tinder…

For the 4.0 release, I plan to move to a Maven-enabled build. It will be difficult to have all libraries that we use now available as Maven libraries, but for most of them, it appears possible. There will be some added challenges in getting the distributions built properly, things like that - but it all seems very doable. We should set some goals for the big 4.0 soon, branch the code and get going!

I’m not sure what changed since Gato made those comments. A good start would be taking the source, wrapping them in a Whack-based project and see what’s going wrong. There’ll be some obvious missing links (reading/setting openfire properties, interacting with the database, stuff like that), but I expect some things to pop up that we didn’t think of too.

Another piece of very useful functionality that we could create is a set of unit tests. We know how the the interface should behave (as it’s described in the XEP), but I haven’t been able to find any set of functional tests. It’ll be quite some work to get those up, but they would be of a lot of value during development. We can make those tests implementation-agnosting (send xml, expect xml), which would make them re-usable for other projects than Openfire. The challenge would be to develop a good framework (on top of junit, for instance). that allows for easy adaption / modification. Thoughts?

Weird coincidence, all this weekend I have been writing a base class that helps with component testing. Not sure it’s exactly what you were thinking about but I made a gist of it here to show you the work in progress so far - git@gist.github.com:56e2938c5d59ad2951c8.git.

Up till now all my component tests are done by mocking a ComponentManager with Mockito, initializing the component with that component manager, then using verify to assert what happened. Here’s an example of two tests (actually from two different unit test classes) - https://gist.github.com/5db35e0d6357728bf415.

The first test makes a bit more sense, the second one shows how you need to use an ArgumentCaptor as equals assertions can be a bit of a pain with Tinder when you don’t care about the specific ID the packet will get.

Hope this makes sense, more than happy to open source or contribute this stuff if you think it isn’t too mental.

PS - in the harness, the AbstractComponent references is our implementation, which is purely an extension of yours, with the ability to always respond to the version IQ request. We do this for convenience as it means that we can verify correct software deployments using XMPP. Sub components will offer up their version number, normally using getClass().getPackage().getImplementationVersion(), which hadily we pull out the Maven version number of the artifact if Maven is setup to do that.

Happy to chat online if you have any questions.

That first gist as a link - https://gist.github.com/56e2938c5d59ad2951c8

PPS, here is my first unit test using the base class - https://gist.github.com/2757ea0fce64bac41811. Hopefully it’s obvious but the brevity of the test is exactly what I want for all my component tests:

@Test

public void verifyComponentRespondsToPingsWithVersionNumber() throws Exception {

IQ versionRequest = new IQ(IQ.Type.get);

versionRequest.setChildElement(“query”, AbstractComponent.NAMESPACE_VERSION);

processPlayerPacket(versionRequest);

assertSent(new VersionResponseIQ(versionRequest, name, version));

}

assertThat(the(lastSentPacketXML()), isEquivalentTo(the(expectedPacket.toXML())));

Poetry

That’s in the general idea I had, yes. I was playing with the tought of having it more low-level (less dependent on Tinder implementation, for instance), by having the assertions done with XPath queries. XML-in, XML-out, from a test perspective. I didn’t give that enough tought to see if it’d actually be workable though.

Thanks

These guys have to take the credit though - http://code.google.com/p/xml-matchers/wiki/Tutorial. You could use that for your xpath style assertions by the looks of it.

Guus,

Smack uses xmlunit for those kinds of assertions. Makes it pretty easy to assert that the XML is equivalent, whether the specific format is or not.

Those matchers actually use xmlunit underneath. I think they are just matcher wrappers round the xmlunit asserts, plus some additional stuff…