diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java index 071278d7b..7fe3da60a 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java @@ -24,48 +24,187 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; /** - * Class encapsulating loading of PhoneNumber Metadata information. Currently this is used only for - * additional data files such as PhoneNumberAlternateFormats, but in the future it is envisaged it - * would handle the main metadata file (PhoneNumberMetadata.xml) as well. + * Manager for loading metadata for alternate formats and short numbers. We also declare some + * constants for phone number metadata loading, to more easily maintain all three types of metadata + * together. + * TODO: Consider managing phone number metadata loading here too. */ final class MetadataManager { + static final String MULTI_FILE_PHONE_NUMBER_METADATA_FILE_PREFIX = + "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProto"; + static final String SINGLE_FILE_PHONE_NUMBER_METADATA_FILE_NAME = + "/com/google/i18n/phonenumbers/data/SingleFilePhoneNumberMetadataProto"; private static final String ALTERNATE_FORMATS_FILE_PREFIX = "/com/google/i18n/phonenumbers/data/PhoneNumberAlternateFormatsProto"; private static final String SHORT_NUMBER_METADATA_FILE_PREFIX = "/com/google/i18n/phonenumbers/data/ShortNumberMetadataProto"; + private static final MetadataLoader METADATA_LOADER = new MetadataLoader() { + @Override + public InputStream loadMetadata(String metadataFileName) { + return MetadataManager.class.getResourceAsStream(metadataFileName); + } + }; + private static final Logger logger = Logger.getLogger(MetadataManager.class.getName()); - private static final Map callingCodeToAlternateFormatsMap = - Collections.synchronizedMap(new HashMap()); - private static final Map regionCodeToShortNumberMetadataMap = - Collections.synchronizedMap(new HashMap()); + // A mapping from a country calling code to the alternate formats for that country calling code. + private static final ConcurrentHashMap alternateFormatsMap = + new ConcurrentHashMap(); + + // A mapping from a region code to the short number metadata for that region code. + private static final ConcurrentHashMap shortNumberMetadataMap = + new ConcurrentHashMap(); - // A set of which country calling codes there are alternate format data for. If the set has an - // entry for a code, then there should be data for that code linked into the resources. - private static final Set countryCodeSet = + // The set of country calling codes for which there are alternate formats. For every country + // calling code in this set there should be metadata linked into the resources. + private static final Set alternateFormatsCountryCodes = AlternateFormatsCountryCodeSet.getCountryCodeSet(); - // A set of which region codes there are short number data for. If the set has an entry for a - // code, then there should be data for that code linked into the resources. - private static final Set regionCodeSet = ShortNumbersRegionCodeSet.getRegionCodeSet(); + // The set of region codes for which there are short number metadata. For every region code in + // this set there should be metadata linked into the resources. + private static final Set shortNumberMetadataRegionCodes = + ShortNumbersRegionCodeSet.getRegionCodeSet(); + + private MetadataManager() {} + + static PhoneMetadata getAlternateFormatsForCountry(int countryCallingCode) { + if (!alternateFormatsCountryCodes.contains(countryCallingCode)) { + return null; + } + return getMultiFileMetadata(countryCallingCode, alternateFormatsMap, + ALTERNATE_FORMATS_FILE_PREFIX, METADATA_LOADER); + } + + static PhoneMetadata getShortNumberMetadataForRegion(String regionCode) { + if (!shortNumberMetadataRegionCodes.contains(regionCode)) { + return null; + } + return getMultiFileMetadata(regionCode, shortNumberMetadataMap, + SHORT_NUMBER_METADATA_FILE_PREFIX, METADATA_LOADER); + } + + static Set getSupportedShortNumberRegions() { + return Collections.unmodifiableSet(shortNumberMetadataRegionCodes); + } + + /** + * @param key the lookup key for the provided map, typically a region code or a country calling + * code + * @param map the map containing mappings of already loaded metadata from their {@code key}. If + * this {@code key}'s metadata isn't already loaded, it will be added to this map after + * loading + * @param filePrefix the prefix of the file to load metadata from + * @param metadataLoader the metadata loader used to inject alternative metadata sources + */ + static PhoneMetadata getMultiFileMetadata(T key, ConcurrentHashMap map, + String filePrefix, MetadataLoader metadataLoader) { + PhoneMetadata metadata = map.get(key); + if (metadata != null) { + return metadata; + } + // We assume key.toString() is well-defined. + String fileName = filePrefix + "_" + key; + List metadataList = getMetadataFromFileName(fileName, metadataLoader); + if (metadataList.size() > 1) { + logger.log(Level.WARNING, "more than one metadata in file " + fileName); + } + metadata = metadataList.get(0); + PhoneMetadata oldValue = map.putIfAbsent(key, metadata); + return (oldValue != null) ? oldValue : metadata; + } + + // Loader and holder for the metadata maps loaded from a single file. + static class SingleFileMetadataMaps { + static SingleFileMetadataMaps load(String fileName, MetadataLoader metadataLoader) { + List metadataList = getMetadataFromFileName(fileName, metadataLoader); + Map regionCodeToMetadata = new HashMap(); + Map countryCallingCodeToMetadata = new HashMap(); + for (PhoneMetadata metadata : metadataList) { + String regionCode = metadata.getId(); + if (PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode)) { + // regionCode belongs to a non-geographical entity. + countryCallingCodeToMetadata.put(metadata.getCountryCode(), metadata); + } else { + regionCodeToMetadata.put(regionCode, metadata); + } + } + return new SingleFileMetadataMaps(regionCodeToMetadata, countryCallingCodeToMetadata); + } + + // A map from a region code to the PhoneMetadata for that region. + // For phone number metadata, the region code "001" is excluded, since that is used for the + // non-geographical phone number entities. + private final Map regionCodeToMetadata; + + // A map from a country calling code to the PhoneMetadata for that country calling code. + // Examples of the country calling codes include 800 (International Toll Free Service) and 808 + // (International Shared Cost Service). + // For phone number metadata, only the non-geographical phone number entities' country calling + // codes are present. + private final Map countryCallingCodeToMetadata; + + private SingleFileMetadataMaps(Map regionCodeToMetadata, + Map countryCallingCodeToMetadata) { + this.regionCodeToMetadata = Collections.unmodifiableMap(regionCodeToMetadata); + this.countryCallingCodeToMetadata = Collections.unmodifiableMap(countryCallingCodeToMetadata); + } + + PhoneMetadata get(String regionCode) { + return regionCodeToMetadata.get(regionCode); + } + + PhoneMetadata get(int countryCallingCode) { + return countryCallingCodeToMetadata.get(countryCallingCode); + } + } + + // Manages the atomic reference lifecycle of a SingleFileMetadataMaps encapsulation. + static SingleFileMetadataMaps getSingleFileMetadataMaps( + AtomicReference ref, String fileName, MetadataLoader metadataLoader) { + SingleFileMetadataMaps maps = ref.get(); + if (maps == null) { + maps = SingleFileMetadataMaps.load(fileName, metadataLoader); + SingleFileMetadataMaps existingValue = ref.getAndSet(maps); + if (existingValue != null) { + maps = existingValue; + } + } + return maps; + } - private MetadataManager() { + private static List getMetadataFromFileName(String fileName, + MetadataLoader metadataLoader) { + InputStream source = metadataLoader.loadMetadata(fileName); + if (source == null) { + // Sanity check; this would only happen if we packaged jars incorrectly. + throw new IllegalStateException("missing metadata: " + fileName); + } + PhoneMetadataCollection metadataCollection = loadMetadataAndCloseInput(source); + List metadataList = metadataCollection.getMetadataList(); + if (metadataList.size() == 0) { + // Sanity check; this should not happen since we build with non-empty metadata. + throw new IllegalStateException("empty metadata: " + fileName); + } + return metadataList; } /** - * Loads and returns the metadata object from the given stream and closes the stream. + * Loads and returns the metadata from the given stream and closes the stream. * * @param source the non-null stream from which metadata is to be read - * @return the loaded metadata object + * @return the loaded metadata */ - static PhoneMetadataCollection loadMetadataAndCloseInput(InputStream source) { + private static PhoneMetadataCollection loadMetadataAndCloseInput(InputStream source) { ObjectInputStream ois = null; try { try { @@ -93,61 +232,4 @@ final class MetadataManager { } } } - - private static void loadAlternateFormatsMetadataFromFile(int countryCallingCode) { - String fileName = ALTERNATE_FORMATS_FILE_PREFIX + "_" + countryCallingCode; - InputStream source = MetadataManager.class.getResourceAsStream(fileName); - if (source == null) { - // Sanity check; this should not happen since we only load things based on the expectation - // that they are present, by checking the map of available data first. - throw new IllegalStateException("missing metadata: " + fileName); - } - PhoneMetadataCollection alternateFormats = loadMetadataAndCloseInput(source); - for (PhoneMetadata metadata : alternateFormats.getMetadataList()) { - callingCodeToAlternateFormatsMap.put(metadata.getCountryCode(), metadata); - } - } - - static PhoneMetadata getAlternateFormatsForCountry(int countryCallingCode) { - if (!countryCodeSet.contains(countryCallingCode)) { - return null; - } - synchronized (callingCodeToAlternateFormatsMap) { - if (!callingCodeToAlternateFormatsMap.containsKey(countryCallingCode)) { - loadAlternateFormatsMetadataFromFile(countryCallingCode); - } - } - return callingCodeToAlternateFormatsMap.get(countryCallingCode); - } - - private static void loadShortNumberMetadataFromFile(String regionCode) { - String fileName = SHORT_NUMBER_METADATA_FILE_PREFIX + "_" + regionCode; - InputStream source = MetadataManager.class.getResourceAsStream(fileName); - if (source == null) { - // Sanity check; this should not happen since we only load things based on the expectation - // that they are present, by checking the map of available data first. - throw new IllegalStateException("missing metadata: " + fileName); - } - PhoneMetadataCollection shortNumberMetadata = loadMetadataAndCloseInput(source); - for (PhoneMetadata metadata : shortNumberMetadata.getMetadataList()) { - regionCodeToShortNumberMetadataMap.put(regionCode, metadata); - } - } - - // @VisibleForTesting - static Set getShortNumberMetadataSupportedRegions() { - return regionCodeSet; - } - - static PhoneMetadata getShortNumberMetadataForRegion(String regionCode) { - if (!regionCodeSet.contains(regionCode)) { - return null; - } - synchronized (regionCodeToShortNumberMetadataMap) { - if (!regionCodeToShortNumberMetadataMap.containsKey(regionCode)) { - loadShortNumberMetadataFromFile(regionCode); - } - } - return regionCodeToShortNumberMetadataMap.get(regionCode); - } } diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java index 36cd4f094..3b0dcba36 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java @@ -17,74 +17,63 @@ package com.google.i18n.phonenumbers; import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata; -import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadataCollection; - -import java.io.InputStream; import java.util.List; import java.util.concurrent.ConcurrentHashMap; -import java.util.logging.Level; -import java.util.logging.Logger; /** * Implementation of {@link MetadataSource} that reads from multiple resource files. */ final class MultiFileMetadataSourceImpl implements MetadataSource { + // The prefix of the binary files containing phone number metadata for different regions. + // This enables us to set up with different metadata, such as for testing. + private final String multiFilePhoneNumberMetadataFilePrefix; - private static final Logger logger = - Logger.getLogger(MultiFileMetadataSourceImpl.class.getName()); - - private static final String META_DATA_FILE_PREFIX = - "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProto"; + // The {@link MetadataLoader} used to inject alternative metadata sources. + private final MetadataLoader metadataLoader; - // A mapping from a region code to the PhoneMetadata for that region. + // A mapping from a region code to the phone number metadata for that region code. + // Unlike the mappings for alternate formats and short number metadata, the phone number metadata + // is loaded from a non-statically determined file prefix; therefore this map is bound to the + // instance and not static. private final ConcurrentHashMap geographicalRegions = new ConcurrentHashMap(); - // A mapping from a country calling code for a non-geographical entity to the PhoneMetadata for - // that country calling code. Examples of the country calling codes include 800 (International - // Toll Free Service) and 808 (International Shared Cost Service). + // A mapping from a country calling code for a non-geographical entity to the phone number + // metadata for that country calling code. Examples of the country calling codes include 800 + // (International Toll Free Service) and 808 (International Shared Cost Service). + // Unlike the mappings for alternate formats and short number metadata, the phone number metadata + // is loaded from a non-statically determined file prefix; therefore this map is bound to the + // instance and not static. private final ConcurrentHashMap nonGeographicalRegions = new ConcurrentHashMap(); - // The prefix of the metadata files from which region data is loaded. - private final String filePrefix; - - // The metadata loader used to inject alternative metadata sources. - private final MetadataLoader metadataLoader; - - // It is assumed that metadataLoader is not null. If needed, checks should happen before passing - // here. + // It is assumed that metadataLoader is not null. Checks should happen before passing here. // @VisibleForTesting - MultiFileMetadataSourceImpl(String filePrefix, MetadataLoader metadataLoader) { - this.filePrefix = filePrefix; + MultiFileMetadataSourceImpl(String multiFilePhoneNumberMetadataFilePrefix, + MetadataLoader metadataLoader) { + this.multiFilePhoneNumberMetadataFilePrefix = multiFilePhoneNumberMetadataFilePrefix; this.metadataLoader = metadataLoader; } - // It is assumed that metadataLoader is not null. If needed, checks should happen before passing - // here. + // It is assumed that metadataLoader is not null. Checks should happen before passing here. public MultiFileMetadataSourceImpl(MetadataLoader metadataLoader) { - this(META_DATA_FILE_PREFIX, metadataLoader); + this(MetadataManager.MULTI_FILE_PHONE_NUMBER_METADATA_FILE_PREFIX, metadataLoader); } @Override public PhoneMetadata getMetadataForRegion(String regionCode) { - PhoneMetadata metadata = geographicalRegions.get(regionCode); - return (metadata != null) ? metadata : loadMetadataFromFile( - regionCode, geographicalRegions, filePrefix, metadataLoader); + return MetadataManager.getMultiFileMetadata(regionCode, geographicalRegions, + multiFilePhoneNumberMetadataFilePrefix, metadataLoader); } @Override public PhoneMetadata getMetadataForNonGeographicalRegion(int countryCallingCode) { - PhoneMetadata metadata = nonGeographicalRegions.get(countryCallingCode); - if (metadata != null) { - return metadata; - } - if (isNonGeographical(countryCallingCode)) { - return loadMetadataFromFile( - countryCallingCode, nonGeographicalRegions, filePrefix, metadataLoader); + if (!isNonGeographical(countryCallingCode)) { + // The given country calling code was for a geographical region. + return null; } - // The given country calling code was for a geographical region. - return null; + return MetadataManager.getMultiFileMetadata(countryCallingCode, nonGeographicalRegions, + multiFilePhoneNumberMetadataFilePrefix, metadataLoader); } // A country calling code is non-geographical if it only maps to the non-geographical region code, @@ -95,38 +84,4 @@ final class MultiFileMetadataSourceImpl implements MetadataSource { return (regionCodes.size() == 1 && PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCodes.get(0))); } - - /** - * @param key The geographical region code or the non-geographical region's country - * calling code. - * @param map The map to contain the mapping from {@code key} to the corresponding - * metadata. - * @param filePrefix The prefix of the metadata files from which region data is loaded. - * @param metadataLoader The metadata loader used to inject alternative metadata sources. - */ - // @VisibleForTesting - static PhoneMetadata loadMetadataFromFile( - T key, ConcurrentHashMap map, String filePrefix, - MetadataLoader metadataLoader) { - // We assume key.toString() is well-defined. - String fileName = filePrefix + "_" + key; - InputStream source = metadataLoader.loadMetadata(fileName); - if (source == null) { - // Sanity check; this should not happen since we only load things based on the expectation - // that they are present, by checking the map of available data first. - throw new IllegalStateException("missing metadata: " + fileName); - } - PhoneMetadataCollection metadataCollection = MetadataManager.loadMetadataAndCloseInput(source); - List metadataList = metadataCollection.getMetadataList(); - if (metadataList.isEmpty()) { - // Sanity check; this should not happen since we build with non-empty metadata. - throw new IllegalStateException("empty metadata: " + fileName); - } - if (metadataList.size() > 1) { - logger.log(Level.WARNING, "invalid metadata (too many entries): " + fileName); - } - PhoneMetadata metadata = metadataList.get(0); - PhoneMetadata oldValue = map.putIfAbsent(key, metadata); - return (oldValue != null) ? oldValue : metadata; - } } diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java index 7c2bbd6c6..64c15341d 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java @@ -454,7 +454,7 @@ public class ShortNumberInfo { * Convenience method to get a list of what regions the library has metadata for. */ Set getSupportedRegions() { - return Collections.unmodifiableSet(MetadataManager.getShortNumberMetadataSupportedRegions()); + return MetadataManager.getSupportedShortNumberRegions(); } /** diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java index 9062ef448..1fecac2b7 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java @@ -17,105 +17,49 @@ package com.google.i18n.phonenumbers; import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata; -import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadataCollection; - -import java.io.InputStream; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.logging.Logger; +import java.util.concurrent.atomic.AtomicReference; /** * Implementation of {@link MetadataSource} that reads from a single resource file. */ final class SingleFileMetadataSourceImpl implements MetadataSource { + // The name of the binary file containing phone number metadata for different regions. + // This enables us to set up with different metadata, such as for testing. + private final String phoneNumberMetadataFileName; - private static final Logger logger = - Logger.getLogger(SingleFileMetadataSourceImpl.class.getName()); - - private static final String META_DATA_FILE_NAME = - "/com/google/i18n/phonenumbers/data/SingleFilePhoneNumberMetadataProto"; - - // A mapping from a region code to the PhoneMetadata for that region. - // Note: Synchronization, though only needed for the Android version of the library, is used in - // all versions for consistency. - private final Map regionToMetadataMap = - Collections.synchronizedMap(new HashMap()); - - // A mapping from a country calling code for a non-geographical entity to the PhoneMetadata for - // that country calling code. Examples of the country calling codes include 800 (International - // Toll Free Service) and 808 (International Shared Cost Service). - // Note: Synchronization, though only needed for the Android version of the library, is used in - // all versions for consistency. - private final Map countryCodeToNonGeographicalMetadataMap = - Collections.synchronizedMap(new HashMap()); - - // The metadata file from which region data is loaded. - private final String fileName; - - // The metadata loader used to inject alternative metadata sources. + // The {@link MetadataLoader} used to inject alternative metadata sources. private final MetadataLoader metadataLoader; - // It is assumed that metadataLoader is not null. - public SingleFileMetadataSourceImpl(String fileName, MetadataLoader metadataLoader) { - this.fileName = fileName; + private final AtomicReference phoneNumberMetadataRef = + new AtomicReference(); + + // It is assumed that metadataLoader is not null. Checks should happen before passing it in here. + // @VisibleForTesting + SingleFileMetadataSourceImpl(String phoneNumberMetadataFileName, MetadataLoader metadataLoader) { + this.phoneNumberMetadataFileName = phoneNumberMetadataFileName; this.metadataLoader = metadataLoader; } - // It is assumed that metadataLoader is not null. + // It is assumed that metadataLoader is not null. Checks should happen before passing it in here. public SingleFileMetadataSourceImpl(MetadataLoader metadataLoader) { - this(META_DATA_FILE_NAME, metadataLoader); + this(MetadataManager.SINGLE_FILE_PHONE_NUMBER_METADATA_FILE_NAME, metadataLoader); } @Override public PhoneMetadata getMetadataForRegion(String regionCode) { - synchronized (regionToMetadataMap) { - if (!regionToMetadataMap.containsKey(regionCode)) { - // The regionCode here will be valid and won't be '001', so we don't need to worry about - // what to pass in for the country calling code. - loadMetadataFromFile(); - } - } - return regionToMetadataMap.get(regionCode); + return MetadataManager.getSingleFileMetadataMaps(phoneNumberMetadataRef, + phoneNumberMetadataFileName, metadataLoader).get(regionCode); } @Override public PhoneMetadata getMetadataForNonGeographicalRegion(int countryCallingCode) { - synchronized (countryCodeToNonGeographicalMetadataMap) { - if (!countryCodeToNonGeographicalMetadataMap.containsKey(countryCallingCode)) { - loadMetadataFromFile(); - } - } - return countryCodeToNonGeographicalMetadataMap.get(countryCallingCode); - } - - // @VisibleForTesting - void loadMetadataFromFile() { - InputStream source = metadataLoader.loadMetadata(fileName); - if (source == null) { - // This should not happen since clients shouldn't be using this implementation directly. - // The single file implementation is experimental, only for when the jars contain a single - // file with all regions' metadata. Currently we do not release such jars. - // TODO(b/30807096): Get the MetadataManager to decide whether to use this or the multi file - // loading depending on what data is available in the jar. - throw new IllegalStateException("missing metadata: " + fileName); - } - PhoneMetadataCollection metadataCollection = MetadataManager.loadMetadataAndCloseInput(source); - List metadataList = metadataCollection.getMetadataList(); - if (metadataList.isEmpty()) { - // This should not happen since clients shouldn't be using this implementation! - throw new IllegalStateException("empty metadata: " + fileName); - } - for (PhoneMetadata metadata : metadataList) { - String regionCode = metadata.getId(); - int countryCallingCode = metadata.getCountryCode(); - boolean isNonGeoRegion = PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode); - if (isNonGeoRegion) { - countryCodeToNonGeographicalMetadataMap.put(countryCallingCode, metadata); - } else { - regionToMetadataMap.put(regionCode, metadata); - } - } + // A country calling code is non-geographical if it only maps to the non-geographical region + // code, i.e. "001". If this is not true of the given country calling code, then we will return + // null here. If not for the atomic reference, such as if we were loading in multiple stages, we + // would check that the passed in country calling code was indeed non-geographical to avoid + // loading costs for a null result. Here though we do not check this since the entire data must + // be loaded anyway if any of it is needed at some point in the life cycle of this class. + return MetadataManager.getSingleFileMetadataMaps(phoneNumberMetadataRef, + phoneNumberMetadataFileName, metadataLoader).get(countryCallingCode); } } diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java index 9955d90a2..d632b63a4 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/MetadataManagerTest.java @@ -17,26 +17,18 @@ package com.google.i18n.phonenumbers; import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata; - +import java.util.concurrent.ConcurrentHashMap; import junit.framework.TestCase; /** - * Some basic tests to check that the phone number metadata can be correctly loaded. + * Some basic tests to check that metadata can be correctly loaded. */ -public class MetadataManagerTest extends TestCase { - - public void testAlternateFormatsContainsData() throws Exception { +public final class MetadataManagerTest extends TestCase { + public void testAlternateFormatsLoadCorrectly() { // We should have some data for Germany. - PhoneMetadata germanyAlternateFormats = MetadataManager.getAlternateFormatsForCountry(49); - assertNotNull(germanyAlternateFormats); - assertTrue(germanyAlternateFormats.numberFormatSize() > 0); - } - - public void testShortNumberMetadataContainsData() throws Exception { - // We should have some data for France. - PhoneMetadata franceShortNumberMetadata = MetadataManager.getShortNumberMetadataForRegion("FR"); - assertNotNull(franceShortNumberMetadata); - assertTrue(franceShortNumberMetadata.hasShortCode()); + PhoneMetadata germanyMetadata = MetadataManager.getAlternateFormatsForCountry(49); + assertNotNull(germanyMetadata); + assertTrue(germanyMetadata.numberFormats().size() > 0); } public void testAlternateFormatsFailsGracefully() throws Exception { @@ -44,8 +36,51 @@ public class MetadataManagerTest extends TestCase { assertNull(noAlternateFormats); } + public void testShortNumberMetadataLoadCorrectly() throws Exception { + // We should have some data for France. + PhoneMetadata franceMetadata = MetadataManager.getShortNumberMetadataForRegion("FR"); + assertNotNull(franceMetadata); + assertTrue(franceMetadata.hasShortCode()); + } + public void testShortNumberMetadataFailsGracefully() throws Exception { PhoneMetadata noShortNumberMetadata = MetadataManager.getShortNumberMetadataForRegion("XXX"); assertNull(noShortNumberMetadata); } + + public void testGetMultiFileMetadata_regionCode() { + ConcurrentHashMap map = new ConcurrentHashMap(); + PhoneMetadata metadata = MetadataManager.getMultiFileMetadata("CA", map, + "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", + PhoneNumberUtil.DEFAULT_METADATA_LOADER); + assertEquals(metadata, map.get("CA")); + } + + public void testGetMultiFileMetadata_countryCallingCode() { + ConcurrentHashMap map = new ConcurrentHashMap(); + PhoneMetadata metadata = MetadataManager.getMultiFileMetadata(800, map, + "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", + PhoneNumberUtil.DEFAULT_METADATA_LOADER); + assertEquals(metadata, map.get(800)); + } + + public void testGetMultiFileMetadata_missingMetadataFileThrowsRuntimeException() { + // 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 + // best we can do is make sure the exception has the file name in it. + try { + MetadataManager.getMultiFileMetadata("XX", new ConcurrentHashMap(), + "no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER); + fail("expected exception"); + } catch (RuntimeException e) { + assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_XX")); + } + try { + MetadataManager.getMultiFileMetadata(123, new ConcurrentHashMap(), + "no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER); + fail("expected exception"); + } catch (RuntimeException e) { + assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_123")); + } + } } diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java index cf9bc90dd..a8925ab80 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java @@ -17,7 +17,6 @@ package com.google.i18n.phonenumbers; import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata; - import java.util.concurrent.ConcurrentHashMap; import junit.framework.TestCase; @@ -25,43 +24,40 @@ import junit.framework.TestCase; * Unit tests for MultiFileMetadataSourceImpl.java. */ public class MultiFileMetadataSourceImplTest extends TestCase { - public MultiFileMetadataSourceImplTest() {} - - public void testGeographicalRegionMetadataLoadsCorrectly() { - ConcurrentHashMap map = new ConcurrentHashMap(); - PhoneMetadata metadata = MultiFileMetadataSourceImpl.loadMetadataFromFile( - "CA", map, "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", - PhoneNumberUtil.DEFAULT_METADATA_LOADER); - assertEquals(metadata, map.get("CA")); - } + 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); - public void testNonGeographicalRegionMetadataLoadsCorrectly() { - ConcurrentHashMap map = new ConcurrentHashMap(); - PhoneMetadata metadata = MultiFileMetadataSourceImpl.loadMetadataFromFile( - 800, map, "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting", - PhoneNumberUtil.DEFAULT_METADATA_LOADER); - assertEquals(metadata, map.get(800)); + public void testGeoPhoneNumberMetadataLoadCorrectly() { + // We should have some data for the UAE. + PhoneMetadata uaeMetadata = source.getMetadataForRegion("AE"); + assertEquals(uaeMetadata.getCountryCode(), 971); + assertTrue(uaeMetadata.hasGeneralDesc()); } - public void testMissingMetadataFileThrowsRuntimeException() { - // 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 - // best we can do is make sure the exception has the file name in it. + public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { try { - MultiFileMetadataSourceImpl.loadMetadataFromFile( - "XX", new ConcurrentHashMap(), "no/such/file", - PhoneNumberUtil.DEFAULT_METADATA_LOADER); + missingFileSource.getMetadataForRegion("AE"); fail("expected exception"); } catch (RuntimeException e) { - assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_XX")); + assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); } + } + + public void testNonGeoPhoneNumberMetadataLoadCorrectly() { + // We should have some data for international toll-free numbers. + PhoneMetadata intlMetadata = source.getMetadataForNonGeographicalRegion(800); + assertEquals(intlMetadata.getId(), "001"); + assertTrue(intlMetadata.hasGeneralDesc()); + } + + public void testNonGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { try { - MultiFileMetadataSourceImpl.loadMetadataFromFile( - 123, new ConcurrentHashMap(), "no/such/file", - PhoneNumberUtil.DEFAULT_METADATA_LOADER); + missingFileSource.getMetadataForNonGeographicalRegion(800); fail("expected exception"); } catch (RuntimeException e) { - assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_123")); + assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); } } } diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java index 5b7f13036..ce2efc113 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java @@ -20,18 +20,26 @@ import junit.framework.TestCase; /** * Unit tests for SingleFileMetadataSourceImpl.java. + * + *

+ * We do not package single file metadata files, so it is only possible to test failures here. */ public class SingleFileMetadataSourceImplTest extends TestCase { - public SingleFileMetadataSourceImplTest() {} + private static final SingleFileMetadataSourceImpl MISSING_FILE_SOURCE = + new SingleFileMetadataSourceImpl("no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER); + + public void testGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { + try { + MISSING_FILE_SOURCE.getMetadataForRegion("AE"); + fail("expected exception"); + } catch (RuntimeException e) { + assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); + } + } - public void testMissingMetadataFileThrowsRuntimeException() { - SingleFileMetadataSourceImpl singleFileMetadataSource = new SingleFileMetadataSourceImpl( - "no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER); - // 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 - // best we can do is make sure the exception has the file name in it. + public void testNonGeoPhoneNumberMetadataLoadFromMissingFileThrowsException() throws Exception { try { - singleFileMetadataSource.loadMetadataFromFile(); + MISSING_FILE_SOURCE.getMetadataForNonGeographicalRegion(800); fail("expected exception"); } catch (RuntimeException e) { assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file")); diff --git a/java/pending_code_changes.txt b/java/pending_code_changes.txt index 8cb8061c6..40ac6b7b7 100644 --- a/java/pending_code_changes.txt +++ b/java/pending_code_changes.txt @@ -1,4 +1,7 @@ Code changes: - - Removing all references to possible_number_pattern other than the proto buffer - itself (and derived files, or hand-crafted files based on it.) This information is - no longer present in the binary. + - Removing all references to possible_number_pattern other than the proto + buffer itself (and derived files, or hand-crafted files based on it.) This + information is no longer present in the binary. + - Refactored metadata loading and removed synchronization for all kinds of + metadata. Clients may experience quicker loading of alternate formats and + short number metadata, but no change is required for callers of the library. diff --git a/tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java b/tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java index 0ee7df4c1..f3f591f95 100644 --- a/tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java +++ b/tools/java/common/test/com/google/i18n/phonenumbers/MetadataFilterTest.java @@ -21,7 +21,7 @@ import java.util.TreeMap; import java.util.TreeSet; import junit.framework.TestCase; -public final class MetadataFilterTest extends TestCase { +public class MetadataFilterTest extends TestCase { // If this behavior changes then consider whether the change in the blacklist is intended, or you // should change the special build configuration. Also look into any change in the size of the // build. diff --git a/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar b/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar index 08328bd36..b9818a77a 100644 Binary files a/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar and b/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar differ diff --git a/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar b/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar index 5dbafbf31..30d4b8c04 100644 Binary files a/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar and b/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar differ