[
https://issues.apache.org/jira/browse/ARTEMIS-3623?focusedWorklogId=703249&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-703249
]
ASF GitHub Bot logged work on ARTEMIS-3623:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 04/Jan/22 10:54
Start Date: 04/Jan/22 10:54
Worklog Time Spent: 10m
Work Description: erwindon edited a comment on pull request #3891:
URL:
https://github.com/apache/activemq-artemis/pull/3891#issuecomment-1004707761
> would be possible to add a test for this?
What should be tested then? only this function? or the complete context
where it is used, i.e. message-expiry?
Each protocol has its own `toPropertyMap` function. Neither of these have
dedicated tests. This is a fix/improvement for the one on the amqp side only.
The amount of work for this may quickly become too much for me.
In that case, alternative 2 or 3 (as listed in
https://issues.apache.org/jira/browse/ARTEMIS-3623) may be more appropriate.
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 703249)
Time Spent: 1h (was: 50m)
> extraProperties._AMQ_ACTUAL_EXPIRY should be numeric in expired AMQP messages
> -----------------------------------------------------------------------------
>
> Key: ARTEMIS-3623
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3623
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: AMQP
> Affects Versions: 2.20.0
> Reporter: Erwin Dondorp
> Priority: Minor
> Attachments: image-2021-12-30-10-40-10-032.png, screenshot-1.png
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> When an AMQP message is expired, it is moved to the ExpiryQueue. With this
> process, a few extra properties are added to describe the original/previous
> location of the message.
> However, property {{extraProperties._AMQ_ACTUAL_EXPIRY}} is added as a string
> value. This could have been an integer value to match the original data type.
> Since JS function {{formatTimestamp()}} is used to represent the value, the
> original string value is displayed again in parentheses.
> Solution is expected to be one of these 3:
> 1) (preferred) {{_AMQ_ACTUAL_EXPIRY}} is made numeric. Function
> {{formatTimestamp()}} can do its work and a proper date-time will be shown.
> 2) (alternative) {{_AMQ_ACTUAL_EXPIRY}} remains a string. Function
> {{formatTimestamp()}} should no longer be called.
> 3) (alternative) {{_AMQ_ACTUAL_EXPIRY}} remains a string. Function
> {{formatTimestamp()}} is called with the {{parseInt()}} value of the string
> when the string consists only of digits.
> I'll create a PR for solution 1.
> !image-2021-12-30-10-40-10-032.png!
> Analysis notes:
> * (OK) example {{org.apache.activemq.artemis.jms.example.ExpiryExample}} uses
> {{getLongProperty("_AMQ_ACTUAL_EXPIRY")}}; and
> * (OK) {{org.apache.activemq.artemis.core.server.impl.QueueImpl}} uses
> {{copy.setBrokerProperty(Message.HDR_ACTUAL_EXPIRY_TIME,
> System.currentTimeMillis());}}; and
> * (OK)
> {{org.apache.activemq.artemis.core.server.transformer.ServerMessageImpl}}
> uses {{long actualExpiryTime = System.currentTimeMillis();
> message.putLongProperty(Message.HDR_ACTUAL_EXPIRY_TIME, actualExpiryTime);}};
> and
> * (OK)
> {{org.apache.activemq.artemis.tests.integration.client.ExpiryAddressTest}}
> uses {{Long actualExpiryTime = (Long)
> tm.getObjectProperty(Message.HDR_ACTUAL_EXPIRY_TIME);}}.
> * ...
> * (!!!)
> {{org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.toPropertyMap()}}
> converts all extraProperties to strings (and truncate them when they are too
> long).
> Development notes:
> Add condition on value type. Numeric values are copied as-is. All other types
> keep their old behaviour.
> Test result:
> !screenshot-1.png!
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
