Validate merged Config before returning from ConfigParser
Previously, `ConfigParser` would just return the merged `Config` object
and leave it up to properties, e.g. `ApiSurfacesConfig.byName` to
perform any validation that was required. However, that meant it was
possible for some code to access `Config` objects in an invalid state
through other properties, e.g. `apiSurfaceList`.
This change performs validation on the merged `Config` object before it
is returned by `ConfigParser`, ensuring that it is valid before any
other code access it.
Bug: 391554590
Test: ./gradlew
Change-Id: I4ed1eb76af294367559a232ed8fca10b716f58fc
diff --git a/metalava/src/main/java/com/android/tools/metalava/config/Config.kt b/metalava/src/main/java/com/android/tools/metalava/config/Config.kt
index d12b249..995d4fb 100644
--- a/metalava/src/main/java/com/android/tools/metalava/config/Config.kt
+++ b/metalava/src/main/java/com/android/tools/metalava/config/Config.kt
@@ -28,7 +28,12 @@
data class Config(
@field:JacksonXmlProperty(localName = "api-surfaces", namespace = CONFIG_NAMESPACE)
val apiSurfaces: ApiSurfacesConfig? = null,
-)
+) {
+ /** Validate this object, i.e. check to make sure that the contained objects are consistent. */
+ internal fun validate() {
+ apiSurfaces?.validate()
+ }
+}
/** A set of [ApiSurfaceConfig]s. */
data class ApiSurfacesConfig(
@@ -49,6 +54,12 @@
error("Found duplicate surfaces called `$name`")
}
}
+
+ /** Validate this object, i.e. check to make sure that the contained objects are consistent. */
+ fun validate() {
+ // Force check for duplicates.
+ byName
+ }
}
/** An API surface that Metalava could generate. */
diff --git a/metalava/src/main/java/com/android/tools/metalava/config/ConfigParser.kt b/metalava/src/main/java/com/android/tools/metalava/config/ConfigParser.kt
index 9676543..6fc9958 100644
--- a/metalava/src/main/java/com/android/tools/metalava/config/ConfigParser.kt
+++ b/metalava/src/main/java/com/android/tools/metalava/config/ConfigParser.kt
@@ -103,6 +103,8 @@
}
// Merge the config objects together.
.reduceOrNull { configLeft, configRight -> merge(configLeft, configRight) }
+ // Validate the config.
+ ?.apply { validate() }
// If no configuration files were created then return an empty Config.
?: Config()
}
diff --git a/metalava/src/test/java/com/android/tools/metalava/config/ConfigParserTest.kt b/metalava/src/test/java/com/android/tools/metalava/config/ConfigParserTest.kt
index f1089c8..eee7490 100644
--- a/metalava/src/test/java/com/android/tools/metalava/config/ConfigParserTest.kt
+++ b/metalava/src/test/java/com/android/tools/metalava/config/ConfigParserTest.kt
@@ -21,7 +21,6 @@
import com.android.tools.metalava.testing.TemporaryFolderOwner
import com.google.common.truth.Truth.assertThat
import org.intellij.lang.annotations.Language
-import org.junit.Assert.assertThrows
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
@@ -343,28 +342,7 @@
</config>
""",
),
- ) {
- assertThat(config)
- .isEqualTo(
- Config(
- apiSurfaces =
- ApiSurfacesConfig(
- apiSurfaceList =
- listOf(
- ApiSurfaceConfig(
- name = "public",
- ),
- ApiSurfaceConfig(
- name = "public",
- ),
- ),
- ),
- )
- )
-
- val exception =
- assertThrows(IllegalStateException::class.java) { config.apiSurfaces?.byName }
- assertThat(exception.message).isEqualTo("Found duplicate surfaces called `public`")
- }
+ expectedFail = "Found duplicate surfaces called `public`"
+ )
}
}
