Commit 697d91dd authored by Michael Droettboom's avatar Michael Droettboom
Browse files

1563177: TimingDistribution should use snake_case for its fields

parent 6341e10c
......@@ -172,7 +172,7 @@ data class TimingDistributionData(
// something is wrong and we should return null.
val category = jsonObject.tryGetString("category").orEmpty()
val name = jsonObject.tryGetString("name") ?: return null
val bucketCount = jsonObject.tryGetInt("bucketCount") ?: return null
val bucketCount = jsonObject.tryGetInt("bucket_count") ?: return null
// If 'range' isn't present, JSONException is thrown
val range = try {
val array = jsonObject.getJSONArray("range")
......@@ -190,8 +190,13 @@ data class TimingDistributionData(
} catch (e: org.json.JSONException) {
return null
}
val rawHistogramType = jsonObject.tryGetInt("histogramType") ?: return null
val histogramType = HistogramType.values().getOrNull(rawHistogramType) ?: return null
val rawHistogramType = jsonObject.tryGetString("histogram_type") ?: return null
var histogramType: HistogramType
try {
histogramType = HistogramType.valueOf(rawHistogramType.capitalize())
} catch (e: IllegalArgumentException) {
return null
}
// Attempt to parse the values map, if it fails then something is wrong and we need to
// return null.
val values = try {
......@@ -206,8 +211,13 @@ data class TimingDistributionData(
return null
}
val sum = jsonObject.tryGetLong("sum") ?: return null
val timeUnit = TimeUnit.values()[jsonObject.tryGetInt("timeUnit")
?: return null]
val rawTimeUnit = jsonObject.tryGetString("time_unit") ?: return null
var timeUnit: TimeUnit
try {
timeUnit = TimeUnit.valueOf(rawTimeUnit.capitalize())
} catch (e: IllegalArgumentException) {
return null
}
return TimingDistributionData(
category = category,
......@@ -262,12 +272,12 @@ data class TimingDistributionData(
return JSONObject(mapOf(
"category" to category,
"name" to name,
"bucketCount" to bucketCount,
"bucket_count" to bucketCount,
"range" to JSONArray(arrayOf(rangeMin, rangeMax)),
"histogramType" to histogramType.ordinal,
"histogram_type" to histogramType.toString().toLowerCase(),
"values" to values.mapKeys { "${it.key}" },
"sum" to sum,
"timeUnit" to timeUnit.ordinal
"time_unit" to timeUnit.toString().toLowerCase()
))
}
......
......@@ -8,12 +8,18 @@ import android.content.Context
import android.content.SharedPreferences
import androidx.test.core.app.ApplicationProvider
import kotlinx.coroutines.runBlocking
import mozilla.components.service.glean.Glean
import mozilla.components.service.glean.checkPingSchema
import mozilla.components.service.glean.private.Lifetime
import mozilla.components.service.glean.private.TimeUnit
import mozilla.components.service.glean.private.TimingDistributionMetricType
import mozilla.components.service.glean.error.ErrorRecording
import mozilla.components.service.glean.getContextWithMockedInfo
import mozilla.components.service.glean.getMockWebServer
import mozilla.components.service.glean.private.PingType
import mozilla.components.service.glean.resetGlean
import mozilla.components.service.glean.timing.TimingManager
import mozilla.components.service.glean.triggerWorkManager
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
......@@ -25,6 +31,7 @@ import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers
import org.mockito.Mockito
import org.robolectric.RobolectricTestRunner
import java.util.concurrent.TimeUnit as AndroidTimeUnit
@RunWith(RobolectricTestRunner::class)
class TimingDistributionsStorageEngineTest {
......@@ -128,6 +135,39 @@ class TimingDistributionsStorageEngineTest {
snapshot["telemetry.test_timing_distribution"]?.toJsonObject().toString())
}
@Test
fun `serializer should serialize timing distribution that matches schema`() {
val ping1 = PingType("store1", true)
val server = getMockWebServer()
resetGlean(getContextWithMockedInfo(), Glean.configuration.copy(
serverEndpoint = "http://" + server.hostName + ":" + server.port,
logPings = true
))
val metric = TimingDistributionMetricType(
disabled = false,
category = "telemetry",
lifetime = Lifetime.User,
name = "test_timing_distribution",
sendInPings = listOf("store1"),
timeUnit = TimeUnit.Millisecond
)
runBlocking {
TimingManager.getElapsedNanos = { 0 }
val id = metric.start()
TimingManager.getElapsedNanos = { 1000000 }
metric.stopAndAccumulate(id)
}
Glean.sendPings(listOf(ping1))
triggerWorkManager()
val pingJson = server.takeRequest(20L, AndroidTimeUnit.SECONDS).body.readUtf8()
checkPingSchema(pingJson)
}
@Test
fun `serializer should correctly serialize timing distributions`() {
run {
......@@ -345,7 +385,7 @@ class TimingDistributionsStorageEngineTest {
assertEquals("JSON name must match Timing Distribution name",
"test_timing_distribution", jsonTdd.getString("name"))
assertEquals("JSON bucket count must match Timing Distribution bucket count",
tdd.bucketCount, jsonTdd.getInt("bucketCount"))
tdd.bucketCount, jsonTdd.getInt("bucket_count"))
assertEquals("JSON name must match Timing Distribution name",
"test_timing_distribution", jsonTdd.getString("name"))
val jsonRange = jsonTdd.getJSONArray("range")
......@@ -354,7 +394,7 @@ class TimingDistributionsStorageEngineTest {
assertEquals("JSON range maximum must match Timing Distribution range maximum",
tdd.rangeMax, jsonRange.getLong(1))
assertEquals("JSON histogram type must match Timing Distribution histogram type",
tdd.histogramType.ordinal, jsonTdd.getInt("histogramType"))
tdd.histogramType.toString().toLowerCase(), jsonTdd.getString("histogram_type"))
val jsonValue = jsonTdd.getJSONObject("values")
assertEquals("JSON values must match Timing Distribution values",
tdd.values[0], jsonValue.getLong("0"))
......@@ -365,6 +405,6 @@ class TimingDistributionsStorageEngineTest {
assertEquals("JSON sum must match Timing Distribution sum",
tdd.sum, jsonTdd.getLong("sum"))
assertEquals("JSON time unit must match Timing Distribution time unit",
tdd.timeUnit.ordinal, jsonTdd.getInt("timeUnit"))
tdd.timeUnit.toString().toLowerCase(), jsonTdd.getString("time_unit"))
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment