Browse Source

Metadata loading cleanups (#1435)

pull/1439/head
Keghani Kouzoujian 9 years ago
committed by GitHub
parent
commit
7e4a2795c3
14 changed files with 99 additions and 93 deletions
  1. +8
    -6
      java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataLoader.java
  2. +17
    -18
      java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java
  3. +2
    -2
      java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java
  4. +21
    -31
      java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java
  5. +1
    -1
      java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java
  6. +1
    -2
      java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java
  7. +14
    -12
      java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java
  8. +8
    -8
      java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java
  9. +1
    -1
      java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java
  10. +20
    -8
      java/libphonenumber/test/com/google/i18n/phonenumbers/TestMetadataTestCase.java
  11. +4
    -0
      java/pending_code_changes.txt
  12. +2
    -4
      tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java
  13. BIN
      tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar
  14. BIN
      tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar

+ 8
- 6
java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataLoader.java View File

@ -20,18 +20,20 @@ import java.io.InputStream;
/** /**
* Interface for clients to specify a customized phone metadata loader, useful for Android apps to * Interface for clients to specify a customized phone metadata loader, useful for Android apps to
* load Android resources since the library loads Java resources by default. Note that
* implementation owners have the responsibility to ensure this is thread-safe.
* load Android resources since the library loads Java resources by default, e.g. with
* <a href="http://developer.android.com/reference/android/content/res/AssetManager.html">
* AssetManager</a>. Note that implementation owners have the responsibility to ensure this is
* thread-safe.
*/ */
public interface MetadataLoader { public interface MetadataLoader {
/** /**
* Returns an input stream corresponding to the metadata to load. This method may be called * Returns an input stream corresponding to the metadata to load. This method may be called
* concurrently so implementations must be thread-safe. * concurrently so implementations must be thread-safe.
* *
* @param metadataFileName File name (including path) of metadata to load. File path is an
* absolute class path like /com/google/i18n/phonenumbers/data/PhoneNumberMetadataProto.
* @return The input stream for the metadata file. The library will close this stream
* after it is done. Return null in case the metadata file could not be found.
* @param metadataFileName file name (including path) of metadata to load. File path is an
* absolute class path like /com/google/i18n/phonenumbers/data/PhoneNumberMetadataProto
* @return the input stream for the metadata file. The library will close this stream
* after it is done. Return null in case the metadata file could not be found
*/ */
public InputStream loadMetadata(String metadataFileName); public InputStream loadMetadata(String metadataFileName);
} }

+ 17
- 18
java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java View File

@ -48,7 +48,7 @@ final class MetadataManager {
private static final String SHORT_NUMBER_METADATA_FILE_PREFIX = private static final String SHORT_NUMBER_METADATA_FILE_PREFIX =
"/com/google/i18n/phonenumbers/data/ShortNumberMetadataProto"; "/com/google/i18n/phonenumbers/data/ShortNumberMetadataProto";
private static final MetadataLoader METADATA_LOADER = new MetadataLoader() {
static final MetadataLoader DEFAULT_METADATA_LOADER = new MetadataLoader() {
@Override @Override
public InputStream loadMetadata(String metadataFileName) { public InputStream loadMetadata(String metadataFileName) {
return MetadataManager.class.getResourceAsStream(metadataFileName); return MetadataManager.class.getResourceAsStream(metadataFileName);
@ -81,16 +81,16 @@ final class MetadataManager {
if (!alternateFormatsCountryCodes.contains(countryCallingCode)) { if (!alternateFormatsCountryCodes.contains(countryCallingCode)) {
return null; return null;
} }
return getMultiFileMetadata(countryCallingCode, alternateFormatsMap,
ALTERNATE_FORMATS_FILE_PREFIX, METADATA_LOADER);
return getMetadataFromMultiFilePrefix(countryCallingCode, alternateFormatsMap,
ALTERNATE_FORMATS_FILE_PREFIX, DEFAULT_METADATA_LOADER);
} }
static PhoneMetadata getShortNumberMetadataForRegion(String regionCode) { static PhoneMetadata getShortNumberMetadataForRegion(String regionCode) {
if (!shortNumberMetadataRegionCodes.contains(regionCode)) { if (!shortNumberMetadataRegionCodes.contains(regionCode)) {
return null; return null;
} }
return getMultiFileMetadata(regionCode, shortNumberMetadataMap,
SHORT_NUMBER_METADATA_FILE_PREFIX, METADATA_LOADER);
return getMetadataFromMultiFilePrefix(regionCode, shortNumberMetadataMap,
SHORT_NUMBER_METADATA_FILE_PREFIX, DEFAULT_METADATA_LOADER);
} }
static Set<String> getSupportedShortNumberRegions() { static Set<String> getSupportedShortNumberRegions() {
@ -106,15 +106,15 @@ final class MetadataManager {
* @param filePrefix the prefix of the file to load metadata from * @param filePrefix the prefix of the file to load metadata from
* @param metadataLoader the metadata loader used to inject alternative metadata sources * @param metadataLoader the metadata loader used to inject alternative metadata sources
*/ */
static <T> PhoneMetadata getMultiFileMetadata(T key, ConcurrentHashMap<T, PhoneMetadata> map,
String filePrefix, MetadataLoader metadataLoader) {
static <T> PhoneMetadata getMetadataFromMultiFilePrefix(T key,
ConcurrentHashMap<T, PhoneMetadata> map, String filePrefix, MetadataLoader metadataLoader) {
PhoneMetadata metadata = map.get(key); PhoneMetadata metadata = map.get(key);
if (metadata != null) { if (metadata != null) {
return metadata; return metadata;
} }
// We assume key.toString() is well-defined. // We assume key.toString() is well-defined.
String fileName = filePrefix + "_" + key; String fileName = filePrefix + "_" + key;
List<PhoneMetadata> metadataList = getMetadataFromFileName(fileName, metadataLoader);
List<PhoneMetadata> metadataList = getMetadataFromSingleFileName(fileName, metadataLoader);
if (metadataList.size() > 1) { if (metadataList.size() > 1) {
logger.log(Level.WARNING, "more than one metadata in file " + fileName); logger.log(Level.WARNING, "more than one metadata in file " + fileName);
} }
@ -126,9 +126,10 @@ final class MetadataManager {
// Loader and holder for the metadata maps loaded from a single file. // Loader and holder for the metadata maps loaded from a single file.
static class SingleFileMetadataMaps { static class SingleFileMetadataMaps {
static SingleFileMetadataMaps load(String fileName, MetadataLoader metadataLoader) { static SingleFileMetadataMaps load(String fileName, MetadataLoader metadataLoader) {
List<PhoneMetadata> metadataList = getMetadataFromFileName(fileName, metadataLoader);
List<PhoneMetadata> metadataList = getMetadataFromSingleFileName(fileName, metadataLoader);
Map<String, PhoneMetadata> regionCodeToMetadata = new HashMap<String, PhoneMetadata>(); Map<String, PhoneMetadata> regionCodeToMetadata = new HashMap<String, PhoneMetadata>();
Map<Integer, PhoneMetadata> countryCallingCodeToMetadata = new HashMap<Integer, PhoneMetadata>();
Map<Integer, PhoneMetadata> countryCallingCodeToMetadata =
new HashMap<Integer, PhoneMetadata>();
for (PhoneMetadata metadata : metadataList) { for (PhoneMetadata metadata : metadataList) {
String regionCode = metadata.getId(); String regionCode = metadata.getId();
if (PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode)) { if (PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode)) {
@ -172,17 +173,15 @@ final class MetadataManager {
static SingleFileMetadataMaps getSingleFileMetadataMaps( static SingleFileMetadataMaps getSingleFileMetadataMaps(
AtomicReference<SingleFileMetadataMaps> ref, String fileName, MetadataLoader metadataLoader) { AtomicReference<SingleFileMetadataMaps> ref, String fileName, MetadataLoader metadataLoader) {
SingleFileMetadataMaps maps = ref.get(); SingleFileMetadataMaps maps = ref.get();
if (maps == null) {
maps = SingleFileMetadataMaps.load(fileName, metadataLoader);
SingleFileMetadataMaps existingValue = ref.getAndSet(maps);
if (existingValue != null) {
maps = existingValue;
}
if (maps != null) {
return maps;
} }
return maps;
maps = SingleFileMetadataMaps.load(fileName, metadataLoader);
ref.compareAndSet(null, maps);
return ref.get();
} }
private static List<PhoneMetadata> getMetadataFromFileName(String fileName,
private static List<PhoneMetadata> getMetadataFromSingleFileName(String fileName,
MetadataLoader metadataLoader) { MetadataLoader metadataLoader) {
InputStream source = metadataLoader.loadMetadata(fileName); InputStream source = metadataLoader.loadMetadata(fileName);
if (source == null) { if (source == null) {


+ 2
- 2
java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java View File

@ -62,7 +62,7 @@ final class MultiFileMetadataSourceImpl implements MetadataSource {
@Override @Override
public PhoneMetadata getMetadataForRegion(String regionCode) { public PhoneMetadata getMetadataForRegion(String regionCode) {
return MetadataManager.getMultiFileMetadata(regionCode, geographicalRegions,
return MetadataManager.getMetadataFromMultiFilePrefix(regionCode, geographicalRegions,
multiFilePhoneNumberMetadataFilePrefix, metadataLoader); multiFilePhoneNumberMetadataFilePrefix, metadataLoader);
} }
@ -72,7 +72,7 @@ final class MultiFileMetadataSourceImpl implements MetadataSource {
// The given country calling code was for a geographical region. // The given country calling code was for a geographical region.
return null; return null;
} }
return MetadataManager.getMultiFileMetadata(countryCallingCode, nonGeographicalRegions,
return MetadataManager.getMetadataFromMultiFilePrefix(countryCallingCode, nonGeographicalRegions,
multiFilePhoneNumberMetadataFilePrefix, metadataLoader); multiFilePhoneNumberMetadataFilePrefix, metadataLoader);
} }


+ 21
- 31
java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java View File

@ -22,7 +22,6 @@ import com.google.i18n.phonenumbers.Phonemetadata.PhoneNumberDesc;
import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber;
import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber.CountryCodeSource; import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber.CountryCodeSource;
import java.io.InputStream;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
@ -52,14 +51,6 @@ import java.util.regex.Pattern;
* @author Shaopeng Jia * @author Shaopeng Jia
*/ */
public class PhoneNumberUtil { public class PhoneNumberUtil {
// @VisibleForTesting
static final MetadataLoader DEFAULT_METADATA_LOADER = new MetadataLoader() {
@Override
public InputStream loadMetadata(String metadataFileName) {
return PhoneNumberUtil.class.getResourceAsStream(metadataFileName);
}
};
private static final Logger logger = Logger.getLogger(PhoneNumberUtil.class.getName()); private static final Logger logger = Logger.getLogger(PhoneNumberUtil.class.getName());
/** Flags to use when compiling regular expressions for phone numbers. */ /** Flags to use when compiling regular expressions for phone numbers. */
@ -958,8 +949,7 @@ public class PhoneNumberUtil {
/** /**
* Gets a {@link PhoneNumberUtil} instance to carry out international phone number formatting, * Gets a {@link PhoneNumberUtil} instance to carry out international phone number formatting,
* parsing, or validation. The instance is loaded with phone number metadata for a number of most
* commonly used regions.
* parsing, or validation. The instance is loaded with all phone number metadata.
* *
* <p>The {@link PhoneNumberUtil} is implemented as a singleton. Therefore, calling getInstance * <p>The {@link PhoneNumberUtil} is implemented as a singleton. Therefore, calling getInstance
* multiple times will only result in one instance being created. * multiple times will only result in one instance being created.
@ -968,7 +958,7 @@ public class PhoneNumberUtil {
*/ */
public static synchronized PhoneNumberUtil getInstance() { public static synchronized PhoneNumberUtil getInstance() {
if (instance == null) { if (instance == null) {
setInstance(createInstance(DEFAULT_METADATA_LOADER));
setInstance(createInstance(MetadataManager.DEFAULT_METADATA_LOADER));
} }
return instance; return instance;
} }
@ -976,40 +966,40 @@ public class PhoneNumberUtil {
/** /**
* Create a new {@link PhoneNumberUtil} instance to carry out international phone number * Create a new {@link PhoneNumberUtil} instance to carry out international phone number
* formatting, parsing, or validation. The instance is loaded with all metadata by * formatting, parsing, or validation. The instance is loaded with all metadata by
* using the metadataSource specified.
* using the metadataLoader specified.
* *
* <p>This method should only be used in the rare case in which you want to manage your own * <p>This method should only be used in the rare case in which you want to manage your own
* metadata loading. Calling this method multiple times is very expensive, as each time * metadata loading. Calling this method multiple times is very expensive, as each time
* a new instance is created from scratch. When in doubt, use {@link #getInstance}. * a new instance is created from scratch. When in doubt, use {@link #getInstance}.
* *
* @param metadataSource customized metadata source. This should not be null.
* @param metadataLoader customized metadata loader. This should not be null
* @return a PhoneNumberUtil instance * @return a PhoneNumberUtil instance
*/ */
public static PhoneNumberUtil createInstance(MetadataSource metadataSource) {
if (metadataSource == null) {
throw new IllegalArgumentException("metadataSource could not be null.");
public static PhoneNumberUtil createInstance(MetadataLoader metadataLoader) {
if (metadataLoader == null) {
throw new IllegalArgumentException("metadataLoader could not be null.");
} }
return new PhoneNumberUtil(metadataSource,
CountryCodeToRegionCodeMap.getCountryCodeToRegionCodeMap());
return createInstance(new MultiFileMetadataSourceImpl(metadataLoader));
} }
/** /**
* Create a new {@link PhoneNumberUtil} instance to carry out international phone number * Create a new {@link PhoneNumberUtil} instance to carry out international phone number
* formatting, parsing, or validation. The instance is loaded with all metadata by * formatting, parsing, or validation. The instance is loaded with all metadata by
* using the metadataLoader specified.
* using the metadataSource specified.
* *
* This method should only be used in the rare case in which you want to manage your own
* <p>This method should only be used in the rare case in which you want to manage your own
* metadata loading. Calling this method multiple times is very expensive, as each time * metadata loading. Calling this method multiple times is very expensive, as each time
* a new instance is created from scratch. When in doubt, use {@link #getInstance}. * a new instance is created from scratch. When in doubt, use {@link #getInstance}.
* *
* @param metadataLoader Customized metadata loader. This should not be null.
* @return a PhoneNumberUtil instance
* @param metadataSource customized metadata source. This should not be null
* @return a PhoneNumberUtil instance
*/ */
public static PhoneNumberUtil createInstance(MetadataLoader metadataLoader) {
if (metadataLoader == null) {
throw new IllegalArgumentException("metadataLoader could not be null.");
private static PhoneNumberUtil createInstance(MetadataSource metadataSource) {
if (metadataSource == null) {
throw new IllegalArgumentException("metadataSource could not be null.");
} }
return createInstance(new MultiFileMetadataSourceImpl(metadataLoader));
return new PhoneNumberUtil(metadataSource,
CountryCodeToRegionCodeMap.getCountryCodeToRegionCodeMap());
} }
/** /**
@ -1078,7 +1068,7 @@ public class PhoneNumberUtil {
// This is the only case where a number can be formatted as E164 without a // This is the only case where a number can be formatted as E164 without a
// leading '+' symbol (but the original number wasn't parseable anyway). // leading '+' symbol (but the original number wasn't parseable anyway).
// TODO: Consider removing the 'if' above so that unparseable // TODO: Consider removing the 'if' above so that unparseable
// strings without raw input format to the empty string instead of "+00"
// strings without raw input format to the empty string instead of "+00".
String rawInput = number.getRawInput(); String rawInput = number.getRawInput();
if (rawInput.length() > 0) { if (rawInput.length() > 0) {
return rawInput; return rawInput;
@ -1148,7 +1138,7 @@ public class PhoneNumberUtil {
// share a country calling code is contained by only one region for performance reasons. For // share a country calling code is contained by only one region for performance reasons. For
// example, for NANPA regions it will be contained in the metadata for US. // example, for NANPA regions it will be contained in the metadata for US.
String regionCode = getRegionCodeForCountryCode(countryCallingCode); String regionCode = getRegionCodeForCountryCode(countryCallingCode);
// Metadata cannot be null because the country calling code is valid
// Metadata cannot be null because the country calling code is valid.
PhoneMetadata metadata = PhoneMetadata metadata =
getMetadataForRegionOrCallingCode(countryCallingCode, regionCode); getMetadataForRegionOrCallingCode(countryCallingCode, regionCode);
@ -1306,8 +1296,8 @@ public class PhoneNumberUtil {
// dialing from a mobile phone, except for short numbers. As a result, we add it back here // dialing from a mobile phone, except for short numbers. As a result, we add it back here
// if it is a valid regular length phone number. // if it is a valid regular length phone number.
formattedNumber = formattedNumber =
getNddPrefixForRegion(regionCode, true /* strip non-digits */) +
" " + format(numberNoExt, PhoneNumberFormat.NATIONAL);
getNddPrefixForRegion(regionCode, true /* strip non-digits */) + " "
+ format(numberNoExt, PhoneNumberFormat.NATIONAL);
} else if (countryCallingCode == NANPA_COUNTRY_CODE) { } else if (countryCallingCode == NANPA_COUNTRY_CODE) {
// For NANPA countries, we output international format for numbers that can be dialed // For NANPA countries, we output international format for numbers that can be dialed
// internationally, since that always works, except for numbers which might potentially be // internationally, since that always works, except for numbers which might potentially be


+ 1
- 1
java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java View File

@ -41,7 +41,7 @@ final class SingleFileMetadataSourceImpl implements MetadataSource {
} }
// It is assumed that metadataLoader is not null. Checks should happen before passing it in here. // It is assumed that metadataLoader is not null. Checks should happen before passing it in here.
public SingleFileMetadataSourceImpl(MetadataLoader metadataLoader) {
SingleFileMetadataSourceImpl(MetadataLoader metadataLoader) {
this(MetadataManager.SINGLE_FILE_PHONE_NUMBER_METADATA_FILE_NAME, metadataLoader); this(MetadataManager.SINGLE_FILE_PHONE_NUMBER_METADATA_FILE_NAME, metadataLoader);
} }


+ 1
- 2
java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java View File

@ -37,8 +37,7 @@ import java.util.logging.Logger;
*/ */
public class ExampleNumbersTest extends TestCase { public class ExampleNumbersTest extends TestCase {
private static final Logger logger = Logger.getLogger(ExampleNumbersTest.class.getName()); private static final Logger logger = Logger.getLogger(ExampleNumbersTest.class.getName());
private PhoneNumberUtil phoneNumberUtil =
PhoneNumberUtil.createInstance(PhoneNumberUtil.DEFAULT_METADATA_LOADER);
private PhoneNumberUtil phoneNumberUtil = PhoneNumberUtil.getInstance();
private ShortNumberInfo shortNumberInfo = ShortNumberInfo.getInstance(); private ShortNumberInfo shortNumberInfo = ShortNumberInfo.getInstance();
private List<PhoneNumber> invalidCases = new ArrayList<PhoneNumber>(); private List<PhoneNumber> invalidCases = new ArrayList<PhoneNumber>();
private List<PhoneNumber> wrongTypeCases = new ArrayList<PhoneNumber>(); private List<PhoneNumber> wrongTypeCases = new ArrayList<PhoneNumber>();


+ 14
- 12
java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java View File

@ -23,7 +23,7 @@ import junit.framework.TestCase;
/** /**
* Some basic tests to check that metadata can be correctly loaded. * Some basic tests to check that metadata can be correctly loaded.
*/ */
public final class MetadataManagerTest extends TestCase {
public class MetadataManagerTest extends TestCase {
public void testAlternateFormatsLoadCorrectly() { public void testAlternateFormatsLoadCorrectly() {
// We should have some data for Germany. // We should have some data for Germany.
PhoneMetadata germanyMetadata = MetadataManager.getAlternateFormatsForCountry(49); PhoneMetadata germanyMetadata = MetadataManager.getAlternateFormatsForCountry(49);
@ -48,36 +48,38 @@ public final class MetadataManagerTest extends TestCase {
assertNull(noShortNumberMetadata); assertNull(noShortNumberMetadata);
} }
public void testGetMultiFileMetadata_regionCode() {
public void testGetMetadataFromMultiFilePrefix_regionCode() {
ConcurrentHashMap<String, PhoneMetadata> map = new ConcurrentHashMap<String, PhoneMetadata>(); ConcurrentHashMap<String, PhoneMetadata> map = new ConcurrentHashMap<String, PhoneMetadata>();
PhoneMetadata metadata = MetadataManager.getMultiFileMetadata("CA", map,
PhoneMetadata metadata = MetadataManager.getMetadataFromMultiFilePrefix("CA", map,
"/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting",
PhoneNumberUtil.DEFAULT_METADATA_LOADER);
MetadataManager.DEFAULT_METADATA_LOADER);
assertEquals(metadata, map.get("CA")); assertEquals(metadata, map.get("CA"));
} }
public void testGetMultiFileMetadata_countryCallingCode() {
public void testGetMetadataFromMultiFilePrefix_countryCallingCode() {
ConcurrentHashMap<Integer, PhoneMetadata> map = new ConcurrentHashMap<Integer, PhoneMetadata>(); ConcurrentHashMap<Integer, PhoneMetadata> map = new ConcurrentHashMap<Integer, PhoneMetadata>();
PhoneMetadata metadata = MetadataManager.getMultiFileMetadata(800, map,
PhoneMetadata metadata = MetadataManager.getMetadataFromMultiFilePrefix(800, map,
"/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting",
PhoneNumberUtil.DEFAULT_METADATA_LOADER);
MetadataManager.DEFAULT_METADATA_LOADER);
assertEquals(metadata, map.get(800)); assertEquals(metadata, map.get(800));
} }
public void testGetMultiFileMetadata_missingMetadataFileThrowsRuntimeException() {
public void testGetMetadataFromMultiFilePrefix_missingMetadataFileThrowsRuntimeException() {
// In normal usage we should never get a state where we are asking to load metadata that doesn't // In normal usage we should never get a state where we are asking to load metadata that doesn't
// exist. However if the library is packaged incorrectly in the jar, this could happen and the // exist. However if the library is packaged incorrectly in the jar, this could happen and the
// best we can do is make sure the exception has the file name in it. // best we can do is make sure the exception has the file name in it.
try { try {
MetadataManager.getMultiFileMetadata("XX", new ConcurrentHashMap<String, PhoneMetadata>(),
"no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER);
MetadataManager.getMetadataFromMultiFilePrefix("XX",
new ConcurrentHashMap<String, PhoneMetadata>(), "no/such/file",
MetadataManager.DEFAULT_METADATA_LOADER);
fail("expected exception"); fail("expected exception");
} catch (RuntimeException e) { } catch (RuntimeException e) {
assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_XX")); assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_XX"));
} }
try { try {
MetadataManager.getMultiFileMetadata(123, new ConcurrentHashMap<Integer, PhoneMetadata>(),
"no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER);
MetadataManager.getMetadataFromMultiFilePrefix(123,
new ConcurrentHashMap<Integer, PhoneMetadata>(), "no/such/file",
MetadataManager.DEFAULT_METADATA_LOADER);
fail("expected exception"); fail("expected exception");
} catch (RuntimeException e) { } catch (RuntimeException e) {
assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_123")); assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_123"));


+ 8
- 8
java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java View File

@ -24,21 +24,21 @@ import junit.framework.TestCase;
* Unit tests for MultiFileMetadataSourceImpl.java. * Unit tests for MultiFileMetadataSourceImpl.java.
*/ */
public class MultiFileMetadataSourceImplTest extends TestCase { public class MultiFileMetadataSourceImplTest extends TestCase {
private static final MultiFileMetadataSourceImpl source =
new MultiFileMetadataSourceImpl(PhoneNumberUtil.DEFAULT_METADATA_LOADER);
private static final MultiFileMetadataSourceImpl missingFileSource =
new MultiFileMetadataSourceImpl("no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER);
private static final MultiFileMetadataSourceImpl SOURCE =
new MultiFileMetadataSourceImpl(MetadataManager.DEFAULT_METADATA_LOADER);
private static final MultiFileMetadataSourceImpl MISSING_FILE_SOURCE =
new MultiFileMetadataSourceImpl("no/such/file", MetadataManager.DEFAULT_METADATA_LOADER);
public void testGeoPhoneNumberMetadataLoadCorrectly() { public void testGeoPhoneNumberMetadataLoadCorrectly() {
// We should have some data for the UAE. // We should have some data for the UAE.
PhoneMetadata uaeMetadata = source.getMetadataForRegion("AE");
PhoneMetadata uaeMetadata = SOURCE.getMetadataForRegion("AE");
assertEquals(uaeMetadata.getCountryCode(), 971); assertEquals(uaeMetadata.getCountryCode(), 971);
assertTrue(uaeMetadata.hasGeneralDesc()); assertTrue(uaeMetadata.hasGeneralDesc());
} }
public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception {
try { try {
missingFileSource.getMetadataForRegion("AE");
MISSING_FILE_SOURCE.getMetadataForRegion("AE");
fail("expected exception"); fail("expected exception");
} catch (RuntimeException e) { } catch (RuntimeException e) {
assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file"));
@ -47,14 +47,14 @@ public class MultiFileMetadataSourceImplTest extends TestCase {
public void testNonGeoPhoneNumberMetadataLoadCorrectly() { public void testNonGeoPhoneNumberMetadataLoadCorrectly() {
// We should have some data for international toll-free numbers. // We should have some data for international toll-free numbers.
PhoneMetadata intlMetadata = source.getMetadataForNonGeographicalRegion(800);
PhoneMetadata intlMetadata = SOURCE.getMetadataForNonGeographicalRegion(800);
assertEquals(intlMetadata.getId(), "001"); assertEquals(intlMetadata.getId(), "001");
assertTrue(intlMetadata.hasGeneralDesc()); assertTrue(intlMetadata.hasGeneralDesc());
} }
public void testNonGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { public void testNonGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception {
try { try {
missingFileSource.getMetadataForNonGeographicalRegion(800);
MISSING_FILE_SOURCE.getMetadataForNonGeographicalRegion(800);
fail("expected exception"); fail("expected exception");
} catch (RuntimeException e) { } catch (RuntimeException e) {
assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file"));


+ 1
- 1
java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java View File

@ -26,7 +26,7 @@ import junit.framework.TestCase;
*/ */
public class SingleFileMetadataSourceImplTest extends TestCase { public class SingleFileMetadataSourceImplTest extends TestCase {
private static final SingleFileMetadataSourceImpl MISSING_FILE_SOURCE = private static final SingleFileMetadataSourceImpl MISSING_FILE_SOURCE =
new SingleFileMetadataSourceImpl("no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER);
new SingleFileMetadataSourceImpl("no/such/file", MetadataManager.DEFAULT_METADATA_LOADER);
public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception {
try { try {


+ 20
- 8
java/libphonenumber/test/com/google/i18n/phonenumbers/TestMetadataTestCase.java View File

@ -23,25 +23,37 @@ import junit.framework.TestCase;
* <p> * <p>
* Note since tests that extend this class do not use the normal metadata file, they should not be * Note since tests that extend this class do not use the normal metadata file, they should not be
* used for regression test purposes. * used for regression test purposes.
* <p>
* Ideally the {@code phoneUtil} field (which uses test metadata) would be the only way that tests
* need to interact with a PhoneNumberUtil instance. However as some static methods in the library
* invoke "getInstance()" internally, we must also inject the test instance as the PhoneNumberUtil
* singleton. This means it is unsafe to run tests derived from this class in parallel with each
* other or at the same time as other tests which might require the singleton instance.
* *
* @author Shaopeng Jia * @author Shaopeng Jia
*/ */
public class TestMetadataTestCase extends TestCase { public class TestMetadataTestCase extends TestCase {
private static final String TEST_META_DATA_FILE_PREFIX =
private static final String TEST_METADATA_FILE_PREFIX =
"/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting"; "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting";
/** An instance of PhoneNumberUtil that uses test metadata. */
protected final PhoneNumberUtil phoneUtil; protected final PhoneNumberUtil phoneUtil;
public TestMetadataTestCase() { public TestMetadataTestCase() {
phoneUtil = initializePhoneUtilForTesting();
phoneUtil = new PhoneNumberUtil(new MultiFileMetadataSourceImpl(TEST_METADATA_FILE_PREFIX,
MetadataManager.DEFAULT_METADATA_LOADER),
CountryCodeToRegionCodeMapForTesting.getCountryCodeToRegionCodeMap());
} }
static PhoneNumberUtil initializePhoneUtilForTesting() {
PhoneNumberUtil phoneUtil = new PhoneNumberUtil(
new MultiFileMetadataSourceImpl(TEST_META_DATA_FILE_PREFIX,
PhoneNumberUtil.DEFAULT_METADATA_LOADER),
CountryCodeToRegionCodeMapForTesting.getCountryCodeToRegionCodeMap());
@Override
protected void setUp() throws Exception {
super.setUp();
PhoneNumberUtil.setInstance(phoneUtil); PhoneNumberUtil.setInstance(phoneUtil);
return phoneUtil;
}
@Override
protected void tearDown() throws Exception {
PhoneNumberUtil.setInstance(null);
super.tearDown();
} }
} }

+ 4
- 0
java/pending_code_changes.txt View File

@ -5,3 +5,7 @@ Code changes:
- Refactored metadata loading and removed synchronization for all kinds of - Refactored metadata loading and removed synchronization for all kinds of
metadata. Clients may experience quicker loading of alternate formats and metadata. Clients may experience quicker loading of alternate formats and
short number metadata, but no change is required for callers of the library. short number metadata, but no change is required for callers of the library.
- Reduced visibility of `public` internal API
`PhoneNumberUtil.createInstance(MetadataSource)` to `private`. MetadataSource
and all its implementations are non-public so this should not affect public
usage of the library.

+ 2
- 4
tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java View File

@ -140,8 +140,7 @@ public class MetadataFilterTest extends TestCase {
public void testParseFieldMapFromString_mixOfGroups() { public void testParseFieldMapFromString_mixOfGroups() {
TreeMap<String, TreeSet<String>> fieldMap = new TreeMap<String, TreeSet<String>>(); TreeMap<String, TreeSet<String>> fieldMap = new TreeMap<String, TreeSet<String>>();
fieldMap.put("uan", new TreeSet<String>(Arrays.asList( fieldMap.put("uan", new TreeSet<String>(Arrays.asList(
"possibleLength", "exampleNumber", "possibleLengthLocalOnly",
"nationalNumberPattern")));
"possibleLength", "exampleNumber", "possibleLengthLocalOnly", "nationalNumberPattern")));
fieldMap.put("pager", new TreeSet<String>(Arrays.asList( fieldMap.put("pager", new TreeSet<String>(Arrays.asList(
"exampleNumber", "nationalNumberPattern"))); "exampleNumber", "nationalNumberPattern")));
fieldMap.put("fixedLine", new TreeSet<String>(Arrays.asList( fieldMap.put("fixedLine", new TreeSet<String>(Arrays.asList(
@ -223,8 +222,7 @@ public class MetadataFilterTest extends TestCase {
// All parent's children covered, some implicitly and some explicitly. // All parent's children covered, some implicitly and some explicitly.
assertEquals( assertEquals(
MetadataFilter.parseFieldMapFromString( MetadataFilter.parseFieldMapFromString(
"uan(nationalNumberPattern,possibleLength,exampleNumber)"
+ ":possibleLengthLocalOnly"),
"uan(nationalNumberPattern,possibleLength,exampleNumber):possibleLengthLocalOnly"),
MetadataFilter.parseFieldMapFromString("uan:possibleLengthLocalOnly")); MetadataFilter.parseFieldMapFromString("uan:possibleLengthLocalOnly"));
// Child field covered by all parents explicitly. // Child field covered by all parents explicitly.


BIN
tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar View File


BIN
tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar View File


Loading…
Cancel
Save