AAPT2: Add order attribute to groups

Require explicit ordering of groups in the configuration file to ensure
that the correct version code is set. Ordering based on a single ABI is
straight forward to ensure Play Store delivers the correct APK, but when
an APK needs more than one ABI things get messy quickly. This also goes
for screen density etc. The only thing that is easily sorted without
this attribute is android-sdk since an artifact can only reference a
single SDK.

Test: unit tests
Test: manually split an APK with update config.xml
Change-Id: I37a2b8b8a8409d6d6ff27c7142d4c8c8065a7a51

1 files changed, 97 insertions, 32 deletions

diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp
index 0329846a5bb5..febbb2ed11a0 100644
--- a/tools/aapt2/configuration/ConfigurationParser_test.cpp
+++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp
@@ -82,22 +82,22 @@ using ::testing::StrEq;
 constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?>
 <post-process xmlns="http://schemas.android.com/tools/aapt">
   <abi-groups>
-    <abi-group label="arm">
-      <abi>armeabi-v7a</abi>
-      <abi>arm64-v8a</abi>
-    </abi-group>
-    <abi-group label="other">
+    <abi-group label="other" version-code-order="2">
       <abi>x86</abi>
       <abi>mips</abi>
     </abi-group>
+    <abi-group label="arm" version-code-order="1">
+      <abi>armeabi-v7a</abi>
+      <abi>arm64-v8a</abi>
+    </abi-group>
   </abi-groups>
   <screen-density-groups>
-    <screen-density-group label="large">
+    <screen-density-group label="large" version-code-order="2">
       <screen-density>xhdpi</screen-density>
       <screen-density>xxhdpi</screen-density>
       <screen-density>xxxhdpi</screen-density>
     </screen-density-group>
-    <screen-density-group label="alldpi">
+    <screen-density-group label="alldpi" version-code-order="1">
       <screen-density>ldpi</screen-density>
       <screen-density>mdpi</screen-density>
       <screen-density>hdpi</screen-density>
@@ -107,17 +107,20 @@ constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?>
     </screen-density-group>
   </screen-density-groups>
   <locale-groups>
-    <locale-group label="europe">
+    <locale-group label="europe" version-code-order="1">
       <locale>en</locale>
       <locale>es</locale>
       <locale>fr</locale>
       <locale>de</locale>
     </locale-group>
-    <locale-group label="north-america">
+    <locale-group label="north-america" version-code-order="2">
       <locale>en</locale>
       <locale>es-rMX</locale>
       <locale>fr-rCA</locale>
     </locale-group>
+    <locale-group label="all" version-code-order="-1">
+      <locale />
+    </locale-group>
   </locale-groups>
   <android-sdks>
     <android-sdk
@@ -131,14 +134,14 @@ constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?>
     </android-sdk>
   </android-sdks>
   <gl-texture-groups>
-    <gl-texture-group label="dxt1">
+    <gl-texture-group label="dxt1" version-code-order="2">
       <gl-texture name="GL_EXT_texture_compression_dxt1">
         <texture-path>assets/dxt1/*</texture-path>
       </gl-texture>
     </gl-texture-group>
   </gl-texture-groups>
   <device-feature-groups>
-    <device-feature-group label="low-latency">
+    <device-feature-group label="low-latency" version-code-order="2">
       <supports-feature>android.hardware.audio.low_latency</supports-feature>
     </device-feature-group>
   </device-feature-groups>
@@ -188,19 +191,22 @@ TEST_F(ConfigurationParserTest, ExtractConfiguration) {
 
   auto& arm = config.abi_groups["arm"];
   auto& other = config.abi_groups["other"];
-  EXPECT_EQ(arm.order, 1ul);
-  EXPECT_EQ(other.order, 2ul);
+  EXPECT_EQ(arm.order, 1);
+  EXPECT_EQ(other.order, 2);
 
   auto& large = config.screen_density_groups["large"];
   auto& alldpi = config.screen_density_groups["alldpi"];
-  EXPECT_EQ(large.order, 1ul);
-  EXPECT_EQ(alldpi.order, 2ul);
+  EXPECT_EQ(large.order, 2);
+  EXPECT_EQ(alldpi.order, 1);
 
   auto& north_america = config.locale_groups["north-america"];
   auto& europe = config.locale_groups["europe"];
+  auto& all = config.locale_groups["all"];
   // Checked in reverse to make sure access order does not matter.
-  EXPECT_EQ(north_america.order, 2ul);
-  EXPECT_EQ(europe.order, 1ul);
+  EXPECT_EQ(north_america.order, 2);
+  EXPECT_EQ(europe.order, 1);
+  EXPECT_EQ(all.order, -1);
+  EXPECT_EQ(3ul, config.locale_groups.size());
 }
 
 TEST_F(ConfigurationParserTest, ValidateFile) {
@@ -392,7 +398,7 @@ TEST_F(ConfigurationParserTest, ArtifactFormatAction) {
 
 TEST_F(ConfigurationParserTest, AbiGroupAction) {
   static constexpr const char* xml = R"xml(
-    <abi-group label="arm">
+    <abi-group label="arm"  version-code-order="2">
       <!-- First comment. -->
       <abi>
         armeabi-v7a
@@ -415,7 +421,8 @@ TEST_F(ConfigurationParserTest, AbiGroupAction) {
 }
 
 TEST_F(ConfigurationParserTest, AbiGroupAction_EmptyGroup) {
-  static constexpr const char* xml = R"xml(<abi-group label="arm64-v8a"/>)xml";
+  static constexpr const char* xml =
+      R"xml(<abi-group label="arm64-v8a" version-code-order="3"/>)xml";
 
   auto doc = test::BuildXmlDom(xml);
 
@@ -426,12 +433,23 @@ TEST_F(ConfigurationParserTest, AbiGroupAction_EmptyGroup) {
   EXPECT_THAT(config.abi_groups, SizeIs(1ul));
   ASSERT_EQ(1u, config.abi_groups.count("arm64-v8a"));
 
-  auto& out = config.abi_groups["arm64-v8a"].entry;
-  ASSERT_THAT(out, ElementsAre(Abi::kArm64V8a));
+  auto& out = config.abi_groups["arm64-v8a"];
+  ASSERT_THAT(out.entry, ElementsAre(Abi::kArm64V8a));
+  EXPECT_EQ(3, out.order);
+}
+
+TEST_F(ConfigurationParserTest, AbiGroupAction_EmptyGroup_NoOrder) {
+  static constexpr const char* xml = R"xml(<abi-group label="arm64-v8a"/>)xml";
+
+  auto doc = test::BuildXmlDom(xml);
+
+  PostProcessingConfiguration config;
+  bool ok = AbiGroupTagHandler(&config, NodeCast<Element>(doc.get()->root.get()), &diag_);
+  ASSERT_FALSE(ok);
 }
 
 TEST_F(ConfigurationParserTest, AbiGroupAction_InvalidEmptyGroup) {
-  static constexpr const char* xml = R"xml(<abi-group label="arm"/>)xml";
+  static constexpr const char* xml = R"xml(<abi-group label="arm" order="2"/>)xml";
 
   auto doc = test::BuildXmlDom(xml);
 
@@ -442,7 +460,7 @@ TEST_F(ConfigurationParserTest, AbiGroupAction_InvalidEmptyGroup) {
 
 TEST_F(ConfigurationParserTest, ScreenDensityGroupAction) {
   static constexpr const char* xml = R"xml(
-    <screen-density-group label="large">
+    <screen-density-group label="large" version-code-order="2">
       <screen-density>xhdpi</screen-density>
       <screen-density>
         xxhdpi
@@ -471,7 +489,8 @@ TEST_F(ConfigurationParserTest, ScreenDensityGroupAction) {
 }
 
 TEST_F(ConfigurationParserTest, ScreenDensityGroupAction_EmtpyGroup) {
-  static constexpr const char* xml = R"xml(<screen-density-group label="xhdpi"/>)xml";
+  static constexpr const char* xml =
+      R"xml(<screen-density-group label="xhdpi" version-code-order="4"/>)xml";
 
   auto doc = test::BuildXmlDom(xml);
 
@@ -485,8 +504,19 @@ TEST_F(ConfigurationParserTest, ScreenDensityGroupAction_EmtpyGroup) {
   ConfigDescription xhdpi;
   xhdpi.density = ResTable_config::DENSITY_XHIGH;
 
-  auto& out = config.screen_density_groups["xhdpi"].entry;
-  ASSERT_THAT(out, ElementsAre(xhdpi));
+  auto& out = config.screen_density_groups["xhdpi"];
+  EXPECT_THAT(out.entry, ElementsAre(xhdpi));
+  EXPECT_EQ(4, out.order);
+}
+
+TEST_F(ConfigurationParserTest, ScreenDensityGroupAction_EmtpyGroup_NoVersion) {
+  static constexpr const char* xml = R"xml(<screen-density-group label="xhdpi"/>)xml";
+
+  auto doc = test::BuildXmlDom(xml);
+
+  PostProcessingConfiguration config;
+  bool ok = ScreenDensityGroupTagHandler(&config, NodeCast<Element>(doc.get()->root.get()), &diag_);
+  ASSERT_FALSE(ok);
 }
 
 TEST_F(ConfigurationParserTest, ScreenDensityGroupAction_InvalidEmtpyGroup) {
@@ -501,7 +531,7 @@ TEST_F(ConfigurationParserTest, ScreenDensityGroupAction_InvalidEmtpyGroup) {
 
 TEST_F(ConfigurationParserTest, LocaleGroupAction) {
   static constexpr const char* xml = R"xml(
-    <locale-group label="europe">
+    <locale-group label="europe" version-code-order="2">
       <locale>en</locale>
       <locale>es</locale>
       <locale>fr</locale>
@@ -528,7 +558,7 @@ TEST_F(ConfigurationParserTest, LocaleGroupAction) {
 }
 
 TEST_F(ConfigurationParserTest, LocaleGroupAction_EmtpyGroup) {
-  static constexpr const char* xml = R"xml(<locale-group label="en"/>)xml";
+  static constexpr const char* xml = R"xml(<locale-group label="en" version-code-order="6"/>)xml";
 
   auto doc = test::BuildXmlDom(xml);
 
@@ -539,11 +569,22 @@ TEST_F(ConfigurationParserTest, LocaleGroupAction_EmtpyGroup) {
   ASSERT_EQ(1ul, config.locale_groups.size());
   ASSERT_EQ(1u, config.locale_groups.count("en"));
 
-  const auto& out = config.locale_groups["en"].entry;
+  const auto& out = config.locale_groups["en"];
 
   ConfigDescription en = test::ParseConfigOrDie("en");
 
-  ASSERT_THAT(out, ElementsAre(en));
+  EXPECT_THAT(out.entry, ElementsAre(en));
+  EXPECT_EQ(6, out.order);
+}
+
+TEST_F(ConfigurationParserTest, LocaleGroupAction_EmtpyGroup_NoOrder) {
+  static constexpr const char* xml = R"xml(<locale-group label="en"/>)xml";
+
+  auto doc = test::BuildXmlDom(xml);
+
+  PostProcessingConfiguration config;
+  bool ok = LocaleGroupTagHandler(&config, NodeCast<Element>(doc.get()->root.get()), &diag_);
+  ASSERT_FALSE(ok);
 }
 
 TEST_F(ConfigurationParserTest, LocaleGroupAction_InvalidEmtpyGroup) {
@@ -695,7 +736,7 @@ TEST_F(ConfigurationParserTest, AndroidSdkGroupAction_NonNumeric) {
 
 TEST_F(ConfigurationParserTest, GlTextureGroupAction) {
   static constexpr const char* xml = R"xml(
-    <gl-texture-group label="dxt1">
+    <gl-texture-group label="dxt1" version-code-order="2">
       <gl-texture name="GL_EXT_texture_compression_dxt1">
         <texture-path>assets/dxt1/main/*</texture-path>
         <texture-path>
@@ -726,7 +767,7 @@ TEST_F(ConfigurationParserTest, GlTextureGroupAction) {
 
 TEST_F(ConfigurationParserTest, DeviceFeatureGroupAction) {
   static constexpr const char* xml = R"xml(
-    <device-feature-group label="low-latency">
+    <device-feature-group label="low-latency" version-code-order="2">
       <supports-feature>android.hardware.audio.low_latency</supports-feature>
       <supports-feature>
         android.hardware.audio.pro
@@ -749,6 +790,30 @@ TEST_F(ConfigurationParserTest, DeviceFeatureGroupAction) {
   ASSERT_THAT(out, ElementsAre(low_latency, pro));
 }
 
+TEST_F(ConfigurationParserTest, Group_Valid) {
+  Group<int32_t> group;
+  group["item1"].order = 1;
+  group["item2"].order = 2;
+  group["item3"].order = 3;
+  group["item4"].order = 4;
+  group["item5"].order = 5;
+  group["item6"].order = 6;
+
+  EXPECT_TRUE(IsGroupValid(group, "test", &diag_));
+}
+
+TEST_F(ConfigurationParserTest, Group_OverlappingOrder) {
+  Group<int32_t> group;
+  group["item1"].order = 1;
+  group["item2"].order = 2;
+  group["item3"].order = 3;
+  group["item4"].order = 2;
+  group["item5"].order = 5;
+  group["item6"].order = 1;
+
+  EXPECT_FALSE(IsGroupValid(group, "test", &diag_));
+}
+
 // Artifact name parser test cases.
 
 TEST(ArtifactTest, Simple) {