I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

1. In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

2. Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

Andrzej Wójcik wrote:

I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

Yes, I think it should be left as it is, unless you have a different suggestion.

Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

To be honest I did not think of a case where we have multiple auth plugins. Please look for some way to optimize it.

Maybe we should have an abstract plugin for all auth plugins. This abstract plugin could have a checking logic implemented, so other plugins would not need to reimplement it. Then it would be easier to optimize the checking code. As far as I remember, preprocessors are executed sequentially in SM. So as soon as the first preprocessors returns "false" we could stop processing the packet by other preprocessors.

Artur Hefczyc wrote:

Andrzej Wójcik wrote:

I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

Yes, I think it should be left as it is, unless you have a different suggestion.

OK

Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

To be honest I did not think of a case where we have multiple auth plugins. Please look for some way to optimize it.

We already have at least 2 (I found 2 of them):

• JabberIqAuth

• SaslAuth

Maybe we should have an abstract plugin for all auth plugins. This abstract plugin could have a checking logic implemented, so other plugins would not need to reimplement it. Then it would be easier to optimize the checking code. As far as I remember, preprocessors are executed sequentially in SM. So as soon as the first preprocessors returns "false" we could stop processing the packet by other preprocessors.

I already started creation of abstract preprocessor for all auth plugins but I cannot find any solution how to optimize this. You are right, first one which will return false will stop processing but for most of packets (on authenticated connections) this will be checked twice for both plugins active, but since checking is rather fast, I do not think we would need to look for a way to optimize this.

I moved implementation checking conditions from DefaultPacketHandler to proper XMPPProcessors (implementing XMPPPreprocessor interface) and verified that moved code is working properly. I needed to apply some changed to make it work but in result code works as before but is now located in proper locations.