From 704fc7ae6c388e5c3bdfd2c645f488198ea30348 Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 17:47:42 +0200 Subject: [PATCH 1/8] optable-targeting: Fall back to host config when the account has none An account can have the module hooks in its execution plan without a module config of its own. ConfigResolver passed the null config to the JSON merger, which threw and failed every hook of the module. --- .../targeting/v1/core/ConfigResolver.java | 5 +++ .../targeting/v1/core/ConfigResolverTest.java | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolverTest.java diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolver.java index 219f37af2ab..a5ea6bdf0ff 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolver.java @@ -24,6 +24,11 @@ public ConfigResolver(ObjectMapper mapper, JsonMerger jsonMerger, OptableTargeti } public OptableTargetingProperties resolve(ObjectNode configNode) { + // an account may have the hooks in its execution plan without any module config of its own + if (configNode == null) { + return globalProperties; + } + final JsonNode mergedNode = jsonMerger.merge(configNode, globalPropertiesObjectNode); return parse(mergedNode).orElse(globalProperties); } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolverTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolverTest.java new file mode 100644 index 00000000000..ca32067fb50 --- /dev/null +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/ConfigResolverTest.java @@ -0,0 +1,41 @@ +package org.prebid.server.hooks.modules.optable.targeting.v1.core; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; +import org.prebid.server.hooks.modules.optable.targeting.v1.BaseOptableTest; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ConfigResolverTest extends BaseOptableTest { + + private OptableTargetingProperties globalProperties; + + private ConfigResolver target; + + @BeforeEach + public void setUp() { + globalProperties = givenOptableTargetingProperties(false); + target = new ConfigResolver(mapper, jsonMerger, globalProperties); + } + + @Test + public void resolveShouldReturnGlobalPropertiesWhenAccountHasNoModuleConfig() { + // when and then + assertThat(target.resolve(null)).isSameAs(globalProperties); + } + + @Test + public void resolveShouldPreferAccountPropertiesOverGlobalOnes() { + // given + final ObjectNode accountConfig = mapper.createObjectNode().put("tenant", "accountTenant"); + + // when + final OptableTargetingProperties result = target.resolve(accountConfig); + + // then + assertThat(result.getTenant()).isEqualTo("accountTenant"); + assertThat(result.getOrigin()).isEqualTo(globalProperties.getOrigin()); + } +} From e5e76a4c57d1f8797e6a14006937390c5123049f Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 17:43:41 +0200 Subject: [PATCH 2/8] optable-targeting: Read old-style bidders from imp.ext.BIDDER The fallback looked for imp.ext.bidder, which is not a format the core knows. Old-style params sit directly under imp.ext and the core moves them to imp.ext.prebid.bidder for every imp that has no bidders there, so the sampler now applies the same rule per imp instead of falling back only when no imp at all has imp.ext.prebid.bidder. --- .../v1/core/BidderEnrichmentSampler.java | 37 ++++++++-------- .../v1/core/BidderEnrichmentSamplerTest.java | 43 ++++++++++++++----- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java index 2f4b9b3a557..edaf725fd74 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java @@ -1,16 +1,16 @@ package org.prebid.server.hooks.modules.optable.targeting.v1.core; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; import lombok.AllArgsConstructor; -import org.apache.commons.collections4.CollectionUtils; import org.prebid.server.auction.aliases.BidderAliases; +import org.prebid.server.auction.requestfactory.Ortb2ImplicitParametersResolver; import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; import org.prebid.server.util.StreamUtil; import java.util.Collection; -import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -18,12 +18,13 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.function.IntSupplier; import java.util.stream.Collectors; +import java.util.stream.Stream; @AllArgsConstructor(staticName = "of") public class BidderEnrichmentSampler { - public static final String PREBID_BIDDER_PATH = "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/prebid/bidder"; - public static final String BIDDER_PATH = "/bidder"; + private static final String PREBID_BIDDER_PATH = "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/prebid/bidder"; + private final AliasesResolver aliasesResolver; private final IntSupplier randomSupplier; @@ -57,29 +58,27 @@ private static int resolvePercentage(BidderAliases aliases, String bidder, } private static Set extractUniqueBidders(BidRequest bidRequest) { - final Set impExt = Optional.ofNullable(bidRequest.getImp()) + return Optional.ofNullable(bidRequest.getImp()) .stream() .flatMap(Collection::stream) .map(Imp::getExt) .filter(Objects::nonNull) + .flatMap(BidderEnrichmentSampler::extractImpBidders) .collect(Collectors.toSet()); - - final Set bidders = extractBiddersByPath(impExt, PREBID_BIDDER_PATH); - return CollectionUtils.isNotEmpty(bidders) - ? bidders - : extractBiddersByPath(impExt, BIDDER_PATH); - } - private static Set extractBiddersByPath(Set impExt, String path) { - if (CollectionUtils.isEmpty(impExt)) { - return Collections.emptySet(); + /** + * Follows the core, which at a later stage moves old-style imp.ext.BIDDER params into imp.ext.prebid.bidder + * for the imps that have no bidders there yet. + */ + private static Stream extractImpBidders(ObjectNode impExt) { + final JsonNode prebidBidders = impExt.at(PREBID_BIDDER_PATH); + if (prebidBidders.isObject() && !prebidBidders.isEmpty()) { + return StreamUtil.asStream(prebidBidders.fieldNames()); } - return impExt.stream() - .map(ext -> ext.at(path)) - .filter(Objects::nonNull) - .flatMap(bidder -> StreamUtil.asStream(bidder.fieldNames())) - .collect(Collectors.toSet()); + return StreamUtil.asStream(impExt.fieldNames()) + .filter(Ortb2ImplicitParametersResolver::isImpExtBidder) + .filter(field -> impExt.get(field).isObject()); } } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java index 0b76c6c7800..d0415148d74 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java @@ -242,13 +242,13 @@ public void sampleShouldDeduplicateBiddersAppearingInMultipleImps() { } @Test - public void sampleShouldReturnBiddersFromOrtbBidderNodeWhenPrebidBidderNodeIsAbsent() { + public void sampleShouldReturnOldStyleImpExtBiddersWhenPrebidBidderNodeIsAbsent() { // given given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); given(randomSupplier.getAsInt()).willReturn(99); final BidRequest bidRequest = givenBidRequest( - request -> request.imp(List.of(givenImp(imp -> imp.ext(givenOrtbBidderExt("bidderA", "bidderB")))))); + request -> request.imp(List.of(givenImp(imp -> imp.ext(givenOldStyleExt("bidderA", "bidderB")))))); // when final Set result = target.sample(bidRequest, givenSampleProperties(100, Collections.emptyMap())); @@ -258,13 +258,13 @@ public void sampleShouldReturnBiddersFromOrtbBidderNodeWhenPrebidBidderNodeIsAbs } @Test - public void sampleShouldPreferPrebidBidderNodeOverOrtbBidderNodeWhenBothArePresent() { + public void sampleShouldIgnoreOldStyleImpExtBiddersWhenImpHasPrebidBidderNode() { // given given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); given(randomSupplier.getAsInt()).willReturn(99); final ObjectNode ext = givenPrebidBidderExt("bidderA"); - ext.set("bidder", givenBidderNode("bidderB")); + ext.putObject("bidderB").put("param", "value"); final BidRequest bidRequest = givenBidRequest( request -> request.imp(List.of(givenImp(imp -> imp.ext(ext))))); @@ -276,13 +276,13 @@ public void sampleShouldPreferPrebidBidderNodeOverOrtbBidderNodeWhenBothArePrese } @Test - public void sampleShouldFallBackToOrtbBidderNodeWhenPrebidBidderNodeIsEmpty() { + public void sampleShouldFallBackToOldStyleImpExtBiddersWhenPrebidBidderNodeIsEmpty() { // given given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); given(randomSupplier.getAsInt()).willReturn(99); final ObjectNode ext = givenPrebidBidderExt(); - ext.set("bidder", givenBidderNode("bidderB")); + ext.putObject("bidderB").put("param", "value"); final BidRequest bidRequest = givenBidRequest( request -> request.imp(List.of(givenImp(imp -> imp.ext(ext))))); @@ -294,20 +294,41 @@ public void sampleShouldFallBackToOrtbBidderNodeWhenPrebidBidderNodeIsEmpty() { } @Test - public void sampleShouldIgnoreOrtbBidderNodeWhenPrebidBidderNodeIsPresentInAnyImp() { + public void sampleShouldCombinePrebidAndOldStyleBiddersOfDifferentImps() { // given given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); given(randomSupplier.getAsInt()).willReturn(99); final BidRequest bidRequest = givenBidRequest(request -> request.imp(List.of( givenImp(imp -> imp.ext(givenPrebidBidderExt("bidderA"))), - givenImp(imp -> imp.ext(givenOrtbBidderExt("bidderB")))))); + givenImp(imp -> imp.ext(givenOldStyleExt("bidderB")))))); + + // when + final Set result = target.sample(bidRequest, givenSampleProperties(100, Collections.emptyMap())); + + // then + assertThat(result).containsExactlyInAnyOrder("bidderA", "bidderB"); + } + + @Test + public void sampleShouldNotTreatReservedOrNonObjectImpExtFieldsAsBidders() { + // given + given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); + given(randomSupplier.getAsInt()).willReturn(99); + + final ObjectNode ext = givenOldStyleExt("bidderA", "context", "data", "skadn", "all", "general"); + ext.put("gpid", "gpid"); + ext.put("tid", "tid"); + ext.putObject("prebid").putObject("storedrequest").put("id", "id"); + final BidRequest bidRequest = givenBidRequest( + request -> request.imp(List.of(givenImp(imp -> imp.ext(ext))))); // when final Set result = target.sample(bidRequest, givenSampleProperties(100, Collections.emptyMap())); // then assertThat(result).containsExactly("bidderA"); + assertThat(target.hasBidders(bidRequest)).isTrue(); } private OptableTargetingProperties givenSampleProperties(int defaultPct, Map bidderPcts) { @@ -325,9 +346,11 @@ private ObjectNode givenPrebidBidderExt(String... bidders) { return ext; } - private ObjectNode givenOrtbBidderExt(String... bidders) { + private ObjectNode givenOldStyleExt(String... bidders) { final ObjectNode ext = mapper.createObjectNode(); - ext.set("bidder", givenBidderNode(bidders)); + for (String bidder : bidders) { + ext.putObject(bidder).put("param", "value"); + } return ext; } From f5c9f2c6651b2180946a20f5008ef7d818f7db26 Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 17:48:49 +0200 Subject: [PATCH 3/8] optable-targeting: Treat null enrichment percentages as defaults An account config with "enrichment-percentage": null or "bidder-enrichment-percentages": null overrides the defaults of the properties class, and the sampler then failed on unboxing. --- .../v1/core/BidderEnrichmentSampler.java | 13 +++++++++---- .../v1/core/BidderEnrichmentSamplerTest.java | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java index edaf725fd74..0872437bc09 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java @@ -5,6 +5,8 @@ import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; import lombok.AllArgsConstructor; +import org.apache.commons.collections4.MapUtils; +import org.apache.commons.lang3.ObjectUtils; import org.prebid.server.auction.aliases.BidderAliases; import org.prebid.server.auction.requestfactory.Ortb2ImplicitParametersResolver; import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; @@ -24,6 +26,7 @@ public class BidderEnrichmentSampler { private static final String PREBID_BIDDER_PATH = "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/prebid/bidder"; + private static final int DEFAULT_ENRICHMENT_PERCENTAGE = 100; private final AliasesResolver aliasesResolver; private final IntSupplier randomSupplier; @@ -33,9 +36,11 @@ public static BidderEnrichmentSampler of(AliasesResolver aliasesResolver) { } public Set sample(BidRequest bidRequest, OptableTargetingProperties optableTargetingProperties) { - final Integer defaultEnrichmentPercentage = optableTargetingProperties.getEnrichmentPercentage(); - final Map bidderEnrichmentPercentage = - optableTargetingProperties.getBidderEnrichmentPercentages(); + // an explicit null in the account config overrides the defaults of the properties class + final int defaultEnrichmentPercentage = ObjectUtils.defaultIfNull( + optableTargetingProperties.getEnrichmentPercentage(), DEFAULT_ENRICHMENT_PERCENTAGE); + final Map bidderEnrichmentPercentage = MapUtils.emptyIfNull( + optableTargetingProperties.getBidderEnrichmentPercentages()); final BidderAliases aliases = aliasesResolver.resolve(bidRequest); return extractUniqueBidders(bidRequest) @@ -49,7 +54,7 @@ public Set sample(BidRequest bidRequest, OptableTargetingProperties opta } private static int resolvePercentage(BidderAliases aliases, String bidder, - Integer defaultEnrichmentPercentage, + int defaultEnrichmentPercentage, Map bidderEnrichmentPercentage) { return Optional.ofNullable(bidderEnrichmentPercentage.get(bidder)) diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java index d0415148d74..37af92edd1a 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSamplerTest.java @@ -331,6 +331,25 @@ public void sampleShouldNotTreatReservedOrNonObjectImpExtFieldsAsBidders() { assertThat(target.hasBidders(bidRequest)).isTrue(); } + @Test + public void sampleShouldEnrichAllBiddersWhenPercentagesAreNull() { + // given + given(bidderAliases.resolveBidder(any())).willAnswer(inv -> inv.getArgument(0)); + given(randomSupplier.getAsInt()).willReturn(99); + + final BidRequest bidRequest = givenBidRequest( + request -> request.imp(List.of(givenImp(imp -> imp.ext(givenPrebidBidderExt("bidderA")))))); + final OptableTargetingProperties properties = new OptableTargetingProperties(); + properties.setEnrichmentPercentage(null); + properties.setBidderEnrichmentPercentages(null); + + // when + final Set result = target.sample(bidRequest, properties); + + // then + assertThat(result).containsExactly("bidderA"); + } + private OptableTargetingProperties givenSampleProperties(int defaultPct, Map bidderPcts) { final OptableTargetingProperties props = new OptableTargetingProperties(); props.setEnrichmentPercentage(defaultPct); From 313e3aac4b524d76015e3c1d65c4da3db68ee06c Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 20:03:53 +0200 Subject: [PATCH 4/8] optable-targeting: Clean deferred requests at the raw stage and defer mixed stored imps When the raw hook deferred the call to the processed hook it returned no_action, so user.ext.optable reached every bidder unless the processed hook was in the plan. The raw hook now always cleans and keeps user.ext.optable in the module context for the deferred call. The raw hook also defers whenever the request or any imp references a stored request, since bidders from stored imps were otherwise never sampled. Sampling now runs only once, at the stage that makes the call. The README now lists the processed hook as required. --- extra/modules/optable-targeting/README.md | 64 ++-- .../targeting/model/ModuleContext.java | 3 + .../v1/OptableRawAuctionRequestHook.java | 12 +- ...eTargetingProcessedAuctionRequestHook.java | 4 +- .../v1/core/BidderEnrichmentSampler.java | 4 + .../v1/core/OptableTargetingFlowResolver.java | 130 +++++-- .../v1/core/TargetingRequestExecutor.java | 7 +- .../v1/OptableRawAuctionRequestHookTest.java | 3 +- .../v1/OptableStoredRequestFlowTest.java | 322 ++++++++++++++++++ ...getingProcessedAuctionRequestHookTest.java | 6 +- 10 files changed, 469 insertions(+), 86 deletions(-) create mode 100644 extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java diff --git a/extra/modules/optable-targeting/README.md b/extra/modules/optable-targeting/README.md index 3f9e7473050..e776b4eee21 100644 --- a/extra/modules/optable-targeting/README.md +++ b/extra/modules/optable-targeting/README.md @@ -13,16 +13,17 @@ Targeting API endpoint is configurable per publisher. ### Execution Plan -This module runs at three stages: +This module runs at four stages, and all four hooks are required: * Raw Auction Request: initiates a non-blocking Optable API call early in the auction lifecycle. +* Processed Auction Request: initiates the API call for requests the raw stage could not handle (see below). * Bidder Request: awaits the API response and enriches individual bidder requests with `user.eids` and `user.data`. * Auction Response: injects ad server targeting. -Requests that rely on stored requests (f.e. Prebid Mobile SDK traffic, where bidders and often `app` live in the -stored request) are only merged after the Raw Auction Request stage. When the bidders or `site`/`app` can't be -determined at that stage, the API call is started by the Bidder Request hook on the merged request instead and is -awaited there, so for such traffic the `bidder-request` hook timeout has to cover the whole API roundtrip. +Stored requests and stored imps are merged only after the Raw Auction Request stage. When a request relies on them +(f.e. Prebid Mobile SDK traffic, where the bidders and often `app` live in the stored request), or when its bidders or +`site`/`app` can't be determined at the raw stage, the API call is started by the Processed Auction Request hook on the +merged request instead. Without the `processed-auction-request` hook such requests are not enriched. We recommend defining the execution plan in the account config so the module is only invoked for specific accounts. See below for an example. @@ -58,6 +59,19 @@ hooks: } ] }, + "processed-auction-request": { + "groups": [ + { + "timeout": 50, + "hook-sequence": [ + { + "module-code": "optable-targeting", + "hook-impl-code": "optable-targeting-processed-auction-request-hook" + } + ] + } + ] + }, "bidder-request": { "groups": [ { @@ -141,41 +155,21 @@ Sample module enablement configuration in JSON and YAML formats: ### Migrating from legacy configuration -Previous versions of the module used a `processed-auction-request` hook (alongside the `auction-response` hook) that both -made the API call and enriched the request synchronously in one step, blocking the auction pipeline. The new -configuration replaces it with two hooks: `raw-auction-request` (initiates the API call early) and `bidder-request` -(awaits the result and enriches per-bidder), while the `auction-response` hook remains unchanged. If your execution plan contains the following fragment, it should -be replaced with the `raw-auction-request` and `bidder-request` hooks shown above: - -```json -"processed-auction-request": { - "groups": [ - { - "timeout": 600, - "hook-sequence": [ - { - "module-code": "optable-targeting", - "hook-impl-code": "optable-targeting-processed-auction-request-hook" - } - ] - } - ] -} -``` +Previous versions of the module used only the `processed-auction-request` hook (alongside the `auction-response` +hook), which made the API call and enriched the whole request synchronously, blocking the auction pipeline. To migrate, +keep the `processed-auction-request` hook and add the `raw-auction-request` and `bidder-request` hooks as shown above. -The `processed-auction-request` hook is still supported for backwards compatibility. It detects whether the new hooks -(`raw-auction-request` and `bidder-request`) are present in the execution plan. If both are active, it passes through -immediately without blocking the pipeline. If the new hooks are absent, it falls back to the legacy synchronous -behavior. This means the legacy fragment can be kept during migration without negating the latency benefit of the new -configuration. +With the new hooks present, the `processed-auction-request` hook no longer blocks: it only starts the API call for +requests that were deferred at the raw stage, and the `bidder-request` hook awaits it. Without the new hooks, it keeps +the legacy synchronous behavior. ### Timeout considerations The `bidder-request` hook timeout is used as the timeout budget for the Optable Targeting API call Future that is -initiated in the `raw-auction-request` stage. The API call runs in parallel with other auction processing, so the -effective wait time at the `bidder-request` stage is typically much shorter than the full API roundtrip. The -`raw-auction-request` hook timeout only needs to cover its own lightweight setup (validation, sampling) and can be kept -short. +initiated in the `raw-auction-request` or `processed-auction-request` stage. The API call runs in parallel with other +auction processing, so the effective wait time at the `bidder-request` stage is typically much shorter than the full +API roundtrip. The `raw-auction-request` and `processed-auction-request` hook timeouts only need to cover their own +lightweight setup (validation, sampling) and can be kept short. **Note:** Do not confuse hook timeout value with the module timeout parameter which is optional. The hook timeout value would depend on the cloud/region where the PBS instance is hosted and the latency to reach the Optable's servers. This diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/ModuleContext.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/ModuleContext.java index 64e18dfedaf..7ced31bbe66 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/ModuleContext.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/ModuleContext.java @@ -1,5 +1,6 @@ package org.prebid.server.hooks.modules.optable.targeting.model; +import com.fasterxml.jackson.databind.JsonNode; import io.vertx.core.Future; import lombok.Data; import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; @@ -37,6 +38,8 @@ public class ModuleContext { private boolean isEarlyCallInitializationCompleted = true; + private JsonNode extUserOptable; + private String id5Signature; public static ModuleContext of(AuctionInvocationContext invocationContext) { diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java index 54788097855..979cfc99595 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java @@ -61,7 +61,7 @@ public Future> call(AuctionRequestPayloa } return optableTargetingFlowResolver.resolveAsyncOptableTargetingFlow( - moduleContext, payload, invocationContext, properties, false); + moduleContext, payload, invocationContext, properties); } public static Future> update( @@ -77,16 +77,6 @@ public static Future> update( .build()); } - public static Future> success(ModuleContext moduleContext) { - - return Future.succeededFuture( - InvocationResultImpl.builder() - .status(InvocationStatus.success) - .action(InvocationAction.no_action) - .moduleContext(moduleContext) - .build()); - } - @Override public String code() { return CODE; diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java index 34449d9b049..2fe59efa8fd 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java @@ -41,8 +41,8 @@ public Future> call(AuctionRequestPayloa if (moduleContext.isEarlyCallInitializationCompleted()) { return success(moduleContext); } else { - return optableTargetingFlowResolver.resolveAsyncOptableTargetingFlow( - moduleContext, auctionRequestPayload, invocationContext, properties, true); + return optableTargetingFlowResolver.resolveDeferredOptableTargetingFlow( + moduleContext, auctionRequestPayload, invocationContext, properties); } } diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java index 0872437bc09..2512f8fd354 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/BidderEnrichmentSampler.java @@ -53,6 +53,10 @@ public Set sample(BidRequest bidRequest, OptableTargetingProperties opta .collect(Collectors.toSet()); } + public boolean hasBidders(BidRequest bidRequest) { + return !extractUniqueBidders(bidRequest).isEmpty(); + } + private static int resolvePercentage(BidderAliases aliases, String bidder, int defaultEnrichmentPercentage, Map bidderEnrichmentPercentage) { diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java index 9f881ca06fe..f4385b5c038 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java @@ -1,6 +1,9 @@ package org.prebid.server.hooks.modules.optable.targeting.v1.core; +import com.fasterxml.jackson.databind.JsonNode; import com.iab.openrtb.request.BidRequest; +import com.iab.openrtb.request.Imp; +import com.iab.openrtb.request.User; import io.vertx.core.Future; import org.apache.commons.collections4.CollectionUtils; import org.prebid.server.hooks.execution.v1.InvocationResultImpl; @@ -17,9 +20,14 @@ import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; import org.prebid.server.log.ConditionalLogger; import org.prebid.server.log.LoggerFactory; +import org.prebid.server.proto.openrtb.ext.request.ExtRequest; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtUser; import org.prebid.server.settings.model.Account; +import java.util.Collection; import java.util.Objects; +import java.util.Optional; import java.util.Set; public class OptableTargetingFlowResolver { @@ -27,6 +35,8 @@ public class OptableTargetingFlowResolver { private static final ConditionalLogger conditionalLogger = new ConditionalLogger( LoggerFactory.getLogger(OptableTargetingProcessedAuctionRequestHook.class)); + private static final String OPTABLE_FIELD = "optable"; + private static final String IMP_STORED_REQUEST_PATH = "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/prebid/storedrequest"; private static final String AUCTION_NOT_PROPERLY_CONFIGURED = "Account not properly configured: tenant and/or origin is missing."; @@ -46,30 +56,81 @@ public OptableTargetingFlowResolver(BidderEnrichmentSampler bidderEnrichmentSamp this.logSamplingRate = logSamplingRate; } + /** + * Raw auction request stage. Stored requests and stored imps are merged only after it, so when the request + * relies on them the call is deferred to the processed auction request hook, which sees the merged request. + */ public Future> resolveAsyncOptableTargetingFlow( ModuleContext moduleContext, AuctionRequestPayload payload, AuctionInvocationContext invocationContext, - OptableTargetingProperties properties, - boolean cleanRequestOnFail) { + OptableTargetingProperties properties) { - final BidRequest bidRequest = invocationContext.auctionContext().getBidRequest(); - if (!PropertiesValidator.isTrafficSourceValid(bidRequest, properties)) { - if (cleanRequestOnFail) { - moduleContext.setShouldSkipEnrichment(true); - } + final BidRequest bidRequest = payload.bidRequest(); + if (shouldDeferTargetingCall(bidRequest)) { moduleContext.setEarlyCallInitializationCompleted(false); - return cleanRequestOnFail - ? update(BidRequestCleaner.instance(), moduleContext) - : success(moduleContext); + // the cleaner strips user.ext.optable at this stage, while the deferred call still needs its ids + moduleContext.setExtUserOptable(extUserOptable(bidRequest)); + } else { + startTargetingCall(moduleContext, bidRequest, invocationContext, properties); + } + + return update(BidRequestCleaner.instance(), moduleContext); + } + + /** + * Processed auction request stage: starts the call deferred by the raw auction request hook. + */ + public Future> resolveDeferredOptableTargetingFlow( + ModuleContext moduleContext, + AuctionRequestPayload payload, + AuctionInvocationContext invocationContext, + OptableTargetingProperties properties) { + + final BidRequest bidRequest = withExtUserOptable(payload.bidRequest(), moduleContext.getExtUserOptable()); + moduleContext.setEarlyCallInitializationCompleted(true); + moduleContext.setExtUserOptable(null); + moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); + + startTargetingCall(moduleContext, bidRequest, invocationContext, properties); + + return update(BidRequestCleaner.instance(), moduleContext); + } + + private boolean shouldDeferTargetingCall(BidRequest bidRequest) { + return (bidRequest.getSite() == null && bidRequest.getApp() == null) + || !bidderEnrichmentSampler.hasBidders(bidRequest) + || hasStoredRequest(bidRequest); + } + + private static boolean hasStoredRequest(BidRequest bidRequest) { + final ExtRequest ext = bidRequest.getExt(); + final ExtRequestPrebid prebid = ext != null ? ext.getPrebid() : null; + if (prebid != null && prebid.getStoredrequest() != null) { + return true; + } + + return Optional.ofNullable(bidRequest.getImp()) + .stream() + .flatMap(Collection::stream) + .map(Imp::getExt) + .filter(Objects::nonNull) + .anyMatch(impExt -> !impExt.at(IMP_STORED_REQUEST_PATH).isMissingNode()); + } + + private void startTargetingCall(ModuleContext moduleContext, + BidRequest bidRequest, + AuctionInvocationContext invocationContext, + OptableTargetingProperties properties) { + + if (!PropertiesValidator.isTrafficSourceValid(bidRequest, properties)) { + moduleContext.setShouldSkipEnrichment(true); + return; } final Set biddersToEnrich = bidderEnrichmentSampler.sample(bidRequest, properties); if (CollectionUtils.isEmpty(biddersToEnrich)) { - moduleContext.setEarlyCallInitializationCompleted(false); - return cleanRequestOnFail - ? update(BidRequestCleaner.instance(), moduleContext) - : success(moduleContext); + return; } moduleContext.setBiddersToEnrich(biddersToEnrich); @@ -77,15 +138,34 @@ public Future> resolveAsyncOptableTarget final long crossHookFutureTimeout = hooksExecutionPlan.getOptableTargetingBidderRequestTimeout(account); - final Future optableTargetingCall = targetingRequestExecutor.makeRequest( - payload, + moduleContext.setOptableTargetingCall(targetingRequestExecutor.makeRequest( + bidRequest, invocationContext, properties, - crossHookFutureTimeout); + crossHookFutureTimeout)); + } - moduleContext.setOptableTargetingCall(optableTargetingCall); + private static JsonNode extUserOptable(BidRequest bidRequest) { + final User user = bidRequest.getUser(); + final ExtUser extUser = user != null ? user.getExt() : null; + return extUser != null ? extUser.getProperty(OPTABLE_FIELD) : null; + } - return update(BidRequestCleaner.instance(), moduleContext); + private static BidRequest withExtUserOptable(BidRequest bidRequest, JsonNode optable) { + if (optable == null) { + return bidRequest; + } + + final User user = bidRequest.getUser(); + final ExtUser extUser = user != null ? user.getExt() : null; + final ExtUser restoredExtUser = extUser != null ? extUser.toBuilder().build() : ExtUser.builder().build(); + if (extUser != null) { + restoredExtUser.addProperties(extUser.getProperties()); + } + restoredExtUser.addProperty(OPTABLE_FIELD, optable); + + final User restoredUser = (user != null ? user.toBuilder() : User.builder()).ext(restoredExtUser).build(); + return bidRequest.toBuilder().user(restoredUser).build(); } /** @@ -170,7 +250,7 @@ private Future resolvePreEarlyNetworkCall( } return targetingRequestExecutor.makeRequest( - payload, + payload.bidRequest(), invocationContext, properties, null); @@ -202,14 +282,4 @@ private static Future> updateWithAnalyti .moduleContext(moduleContext) .build()); } - - public static Future> success(ModuleContext moduleContext) { - - return Future.succeededFuture( - InvocationResultImpl.builder() - .status(InvocationStatus.success) - .action(InvocationAction.no_action) - .moduleContext(moduleContext) - .build()); - } } diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java index 8a93df9fbdf..1b2dbfdcd2e 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java @@ -20,7 +20,6 @@ import org.prebid.server.hooks.modules.optable.targeting.model.openrtb.TargetingResult; import org.prebid.server.hooks.modules.optable.targeting.v1.OptableTargetingModule; import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; -import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; import java.util.Objects; @@ -42,12 +41,12 @@ public TargetingRequestExecutor(OptableTargeting optableTargeting, this.logSamplingRate = logSamplingRate; } - public Future makeRequest(AuctionRequestPayload payload, + public Future makeRequest(BidRequest bidRequest, AuctionInvocationContext invocationContext, OptableTargetingProperties properties, Long apiTimeout) { - final BidRequest bidRequest = applyActivityRestrictions(payload.bidRequest(), invocationContext); + final BidRequest restrictedBidRequest = applyActivityRestrictions(bidRequest, invocationContext); final Timeout timeout = apiTimeout == null ? getHookTimeout(invocationContext) @@ -57,7 +56,7 @@ public Future makeRequest(AuctionRequestPayload payload, properties.getTimeout(), logSamplingRate); - return optableTargeting.getTargeting(properties, bidRequest, attributes, timeout); + return optableTargeting.getTargeting(properties, restrictedBidRequest, attributes, timeout); } private static Timeout getHookTimeout(AuctionInvocationContext invocationContext) { diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java index 6d527b0f663..97d29cecfcf 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java @@ -72,6 +72,7 @@ public void setUp() { when(invocationContext.timeout()).thenReturn(timeout); when(activityInfrastructure.isAllowed(any(), any())).thenReturn(true); when(timeout.remaining()).thenReturn(1000L); + when(bidderEnrichmentSampler.hasBidders(any())).thenReturn(true); } private OptableTargetingFlowResolver givenEarlyOptableCallResolver() { @@ -212,7 +213,7 @@ public void shouldNotInjectEarlyNetworkCallToModuleContextWhenNoBiddersToEnrich( final ModuleContext moduleContext = cxt.result(); assertThat(moduleContext.isShouldSkipEnrichment()).isFalse(); assertThat(moduleContext.getOptableTargetingCall()).isNull(); - assertThat(moduleContext.isEarlyCallInitializationCompleted()).isFalse(); + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isTrue(); }); vertxTestContext.completeNow(); }); diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java new file mode 100644 index 00000000000..417832b876c --- /dev/null +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java @@ -0,0 +1,322 @@ +package org.prebid.server.hooks.modules.optable.targeting.v1; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.iab.openrtb.request.App; +import com.iab.openrtb.request.BidRequest; +import com.iab.openrtb.request.Imp; +import io.vertx.core.Future; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.prebid.server.activity.infrastructure.ActivityInfrastructure; +import org.prebid.server.auction.model.AuctionContext; +import org.prebid.server.auction.privacy.enforcement.mask.UserFpdActivityMask; +import org.prebid.server.bidder.BidderCatalog; +import org.prebid.server.execution.timeout.Timeout; +import org.prebid.server.execution.timeout.TimeoutFactory; +import org.prebid.server.hooks.execution.model.ExecutionPlan; +import org.prebid.server.hooks.execution.v1.auction.AuctionRequestPayloadImpl; +import org.prebid.server.hooks.execution.v1.bidder.BidderRequestPayloadImpl; +import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; +import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.AliasesResolver; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidderEnrichmentSampler; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.CompositeHookExecutionPlan; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.ConfigResolver; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargeting; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargetingFlowResolver; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.TargetingRequestExecutor; +import org.prebid.server.hooks.v1.InvocationAction; +import org.prebid.server.hooks.v1.InvocationResult; +import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; +import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; +import org.prebid.server.hooks.v1.bidder.BidderInvocationContext; +import org.prebid.server.hooks.v1.bidder.BidderRequestPayload; + +import java.util.List; +import java.util.Map; +import java.util.function.IntSupplier; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** + * Walks a request through the raw auction request, processed auction request and bidder request hooks the way + * the core does for requests that rely on stored requests: the raw stage sees only stored request ids, the later + * stages see the merged request. + */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class OptableStoredRequestFlowTest extends BaseOptableTest { + + @Mock + private OptableTargeting optableTargeting; + @Mock + private UserFpdActivityMask userFpdActivityMask; + @Mock + private ActivityInfrastructure activityInfrastructure; + @Mock + private AuctionRequestPayload rawPayload; + @Mock + private AuctionInvocationContext rawInvocationContext; + @Mock + private AuctionInvocationContext processedInvocationContext; + @Mock + private BidderInvocationContext bidderInvocationContext; + @Mock + private Timeout timeout; + @Mock + private TimeoutFactory timeoutFactory; + @Mock + private BidderCatalog bidderCatalog; + @Mock + private IntSupplier randomSupplier; + + private OptableTargetingProperties properties; + + private OptableRawAuctionRequestHook rawHook; + private OptableTargetingProcessedAuctionRequestHook processedHook; + private OptableBidderRequestHook bidderHook; + private InvocationResult rawResult; + + @BeforeEach + public void setUp() { + when(userFpdActivityMask.maskUser(any(), anyBoolean(), anyBoolean())) + .thenAnswer(answer -> answer.getArgument(0)); + when(userFpdActivityMask.maskDevice(any(), anyBoolean(), anyBoolean())) + .thenAnswer(answer -> answer.getArgument(0)); + when(activityInfrastructure.isAllowed(any(), any())).thenReturn(true); + when(timeout.remaining()).thenReturn(1000L); + when(rawInvocationContext.timeout()).thenReturn(timeout); + when(rawInvocationContext.accountConfig()).thenAnswer(invocation -> mapper.valueToTree(properties)); + when(processedInvocationContext.timeout()).thenReturn(timeout); + when(processedInvocationContext.accountConfig()).thenAnswer(invocation -> mapper.valueToTree(properties)); + when(bidderInvocationContext.timeout()).thenReturn(timeout); + when(optableTargeting.getTargeting(any(), any(), any(), any())) + .thenReturn(Future.succeededFuture(givenTargetingResult())); + + properties = givenOptableTargetingProperties(false); + properties.setEnrichApp(true); + + final OptableTargetingFlowResolver flowResolver = new OptableTargetingFlowResolver( + BidderEnrichmentSampler.of(AliasesResolver.of(bidderCatalog), randomSupplier), + new TargetingRequestExecutor(optableTargeting, userFpdActivityMask, timeoutFactory, 0.01), + CompositeHookExecutionPlan.of(ExecutionPlan.empty()), + 0.01); + final ConfigResolver configResolver = new ConfigResolver(mapper, jsonMerger, properties); + rawHook = new OptableRawAuctionRequestHook(configResolver, flowResolver, 0.01); + processedHook = new OptableTargetingProcessedAuctionRequestHook(configResolver, flowResolver); + bidderHook = new OptableBidderRequestHook(); + } + + @Test + public void shouldEnrichBiddersThatComeFromStoredImps() { + // given + final BidRequest rawRequest = givenBidRequest().toBuilder().imp(List.of(givenStoredImp())).build(); + + // when + final ModuleContext moduleContext = callRawHook(rawRequest); + + // then + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isFalse(); + verifyNoInteractions(optableTargeting); + + // when + final BidRequest mergedRequest = givenCleaned(rawRequest).toBuilder() + .imp(List.of(givenImp("bidderA", "bidderB"))) + .build(); + callProcessedHook(mergedRequest, moduleContext); + final InvocationResult bidderAResult = + callBidderHook("bidderA", mergedRequest, moduleContext); + final InvocationResult bidderBResult = + callBidderHook("bidderB", mergedRequest, moduleContext); + + // then + assertThat(bidderAResult.action()).isEqualTo(InvocationAction.update); + assertThat(bidderBResult.action()).isEqualTo(InvocationAction.update); + assertThat(moduleContext.getBiddersToEnrich()).containsExactlyInAnyOrder("bidderA", "bidderB"); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(BidRequest.class); + verify(optableTargeting, times(1)).getTargeting(any(), captor.capture(), any(), any()); + final ObjectNode optable = (ObjectNode) captor.getValue().getUser().getExt().getProperty("optable"); + assertThat(optable.get("email").asText()).isEqualTo("email"); + assertThat(mergedRequest.getUser().getExt().getProperty("optable")).isNull(); + + final BidRequest enriched = bidderAResult.payloadUpdate() + .apply(BidderRequestPayloadImpl.of(mergedRequest)) + .bidRequest(); + assertThat(enriched.getUser().getEids()).isNotEmpty(); + } + + @Test + public void shouldEnrichWhenAppComesFromStoredRequest() { + // given + final BidRequest rawRequest = givenBidRequest().toBuilder() + .site(null) + .imp(List.of(givenImp("bidderA"))) + .build(); + + // when + final ModuleContext moduleContext = callRawHook(rawRequest); + + // then + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isFalse(); + assertThat(moduleContext.isShouldSkipEnrichment()).isFalse(); + + // when + final BidRequest mergedRequest = givenCleaned(rawRequest).toBuilder() + .app(App.builder().bundle("bundle").build()) + .build(); + callProcessedHook(mergedRequest, moduleContext); + final InvocationResult result = + callBidderHook("bidderA", mergedRequest, moduleContext); + + // then + assertThat(result.action()).isEqualTo(InvocationAction.update); + assertThat(moduleContext.isShouldSkipEnrichment()).isFalse(); + verify(optableTargeting).getTargeting(any(), any(), any(), any()); + } + + @Test + public void shouldHonourDisabledTrafficSourceOnMergedRequest() { + // given + properties.setEnrichApp(false); + final BidRequest rawRequest = givenBidRequest().toBuilder() + .site(null) + .imp(List.of(givenImp("bidderA"))) + .build(); + + // when + final ModuleContext moduleContext = callRawHook(rawRequest); + final BidRequest mergedRequest = givenCleaned(rawRequest).toBuilder() + .app(App.builder().bundle("bundle").build()) + .build(); + callProcessedHook(mergedRequest, moduleContext); + final InvocationResult result = + callBidderHook("bidderA", mergedRequest, moduleContext); + + // then + assertThat(result.action()).isEqualTo(InvocationAction.no_action); + assertThat(moduleContext.isShouldSkipEnrichment()).isTrue(); + verifyNoInteractions(optableTargeting); + } + + @Test + public void shouldNotSampleAgainWhenSamplingRejectedAllBiddersAtRawStage() { + // given + properties.setEnrichmentPercentage(10); + when(randomSupplier.getAsInt()).thenReturn(50, 0); + final BidRequest rawRequest = givenBidRequest().toBuilder().imp(List.of(givenImp("bidderA"))).build(); + + // when + final ModuleContext moduleContext = callRawHook(rawRequest); + callProcessedHook(givenCleaned(rawRequest), moduleContext); + final InvocationResult result = + callBidderHook("bidderA", givenCleaned(rawRequest), moduleContext); + + // then + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isTrue(); + assertThat(result.action()).isEqualTo(InvocationAction.no_action); + verify(randomSupplier, times(1)).getAsInt(); + verifyNoInteractions(optableTargeting); + } + + @Test + public void shouldDeferWhenOnlySomeImpsRelyOnStoredImps() { + // given + final BidRequest rawRequest = givenBidRequest().toBuilder() + .imp(List.of(givenImp("bidderA"), givenStoredImp())) + .build(); + + // when + final ModuleContext moduleContext = callRawHook(rawRequest); + + // then + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isFalse(); + verifyNoInteractions(optableTargeting); + + // when + final BidRequest mergedRequest = givenCleaned(rawRequest).toBuilder() + .imp(List.of(givenImp("bidderA"), givenImp("bidderB"))) + .build(); + callProcessedHook(mergedRequest, moduleContext); + final InvocationResult bidderBResult = + callBidderHook("bidderB", mergedRequest, moduleContext); + + // then + assertThat(moduleContext.getBiddersToEnrich()).containsExactlyInAnyOrder("bidderA", "bidderB"); + assertThat(bidderBResult.action()).isEqualTo(InvocationAction.update); + } + + @Test + public void shouldCleanRequestAtRawStageWhenCallIsDeferred() { + // given + final BidRequest rawRequest = givenBidRequest().toBuilder().imp(List.of(givenStoredImp())).build(); + + // when + callRawHook(rawRequest); + + // then + assertThat(rawResult.action()).isEqualTo(InvocationAction.update); + assertThat(givenCleaned(rawRequest).getUser().getExt().getProperty("optable")).isNull(); + } + + private void callProcessedHook(BidRequest mergedRequest, ModuleContext moduleContext) { + when(processedInvocationContext.moduleContext()).thenReturn(moduleContext); + when(processedInvocationContext.auctionContext()).thenReturn(givenAuctionContext(mergedRequest)); + + processedHook.call(AuctionRequestPayloadImpl.of(mergedRequest), processedInvocationContext).result(); + } + + private ModuleContext callRawHook(BidRequest bidRequest) { + when(rawPayload.bidRequest()).thenReturn(bidRequest); + when(rawInvocationContext.auctionContext()).thenReturn(givenAuctionContext(bidRequest)); + + rawResult = rawHook.call(rawPayload, rawInvocationContext).result(); + return (ModuleContext) rawResult.moduleContext(); + } + + private InvocationResult callBidderHook(String bidder, + BidRequest mergedRequest, + ModuleContext moduleContext) { + + when(bidderInvocationContext.bidder()).thenReturn(bidder); + when(bidderInvocationContext.moduleContext()).thenReturn(moduleContext); + when(bidderInvocationContext.auctionContext()).thenReturn(givenAuctionContext(mergedRequest)); + + return bidderHook.call(BidderRequestPayloadImpl.of(mergedRequest), bidderInvocationContext).result(); + } + + private AuctionContext givenAuctionContext(BidRequest bidRequest) { + return givenAuctionContext(activityInfrastructure, timeout).toBuilder().bidRequest(bidRequest).build(); + } + + private BidRequest givenCleaned(BidRequest bidRequest) { + return rawResult.payloadUpdate().apply(AuctionRequestPayloadImpl.of(bidRequest)).bidRequest(); + } + + private Imp givenStoredImp() { + final ObjectNode ext = mapper.createObjectNode(); + ext.putObject("prebid").putObject("storedrequest").put("id", "storedImpId"); + return Imp.builder().id("impId").ext(ext).build(); + } + + private Imp givenImp(String... bidders) { + final ObjectNode ext = mapper.createObjectNode(); + final ObjectNode bidderNode = ext.putObject("prebid").putObject("bidder"); + for (String bidder : bidders) { + bidderNode.set(bidder, mapper.valueToTree(Map.of("param", "value"))); + } + return Imp.builder().id("impId").ext(ext).build(); + } +} diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java index a942f570803..893ae90060a 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java @@ -228,10 +228,9 @@ void callShouldReturnResultWithNoActionWhenEarlyOptableCallIsEnabledAndInitializ when(optableTargeting.getTargeting(any(), any(), any(), any())) .thenReturn(Future.succeededFuture(givenTargetingResult())); when(invocationContext.moduleContext()).thenReturn(moduleContext); - when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); moduleContext.setOptableTargetingCall( targetingRequestExecutor.makeRequest( - auctionRequestPayload, + givenBidRequest(), invocationContext, givenOptableTargetingProperties("key", "tenant", "origin", false), null)); @@ -303,6 +302,7 @@ void callShouldNotRetryEarlyNetworkCallInitializationWhenNoBiddersToEnrich() { moduleContext.setEarlyCallInitializationCompleted(false); when(invocationContext.moduleContext()).thenReturn(moduleContext); + when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); when(bidderEnrichmentSampler.sample(any(), any())).thenReturn(Set.of()); // when @@ -319,7 +319,7 @@ void callShouldNotRetryEarlyNetworkCallInitializationWhenNoBiddersToEnrich() { .returns(InvocationAction.update, InvocationResult::action) .extracting(InvocationResult::errors).isNull(); assertThat(moduleContext.getOptableTargetingCall()).isNull(); - assertThat(moduleContext.isEarlyCallInitializationCompleted()).isFalse(); + assertThat(moduleContext.isEarlyCallInitializationCompleted()).isTrue(); } @Test From 7d38d04c8243f369c8b9bfd5e6daa8de11ed0795 Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 20:04:58 +0200 Subject: [PATCH 5/8] optable-targeting: Always clean user.ext.optable when a hook fails An exception in the raw or processed auction request hook failed the hook without applying the cleaner, so user.ext.optable reached the bidders. Both hooks now catch it, log it and still return the cleaner update. The call and the bidders to enrich are set together, so the bidder request hook never sees bidders without a call to await. --- .../config/OptableTargetingConfig.java | 3 +- .../v1/OptableRawAuctionRequestHook.java | 23 +++++++++++- ...eTargetingProcessedAuctionRequestHook.java | 29 ++++++++++++++- .../v1/core/OptableTargetingFlowResolver.java | 15 ++++++-- .../v1/OptableRawAuctionRequestHookTest.java | 24 +++++++++++++ .../v1/OptableStoredRequestFlowTest.java | 2 +- ...getingProcessedAuctionRequestHookTest.java | 35 +++++++++++++++---- 7 files changed, 117 insertions(+), 14 deletions(-) diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java index ebdc4f0ee0f..08ecdc310ff 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java @@ -119,7 +119,8 @@ OptableTargetingModule optableTargetingModule(ConfigResolver configResolver, logSamplingRate), new OptableTargetingProcessedAuctionRequestHook( configResolver, - earlyOptableCallResolver), + earlyOptableCallResolver, + logSamplingRate), new OptableBidderRequestHook(), new OptableTargetingAuctionResponseHook( configResolver, diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java index 979cfc99595..6efbdf59a53 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHook.java @@ -44,10 +44,31 @@ public OptableRawAuctionRequestHook(ConfigResolver configResolver, public Future> call(AuctionRequestPayload payload, AuctionInvocationContext invocationContext) { - final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig()); final ModuleContext moduleContext = new ModuleContext(); moduleContext.setEarlyNetworkCallEnabled(true); moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); + + // whatever goes wrong here, the cleaner has to be applied, or user.ext.optable ids reach the bidders + try { + return resolveTargetingFlow(payload, invocationContext, moduleContext); + } catch (RuntimeException e) { + conditionalLogger.error("Failed to initiate Optable targeting call: " + e.getMessage(), logSamplingRate); + + moduleContext.setEarlyCallInitializationCompleted(true); + moduleContext.setExtUserOptable(null); + moduleContext.failWithExecutionTime( + System.currentTimeMillis() - moduleContext.getCallTargetingAPITimestamp()); + + return update(BidRequestCleaner.instance(), moduleContext); + } + } + + private Future> resolveTargetingFlow( + AuctionRequestPayload payload, + AuctionInvocationContext invocationContext, + ModuleContext moduleContext) { + + final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig()); moduleContext.setOptableTargetingProperties(properties); if (!PropertiesValidator.isValid(properties)) { diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java index 2fe59efa8fd..1f1590c90ff 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java @@ -12,6 +12,8 @@ import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook; +import org.prebid.server.log.ConditionalLogger; +import org.prebid.server.log.LoggerFactory; import java.util.Objects; @@ -19,15 +21,22 @@ public class OptableTargetingProcessedAuctionRequestHook implements ProcessedAuc public static final String CODE = "optable-targeting-processed-auction-request-hook"; + private static final ConditionalLogger conditionalLogger = new ConditionalLogger( + LoggerFactory.getLogger(OptableTargetingProcessedAuctionRequestHook.class)); + private final ConfigResolver configResolver; private final OptableTargetingFlowResolver optableTargetingFlowResolver; + private final double logSamplingRate; + public OptableTargetingProcessedAuctionRequestHook(ConfigResolver configResolver, - OptableTargetingFlowResolver earlyOptableCallResolver) { + OptableTargetingFlowResolver earlyOptableCallResolver, + double logSamplingRate) { this.configResolver = Objects.requireNonNull(configResolver); this.optableTargetingFlowResolver = Objects.requireNonNull(earlyOptableCallResolver); + this.logSamplingRate = logSamplingRate; } @Override @@ -35,6 +44,24 @@ public Future> call(AuctionRequestPayloa AuctionInvocationContext invocationContext) { final ModuleContext moduleContext = ModuleContext.of(invocationContext); + + // whatever goes wrong here, the cleaner has to be applied, or user.ext.optable ids reach the bidders + try { + return resolveTargetingFlow(auctionRequestPayload, invocationContext, moduleContext); + } catch (RuntimeException e) { + conditionalLogger.error("Failed to initiate Optable targeting call: " + e.getMessage(), logSamplingRate); + + moduleContext.setEarlyCallInitializationCompleted(true); + moduleContext.setExtUserOptable(null); + return optableTargetingFlowResolver.failed(moduleContext); + } + } + + private Future> resolveTargetingFlow( + AuctionRequestPayload auctionRequestPayload, + AuctionInvocationContext invocationContext, + ModuleContext moduleContext) { + final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig()); if (moduleContext.isEarlyNetworkCallEnabled()) { diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java index f4385b5c038..ba7abf75c94 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java @@ -133,16 +133,19 @@ private void startTargetingCall(ModuleContext moduleContext, return; } - moduleContext.setBiddersToEnrich(biddersToEnrich); final Account account = invocationContext.auctionContext().getAccount(); final long crossHookFutureTimeout = hooksExecutionPlan.getOptableTargetingBidderRequestTimeout(account); - moduleContext.setOptableTargetingCall(targetingRequestExecutor.makeRequest( + final Future optableTargetingCall = targetingRequestExecutor.makeRequest( bidRequest, invocationContext, properties, - crossHookFutureTimeout)); + crossHookFutureTimeout); + + // set together, so that the bidder request hook never sees bidders without a call to await + moduleContext.setBiddersToEnrich(biddersToEnrich); + moduleContext.setOptableTargetingCall(optableTargetingCall); } private static JsonNode extUserOptable(BidRequest bidRequest) { @@ -269,6 +272,12 @@ private static Future> update( .build()); } + public Future> failed(ModuleContext moduleContext) { + moduleContext.failWithExecutionTime( + moduleContext.getCallTargetingAPITimestamp() > 0 ? calcAPICallExecutionTime(moduleContext) : 0); + return updateWithAnalytics(BidRequestCleaner.instance(), moduleContext); + } + private static Future> updateWithAnalytics( PayloadUpdate payloadUpdate, ModuleContext moduleContext) { diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java index 97d29cecfcf..64caa7fdbb0 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java @@ -17,13 +17,17 @@ import org.prebid.server.execution.timeout.TimeoutFactory; import org.prebid.server.hooks.execution.model.ExecutionPlan; import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; +import org.prebid.server.hooks.modules.optable.targeting.model.Status; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidRequestCleaner; import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidderEnrichmentSampler; import org.prebid.server.hooks.modules.optable.targeting.v1.core.CompositeHookExecutionPlan; import org.prebid.server.hooks.modules.optable.targeting.v1.core.ConfigResolver; import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargetingFlowResolver; import org.prebid.server.hooks.modules.optable.targeting.v1.core.TargetingRequestExecutor; import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargeting; +import org.prebid.server.hooks.v1.InvocationAction; import org.prebid.server.hooks.v1.InvocationResult; +import org.prebid.server.hooks.v1.InvocationStatus; import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; @@ -32,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @MockitoSettings(strictness = Strictness.LENIENT) @@ -218,4 +223,23 @@ public void shouldNotInjectEarlyNetworkCallToModuleContextWhenNoBiddersToEnrich( vertxTestContext.completeNow(); }); } + + @Test + public void shouldCleanRequestWhenInitiatingCallFails() { + // given + final ConfigResolver failingConfigResolver = mock(ConfigResolver.class); + when(failingConfigResolver.resolve(any())).thenThrow(new IllegalStateException("failure")); + target = new OptableRawAuctionRequestHook(failingConfigResolver, givenEarlyOptableCallResolver(), 0.01); + + // when + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); + + // then + assertThat(result.status()).isEqualTo(InvocationStatus.success); + assertThat(result.action()).isEqualTo(InvocationAction.update); + assertThat(result.payloadUpdate()).isInstanceOf(BidRequestCleaner.class); + assertThat(((ModuleContext) result.moduleContext()).getEnrichRequestStatus().getStatus()) + .isEqualTo(Status.FAIL); + } } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java index 417832b876c..baf9e2957a1 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java @@ -115,7 +115,7 @@ public void setUp() { 0.01); final ConfigResolver configResolver = new ConfigResolver(mapper, jsonMerger, properties); rawHook = new OptableRawAuctionRequestHook(configResolver, flowResolver, 0.01); - processedHook = new OptableTargetingProcessedAuctionRequestHook(configResolver, flowResolver); + processedHook = new OptableTargetingProcessedAuctionRequestHook(configResolver, flowResolver, 0.01); bidderHook = new OptableBidderRequestHook(); } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java index 893ae90060a..1e5d45b21bb 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java @@ -27,6 +27,7 @@ import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; import org.prebid.server.hooks.modules.optable.targeting.model.Status; import org.prebid.server.hooks.modules.optable.targeting.model.openrtb.TargetingResult; +import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidRequestCleaner; import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidderEnrichmentSampler; import org.prebid.server.hooks.modules.optable.targeting.v1.core.CompositeHookExecutionPlan; import org.prebid.server.hooks.modules.optable.targeting.v1.core.ConfigResolver; @@ -47,6 +48,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -90,7 +92,7 @@ void setUp() { targetingRequestExecutor = new TargetingRequestExecutor( optableTargeting, userFpdActivityMask, timeoutFactory, 0.01); target = new OptableTargetingProcessedAuctionRequestHook( - configResolver, givenFlowResolver(ExecutionPlan.empty())); + configResolver, givenFlowResolver(ExecutionPlan.empty()), 0.01); when(invocationContext.accountConfig()).thenReturn(givenAccountConfig(true)); when(invocationContext.auctionContext()).thenReturn( @@ -224,7 +226,7 @@ void callShouldReturnResultWithNoActionWhenEarlyOptableCallIsEnabledAndInitializ moduleContext.setEarlyCallInitializationCompleted(true); target = new OptableTargetingProcessedAuctionRequestHook( configResolver, - givenFlowResolver(givenExecutionPlan(true, false))); + givenFlowResolver(givenExecutionPlan(true, false)), 0.01); when(optableTargeting.getTargeting(any(), any(), any(), any())) .thenReturn(Future.succeededFuture(givenTargetingResult())); when(invocationContext.moduleContext()).thenReturn(moduleContext); @@ -327,7 +329,7 @@ void callShouldReturnResultWithEnrichedBidRequestWhenBothHooksAreAbsent() { // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, - givenFlowResolver(givenExecutionPlan(false, false))); + givenFlowResolver(givenExecutionPlan(false, false)), 0.01); when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); when(optableTargeting.getTargeting(any(), any(), any(), any())) .thenReturn(Future.succeededFuture(givenTargetingResult())); @@ -364,7 +366,7 @@ void callShouldReturnResultWithEnrichedBidRequestWhenOnlyBidderRequestHookIsPres // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, - givenFlowResolver(givenExecutionPlan(false, true))); + givenFlowResolver(givenExecutionPlan(false, true)), 0.01); when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); when(optableTargeting.getTargeting(any(), any(), any(), any())) .thenReturn(Future.succeededFuture(givenTargetingResult())); @@ -401,7 +403,7 @@ void callShouldReturnResultWithoutEnrichedBidRequestWhenBothHooksArePresent() { // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, - givenFlowResolver(givenExecutionPlan(true, true))); + givenFlowResolver(givenExecutionPlan(true, true)), 0.01); when(invocationContext.moduleContext()).thenReturn(new ModuleContext()); // when @@ -433,7 +435,7 @@ void callShouldReturnFailWhenOriginIsAbsentInAccountConfiguration() { jsonMerger, givenOptableTargetingProperties("key", "tenant", null, false)); target = new OptableTargetingProcessedAuctionRequestHook( - configResolver, givenFlowResolver(ExecutionPlan.empty())); + configResolver, givenFlowResolver(ExecutionPlan.empty()), 0.01); when(invocationContext.accountConfig()) .thenReturn(givenAccountConfig("key", "tenant", null, true)); @@ -462,7 +464,7 @@ void callShouldReturnFailWhenTenantIsAbsentInAccountConfiguration() { jsonMerger, givenOptableTargetingProperties("key", null, "origin", false)); target = new OptableTargetingProcessedAuctionRequestHook( - configResolver, givenFlowResolver(ExecutionPlan.empty())); + configResolver, givenFlowResolver(ExecutionPlan.empty()), 0.01); when(invocationContext.accountConfig()) .thenReturn(givenAccountConfig("key", null, null, true)); @@ -577,4 +579,23 @@ private ExecutionPlan givenExecutionPlan(boolean hasRawAuctionRequestHook, boole return ExecutionPlan.of(null, Map.of(HookHttpEndpoint.POST_AUCTION, endpointExecutionPlan)); } + + @Test + void callShouldCleanRequestWhenResolvingFlowFails() { + // given + final ConfigResolver failingConfigResolver = mock(ConfigResolver.class); + when(failingConfigResolver.resolve(any())).thenThrow(new IllegalStateException("failure")); + target = new OptableTargetingProcessedAuctionRequestHook( + failingConfigResolver, givenFlowResolver(ExecutionPlan.empty()), 0.01); + + // when + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); + + // then + assertThat(result.action()).isEqualTo(InvocationAction.update); + assertThat(result.payloadUpdate()).isInstanceOf(BidRequestCleaner.class); + assertThat(((ModuleContext) result.moduleContext()).getEnrichRequestStatus().getStatus()) + .isEqualTo(Status.FAIL); + } } From b446de9517d117607aecbe9d0521e51b17b45632 Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 20:08:36 +0200 Subject: [PATCH 6/8] optable-targeting: Resolve hooks per endpoint and bound the call by api-timeout CompositeHookExecutionPlan ignored hooks.default-account-execution-plan, only looked at /openrtb2/auction, took the timeout of the first group of the bidder request stage and cached the per-account result forever. It now only tells whether the bidder request hook is configured for the request, combining the host plan with the account plan (or the default account plan) for the request's endpoint, without caching. Whether the raw hook has run comes from the module context. The call awaited by the bidder request hook is bound by the new api-timeout parameter, or by the time remaining for the auction. The raw stage never runs for amp and video requests, so when the bidder request hook is configured the processed hook now starts the call there instead of passing through without one. When the raw hook has run and the bidder request hook is absent, the processed hook awaits the early call and enriches the request, as before. --- extra/modules/optable-targeting/README.md | 32 +- .../config/OptableTargetingConfig.java | 15 +- .../config/OptableTargetingProperties.java | 3 + ...eTargetingProcessedAuctionRequestHook.java | 36 +- .../v1/core/CompositeHookExecutionPlan.java | 126 ++----- .../v1/core/OptableTargetingFlowResolver.java | 120 +++--- .../v1/core/TargetingRequestExecutor.java | 25 +- .../optable/targeting/v1/BaseOptableTest.java | 16 + .../v1/OptableRawAuctionRequestHookTest.java | 2 +- .../v1/OptableStoredRequestFlowTest.java | 2 +- ...getingProcessedAuctionRequestHookTest.java | 94 +++-- .../core/CompositeHookExecutionPlanTest.java | 344 +++--------------- 12 files changed, 260 insertions(+), 555 deletions(-) diff --git a/extra/modules/optable-targeting/README.md b/extra/modules/optable-targeting/README.md index e776b4eee21..ecec5445f0c 100644 --- a/extra/modules/optable-targeting/README.md +++ b/extra/modules/optable-targeting/README.md @@ -25,6 +25,10 @@ Stored requests and stored imps are merged only after the Raw Auction Request st `site`/`app` can't be determined at the raw stage, the API call is started by the Processed Auction Request hook on the merged request instead. Without the `processed-auction-request` hook such requests are not enriched. +The Raw Auction Request stage does not run for the `/openrtb2/amp` and `/openrtb2/video` endpoints. To enrich those, add +the `processed-auction-request` and `bidder-request` hooks to their execution plans as well: the API call is then +started by the Processed Auction Request hook. + We recommend defining the execution plan in the account config so the module is only invoked for specific accounts. See below for an example. @@ -159,21 +163,26 @@ Previous versions of the module used only the `processed-auction-request` hook ( hook), which made the API call and enriched the whole request synchronously, blocking the auction pipeline. To migrate, keep the `processed-auction-request` hook and add the `raw-auction-request` and `bidder-request` hooks as shown above. -With the new hooks present, the `processed-auction-request` hook no longer blocks: it only starts the API call for -requests that were deferred at the raw stage, and the `bidder-request` hook awaits it. Without the new hooks, it keeps -the legacy synchronous behavior. +When the `bidder-request` hook is in the execution plan, the `processed-auction-request` hook no longer blocks: it only +starts the API call for requests the raw stage deferred or did not see, and the `bidder-request` hook awaits it. Without +the `bidder-request` hook, it keeps the legacy synchronous behavior. ### Timeout considerations -The `bidder-request` hook timeout is used as the timeout budget for the Optable Targeting API call Future that is -initiated in the `raw-auction-request` or `processed-auction-request` stage. The API call runs in parallel with other -auction processing, so the effective wait time at the `bidder-request` stage is typically much shorter than the full -API roundtrip. The `raw-auction-request` and `processed-auction-request` hook timeouts only need to cover their own -lightweight setup (validation, sampling) and can be kept short. +The Optable Targeting API call is initiated in the `raw-auction-request` or `processed-auction-request` stage and runs +in parallel with other auction processing, so the effective wait time at the `bidder-request` stage is typically much +shorter than the full API roundtrip. There are two limits on it: + +* the `api-timeout` module parameter limits the call itself. When it is not set, the call is limited by the time + remaining for the auction (`tmax`). +* the `bidder-request` hook timeout limits how long a bidder request waits for the result of the call. + +The `raw-auction-request` and `processed-auction-request` hook timeouts only need to cover their own lightweight setup +(validation, sampling) and can be kept short. -**Note:** Do not confuse hook timeout value with the module timeout parameter which is optional. The hook timeout value -would depend on the cloud/region where the PBS instance is hosted and the latency to reach the Optable's servers. This -will need to be verified experimentally upon deployment. +**Note:** Do not confuse these with the module `timeout` parameter, which is an optional hint passed to the Targeting +API. The `api-timeout` and the hook timeout values would depend on the cloud/region where the PBS instance is hosted +and the latency to reach the Optable's servers. This will need to be verified experimentally upon deployment. The timeout value for the `auction-response` can be set to 10 ms - usually it will be sub-millisecond time as there are no HTTP calls made in this hook - Optable-specific keywords are cached on earlier stages and retrieved from the module @@ -201,6 +210,7 @@ would result in this nesting in the JSON configuration: | ppid-mapping | no | map | none | This specifies PPID source (`user.ext.eids[].source`) to a custom identifier prefix mapping, f.e. `{"example.com" : "c"}`. See the section on ID Mapping below for more detail. | | adserver-targeting | no | boolean | false | If set to true - will add the Optable-specific adserver targeting keywords into the PBS response for every `seatbid[].bid[].ext.prebid.targeting` | | timeout | no | integer | none | A soft timeout (in ms) sent as a hint to the Targeting API endpoint to limit the request times to Optable's external tokenizer services | +| api-timeout | no | integer | none | A hard timeout (in ms) for the Targeting API call that is awaited by the `bidder-request` hook. When not set, the call is limited by the time remaining for the auction. See Timeout considerations above. | | id-prefix-order | no | string | none | An optional string of comma separated id prefixes that prioritizes and specifies the order in which ids are provided to Targeting API in a query string. F.e. "c,c1,id5" will guarantee that Targeting API will see id=c:...,c1:...,id5:... if these ids are provided. id-prefixes not mentioned in this list will be added in arbitrary order after the priority prefix ids. This affects Targeting API processing logic | | hid-prefixes | no | string | none | An optional string of comma separated id prefixes that should additionally be sent to the Targeting API as resolver hints in `hid=prefix:value` query parameters. See the section on Resolver Hints (hid) below for more detail. | | enrichment-percentage | no | integer | 100 | Default percentage (0-100) of bid requests per bidder that will receive enrichment data. Set to 100 to enrich all requests, 0 to disable enrichment by default. | diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java index 08ecdc310ff..6b11c219247 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java @@ -137,17 +137,22 @@ BidderEnrichmentSampler bidderEnrichmentSampler(BidderCatalog bidderCatalog) { OptableTargetingFlowResolver earlyOptableCallResolver( BidderEnrichmentSampler bidderEnrichmentSampler, TargetingRequestExecutor targetingRequestExecutor, - @Value("${hooks.host-execution-plan:}") - String executionPlan, + @Value("${hooks.host-execution-plan:}") String hostExecutionPlan, + @Value("${hooks.default-account-execution-plan:}") String defaultAccountExecutionPlan, JacksonMapper mapper, @Value("${logging.sampling-rate:0.01}") double logSamplingRate) { final CompositeHookExecutionPlan hooksExecutionPlan = CompositeHookExecutionPlan.of( - StringUtils.isNoneEmpty(executionPlan) - ? mapper.decodeValue(executionPlan, ExecutionPlan.class) - : null); + parseExecutionPlan(hostExecutionPlan, mapper), + parseExecutionPlan(defaultAccountExecutionPlan, mapper)); return new OptableTargetingFlowResolver( bidderEnrichmentSampler, targetingRequestExecutor, hooksExecutionPlan, logSamplingRate); } + + private static ExecutionPlan parseExecutionPlan(String executionPlan, JacksonMapper mapper) { + return StringUtils.isNotBlank(executionPlan) + ? mapper.decodeValue(executionPlan, ExecutionPlan.class) + : ExecutionPlan.empty(); + } } diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/config/OptableTargetingProperties.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/config/OptableTargetingProperties.java index 314e2639cef..527eb14f53d 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/config/OptableTargetingProperties.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/model/config/OptableTargetingProperties.java @@ -29,6 +29,9 @@ public final class OptableTargetingProperties { Long timeout; + @JsonProperty("api-timeout") + Long apiTimeout; + @JsonProperty("id-prefix-order") String idPrefixOrder; diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java index 1f1590c90ff..a502e3c6608 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java @@ -1,14 +1,11 @@ package org.prebid.server.hooks.modules.optable.targeting.v1; import io.vertx.core.Future; -import org.prebid.server.hooks.execution.v1.InvocationResultImpl; import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; import org.prebid.server.hooks.modules.optable.targeting.v1.core.ConfigResolver; import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargetingFlowResolver; -import org.prebid.server.hooks.v1.InvocationAction; import org.prebid.server.hooks.v1.InvocationResult; -import org.prebid.server.hooks.v1.InvocationStatus; import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook; @@ -47,7 +44,9 @@ public Future> call(AuctionRequestPayloa // whatever goes wrong here, the cleaner has to be applied, or user.ext.optable ids reach the bidders try { - return resolveTargetingFlow(auctionRequestPayload, invocationContext, moduleContext); + final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig()); + return optableTargetingFlowResolver.resolveOptableTargetingFlow( + auctionRequestPayload, invocationContext, moduleContext, properties); } catch (RuntimeException e) { conditionalLogger.error("Failed to initiate Optable targeting call: " + e.getMessage(), logSamplingRate); @@ -57,35 +56,6 @@ public Future> call(AuctionRequestPayloa } } - private Future> resolveTargetingFlow( - AuctionRequestPayload auctionRequestPayload, - AuctionInvocationContext invocationContext, - ModuleContext moduleContext) { - - final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig()); - - if (moduleContext.isEarlyNetworkCallEnabled()) { - if (moduleContext.isEarlyCallInitializationCompleted()) { - return success(moduleContext); - } else { - return optableTargetingFlowResolver.resolveDeferredOptableTargetingFlow( - moduleContext, auctionRequestPayload, invocationContext, properties); - } - } - - return optableTargetingFlowResolver.resolveOptableTargetingFlow( - auctionRequestPayload, invocationContext, moduleContext, properties); - } - - public static Future> success(ModuleContext moduleContext) { - return Future.succeededFuture( - InvocationResultImpl.builder() - .status(InvocationStatus.success) - .action(InvocationAction.no_action) - .moduleContext(moduleContext) - .build()); - } - @Override public String code() { return CODE; diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlan.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlan.java index 0d5bda38be9..1cb703a415f 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlan.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlan.java @@ -1,120 +1,72 @@ package org.prebid.server.hooks.modules.optable.targeting.v1.core; -import org.apache.commons.lang3.StringUtils; +import org.prebid.server.auction.model.AuctionContext; import org.prebid.server.hooks.execution.model.EndpointExecutionPlan; import org.prebid.server.hooks.execution.model.ExecutionGroup; import org.prebid.server.hooks.execution.model.ExecutionPlan; +import org.prebid.server.hooks.execution.model.HookExecutionContext; import org.prebid.server.hooks.execution.model.HookHttpEndpoint; import org.prebid.server.hooks.execution.model.Stage; import org.prebid.server.hooks.execution.model.StageExecutionPlan; import org.prebid.server.hooks.modules.optable.targeting.v1.OptableBidderRequestHook; -import org.prebid.server.hooks.modules.optable.targeting.v1.OptableRawAuctionRequestHook; import org.prebid.server.settings.model.Account; +import org.prebid.server.settings.model.AccountHooksConfiguration; -import java.util.List; +import java.util.Collection; +import java.util.Objects; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; +/** + * Resolves the hooks configured for a request the way the core does: the host execution plan combined with the + * account execution plan, or with the default account execution plan when the account has none, for the endpoint + * of the request. + */ public class CompositeHookExecutionPlan { - private static final HookHttpEndpoint ENDPOINT_AUCTION = HookHttpEndpoint.POST_AUCTION; - private static final String STAGE_RAW_AUCTION_REQUEST = "raw_auction_request"; - private static final String STAGE_BIDDER_REQUEST = "bidder_request"; - private static final String HOOK_CODE_OPTABLE_RAW_AUCTION = OptableRawAuctionRequestHook.CODE; - private static final String HOOK_CODE_OPTABLE_BIDDER_REQUEST = OptableBidderRequestHook.CODE; + private final ExecutionPlan hostExecutionPlan; + private final ExecutionPlan defaultAccountExecutionPlan; - private final boolean hasGlobalRawAuctionRequestHook; - - private final boolean hasGlobalBidderRequestHook; - - private final long globalBidderRequestHookTimeout; - - private final ConcurrentHashMap rawAuctionRequestHookCache = new ConcurrentHashMap<>(); - private final ConcurrentHashMap bidderRequestHookCache = new ConcurrentHashMap<>(); - - private final ConcurrentHashMap bidderRequestHookTimeoutCache = new ConcurrentHashMap<>(); - - private CompositeHookExecutionPlan(boolean hasGlobalRawAuctionRequestHook, - boolean hasGlobalBidderRequestHook, - long globalBidderRequestHookTimeout) { - - this.hasGlobalRawAuctionRequestHook = hasGlobalRawAuctionRequestHook; - this.hasGlobalBidderRequestHook = hasGlobalBidderRequestHook; - this.globalBidderRequestHookTimeout = globalBidderRequestHookTimeout; - } - - public static CompositeHookExecutionPlan of(ExecutionPlan globalExecutionPlan) { - return globalExecutionPlan == null - ? new CompositeHookExecutionPlan(false, false, 0) - : new CompositeHookExecutionPlan( - hasHook(globalExecutionPlan, STAGE_RAW_AUCTION_REQUEST, HOOK_CODE_OPTABLE_RAW_AUCTION), - hasHook(globalExecutionPlan, STAGE_BIDDER_REQUEST, HOOK_CODE_OPTABLE_BIDDER_REQUEST), - getHookTimeout(globalExecutionPlan, - STAGE_BIDDER_REQUEST, HOOK_CODE_OPTABLE_BIDDER_REQUEST)); + private CompositeHookExecutionPlan(ExecutionPlan hostExecutionPlan, ExecutionPlan defaultAccountExecutionPlan) { + this.hostExecutionPlan = Objects.requireNonNull(hostExecutionPlan); + this.defaultAccountExecutionPlan = Objects.requireNonNull(defaultAccountExecutionPlan); } - private T computeFromAccount(Account account, - ConcurrentHashMap cache, - T defaultValue, - Function compute) { - final String accountId = account != null ? account.getId() : null; - return StringUtils.isNotEmpty(accountId) - ? cache.computeIfAbsent(accountId, id -> compute.apply(resolveExecutionPlan(account))) - : defaultValue; - } + public static CompositeHookExecutionPlan of(ExecutionPlan hostExecutionPlan, + ExecutionPlan defaultAccountExecutionPlan) { - public boolean hasRawAuctionRequestHook(Account account) { - return computeFromAccount(account, rawAuctionRequestHookCache, false, - plan -> hasHook(plan, STAGE_RAW_AUCTION_REQUEST, HOOK_CODE_OPTABLE_RAW_AUCTION) - || hasGlobalRawAuctionRequestHook); + return new CompositeHookExecutionPlan( + Objects.requireNonNullElse(hostExecutionPlan, ExecutionPlan.empty()), + Objects.requireNonNullElse(defaultAccountExecutionPlan, ExecutionPlan.empty())); } - public boolean hasBidderRequestHook(Account account) { - return computeFromAccount(account, bidderRequestHookCache, false, - plan -> hasHook(plan, STAGE_BIDDER_REQUEST, HOOK_CODE_OPTABLE_BIDDER_REQUEST) - || hasGlobalBidderRequestHook); - } + public boolean hasBidderRequestHook(AuctionContext auctionContext) { + final HookHttpEndpoint endpoint = Optional.ofNullable(auctionContext) + .map(AuctionContext::getHookExecutionContext) + .map(HookExecutionContext::getEndpoint) + .orElse(HookHttpEndpoint.POST_AUCTION); - public long getOptableTargetingBidderRequestTimeout(Account account) { - return computeFromAccount(account, bidderRequestHookTimeoutCache, globalBidderRequestHookTimeout, - plan -> { - final long timeout = getHookTimeout(plan, STAGE_BIDDER_REQUEST, HOOK_CODE_OPTABLE_BIDDER_REQUEST); - return timeout != 0 ? timeout : globalBidderRequestHookTimeout; - }); + return hasHook(hostExecutionPlan, endpoint) + || hasHook(accountExecutionPlan(auctionContext != null ? auctionContext.getAccount() : null), endpoint); } - private ExecutionPlan resolveExecutionPlan(Account account) { + private ExecutionPlan accountExecutionPlan(Account account) { return Optional.ofNullable(account) - .map(org.prebid.server.settings.model.Account::getHooks) - .map(org.prebid.server.settings.model.AccountHooksConfiguration::getExecutionPlan) - .orElse(null); + .map(Account::getHooks) + .map(AccountHooksConfiguration::getExecutionPlan) + .orElse(defaultAccountExecutionPlan); } - private static boolean hasHook(ExecutionPlan executionPlan, String stage, String hookCode) { - return Optional.ofNullable(executionPlan) - .map(ExecutionPlan::getEndpoints) - .map(endpoints -> endpoints.get(ENDPOINT_AUCTION)) + private static boolean hasHook(ExecutionPlan executionPlan, HookHttpEndpoint endpoint) { + return Optional.ofNullable(executionPlan.getEndpoints()) + .map(endpoints -> endpoints.get(endpoint)) .map(EndpointExecutionPlan::getStages) - .map(stages -> stages.get(Stage.valueOf(stage))) + .map(stages -> stages.get(Stage.bidder_request)) .map(StageExecutionPlan::getGroups) - .orElseGet(List::of) .stream() + .flatMap(Collection::stream) .map(ExecutionGroup::getHookSequence) - .flatMap(java.util.Collection::stream) - .anyMatch(hook -> hookCode.equals(hook.getHookImplCode())); - } - - private static long getHookTimeout(ExecutionPlan executionPlan, String stage, String hookCode) { - return Optional.ofNullable(executionPlan) - .map(ExecutionPlan::getEndpoints) - .map(endpoints -> endpoints.get(ENDPOINT_AUCTION)) - .map(EndpointExecutionPlan::getStages) - .map(stages -> stages.get(Stage.valueOf(stage))) - .map(StageExecutionPlan::getGroups) - .orElseGet(List::of) - .stream().findFirst() - .map(ExecutionGroup::getTimeout) - .orElse(0L); + .filter(Objects::nonNull) + .flatMap(Collection::stream) + .anyMatch(hookId -> OptableBidderRequestHook.CODE.equals(hookId.getHookImplCode())); } } diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java index ba7abf75c94..153e46c601e 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java @@ -23,7 +23,6 @@ import org.prebid.server.proto.openrtb.ext.request.ExtRequest; import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; import org.prebid.server.proto.openrtb.ext.request.ExtUser; -import org.prebid.server.settings.model.Account; import java.util.Collection; import java.util.Objects; @@ -72,29 +71,24 @@ public Future> resolveAsyncOptableTarget // the cleaner strips user.ext.optable at this stage, while the deferred call still needs its ids moduleContext.setExtUserOptable(extUserOptable(bidRequest)); } else { - startTargetingCall(moduleContext, bidRequest, invocationContext, properties); + startTargetingCall(moduleContext, bidRequest, invocationContext, properties, true); } return update(BidRequestCleaner.instance(), moduleContext); } - /** - * Processed auction request stage: starts the call deferred by the raw auction request hook. - */ - public Future> resolveDeferredOptableTargetingFlow( - ModuleContext moduleContext, - AuctionRequestPayload payload, - AuctionInvocationContext invocationContext, - OptableTargetingProperties properties) { + private void startDeferredTargetingCall(ModuleContext moduleContext, + AuctionRequestPayload payload, + AuctionInvocationContext invocationContext, + OptableTargetingProperties properties, + boolean awaitedByBidderRequestHook) { final BidRequest bidRequest = withExtUserOptable(payload.bidRequest(), moduleContext.getExtUserOptable()); moduleContext.setEarlyCallInitializationCompleted(true); moduleContext.setExtUserOptable(null); moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); - startTargetingCall(moduleContext, bidRequest, invocationContext, properties); - - return update(BidRequestCleaner.instance(), moduleContext); + startTargetingCall(moduleContext, bidRequest, invocationContext, properties, awaitedByBidderRequestHook); } private boolean shouldDeferTargetingCall(BidRequest bidRequest) { @@ -121,7 +115,8 @@ private static boolean hasStoredRequest(BidRequest bidRequest) { private void startTargetingCall(ModuleContext moduleContext, BidRequest bidRequest, AuctionInvocationContext invocationContext, - OptableTargetingProperties properties) { + OptableTargetingProperties properties, + boolean awaitedByLaterHook) { if (!PropertiesValidator.isTrafficSourceValid(bidRequest, properties)) { moduleContext.setShouldSkipEnrichment(true); @@ -133,15 +128,11 @@ private void startTargetingCall(ModuleContext moduleContext, return; } - final Account account = invocationContext.auctionContext().getAccount(); - final long crossHookFutureTimeout = - hooksExecutionPlan.getOptableTargetingBidderRequestTimeout(account); - final Future optableTargetingCall = targetingRequestExecutor.makeRequest( bidRequest, invocationContext, properties, - crossHookFutureTimeout); + awaitedByLaterHook); // set together, so that the bidder request hook never sees bidders without a call to await moduleContext.setBiddersToEnrich(biddersToEnrich); @@ -172,34 +163,60 @@ private static BidRequest withExtUserOptable(BidRequest bidRequest, JsonNode opt } /** - * @deprecated This call is deprecated and will be removed in a future release. + * Processed auction request stage. */ - @Deprecated public Future> resolveOptableTargetingFlow( AuctionRequestPayload auctionRequestPayload, AuctionInvocationContext invocationContext, ModuleContext moduleContext, OptableTargetingProperties properties) { - if (moduleContext.isShouldSkipEnrichment()) { - moduleContext.setOptableTargetingExecutionTime(calcAPICallExecutionTime(moduleContext)); - return updateWithAnalytics(BidRequestCleaner.instance(), moduleContext); + final boolean hasBidderRequestHook = + hooksExecutionPlan.hasBidderRequestHook(invocationContext.auctionContext()); + + if (moduleContext.isEarlyNetworkCallEnabled()) { + final boolean deferred = !moduleContext.isEarlyCallInitializationCompleted(); + if (deferred) { + startDeferredTargetingCall( + moduleContext, auctionRequestPayload, invocationContext, properties, hasBidderRequestHook); + } + + if (hasBidderRequestHook) { + return deferred ? update(BidRequestCleaner.instance(), moduleContext) : noAction(moduleContext); + } + + return enrichWhenCompleted(moduleContext.getOptableTargetingCall(), moduleContext, properties); } - final Account account = invocationContext.auctionContext().getAccount(); - final boolean hasRawAuctionRequestHook = hooksExecutionPlan.hasRawAuctionRequestHook(account); - final boolean hasBidderRequestHook = hooksExecutionPlan.hasBidderRequestHook(account); + moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); + moduleContext.setOptableTargetingProperties(properties); + if (!PropertiesValidator.isValid(properties)) { + conditionalLogger.error(AUCTION_NOT_PROPERLY_CONFIGURED, logSamplingRate); + return failed(moduleContext); + } - if (hasRawAuctionRequestHook && hasBidderRequestHook) { - return updateWithAnalytics(BidRequestCleaner.instance(), moduleContext); + // the raw auction request stage does not run for f.e. amp and video requests + if (hasBidderRequestHook) { + startTargetingCall(moduleContext, auctionRequestPayload.bidRequest(), invocationContext, properties, true); + return update(BidRequestCleaner.instance(), moduleContext); } - final Future optableTargetingCall = hasRawAuctionRequestHook - ? resolveEarlyNetworkCall(moduleContext) - : resolvePreEarlyNetworkCall(auctionRequestPayload, invocationContext, moduleContext, properties); + final Future optableTargetingCall = targetingRequestExecutor.makeRequest( + auctionRequestPayload.bidRequest(), + invocationContext, + properties, + false); + + return enrichWhenCompleted(optableTargetingCall, moduleContext, properties); + } + + private Future> enrichWhenCompleted( + Future optableTargetingCall, + ModuleContext moduleContext, + OptableTargetingProperties properties) { - if (optableTargetingCall == null) { - moduleContext.failWithExecutionTime(calcAPICallExecutionTime(moduleContext)); + if (moduleContext.isShouldSkipEnrichment() || optableTargetingCall == null) { + moduleContext.setOptableTargetingExecutionTime(calcAPICallExecutionTime(moduleContext)); return updateWithAnalytics(BidRequestCleaner.instance(), moduleContext); } @@ -229,36 +246,10 @@ private Future> enrichPayload( return updateWithAnalytics(payloadUpdate, moduleContext); } - private Future resolveEarlyNetworkCall(ModuleContext moduleContext) { - return moduleContext.getOptableTargetingCall(); - } - private static long calcAPICallExecutionTime(ModuleContext moduleContext) { return System.currentTimeMillis() - moduleContext.getCallTargetingAPITimestamp(); } - private Future resolvePreEarlyNetworkCall( - AuctionRequestPayload payload, - AuctionInvocationContext invocationContext, - ModuleContext moduleContext, - OptableTargetingProperties properties) { - - moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); - if (!PropertiesValidator.isValid(properties)) { - conditionalLogger.error(AUCTION_NOT_PROPERLY_CONFIGURED, logSamplingRate); - - moduleContext.failWithExecutionTime( - System.currentTimeMillis() - moduleContext.getCallTargetingAPITimestamp()); - return Future.failedFuture(AUCTION_NOT_PROPERLY_CONFIGURED); - } - - return targetingRequestExecutor.makeRequest( - payload.bidRequest(), - invocationContext, - properties, - null); - } - private static Future> update( PayloadUpdate payloadUpdate, ModuleContext moduleContext) { @@ -272,6 +263,15 @@ private static Future> update( .build()); } + private static Future> noAction(ModuleContext moduleContext) { + return Future.succeededFuture( + InvocationResultImpl.builder() + .status(InvocationStatus.success) + .action(InvocationAction.no_action) + .moduleContext(moduleContext) + .build()); + } + public Future> failed(ModuleContext moduleContext) { moduleContext.failWithExecutionTime( moduleContext.getCallTargetingAPITimestamp() > 0 ? calcAPICallExecutionTime(moduleContext) : 0); diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java index 1b2dbfdcd2e..bc73cbdaee2 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/TargetingRequestExecutor.java @@ -12,6 +12,7 @@ import org.prebid.server.activity.infrastructure.payload.impl.ActivityInvocationPayloadImpl; import org.prebid.server.activity.infrastructure.payload.impl.BidRequestActivityInvocationPayload; import org.prebid.server.auction.model.AuctionContext; +import org.prebid.server.auction.model.TimeoutContext; import org.prebid.server.auction.privacy.enforcement.mask.UserFpdActivityMask; import org.prebid.server.execution.timeout.Timeout; import org.prebid.server.execution.timeout.TimeoutFactory; @@ -44,13 +45,13 @@ public TargetingRequestExecutor(OptableTargeting optableTargeting, public Future makeRequest(BidRequest bidRequest, AuctionInvocationContext invocationContext, OptableTargetingProperties properties, - Long apiTimeout) { + boolean awaitedByLaterHook) { final BidRequest restrictedBidRequest = applyActivityRestrictions(bidRequest, invocationContext); - final Timeout timeout = apiTimeout == null - ? getHookTimeout(invocationContext) - : timeoutFactory.create(getHookTimeout(invocationContext).remaining() + apiTimeout); + final Timeout timeout = awaitedByLaterHook + ? resolveCrossHookTimeout(invocationContext, properties) + : invocationContext.timeout(); final OptableAttributes attributes = OptableAttributesResolver.resolveAttributes( invocationContext.auctionContext(), properties.getTimeout(), @@ -59,8 +60,20 @@ public Future makeRequest(BidRequest bidRequest, return optableTargeting.getTargeting(properties, restrictedBidRequest, attributes, timeout); } - private static Timeout getHookTimeout(AuctionInvocationContext invocationContext) { - return invocationContext.timeout(); + /** + * A call that is awaited by a later hook can't be bound by the timeout of the hook that starts it. + */ + private Timeout resolveCrossHookTimeout(AuctionInvocationContext invocationContext, + OptableTargetingProperties properties) { + + final Long apiTimeout = properties.getApiTimeout(); + if (apiTimeout != null && apiTimeout > 0) { + return timeoutFactory.create(apiTimeout); + } + + final TimeoutContext timeoutContext = invocationContext.auctionContext().getTimeoutContext(); + final Timeout auctionTimeout = timeoutContext != null ? timeoutContext.getTimeout() : null; + return auctionTimeout != null ? auctionTimeout : invocationContext.timeout(); } private BidRequest applyActivityRestrictions(BidRequest bidRequest, diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/BaseOptableTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/BaseOptableTest.java index 386055b6c84..a373e6283a5 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/BaseOptableTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/BaseOptableTest.java @@ -27,6 +27,13 @@ import org.prebid.server.auction.model.AuctionContext; import org.prebid.server.auction.model.TimeoutContext; import org.prebid.server.execution.timeout.Timeout; +import org.prebid.server.hooks.execution.model.EndpointExecutionPlan; +import org.prebid.server.hooks.execution.model.ExecutionGroup; +import org.prebid.server.hooks.execution.model.ExecutionPlan; +import org.prebid.server.hooks.execution.model.HookHttpEndpoint; +import org.prebid.server.hooks.execution.model.HookId; +import org.prebid.server.hooks.execution.model.Stage; +import org.prebid.server.hooks.execution.model.StageExecutionPlan; import org.prebid.server.hooks.modules.optable.targeting.model.EnrichmentStatus; import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; import org.prebid.server.hooks.modules.optable.targeting.model.Query; @@ -123,6 +130,15 @@ protected AuctionContext givenAuctionContext(ActivityInfrastructure activityInfr return givenAuctionContext(activityInfrastructure, timeout, null); } + protected static ExecutionPlan givenBidderRequestHookPlan() { + final StageExecutionPlan bidderRequestStage = StageExecutionPlan.of(List.of(ExecutionGroup.of( + null, List.of(HookId.of("optable-targeting", "optable-targeting-bidder-request-hook"))))); + + return ExecutionPlan.of(null, Map.of( + HookHttpEndpoint.POST_AUCTION, + EndpointExecutionPlan.of(Map.of(Stage.bidder_request, bidderRequestStage)))); + } + protected HttpRequestContext givenHttpRequestContext() { return givenHttpRequestContext(null); } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java index 64caa7fdbb0..ed6e20a4a4e 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableRawAuctionRequestHookTest.java @@ -84,7 +84,7 @@ private OptableTargetingFlowResolver givenEarlyOptableCallResolver() { return new OptableTargetingFlowResolver( bidderEnrichmentSampler, targetingRequestExecutor, - CompositeHookExecutionPlan.of(ExecutionPlan.empty()), + CompositeHookExecutionPlan.of(ExecutionPlan.empty(), ExecutionPlan.empty()), 0.01); } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java index baf9e2957a1..d39ea6fe1ac 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableStoredRequestFlowTest.java @@ -111,7 +111,7 @@ public void setUp() { final OptableTargetingFlowResolver flowResolver = new OptableTargetingFlowResolver( BidderEnrichmentSampler.of(AliasesResolver.of(bidderCatalog), randomSupplier), new TargetingRequestExecutor(optableTargeting, userFpdActivityMask, timeoutFactory, 0.01), - CompositeHookExecutionPlan.of(ExecutionPlan.empty()), + CompositeHookExecutionPlan.of(givenBidderRequestHookPlan(), ExecutionPlan.empty()), 0.01); final ConfigResolver configResolver = new ConfigResolver(mapper, jsonMerger, properties); rawHook = new OptableRawAuctionRequestHook(configResolver, flowResolver, 0.01); diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java index 1e5d45b21bb..eb028422b2c 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java @@ -106,7 +106,7 @@ private OptableTargetingFlowResolver givenFlowResolver(ExecutionPlan executionPl return new OptableTargetingFlowResolver( bidderEnrichmentSampler, targetingRequestExecutor, - CompositeHookExecutionPlan.of(executionPlan), + CompositeHookExecutionPlan.of(executionPlan, ExecutionPlan.empty()), 0.01); } @@ -219,11 +219,12 @@ void callShouldLeaveId5SignatureNullWhenTargetingResultHasNoId5Signature() { } @Test - void callShouldReturnResultWithNoActionWhenEarlyOptableCallIsEnabledAndInitialized() { + void callShouldEnrichRequestWithEarlyCallResultWhenBidderRequestHookIsAbsent() { // given final ModuleContext moduleContext = new ModuleContext(); moduleContext.setEarlyNetworkCallEnabled(true); moduleContext.setEarlyCallInitializationCompleted(true); + moduleContext.setCallTargetingAPITimestamp(System.currentTimeMillis()); target = new OptableTargetingProcessedAuctionRequestHook( configResolver, givenFlowResolver(givenExecutionPlan(true, false)), 0.01); @@ -235,24 +236,22 @@ void callShouldReturnResultWithNoActionWhenEarlyOptableCallIsEnabledAndInitializ givenBidRequest(), invocationContext, givenOptableTargetingProperties("key", "tenant", "origin", false), - null)); + false)); // when - final Future> future = target.call(auctionRequestPayload, - invocationContext); + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); // then - assertThat(future).isNotNull(); - assertThat(future.succeeded()).isTrue(); - - final InvocationResult result = future.result(); - assertThat(result).isNotNull() - .returns(InvocationStatus.success, InvocationResult::status) - .returns(InvocationAction.no_action, InvocationResult::action) - .extracting(InvocationResult::errors).isNull(); - assertThat(result.payloadUpdate()).isNull(); - assertThat(moduleContext.getOptableTargetingCall()).isNotNull(); - assertThat(moduleContext.getOptableTargetingCall().succeeded()).isTrue(); + assertThat(result.action()).isEqualTo(InvocationAction.update); + final BidRequest bidRequest = result + .payloadUpdate() + .apply(AuctionRequestPayloadImpl.of(givenBidRequest())) + .bidRequest(); + assertThat(bidRequest.getUser().getEids()) + .flatExtracting(Eid::getUids) + .extracting(Uid::getId) + .containsExactly("id"); } @Test @@ -261,6 +260,9 @@ void callShouldRetryEarlyNetworkCallInitializationWhenItWasNotCompletedOnRawAuct final ModuleContext moduleContext = new ModuleContext(); moduleContext.setEarlyNetworkCallEnabled(true); moduleContext.setEarlyCallInitializationCompleted(false); + target = new OptableTargetingProcessedAuctionRequestHook( + configResolver, + givenFlowResolver(givenExecutionPlan(true, true)), 0.01); final TargetingResult targetingResult = givenTargetingResult(); when(invocationContext.moduleContext()).thenReturn(moduleContext); @@ -302,6 +304,9 @@ void callShouldNotRetryEarlyNetworkCallInitializationWhenNoBiddersToEnrich() { final ModuleContext moduleContext = new ModuleContext(); moduleContext.setEarlyNetworkCallEnabled(true); moduleContext.setEarlyCallInitializationCompleted(false); + target = new OptableTargetingProcessedAuctionRequestHook( + configResolver, + givenFlowResolver(givenExecutionPlan(true, true)), 0.01); when(invocationContext.moduleContext()).thenReturn(moduleContext); when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); @@ -362,7 +367,7 @@ void callShouldReturnResultWithEnrichedBidRequestWhenBothHooksAreAbsent() { } @Test - void callShouldReturnResultWithEnrichedBidRequestWhenOnlyBidderRequestHookIsPresent() { + void callShouldStartCallForBidderRequestHookWhenRawAuctionRequestHookDidNotRun() { // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, @@ -370,61 +375,45 @@ void callShouldReturnResultWithEnrichedBidRequestWhenOnlyBidderRequestHookIsPres when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); when(optableTargeting.getTargeting(any(), any(), any(), any())) .thenReturn(Future.succeededFuture(givenTargetingResult())); + when(bidderEnrichmentSampler.sample(any(), any())).thenReturn(Set.of("bidder")); // when - final Future> future = target.call(auctionRequestPayload, - invocationContext); + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); // then - assertThat(future).isNotNull(); - assertThat(future.succeeded()).isTrue(); + assertThat(result.action()).isEqualTo(InvocationAction.update); + final ModuleContext moduleContext = (ModuleContext) result.moduleContext(); + assertThat(moduleContext.getBiddersToEnrich()).containsExactly("bidder"); + assertThat(moduleContext.getOptableTargetingCall().succeeded()).isTrue(); + assertThat(moduleContext.getOptableTargetingProperties()).isNotNull(); - final InvocationResult result = future.result(); - assertThat(result).isNotNull() - .returns(InvocationStatus.success, InvocationResult::status) - .returns(InvocationAction.update, InvocationResult::action) - .extracting(InvocationResult::errors).isNull(); final BidRequest bidRequest = result .payloadUpdate() .apply(AuctionRequestPayloadImpl.of(givenBidRequest())) .bidRequest(); - assertThat(bidRequest.getUser().getEids()) - .flatExtracting(Eid::getUids) - .extracting(Uid::getId) - .containsExactly("id"); - assertThat(bidRequest.getUser().getData()) - .flatExtracting(Data::getSegment) - .extracting(Segment::getId) - .containsExactly("id"); + assertThat(bidRequest.getUser().getEids()).isNull(); + assertThat(bidRequest.getUser().getExt().getProperty("optable")).isNull(); } @Test - void callShouldReturnResultWithoutEnrichedBidRequestWhenBothHooksArePresent() { + void callShouldReturnNoActionWhenRawAuctionRequestHookStartedCallForBidderRequestHook() { // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, givenFlowResolver(givenExecutionPlan(true, true)), 0.01); - when(invocationContext.moduleContext()).thenReturn(new ModuleContext()); + final ModuleContext moduleContext = new ModuleContext(); + moduleContext.setEarlyNetworkCallEnabled(true); + when(invocationContext.moduleContext()).thenReturn(moduleContext); // when - final Future> future = target.call(auctionRequestPayload, - invocationContext); + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); // then - assertThat(future).isNotNull(); - assertThat(future.succeeded()).isTrue(); - - final InvocationResult result = future.result(); - assertThat(result).isNotNull() - .returns(InvocationStatus.success, InvocationResult::status) - .returns(InvocationAction.update, InvocationResult::action) - .extracting(InvocationResult::errors).isNull(); - final BidRequest bidRequest = result - .payloadUpdate() - .apply(AuctionRequestPayloadImpl.of(givenBidRequest())) - .bidRequest(); - assertThat(bidRequest.getUser().getEids()).isNull(); - assertThat(bidRequest.getUser().getData()).isNull(); + assertThat(result.status()).isEqualTo(InvocationStatus.success); + assertThat(result.action()).isEqualTo(InvocationAction.no_action); + assertThat(result.payloadUpdate()).isNull(); } @Test @@ -539,6 +528,7 @@ void callShouldReturnResultWithUpdateWhenOptableTargetingDoesNotReturnResult() { void callShouldReturnUpdateWhenTrafficSourceIsInvalid() { // given final ModuleContext moduleContext = new ModuleContext(); + moduleContext.setEarlyNetworkCallEnabled(true); moduleContext.setShouldSkipEnrichment(true); when(invocationContext.moduleContext()).thenReturn(moduleContext); diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlanTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlanTest.java index c836de388a9..09b2b728b51 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlanTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/CompositeHookExecutionPlanTest.java @@ -1,9 +1,11 @@ package org.prebid.server.hooks.modules.optable.targeting.v1.core; import org.junit.jupiter.api.Test; +import org.prebid.server.auction.model.AuctionContext; import org.prebid.server.hooks.execution.model.EndpointExecutionPlan; import org.prebid.server.hooks.execution.model.ExecutionGroup; import org.prebid.server.hooks.execution.model.ExecutionPlan; +import org.prebid.server.hooks.execution.model.HookExecutionContext; import org.prebid.server.hooks.execution.model.HookHttpEndpoint; import org.prebid.server.hooks.execution.model.HookId; import org.prebid.server.hooks.execution.model.Stage; @@ -16,345 +18,89 @@ import static org.assertj.core.api.Assertions.assertThat; -public class CompositeHookExecutionPlanTest { +class CompositeHookExecutionPlanTest { - @Test - public void hasRawAuctionRequestHookShouldReturnTrueWhenGlobalPlanHasHook() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnTrueWhenAccountPlanHasHook() { - // given - final ExecutionPlan accountPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnTrueWhenBothPlansHaveHook() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final ExecutionPlan accountPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnFalseWhenNeitherPlanHasHook() { - // given - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isFalse(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnFalseWhenAccountIsNull() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - - // when and then - assertThat(target.hasRawAuctionRequestHook(null)).isFalse(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnFalseWhenAccountIdIsEmpty() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("").build(); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isFalse(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnGlobalFlagWhenAccountHasNoHooksConfig() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - } - - @Test - public void hasRawAuctionRequestHookShouldReturnSameResultOnRepeatedCallsForSameAccount() { - // given - final ExecutionPlan accountPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - assertThat(target.hasRawAuctionRequestHook(account)).isTrue(); - } - - @Test - public void hasBidderRequestHookShouldReturnTrueWhenGlobalPlanHasHook() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isTrue(); - } - - @Test - public void hasBidderRequestHookShouldReturnTrueWhenAccountPlanHasHook() { - // given - final ExecutionPlan accountPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isTrue(); - } - - @Test - public void hasBidderRequestHookShouldReturnTrueWhenBothPlansHaveHook() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final ExecutionPlan accountPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isTrue(); - } - - @Test - public void hasBidderRequestHookShouldReturnFalseWhenNeitherPlanHasHook() { - // given - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isFalse(); - } - - @Test - public void hasBidderRequestHookShouldReturnFalseWhenAccountIsNull() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - - // when and then - assertThat(target.hasBidderRequestHook(null)).isFalse(); - } - - @Test - public void hasBidderRequestHookShouldReturnFalseWhenAccountIdIsEmpty() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("").build(); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isFalse(); - } - - @Test - public void hasBidderRequestHookShouldReturnGlobalFlagWhenAccountHasNoHooksConfig() { - // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.hasBidderRequestHook(account)).isTrue(); - } + private static final HookId BIDDER_REQUEST_HOOK = + HookId.of("optable-targeting", "optable-targeting-bidder-request-hook"); @Test - public void hasBidderRequestHookShouldReturnSameResultOnRepeatedCallsForSameAccount() { + void hasBidderRequestHookShouldFindHookInHostPlan() { // given - final ExecutionPlan accountPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = givenAccount("accountId", accountPlan); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of( + givenPlan(HookHttpEndpoint.POST_AUCTION, BIDDER_REQUEST_HOOK), null); // when and then - assertThat(target.hasBidderRequestHook(account)).isTrue(); - assertThat(target.hasBidderRequestHook(account)).isTrue(); + assertThat(target.hasBidderRequestHook(givenAuctionContext(HookHttpEndpoint.POST_AUCTION, null))).isTrue(); } @Test - public void hasRawAuctionRequestHookShouldReturnFalseWhenOnlyBidderRequestHookIsInGlobalPlan() { + void hasBidderRequestHookShouldFindHookInAccountPlan() { // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "bidder_request", "optable-targeting-bidder-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null, null); + final ExecutionPlan accountPlan = givenPlan(HookHttpEndpoint.POST_AUCTION, BIDDER_REQUEST_HOOK); // when and then - assertThat(target.hasRawAuctionRequestHook(account)).isFalse(); + assertThat(target.hasBidderRequestHook(givenAuctionContext(HookHttpEndpoint.POST_AUCTION, accountPlan))) + .isTrue(); } @Test - public void hasBidderRequestHookShouldReturnFalseWhenOnlyRawAuctionRequestHookIsInGlobalPlan() { + void hasBidderRequestHookShouldFallBackToDefaultAccountPlanWhenAccountHasNone() { // given - final ExecutionPlan globalPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of( + null, givenPlan(HookHttpEndpoint.POST_AUCTION, BIDDER_REQUEST_HOOK)); // when and then - assertThat(target.hasBidderRequestHook(account)).isFalse(); + assertThat(target.hasBidderRequestHook(givenAuctionContext(HookHttpEndpoint.POST_AUCTION, null))).isTrue(); } @Test - public void getBidderRequestTimeoutShouldReturnGlobalTimeoutWhenConfigured() { + void hasBidderRequestHookShouldIgnoreDefaultAccountPlanWhenAccountHasOne() { // given - final ExecutionPlan globalPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 500L); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("accountId").build(); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of( + null, givenPlan(HookHttpEndpoint.POST_AUCTION, BIDDER_REQUEST_HOOK)); // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(500L); + assertThat(target.hasBidderRequestHook( + givenAuctionContext(HookHttpEndpoint.POST_AUCTION, ExecutionPlan.empty()))).isFalse(); } @Test - public void getBidderRequestTimeoutShouldReturnAccountTimeoutWhenAccountPlanOverrides() { + void hasBidderRequestHookShouldLookAtEndpointOfRequest() { // given - final ExecutionPlan globalPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 500L); - final ExecutionPlan accountPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 200L); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = givenAccount("accountId", accountPlan); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of( + givenPlan(HookHttpEndpoint.POST_AUCTION, BIDDER_REQUEST_HOOK), null); // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(200L); + assertThat(target.hasBidderRequestHook(givenAuctionContext(HookHttpEndpoint.AMP, null))).isFalse(); } @Test - public void getBidderRequestTimeoutShouldFallbackToGlobalWhenAccountPlanHasNoTimeout() { + void hasBidderRequestHookShouldFindHookInAnyGroupOfStage() { // given - final ExecutionPlan globalPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 300L); - final ExecutionPlan accountPlan = givenExecutionPlan( - "raw_auction_request", "optable-targeting-raw-auction-request-hook"); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = givenAccount("accountId", accountPlan); + final StageExecutionPlan stage = StageExecutionPlan.of(List.of( + ExecutionGroup.of(10L, List.of(HookId.of("other", "other-hook"))), + ExecutionGroup.of(20L, List.of(BIDDER_REQUEST_HOOK)))); + final ExecutionPlan plan = ExecutionPlan.of(null, Map.of( + HookHttpEndpoint.AMP, EndpointExecutionPlan.of(Map.of(Stage.bidder_request, stage)))); + final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(plan, null); // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(300L); + assertThat(target.hasBidderRequestHook(givenAuctionContext(HookHttpEndpoint.AMP, null))).isTrue(); } - @Test - public void getBidderRequestTimeoutShouldReturnZeroWhenNoPlanIsConfigured() { - // given - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = Account.builder().id("accountId").build(); - - // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(0L); + private static ExecutionPlan givenPlan(HookHttpEndpoint endpoint, HookId hookId) { + final StageExecutionPlan stage = StageExecutionPlan.of(List.of(ExecutionGroup.of(10L, List.of(hookId)))); + return ExecutionPlan.of(null, Map.of(endpoint, EndpointExecutionPlan.of(Map.of(Stage.bidder_request, stage)))); } - @Test - public void getBidderRequestTimeoutShouldReturnGlobalTimeoutWhenAccountIsNull() { - // given - final ExecutionPlan globalPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 400L); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - - // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(null)).isEqualTo(400L); - } - - @Test - public void getBidderRequestTimeoutShouldReturnGlobalTimeoutWhenAccountIdIsEmpty() { - // given - final ExecutionPlan globalPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 150L); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(globalPlan); - final Account account = Account.builder().id("").build(); - - // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(150L); - } - - @Test - public void getBidderRequestTimeoutShouldReturnSameResultOnRepeatedCallsForSameAccount() { - // given - final ExecutionPlan accountPlan = givenExecutionPlanWithTimeout( - "bidder_request", - "optable-targeting-bidder-request-hook", - 250L); - final CompositeHookExecutionPlan target = CompositeHookExecutionPlan.of(null); - final Account account = givenAccount("accountId", accountPlan); - - // when and then - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(250L); - assertThat(target.getOptableTargetingBidderRequestTimeout(account)).isEqualTo(250L); - } - - private ExecutionPlan givenExecutionPlan(String stage, String hookCode) { - final HookId hookId = HookId.of("optable-targeting", hookCode); - final ExecutionGroup group = ExecutionGroup.of(null, List.of(hookId)); - final StageExecutionPlan stagePlan = StageExecutionPlan.of(List.of(group)); - final EndpointExecutionPlan endpointPlan = EndpointExecutionPlan.of(Map.of(Stage.valueOf(stage), stagePlan)); - return ExecutionPlan.of(null, Map.of(HookHttpEndpoint.POST_AUCTION, endpointPlan)); - } - - private ExecutionPlan givenExecutionPlanWithTimeout(String stage, String hookCode, long timeout) { - final HookId hookId = HookId.of("optable-targeting", hookCode); - final ExecutionGroup group = ExecutionGroup.of(timeout, List.of(hookId)); - final StageExecutionPlan stagePlan = StageExecutionPlan.of(List.of(group)); - final EndpointExecutionPlan endpointPlan = EndpointExecutionPlan.of(Map.of(Stage.valueOf(stage), stagePlan)); - return ExecutionPlan.of(null, Map.of(HookHttpEndpoint.POST_AUCTION, endpointPlan)); - } - - private Account givenAccount(String accountId, ExecutionPlan executionPlan) { - return Account.builder() - .id(accountId) - .hooks(AccountHooksConfiguration.of(executionPlan, null, null)) + private static AuctionContext givenAuctionContext(HookHttpEndpoint endpoint, ExecutionPlan accountPlan) { + return AuctionContext.builder() + .hookExecutionContext(HookExecutionContext.of(endpoint)) + .account(Account.builder() + .id("accountId") + .hooks(AccountHooksConfiguration.of(accountPlan, null, null)) + .build()) .build(); } } - From 8b3ab219859007b4c9a55eb7aa5d3c598d41405d Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Wed, 30 Sep 2026 20:09:16 +0200 Subject: [PATCH 7/8] optable-targeting: Apply enrich-web/enrich-app and sampling in the legacy setup Without the raw and bidder request hooks the processed hook called the API and enriched every request, whatever enrich-web, enrich-app and the enrichment percentages said. It now skips disabled traffic sources and only calls the API when sampling selects at least one bidder. --- extra/modules/optable-targeting/README.md | 3 +- .../v1/core/OptableTargetingFlowResolver.java | 16 ++++--- ...getingProcessedAuctionRequestHookTest.java | 43 +++++++++++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/extra/modules/optable-targeting/README.md b/extra/modules/optable-targeting/README.md index ecec5445f0c..215b1146e70 100644 --- a/extra/modules/optable-targeting/README.md +++ b/extra/modules/optable-targeting/README.md @@ -165,7 +165,8 @@ keep the `processed-auction-request` hook and add the `raw-auction-request` and When the `bidder-request` hook is in the execution plan, the `processed-auction-request` hook no longer blocks: it only starts the API call for requests the raw stage deferred or did not see, and the `bidder-request` hook awaits it. Without -the `bidder-request` hook, it keeps the legacy synchronous behavior. +the `bidder-request` hook, it keeps the legacy synchronous behavior: it enriches the whole request, so the request is +enriched for all bidders whenever `enrichment-percentage` and `bidder-enrichment-percentages` select any of them. ### Timeout considerations diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java index 153e46c601e..936419fd0c7 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java @@ -201,11 +201,17 @@ public Future> resolveOptableTargetingFl return update(BidRequestCleaner.instance(), moduleContext); } - final Future optableTargetingCall = targetingRequestExecutor.makeRequest( - auctionRequestPayload.bidRequest(), - invocationContext, - properties, - false); + final BidRequest bidRequest = auctionRequestPayload.bidRequest(); + if (!PropertiesValidator.isTrafficSourceValid(bidRequest, properties)) { + moduleContext.setShouldSkipEnrichment(true); + return enrichWhenCompleted(null, moduleContext, properties); + } + + // the whole request is enriched here, so it is enriched for all bidders when any of them is sampled + final Future optableTargetingCall = + CollectionUtils.isNotEmpty(bidderEnrichmentSampler.sample(bidRequest, properties)) + ? targetingRequestExecutor.makeRequest(bidRequest, invocationContext, properties, false) + : null; return enrichWhenCompleted(optableTargetingCall, moduleContext, properties); } diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java index eb028422b2c..b05c3466a86 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java @@ -26,6 +26,7 @@ import org.prebid.server.hooks.execution.v1.auction.AuctionRequestPayloadImpl; import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext; import org.prebid.server.hooks.modules.optable.targeting.model.Status; +import org.prebid.server.hooks.modules.optable.targeting.model.config.OptableTargetingProperties; import org.prebid.server.hooks.modules.optable.targeting.model.openrtb.TargetingResult; import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidRequestCleaner; import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidderEnrichmentSampler; @@ -49,6 +50,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -100,6 +102,7 @@ void setUp() { when(invocationContext.timeout()).thenReturn(timeout); when(activityInfrastructure.isAllowed(any(), any())).thenReturn(true); when(timeout.remaining()).thenReturn(1000L); + when(bidderEnrichmentSampler.sample(any(), any())).thenReturn(Set.of("bidder")); } private OptableTargetingFlowResolver givenFlowResolver(ExecutionPlan executionPlan) { @@ -588,4 +591,44 @@ void callShouldCleanRequestWhenResolvingFlowFails() { assertThat(((ModuleContext) result.moduleContext()).getEnrichRequestStatus().getStatus()) .isEqualTo(Status.FAIL); } + + @Test + void callShouldNotCallApiInLegacyModeWhenTrafficSourceIsDisabled() { + // given + final OptableTargetingProperties properties = givenOptableTargetingProperties(false); + properties.setEnrichWeb(false); + target = new OptableTargetingProcessedAuctionRequestHook( + new ConfigResolver(mapper, jsonMerger, properties), givenFlowResolver(ExecutionPlan.empty()), 0.01); + when(invocationContext.accountConfig()).thenReturn(null); + when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); + + // when + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); + + // then + assertThat(result.action()).isEqualTo(InvocationAction.update); + assertThat(((ModuleContext) result.moduleContext()).isShouldSkipEnrichment()).isTrue(); + verifyNoInteractions(optableTargeting); + } + + @Test + void callShouldNotCallApiInLegacyModeWhenNoBidderIsSampled() { + // given + when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); + when(bidderEnrichmentSampler.sample(any(), any())).thenReturn(Set.of()); + + // when + final InvocationResult result = + target.call(auctionRequestPayload, invocationContext).result(); + + // then + assertThat(result.action()).isEqualTo(InvocationAction.update); + final BidRequest bidRequest = result + .payloadUpdate() + .apply(AuctionRequestPayloadImpl.of(givenBidRequest())) + .bidRequest(); + assertThat(bidRequest.getUser().getEids()).isNull(); + verifyNoInteractions(optableTargeting); + } } From 23393e8e415750b96f5e146423ec2568bfcd618b Mon Sep 17 00:00:00 2001 From: Eugene Dorfman Date: Thu, 1 Oct 2026 22:29:06 +0200 Subject: [PATCH 8/8] optable-targeting: Leave amp and video requests unenriched The raw auction request stage does not run for them, and they were not enriched before, so the processed hook keeps passing them through when the bidder request hook is configured instead of starting the call. --- extra/modules/optable-targeting/README.md | 4 ---- .../v1/core/OptableTargetingFlowResolver.java | 5 ++--- ...getingProcessedAuctionRequestHookTest.java | 20 ++++--------------- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/extra/modules/optable-targeting/README.md b/extra/modules/optable-targeting/README.md index 1b3243f29cb..307b3f04737 100644 --- a/extra/modules/optable-targeting/README.md +++ b/extra/modules/optable-targeting/README.md @@ -24,10 +24,6 @@ Stored requests and stored imps are merged after the Raw Auction Request stage. not known at that stage (f.e. Prebid Mobile SDK traffic, where they live in the stored request), the Processed Auction Request hook starts the call on the merged request instead. Without it such requests are not enriched. -The Raw Auction Request stage does not run for `/openrtb2/amp` and `/openrtb2/video`. To enrich those, add the -`processed-auction-request` and `bidder-request` hooks to their plans: the Processed Auction Request hook then starts -the call. - We recommend defining the execution plan in the account config so the module is only invoked for specific accounts. See below for an example. diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java index 72fe02547fd..f3ff0bc65b5 100644 --- a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java +++ b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/core/OptableTargetingFlowResolver.java @@ -195,10 +195,9 @@ public Future> resolveOptableTargetingFl return failed(moduleContext); } - // the raw auction request stage does not run for f.e. amp and video requests + // the raw auction request stage does not run for f.e. amp and video requests, which are not enriched then if (hasBidderRequestHook) { - startTargetingCall(moduleContext, auctionRequestPayload.bidRequest(), invocationContext, properties, true); - return update(AuctionRequestCleaner.instance(), moduleContext); + return updateWithAnalytics(AuctionRequestCleaner.instance(), moduleContext); } final BidRequest bidRequest = auctionRequestPayload.bidRequest(); diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java index de3e865defe..558ca437ad3 100644 --- a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java +++ b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java @@ -370,15 +370,11 @@ void callShouldReturnResultWithEnrichedBidRequestWhenBothHooksAreAbsent() { } @Test - void callShouldStartCallForBidderRequestHookWhenRawAuctionRequestHookDidNotRun() { + void callShouldOnlyCleanRequestWhenRawAuctionRequestHookDidNotRunAndBidderRequestHookIsPresent() { // given target = new OptableTargetingProcessedAuctionRequestHook( configResolver, givenFlowResolver(givenExecutionPlan(false, true)), 0.01); - when(auctionRequestPayload.bidRequest()).thenReturn(givenBidRequest()); - when(optableTargeting.getTargeting(any(), any(), any(), any())) - .thenReturn(Future.succeededFuture(givenTargetingResult())); - when(bidderEnrichmentSampler.sample(any(), any())).thenReturn(Set.of("bidder")); // when final InvocationResult result = @@ -386,17 +382,9 @@ void callShouldStartCallForBidderRequestHookWhenRawAuctionRequestHookDidNotRun() // then assertThat(result.action()).isEqualTo(InvocationAction.update); - final ModuleContext moduleContext = (ModuleContext) result.moduleContext(); - assertThat(moduleContext.getBiddersToEnrich()).containsExactly("bidder"); - assertThat(moduleContext.getOptableTargetingCall().succeeded()).isTrue(); - assertThat(moduleContext.getOptableTargetingProperties()).isNotNull(); - - final BidRequest bidRequest = result - .payloadUpdate() - .apply(AuctionRequestPayloadImpl.of(givenBidRequest())) - .bidRequest(); - assertThat(bidRequest.getUser().getEids()).isNull(); - assertThat(bidRequest.getUser().getExt().getProperty("optable")).isNull(); + assertThat(result.payloadUpdate()).isInstanceOf(AuctionRequestCleaner.class); + assertThat(((ModuleContext) result.moduleContext()).getOptableTargetingCall()).isNull(); + verifyNoInteractions(optableTargeting); } @Test