Update cross-media-measurement-api dep for DataProvider capabilities.

[SanjayVas]

No description provided.

[wfa-reviewable]

This change is 

[SanjayVas]

Depends on world-federation-of-advertisers/cross-media-measurement-api#199

[stevenwarejones]

Reviewed 7 of 19 files at r1, all commit messages.
Reviewable status: 7 of 19 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/SpannerDataProvidersService.kt line 38 at r1 (raw file):

import org.wfanet.measurement.kingdom.deploy.gcloud.spanner.writers.ReplaceDataProviderRequiredDuchies

// TODO(@marcopremier): Add method to update data provider required duchies list.

is this TODO still needed with the move to HM Shuffle?

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/writers/CreateMeasurements.kt line 141 at r1 (raw file):

    @Suppress("WHEN_ENUM_CAN_BE_NULL_IN_JAVA") // Protobuf enum fields are never null.
    return when (request.measurement.details.protocolConfig.protocolCase) {

shouldn't the kingdom be setting the protocol? eg aggressively choosing the cheapest option appropriate for that market but falling back to other methods if certain EDPs do not support?

[SanjayVas]

Reviewable status: 6 of 19 files reviewed, 2 unresolved discussions (waiting on @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/SpannerDataProvidersService.kt line 38 at r1 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

is this TODO still needed with the move to HM Shuffle?

Done. The method appears to have already been added (replaceDataProviderRequiredDuchies).

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/writers/CreateMeasurements.kt line 141 at r1 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

shouldn't the kingdom be setting the protocol? eg aggressively choosing the cheapest option appropriate for that market but falling back to other methods if certain EDPs do not support?

The Kingdom is setting the protocol. It's handled in the public API service impl.

[SanjayVas]

Reviewable status: 6 of 19 files reviewed, 2 unresolved discussions (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/writers/CreateMeasurements.kt line 141 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

The Kingdom is setting the protocol. It's handled in the public API service impl.

Ah wait, I see your point. I'll need to have the public API service make separate calls to get the DataProviders then, as the logic that select the protocol is in the public service impl. The internal service doesn't make decisions and just does what it's told.

[SanjayVas]

Reviewable status: 3 of 23 files reviewed, 2 unresolved discussions (waiting on @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/writers/CreateMeasurements.kt line 141 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Ah wait, I see your point. I'll need to have the public API service make separate calls to get the DataProviders then, as the logic that select the protocol is in the public service impl. The internal service doesn't make decisions and just does what it's told.

Done.

[SanjayVas]

CC @renjiezh for HMSS capability

[stevenwarejones]

Reviewed 4 of 19 files at r1, 13 of 19 files at r2, all commit messages.
Reviewable status: 17 of 23 files reviewed, 4 unresolved discussions (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 384 at r2 (raw file):

      val internalCreateMeasurementRequest =
        createMeasurementRequest.buildInternalCreateMeasurementRequest(
          dataProviderCapabilities.filterKeys { it in externalDataProviderIds }.values,

why do we need to filter here? Aren't they filtered above already? When wouldn't it already be present in externalDataProviderIds?

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

          }
        } else {
          if (reachOnlyLlV2Enabled) {

don't we support Hmss for reach only, too?

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 571 at r2 (raw file):

      }
      MeasurementSpec.MeasurementTypeCase.POPULATION -> {
        protocolConfig { direct = InternalProtocolConfig.Direct.getDefaultInstance() }

I believe for population, deterministicCount should be set.

[renjiezh]

Reviewed 4 of 19 files at r1, 19 of 19 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 384 at r2 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

why do we need to filter here? Aren't they filtered above already? When wouldn't it already be present in externalDataProviderIds?

This function is for batch creation usage. externalDataProviderIds has all ids for multiple Measurements.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

don't we support Hmss for reach only, too?

I will change the feature flag here so that reach-only Measurements uses hmss as well.

[renjiezh]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @SanjayVas and @stevenwarejones)

[stevenwarejones]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @renjiezh and @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 384 at r2 (raw file):

Previously, renjiezh wrote…

This function is for batch creation usage. externalDataProviderIds has all ids for multiple Measurements.

maybe add a comment around that. that was non-obvious to me at first glance.

[stevenwarejones]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @renjiezh and @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

Previously, renjiezh wrote…

I will change the feature flag here so that reach-only Measurements uses hmss as well.

can't we go ahead and add it this pr? its already in this pr for r&f

[renjiezh]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 477 at r2 (raw file):

  }

  private fun buildInternalProtocolConfig(

nit: what about moving this to ProtoConversion file?

[renjiezh]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

can't we go ahead and add it this pr? its already in this pr for r&f

HMSS is still under implementing. The feature flag will only be ready along with the executable class. A TODO here might help to resolve the confusion.

[SanjayVas]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @renjiezh and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 477 at r2 (raw file):

Previously, renjiezh wrote…

nit: what about moving this to ProtoConversion file?

I actually moved this logic out of there intentionally to avoid having to pass instance state fields over (e.g. noiseMechanisms).

As an aside, I don't actually like the central ProtoConversions.kt file as it forces each service library target to depend on the protobufs for all of the services. IMO only the ones that need to be shared between services should be extracted to a common location.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

Previously, renjiezh wrote…

HMSS is still under implementing. The feature flag will only be ready along with the executable class. A TODO here might help to resolve the confusion.

@renjiezh I had thought HMSS was only for R/F. Does it have a different config for reach-only? If it's the same config, I can include it in this PR. If not, I'll leave a TODO.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 571 at r2 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

I believe for population, deterministicCount should be set.

Will fix.

[renjiezh]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

@renjiezh I had thought HMSS was only for R/F. Does it have a different config for reach-only? If it's the same config, I can include it in this PR. If not, I'll leave a TODO.

It could be used for reach-only Measurement. The config is the same but the dp params is different (specified by MC though)

[SanjayVas]

Reviewable status: 21 of 23 files reviewed, 2 unresolved discussions (waiting on @renjiezh and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 384 at r2 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

maybe add a comment around that. that was non-obvious to me at first glance.

Renamed to allDataProviderCapabilities paralleling allExternalDataProviderIds.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 500 at r2 (raw file):
Done.

The config is the same but the dp params is different (specified by MC though)

I don't think that's relevant to this PR, as this is just concerned with the protocol config.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 571 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Will fix.

Done. Note that this is different than the code state prior to this PR, as the original code was extracted from ProtoConversions.kt

[stevenwarejones]

Reviewed 6 of 19 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[SanjayVas]

Reviewed 2 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[SanjayVas]

Reviewed 1 of 1 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[SanjayVas]

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[renjiezh]

Reviewed 2 of 2 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @SanjayVas)

[SanjayVas]

Reviewed 1 of 1 files at r7.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @SanjayVas)