Skip to content

Commit 9ffdae0

Browse files
fryanpanclaude
andcommitted
ADFA-4128 (7/11): address CodeRabbit review
- F1719-1 read setup.json arrays type-checked, so parse returns null instead of throwing - F1719-4 drop the dead telemetry.report import from QuickBuildProjectLayoutTest Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7
1 parent 857a764 commit 9ffdae0

3 files changed

Lines changed: 59 additions & 6 deletions

File tree

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProxyAppInfo.kt‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.appdevforall.cotg.quickbuild.data
22

3+
import com.google.gson.JsonArray
34
import com.google.gson.JsonObject
45
import com.google.gson.JsonParser
56
import org.appdevforall.cotg.quickbuild.domain.reload.ComponentInfo
@@ -140,15 +141,15 @@ data class ProxyAppInfo(
140141

141142
val classpath =
142143
obj
143-
.getAsJsonArray("classpath")
144+
.jsonArray("classpath")
144145
?.mapNotNull { it.takeIf(com.google.gson.JsonElement::isJsonPrimitive)?.asString }
145146
?.map { resolve(it, baseDir) }
146147
?: emptyList()
147148
// Generated project-scope jars (R.jar and kin) ride the compile classpath:
148149
// hot compiles reference R, which the variant compile classpath lacks.
149150
val payloadJars =
150151
obj
151-
.getAsJsonArray("payloadJars")
152+
.jsonArray("payloadJars")
152153
?.mapNotNull { it.takeIf(com.google.gson.JsonElement::isJsonPrimitive)?.asString }
153154
?.map { resolve(it, baseDir) }
154155
?: emptyList()
@@ -175,7 +176,7 @@ data class ProxyAppInfo(
175176
?.asInt ?: 0,
176177
components =
177178
obj
178-
.getAsJsonArray("components")
179+
.jsonArray("components")
179180
?.mapNotNull { element -> (element as? JsonObject)?.let(::parseComponent) }
180181
?: emptyList(),
181182
annotationProcessors = obj.stringArray("annotationProcessors"),
@@ -192,6 +193,20 @@ data class ProxyAppInfo(
192193
)
193194
}
194195

196+
/**
197+
* The JSON array under [key], or null when the key is absent, explicitly null, or
198+
* holds something that is not an array.
199+
*
200+
* Gson's `getAsJsonArray` is a raw cast: a scalar, an object or an explicit JSON null
201+
* there throws [ClassCastException]. [parse]'s `runCatching` wraps only the initial
202+
* document parse, so that would escape a function whose own contract is to return
203+
* null. Every other field here is read defensively; this makes the array reads match.
204+
*
205+
* @param key the key to read.
206+
* @return the array, or null rather than a throw for any other shape.
207+
*/
208+
private fun JsonObject.jsonArray(key: String): JsonArray? = get(key) as? JsonArray
209+
195210
/**
196211
* A JSON array of strings; empty when the key is absent or not an array.
197212
*
@@ -200,7 +215,7 @@ data class ProxyAppInfo(
200215
* dropped rather than treated as an error.
201216
*/
202217
private fun JsonObject.stringArray(key: String): List<String> =
203-
getAsJsonArray(key)
218+
jsonArray(key)
204219
?.mapNotNull { it.takeIf(com.google.gson.JsonElement::isJsonPrimitive)?.asString }
205220
?.filter { it.isNotBlank() }
206221
?: emptyList()
@@ -256,7 +271,7 @@ data class ProxyAppInfo(
256271
?.asBoolean == true,
257272
supertypes =
258273
obj
259-
.getAsJsonArray("supertypes")
274+
.jsonArray("supertypes")
260275
?.mapNotNull { it.takeIf(com.google.gson.JsonElement::isJsonPrimitive)?.asString }
261276
?: emptyList(),
262277
)

‎quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/ProxyAppInfoEdgeTest.kt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,45 @@ class ProxyAppInfoEdgeTest {
2323
}
2424
""".trimIndent()
2525

26+
@Test
27+
fun `an array key holding a scalar is ignored, not a crash`() {
28+
// getAsJsonArray is a raw cast in Gson, and parse's runCatching covers only the
29+
// initial document parse - so a hand-edited or older setup.json used to throw
30+
// ClassCastException out of a function whose contract is to return null.
31+
val text = json(""","classpath": "/libs/one.jar", "sourceRoots": 7""")
32+
33+
val info = ProxyAppInfo.parse(text, baseDir)
34+
35+
assertThat(info).isNotNull()
36+
assertThat(info!!.classpath).isEmpty()
37+
assertThat(info.sourceRoots).isEmpty()
38+
}
39+
40+
@Test
41+
fun `an explicit JSON null array key is ignored, not a crash`() {
42+
val text = json(""","components": null, "payloadJars": null""")
43+
44+
val info = ProxyAppInfo.parse(text, baseDir)
45+
46+
assertThat(info).isNotNull()
47+
assertThat(info!!.components).isEmpty()
48+
assertThat(info.classpath).isEmpty()
49+
}
50+
51+
@Test
52+
fun `a component whose supertypes key is an object is ignored, not a crash`() {
53+
val text =
54+
json(
55+
""","components": [{"type":"activity","userClass":"com.example.Main",""" +
56+
""""supertypes": {"0": "android.app.Activity"}}]""",
57+
)
58+
59+
val info = ProxyAppInfo.parse(text, baseDir)
60+
61+
assertThat(info).isNotNull()
62+
assertThat(info!!.components.single().supertypes).isEmpty()
63+
}
64+
2665
@Test
2766
fun `non-JSON text parses to null`() {
2867
assertThat(ProxyAppInfo.parse("not json at all", baseDir)).isNull()

‎quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/QuickBuildProjectLayoutTest.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.appdevforall.cotg.quickbuild.data
22

33
import com.google.common.truth.Truth.assertThat
4-
import org.appdevforall.cotg.quickbuild.service.telemetry.report
54
import org.junit.jupiter.api.Test
65
import org.junit.jupiter.api.io.TempDir
76
import java.io.File

0 commit comments

Comments
 (0)