AAPT2: Push more configuration code into the parser

When parsing is complete, we now have a list of output artifacts that
have their referential integrity validated. This means that once the
configuration file is parsed, the only errors that can occur are related
to APK processing, and not the configuration itself.

This reduces the number of errors that could cause a partial output of
APK artifacts. It simplifies the public API and reduces the complexity of
the code to generate multiple APKs.

Test: Ran unit tests
Test: manually ran the optimize command to ensure it still works

Change-Id: I3f2d885b207a84c958f5348a4baa6718598184a4

3 files changed, 57 insertions, 175 deletions

diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp
index e2d738aec5a2..16898d6f3e03 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.cpp
+++ b/tools/aapt2/optimize/MultiApkGenerator.cpp
@@ -40,33 +40,11 @@
 namespace aapt {
 
 using ::aapt::configuration::AndroidSdk;
-using ::aapt::configuration::Artifact;
-using ::aapt::configuration::PostProcessingConfiguration;
+using ::aapt::configuration::OutputArtifact;
 using ::aapt::xml::kSchemaAndroid;
 using ::aapt::xml::XmlResource;
 using ::android::StringPiece;
 
-namespace {
-
-Maybe<AndroidSdk> GetAndroidSdk(const Artifact& artifact, const PostProcessingConfiguration& config,
-                                IDiagnostics* diag) {
-  if (!artifact.android_sdk_group) {
-    return {};
-  }
-
-  const std::string& group_name = artifact.android_sdk_group.value();
-  auto group = config.android_sdk_groups.find(group_name);
-  // TODO: Remove validation when configuration parser ensures referential integrity.
-  if (group == config.android_sdk_groups.end()) {
-    diag->Error(DiagMessage() << "could not find referenced group '" << group_name << "'");
-    return {};
-  }
-
-  return group->second;
-}
-
-}  // namespace
-
 /**
  * Context wrapper that allows the min Android SDK value to be overridden.
  */
@@ -115,8 +93,9 @@ class ContextWrapper : public IAaptContext {
     min_sdk_ = min_sdk;
   }
 
-  void SetSource(const Source& source) {
-    source_diag_ = util::make_unique<SourcePathDiagnostics>(source, context_->GetDiagnostics());
+  void SetSource(const std::string& source) {
+    source_diag_ =
+        util::make_unique<SourcePathDiagnostics>(Source{source}, context_->GetDiagnostics());
   }
 
  private:
@@ -142,82 +121,60 @@ MultiApkGenerator::MultiApkGenerator(LoadedApk* apk, IAaptContext* context)
 
 bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) {
   // TODO(safarmer): Handle APK version codes for the generated APKs.
-  const PostProcessingConfiguration& config = options.config;
-
-  const std::string& apk_name = file::GetFilename(apk_->GetSource().path).to_string();
-  const StringPiece ext = file::GetExtension(apk_name);
-  const std::string base_name = apk_name.substr(0, apk_name.rfind(ext.to_string()));
 
   std::unordered_set<std::string> artifacts_to_keep = options.kept_artifacts;
   std::unordered_set<std::string> filtered_artifacts;
   std::unordered_set<std::string> kept_artifacts;
 
   // For now, just write out the stripped APK since ABI splitting doesn't modify anything else.
-  for (const Artifact& artifact : config.artifacts) {
-    SourcePathDiagnostics diag{{apk_name}, context_->GetDiagnostics()};
-
+  for (const OutputArtifact& artifact : options.apk_artifacts) {
     FilterChain filters;
 
-    if (!artifact.name && !config.artifact_format) {
-      diag.Error(
-          DiagMessage() << "Artifact does not have a name and no global name template defined");
-      return false;
-    }
-
-    Maybe<std::string> maybe_artifact_name =
-        (artifact.name) ? artifact.Name(apk_name, &diag)
-                        : artifact.ToArtifactName(config.artifact_format.value(), apk_name, &diag);
-
-    if (!maybe_artifact_name) {
-      diag.Error(DiagMessage() << "Could not determine split APK artifact name");
-      return false;
-    }
-
-    const std::string& artifact_name = maybe_artifact_name.value();
-
     ContextWrapper wrapped_context{context_};
-    wrapped_context.SetSource({artifact_name});
+    wrapped_context.SetSource(artifact.name);
 
     if (!options.kept_artifacts.empty()) {
-      const auto& it = artifacts_to_keep.find(artifact_name);
+      const auto& it = artifacts_to_keep.find(artifact.name);
       if (it == artifacts_to_keep.end()) {
-        filtered_artifacts.insert(artifact_name);
+        filtered_artifacts.insert(artifact.name);
         if (context_->IsVerbose()) {
-          context_->GetDiagnostics()->Note(DiagMessage(artifact_name) << "skipping artifact");
+          context_->GetDiagnostics()->Note(DiagMessage(artifact.name) << "skipping artifact");
         }
         continue;
       } else {
         artifacts_to_keep.erase(it);
-        kept_artifacts.insert(artifact_name);
+        kept_artifacts.insert(artifact.name);
       }
     }
 
     std::unique_ptr<ResourceTable> table =
-        FilterTable(artifact, config, *apk_->GetResourceTable(), &wrapped_context, &filters);
+        FilterTable(context_, artifact, *apk_->GetResourceTable(), &filters);
     if (!table) {
       return false;
     }
 
+    IDiagnostics* diag = wrapped_context.GetDiagnostics();
+
     std::unique_ptr<XmlResource> manifest;
-    if (!UpdateManifest(artifact, config, &manifest, &diag)) {
-      diag.Error(DiagMessage() << "could not update AndroidManifest.xml for output artifact");
+    if (!UpdateManifest(artifact, &manifest, diag)) {
+      diag->Error(DiagMessage() << "could not update AndroidManifest.xml for output artifact");
       return false;
     }
 
     std::string out = options.out_dir;
     if (!file::mkdirs(out)) {
-      diag.Warn(DiagMessage() << "could not create out dir: " << out);
+      diag->Warn(DiagMessage() << "could not create out dir: " << out);
     }
-    file::AppendPath(&out, artifact_name);
+    file::AppendPath(&out, artifact.name);
 
     if (context_->IsVerbose()) {
-      diag.Note(DiagMessage() << "Generating split: " << out);
+      diag->Note(DiagMessage() << "Generating split: " << out);
     }
 
-    std::unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(&diag, out);
+    std::unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(diag, out);
 
     if (context_->IsVerbose()) {
-      diag.Note(DiagMessage() << "Writing output: " << out);
+      diag->Note(DiagMessage() << "Writing output: " << out);
     }
 
     filters.AddFilter(util::make_unique<SignatureFilter>());
@@ -254,64 +211,34 @@ bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) {
   return true;
 }
 
-std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(
-    const configuration::Artifact& artifact,
-    const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table,
-    IAaptContext* context, FilterChain* filters) {
+std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(IAaptContext* context,
+                                                              const OutputArtifact& artifact,
+                                                              const ResourceTable& old_table,
+                                                              FilterChain* filters) {
   TableSplitterOptions splits;
   AxisConfigFilter axis_filter;
   ContextWrapper wrapped_context{context};
+  wrapped_context.SetSource(artifact.name);
 
-  if (artifact.abi_group) {
-    const std::string& group_name = artifact.abi_group.value();
-
-    auto group = config.abi_groups.find(group_name);
-    // TODO: Remove validation when configuration parser ensures referential integrity.
-    if (group == config.abi_groups.end()) {
-      context->GetDiagnostics()->Error(DiagMessage() << "could not find referenced ABI group '"
-                                                     << group_name << "'");
-      return {};
-    }
-    filters->AddFilter(AbiFilter::FromAbiList(group->second));
+  if (!artifact.abis.empty()) {
+    filters->AddFilter(AbiFilter::FromAbiList(artifact.abis));
   }
 
-  if (artifact.screen_density_group) {
-    const std::string& group_name = artifact.screen_density_group.value();
-
-    auto group = config.screen_density_groups.find(group_name);
-    // TODO: Remove validation when configuration parser ensures referential integrity.
-    if (group == config.screen_density_groups.end()) {
-      context->GetDiagnostics()->Error(DiagMessage()
-                                       << "could not find referenced group '" << group_name << "'");
-      return {};
-    }
-
-    const std::vector<ConfigDescription>& densities = group->second;
-    for (const auto& density_config : densities) {
+  if (!artifact.screen_densities.empty()) {
+    for (const auto& density_config : artifact.screen_densities) {
       splits.preferred_densities.push_back(density_config.density);
     }
   }
 
-  if (artifact.locale_group) {
-    const std::string& group_name = artifact.locale_group.value();
-    auto group = config.locale_groups.find(group_name);
-    // TODO: Remove validation when configuration parser ensures referential integrity.
-    if (group == config.locale_groups.end()) {
-      context->GetDiagnostics()->Error(DiagMessage()
-                                       << "could not find referenced group '" << group_name << "'");
-      return {};
-    }
-
-    const std::vector<ConfigDescription>& locales = group->second;
-    for (const auto& locale : locales) {
+  if (!artifact.locales.empty()) {
+    for (const auto& locale : artifact.locales) {
       axis_filter.AddConfig(locale);
     }
     splits.config_filter = &axis_filter;
   }
 
-  Maybe<AndroidSdk> sdk = GetAndroidSdk(artifact, config, context->GetDiagnostics());
-  if (sdk && sdk.value().min_sdk_version) {
-    wrapped_context.SetMinSdkVersion(sdk.value().min_sdk_version.value());
+  if (artifact.android_sdk && artifact.android_sdk.value().min_sdk_version) {
+    wrapped_context.SetMinSdkVersion(artifact.android_sdk.value().min_sdk_version.value());
   }
 
   std::unique_ptr<ResourceTable> table = old_table.Clone();
@@ -327,8 +254,7 @@ std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(
   return table;
 }
 
-bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
-                                       const PostProcessingConfiguration& config,
+bool MultiApkGenerator::UpdateManifest(const OutputArtifact& artifact,
                                        std::unique_ptr<XmlResource>* updated_manifest,
                                        IDiagnostics* diag) {
   const xml::XmlResource* apk_manifest = apk_->GetManifest();
@@ -367,10 +293,9 @@ bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
   versionCode->compiled_value = ResourceUtils::TryParseInt(std::to_string(new_version));
 
   // Check to see if the minSdkVersion needs to be updated.
-  Maybe<AndroidSdk> maybe_sdk = GetAndroidSdk(artifact, config, diag);
-  if (maybe_sdk) {
+  if (artifact.android_sdk) {
     // TODO(safarmer): Handle the rest of the Android SDK.
-    const AndroidSdk& android_sdk = maybe_sdk.value();
+    const AndroidSdk& android_sdk = artifact.android_sdk.value();
 
     if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) {
       if (xml::Attribute* min_sdk_attr =
@@ -392,10 +317,7 @@ bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
     }
   }
 
-  if (artifact.screen_density_group) {
-    auto densities = config.screen_density_groups.find(artifact.screen_density_group.value());
-    CHECK(densities != config.screen_density_groups.end()) << "Missing density group";
-
+  if (!artifact.screen_densities.empty()) {
     xml::Element* screens_el = manifest_el->FindChild({}, "compatible-screens");
     if (!screens_el) {
       // create a new element.
@@ -408,7 +330,7 @@ bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
       screens_el->GetChildElements().clear();
     }
 
-    for (const auto& density : densities->second) {
+    for (const auto& density : artifact.screen_densities) {
       std::unique_ptr<xml::Element> screen_el = util::make_unique<xml::Element>();
       screen_el->name = "screen";
       const char* density_str = density.toString().string();
diff --git a/tools/aapt2/optimize/MultiApkGenerator.h b/tools/aapt2/optimize/MultiApkGenerator.h
index dedb610e1caa..19f64cc0e588 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.h
+++ b/tools/aapt2/optimize/MultiApkGenerator.h
@@ -20,6 +20,7 @@
 #include <memory>
 #include <string>
 #include <unordered_set>
+#include <vector>
 
 #include "Diagnostics.h"
 #include "LoadedApk.h"
@@ -29,7 +30,7 @@ namespace aapt {
 
 struct MultiApkGeneratorOptions {
   std::string out_dir;
-  configuration::PostProcessingConfiguration config;
+  std::vector<configuration::OutputArtifact> apk_artifacts;
   TableFlattenerOptions table_flattener_options;
   std::unordered_set<std::string> kept_artifacts;
 };
@@ -49,18 +50,17 @@ class MultiApkGenerator {
   bool FromBaseApk(const MultiApkGeneratorOptions& options);
 
  protected:
-  virtual std::unique_ptr<ResourceTable> FilterTable(
-      const configuration::Artifact& artifact,
-      const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table,
-      IAaptContext* context, FilterChain* chain);
+  virtual std::unique_ptr<ResourceTable> FilterTable(IAaptContext* context,
+                                                     const configuration::OutputArtifact& artifact,
+                                                     const ResourceTable& old_table,
+                                                     FilterChain* chain);
 
  private:
   IDiagnostics* GetDiagnostics() {
     return context_->GetDiagnostics();
   }
 
-  bool UpdateManifest(const configuration::Artifact& artifact,
-                      const configuration::PostProcessingConfiguration& config,
+  bool UpdateManifest(const configuration::OutputArtifact& artifact,
                       std::unique_ptr<xml::XmlResource>* updated_manifest, IDiagnostics* diag);
 
   LoadedApk* apk_;
diff --git a/tools/aapt2/optimize/MultiApkGenerator_test.cpp b/tools/aapt2/optimize/MultiApkGenerator_test.cpp
index 2d303e220f8f..e1d951fc9776 100644
--- a/tools/aapt2/optimize/MultiApkGenerator_test.cpp
+++ b/tools/aapt2/optimize/MultiApkGenerator_test.cpp
@@ -36,8 +36,7 @@ namespace {
 
 using ::aapt::configuration::Abi;
 using ::aapt::configuration::AndroidSdk;
-using ::aapt::configuration::Artifact;
-using ::aapt::configuration::PostProcessingConfiguration;
+using ::aapt::configuration::OutputArtifact;
 using ::aapt::test::GetValue;
 using ::aapt::test::GetValueForConfig;
 using ::aapt::test::ParseConfigOrDie;
@@ -48,7 +47,6 @@ using ::testing::NotNull;
 using ::testing::PrintToString;
 using ::testing::Return;
 using ::testing::Test;
-using ::testing::_;
 
 /**
  * Subclass the MultiApkGenerator class so that we can access the protected FilterTable method to
@@ -60,11 +58,11 @@ class MultiApkGeneratorWrapper : public MultiApkGenerator {
       : MultiApkGenerator(apk, context) {
   }
 
-  std::unique_ptr<ResourceTable> FilterTable(
-      const configuration::Artifact& artifact,
-      const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table,
-      IAaptContext* context, FilterChain* filter_chain) override {
-    return MultiApkGenerator::FilterTable(artifact, config, old_table, context, filter_chain);
+  std::unique_ptr<ResourceTable> FilterTable(IAaptContext* context,
+                                             const configuration::OutputArtifact& artifact,
+                                             const ResourceTable& old_table,
+                                             FilterChain* filter_chain) override {
+    return MultiApkGenerator::FilterTable(context, artifact, old_table, filter_chain);
   }
 };
 
@@ -108,27 +106,13 @@ TEST_F(MultiApkGeneratorTest, VersionFilterNewerVersion) {
 
   LoadedApk apk = {{"test.apk"}, {}, std::move(table), {}};
   std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(19).Build();
-  PostProcessingConfiguration empty_config;
-  TableFlattenerOptions table_flattener_options;
   FilterChain chain;
 
-  Artifact x64 = test::ArtifactBuilder()
-                     .SetName("${basename}.${sdk}.apk")
-                     .SetDensityGroup("xhdpi")
-                     .SetAndroidSdk("v23")
-                     .Build();
-
-  auto config = test::PostProcessingConfigurationBuilder()
-                    .SetLocaleGroup("en", {"en"})
-                    .SetAbiGroup("x64", {Abi::kX86_64})
-                    .SetDensityGroup("xhdpi", {"xhdpi"})
-                    .SetAndroidSdk("v23", AndroidSdk::ForMinSdk(23))
-                    .AddArtifact(x64)
-                    .Build();
+  OutputArtifact artifact = test::ArtifactBuilder().AddDensity(xhdpi_).SetAndroidSdk(23).Build();
 
   MultiApkGeneratorWrapper generator{&apk, ctx.get()};
   std::unique_ptr<ResourceTable> split =
-      generator.FilterTable(x64, config, *apk.GetResourceTable(), ctx.get(), &chain);
+      generator.FilterTable(ctx.get(), artifact, *apk.GetResourceTable(), &chain);
 
   ResourceTable* new_table = split.get();
   EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull());
@@ -149,27 +133,13 @@ TEST_F(MultiApkGeneratorTest, VersionFilterOlderVersion) {
 
   LoadedApk apk = {{"test.apk"}, {}, std::move(table), {}};
   std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(1).Build();
-  PostProcessingConfiguration empty_config;
-  TableFlattenerOptions table_flattener_options;
   FilterChain chain;
 
-  Artifact x64 = test::ArtifactBuilder()
-                     .SetName("${basename}.${sdk}.apk")
-                     .SetDensityGroup("xhdpi")
-                     .SetAndroidSdk("v4")
-                     .Build();
-
-  auto config = test::PostProcessingConfigurationBuilder()
-                    .SetLocaleGroup("en", {"en"})
-                    .SetAbiGroup("x64", {Abi::kX86_64})
-                    .SetDensityGroup("xhdpi", {"xhdpi"})
-                    .SetAndroidSdk("v4", AndroidSdk::ForMinSdk(4))
-                    .AddArtifact(x64)
-                    .Build();
+  OutputArtifact artifact = test::ArtifactBuilder().AddDensity(xhdpi_).SetAndroidSdk(4).Build();
 
   MultiApkGeneratorWrapper generator{&apk, ctx.get()};;
   std::unique_ptr<ResourceTable> split =
-      generator.FilterTable(x64, config, *apk.GetResourceTable(), ctx.get(), &chain);
+      generator.FilterTable(ctx.get(), artifact, *apk.GetResourceTable(), &chain);
 
   ResourceTable* new_table = split.get();
   EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull());
@@ -188,23 +158,13 @@ TEST_F(MultiApkGeneratorTest, VersionFilterNoVersion) {
 
   LoadedApk apk = {{"test.apk"}, {}, std::move(table), {}};
   std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(1).Build();
-  PostProcessingConfiguration empty_config;
-  TableFlattenerOptions table_flattener_options;
   FilterChain chain;
 
-  Artifact x64 =
-      test::ArtifactBuilder().SetName("${basename}.${sdk}.apk").SetDensityGroup("xhdpi").Build();
-
-  auto config = test::PostProcessingConfigurationBuilder()
-                    .SetLocaleGroup("en", {"en"})
-                    .SetAbiGroup("x64", {Abi::kX86_64})
-                    .SetDensityGroup("xhdpi", {"xhdpi"})
-                    .AddArtifact(x64)
-                    .Build();
+  OutputArtifact artifact = test::ArtifactBuilder().AddDensity(xhdpi_).Build();
 
   MultiApkGeneratorWrapper generator{&apk, ctx.get()};
   std::unique_ptr<ResourceTable> split =
-      generator.FilterTable(x64, config, *apk.GetResourceTable(), ctx.get(), &chain);
+      generator.FilterTable(ctx.get(), artifact, *apk.GetResourceTable(), &chain);
 
   ResourceTable* new_table = split.get();
   EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull());