Make robust SingleFileMetadataSourceImpl and make it consistent with

MultiFileMetadataSourceImpl.

java/libphonenumber/pom.xml

@@ -36,6 +36,8 @@
           <excludes>
             <exclude>**/SingleFileMetadataSourceImpl.class</exclude>
             <exclude>**/SingleFileMetadataSourceImpl.java</exclude>
+            <exclude>**/SingleFileMetadataSourceImplTest.class</exclude>
+            <exclude>**/SingleFileMetadataSourceImplTest.java</exclude>
             <exclude>**/SingleFilePhoneNumberMetadataProto</exclude>
             <exclude>**/SingleFilePhoneNumberMetadataProtoForTesting</exclude>
           </excludes>

java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataSource.java

@@ -19,7 +19,7 @@ package com.google.i18n.phonenumbers;
 import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata;
 
 /**
- * A source for phone metadata from resources.
+ * A source for phone metadata for all regions.
  */
 interface MetadataSource {
   /**

java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java

@@ -129,8 +129,7 @@ final class MultiFileMetadataSourceImpl implements MetadataSource {
 
   /**
    * Loads the metadata protocol buffer from the given stream and closes the stream afterwards. Any
-   * exceptions that occur while reading the stream are propagated (though exceptions that occur
-   * when the stream is closed will be ignored).
+   * exceptions that occur while reading or closing the stream are ignored.
    *
    * @param source  the non-null stream from which metadata is to be read.
    * @return        the loaded metadata protocol buffer.

java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java

@@ -1,3 +1,19 @@
+/*
+ * Copyright (C) 2015 The Libphonenumber Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package com.google.i18n.phonenumbers;
 
 import com.google.i18n.phonenumbers.Phonemetadata.PhoneMetadata;
@@ -7,23 +23,21 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
 import java.util.Collections;
-import java.util.EnumMap;
 import java.util.HashMap;
-import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * Implementation of {@link MetadataSource} that reads from a resource file
- * during initialization.
+ * Implementation of {@link MetadataSource} that reads from a single resource file.
  */
-public final class SingleFileMetadataSourceImpl implements MetadataSource {
+final class SingleFileMetadataSourceImpl implements MetadataSource {
 
   private static final Logger logger =
       Logger.getLogger(SingleFileMetadataSourceImpl.class.getName());
 
-  private static final String META_DATA_FILE =
+  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.
@@ -40,57 +54,95 @@ public final class SingleFileMetadataSourceImpl implements MetadataSource {
   private final Map<Integer, PhoneMetadata> countryCodeToNonGeographicalMetadataMap =
       Collections.synchronizedMap(new HashMap<Integer, PhoneMetadata>());
 
+  // The metadata file from which region data is loaded.
+  private final String fileName;
+
+  // The metadata loader 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;
+    this.metadataLoader = metadataLoader;
+  }
+
   // It is assumed that metadataLoader is not null.
   public SingleFileMetadataSourceImpl(MetadataLoader metadataLoader) {
-    InputStream input = metadataLoader.loadMetadata(META_DATA_FILE);
-    if (input == null) {
-      throw new IllegalStateException(
-          "no metadata available for PhoneNumberUtil: " + META_DATA_FILE);
-    }
-    PhoneMetadataCollection metadataCollection = loadMetadataAndCloseInput(input);
-    for (PhoneMetadata metadata : metadataCollection.getMetadataList()) {
-      String regionCode = metadata.getId();
-      int countryCallingCode = metadata.getCountryCode();
-      if (PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode)) {
-        countryCodeToNonGeographicalMetadataMap.put(countryCallingCode, metadata);
-      } else {
-        regionToMetadataMap.put(regionCode, metadata);
-      }
-    }
+    this(META_DATA_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);
   }
 
   @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) {
+      logger.log(Level.SEVERE, "missing metadata: " + fileName);
+      throw new IllegalStateException("missing metadata: " + fileName);
+    }
+    ObjectInputStream in = null;
+    try {
+      in = new ObjectInputStream(source);
+      PhoneMetadataCollection metadataCollection = loadMetadataAndCloseInput(in);
+      List<PhoneMetadata> metadataList = metadataCollection.getMetadataList();
+      if (metadataList.isEmpty()) {
+        logger.log(Level.SEVERE, "empty metadata: " + fileName);
+        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);
+        }
+      }
+    } catch (IOException e) {
+      logger.log(Level.SEVERE, "cannot load/parse metadata: " + fileName, e);
+      throw new RuntimeException("cannot load/parse metadata: " + fileName, e);
+    }
+  }
+
   /**
    * Loads the metadata protocol buffer from the given stream and closes the stream afterwards. Any
-   * exceptions that occur while reading the stream are propagated (though exceptions that occur
-   * when the stream is closed will be ignored).
+   * exceptions that occur while reading or closing the stream are ignored.
    *
    * @param source  the non-null stream from which metadata is to be read.
    * @return        the loaded metadata protocol buffer.
    */
-  private static PhoneMetadataCollection loadMetadataAndCloseInput(InputStream source) {
+  private static PhoneMetadataCollection loadMetadataAndCloseInput(ObjectInputStream source) {
     PhoneMetadataCollection metadataCollection = new PhoneMetadataCollection();
     try {
-      // Read in metadata for each region.
-      ObjectInputStream in = new ObjectInputStream(source);
-      metadataCollection.readExternal(in);
-      return metadataCollection;
+      metadataCollection.readExternal(source);
     } catch (IOException e) {
-      logger.log(Level.WARNING, e.toString());
+      logger.log(Level.WARNING, "error reading input (ignored)", e);
     } finally {
       try {
         source.close();
       } catch (IOException e) {
-        logger.log(Level.WARNING, e.toString());
+        logger.log(Level.WARNING, "error closing input stream (ignored)", e);
       }
     }
     return metadataCollection;

java/libphonenumber/test/com/google/i18n/phonenumbers/MultiFileMetadataSourceImplTest.java

@@ -16,19 +16,17 @@
 
 package com.google.i18n.phonenumbers;
 
+import junit.framework.TestCase;
+
 /**
  * Unit tests for MultiFileMetadataSourceImpl.java.
  */
-public class MultiFileMetadataSourceImplTest extends TestMetadataTestCase  {
-
-  private final MultiFileMetadataSourceImpl multiFileMetadataSource;
-
-  public MultiFileMetadataSourceImplTest() {
-    multiFileMetadataSource = new MultiFileMetadataSourceImpl(
-        "no/such/file", PhoneNumberUtil.DEFAULT_METADATA_LOADER);
-  }
+public class MultiFileMetadataSourceImplTest extends TestCase {
+  public MultiFileMetadataSourceImplTest() {}
 
   public void testMissingMetadataFileThrowsRuntimeException() {
+    MultiFileMetadataSourceImpl multiFileMetadataSource = new MultiFileMetadataSourceImpl(
+        "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.
@@ -36,7 +34,7 @@ public class MultiFileMetadataSourceImplTest extends TestMetadataTestCase  {
       multiFileMetadataSource.loadMetadataFromFile("XX", -1);
       fail("expected exception");
     } catch (RuntimeException e) {
-      assertTrue("Unexpected error: " + e, e.toString().contains("no/such/file_XX"));
+      assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file_XX"));
     }
     try {
       multiFileMetadataSource.loadMetadataFromFile(

java/libphonenumber/test/com/google/i18n/phonenumbers/SingleFileMetadataSourceImplTest.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2015 The Libphonenumber Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.i18n.phonenumbers;
+
+import junit.framework.TestCase;
+
+/**
+ * Unit tests for SingleFileMetadataSourceImpl.java.
+ */
+public class SingleFileMetadataSourceImplTest extends TestCase {
+  public SingleFileMetadataSourceImplTest() {}
+
+  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.
+    try {
+      singleFileMetadataSource.loadMetadataFromFile();
+      fail("expected exception");
+    } catch (RuntimeException e) {
+      assertTrue("Unexpected error: " + e, e.getMessage().contains("no/such/file"));
+    }
+  }
+}

java/libphonenumber/test/com/google/i18n/phonenumbers/TestMetadataTestCase.java

@@ -27,7 +27,7 @@ import junit.framework.TestCase;
  * @author Shaopeng Jia
  */
 public class TestMetadataTestCase extends TestCase {
-  protected static final String TEST_META_DATA_FILE_PREFIX =
+  private static final String TEST_META_DATA_FILE_PREFIX =
       "/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting";
 
   protected final PhoneNumberUtil phoneUtil;