1 files changed, 28 insertions, 78 deletions

diff --git a/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java b/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java
index d89052c8..9a0b8e69 100644
--- a/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java
+++ b/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java
@@ -17,78 +17,62 @@
 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.io.ObjectInputStream;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 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 phoneNumberMetadataFilePrefix;
 
-  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<String, PhoneMetadata> geographicalRegions =
       new ConcurrentHashMap<String, PhoneMetadata>();
 
-  // 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<Integer, PhoneMetadata> nonGeographicalRegions =
       new ConcurrentHashMap<Integer, PhoneMetadata>();
 
-  // 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 it in here.
   // @VisibleForTesting
-  MultiFileMetadataSourceImpl(String filePrefix, MetadataLoader metadataLoader) {
-    this.filePrefix = filePrefix;
+  MultiFileMetadataSourceImpl(String phoneNumberMetadataFilePrefix, MetadataLoader metadataLoader) {
+    this.phoneNumberMetadataFilePrefix = phoneNumberMetadataFilePrefix;
     this.metadataLoader = metadataLoader;
   }
 
-  // It is assumed that metadataLoader is not null. If needed, checks should happen before passing
-  // here.
-  public MultiFileMetadataSourceImpl(MetadataLoader metadataLoader) {
-    this(META_DATA_FILE_PREFIX, metadataLoader);
+  // It is assumed that metadataLoader is not null. Checks should happen before passing it in here.
+  MultiFileMetadataSourceImpl(MetadataLoader 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.getMetadataFromMultiFilePrefix(regionCode, geographicalRegions,
+        phoneNumberMetadataFilePrefix, 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.getMetadataFromMultiFilePrefix(countryCallingCode, nonGeographicalRegions,
+        phoneNumberMetadataFilePrefix, metadataLoader);
   }
 
   // A country calling code is non-geographical if it only maps to the non-geographical region code,
@@ -99,38 +83,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 <T> PhoneMetadata loadMetadataFromFile(
-      T key, ConcurrentHashMap<T, PhoneMetadata> 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<PhoneMetadata> 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;
-  }
 }