optable-targeting: update macros, fail early if tenant and origin are not configured (#4131)

justadreamer · web-flow · commit de5d14f10750 · 2025-08-21T13:45:32.000+02:00
diff --git a/extra/modules/optable-targeting/sample-requests/data.json b/extra/modules/optable-targeting/sample-requests/data.json
@@ -46,7 +46,7 @@
     {
       "optable":
       {
-        "email": "fd911bd8cac2e603a80efafca2210b7a917c97410f0c29d9f2bfb99867e5a589"
+        "email": "5837d278eabede28e37b5766399ed0d1a4cdc36acee8d35710a255032f45beda"
       },
       "eids":
       [
@@ -79,16 +79,6 @@
             }
           ]
         },
-        {
-          "source": "id5-sync.com",
-          "uids":
-          [
-            {
-              "id": "ID5*dd1b31e65f5e45548c11a0275ba3a8072c00e3a2a0493e8f5a8f54f8067e8b00",
-              "atype": 1
-            }
-          ]
-        },
         {
           "source": "amxdt.net",
           "uids":
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
@@ -90,13 +90,15 @@ ConfigResolver configResolver(JsonMerger jsonMerger, OptableTargetingProperties
     OptableTargetingModule optableTargetingModule(ConfigResolver configResolver,
                                                   OptableTargeting optableTargeting,
                                                   UserFpdActivityMask userFpdActivityMask,
-                                                  JsonMerger jsonMerger) {
+                                                  JsonMerger jsonMerger,
+                                                  @Value("${logging.sampling-rate:0.01}") double logSamplingRate) {
 
         return new OptableTargetingModule(List.of(
                 new OptableTargetingProcessedAuctionRequestHook(
                         configResolver,
                         optableTargeting,
-                        userFpdActivityMask),
+                        userFpdActivityMask,
+                        logSamplingRate),
                 new OptableTargetingAuctionResponseHook(
                         configResolver,
                         ObjectMapperProvider.mapper(),
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
@@ -4,6 +4,7 @@
 import com.iab.openrtb.request.Device;
 import com.iab.openrtb.request.User;
 import io.vertx.core.Future;
+import org.apache.commons.lang3.StringUtils;
 import org.prebid.server.activity.Activity;
 import org.prebid.server.activity.ComponentType;
 import org.prebid.server.activity.infrastructure.ActivityInfrastructure;
@@ -32,24 +33,32 @@
 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;
 
 public class OptableTargetingProcessedAuctionRequestHook implements ProcessedAuctionRequestHook {
 
+    private static final ConditionalLogger conditionalLogger = new ConditionalLogger(
+            LoggerFactory.getLogger(OptableTargetingProcessedAuctionRequestHook.class));
+
     public static final String CODE = "optable-targeting-processed-auction-request-hook";
 
     private final ConfigResolver configResolver;
     private final OptableTargeting optableTargeting;
     private final UserFpdActivityMask userFpdActivityMask;
+    private final double logSamplingRate;
 
     public OptableTargetingProcessedAuctionRequestHook(ConfigResolver configResolver,
                                                        OptableTargeting optableTargeting,
-                                                       UserFpdActivityMask userFpdActivityMask) {
+                                                       UserFpdActivityMask userFpdActivityMask,
+                                                       double logSamplingRate) {
 
         this.configResolver = Objects.requireNonNull(configResolver);
         this.optableTargeting = Objects.requireNonNull(optableTargeting);
         this.userFpdActivityMask = Objects.requireNonNull(userFpdActivityMask);
+        this.logSamplingRate = logSamplingRate;
     }
 
     @Override
@@ -58,6 +67,16 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
 
         final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig());
         final ModuleContext moduleContext = new ModuleContext();
+        final long callTargetingAPITimestamp = System.currentTimeMillis();
+
+        if (!isTargetingPropertiesValid(properties)) {
+            conditionalLogger.error(
+                    "Account not properly configured: tenant and/or origin is missing.", logSamplingRate);
+
+            moduleContext.setOptableTargetingExecutionTime(System.currentTimeMillis() - callTargetingAPITimestamp);
+            moduleContext.setEnrichRequestStatus(EnrichmentStatus.failure());
+            return update(BidRequestCleaner.instance(), moduleContext);
+        }
 
         final BidRequest bidRequest = applyActivityRestrictions(auctionRequestPayload.bidRequest(), invocationContext);
 
@@ -66,7 +85,6 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
                 invocationContext.auctionContext(),
                 properties.getTimeout());
 
-        final long callTargetingAPITimestamp = System.currentTimeMillis();
         return optableTargeting.getTargeting(properties, bidRequest, attributes, timeout)
                 .compose(targetingResult -> {
                     moduleContext.setOptableTargetingExecutionTime(
@@ -81,6 +99,10 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
                 });
     }
 
+    private boolean isTargetingPropertiesValid(OptableTargetingProperties properties) {
+        return !StringUtils.isEmpty(properties.getOrigin()) && !StringUtils.isEmpty(properties.getTenant());
+    }
+
     private BidRequest applyActivityRestrictions(BidRequest bidRequest,
                                                  AuctionInvocationContext auctionInvocationContext) {
 
diff --git a/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImpl.java b/extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImpl.java
@@ -27,8 +27,8 @@ public class APIClientImpl implements APIClient {
     private static final Logger logger = LoggerFactory.getLogger(APIClientImpl.class);
     private static final ConditionalLogger conditionalLogger = new ConditionalLogger(logger);
 
-    private static final String TENANT = "{TENANT}";
-    private static final String ORIGIN = "{ORIGIN}";
+    private static final String TENANT = "{{TENANT}}";
+    private static final String ORIGIN = "{{ORIGIN}}";
 
     private final String endpoint;
     private final HttpClient httpClient;
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
@@ -224,17 +224,24 @@ protected String givenBodyFromFile(String fileName) {
     }
 
     protected OptableTargetingProperties givenOptableTargetingProperties(boolean enableCache) {
-        return givenOptableTargetingProperties("key", enableCache);
+        return givenOptableTargetingProperties("key", "accountId", "origin", enableCache);
     }
 
     protected OptableTargetingProperties givenOptableTargetingProperties(String key, boolean enableCache) {
+        return givenOptableTargetingProperties(key, "accountId", "origin", enableCache);
+    }
+
+    protected OptableTargetingProperties givenOptableTargetingProperties(String key,
+                                                                         String tenant,
+                                                                         String origin,
+                                                                         boolean enableCache) {
         final CacheProperties cacheProperties = new CacheProperties();
         cacheProperties.setEnabled(enableCache);
 
         final OptableTargetingProperties optableTargetingProperties = new OptableTargetingProperties();
         optableTargetingProperties.setApiEndpoint("endpoint");
-        optableTargetingProperties.setTenant("accountId");
-        optableTargetingProperties.setOrigin("origin");
+        optableTargetingProperties.setTenant(tenant);
+        optableTargetingProperties.setOrigin(origin);
         optableTargetingProperties.setApiKey(key);
         optableTargetingProperties.setPpidMapping(Map.of("c", "id"));
         optableTargetingProperties.setAdserverTargeting(true);
@@ -245,6 +252,6 @@ protected OptableTargetingProperties givenOptableTargetingProperties(String key,
     }
 
     protected Query givenQuery() {
-        return Query.of("que", "ry");
+        return Query.of("?que", "ry");
     }
 }
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
@@ -14,6 +14,8 @@
 import org.prebid.server.auction.privacy.enforcement.mask.UserFpdActivityMask;
 import org.prebid.server.execution.timeout.Timeout;
 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.v1.core.ConfigResolver;
 import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargeting;
 import org.prebid.server.hooks.v1.InvocationAction;
@@ -61,7 +63,8 @@ public void setUp() {
         target = new OptableTargetingProcessedAuctionRequestHook(
                 configResolver,
                 optableTargeting,
-                userFpdActivityMask);
+                userFpdActivityMask,
+                0.01);
 
         when(invocationContext.accountConfig()).thenReturn(givenAccountConfig(true));
         when(invocationContext.auctionContext()).thenReturn(givenAuctionContext(activityInfrastructure, timeout));
@@ -128,6 +131,70 @@ public void shouldReturnResultWithUpdateActionWhenOptableTargetingReturnTargetin
         assertThat(bidRequest.getUser().getData().getFirst().getSegment().getFirst().getId()).isEqualTo("id");
     }
 
+    @Test
+    public void shouldReturnFailWhenOriginIsAbsentInAccountConfiguration() {
+        // given
+        configResolver = new ConfigResolver(
+                mapper,
+                jsonMerger,
+                givenOptableTargetingProperties("key", "tenant", null, false));
+        target = new OptableTargetingProcessedAuctionRequestHook(
+                configResolver,
+                optableTargeting,
+                userFpdActivityMask,
+                0.01);
+        when(invocationContext.accountConfig())
+                .thenReturn(givenAccountConfig("key", "tenant", null, true));
+
+        // when
+        final Future<InvocationResult<AuctionRequestPayload>> future = target.call(auctionRequestPayload,
+                invocationContext);
+
+        // then
+        assertThat(future).isNotNull();
+        assertThat(future.succeeded()).isTrue();
+
+        final InvocationResult<AuctionRequestPayload> result = future.result();
+        assertThat(result).isNotNull();
+        assertThat(result.status()).isEqualTo(InvocationStatus.success);
+        assertThat(result.action()).isEqualTo(InvocationAction.update);
+        assertThat((ModuleContext) result.moduleContext())
+                .extracting(it -> it.getEnrichRequestStatus().getStatus())
+                .isEqualTo(Status.FAIL);
+    }
+
+    @Test
+    public void shouldReturnFailWhenTenantIsAbsentInAccountConfiguration() {
+        // given
+        configResolver = new ConfigResolver(
+                mapper,
+                jsonMerger,
+                givenOptableTargetingProperties("key", null, "origin", false));
+        target = new OptableTargetingProcessedAuctionRequestHook(
+                configResolver,
+                optableTargeting,
+                userFpdActivityMask,
+                0.01);
+        when(invocationContext.accountConfig())
+                .thenReturn(givenAccountConfig("key", null, null, true));
+
+        // when
+        final Future<InvocationResult<AuctionRequestPayload>> future = target.call(auctionRequestPayload,
+                invocationContext);
+
+        // then
+        assertThat(future).isNotNull();
+        assertThat(future.succeeded()).isTrue();
+
+        final InvocationResult<AuctionRequestPayload> result = future.result();
+        assertThat(result).isNotNull();
+        assertThat(result.status()).isEqualTo(InvocationStatus.success);
+        assertThat(result.action()).isEqualTo(InvocationAction.update);
+        assertThat((ModuleContext) result.moduleContext())
+                .extracting(it -> it.getEnrichRequestStatus().getStatus())
+                .isEqualTo(Status.FAIL);
+    }
+
     @Test
     public void shouldReturnResultWithCleanedUpUserExtOptableTag() {
         // given
@@ -180,6 +247,10 @@ public void shouldReturnResultWithUpdateWhenOptableTargetingDoesntReturnResult()
     }
 
     private ObjectNode givenAccountConfig(boolean cacheEnabled) {
-        return mapper.valueToTree(givenOptableTargetingProperties(cacheEnabled));
+        return givenAccountConfig("key", "tenant", "origin", cacheEnabled);
+    }
+
+    private ObjectNode givenAccountConfig(String key, String tenant, String origin, boolean cacheEnabled) {
+        return mapper.valueToTree(givenOptableTargetingProperties(key, tenant, origin, cacheEnabled));
     }
 }
diff --git a/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImplTest.java b/extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImplTest.java
@@ -153,6 +153,31 @@ public void shouldUseAuthorizationHeaderIfApiKeyIsPresent() {
         assertThat(result.result()).isNull();
     }
 
+    @Test
+    public void shouldBuildApiUrlByReplacingTenantAndOriginMacros() {
+        //  given
+        target = new APIClientImpl(
+                "http://endpoint.optable.com?t={{TENANT}}&o={{ORIGIN}}",
+                httpClient,
+                jacksonMapper,
+                10);
+
+        when(httpClient.get(any(), any(), anyLong()))
+                .thenReturn(Future.succeededFuture(givenFailHttpResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR,
+                        "plain_text_response.json")));
+
+        // when
+        final Future<TargetingResult> result = target.getTargeting(givenOptableTargetingProperties(false),
+                givenQuery(), List.of("8.8.8.8"), timeout);
+
+        // then
+        final ArgumentCaptor<String> endpointCaptor = ArgumentCaptor.forClass(String.class);
+        verify(httpClient).get(endpointCaptor.capture(), any(), anyLong());
+        assertThat(endpointCaptor.getValue())
+                .isEqualTo("http://endpoint.optable.com?t=accountId&o=origin?query");
+        assertThat(result.result()).isNull();
+    }
+
     @Test
     public void shouldNotUseAuthorizationHeaderIfApiKeyIsAbsent() {
         //  given
diff --git a/sample/configs/prebid-config-with-optable.yaml b/sample/configs/prebid-config-with-optable.yaml
@@ -50,4 +50,4 @@ hooks:
     enabled: true
   modules:
     optable-targeting:
-      api-endpoint: https://na.edge.optable.co/v2/targeting?t={TENANT}&o={ORIGIN}
+      api-endpoint: https://na.edge.optable.co/v2/targeting?t={{TENANT}}&o={{ORIGIN}}

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@`
`46`	`46`	`{`
`47`	`47`	`"optable":`
`48`	`48`	`{`
`49`		`- "email": "fd911bd8cac2e603a80efafca2210b7a917c97410f0c29d9f2bfb99867e5a589"`
	`49`	`+ "email": "5837d278eabede28e37b5766399ed0d1a4cdc36acee8d35710a255032f45beda"`
`50`	`50`	`},`
`51`	`51`	`"eids":`
`52`	`52`	`[`
`@@ -79,16 +79,6 @@`
`79`	`79`	`}`
`80`	`80`	`]`
`81`	`81`	`},`
`82`		`- {`
`83`		`- "source": "id5-sync.com",`
`84`		`- "uids":`
`85`		`- [`
`86`		`- {`
`87`		`- "id": "ID5*dd1b31e65f5e45548c11a0275ba3a8072c00e3a2a0493e8f5a8f54f8067e8b00",`
`88`		`- "atype": 1`
`89`		`- }`
`90`		`- ]`
`91`		`- },`
`92`	`82`	`{`
`93`	`83`	`"source": "amxdt.net",`
`94`	`84`	`"uids":`