Skip to content

Instantly share code, notes, and snippets.

@jodastephen

jodastephen/ThreeTen249

Created January 31, 2013 19:30
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save jodastephen/4685652 to your computer and use it in GitHub Desktop.
Save jodastephen/4685652 to your computer and use it in GitHub Desktop.
Download ZIP
APPLIED: Patch to fix ZoneRulesProvider BoundProvider, see #ThreeTen/249
Raw
ThreeTen249
diff --git a/src/share/classes/java/time/ZoneRegion.java b/src/share/classes/java/time/ZoneRegion.java
--- a/src/share/classes/java/time/ZoneRegion.java
+++ b/src/share/classes/java/time/ZoneRegion.java
@@ -146,7 +146,7 @@
ZoneRules rules = null;
try {
// always attempt load for better behavior after deserialization
- rules = ZoneRulesProvider.getRules(zoneId);
+ rules = ZoneRulesProvider.getRules(zoneId, true);
} catch (ZoneRulesException ex) {
if (checkAvailable) {
throw ex;
@@ -176,8 +176,8 @@
@Override
public ZoneRules getRules() {
// additional query for group provider when null allows for possibility
- // that the provider was added after the ZoneId was created
- return (rules != null ? rules : ZoneRulesProvider.getRules(id));
+ // that the provider was updated after the ZoneId was created
+ return (rules != null ? rules : ZoneRulesProvider.getRules(id, false));
}
//-----------------------------------------------------------------------
diff --git a/src/share/classes/java/time/zone/TzdbZoneRulesProvider.java b/src/share/classes/java/time/zone/TzdbZoneRulesProvider.java
--- a/src/share/classes/java/time/zone/TzdbZoneRulesProvider.java
+++ b/src/share/classes/java/time/zone/TzdbZoneRulesProvider.java
@@ -88,12 +88,10 @@
* All the regions that are available.
*/
private List<String> regionIds;
-
/**
* Version Id of this tzdb rules
*/
private String versionId;
-
/**
* Region to rules mapping
*/
@@ -125,7 +123,8 @@
}
@Override
- protected ZoneRules provideRules(String zoneId) {
+ protected ZoneRules provideRules(String zoneId, boolean forCaching) {
+ // forCaching flag is ignored because this is not a dynamic provider
Object obj = regionToRules.get(zoneId);
if (obj == null) {
throw new ZoneRulesException("Unknown time-zone ID: " + zoneId);
@@ -146,25 +145,20 @@
@Override
protected NavigableMap<String, ZoneRules> provideVersions(String zoneId) {
TreeMap<String, ZoneRules> map = new TreeMap<>();
- ZoneRules rules = getRules(zoneId);
+ ZoneRules rules = getRules(zoneId, false);
if (rules != null) {
map.put(versionId, rules);
}
return map;
}
- @Override
- protected ZoneRulesProvider provideBind(String zoneId) {
- return this;
- }
-
/**
* Loads the rules from a DateInputStream, often in a jar file.
*
* @param dis the DateInputStream to load, not null
* @throws Exception if an error occurs
*/
- private void load(DataInputStream dis) throws ClassNotFoundException, IOException {
+ private void load(DataInputStream dis) throws Exception {
if (dis.readByte() != 1) {
throw new StreamCorruptedException("File format not recognised");
}
diff --git a/src/share/classes/java/time/zone/ZoneRulesProvider.java b/src/share/classes/java/time/zone/ZoneRulesProvider.java
--- a/src/share/classes/java/time/zone/ZoneRulesProvider.java
+++ b/src/share/classes/java/time/zone/ZoneRulesProvider.java
@@ -63,11 +63,9 @@
import java.security.AccessController;
import java.security.PrivilegedAction;
-import java.time.DateTimeException;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -95,13 +93,13 @@
* {@code java.time.zone.ZoneRulesProvider} in the resource directory
* {@code META-INF/services}. The file should contain a line that specifies the
* fully qualified concrete zonerules-provider class name.
- * Providers may also be made availble by adding them to the class path or by
+ * Providers may also be made available by adding them to the class path or by
* registering themselves via {@link #registerProvider} method.
* <p>
* The Java virtual machine has a default provider that provides zone rules
* for the time-zones defined by IANA Time Zone Database (TZDB). If the system
* property {@code java.time.zone.DefaultZoneRulesProvider} is defined then
- * it is taken to be the fully-qulified name of a concrete ZoneRulesProvider
+ * it is taken to be the fully-qualified name of a concrete ZoneRulesProvider
* class to be loaded as the default provider, using the system class loader.
* If this system property is not defined, a system-default provider will be
* loaded to serve as the default provider.
@@ -112,6 +110,7 @@
* Time-zone rules are political, thus the data can change at any time.
* Each provider will provide the latest rules for each zone ID, but they
* may also provide the history of how the rules changed.
+ *
* <h3>Specification for implementors</h3>
* This interface is a service provider that can be called by multiple threads.
* Implementations must be immutable and thread-safe.
@@ -119,6 +118,8 @@
* Providers must ensure that once a rule has been seen by the application, the
* rule must continue to be available.
* <p>
+ * Providers are encouraged to implement a meaningful {@code toString} method.
+ * <p>
* Many systems would like to update time-zone rules dynamically without stopping the JVM.
* When examined in detail, this is a complex problem.
* Providers may choose to handle dynamic updates, however the default provider does not.
@@ -132,7 +133,7 @@
*/
private static final CopyOnWriteArrayList<ZoneRulesProvider> PROVIDERS = new CopyOnWriteArrayList<>();
/**
- * The lookup from zone region ID to provider.
+ * The lookup from zone ID to provider.
*/
private static final ConcurrentMap<String, ZoneRulesProvider> ZONES = new ConcurrentHashMap<>(512, 0.75f, 2);
@@ -190,7 +191,7 @@
/**
* Gets the set of available zone IDs.
* <p>
- * These zone IDs are loaded and available for use by {@code ZoneId}.
+ * The IDs are the string form of a {@link ZoneId}.
*
* @return a modifiable copy of the set of zone IDs, not null
*/
@@ -205,14 +206,25 @@
* <p>
* This method relies on time-zone data provider files that are configured.
* These are loaded using a {@code ServiceLoader}.
+ * <p>
+ * The caching flag is designed to allow provider implementations to
+ * prevent the rules being cached in {@code ZoneId}.
+ * Under normal circumstances, the caching of zone rules is highly desirable
+ * as it will provide greater performance. However, there is a use case where
+ * the caching would not be desirable
*
- * @param zoneId the zone region ID as used by {@code ZoneId}, not null
- * @return the rules for the ID, not null
- * @throws ZoneRulesException if the zone ID is unknown
+ * @param zoneId the zone ID as defined by {@code ZoneId}, not null
+ * @param forCaching whether the rules are being queried for caching,
+ * true if the returned rules will be cached by {@code ZoneId},
+ * false if they will be returned to the user without being cached in {@code ZoneId}
+ * @return the rules, null if {@code forCaching} is true and this
+ * is a dynamic provider that wants to prevent caching in {@code ZoneId},
+ * otherwise not null
+ * @throws ZoneRulesException if rules cannot be obtained for the zone ID
*/
- public static ZoneRules getRules(String zoneId) {
+ public static ZoneRules getRules(String zoneId, boolean forCaching) {
Objects.requireNonNull(zoneId, "zoneId");
- return getProvider(zoneId).provideRules(zoneId);
+ return getProvider(zoneId).provideRules(zoneId, forCaching);
}
/**
@@ -234,10 +246,10 @@
* Thus the map will always contain one element, and will only contain more
* than one element if historical rule information is available.
*
- * @param zoneId the zone region ID as used by {@code ZoneId}, not null
+ * @param zoneId the zone ID as defined by {@code ZoneId}, not null
* @return a modifiable copy of the history of the rules for the ID, sorted
* from oldest to newest, not null
- * @throws ZoneRulesException if the zone ID is unknown
+ * @throws ZoneRulesException if history cannot be obtained for the zone ID
*/
public static NavigableMap<String, ZoneRules> getVersions(String zoneId) {
Objects.requireNonNull(zoneId, "zoneId");
@@ -247,7 +259,7 @@
/**
* Gets the provider for the zone ID.
*
- * @param zoneId the zone region ID as used by {@code ZoneId}, not null
+ * @param zoneId the zone ID as defined by {@code ZoneId}, not null
* @return the provider, not null
* @throws ZoneRulesException if the zone ID is unknown
*/
@@ -276,7 +288,7 @@
* to deregister providers.
*
* @param provider the provider to register, not null
- * @throws ZoneRulesException if a region is already registered
+ * @throws ZoneRulesException if a zone ID is already registered
*/
public static void registerProvider(ZoneRulesProvider provider) {
Objects.requireNonNull(provider, "provider");
@@ -293,7 +305,7 @@
private static void registerProvider0(ZoneRulesProvider provider) {
for (String zoneId : provider.provideZoneIds()) {
Objects.requireNonNull(zoneId, "zoneId");
- ZoneRulesProvider old = ZONES.putIfAbsent(zoneId, provider.provideBind(zoneId));
+ ZoneRulesProvider old = ZONES.putIfAbsent(zoneId, provider);
if (old != null) {
throw new ZoneRulesException(
"Unable to register zone as one already registered with that ID: " + zoneId +
@@ -305,13 +317,22 @@
/**
* Refreshes the rules from the underlying data provider.
* <p>
- * This method is an extension point that allows providers to refresh their
- * rules dynamically at a time of the applications choosing.
+ * This method allows an application to request that the providers check
+ * for any updates to the provided rules.
* After calling this method, the offset stored in any {@link ZonedDateTime}
* may be invalid for the zone ID.
* <p>
- * Dynamic behavior is entirely optional and most providers, including the
- * default provider, do not support it.
+ * Dynamic update of rules is a complex problem and most applications
+ * should not use this method or dynamic rules.
+ * To achieve dynamic rules, a provider implementation will have to be written
+ * as per the specification of this class.
+ * In addition, instances of {@code ZoneRules} must not be cached in the
+ * application as they will become stale. However, the boolean flag on
+ * {@link #provideRules(String, boolean)} allows provider implementations
+ * to control the caching of {@code ZoneId}, potentially ensuring that
+ * all objects in the system see the new rules.
+ * Note that there is likely to be a cost in performance of a dynamic rules
+ * provider. Note also that no dynamic rules provider is in this specification.
*
* @return true if the rules were updated
* @throws ZoneRulesException if an error occurs during the refresh
@@ -335,49 +356,44 @@
* SPI method to get the available zone IDs.
* <p>
* This obtains the IDs that this {@code ZoneRulesProvider} provides.
- * A provider should provide data for at least one region.
+ * A provider should provide data for at least one zone ID.
+ * The IDs are the string form of a {@link ZoneId}.
* <p>
- * The returned regions remain available and valid for the lifetime of the application.
- * A dynamic provider may increase the set of regions as more data becomes available.
+ * The returned zone IDs remain available and valid for the lifetime of the application.
+ * A dynamic provider may increase the set of IDs as more data becomes available.
*
- * @return the unmodifiable set of region IDs being provided, not null
+ * @return the set of zone IDs being provided, not null
+ * @throws ZoneRulesException if a problem occurs while providing the IDs
*/
protected abstract Set<String> provideZoneIds();
/**
- * SPI method to bind to the specified zone ID.
- * <p>
- * {@code ZoneRulesProvider} has a lookup from zone ID to provider.
- * This method is used when building that lookup, allowing providers
- * to insert a derived provider that is precisely tuned to the zone ID.
- * This replaces two hash map lookups by one, enhancing performance.
- * <p>
- * This optimization is optional. Returning {@code this} is acceptable.
- * <p>
- * This implementation creates a bound provider that caches the
- * rules from the underlying provider. The request to version history
- * is forward on to the underlying. This is suitable for providers that
- * cannot change their contents during the lifetime of the JVM.
- *
- * @param zoneId the zone region ID as used by {@code ZoneId}, not null
- * @return the resolved provider for the ID, not null
- * @throws DateTimeException if there is no provider for the specified group
- */
- protected ZoneRulesProvider provideBind(String zoneId) {
- return new BoundProvider(this, zoneId);
- }
-
- /**
* SPI method to get the rules for the zone ID.
* <p>
- * This loads the rules for the region and version specified.
- * The version may be null to indicate the "latest" version.
+ * This loads the rules for the specified zone ID.
+ * The provider implementation must validate that the zone ID is valid and
+ * available, throwing a {@code ZoneRulesException} if it is not.
+ * The result of the method in the valid case depends on the caching flag.
+ * <p>
+ * If the provider implementation is not dynamic, then the result of the
+ * method must be the non-null set of rules selected by the ID.
+ * <p>
+ * If the provider implementation is dynamic, then the flag gives the option
+ * of preventing the returned rules from being cached in {@link ZoneId}.
+ * When the flag is true, the provider is permitted to return null, where
+ * null will prevent the rules from being cached in {@code ZoneId}.
+ * When the flag is false, the provider must return non-null rules.
*
- * @param regionId the time-zone region ID, not null
- * @return the rules, not null
- * @throws DateTimeException if rules cannot be obtained
+ * @param zoneId the zone ID as defined by {@code ZoneId}, not null
+ * @param forCaching whether the rules are being queried for caching,
+ * true if the returned rules will be cached by {@code ZoneId},
+ * false if they will be returned to the user without being cached in {@code ZoneId}
+ * @return the rules, null if {@code forCaching} is true and this
+ * is a dynamic provider that wants to prevent caching in {@code ZoneId},
+ * otherwise not null
+ * @throws ZoneRulesException if rules cannot be obtained for the zone ID
*/
- protected abstract ZoneRules provideRules(String regionId);
+ protected abstract ZoneRules provideRules(String zoneId, boolean forCaching);
/**
* SPI method to get the history of rules for the zone ID.
@@ -391,16 +407,16 @@
* <p>
* Implementations must provide a result for each valid zone ID, however
* they do not have to provide a history of rules.
- * Thus the map will always contain one element, and will only contain more
- * than one element if historical rule information is available.
+ * Thus the map will contain at least one element, and will only contain
+ * more than one element if historical rule information is available.
* <p>
* The returned versions remain available and valid for the lifetime of the application.
* A dynamic provider may increase the set of versions as more data becomes available.
*
- * @param zoneId the zone region ID as used by {@code ZoneId}, not null
+ * @param zoneId the zone ID as defined by {@code ZoneId}, not null
* @return a modifiable copy of the history of the rules for the ID, sorted
* from oldest to newest, not null
- * @throws ZoneRulesException if the zone ID is unknown
+ * @throws ZoneRulesException if history cannot be obtained for the zone ID
*/
protected abstract NavigableMap<String, ZoneRules> provideVersions(String zoneId);
@@ -415,46 +431,10 @@
* This implementation returns false.
*
* @return true if the rules were updated
- * @throws DateTimeException if an error occurs during the refresh
+ * @throws ZoneRulesException if an error occurs during the refresh
*/
protected boolean provideRefresh() {
return false;
}
- //-------------------------------------------------------------------------
- /**
- * A provider bound to a single zone ID.
- */
- private static class BoundProvider extends ZoneRulesProvider {
- private final ZoneRulesProvider provider;
- private final String zoneId;
- private final ZoneRules rules;
-
- private BoundProvider(ZoneRulesProvider provider, String zoneId) {
- this.provider = provider;
- this.zoneId = zoneId;
- this.rules = provider.provideRules(zoneId);
- }
-
- @Override
- protected Set<String> provideZoneIds() {
- return new HashSet<>(Collections.singleton(zoneId));
- }
-
- @Override
- protected ZoneRules provideRules(String regionId) {
- return rules;
- }
-
- @Override
- protected NavigableMap<String, ZoneRules> provideVersions(String zoneId) {
- return provider.provideVersions(zoneId);
- }
-
- @Override
- public String toString() {
- return zoneId;
- }
- }
-
}
diff --git a/test/java/time/tck/java/time/zone/TCKZoneRulesProvider.java b/test/java/time/tck/java/time/zone/TCKZoneRulesProvider.java
--- a/test/java/time/tck/java/time/zone/TCKZoneRulesProvider.java
+++ b/test/java/time/tck/java/time/zone/TCKZoneRulesProvider.java
@@ -59,6 +59,7 @@
*/
package tck.java.time.zone;
+import java.time.ZoneId;
import java.time.zone.*;
import test.java.time.zone.*;
@@ -87,7 +88,7 @@
//-----------------------------------------------------------------------
// getAvailableZoneIds()
//-----------------------------------------------------------------------
- @Test(groups={"tck"})
+ @Test
public void test_getAvailableGroupIds() {
Set<String> zoneIds = ZoneRulesProvider.getAvailableZoneIds();
assertEquals(zoneIds.contains("Europe/London"), true);
@@ -98,34 +99,48 @@
}
//-----------------------------------------------------------------------
- // getRules(String)
+ // getRules(String,boolean)
//-----------------------------------------------------------------------
- @Test(groups={"tck"})
- public void test_getRules_String() {
- ZoneRules rules = ZoneRulesProvider.getRules("Europe/London");
+ @Test
+ public void test_getRules_StringBoolean() {
+ ZoneRules rules = ZoneRulesProvider.getRules("Europe/London", false);
assertNotNull(rules);
- ZoneRules rules2 = ZoneRulesProvider.getRules("Europe/London");
+ ZoneRules rules2 = ZoneRulesProvider.getRules("Europe/London", false);
assertEquals(rules2, rules);
}
- @Test(groups={"tck"}, expectedExceptions=ZoneRulesException.class)
- public void test_getRules_String_unknownId() {
- ZoneRulesProvider.getRules("Europe/Lon");
+ @Test(expectedExceptions=ZoneRulesException.class)
+ public void test_getRules_StringBoolean_unknownId() {
+ ZoneRulesProvider.getRules("Europe/Lon", false);
}
- @Test(groups={"tck"}, expectedExceptions=NullPointerException.class)
- public void test_getRules_String_null() {
- ZoneRulesProvider.getRules(null);
+ @Test(expectedExceptions=NullPointerException.class)
+ public void test_getRules_StringBoolean_null() {
+ ZoneRulesProvider.getRules(null, false);
+ }
+
+ @Test
+ public void test_getRules_StringBoolean_dynamic() {
+ MockDynamicProvider dynamicProvider = new MockDynamicProvider();
+ ZoneRulesProvider.registerProvider(dynamicProvider);
+
+ assertEquals(dynamicProvider.count, 0);
+ ZoneRules rules1 = ZoneId.of("DynamicLocation").getRules();
+ assertEquals(dynamicProvider.count, 2);
+ assertEquals(rules1, dynamicProvider.BASE);
+ ZoneRules rules2 = ZoneId.of("DynamicLocation").getRules();
+ assertEquals(dynamicProvider.count, 4);
+ assertEquals(rules2, dynamicProvider.ALTERNATE);
}
//-----------------------------------------------------------------------
// getVersions(String)
//-----------------------------------------------------------------------
- @Test(groups={"tck"})
+ @Test
public void test_getVersions_String() {
NavigableMap<String, ZoneRules> versions = ZoneRulesProvider.getVersions("Europe/London");
assertTrue(versions.size() >= 1);
- ZoneRules rules = ZoneRulesProvider.getRules("Europe/London");
+ ZoneRules rules = ZoneRulesProvider.getRules("Europe/London", false);
assertEquals(versions.lastEntry().getValue(), rules);
NavigableMap<String, ZoneRules> copy = new TreeMap<>(versions);
@@ -135,12 +150,12 @@
assertEquals(versions2, copy);
}
- @Test(groups={"tck"}, expectedExceptions=ZoneRulesException.class)
+ @Test(expectedExceptions=ZoneRulesException.class)
public void test_getVersions_String_unknownId() {
ZoneRulesProvider.getVersions("Europe/Lon");
}
- @Test(groups={"tck"}, expectedExceptions=NullPointerException.class)
+ @Test(expectedExceptions=NullPointerException.class)
public void test_getVersions_String_null() {
ZoneRulesProvider.getVersions(null);
}
@@ -148,7 +163,7 @@
//-----------------------------------------------------------------------
// refresh()
//-----------------------------------------------------------------------
- @Test(groups={"tck"})
+ @Test
public void test_refresh() {
assertEquals(ZoneRulesProvider.refresh(), false);
}
@@ -156,7 +171,7 @@
//-----------------------------------------------------------------------
// registerProvider()
//-----------------------------------------------------------------------
- @Test(groups={"tck"})
+ @Test
public void test_registerProvider() {
Set<String> pre = ZoneRulesProvider.getAvailableZoneIds();
assertEquals(pre.contains("FooLocation"), false);
@@ -165,18 +180,15 @@
Set<String> post = ZoneRulesProvider.getAvailableZoneIds();
assertEquals(post.contains("FooLocation"), true);
- assertEquals(ZoneRulesProvider.getRules("FooLocation"), ZoneOffset.of("+01:45").getRules());
+ assertEquals(ZoneRulesProvider.getRules("FooLocation", false), ZoneOffset.of("+01:45").getRules());
}
+ //-----------------------------------------------------------------------
static class MockTempProvider extends ZoneRulesProvider {
final ZoneRules rules = ZoneOffset.of("+01:45").getRules();
@Override
public Set<String> provideZoneIds() {
- return new HashSet<String>(Collections.singleton("FooLocation"));
- }
- @Override
- protected ZoneRulesProvider provideBind(String zoneId) {
- return this;
+ return new HashSet<>(Collections.singleton("FooLocation"));
}
@Override
protected NavigableMap<String, ZoneRules> provideVersions(String zoneId) {
@@ -185,7 +197,7 @@
return result;
}
@Override
- protected ZoneRules provideRules(String zoneId) {
+ protected ZoneRules provideRules(String zoneId, boolean forCaching) {
if (zoneId.equals("FooLocation")) {
return rules;
}
@@ -193,4 +205,31 @@
}
}
+ static class MockDynamicProvider extends ZoneRulesProvider {
+ final ZoneRules BASE = ZoneOffset.of("+01:15").getRules();
+ final ZoneRules ALTERNATE = ZoneOffset.of("+01:30").getRules();
+ int count = 0;
+ @Override
+ public Set<String> provideZoneIds() {
+ return new HashSet<>(Collections.singleton("DynamicLocation"));
+ }
+ @Override
+ protected NavigableMap<String, ZoneRules> provideVersions(String zoneId) {
+ NavigableMap<String, ZoneRules> result = new TreeMap<>();
+ result.put("DynamicVersion1", BASE);
+ if (count > 2) {
+ result.put("DynamicVersion2", ALTERNATE);
+ }
+ return result;
+ }
+ @Override
+ protected ZoneRules provideRules(String zoneId, boolean forCaching) {
+ count++;
+ if (zoneId.equals("DynamicLocation")) {
+ return (forCaching ? null : (count > 2 ? ALTERNATE : BASE));
+ }
+ throw new ZoneRulesException("Invalid");
+ }
+ }
+
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment