Fix Valid<T> to contain non nullable value

This corrects a design mistake: the Valid<T> subclass of
ValidationResult should have a non-null value, so the value
can be taken directly after a smart-cast.

Usage becomes more concise:

when (val result = validateInput(source) {
    is Valid -> processValue(result.value) // also result.warnings
    is Invalid -> handleInvalid(result.errors)
}

Bug: 309960444
Test: atest com.android.intentresolver.v2.validation
Change-Id: Ia4ee53413d729e551b8b7ec21a8765ae7d4f5e95

10 files changed, 85 insertions, 74 deletions

diff --git a/java/src/com/android/intentresolver/contentpreview/SelectionChangeCallback.kt b/java/src/com/android/intentresolver/contentpreview/SelectionChangeCallback.kt
index 5c916882..6b33e1cd 100644
--- a/java/src/com/android/intentresolver/contentpreview/SelectionChangeCallback.kt
+++ b/java/src/com/android/intentresolver/contentpreview/SelectionChangeCallback.kt
@@ -31,7 +31,10 @@ import android.service.chooser.ChooserTarget
 import com.android.intentresolver.contentpreview.PayloadToggleInteractor.ShareouselUpdate
 import com.android.intentresolver.v2.ui.viewmodel.readAlternateIntents
 import com.android.intentresolver.v2.ui.viewmodel.readChooserActions
+import com.android.intentresolver.v2.validation.Invalid
+import com.android.intentresolver.v2.validation.Valid
 import com.android.intentresolver.v2.validation.ValidationResult
+import com.android.intentresolver.v2.validation.log
 import com.android.intentresolver.v2.validation.types.array
 import com.android.intentresolver.v2.validation.types.value
 import com.android.intentresolver.v2.validation.validateFrom
@@ -61,11 +64,10 @@ class SelectionChangeCallback(
                 }
             )
             ?.let { bundle ->
-                readCallbackResponse(bundle).let { validation ->
-                    if (validation.isSuccess()) {
-                        validation.value
-                    } else {
-                        validation.reportToLogcat(TAG)
+                return when (val result = readCallbackResponse(bundle)) {
+                    is Valid -> result.value
+                    is Invalid -> {
+                        result.errors.forEach { it.log(TAG) }
                         null
                     }
                 }
diff --git a/java/src/com/android/intentresolver/v2/ResolverActivity.java b/java/src/com/android/intentresolver/v2/ResolverActivity.java
index 98e82b00..52d5f2de 100644
--- a/java/src/com/android/intentresolver/v2/ResolverActivity.java
+++ b/java/src/com/android/intentresolver/v2/ResolverActivity.java
@@ -115,6 +115,10 @@ import com.android.intentresolver.v2.profiles.ResolverMultiProfilePagerAdapter;
 import com.android.intentresolver.v2.ui.ActionTitle;
 import com.android.intentresolver.v2.ui.model.ActivityModel;
 import com.android.intentresolver.v2.ui.model.ResolverRequest;
+import com.android.intentresolver.v2.validation.Finding;
+import com.android.intentresolver.v2.validation.FindingsKt;
+import com.android.intentresolver.v2.validation.Invalid;
+import com.android.intentresolver.v2.validation.Valid;
 import com.android.intentresolver.v2.validation.ValidationResult;
 import com.android.intentresolver.widget.ResolverDrawerLayout;
 import com.android.internal.annotations.VisibleForTesting;
@@ -135,6 +139,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import javax.inject.Inject;
 
@@ -243,11 +248,16 @@ public class ResolverActivity extends Hilt_ResolverActivity implements
         }
 
         ValidationResult<ResolverRequest> result = readResolverRequest(mActivityModel);
-        if (!result.isSuccess()) {
-            result.reportToLogcat(TAG);
+        if (result instanceof Invalid) {
+            ((Invalid) result).getErrors().forEach(new Consumer<Finding>() {
+                @Override
+                public void accept(Finding finding) {
+                    FindingsKt.log(finding, TAG);
+                }
+            });
             finish();
         }
-        mResolverRequest = result.getOrThrow();
+        mResolverRequest = ((Valid<ResolverRequest>) result).getValue();
         mLogic = createActivityLogic();
         mResolvingHome = mResolverRequest.isResolvingHome();
         mTargetDataLoader = new DefaultTargetDataLoader(
diff --git a/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt b/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt
index cd1a16e3..424f36cd 100644
--- a/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt
+++ b/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt
@@ -22,7 +22,10 @@ import com.android.intentresolver.inject.ChooserServiceFlags
 import com.android.intentresolver.v2.ui.model.ActivityModel
 import com.android.intentresolver.v2.ui.model.ActivityModel.Companion.ACTIVITY_MODEL_KEY
 import com.android.intentresolver.v2.ui.model.ChooserRequest
+import com.android.intentresolver.v2.validation.Invalid
+import com.android.intentresolver.v2.validation.Valid
 import com.android.intentresolver.v2.validation.ValidationResult
+import com.android.intentresolver.v2.validation.log
 import dagger.hilt.android.lifecycle.HiltViewModel
 import javax.inject.Inject
 
@@ -45,12 +48,17 @@ constructor(
     private val status: ValidationResult<ChooserRequest> =
         readChooserRequest(mActivityModel, flags)
 
-    val chooserRequest: ChooserRequest by lazy { status.getOrThrow() }
+    val chooserRequest: ChooserRequest by lazy {
+        when(status) {
+            is Valid -> status.value
+            is Invalid -> error(status.errors)
+        }
+    }
 
     fun init(): Boolean {
         Log.i(TAG, "viewModel init")
-        if (!status.isSuccess()) {
-            status.reportToLogcat(TAG)
+        if (status is Invalid) {
+            status.errors.forEach { finding -> finding.log(TAG) }
             return false
         }
         Log.i(TAG, "request = $chooserRequest")
diff --git a/java/src/com/android/intentresolver/v2/validation/Findings.kt b/java/src/com/android/intentresolver/v2/validation/Findings.kt
index 9a3cc9c7..bdf2f00a 100644
--- a/java/src/com/android/intentresolver/v2/validation/Findings.kt
+++ b/java/src/com/android/intentresolver/v2/validation/Findings.kt
@@ -34,9 +34,13 @@ val Finding.logcatPriority
     get() =
         when (importance) {
             CRITICAL -> Log.ERROR
-            else -> Log.WARN
+            WARNING -> Log.WARN
         }
 
+fun Finding.log(tag: String) {
+    Log.println(logcatPriority, tag, message)
+}
+
 private fun formatMessage(key: String? = null, msg: String) = buildString {
     key?.also { append("['$key']: ") }
     append(msg)
@@ -52,18 +56,21 @@ data class IgnoredValue(
         get() = formatMessage(key, "Ignored. $reason")
 }
 
-data class RequiredValueMissing(
+data class NoValue(
     val key: String,
+    override val importance: Importance,
     val allowedType: KClass<*>,
 ) : Finding {
 
-    override val importance = CRITICAL
-
     override val message: String
         get() =
             formatMessage(
                 key,
-                "expected value of ${allowedType.simpleName}, " + "but no value was present"
+                if (importance == CRITICAL) {
+                    "expected value of ${allowedType.simpleName}, " + "but no value was present"
+                } else {
+                    "no ${allowedType.simpleName} value present"
+                }
             )
 }
 
diff --git a/java/src/com/android/intentresolver/v2/validation/Validation.kt b/java/src/com/android/intentresolver/v2/validation/Validation.kt
index 46939602..6072ec9f 100644
--- a/java/src/com/android/intentresolver/v2/validation/Validation.kt
+++ b/java/src/com/android/intentresolver/v2/validation/Validation.kt
@@ -90,7 +90,7 @@ fun <T> validateFrom(source: (String) -> Any?, validate: Validation.() -> T): Va
                     is InvalidResultError -> Invalid(validation.findings)
 
                     // Some other exception was thrown from [validate],
-                    else -> Invalid(findings = listOf(UncaughtException(it)))
+                    else -> Invalid(error = UncaughtException(it))
                 }
             }
         )
@@ -107,8 +107,8 @@ private class ValidationImpl(val source: (String) -> Any?) : Validation {
 
     override fun <T> ignored(property: Validator<T>, reason: String) {
         val result = property.validate(source, WARNING)
-        if (result.value != null) {
-            // Note: Any findings about the value (result.findings) are ignored.
+        if (result is Valid) {
+            // Note: Any warnings about the value itself (result.findings) are ignored.
             findings += IgnoredValue(property.key, reason)
         }
     }
@@ -117,8 +117,16 @@ private class ValidationImpl(val source: (String) -> Any?) : Validation {
         return runCatching { property.validate(source, importance) }
             .fold(
                 onSuccess = { result ->
-                    findings += result.findings
-                    result.value
+                    return when (result) {
+                        is Valid -> {
+                            findings += result.warnings
+                            result.value
+                        }
+                        is Invalid -> {
+                            findings += result.errors
+                            null
+                        }
+                    }
                 },
                 onFailure = {
                     findings += UncaughtException(it, property.key)
diff --git a/java/src/com/android/intentresolver/v2/validation/ValidationResult.kt b/java/src/com/android/intentresolver/v2/validation/ValidationResult.kt
index 856a521e..f5c467dc 100644
--- a/java/src/com/android/intentresolver/v2/validation/ValidationResult.kt
+++ b/java/src/com/android/intentresolver/v2/validation/ValidationResult.kt
@@ -15,25 +15,12 @@
  */
 package com.android.intentresolver.v2.validation
 
-import android.util.Log
+sealed interface ValidationResult<T>
 
-sealed interface ValidationResult<T> {
-    val value: T?
-    val findings: List<Finding>
-
-    fun isSuccess() = value != null
-
-    fun getOrThrow(): T =
-        checkNotNull(value) { "The result was invalid: " + findings.joinToString(separator = "\n") }
-
-    fun reportToLogcat(tag: String) {
-        findings.forEach { Log.println(it.logcatPriority, tag, it.toString()) }
-    }
+data class Valid<T>(val value: T, val warnings: List<Finding> = emptyList()) : ValidationResult<T> {
+    constructor(value: T, warning: Finding) : this(value, listOf(warning))
 }
 
-data class Valid<T>(override val value: T?, override val findings: List<Finding> = emptyList()) :
-    ValidationResult<T>
-
-data class Invalid<T>(override val findings: List<Finding>) : ValidationResult<T> {
-    override val value: T? = null
+data class Invalid<T>(val errors: List<Finding> = emptyList()) : ValidationResult<T> {
+    constructor(error: Finding) : this(listOf(error))
 }
diff --git a/java/src/com/android/intentresolver/v2/validation/types/IntentOrUri.kt b/java/src/com/android/intentresolver/v2/validation/types/IntentOrUri.kt
index 3cefeb15..050bd895 100644
--- a/java/src/com/android/intentresolver/v2/validation/types/IntentOrUri.kt
+++ b/java/src/com/android/intentresolver/v2/validation/types/IntentOrUri.kt
@@ -18,7 +18,8 @@ package com.android.intentresolver.v2.validation.types
 import android.content.Intent
 import android.net.Uri
 import com.android.intentresolver.v2.validation.Importance
-import com.android.intentresolver.v2.validation.RequiredValueMissing
+import com.android.intentresolver.v2.validation.Invalid
+import com.android.intentresolver.v2.validation.NoValue
 import com.android.intentresolver.v2.validation.Valid
 import com.android.intentresolver.v2.validation.ValidationResult
 import com.android.intentresolver.v2.validation.Validator
@@ -40,12 +41,14 @@ class IntentOrUri(override val key: String) : Validator<Intent> {
             is Uri -> Valid(Intent.parseUri(value.toString(), Intent.URI_INTENT_SCHEME))
 
             // No value present.
-            null -> createResult(importance, RequiredValueMissing(key, Intent::class))
+            null -> when (importance) {
+                Importance.WARNING -> Invalid() // No warnings if optional, but missing
+                Importance.CRITICAL -> Invalid(NoValue(key, importance, Intent::class))
+            }
 
             // Some other type.
             else -> {
-                return createResult(
-                    importance,
+                return Invalid(
                     ValueIsWrongType(
                         key,
                         importance,
diff --git a/java/src/com/android/intentresolver/v2/validation/types/ParceledArray.kt b/java/src/com/android/intentresolver/v2/validation/types/ParceledArray.kt
index c6c4abba..78adfd36 100644
--- a/java/src/com/android/intentresolver/v2/validation/types/ParceledArray.kt
+++ b/java/src/com/android/intentresolver/v2/validation/types/ParceledArray.kt
@@ -15,8 +15,10 @@
  */
 package com.android.intentresolver.v2.validation.types
 
+import android.content.Intent
 import com.android.intentresolver.v2.validation.Importance
-import com.android.intentresolver.v2.validation.RequiredValueMissing
+import com.android.intentresolver.v2.validation.Invalid
+import com.android.intentresolver.v2.validation.NoValue
 import com.android.intentresolver.v2.validation.Valid
 import com.android.intentresolver.v2.validation.ValidationResult
 import com.android.intentresolver.v2.validation.Validator
@@ -37,8 +39,10 @@ class ParceledArray<T : Any>(
 
         return when (val value: Any? = source(key)) {
             // No value present.
-            null -> createResult(importance, RequiredValueMissing(key, elementType))
-
+            null -> when (importance) {
+                Importance.WARNING -> Invalid() // No warnings if optional, but missing
+                Importance.CRITICAL -> Invalid(NoValue(key, importance, elementType))
+            }
             // A parcel does not transfer the element type information for parcelable
             // arrays. This leads to a restored type of Array<Parcelable>, which is
             // incompatible with Array<T : Parcelable>.
@@ -54,8 +58,7 @@ class ParceledArray<T : Any>(
 
                     // At least one incorrect element type found.
                     else ->
-                        createResult(
-                            importance,
+                        Invalid(
                             WrongElementType(
                                 key,
                                 importance,
@@ -69,8 +72,7 @@ class ParceledArray<T : Any>(
 
             // The value is not an Array at all.
             else ->
-                createResult(
-                    importance,
+                Invalid(
                     ValueIsWrongType(
                         key,
                         importance,
diff --git a/java/src/com/android/intentresolver/v2/validation/types/SimpleValue.kt b/java/src/com/android/intentresolver/v2/validation/types/SimpleValue.kt
index 3287b84b..0105541d 100644
--- a/java/src/com/android/intentresolver/v2/validation/types/SimpleValue.kt
+++ b/java/src/com/android/intentresolver/v2/validation/types/SimpleValue.kt
@@ -16,7 +16,8 @@
 package com.android.intentresolver.v2.validation.types
 
 import com.android.intentresolver.v2.validation.Importance
-import com.android.intentresolver.v2.validation.RequiredValueMissing
+import com.android.intentresolver.v2.validation.Invalid
+import com.android.intentresolver.v2.validation.NoValue
 import com.android.intentresolver.v2.validation.Valid
 import com.android.intentresolver.v2.validation.ValidationResult
 import com.android.intentresolver.v2.validation.Validator
@@ -36,19 +37,21 @@ class SimpleValue<T : Any>(
             expected.isInstance(value) -> return Valid(expected.cast(value))
 
             // No value is present.
-            value == null -> createResult(importance, RequiredValueMissing(key, expected))
+            value == null -> when (importance) {
+                Importance.WARNING -> Invalid() // No warnings if optional, but missing
+                Importance.CRITICAL -> Invalid(NoValue(key, importance, expected))
+            }
 
             // The value is some other type.
             else ->
-                createResult(
-                    importance,
+                Invalid(listOf(
                     ValueIsWrongType(
                         key,
                         importance,
                         actualType = value::class,
                         allowedTypes = listOf(expected)
                     )
-                )
+                ))
         }
     }
 }
diff --git a/java/src/com/android/intentresolver/v2/validation/types/Validators.kt b/java/src/com/android/intentresolver/v2/validation/types/Validators.kt
index 4e6e5dff..70993b4d 100644
--- a/java/src/com/android/intentresolver/v2/validation/types/Validators.kt
+++ b/java/src/com/android/intentresolver/v2/validation/types/Validators.kt
@@ -15,13 +15,6 @@
  */
 package com.android.intentresolver.v2.validation.types
 
-import com.android.intentresolver.v2.validation.Finding
-import com.android.intentresolver.v2.validation.Importance
-import com.android.intentresolver.v2.validation.Importance.CRITICAL
-import com.android.intentresolver.v2.validation.Importance.WARNING
-import com.android.intentresolver.v2.validation.Invalid
-import com.android.intentresolver.v2.validation.Valid
-import com.android.intentresolver.v2.validation.ValidationResult
 import com.android.intentresolver.v2.validation.Validator
 
 inline fun <reified T : Any> value(key: String): Validator<T> {
@@ -31,15 +24,3 @@ inline fun <reified T : Any> value(key: String): Validator<T> {
 inline fun <reified T : Any> array(key: String): Validator<List<T>> {
     return ParceledArray(key, T::class)
 }
-
-/**
- * Convenience function to wrap a finding in an appropriate result type.
- *
- * An error [finding] is suppressed when [importance] == [WARNING]
- */
-internal fun <T> createResult(importance: Importance, finding: Finding): ValidationResult<T> {
-    return when (importance) {
-        WARNING -> Valid(null, listOf(finding).filter { it.importance == WARNING })
-        CRITICAL -> Invalid(listOf(finding))
-    }
-}