You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by GitBox <gi...@apache.org> on 2020/11/06 07:40:37 UTC
[GitHub] [tomee] tingung opened a new pull request #716: Change WrappingTextMessage to become serializable
tingung opened a new pull request #716:
URL: https://github.com/apache/tomee/pull/716
There is a business use case which need to persist WrappingTextMessage (or TextMessage) into database. Required WrappingTextMessage to become serializable.
TextMessage implementation by both IBM Websphere and Weblogic are serializable.
https://www.ibm.com/support/knowledgecenter/en/SSFKSJ_7.5.0/com.ibm.mq.javadoc.doc/WMQJMSClasses/com/ibm/jms/JMSMessage.html
https://docs.oracle.com/middleware/12213/wls/WLAPI/weblogic/jms/common/TextMessageImpl.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] tingung commented on pull request #716: Change WrappingTextMessage to become serializable
Posted by GitBox <gi...@apache.org>.
tingung commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-722932152
> If `java.io.Serializable` is implemented here, you should add a corresponding `serialVersionUID`.
ok. let me add serialVersionUID.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #716: Change WrappingTextMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-723081968
@tingung Do you want to create an issue yourself via https://issues.apache.org/jira/projects/TOMEE or shall I do this for you?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] tingung commented on pull request #716: Change WrappingTextMessage to become serializable
Posted by GitBox <gi...@apache.org>.
tingung commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-723150164
@rzo1 , This is my first issue. Let me try to create JIRA ticket. I am not aware of the process.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 edited a comment on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rzo1 edited a comment on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-759317347
I have added a simple unit test (see https://github.com/rzo1/tomee/blob/patch-1/container/openejb-core/src/test/java/org/apache/openejb/resource/activemq/jms2/WrappingMessageSerializationTest.java) based on your branch.
It seems, that the messages from amq do **not** implement serializable. Subsequently, trying to serialize a wrapping message will fail.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-759325047
The related PR on the AMQ site was closed https://github.com/apache/activemq/pull/570, so AMQ messages will not become **serializable** in the near future:
> This is not necessary and is not going to work as there are many subtypes in those messages that are not serializable. You can use OpenWireFormat to marshall/unmarshall the data into bytes and persist it to a database. See https://github.com/apache/activemq/blob/master/activemq-client/src/main/java/org/apache/activemq/openwire/OpenWireFormat.java
>
> Also, in the future a Jira needs to be created and tests need to be added.
Thus, I would propose to close this PR as well as the related JIRA as "wont fix".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] tingung commented on pull request #716: Change WrappingTextMessage to become serializable
Posted by GitBox <gi...@apache.org>.
tingung commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-722937379
> side note: this is true for all messages of this package so can be worth doing it at once to avoid 5 issues + needs an issue on https://issues.apache.org/jira/projects/TOMEE to be in the changelog ;)
Ok. Then I will change all to implement Serializable. Thanks for prompt response.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] dblevins closed pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
dblevins closed pull request #716:
URL: https://github.com/apache/tomee/pull/716
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-723152600
> @rzo1 , This is my first issue. Let me try to create JIRA ticket. I am not aware of the process.
Feel free :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-759317347
I have added a simple unit test (see https://github.com/rzo1/tomee/commit/b3fdbc920955496ba42468abd39c152e6e27662c) based on your branch.
It seems, that the messages from amq do **not** implement serializable. Subsequently, trying to serialize a wrapping message will fail.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-723474443
Maybe a trivial test case ensuring it is actually serializable is worth it, something around https://github.com/apache/tomee/blob/master/container/openejb-core/src/test/java/org/apache/openejb/util/proxy/LocalBeanProxySerializationTest.java in https://github.com/apache/tomee/blob/master/container/openejb-core/src/test/java/org/apache/openejb/activemq/JMS2AMQTest.java (to reuse jms setup) looks not that hard and will prevent regressions at very least.
side note to tomee committers: got in touch with some amq guys and amq (5) should get JMS 2 support so all this code can be dropped at some point (tomee can also push it to amq to make it happen faster).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on pull request #716: Change WrappingTextMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-722936518
side note: this is true for all messages of this package so can be worth doing it at once to avoid 5 issues + needs an issue on https://issues.apache.org/jira/projects/TOMEE to be in the changelog ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] tingung commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
tingung commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-723153435
@rzo1 , I have created JIRA ticket and linked it to this issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on pull request #716: TOMEE-2917 - Change All WrappingMessage to become serializable
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #716:
URL: https://github.com/apache/tomee/pull/716#issuecomment-759342409
Side note if it helps: all messages can become serializable implementing writeReplace callback since JMS messages are of serializable types (String, byte[], Map, serializable mainly) so it is mainly about having a MessageReplacer returned by writeReplace and a writeReplace in the MessageReplacer to get back an AMQ instance (or not since we just need to respect the JMS API so personally I would just make one MessageReplacer per JMS message type and that's it).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org