You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "poliglots (via GitHub)" <gi...@apache.org> on 2026/08/17 12:19:47 UTC
[PR] fix(api-core): properly implement Clock.withZone() in MonotonicClock [maven]
poliglots opened a new pull request, #12753:
URL: https://github.com/apache/maven/pull/12753
Fix issue #12608: MonotonicClock.withZone() was ignoring the zone parameter and returning 'this', violating the java.time.Clock contract.
Changes:
- Add a 'zone' field to track timezone per instance
- Implement withZone() to create a new clock variant with the requested timezone, preserving monotonic timing
- Update getZone() to return the instance's actual zone
- Throw IllegalArgumentException on null zone (per Clock spec)
- Add tests verifying withZone() returns a new instance with correct zone and that monotonic timing is preserved
The singleton instance continues to use UTC. Creating timezone variants does not compromise monotonicity since Instant is timezone-agnostic and monotonicity derives from System.nanoTime().
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(api-core): properly implement Clock.withZone() in MonotonicClock [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #12753:
URL: https://github.com/apache/maven/pull/12753#discussion_r3796358302
##########
api/maven-api-core/src/main/java/org/apache/maven/api/MonotonicClock.java:
##########
@@ -146,27 +148,50 @@ public Duration elapsedTime() {
}
/**
- * Returns the zone ID of this clock, which is always UTC.
+ * Returns the zone ID of this clock.
+ * <p>
+ * The singleton instance always returns UTC. Clock instances created
+ * via {@link #withZone(ZoneId)} return their configured timezone.
*
- * @return the UTC zone ID
+ * @return the zone ID
*/
@Override
public ZoneId getZone() {
- return ZoneOffset.UTC;
+ return zone;
}
/**
- * Returns this clock since timezone adjustments are not supported.
+ * Returns a copy of this clock with the specified timezone.
* <p>
- * This implementation maintains UTC time to ensure monotonic behavior.
- * The provided zone parameter is ignored.
+ * Since {@link Instant} instances are timezone-agnostic and the monotonic
+ * property derives from {@link System#nanoTime()}, the returned clock
+ * maintains identical monotonic timing but reports the requested timezone.
*
- * @param zone the target timezone (ignored)
- * @return this clock instance
+ * @param zone the target timezone, or {@code null} to use UTC
+ * @return a new clock with the specified timezone
+ * @throws IllegalArgumentException if zone is {@code null}
Review Comment:
**Contradictory Javadoc + wrong exception type**: The `@param` says _"or {@code null} to use UTC"_ but the code throws on null — the `@param` should say `@param zone the target timezone, not null`.
Also, the JDK `Clock.withZone()` convention uses `NullPointerException` (via `Objects.requireNonNull`), not `IllegalArgumentException`. Consider:
```suggestion
public Clock withZone(ZoneId zone) {
java.util.Objects.requireNonNull(zone, "zone");
if (zone.equals(this.zone)) {
return this;
}
return new MonotonicClock(startNanos, startInstant, zone);
}
```
This also adds the same-zone optimization (returns `this` when the zone matches, matching JDK convention).
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(api-core): properly implement Clock.withZone() in MonotonicClock [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #12753:
URL: https://github.com/apache/maven/pull/12753
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(api-core): properly implement Clock.withZone() in MonotonicClock [maven]
Posted by "poliglots (via GitHub)" <gi...@apache.org>.
poliglots commented on code in PR #12753:
URL: https://github.com/apache/maven/pull/12753#discussion_r3802047268
##########
api/maven-api-core/src/main/java/org/apache/maven/api/MonotonicClock.java:
##########
@@ -146,27 +148,50 @@ public Duration elapsedTime() {
}
/**
- * Returns the zone ID of this clock, which is always UTC.
+ * Returns the zone ID of this clock.
+ * <p>
+ * The singleton instance always returns UTC. Clock instances created
+ * via {@link #withZone(ZoneId)} return their configured timezone.
*
- * @return the UTC zone ID
+ * @return the zone ID
*/
@Override
public ZoneId getZone() {
- return ZoneOffset.UTC;
+ return zone;
}
/**
- * Returns this clock since timezone adjustments are not supported.
+ * Returns a copy of this clock with the specified timezone.
* <p>
- * This implementation maintains UTC time to ensure monotonic behavior.
- * The provided zone parameter is ignored.
+ * Since {@link Instant} instances are timezone-agnostic and the monotonic
+ * property derives from {@link System#nanoTime()}, the returned clock
+ * maintains identical monotonic timing but reports the requested timezone.
*
- * @param zone the target timezone (ignored)
- * @return this clock instance
+ * @param zone the target timezone, or {@code null} to use UTC
+ * @return a new clock with the specified timezone
+ * @throws IllegalArgumentException if zone is {@code null}
Review Comment:
Fixed review findings in the commit [41cc260](https://github.com/apache/maven/pull/12753/commits/41cc260375e71812aaaebc504963420decd20b37)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org