[
https://issues.apache.org/jira/browse/JAMES-2167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16539540#comment-16539540
]
ASF GitHub Bot commented on JAMES-2167:
---------------------------------------
Github user chibenwa commented on a diff in the pull request:
https://github.com/apache/james-project/pull/126#discussion_r201560161
--- Diff:
server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java
---
@@ -442,16 +426,39 @@ protected void populateMail(Message message, MailImpl
mail) throws JMSException
}
/**
- * Convert the attribute value if necessary.
+ * Retrieves the attribute by {@code name} form {@code message} and
tries to add it on {@code mail}.
+ *
+ * @param message The attribute source.
+ * @param mail The mail on which attribute should be set.
+ * @param name The attribute name.
+ */
+ private void setMailAttribute(Message message, Mail mail, String name)
{
+ // Now cast the property back to Serializable and set it as
attribute.
+ // See JAMES-1241
+ Object attrValue =
Throwing.function(message::getObjectProperty).apply(name);
+
+ // can be a base64 representation of serialized object try decode
and deserialize. See JAMES-2167.
+ if (attrValue instanceof String) {
+ mail.setAttribute(name, tryDeserialize((String) attrValue));
+ } else if (attrValue instanceof Serializable) {
+ mail.setAttribute(name, (Serializable) attrValue);
+ } else {
+ LOGGER.error("Not supported mail attribute {} of type {} for
mail {}", name, attrValue, mail.getName());
+ }
+ }
+
+ /**
+ * Tries to deserialize given argument and when fails returns itself.
*
- * @param value
- * @return convertedValue
+ * @param attrValue The input string.
+ * @return The deserialized object or itself.
*/
- protected Object convertAttributeValue(Object value) {
- if (value == null || value instanceof String || value instanceof
Byte || value instanceof Long || value instanceof Double || value instanceof
Boolean || value instanceof Integer || value instanceof Short || value
instanceof Float) {
- return value;
+ private Serializable tryDeserialize(String attrValue) {
--- End diff --
This should be moved to `JMSSerializationUtils`.
Also, you should not rely on an exception of deserializing to handle
"native" values. If doing so, a user providing a `base64 serialized integer
string` as an input will get an `Integer` as an output.
We miss the `bijection` behaviour that we would expect from such a feature.
That may lead to subtle bugs.
My opinion on this is to promote correctness over performances. Hence I'd
advocate**use base64 serialization even for native values**.
This would also have some cool benefits:
- We would stop assuming ActiveMQ behaviour in James code as it is done in
`JMSSerializationUtils::hasJMSNativeSupport`
- We will simplify the code by removing one conditional branch.
- `JMSSerializationUtils` would not be JMS specific anymore. We will be
able to change its name, & in a near future use it in another context.
- This would also remove the `if (X instanceOf String)` in
`this::setMailAttribute`.
> Serializable attributes are not preserved by enqueue/dequeue on a JMS queue
> ---------------------------------------------------------------------------
>
> Key: JAMES-2167
> URL: https://issues.apache.org/jira/browse/JAMES-2167
> Project: James Server
> Issue Type: Bug
> Components: Queue
> Affects Versions: master
> Reporter: Tellier Benoit
> Priority: Major
>
> A call to toString breaks convertion for generic serializable attributes.
> The dequeued email will have only a toString version of it. We are expecting
> the exact same value, just deserialized.
> We should ensure the value of Serializable attributes gets preserved by
> enqueue/dequeue operations. We should add a unit test for this, and fix it.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
