[
https://issues.apache.org/jira/browse/ARTEMIS-5769?focusedWorklogId=995626&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-995626
]
ASF GitHub Bot logged work on ARTEMIS-5769:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 10/Dec/25 21:05
Start Date: 10/Dec/25 21:05
Worklog Time Spent: 10m
Work Description: jbertram commented on code in PR #6119:
URL: https://github.com/apache/artemis/pull/6119#discussion_r2608207381
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4385,10 +4386,9 @@ public void testListConsumers() throws Exception {
assertNotEquals("",
jsonConsumer.getString(ConsumerField.MESSAGES_DELIVERED.getName()),
"messagesDelivered");
assertNotEquals("",
jsonConsumer.getString(ConsumerField.MESSAGES_DELIVERED_SIZE.getName()),
"messagesDeliveredSize");
assertNotEquals("",
jsonConsumer.getString(ConsumerField.MESSAGES_ACKNOWLEDGED.getName()),
"messagesAcknowledged");
- assertEquals(0,
jsonConsumer.getInt(ConsumerField.LAST_DELIVERED_TIME.getName()),
"lastDeliveredTime");
- assertEquals(0,
jsonConsumer.getInt(ConsumerField.LAST_ACKNOWLEDGED_TIME.getName()),
"lastAcknowledgedTime");
+ assertNotEquals("",
jsonConsumer.getString(ConsumerField.LAST_DELIVERED_TIME.getName()),
"lastDeliveredTime");
+ assertNotEquals("",
jsonConsumer.getString(ConsumerField.LAST_ACKNOWLEDGED_TIME.getName()),
"lastAcknowledgedTime");
Review Comment:
> I dont really 'get' this change, or more the previous one that broke the
test.
So...The idea here was two-fold
1. make all of the _time_ values reported to the web console consistent
2. avoid creating unnecessary instances of `java.util.Date`
Before 0500c4eab466620dd4aff8adaca9d0102d38ef22 some values created via
`Date#toString()` and some were just `long` values, i.e.:
- Connection
- `creationTime`: `Date#toString()`
- Session
- `creationTime`: `Date#toString()`
- Consumer
- `creationTime`: `Date#toString()`
- `lastDeliveredTime`: `long`
- `lastAcknowledgedTime`: `long`
- Producer
- `creationTime`: `long`
Now, all these values are explicitly and consistently formatted using
[`java.time.format.DateTimeFormatter#RFC_1123_DATE_TIME`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/format/DateTimeFormatter.html#RFC_1123_DATE_TIME).
Although, one could make an argument that they should all essentially be
`long` values instead so that the ultimate formatting could be more flexible.
To your point about this being a breaking change...Technically speaking, it
is a breaking change, but these management methods and the values that they
return were designed and added to the API specifically for use by the web
console and the web console is made for humans, not machines. Therefore, in my
opinion, the same rules don't apply for such changes.
> This was known to be a fixed 0 integer before your previous change, but
now all it checks is its got a non-empty string value.
That's correct. This is in line with the checks that are being performed for
the other attributes, e.g.:
```java
assertNotEquals("",
jsonConsumer.getString(ConsumerField.CREATION_TIME.getName()), "creationTime");
```
The test here is just for presence, not a specific value.
> Was the prior 0 reflective of not-delivering or not-acknowledging ?
Yes.
> Would an empty value / absence not be more reflective of that?
An explicit indication of no-delivery and/or no-acknowledgement would
probably be helpful, although I'm thinking something like like `never` since
this value is meant to be read by humans on the web console.
> From the other change I guess this a string saying 1970 etc for the epoch?
Correct.
> Not sure it feels like an improvement, especially as its a breaking change
on top.
Currently what folks see on the web console for "Last Delivery Time" would
be something like `1765400556` which isn't very helpful. The user would have to
convert the value himself to get something meaningful. The changes in
0500c4eab466620dd4aff8adaca9d0102d38ef22 fix that.
To be clear, I'm not opposed to reverting
0500c4eab466620dd4aff8adaca9d0102d38ef22 and doing something else to improve
the user experience on the web console. My aim here was simply to explain the
rationale for the existing change and answer your specific questions.
Feedback welcome!
Issue Time Tracking
-------------------
Worklog Id: (was: 995626)
Time Spent: 40m (was: 0.5h)
> Standardize date-time strings for web console
> ---------------------------------------------
>
> Key: ARTEMIS-5769
> URL: https://issues.apache.org/jira/browse/ARTEMIS-5769
> Project: Artemis
> Issue Type: Task
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Labels: pull-request-available
> Fix For: 2.45.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently many of the fields requested by the web console that contain
> date-time info are formatted on the broker using:
> {code:java}
> new Date(long).toString(){code}
> This results in output like this:
> {noformat}
> Wed Dec 31 18:00:00 CST 1969{noformat}
> This results in unnecessary garbage in the JVM heap and uses an implicit
> format. It would be better to use an explicit format and avoid any garbage in
> the heap.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
