Refactoring before changing behavior
This commit changes the Errors class to be more involved in flow
control and updates the two users accordingly.
It also renames some long variable names and contains other tidy ups.
Bug: 155738410
Test: Ran update-tzdata.py
Change-Id: If4eee63b144d9b64d41da400f3296f28b2752cd2
diff --git a/input_tools/android/common/src/main/java/com/android/libcore/timezone/util/Errors.java b/input_tools/android/common/src/main/java/com/android/libcore/timezone/util/Errors.java
index 24c608c..e1e48f9 100644
--- a/input_tools/android/common/src/main/java/com/android/libcore/timezone/util/Errors.java
+++ b/input_tools/android/common/src/main/java/com/android/libcore/timezone/util/Errors.java
@@ -16,12 +16,17 @@
package com.android.libcore.timezone.util;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
/**
- * Stores context, errors and error severity for logging and flow control.
+ * Stores context, errors and error severity for logging and flow control. This class distinguishes
+ * between warnings (just info), errors (may not be immediately fatal) and fatal (immediately
+ * fatal).
*/
public final class Errors {
@@ -45,19 +50,29 @@
return scopes.removeLast();
}
- public void addFatal(String msg) {
- level = Math.max(level, LEVEL_FATAL);
- add(msg);
+ /** Adds a fatal error, and immediately throws {@link HaltExecutionException}. */
+ public HaltExecutionException addFatalAndHalt(String msg) throws HaltExecutionException {
+ addInternal(msg, null, LEVEL_FATAL);
+ throw new HaltExecutionException("Fatal error");
+ }
+
+ /** Adds a fatal error, and immediately throws {@link HaltExecutionException}. */
+ public HaltExecutionException addFatalAndHalt(String msg, Throwable t)
+ throws HaltExecutionException {
+ addInternal(msg, t, LEVEL_FATAL);
+ throw new HaltExecutionException("Fatal error");
}
public void addError(String msg) {
- level = Math.max(level, LEVEL_ERROR);
- add(msg);
+ addInternal(msg, null, LEVEL_ERROR);
+ }
+
+ public void addError(String msg, Throwable t) {
+ addInternal(msg, t, LEVEL_ERROR);
}
public void addWarning(String msg) {
- level = Math.max(level, LEVEL_WARNING);
- add(msg);
+ addInternal(msg, null, LEVEL_WARNING);
}
public String asString() {
@@ -73,15 +88,46 @@
return messages.isEmpty();
}
+ /** True if there are error or fatal messages. */
public boolean hasError() {
return level >= LEVEL_ERROR;
}
+ /** True if there are fatal messages. */
public boolean hasFatal() {
return level >= LEVEL_FATAL;
}
- private void add(String msg) {
+ private void addInternal(String msg, Throwable t, int level) {
+ this.level = Math.max(this.level, level);
+ addMessage(msg);
+ if (t != null) {
+ try (StringWriter out = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(out)) {
+ t.printStackTrace(printWriter);
+ addMessage(out.toString());
+ } catch (IOException e) {
+ // Impossible - this is actually a compiler bug. Nothing throws IOException above.
+ throw new AssertionError("Impossible exception thrown", e);
+ }
+ }
+ }
+
+ private void addMessage(String msg) {
messages.add(scopes.toString() + ": " + msg);
}
+
+ /** Throws a {@link HaltExecutionException} if there are any error or fatal messages. */
+ public void throwIfError(String why) throws HaltExecutionException {
+ if (hasError()) {
+ throw new HaltExecutionException(why);
+ }
+ }
+
+ /** Thrown to halt execution. */
+ public static class HaltExecutionException extends Exception {
+ HaltExecutionException(String why) {
+ super(why);
+ }
+ }
}
diff --git a/input_tools/android/common/src/test/java/com/android/libcore/timezone/util/ErrorsTest.java b/input_tools/android/common/src/test/java/com/android/libcore/timezone/util/ErrorsTest.java
index 5a6e717..1d568e5 100644
--- a/input_tools/android/common/src/test/java/com/android/libcore/timezone/util/ErrorsTest.java
+++ b/input_tools/android/common/src/test/java/com/android/libcore/timezone/util/ErrorsTest.java
@@ -20,8 +20,10 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import com.android.libcore.timezone.testing.TestUtils;
+import com.android.libcore.timezone.util.Errors.HaltExecutionException;
public class ErrorsTest {
@@ -62,7 +64,10 @@
assertFalse(errors.hasError());
assertFalse(errors.hasFatal());
- errors.addFatal("Hello");
+ try {
+ throw errors.addFatalAndHalt("Hello");
+ } catch (HaltExecutionException expected) {}
+
assertFalse(errors.isEmpty());
assertTrue(errors.hasError());
assertTrue(errors.hasFatal());
@@ -80,13 +85,19 @@
errors.addError("John Cleese");
errors.pushScope("Holy grail");
- errors.addFatal("Silly place");
+ try {
+ errors.addFatalAndHalt("Silly place");
+ fail();
+ } catch (HaltExecutionException expected) {}
errors.popScope();
errors.addError("Michael Palin");
errors.pushScope("Parrot sketch");
- errors.addFatal("Fjords");
+ try {
+ errors.addFatalAndHalt("Fjords");
+ fail();
+ } catch (HaltExecutionException expected) {}
errors.popScope();
String[] lines = errors.asString().split("\n");
diff --git a/input_tools/android/telephonylookup_generator/src/main/java/com/android/libcore/timezone/telephonylookup/TelephonyLookupGenerator.java b/input_tools/android/telephonylookup_generator/src/main/java/com/android/libcore/timezone/telephonylookup/TelephonyLookupGenerator.java
index 978d264..4626d8d 100644
--- a/input_tools/android/telephonylookup_generator/src/main/java/com/android/libcore/timezone/telephonylookup/TelephonyLookupGenerator.java
+++ b/input_tools/android/telephonylookup_generator/src/main/java/com/android/libcore/timezone/telephonylookup/TelephonyLookupGenerator.java
@@ -19,6 +19,7 @@
import com.android.libcore.timezone.telephonylookup.proto.TelephonyLookupProtoFile;
import com.android.libcore.timezone.util.Errors;
+import com.android.libcore.timezone.util.Errors.HaltExecutionException;
import com.ibm.icu.util.ULocale;
@@ -68,78 +69,75 @@
}
boolean execute() throws IOException {
- // Parse the countryzones input file.
- TelephonyLookupProtoFile.TelephonyLookup telephonyLookupIn;
+ Errors errors = new Errors();
try {
- telephonyLookupIn = parseTelephonyLookupTextFile(telephonyLookupProtoFile);
- } catch (ParseException e) {
- logError("Unable to parse " + telephonyLookupProtoFile, e);
- return false;
- }
+ // Parse the countryzones input file.
+ TelephonyLookupProtoFile.TelephonyLookup telephonyLookupIn;
+ try {
+ telephonyLookupIn = parseTelephonyLookupTextFile(telephonyLookupProtoFile);
+ } catch (ParseException e) {
+ throw errors.addFatalAndHalt("Unable to parse " + telephonyLookupProtoFile, e);
+ }
- List<TelephonyLookupProtoFile.Network> networksIn = telephonyLookupIn.getNetworksList();
+ List<TelephonyLookupProtoFile.Network> networksIn = telephonyLookupIn.getNetworksList();
- Errors processingErrors = new Errors();
- processingErrors.pushScope("Validation");
- validateNetworks(networksIn, processingErrors);
- processingErrors.popScope();
+ validateNetworks(networksIn, errors);
+ errors.throwIfError("One or more validation errors encountered");
- // Validation failed, so stop.
- if (processingErrors.hasFatal()) {
- logInfo("Issues:\n" + processingErrors.asString());
- return false;
- }
-
- TelephonyLookupXmlFile.TelephonyLookup telephonyLookupOut =
- createOutputTelephonyLookup(networksIn);
- if (!processingErrors.hasError()) {
- // Write the output structure if there wasn't an error.
+ TelephonyLookupXmlFile.TelephonyLookup telephonyLookupOut =
+ createOutputTelephonyLookup(networksIn);
logInfo("Writing " + outputFile);
try {
TelephonyLookupXmlFile.write(telephonyLookupOut, outputFile);
} catch (XMLStreamException e) {
- e.printStackTrace(System.err);
- processingErrors.addFatal("Unable to write output file");
+ throw errors.addFatalAndHalt("Unable to write output file", e);
+ }
+ } catch (HaltExecutionException e) {
+ e.printStackTrace();
+ logError("Stopping due to fatal error: " + e.getMessage());
+ } finally {
+ // Report all warnings / errors
+ if (!errors.isEmpty()) {
+ logInfo("Issues:\n" + errors.asString());
}
}
-
- // Report all warnings / errors
- if (!processingErrors.isEmpty()) {
- logInfo("Issues:\n" + processingErrors.asString());
- }
-
- return !processingErrors.hasError();
+ return !errors.hasError();
}
- private static void validateNetworks(
- List<TelephonyLookupProtoFile.Network> networksIn, Errors processingErrors) {
- Set<String> knownIsoCountries = getLowerCaseCountryIsoCodes();
- Set<String> mccMncSet = new HashSet<>();
- for (TelephonyLookupProtoFile.Network networkIn : networksIn) {
- String mcc = networkIn.getMcc();
- if (mcc.length() != 3 || !isAsciiNumeric(mcc)) {
- processingErrors.addFatal("mcc=" + mcc + " must have 3 decimal digits");
- }
+ private static void validateNetworks(List<TelephonyLookupProtoFile.Network> networksIn,
+ Errors errors) {
+ errors.pushScope("validateNetworks");
+ try {
+ Set<String> knownIsoCountries = getLowerCaseCountryIsoCodes();
+ Set<String> mccMncSet = new HashSet<>();
+ for (TelephonyLookupProtoFile.Network networkIn : networksIn) {
+ String mcc = networkIn.getMcc();
+ if (mcc.length() != 3 || !isAsciiNumeric(mcc)) {
+ errors.addError("mcc=" + mcc + " must have 3 decimal digits");
+ }
- String mnc = networkIn.getMnc();
- if (!(mnc.length() == 2 || mnc.length() == 3) || !isAsciiNumeric(mnc)) {
- processingErrors.addFatal("mnc=" + mnc + " must have 2 or 3 decimal digits");
- }
+ String mnc = networkIn.getMnc();
+ if (!(mnc.length() == 2 || mnc.length() == 3) || !isAsciiNumeric(mnc)) {
+ errors.addError("mnc=" + mnc + " must have 2 or 3 decimal digits");
+ }
- String mccMnc = "" + mcc + mnc;
- if (!mccMncSet.add(mccMnc)) {
- processingErrors.addFatal("Duplicate entry for mcc=" + mcc + ", mnc=" + mnc);
- }
+ String mccMnc = "" + mcc + mnc;
+ if (!mccMncSet.add(mccMnc)) {
+ errors.addError("Duplicate entry for mcc=" + mcc + ", mnc=" + mnc);
+ }
- String countryIsoCode = networkIn.getCountryIsoCode();
- String countryIsoCodeLower = countryIsoCode.toLowerCase(Locale.ROOT);
- if (!countryIsoCodeLower.equals(countryIsoCode)) {
- processingErrors.addFatal("Country code not lower case: " + countryIsoCode);
- }
+ String countryIsoCode = networkIn.getCountryIsoCode();
+ String countryIsoCodeLower = countryIsoCode.toLowerCase(Locale.ROOT);
+ if (!countryIsoCodeLower.equals(countryIsoCode)) {
+ errors.addError("Country code not lower case: " + countryIsoCode);
+ }
- if (!knownIsoCountries.contains(countryIsoCodeLower)) {
- processingErrors.addFatal("Country code not known: " + countryIsoCode);
+ if (!knownIsoCountries.contains(countryIsoCodeLower)) {
+ errors.addError("Country code not known: " + countryIsoCode);
+ }
}
+ } finally {
+ errors.popScope();
}
}
diff --git a/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/TzLookupGenerator.java b/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/TzLookupGenerator.java
index 95525bf..f1175f0 100644
--- a/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/TzLookupGenerator.java
+++ b/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/TzLookupGenerator.java
@@ -19,6 +19,7 @@
import com.android.libcore.timezone.tzlookup.zonetree.CountryZoneTree;
import com.android.libcore.timezone.tzlookup.zonetree.CountryZoneUsage;
import com.android.libcore.timezone.util.Errors;
+import com.android.libcore.timezone.util.Errors.HaltExecutionException;
import com.ibm.icu.util.BasicTimeZone;
import com.ibm.icu.util.Calendar;
import com.ibm.icu.util.GregorianCalendar;
@@ -74,11 +75,6 @@
/**
* Executes the generator.
- *
- * Positional arguments:
- * 1: The countryzones.txt file
- * 2: the zone.tab file
- * 3: the file to generate
*/
public static void main(String[] args) throws Exception {
if (args.length != 3) {
@@ -97,85 +93,110 @@
this.outputFile = outputFile;
}
- boolean execute() throws IOException {
- // Parse the countryzones input file.
- CountryZonesFile.CountryZones countryZonesIn;
+ boolean execute() {
+ Errors errors = new Errors();
try {
- countryZonesIn = CountryZonesFileSupport.parseCountryZonesTextFile(countryZonesFile);
- } catch (ParseException e) {
- logError("Unable to parse " + countryZonesFile, e);
- return false;
- }
+ // Parse the countryzones input file.
+ CountryZonesFile.CountryZones countryZonesIn =
+ parseAndValidateCountryZones(countryZonesFile, errors);
- // Check the countryzones rules version matches the version that ICU is using.
- String icuTzDataVersion = TimeZone.getTZDataVersion();
- String inputIanaVersion = countryZonesIn.getIanaVersion();
- if (!icuTzDataVersion.equals(inputIanaVersion)) {
- logError("Input data (countryzones.txt) is for " + inputIanaVersion
- + " but the ICU you have is for " + icuTzDataVersion);
- return false;
- }
+ // Check the countryzones.txt rules version matches the version that ICU is using.
+ String icuTzDataVersion = TimeZone.getTZDataVersion();
+ String inputIanaVersion = countryZonesIn.getIanaVersion();
+ if (!icuTzDataVersion.equals(inputIanaVersion)) {
+ throw errors.addFatalAndHalt("Input data (countryzones.txt) is for "
+ + inputIanaVersion + " but the ICU you have is for " + icuTzDataVersion);
+ }
- // Pull out information we want to validate against from zone.tab (which we have to assume
- // matches the ICU version since it doesn't contain its own version info).
- ZoneTabFile zoneTabIn = ZoneTabFile.parse(zoneTabFile);
- Map<String, List<String>> zoneTabMapping =
- ZoneTabFile.createCountryToOlsonIdsMap(zoneTabIn);
- List<CountryZonesFile.Country> countriesIn = countryZonesIn.getCountriesList();
- List<String> countriesInIsos = CountryZonesFileSupport.extractIsoCodes(countriesIn);
- // Sanity check the countryzones file only contains lower-case country codes. The output
- // file uses them and the on-device code assumes lower case.
- if (!Utils.allLowerCaseAscii(countriesInIsos)) {
- logError("Non-lowercase country ISO codes found in: " + countriesInIsos);
- return false;
- }
- // Sanity check the countryzones file doesn't contain duplicate country entries.
- if (!Utils.allUnique(countriesInIsos)) {
- logError("Duplicate input country entries found: " + countriesInIsos);
- return false;
- }
+ // Pull out information we want to validate against from zone.tab (which we have to
+ // assume matches the ICU version since it doesn't contain its own version info).
+ Map<String, List<String>> zoneTabMapping = parseZoneTabFile(zoneTabFile, errors);
- // Validate the country iso codes found in the countryzones against those in zone.tab.
- // zone.tab uses upper case, countryzones uses lower case.
- List<String> upperCaseCountriesInIsos = Utils.toUpperCase(countriesInIsos);
- Set<String> timezonesCountryIsos = new HashSet<>(upperCaseCountriesInIsos);
- Set<String> zoneTabCountryIsos = zoneTabMapping.keySet();
- if (!zoneTabCountryIsos.equals(timezonesCountryIsos)) {
- logError(zoneTabFile + " contains "
- + Utils.subtract(zoneTabCountryIsos, timezonesCountryIsos)
- + " not present in countryzones, "
- + countryZonesFile + " contains "
- + Utils.subtract(timezonesCountryIsos, zoneTabCountryIsos)
- + " not present in zonetab.");
- return false;
- }
+ List<CountryZonesFile.Country> countriesIn = countryZonesIn.getCountriesList();
+ List<String> countriesInIsos = CountryZonesFileSupport.extractIsoCodes(countriesIn);
- Errors processingErrors = new Errors();
- TzLookupFile.TimeZones timeZonesOut = createOutputTimeZones(
- inputIanaVersion, zoneTabMapping, countriesIn, processingErrors);
- if (!processingErrors.hasError()) {
+ // Sanity check the countryzones file only contains lower-case country codes. The output
+ // file uses them and the on-device code assumes lower case.
+ if (!Utils.allLowerCaseAscii(countriesInIsos)) {
+ throw errors.addFatalAndHalt(
+ "Non-lowercase country ISO codes found in: " + countriesInIsos);
+ }
+ // Sanity check the countryzones file doesn't contain duplicate country entries.
+ if (!Utils.allUnique(countriesInIsos)) {
+ throw errors.addFatalAndHalt(
+ "Duplicate input country entries found: " + countriesInIsos);
+ }
+
+ // Validate the country iso codes found in the countryzones.txt against those in
+ // zone.tab. zone.tab uses upper case, countryzones uses lower case.
+ List<String> upperCaseCountriesInIsos = Utils.toUpperCase(countriesInIsos);
+ Set<String> timezonesCountryIsos = new HashSet<>(upperCaseCountriesInIsos);
+ Set<String> zoneTabCountryIsos = zoneTabMapping.keySet();
+ if (!zoneTabCountryIsos.equals(timezonesCountryIsos)) {
+ throw errors.addFatalAndHalt(zoneTabFile + " contains "
+ + Utils.subtract(zoneTabCountryIsos, timezonesCountryIsos)
+ + " not present in countryzones, "
+ + countryZonesFile + " contains "
+ + Utils.subtract(timezonesCountryIsos, zoneTabCountryIsos)
+ + " not present in zonetab.");
+ }
+
+ TzLookupFile.TimeZones timeZonesOut = createOutputTimeZones(
+ inputIanaVersion, zoneTabMapping, countriesIn, errors);
+ errors.throwIfError("Errors accumulated");
+
// Write the output structure if there wasn't an error.
logInfo("Writing " + outputFile);
try {
TzLookupFile.write(timeZonesOut, outputFile);
} catch (XMLStreamException e) {
- e.printStackTrace(System.err);
- processingErrors.addFatal("Unable to write output file");
+ throw errors.addFatalAndHalt("Unable to write output file", e);
+ }
+ return true;
+ } catch (HaltExecutionException | IOException e) {
+ logError("Stopping due to fatal condition", e);
+ return false;
+ } finally {
+ // Report all warnings / errors
+ if (!errors.isEmpty()) {
+ logInfo("Issues:\n" + errors.asString());
}
}
+ }
- // Report all warnings / errors
- if (!processingErrors.isEmpty()) {
- logInfo("Issues:\n" + processingErrors.asString());
+ private Map<String, List<String>> parseZoneTabFile(String zoneTabFile, Errors errors)
+ throws HaltExecutionException {
+ errors.pushScope("Parsing " + zoneTabFile);
+ try {
+ ZoneTabFile zoneTabIn;
+ zoneTabIn = ZoneTabFile.parse(zoneTabFile);
+ return ZoneTabFile.createCountryToOlsonIdsMap(zoneTabIn);
+ } catch (ParseException | IOException e) {
+ throw errors.addFatalAndHalt("Unable to parse " + zoneTabFile, e);
+ } finally {
+ errors.popScope();
}
+ }
- return !processingErrors.hasError();
+ private static CountryZonesFile.CountryZones parseAndValidateCountryZones(
+ String countryZonesFile, Errors errors) throws HaltExecutionException {
+ errors.pushScope("Parsing " + countryZonesFile);
+ try {
+ CountryZonesFile.CountryZones countryZonesIn;
+ countryZonesIn = CountryZonesFileSupport.parseCountryZonesTextFile(countryZonesFile);
+ return countryZonesIn;
+ } catch (ParseException | IOException e) {
+ throw errors.addFatalAndHalt("Unable to parse " + countryZonesFile, e);
+ } finally {
+ errors.popScope();
+ }
}
private static TzLookupFile.TimeZones createOutputTimeZones(String inputIanaVersion,
Map<String, List<String>> zoneTabMapping, List<CountryZonesFile.Country> countriesIn,
- Errors processingErrors) {
+ Errors errors) throws HaltExecutionException {
+
// Start constructing the output structure.
TzLookupFile.TimeZones timeZonesOut = new TzLookupFile.TimeZones(inputIanaVersion);
TzLookupFile.CountryZones countryZonesOut = new TzLookupFile.CountryZones();
@@ -194,63 +215,60 @@
String isoCode = countryIn.getIsoCode();
List<String> zoneTabCountryTimeZoneIds = zoneTabMapping.get(isoCode.toUpperCase());
if (zoneTabCountryTimeZoneIds == null) {
- processingErrors.addError("Country=" + isoCode + " missing from zone.tab");
+ errors.addError("Country=" + isoCode + " missing from zone.tab");
// No point in continuing.
continue;
}
TzLookupFile.Country countryOut = processCountry(
offsetSampleTimeMillis, everUseUtcStartTimeMillis, countryIn,
- zoneTabCountryTimeZoneIds, processingErrors);
- if (processingErrors.hasFatal()) {
- // Stop if there's a fatal error, continue processing countries if there are just
- // errors.
- break;
- } else if (countryOut == null) {
+ zoneTabCountryTimeZoneIds, errors);
+ if (countryOut == null) {
+ // Continue processing countries if there are only errors.
continue;
}
countryZonesOut.addCountry(countryOut);
}
+ errors.throwIfError("One or more countries failed");
return timeZonesOut;
}
private static TzLookupFile.Country processCountry(long offsetSampleTimeMillis,
long everUseUtcStartTimeMillis, CountryZonesFile.Country countryIn,
- List<String> zoneTabCountryTimeZoneIds,
- Errors processingErrors) {
+ List<String> zoneTabCountryTimeZoneIds, Errors errors) {
String isoCode = countryIn.getIsoCode();
- processingErrors.pushScope("country=" + isoCode);
+ errors.pushScope("country=" + isoCode);
try {
// Each Country must have >= 1 time zone.
List<CountryZonesFile.TimeZoneMapping> timeZonesIn =
countryIn.getTimeZoneMappingsList();
if (timeZonesIn.isEmpty()) {
- processingErrors.addError("No time zones");
+ errors.addError("No time zones");
// No point in continuing.
return null;
}
- // Look for duplicate time zone IDs.
List<String> countryTimeZoneIds = CountryZonesFileSupport.extractIds(timeZonesIn);
+
+ // Look for duplicate time zone IDs.
if (!Utils.allUnique(countryTimeZoneIds)) {
- processingErrors.addError("country's zones=" + countryTimeZoneIds
- + " contains duplicates");
+ errors.addError("country's zones=" + countryTimeZoneIds + " contains duplicates");
// No point in continuing.
return null;
}
// Each Country needs a default time zone ID (but we can guess in some cases).
- String defaultTimeZoneId = determineCountryDefaultZoneId(countryIn, processingErrors);
+ String defaultTimeZoneId = determineCountryDefaultZoneId(countryIn, errors);
if (defaultTimeZoneId == null) {
// No point in continuing.
return null;
}
boolean defaultTimeZoneBoost =
- determineCountryDefaultTimeZoneBoost(countryIn, processingErrors);
+ determineCountryDefaultTimeZoneBoost(countryIn, errors);
// Validate the default.
if (!countryTimeZoneIds.contains(defaultTimeZoneId)) {
- processingErrors.addError("defaultTimeZoneId=" + defaultTimeZoneId
+ errors.addError("defaultTimeZoneId=" + defaultTimeZoneId
+ " is not one of the country's zones=" + countryTimeZoneIds);
// No point in continuing.
return null;
@@ -258,52 +276,47 @@
// Validate the other zone IDs.
try {
- processingErrors.pushScope("validate country zone ids");
- boolean errors = false;
+ errors.pushScope("validate country zone ids");
for (String countryTimeZoneId : countryTimeZoneIds) {
if (invalidTimeZoneId(countryTimeZoneId)) {
- processingErrors.addError("countryTimeZoneId=" + countryTimeZoneId
+ errors.addError("countryTimeZoneId=" + countryTimeZoneId
+ " is not a valid zone ID");
- errors = true;
}
}
- if (errors) {
+ if (errors.hasError()) {
// No point in continuing.
return null;
}
} finally {
- processingErrors.popScope();
+ errors.popScope();
}
// Work out the hint for whether the country uses a zero offset from UTC.
boolean everUsesUtc = anyZonesUseUtc(countryTimeZoneIds, everUseUtcStartTimeMillis);
// Validate the country information against the equivalent information in zone.tab.
- processingErrors.pushScope("zone.tab comparison");
+ errors.pushScope("zone.tab comparison");
try {
// Look for unexpected duplicate time zone IDs in zone.tab
if (!Utils.allUnique(zoneTabCountryTimeZoneIds)) {
- processingErrors.addError(
- "Duplicate time zone IDs found:" + zoneTabCountryTimeZoneIds);
+ errors.addError("Duplicate time zone IDs found:" + zoneTabCountryTimeZoneIds);
// No point in continuing.
return null;
-
}
if (!Utils.setEquals(zoneTabCountryTimeZoneIds, countryTimeZoneIds)) {
- processingErrors.addError("IANA lists " + isoCode
+ errors.addError("IANA lists " + isoCode
+ " as having zones: " + zoneTabCountryTimeZoneIds
+ ", but countryzones has " + countryTimeZoneIds);
// No point in continuing.
return null;
}
} finally {
- processingErrors.popScope();
+ errors.popScope();
}
// Calculate countryZoneUsage.
- CountryZoneUsage countryZoneUsage =
- calculateCountryZoneUsage(countryIn, processingErrors);
+ CountryZoneUsage countryZoneUsage = calculateCountryZoneUsage(countryIn, errors);
if (countryZoneUsage == null) {
// No point in continuing with this country.
return null;
@@ -315,20 +328,18 @@
// Process each input time zone.
for (CountryZonesFile.TimeZoneMapping timeZoneIn : timeZonesIn) {
- processingErrors.pushScope(
+ errors.pushScope(
"id=" + timeZoneIn.getId() + ", offset=" + timeZoneIn.getUtcOffset()
+ ", shownInPicker=" + timeZoneIn.getShownInPicker());
try {
// Validate the offset information in countryIn.
- validateNonDstOffset(offsetSampleTimeMillis, countryIn, timeZoneIn,
- processingErrors);
+ validateNonDstOffset(offsetSampleTimeMillis, countryIn, timeZoneIn, errors);
String timeZoneInId = timeZoneIn.getId();
boolean shownInPicker = timeZoneIn.getShownInPicker();
if (!countryZoneUsage.hasEntry(timeZoneInId)) {
// This implies a programming error.
- processingErrors.addFatal(
- "No entry in CountryZoneUsage for " + timeZoneInId);
+ errors.addError("No entry in CountryZoneUsage for " + timeZoneInId);
return null;
}
@@ -343,13 +354,13 @@
timeZoneInId, shownInPicker, notUsedAfterInstant);
countryOut.addTimeZoneIdentifier(timeZoneIdOut);
} finally {
- processingErrors.popScope();
+ errors.popScope();
}
}
return countryOut;
} finally{
// End of country processing.
- processingErrors.popScope();
+ errors.popScope();
}
}
@@ -357,20 +368,20 @@
* Determines the default zone ID for the country.
*/
private static String determineCountryDefaultZoneId(
- CountryZonesFile.Country countryIn, Errors processingErrorsOut) {
+ CountryZonesFile.Country countryIn, Errors errors) {
List<CountryZonesFile.TimeZoneMapping> timeZonesIn = countryIn.getTimeZoneMappingsList();
String defaultTimeZoneId;
if (countryIn.hasDefaultTimeZoneId()) {
defaultTimeZoneId = countryIn.getDefaultTimeZoneId();
if (invalidTimeZoneId(defaultTimeZoneId)) {
- processingErrorsOut.addError(
+ errors.addError(
"Default time zone ID " + defaultTimeZoneId + " is not valid");
// No point in continuing.
return null;
}
} else {
if (timeZonesIn.size() > 1) {
- processingErrorsOut.addError(
+ errors.addError(
"To pick a default time zone there must be a single offset group");
// No point in continuing.
return null;
@@ -384,14 +395,14 @@
* Determines the defaultTimeZoneBoost value for the country.
*/
private static boolean determineCountryDefaultTimeZoneBoost(
- CountryZonesFile.Country countryIn, Errors processingErrorsOut) {
+ CountryZonesFile.Country countryIn, Errors errors) {
if (!countryIn.hasDefaultTimeZoneBoost()) {
return false;
}
boolean defaultTimeZoneBoost = countryIn.getDefaultTimeZoneBoost();
if (!countryIn.hasDefaultTimeZoneId() && defaultTimeZoneBoost) {
- processingErrorsOut.addError(
+ errors.addError(
"defaultTimeZoneBoost is specified but defaultTimeZoneId is not explicit");
}
@@ -459,7 +470,7 @@
try {
utcOffsetMillis = Utils.parseUtcOffsetToMillis(utcOffsetString);
} catch (ParseException e) {
- errors.addFatal("Bad offset string: " + utcOffsetString);
+ errors.addError("Bad offset string: " + utcOffsetString);
return;
}
@@ -471,7 +482,7 @@
String timeZoneIdIn = timeZoneIn.getId();
if (invalidTimeZoneId(timeZoneIdIn)) {
- errors.addFatal("Time zone ID=" + timeZoneIdIn + " is not valid");
+ errors.addError("Time zone ID=" + timeZoneIdIn + " is not valid");
return;
}
@@ -481,7 +492,7 @@
timeZone.getOffset(offsetSampleTimeMillis, false /* local */, offsets);
int actualOffsetMillis = offsets[0];
if (actualOffsetMillis != utcOffsetMillis) {
- errors.addFatal("Offset mismatch: You will want to confirm the ordering for "
+ errors.addError("Offset mismatch: You will want to confirm the ordering for "
+ country.getIsoCode() + " still makes sense. Raw offset for "
+ timeZoneIdIn + " is " + Utils.toUtcOffsetString(actualOffsetMillis)
+ " and not " + Utils.toUtcOffsetString(utcOffsetMillis)
@@ -490,21 +501,20 @@
}
private static CountryZoneUsage calculateCountryZoneUsage(
- CountryZonesFile.Country countryIn, Errors processingErrors) {
- processingErrors.pushScope("Building zone tree");
+ CountryZonesFile.Country countryIn, Errors errors) {
+ errors.pushScope("Building zone tree");
try {
CountryZoneTree countryZoneTree = CountryZoneTree.create(
countryIn, ZONE_USAGE_CALCS_START, ZONE_USAGE_CALCS_END);
List<String> countryIssues = countryZoneTree.validateNoPriorityClashes();
if (!countryIssues.isEmpty()) {
- processingErrors
- .addError("Issues validating country zone trees. Adjust priorities:");
- countryIssues.forEach(processingErrors::addError);
+ errors.addError("Issues validating country zone trees. Adjust priorities:");
+ countryIssues.forEach(errors::addError);
return null;
}
return countryZoneTree.calculateCountryZoneUsage(ZONE_USAGE_NOT_AFTER_CUT_OFF);
} finally {
- processingErrors.popScope();
+ errors.popScope();
}
}
diff --git a/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/ZoneTabFile.java b/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/ZoneTabFile.java
index a939ca5..6b960f9 100644
--- a/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/ZoneTabFile.java
+++ b/input_tools/android/tzlookup_generator/src/main/java/com/android/libcore/timezone/tzlookup/ZoneTabFile.java
@@ -19,6 +19,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
+import java.text.ParseException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -36,7 +37,7 @@
private ZoneTabFile() {}
- static ZoneTabFile parse(String zoneTabFile) throws IOException {
+ static ZoneTabFile parse(String zoneTabFile) throws IOException, ParseException {
ZoneTabFile zoneTab = new ZoneTabFile();
List<String> lines = Files
@@ -49,9 +50,9 @@
.collect(Collectors.toList());
for (String mappingLine : mappingLines) {
- String[] fields = mappingLine.split("\t");
+ String[] fields = mappingLine.split("\t+");
if (fields.length < 3) {
- throw new IOException("Line is malformed: " + mappingLine);
+ throw new ParseException("Line is malformed: " + mappingLine, 0);
}
CountryEntry countryEntry = new CountryEntry(fields[0], fields[2]);
zoneTab.addCountryEntry(countryEntry);
@@ -74,7 +75,10 @@
countryEntry.isoCode, k -> new ArrayList<>());
olsonIds.add(countryEntry.olsonId);
}
- return countryIsoToOlsonIdsMap;
+ // Replace each list value with an immutable one.
+ countryIsoToOlsonIdsMap.forEach(
+ (k, v) -> countryIsoToOlsonIdsMap.put(k, Collections.unmodifiableList(v)));
+ return Collections.unmodifiableMap(countryIsoToOlsonIdsMap);
}
static class CountryEntry {
diff --git a/input_tools/android/tzlookup_generator/src/test/java/com/android/libcore/timezone/tzlookup/ZoneTabFileTest.java b/input_tools/android/tzlookup_generator/src/test/java/com/android/libcore/timezone/tzlookup/ZoneTabFileTest.java
index b581ffd..108b902 100644
--- a/input_tools/android/tzlookup_generator/src/test/java/com/android/libcore/timezone/tzlookup/ZoneTabFileTest.java
+++ b/input_tools/android/tzlookup_generator/src/test/java/com/android/libcore/timezone/tzlookup/ZoneTabFileTest.java
@@ -25,6 +25,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.text.ParseException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -32,7 +33,6 @@
import static junit.framework.TestCase.assertEquals;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
public class ZoneTabFileTest {
@@ -75,7 +75,7 @@
"# This is a comment",
"GB\tStuff\tEurope/London\tStuff",
"# This is a comment",
- "US\tStuff\tAmerica/New_York\tStuff",
+ "US\tStuff\t\tAmerica/New_York\tStuff",
"# This is a comment",
"US\tStuff\tAmerica/Los_Angeles",
"# This is a comment"
@@ -89,14 +89,11 @@
zoneTab.getCountryEntries());
}
- @Test
+ @Test(expected = ParseException.class)
public void parseMalformedFile() throws Exception {
// Mapping lines are expected to have at least three tab-separated columns.
String file = createFile("GB\tStuff");
- try {
- ZoneTabFile.parse(file);
- fail();
- } catch (IOException expected) {}
+ ZoneTabFile.parse(file);
}
@Test
