-
Notifications
You must be signed in to change notification settings - Fork 1.1k
INT-3383: Update to Smack 4.0.0 #1137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright 2002-2010 the original author or authors. | ||
| * Copyright 2002-2014 the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
|
|
@@ -19,7 +19,9 @@ | |
| import org.jivesoftware.smack.ConnectionConfiguration; | ||
| import org.jivesoftware.smack.ConnectionListener; | ||
| import org.jivesoftware.smack.Roster; | ||
| import org.jivesoftware.smack.SmackException.NotConnectedException; | ||
| import org.jivesoftware.smack.XMPPConnection; | ||
| import org.jivesoftware.smack.tcp.XMPPTCPConnection; | ||
| import org.springframework.beans.factory.BeanInitializationException; | ||
| import org.springframework.beans.factory.config.AbstractFactoryBean; | ||
| import org.springframework.context.SmartLifecycle; | ||
|
|
@@ -92,7 +94,7 @@ public Class<? extends XMPPConnection> getObjectType() { | |
|
|
||
| @Override | ||
| protected XMPPConnection createInstance() throws Exception { | ||
| this.connection = new XMPPConnection(this.connectionConfiguration); | ||
| this.connection = new XMPPTCPConnection(this.connectionConfiguration); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there are other variants e.g. |
||
| return this.connection; | ||
| } | ||
|
|
||
|
|
@@ -118,15 +120,19 @@ public void start() { | |
| this.running = true; | ||
| } | ||
| catch (Exception e) { | ||
| throw new BeanInitializationException("failed to connect to " + this.connectionConfiguration.getHost(), e); | ||
| throw new BeanInitializationException("failed to connect to XMPP service for " + this.connectionConfiguration.getServiceName(), e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public void stop() { | ||
| synchronized (this.lifecycleMonitor) { | ||
| if (this.isRunning()) { | ||
| this.connection.disconnect(); | ||
| try { | ||
| this.connection.disconnect(); | ||
| } catch (NotConnectedException e) { | ||
| // Ignore | ||
| } | ||
| this.running = false; | ||
| } | ||
| } | ||
|
|
@@ -171,6 +177,16 @@ public void connectionClosedOnError(Exception e) { | |
| public void connectionClosed() { | ||
| logger.debug("Connection closed"); | ||
| } | ||
|
|
||
| @Override | ||
| public void connected(XMPPConnection connection) { | ||
| logger.debug("Connection connected"); | ||
| } | ||
|
|
||
| @Override | ||
| public void authenticated(XMPPConnection connection) { | ||
| logger.debug("Connection authenticated"); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,8 @@ | |
| import java.util.Map; | ||
|
|
||
| import org.jivesoftware.smack.packet.Message; | ||
|
|
||
| import org.jivesoftware.smackx.jiveproperties.JivePropertiesManager; | ||
| import org.jivesoftware.smackx.jiveproperties.packet.JivePropertiesExtension; | ||
| import org.springframework.integration.mapping.AbstractHeaderMapper; | ||
| import org.springframework.integration.xmpp.XmppHeaders; | ||
| import org.springframework.util.StringUtils; | ||
|
|
@@ -86,8 +87,12 @@ protected Map<String, Object> extractStandardHeaders(Message source) { | |
| @Override | ||
| protected Map<String, Object> extractUserDefinedHeaders(Message source) { | ||
| Map<String, Object> headers = new HashMap<String, Object>(); | ||
| for (String propertyName : source.getPropertyNames()) { | ||
| headers.put(propertyName, source.getProperty(propertyName)); | ||
| JivePropertiesExtension jpe = (JivePropertiesExtension) source.getExtension(JivePropertiesExtension.NAMESPACE); | ||
| if (jpe == null) { | ||
| return headers; | ||
| } | ||
| for (String propertyName : jpe.getPropertyNames()) { | ||
| headers.put(propertyName, JivePropertiesManager.getProperty(source, propertyName)); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you provide a sample how target properties look in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
"target properties"?
Which you shouldn't do. Don't roll your in code, simply use the provided one. |
||
| return headers; | ||
| } | ||
|
|
@@ -129,7 +134,7 @@ protected void populateStandardHeaders(Map<String, Object> headers, Message targ | |
|
|
||
| @Override | ||
| protected void populateUserDefinedHeader(String headerName, Object headerValue, Message target) { | ||
| target.setProperty(headerName, headerValue); | ||
| JivePropertiesManager.addProperty(target, headerName, headerValue); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I see there is an issue with new Smack and old Spring WS.
Is Smack 4.0 compatible with a Smack 3.0? I mean can we have in the classpath both versions?
I'm not sure how quickly the Spring WS (2.3.0) with new Smack will be released so we should be carefull with the new Smack for SI and old one for WS, because we can't release our project with a milestone dependency.
Just in case: can we live with both Smacks in the CP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would need some kind of isolation to have two different versions of the same library loaded (like OSGi). Not sure if this is true or possible in Spring. But even if you could do it, then there is also the API change from Smack 3 to 4. How should the user/JVM decide which version to load?
That's actually what I tried to explain in my JIRA comment: I don't know how the Spring project handles these situations, as it has many release lines, other stuff and sure some policies. All I can say is that the Smack API in spring-integration and spring-ws needs to be compatible. So it's either Smack 3 for both or Smack 4. How you achieve that is up to you.
BTW: I'm not sure if those inter-dependencies are a good design, as it causes situations like we have now, but hadn't time yet to look closer at it.