From eb8ef01791f3c35e28dcbd53f9352f0f81a6d361 Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Mon, 22 Jun 2015 20:15:19 -0700 Subject: More thorough XML testing. Include vendor.xml in unit tests. Note that changing the build target with lunch can switch your vendor.xml. Update DefaultCarrierConfigService#readConfigFromXml to throw errors instead of returning an empty bundle. This makes failed test output much better. Add a test to check that every variable in XML files matches a KEY in CarrierConfigManager. Increase detail of error messages with XmlPullParser#getPositionDescription Bug: 21619172 Change-Id: I500f4b2476a6fe4fdd6ae0232e293a0f1d82b7b0 --- .../carrierconfig/DefaultCarrierConfigService.java | 69 ++++++++--------- .../android/carrierconfig/CarrierConfigTest.java | 86 +++++++++++++++++++++- 2 files changed, 119 insertions(+), 36 deletions(-) diff --git a/src/com/android/carrierconfig/DefaultCarrierConfigService.java b/src/com/android/carrierconfig/DefaultCarrierConfigService.java index 2317156..9081821 100644 --- a/src/com/android/carrierconfig/DefaultCarrierConfigService.java +++ b/src/com/android/carrierconfig/DefaultCarrierConfigService.java @@ -58,35 +58,45 @@ public class DefaultCarrierConfigService extends CarrierService { return null; } - String fileName = "carrier_config_" + id.getMcc() + id.getMnc() + ".xml"; + + PersistableBundle config = null; try { synchronized (this) { if (mFactory == null) { mFactory = XmlPullParserFactory.newInstance(); } } + XmlPullParser parser = mFactory.newPullParser(); + String fileName = "carrier_config_" + id.getMcc() + id.getMnc() + ".xml"; parser.setInput(getApplicationContext().getAssets().open(fileName), "utf-8"); - PersistableBundle config = readConfigFromXml(parser, id); - // Treat vendor.xml as if it were appended to the carrier config file we read. - XmlPullParser vendorInput = getApplicationContext().getResources().getXml(R.xml.vendor); + config = readConfigFromXml(parser, id); + } + catch (IOException | XmlPullParserException e) { + Log.d(TAG, e.toString()); + // We can return an empty config for unknown networks. + config = new PersistableBundle(); + } + + // Treat vendor.xml as if it were appended to the carrier config file we read. + XmlPullParser vendorInput = getApplicationContext().getResources().getXml(R.xml.vendor); + try { PersistableBundle vendorConfig = readConfigFromXml(vendorInput, id); config.putAll(vendorConfig); - return config; } catch (IOException | XmlPullParserException e) { - // Return null for unknown networks - they should use the defaults. Log.e(TAG, e.toString()); - return null; } + + return config; } /** * Parses an XML document and returns a PersistableBundle. * - *

This function iterates over each {@code } node in the XML document and parses - * it into a bundle if its filters match {@code id}. The format of XML bundles is defined by - * {@link PersistableBundle#restoreFromXml}. All the matching bundles will be flattened and + *

This function iterates over each {@code } node in the XML document and + * parses it into a bundle if its filters match {@code id}. The format of XML bundles is defined + * by {@link PersistableBundle#restoreFromXml}. All the matching bundles will be flattened and * returned as a single bundle.

* *

Here is an example document. The second bundle will be applied to the first only if the @@ -106,34 +116,27 @@ public class DefaultCarrierConfigService extends CarrierService { * @param id the details of the SIM operator used to filter parts of the document * @return a possibly empty PersistableBundle containing the config values. */ - static PersistableBundle readConfigFromXml(XmlPullParser parser, CarrierIdentifier id) { + static PersistableBundle readConfigFromXml(XmlPullParser parser, CarrierIdentifier id) + throws IOException, XmlPullParserException { PersistableBundle config = new PersistableBundle(); if (parser == null) { return config; } - try { - // Iterate over each node in the document and add it to the returned - // bundle if its filters match. - int event; - while (((event = parser.next()) != XmlPullParser.END_DOCUMENT)) { - if (event == XmlPullParser.START_TAG && "carrier_config".equals(parser.getName())) { - // Skip this fragment if it has filters that don't match. - if (!checkFilters(parser, id)) { - continue; - } - PersistableBundle configFragment = PersistableBundle.restoreFromXml(parser); - config.putAll(configFragment); + // Iterate over each node in the document and add it to the returned + // bundle if its filters match. + int event; + while (((event = parser.next()) != XmlPullParser.END_DOCUMENT)) { + if (event == XmlPullParser.START_TAG && "carrier_config".equals(parser.getName())) { + // Skip this fragment if it has filters that don't match. + if (!checkFilters(parser, id)) { + continue; } + PersistableBundle configFragment = PersistableBundle.restoreFromXml(parser); + config.putAll(configFragment); } } - catch (XmlPullParserException e) { - Log.e(TAG, e.toString()); - } - catch (IOException e) { - Log.e(TAG, e.toString()); - } return config; } @@ -141,10 +144,10 @@ public class DefaultCarrierConfigService extends CarrierService { /** * Checks to see if an XML node matches carrier filters. * - *

This iterates over the attributes of the current tag pointed to by {@code parser} and checks - * each one against {@code id} or {@link Build.DEVICE}. Attributes that are not specified in the - * node will not be checked, so a node with no attributes will always return true. The supported - * filter attributes are, + *

This iterates over the attributes of the current tag pointed to by {@code parser} and + * checks each one against {@code id} or {@link Build.DEVICE}. Attributes that are not specified + * in the node will not be checked, so a node with no attributes will always return true. The + * supported filter attributes are, *