`InstantPatternLegacyFormatter` computes precision incorrectly

[vy]

InstantPatternLegacyFormatter uses InstantPatternDynamicFormatter to determine the precision of the legacy date & time pattern it receives. As reported by @ppkarwasz (see the review discussion), this results in incorrect precision (i.e., microseconds are currently classified as nanoseconds), and hence, causes incorrect cache invalidation.

In 3799799ca77b1724e57214927d76ef0d1cff055a, @ppkarwasz fixes this by converting the legacy pattern back to DateTimeFormatter-compatible (i.e., modern) variant before passing it to InstantPatternDynamicFormatter. I am personally in favor of copying InstantPatternDynamicFormatter::patternPrecision to InstantPatternLegacyFormatter, and adapting it, since this will ensure legacy code remains an island.

[ppkarwasz]

Hi @vy,

As reported by @ppkarwasz (see the review discussion), this results in incorrect precision (i.e., microseconds are currently classified as nanoseconds), and hence, causes incorrect cache invalidation.

This is something we should fix to ensure all implementations adhere to the InstantPattern contract. However, the issue currently has no observable effect:

• InstantPatternLegacyFormatter results are not cached.
• Even if they were, we only cache timestamps with millisecond precision, so higher-resolution discrepancies (microseconds vs nanoseconds) don’t matter in practice.

https://github.com/apache/logging-log4j2/blob/8ec5703670fc24d8883db39df8be244c34c8e0bd/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternFormatter.java#L125-L152

In 3799799, @ppkarwasz fixes this by converting the legacy pattern back to DateTimeFormatter-compatible (i.e., modern) variant before passing it to InstantPatternDynamicFormatter. I am personally in favor of copying InstantPatternDynamicFormatter::patternPrecision to InstantPatternLegacyFormatter, and adapting it, since this will ensure legacy code remains an island.

I don't think duplicating code is necessary here: InstantPatternLegacyFormatter has been destined for deprecation and subsequent removal from the start.

Once FixedDateFormat is removed, we should retire both InstantPatternLegacyFormatter and support for legacy patterns. There's no reason to continue supporting legacy patterns with the new implementation. Since enabling them requires a manual configuration change, users might as well migrate to the modern DTF-style patterns.