diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java index 0d998c751..414a9dc77 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/MetadataManager.java +++ b/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 callingCodeToAlternateFormatsMap = Collections.synchronizedMap(new HashMap()); @@ -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); } } diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java index 37d7bca6a..211b7f0f0 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/MultiFileMetadataSourceImpl.java +++ b/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); - } - } - } } diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java index b89c234de..f21d15aaa 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/SingleFileMetadataSourceImpl.java +++ b/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; } } diff --git a/java/pending_code_changes.txt b/java/pending_code_changes.txt index 8b1378917..b7c555e2e 100644 --- a/java/pending_code_changes.txt +++ b/java/pending_code_changes.txt @@ -1 +1,2 @@ - +Code changes: + - Refactored metadata loading and closed all streams after loading.