Hi,
seems like ToContainsFilter was removed from smack 4.0
However, I don’t see it mentioned in the upgrade guide or changelog.
BTW: there still seems to be an integration test using it: https://github.com/igniterealtime/Smack/blob/master/smack-core/src/integration-t est/java/org/jivesoftware/smack/filter/ToContainsFilterTest.java
How does that even compile?
Using the ToContains filter could lead to potential insecure code. It was removed in 980047. If you really want/have to use it, then you have to re-implement your own ToContains filter. But I really wonder what the use-case would be. ToContains was removed also because it was unused in the existing Smack codebase.
BTW: there still seems to be an integration test using it:https://github.com/igniterealtime/Smack/blob/master/smack-core/src/integration-t est/java/org/jivesoftware/smack/filter/ToContainsFilterTest.java
How does that even compile?
It does not. The integration tests are currently unused and not build.
Flow wrote:
Using the ToContains filter could lead to potential insecure code. It was removed in 980047. If you really want/have to use it, then you have to re-implement your own ToContains filter. But I really wonder what the use-case would be.
I had used the filter to get only chats for a specific user. Cf.
GitHub - jenkinsci/jabber-plugin at 8e42c15b9ab22bf205c5f3a0ec57d937f4ee537d f4ee537d/src/main/java/hudson/plugins/jabber/im/transport/JabberIMConnection.jav a#L477
I guess, I’ve never been really sure if it was actually necessary to have the filter. I assume that the filter would get by default only the messages for the current user, right?
Flow wrote:
ToContains was removed also because it was unused in the existing Smack codebase.
Well the point of a library is that it will also provide helpful classes, which are not used in the library itself, but only by clients of it, isn’t it?
Basically, I just wondered that it was removed without notice.
Flow wrote:
BTW: there still seems to be an integration test using it:https://github.com/igniterealtime/Smack/blob/master/smack-core/src/integration-t est/java/org/jivesoftware/smack/filter/ToContainsFilterTest.java
How does that even compile?
It does not. The integration tests are currently unused and not build.
Makes total sense to have integration tests and then not use them
I guess, I’ve never been really sure if it was actually necessary to have the filter. I assume that the filter would get by default only the messages for the current user, right?
Any stanza send to an client will always, as per the XMPP spec, have one of the following values as to attribute:
the full JID of the client
the bare JID of the client
no to attribute at all
Therefore the ToContains filter is actually
harmful, because there could be valid stanzas without a to value
unnecessary, because every stanza send by a standard compliant XMPP server to the client, is addressed, implicitly or explicitly, to the client
Well the point of a library is that it will also provide helpful classes, which are not used in the library itself, but only by clients of it, isn’t it?
That’s right, but it’s a harmful and in most cases unnecessary, trival piece of code that can easily be re-implemented if you really need it (but then I would to eager to hear the use case).
Basically, I just wondered that it was removed without notice.
Smack 4 has undergone the greatest refactoring in the history of Smack. Yes it should be documented, but I’m currently a one man show and therefore have not many resources available. I’m happy about every report or contribution, so that I can update the “Smack 4 Readme and Upgrade Guide”, which I just did. Thanks for you report.
Makes total sense to have integration tests and then not use them
Again, one-man show, not much time/resources. Integration tests are not a top priority right now, especially due to their server implementation dependend nature (you want tests against the XMPP spec, not against a particular XMPP server). Good unit tests are more important (which we have, but there is also room for improvement). Contributions are always welcome.
I had used the filter to get only chats for a specific user. Cf.
I’ve made some comments on your commit. Hope you don’t mind. Note that another new feature of the Smack 4 API is MultiUserChat.createOrJoin(), you may want to use this method.
Thanks, comments are most welcome!
Flow wrote:
Makes total sense to have integration tests and then not use them
Again, one-man show, not much time/resources. Integration tests are not a top priority right now, especially due to their server implementation dependend nature (you want tests against the XMPP spec, not against a particular XMPP server). Good unit tests are more important (which we have, but there is also room for improvement). Contributions are always welcome.
I can totally understand it - I know how much time it takes to maintain open source projects (in your spare time)
However, implementations don’t always never fully comply to the specs, some some integration tests would be very valuable IMHO.
