From 42e89508eeb7d24bac5eb71b48aa89fbf5ed8aee Mon Sep 17 00:00:00 2001
From: Spandan Das <spandandas@google.com>
Date: Fri, 6 May 2022 22:12:55 +0000
Subject: [PATCH] Enforce updatable=true on apps of updatable apexes

- Update apex_info (a topdown mutator) so that it sets updatable=true on
  apps of updatable apexes
- Write a unit test that tests different combinations of
  updatable/non-updatable apks-in-apexes
- Update an existing unit test that asserts a different error

Test: go test ./java
Test: m nothing (in internal)
Bug: 209409604

Change-Id: Ie8881b857afcec44addf27fc360c5b8abf726bd2
---
 apex/apex.go      | 19 +++++++++++++-
 apex/apex_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 java/app.go       |  4 +++
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/apex/apex.go b/apex/apex.go
index a7b0a4fe2..4bcba7e43 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -994,6 +994,7 @@ type ApexInfoMutator interface {
 
 // apexInfoMutator delegates the work of identifying which modules need an ApexInfo and apex
 // specific variant to modules that support the ApexInfoMutator.
+// It also propagates updatable=true to apps of updatable apexes
 func apexInfoMutator(mctx android.TopDownMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
@@ -1001,8 +1002,8 @@ func apexInfoMutator(mctx android.TopDownMutatorContext) {
 
 	if a, ok := mctx.Module().(ApexInfoMutator); ok {
 		a.ApexInfoMutator(mctx)
-		return
 	}
+	enforceAppUpdatability(mctx)
 }
 
 // apexStrictUpdatibilityLintMutator propagates strict_updatability_linting to transitive deps of a mainline module
@@ -1033,6 +1034,22 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) {
 	}
 }
 
+// enforceAppUpdatability propagates updatable=true to apps of updatable apexes
+func enforceAppUpdatability(mctx android.TopDownMutatorContext) {
+	if !mctx.Module().Enabled() {
+		return
+	}
+	if apex, ok := mctx.Module().(*apexBundle); ok && apex.Updatable() {
+		// checking direct deps is sufficient since apex->apk is a direct edge, even when inherited via apex_defaults
+		mctx.VisitDirectDeps(func(module android.Module) {
+			// ignore android_test_app
+			if app, ok := module.(*java.AndroidApp); ok {
+				app.SetUpdatable(true)
+			}
+		})
+	}
+}
+
 // TODO: b/215736885 Whittle the denylist
 // Transitive deps of certain mainline modules baseline NewApi errors
 // Skip these mainline modules for now
diff --git a/apex/apex_test.go b/apex/apex_test.go
index 77cbb58c3..bb3556c3c 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -2393,6 +2393,7 @@ func TestApexMinSdkVersion_ErrorIfDepIsNewer_Java(t *testing.T) {
 			key: "myapex.key",
 			apps: ["AppFoo"],
 			min_sdk_version: "29",
+			updatable: false,
 		}
 
 		apex_key {
@@ -9324,6 +9325,69 @@ func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) {
 	}
 }
 
+// updatable apexes should propagate updatable=true to its apps
+func TestUpdatableApexEnforcesAppUpdatability(t *testing.T) {
+	bp := `
+		apex {
+			name: "myapex",
+			key: "myapex.key",
+			updatable: %v,
+			apps: [
+				"myapp",
+			],
+			min_sdk_version: "30",
+		}
+		apex_key {
+			name: "myapex.key",
+		}
+		android_app {
+			name: "myapp",
+			updatable: %v,
+			apex_available: [
+				"myapex",
+			],
+			sdk_version: "current",
+			min_sdk_version: "30",
+		}
+		`
+	testCases := []struct {
+		name                      string
+		apex_is_updatable_bp      bool
+		app_is_updatable_bp       bool
+		app_is_updatable_expected bool
+	}{
+		{
+			name:                      "Non-updatable apex respects updatable property of non-updatable app",
+			apex_is_updatable_bp:      false,
+			app_is_updatable_bp:       false,
+			app_is_updatable_expected: false,
+		},
+		{
+			name:                      "Non-updatable apex respects updatable property of updatable app",
+			apex_is_updatable_bp:      false,
+			app_is_updatable_bp:       true,
+			app_is_updatable_expected: true,
+		},
+		{
+			name:                      "Updatable apex respects updatable property of updatable app",
+			apex_is_updatable_bp:      true,
+			app_is_updatable_bp:       true,
+			app_is_updatable_expected: true,
+		},
+		{
+			name:                      "Updatable apex sets updatable=true on non-updatable app",
+			apex_is_updatable_bp:      true,
+			app_is_updatable_bp:       false,
+			app_is_updatable_expected: true,
+		},
+	}
+	for _, testCase := range testCases {
+		result := testApex(t, fmt.Sprintf(bp, testCase.apex_is_updatable_bp, testCase.app_is_updatable_bp))
+		myapp := result.ModuleForTests("myapp", "android_common").Module().(*java.AndroidApp)
+		android.AssertBoolEquals(t, testCase.name, testCase.app_is_updatable_expected, myapp.Updatable())
+	}
+}
+
 func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
diff --git a/java/app.go b/java/app.go
index 036b4b544..3b512f1d3 100755
--- a/java/app.go
+++ b/java/app.go
@@ -838,6 +838,10 @@ func (a *AndroidApp) Updatable() bool {
 	return Bool(a.appProperties.Updatable)
 }
 
+func (a *AndroidApp) SetUpdatable(val bool) {
+	a.appProperties.Updatable = &val
+}
+
 func (a *AndroidApp) getCertString(ctx android.BaseModuleContext) string {
 	certificate, overridden := ctx.DeviceConfig().OverrideCertificateFor(ctx.ModuleName())
 	if overridden {