diff --git a/src/main/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsService.kt b/src/main/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsService.kt index cbdcbe9ef73..7fa258021e1 100644 --- a/src/main/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsService.kt +++ b/src/main/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsService.kt @@ -784,14 +784,6 @@ class ReportsService( internalMetricCalculationSpec.details.metricFrequencySpec, trailingWindow, ) - grpcRequire(generatedTimeIntervals.isNotEmpty()) { - "No time intervals can be generated from the combination of the reporting_interval and metric_calculation_spec ${ - MetricCalculationSpecKey( - internalMetricCalculationSpec.cmmsMeasurementConsumerId, - internalMetricCalculationSpec.externalMetricCalculationSpecId, - ).toName() - }" - } generatedTimeIntervals } } @@ -982,7 +974,7 @@ private fun validateTime(report: Report) { } .isBefore(reportEnd) ) { - "report_end be after report_start." + "report_end is not after report_start." } } else { failGrpc { "time is not set." } @@ -1131,10 +1123,6 @@ private fun generateTimeIntervals( if (ChronoUnit.SECONDS.between(reportStartTemporal, nextTimeIntervalEndTemporal) > 0L) { val nextTimeIntervalEndTimestamp = nextTimeIntervalEndTemporal.toTimestamp() - if (Timestamps.compare(nextTimeIntervalEndTimestamp, reportEndTimestamp) > 0) { - break - } - val nextTimeIntervalStartTimestamp: Timestamp = if (isWindowReportStart) { reportStartTimestamp @@ -1153,6 +1141,18 @@ private fun generateTimeIntervals( } } + if (Timestamps.compare(nextTimeIntervalEndTimestamp, reportEndTimestamp) > 0) { + if (Timestamps.compare(nextTimeIntervalStartTimestamp, reportEndTimestamp) < 0) { + add( + interval { + startTime = nextTimeIntervalStartTimestamp + endTime = reportEndTimestamp + } + ) + } + break + } + add( interval { startTime = nextTimeIntervalStartTimestamp diff --git a/src/main/proto/wfa/measurement/reporting/v2alpha/metric_calculation_spec.proto b/src/main/proto/wfa/measurement/reporting/v2alpha/metric_calculation_spec.proto index 065cbcc9f0f..b5d1e58359b 100644 --- a/src/main/proto/wfa/measurement/reporting/v2alpha/metric_calculation_spec.proto +++ b/src/main/proto/wfa/measurement/reporting/v2alpha/metric_calculation_spec.proto @@ -98,16 +98,21 @@ message MetricCalculationSpec { // A time interval is created for each day. Daily daily = 1; // A time interval is created for each week, starting on the first - // occurrence of the `day_of_week`. If `report_start` date is Monday and + // occurrence of the `day_of_week`. If `report_start` date is a Monday and // `day_of_week` is Tuesday, the first time interval will be created for - // the following day. + // the time period between that Monday and Tuesday, which will be 1 day + // long. The last time interval created will cover up to + // `reporting_interval.report_end` in a `Report` instead of the + // `day_of_week` specified. Weekly weekly = 2; // A time interval is created for each month, starting on the first // occurrence of the `day_of_month`. If `report_start` date is the 1st of // the month and `day_of_month` is 14, the first time interval will be - // created starting with the 14th of that month. If 'day_of_month` is + // from the 1st to the 14th of the month. If 'day_of_month` is // larger than the amount of days the month has, a time interval will be - // created for the end of the month. + // created for the end of the month. The last time interval created will + // cover up to `reporting_interval.report_end` in a `Report` instead of + // the `day_of_month` specified. Monthly monthly = 3; } } @@ -141,7 +146,8 @@ message MetricCalculationSpec { } // The `TrailingWindow` for this `MetricCalculationSpec`. The beginning of the - // time covered is bounded by `ReportingInterval.report_start`. + // time covered is bounded by `ReportingInterval.report_start` and the end of + // the time covered is bounded by `ReportingInterval.report_end`. // // If set, but `metric_frequency_spec` is not, an error will be thrown. // If set and `metric_frequency_spec` is also set, the combination of this diff --git a/src/main/proto/wfa/measurement/reporting/v2alpha/report.proto b/src/main/proto/wfa/measurement/reporting/v2alpha/report.proto index d995534c070..cf86ceb60db 100644 --- a/src/main/proto/wfa/measurement/reporting/v2alpha/report.proto +++ b/src/main/proto/wfa/measurement/reporting/v2alpha/report.proto @@ -84,7 +84,7 @@ message Report { ]; // The interval over which Metrics are generated. This interval is - // closed on the left hand side and open on the right hand side. + // closed on both sides. message ReportingInterval { // The date and time of the start of the report. The year, // month, day, and time_offset are all required. @@ -99,7 +99,7 @@ message Report { ]; // The end date of the report. The time is considered to be the same as - // that specified in the report start. This side of the interval is open. + // that specified in the report start. This side of the interval is closed. google.type.Date report_end = 2 [ (google.api.field_behavior) = REQUIRED, (google.api.field_behavior) = IMMUTABLE @@ -117,7 +117,8 @@ message Report { // A single time interval specifying the start and end time of the `Report`. // If this is specified, then for each `MetricCalculationSpec`, // `metric_frequency_spec` and `trailing_window` will be used to divide the - // interval. Any interval created will have a lower bound of report_start. + // interval. Any interval created will have a lower bound of `report_start` + // and an upper bound of `report_end`. // If no `metric_frequency_spec` is specified then the entire interval is // reported on. ReportingInterval reporting_interval = 10; diff --git a/src/test/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsServiceTest.kt b/src/test/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsServiceTest.kt index ba07d5dc22f..f8a069ceb04 100644 --- a/src/test/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsServiceTest.kt +++ b/src/test/kotlin/org/wfanet/measurement/reporting/service/api/v2alpha/ReportsServiceTest.kt @@ -747,6 +747,16 @@ class ReportsServiceTest { } } ) + add( + interval { + startTime = timestamp { + seconds = 1705392000 // January 16, 2024 at 12:00 AM, America/Los_Angeles + } + endTime = timestamp { + seconds = 1705564800 // January 18, 2024 at 12:00 AM, America/Los_Angeles + } + } + ) } val frequencySpec = @@ -868,6 +878,16 @@ class ReportsServiceTest { } } ) + add( + interval { + startTime = timestamp { + seconds = 1709280000 // March 1, 2024 at 12:00 AM, America/Los_Angeles + } + endTime = timestamp { + seconds = 1709625600 // March 5, 2024 at 12:00 AM, America/Los_Angeles + } + } + ) } val frequencySpec = @@ -997,6 +1017,16 @@ class ReportsServiceTest { } } ) + add( + interval { + startTime = timestamp { + seconds = 1709280000 // March 1, 2024 at 12:00 AM, America/Los_Angeles + } + endTime = timestamp { + seconds = 1709625600 // March 5, 2024 at 12:00 AM, America/Los_Angeles + } + } + ) } val frequencySpec = @@ -1117,6 +1147,16 @@ class ReportsServiceTest { } } ) + add( + interval { + startTime = timestamp { + seconds = 1704096000 // January 1, 2024 at 12:00 AM, America/Los_Angeles + } + endTime = timestamp { + seconds = 1709625600 // March 5, 2024 at 12:00 AM, America/Los_Angeles + } + } + ) } val frequencySpec = @@ -1165,6 +1205,16 @@ class ReportsServiceTest { } } ) + add( + interval { + startTime = timestamp { + seconds = 1704096000 // January 1, 2024 at 12:00 AM, America/Los_Angeles + } + endTime = timestamp { + seconds = 1705046400 // January 12, 2024 at 12:00 AM, America/Los_Angeles + } + } + ) } val frequencySpec = @@ -2828,31 +2878,7 @@ class ReportsServiceTest { } @Test - fun `createReport throws INVALID_ARGUMENT when reporting interval results in 0 time intervals`() { - runBlocking { - whenever(internalMetricCalculationSpecsMock.batchGetMetricCalculationSpecs(any())) - .thenReturn( - batchGetMetricCalculationSpecsResponse { - metricCalculationSpecs += - INTERNAL_REACH_METRIC_CALCULATION_SPEC.copy { - details = - INTERNAL_REACH_METRIC_CALCULATION_SPEC.details.copy { - metricFrequencySpec = - MetricCalculationSpecKt.metricFrequencySpec { - monthly = - MetricCalculationSpecKt.MetricFrequencySpecKt.monthly { dayOfMonth = 15 } - } - trailingWindow = - MetricCalculationSpecKt.trailingWindow { - count = 1 - increment = MetricCalculationSpec.TrailingWindow.Increment.WEEK - } - } - } - } - ) - } - + fun `createReport throws INVALID_ARGUMENT when report_start after report_end`() { val request = createReportRequest { parent = MEASUREMENT_CONSUMER_KEYS.first().toName() report = @@ -2864,15 +2890,15 @@ class ReportsServiceTest { reportingInterval = ReportKt.reportingInterval { reportStart = dateTime { - year = 2023 + year = 3000 month = 1 day = 1 - utcOffset = duration { seconds = 60 * 60 * -8 } + timeZone = timeZone { id = "America/Los_Angeles" } } reportEnd = date { - year = 2023 + year = 2024 month = 1 - day = 5 + day = 1 } } } @@ -2886,11 +2912,11 @@ class ReportsServiceTest { } } assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT) - assertThat(exception.message).contains("No time intervals") + assertThat(exception.message).contains("report_end") } @Test - fun `createReport throws INVALID_ARGUMENT when report_start after report_end`() { + fun `createReport throws INVALID_ARGUMENT when report_start same as report_end`() { val request = createReportRequest { parent = MEASUREMENT_CONSUMER_KEYS.first().toName() report = @@ -2902,7 +2928,7 @@ class ReportsServiceTest { reportingInterval = ReportKt.reportingInterval { reportStart = dateTime { - year = 3000 + year = 2024 month = 1 day = 1 timeZone = timeZone { id = "America/Los_Angeles" }