MID-11078 Fix false dateTime updates due to seconds normalization in admin GUI#583
MID-11078 Fix false dateTime updates due to seconds normalization in admin GUI#583
Conversation
|
|
||
| @AfterMethod | ||
| void restoreDefaultTimeZone() { | ||
| TimeZone.setDefault(originalDefaultTimeZone); |
There was a problem hiding this comment.
This is probably not a good idea changing timezone for whole VM. Same below.
|
|
||
| public class TestXmlGregorianCalendarModel extends AbstractGuiUnitTest { | ||
|
|
||
| private static final DatatypeFactory DF = datatypeFactory(); |
There was a problem hiding this comment.
Not necessary to setup your datafactory, we have util class that helps with XML gregorian calendar, check XmlTypeConverter.createXMLGregorianCalendar(...) and other methods.
| } | ||
|
|
||
| private static Date minuteDate(int minute) { | ||
| GregorianCalendar calendar = new GregorianCalendar(TimeZone.getTimeZone("Europe/Prague")); |
There was a problem hiding this comment.
Probably nicer to call Calendar.getInstance(TimeZone.getTimeZone(...))
| return calendar.getTime(); | ||
| } | ||
|
|
||
| private static XMLGregorianCalendar storedFromDate(Date date) { |
There was a problem hiding this comment.
Same here, use if possible XmlTypeConverter
| } | ||
| } | ||
|
|
||
| private static final class SimpleModel implements IModel<XMLGregorianCalendar> { |
There was a problem hiding this comment.
This class is unnecessary, you can use Model.of(object)
| } | ||
|
|
||
| @Test | ||
| void testKeepStoredValueWhenGuiOnlyDropsSeconds() { |
There was a problem hiding this comment.
This test seems to be way too complicated. Aren't we testing XmlGregorianCalendarModel whether it will NOT change value if model.setObject() is called with the same value as it's already in model (only that new value has seconds/millis zeroed?) Or am I missing something?
|
|
||
| // Forces toGregorianCalendar() to return a calendar configuration that reproduces | ||
| // the old GregorianCalendar.equals(...) mismatch for the same normalized minute. | ||
| private static final class CalendarOverride extends XMLGregorianCalendar { |
There was a problem hiding this comment.
I don't understand why this is needed. Both params have the same date/time only types are different.
|
1.)Replaced DatatypeFactory usage with XmlTypeConverter as suggested. 5.)The test was simplified where possible, but it still needs to reproduce a specific edge case. The issue here is that two values can represent the same normalized minute, but their GregorianCalendar representations are not equal (GregorianCalendar.equals(...) returns false). The previous implementation relied on equals(), which caused incorrect replacement. CalendarOverride is used to force this mismatch so the test actually reproduces the original failure. Without it, the test would also pass on master and would not verify the fix. |
2 participants
The admin GUI date picker operates at minute precision and drops seconds/milliseconds on submit.
The existing guard in XmlGregorianCalendarModel used GregorianCalendar.equals(...), which compares full calendar state (including timezone), not just the actual time value.
This caused false differences when:
-stored value contained seconds (e.g. 09:32:12),
-GUI round-trip produced 09:32:00,
-or calendars differed only in internal representation (e.g. timezone/configuration),
-even though both represented the same effective minute.
As a result, the GUI incorrectly overwrote values and preview showed spurious changes.
Solution
Replace strict GregorianCalendar.equals(...) with comparison of normalized minute-precision timestamps using getTimeInMillis() after stripping seconds and milliseconds.
This ensures:
-no change is produced when only GUI precision differs,
-false differences caused by calendar metadata (including timezone representation) are avoided,
-real differences in actual time are still detected.
In particular:
-real timezone differences → still cause update (correct)
-fake differences from calendar equality → no longer cause update (fixed)