Refactor metadata loading and close all streams after loading metadata

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

@@ -36,13 +36,13 @@ import java.util.logging.Logger;
  * additional data files such as PhoneNumberAlternateFormats, but in the future it is envisaged it
  * would handle the main metadata file (PhoneNumberMetadata.xml) as well.
  */
-class MetadataManager {
+final class MetadataManager {
   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 Logger LOGGER = Logger.getLogger(MetadataManager.class.getName());
+  private static final Logger logger = Logger.getLogger(MetadataManager.class.getName());
 
   private static final Map<Integer, PhoneMetadata> callingCodeToAlternateFormatsMap =
       Collections.synchronizedMap(new HashMap<Integer, PhoneMetadata>());
@@ -61,19 +61,15 @@ class MetadataManager {
   private MetadataManager() {
   }
 
-  private static void close(InputStream in) {
-    if (in != null) {
-      try {
-        in.close();
-      } catch (IOException e) {
-        LOGGER.log(Level.WARNING, e.toString());
-      }
-    }
-  }
+  // The size of the byte buffer in bytes used to convert a stream containing metadata for a single
+  // region, to a nanoproto-compatible CodedInputByteBufferNano. This was determined by the size of
+  // the binary metadata files that contain each region's metadata.
+  static final int DEFAULT_BUFFER_SIZE = 16 * 1024;
 
-  // The size of the byte buffer used for deserializing the alternate formats and short number
-  // metadata files for each region.
-  private static final int BUFFER_SIZE = 16 * 1024;
+  // The size of the byte buffer in bytes used to convert a stream containing metadata for all
+  // regions, to a nanoproto-compatible CodedInputByteBufferNano. This was determined by the size of
+  // the binary metadata file that contains all regions' metadata.
+  static final int ALL_REGIONS_BUFFER_SIZE = 256 * 1024;
 
   static CodedInputByteBufferNano convertStreamToByteBuffer(ObjectInputStream in, int bufferSize)
       throws IOException {
@@ -89,22 +85,55 @@ class MetadataManager {
     return CodedInputByteBufferNano.newInstance(outputStream.toByteArray());
   }
 
-  private static void loadAlternateFormatsMetadataFromFile(int countryCallingCode) {
-    InputStream source = PhoneNumberMatcher.class.getResourceAsStream(
-        ALTERNATE_FORMATS_FILE_PREFIX + "_" + countryCallingCode);
-    ObjectInputStream in = null;
+  /**
+   * Loads and returns the metadata protocol buffer from the given stream and closes the stream.
+   *
+   * @param source      the non-null stream from which metadata is to be read.
+   * @param bufferSize  the size of the buffer in bytes used to convert the stream to a
+                        nanoproto-compatible {@code CodedInputByteBufferNano}.
+   * @return            the loaded metadata protocol buffer.
+   */
+  static PhoneMetadataCollection loadMetadataAndCloseInput(InputStream source, int bufferSize) {
+    ObjectInputStream ois = null;
     try {
-      in = new ObjectInputStream(source);
-      CodedInputByteBufferNano byteBuffer = convertStreamToByteBuffer(in, BUFFER_SIZE);
-      PhoneMetadataCollection alternateFormats = new PhoneMetadataCollection();
-      alternateFormats.mergeFrom(byteBuffer);
-      for (PhoneMetadata metadata : alternateFormats.metadata) {
-        callingCodeToAlternateFormatsMap.put(metadata.countryCode, metadata);
+      try {
+        ois = new ObjectInputStream(source);
+      } catch (IOException e) {
+        throw new RuntimeException("cannot load/parse metadata", e);
+      }
+      PhoneMetadataCollection metadataCollection = new PhoneMetadataCollection();
+      try {
+        metadataCollection.mergeFrom(convertStreamToByteBuffer(ois, bufferSize));
+      } catch (IOException e) {
+        throw new RuntimeException("cannot load/parse metadata", e);
       }
-    } catch (IOException e) {
-      LOGGER.log(Level.WARNING, e.toString());
+      return metadataCollection;
     } finally {
-      close(in);
+      try {
+        if (ois != null) {
+          // This will close all underlying streams as well, including source.
+          ois.close();
+        } else {
+          source.close();
+        }
+      } catch (IOException e) {
+        logger.log(Level.WARNING, "error closing input stream (ignored)", e);
+      }
+    }
+  }
+
+  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 alternateFormatData =
+        loadMetadataAndCloseInput(source, DEFAULT_BUFFER_SIZE);
+    for (PhoneMetadata metadata : alternateFormatData.metadata) {
+      callingCodeToAlternateFormatsMap.put(metadata.countryCode, metadata);
     }
   }
 
@@ -121,21 +150,17 @@ class MetadataManager {
   }
 
   private static void loadShortNumberMetadataFromFile(String regionCode) {
-    InputStream source = PhoneNumberMatcher.class.getResourceAsStream(
-        SHORT_NUMBER_METADATA_FILE_PREFIX + "_" + regionCode);
-    ObjectInputStream in = null;
-    try {
-      in = new ObjectInputStream(source);
-      CodedInputByteBufferNano byteBuffer = convertStreamToByteBuffer(in, BUFFER_SIZE);
-      PhoneMetadataCollection shortNumberMetadata = new PhoneMetadataCollection();
-      shortNumberMetadata.mergeFrom(byteBuffer);
-      for (PhoneMetadata metadata : shortNumberMetadata.metadata) {
+    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 shortNumberData =
+        loadMetadataAndCloseInput(source, DEFAULT_BUFFER_SIZE);
+    for (PhoneMetadata metadata : shortNumberData.metadata) {
         regionCodeToShortNumberMetadataMap.put(regionCode, metadata);
-      }
-    } catch (IOException e) {
-      LOGGER.log(Level.WARNING, e.toString());
-    } finally {
-      close(in);
     }
   }
 

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

@@ -19,9 +19,7 @@ package com.google.i18n.phonenumbers;
 import com.google.i18n.phonenumbers.nano.Phonemetadata.PhoneMetadata;
 import com.google.i18n.phonenumbers.nano.Phonemetadata.PhoneMetadataCollection;
 
-import java.io.IOException;
 import java.io.InputStream;
-import java.io.ObjectInputStream;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.List;
 import java.util.logging.Level;
@@ -114,43 +112,22 @@ final class MultiFileMetadataSourceImpl implements MetadataSource {
     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 = loadMetadataAndCloseInput(source);
-    PhoneMetadata[] metadataList = metadataCollection.metadata;
-    if (metadataList.length == 0) {
+    PhoneMetadataCollection metadataCollection =
+        MetadataManager.loadMetadataAndCloseInput(source, MetadataManager.DEFAULT_BUFFER_SIZE);
+    PhoneMetadata[] metadatas = metadataCollection.metadata;
+    if (metadatas.length == 0) {
+      // Sanity check; this should not happen since we build with non-empty metadata.
       throw new IllegalStateException("empty metadata: " + fileName);
     }
-    if (metadataList.length > 1) {
+    if (metadatas.length > 1) {
       logger.log(Level.WARNING, "invalid metadata (too many entries): " + fileName);
     }
-    PhoneMetadata metadata = metadataList[0];
+    PhoneMetadata metadata = metadatas[0];
     PhoneMetadata oldValue = map.putIfAbsent(key, metadata);
     return (oldValue != null) ? oldValue : metadata;
   }
-
-  /**
-   * Loads and returns the metadata protocol buffer from the given stream and closes the stream.
-   */
-  private static PhoneMetadataCollection loadMetadataAndCloseInput(InputStream source) {
-    // The size of the byte buffer used for deserializing the phone number metadata files for each
-    // region.
-    final int MULTI_FILE_BUFFER_SIZE = 16 * 1024;
-
-    try {
-      // Read in metadata for each region.
-      PhoneMetadataCollection metadataCollection = new PhoneMetadataCollection();
-      metadataCollection.mergeFrom(MetadataManager.convertStreamToByteBuffer(
-          new ObjectInputStream(source), MULTI_FILE_BUFFER_SIZE));
-      return metadataCollection;
-    } catch (IOException e) {
-      throw new RuntimeException("cannot load/parse metadata", e);
-    } finally {
-      try {
-        source.close();
-      } catch (IOException e) {
-        logger.log(Level.WARNING, "error closing input stream (ignored)", e);
-      }
-    }
-  }
 }

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

@@ -19,14 +19,11 @@ package com.google.i18n.phonenumbers;
 import com.google.i18n.phonenumbers.nano.Phonemetadata.PhoneMetadata;
 import com.google.i18n.phonenumbers.nano.Phonemetadata.PhoneMetadataCollection;
 
-import java.io.IOException;
 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.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -97,58 +94,29 @@ final class SingleFileMetadataSourceImpl implements MetadataSource {
   void loadMetadataFromFile() {
     InputStream source = metadataLoader.loadMetadata(fileName);
     if (source == null) {
-      logger.log(Level.SEVERE, "missing metadata: " + fileName);
+      // 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);
     }
-    try {
-      PhoneMetadataCollection metadataCollection =
-          loadMetadataAndCloseInput(new ObjectInputStream(source));
-      PhoneMetadata[] metadataList = metadataCollection.metadata;
-      if (metadataList.length == 0) {
-        logger.log(Level.SEVERE, "empty metadata: " + fileName);
-        throw new IllegalStateException("empty metadata: " + fileName);
-      }
-      for (PhoneMetadata metadata : metadataList) {
-        String regionCode = metadata.id;
-        int countryCallingCode = metadata.countryCode;
-        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);
+    PhoneMetadataCollection metadataCollection =
+        MetadataManager.loadMetadataAndCloseInput(source, MetadataManager.ALL_REGIONS_BUFFER_SIZE);
+    PhoneMetadata[] metadatas = metadataCollection.metadata;
+    if (metadatas.length == 0) {
+      // This should not happen since clients shouldn't be using this implementation!
+      throw new IllegalStateException("empty metadata: " + fileName);
     }
-  }
-
-  /**
-   * Loads the metadata protocol buffer from the given stream and closes the stream afterwards. Any
-   * 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(ObjectInputStream source) {
-    // The size of the byte buffer for deserializing the single nano metadata file which holds
-    // metadata for all regions.
-    final int SINGLE_FILE_BUFFER_SIZE = 256 * 1024;
-
-    PhoneMetadataCollection metadataCollection = new PhoneMetadataCollection();
-    try {
-      metadataCollection.mergeFrom(
-          MetadataManager.convertStreamToByteBuffer(source, SINGLE_FILE_BUFFER_SIZE));
-    } catch (IOException e) {
-      logger.log(Level.WARNING, "error reading input (ignored)", e);
-    } finally {
-      try {
-        source.close();
-      } catch (IOException e) {
-        logger.log(Level.WARNING, "error closing input stream (ignored)", e);
+    for (PhoneMetadata metadata : metadatas) {
+      String regionCode = metadata.id;
+      int countryCallingCode = metadata.countryCode;
+      boolean isNonGeoRegion = PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY.equals(regionCode);
+      if (isNonGeoRegion) {
+        countryCodeToNonGeographicalMetadataMap.put(countryCallingCode, metadata);
+      } else {
+        regionToMetadataMap.put(regionCode, metadata);
       }
     }
-    return metadataCollection;
   }
 }

java/pending_code_changes.txt

@@ -1 +1,2 @@
-
+Code changes:
+ - Refactored metadata loading and closed all streams after loading.