Fix Reporting V1 RF Result (#1182)

world-federation-of-advertisers · Aug 23, 2023 · 7e72633 · 7e72633
1 parent 4979e9f
commit 7e72633
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 8 deletions.
diff --git a/...n/kotlin/org/wfanet/measurement/reporting/deploy/postgres/writers/SetMeasurementResult.kt b/...n/kotlin/org/wfanet/measurement/reporting/deploy/postgres/writers/SetMeasurementResult.kt
@@ -368,14 +368,13 @@ class SetMeasurementResult(private val request: SetMeasurementResultRequest) :
           }
           aggregatedFrequencyHistogramMap
         }
-    val resultsList = aggregatedFrequencyHistogramMap.entries.sortedBy { it.key }.map { it.value }
-    return if (resultsList.size < maximumFrequency) {
-      val paddedResultsList = resultsList.toMutableList()
-      paddedResultsList.addAll(DoubleArray(maximumFrequency - resultsList.size) { 0.0 }.toList())
-      paddedResultsList
-    } else {
-      resultsList
+
+    val resultFrequencyHistogramMap = mutableMapOf<Long, Double>()
+    for (i in 1L..maximumFrequency) {
+      resultFrequencyHistogramMap[i] = aggregatedFrequencyHistogramMap.getOrDefault(i, 0.0)
     }
+
+    return resultFrequencyHistogramMap.entries.sortedBy { it.key }.map { it.value }
   }
 
   /** Convert a [Metric] to a [Report.Details.Result.HistogramTable] of a [Report] */

diff --git a/...tlin/org/wfanet/measurement/reporting/service/internal/testing/MeasurementsServiceTest.kt b/...tlin/org/wfanet/measurement/reporting/service/internal/testing/MeasurementsServiceTest.kt
@@ -319,7 +319,7 @@ abstract class MeasurementsServiceTest<T : MeasurementsCoroutineImplBase> {
   }
 
   @Test
-  fun `setMeasurementResult sets result for RF metric report with padding`() {
+  fun `setMeasurementResult sets result for RF metric report with padding at the end`() {
     val metricDetails =
       MetricKt.details {
         frequencyHistogram = MetricKt.frequencyHistogramParams { maximumFrequencyPerUser = 3 }
@@ -445,6 +445,115 @@ abstract class MeasurementsServiceTest<T : MeasurementsCoroutineImplBase> {
       )
   }
 
+  @Test
+  fun `setMeasurementResult sets result for RF metric report with padding at the beginning`() {
+    val metricDetails =
+      MetricKt.details {
+        frequencyHistogram = MetricKt.frequencyHistogramParams { maximumFrequencyPerUser = 2 }
+      }
+    val createdReport = runBlocking {
+      reportsService.createReport(
+        createReportRequest {
+          measurements +=
+            CreateReportRequestKt.measurementKey {
+              measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+              measurementReferenceId = MEASUREMENT_REFERENCE_ID
+            }
+          measurements +=
+            CreateReportRequestKt.measurementKey {
+              measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+              measurementReferenceId = MEASUREMENT_REFERENCE_ID_2
+            }
+          report = report {
+            measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+            reportIdempotencyKey = "1235"
+            periodicTimeInterval = PERIODIC_TIME_INTERVAL
+            metrics += metric {
+              details = metricDetails
+              namedSetOperations += NAMED_SET_OPERATION
+            }
+          }
+        }
+      )
+    }
+    runBlocking {
+      service.setMeasurementResult(
+        setMeasurementResultRequest {
+          measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+          measurementReferenceId = MEASUREMENT_REFERENCE_ID
+          result =
+            MeasurementKt.result {
+              reach = MeasurementKt.ResultKt.reach { value = 100L }
+              frequency =
+                MeasurementKt.ResultKt.frequency { relativeFrequencyDistribution[2] = 0.2 }
+            }
+        }
+      )
+      service.setMeasurementResult(
+        setMeasurementResultRequest {
+          measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+          measurementReferenceId = MEASUREMENT_REFERENCE_ID_2
+          result =
+            MeasurementKt.result {
+              reach = MeasurementKt.ResultKt.reach { value = 200L }
+              frequency =
+                MeasurementKt.ResultKt.frequency { relativeFrequencyDistribution[2] = 0.7 }
+            }
+        }
+      )
+    }
+    val retrievedReport = runBlocking {
+      reportsService.getReport(
+        getReportRequest {
+          measurementConsumerReferenceId = MEASUREMENT_CONSUMER_REFERENCE_ID
+          externalReportId = createdReport.externalReportId
+        }
+      )
+    }
+    assertThat(retrievedReport.state).isEqualTo(Report.State.SUCCEEDED)
+    assertThat(retrievedReport.details.result)
+      .usingDoubleTolerance(2.0)
+      .isEqualTo(
+        ReportKt.DetailsKt.result {
+          histogramTables +=
+            ReportKt.DetailsKt.ResultKt.histogramTable {
+              rows +=
+                ReportKt.DetailsKt.ResultKt.HistogramTableKt.row {
+                  rowHeader = "1970-01-01T00:01:40.000000010Z-1970-01-01T00:01:50.000000011Z"
+                  frequency = 1
+                }
+              rows +=
+                ReportKt.DetailsKt.ResultKt.HistogramTableKt.row {
+                  rowHeader = "1970-01-01T00:01:40.000000010Z-1970-01-01T00:01:50.000000011Z"
+                  frequency = 2
+                }
+              rows +=
+                ReportKt.DetailsKt.ResultKt.HistogramTableKt.row {
+                  rowHeader = "1970-01-01T00:01:50.000000011Z-1970-01-01T00:02:00.000000012Z"
+                  frequency = 1
+                }
+              rows +=
+                ReportKt.DetailsKt.ResultKt.HistogramTableKt.row {
+                  rowHeader = "1970-01-01T00:01:50.000000011Z-1970-01-01T00:02:00.000000012Z"
+                  frequency = 2
+                }
+              columns +=
+                ReportKt.DetailsKt.ResultKt.column {
+                  columnHeader =
+                    buildColumnHeader(
+                      metricDetails.metricTypeCase.name,
+                      NAMED_SET_OPERATION.displayName
+                    )
+                  setOperations += 0.0
+                  setOperations += 440.0
+                  setOperations += 0.0
+                  setOperations += 880.0
+                }
+            }
+        }
+      )
+  }
+
   @Test
   fun `setMeasurementResult succeeds in setting the result for report with duration metric`() {
     val metricDetails =