Skip to content

Commit de5d14f

Browse files
justadreamerjustadreamer
authored
optable-targeting: update macros, fail early if tenant and origin are not configured (#4131)
1 parent 0017139 commit de5d14f

8 files changed

Lines changed: 141 additions & 24 deletions

File tree

‎extra/modules/optable-targeting/sample-requests/data.json‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
{
4747
"optable":
4848
{
49-
"email": "fd911bd8cac2e603a80efafca2210b7a917c97410f0c29d9f2bfb99867e5a589"
49+
"email": "5837d278eabede28e37b5766399ed0d1a4cdc36acee8d35710a255032f45beda"
5050
},
5151
"eids":
5252
[
@@ -79,16 +79,6 @@
7979
}
8080
]
8181
},
82-
{
83-
"source": "id5-sync.com",
84-
"uids":
85-
[
86-
{
87-
"id": "ID5*dd1b31e65f5e45548c11a0275ba3a8072c00e3a2a0493e8f5a8f54f8067e8b00",
88-
"atype": 1
89-
}
90-
]
91-
},
9282
{
9383
"source": "amxdt.net",
9484
"uids":

‎extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/config/OptableTargetingConfig.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,15 @@ ConfigResolver configResolver(JsonMerger jsonMerger, OptableTargetingProperties
9090
OptableTargetingModule optableTargetingModule(ConfigResolver configResolver,
9191
OptableTargeting optableTargeting,
9292
UserFpdActivityMask userFpdActivityMask,
93-
JsonMerger jsonMerger) {
93+
JsonMerger jsonMerger,
94+
@Value("${logging.sampling-rate:0.01}") double logSamplingRate) {
9495

9596
return new OptableTargetingModule(List.of(
9697
new OptableTargetingProcessedAuctionRequestHook(
9798
configResolver,
9899
optableTargeting,
99-
userFpdActivityMask),
100+
userFpdActivityMask,
101+
logSamplingRate),
100102
new OptableTargetingAuctionResponseHook(
101103
configResolver,
102104
ObjectMapperProvider.mapper(),

‎extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHook.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.iab.openrtb.request.Device;
55
import com.iab.openrtb.request.User;
66
import io.vertx.core.Future;
7+
import org.apache.commons.lang3.StringUtils;
78
import org.prebid.server.activity.Activity;
89
import org.prebid.server.activity.ComponentType;
910
import org.prebid.server.activity.infrastructure.ActivityInfrastructure;
@@ -32,24 +33,32 @@
3233
import org.prebid.server.hooks.v1.auction.AuctionInvocationContext;
3334
import org.prebid.server.hooks.v1.auction.AuctionRequestPayload;
3435
import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook;
36+
import org.prebid.server.log.ConditionalLogger;
37+
import org.prebid.server.log.LoggerFactory;
3538

3639
import java.util.Objects;
3740

3841
public class OptableTargetingProcessedAuctionRequestHook implements ProcessedAuctionRequestHook {
3942

43+
private static final ConditionalLogger conditionalLogger = new ConditionalLogger(
44+
LoggerFactory.getLogger(OptableTargetingProcessedAuctionRequestHook.class));
45+
4046
public static final String CODE = "optable-targeting-processed-auction-request-hook";
4147

4248
private final ConfigResolver configResolver;
4349
private final OptableTargeting optableTargeting;
4450
private final UserFpdActivityMask userFpdActivityMask;
51+
private final double logSamplingRate;
4552

4653
public OptableTargetingProcessedAuctionRequestHook(ConfigResolver configResolver,
4754
OptableTargeting optableTargeting,
48-
UserFpdActivityMask userFpdActivityMask) {
55+
UserFpdActivityMask userFpdActivityMask,
56+
double logSamplingRate) {
4957

5058
this.configResolver = Objects.requireNonNull(configResolver);
5159
this.optableTargeting = Objects.requireNonNull(optableTargeting);
5260
this.userFpdActivityMask = Objects.requireNonNull(userFpdActivityMask);
61+
this.logSamplingRate = logSamplingRate;
5362
}
5463

5564
@Override
@@ -58,6 +67,16 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
5867

5968
final OptableTargetingProperties properties = configResolver.resolve(invocationContext.accountConfig());
6069
final ModuleContext moduleContext = new ModuleContext();
70+
final long callTargetingAPITimestamp = System.currentTimeMillis();
71+
72+
if (!isTargetingPropertiesValid(properties)) {
73+
conditionalLogger.error(
74+
"Account not properly configured: tenant and/or origin is missing.", logSamplingRate);
75+
76+
moduleContext.setOptableTargetingExecutionTime(System.currentTimeMillis() - callTargetingAPITimestamp);
77+
moduleContext.setEnrichRequestStatus(EnrichmentStatus.failure());
78+
return update(BidRequestCleaner.instance(), moduleContext);
79+
}
6180

6281
final BidRequest bidRequest = applyActivityRestrictions(auctionRequestPayload.bidRequest(), invocationContext);
6382

@@ -66,7 +85,6 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
6685
invocationContext.auctionContext(),
6786
properties.getTimeout());
6887

69-
final long callTargetingAPITimestamp = System.currentTimeMillis();
7088
return optableTargeting.getTargeting(properties, bidRequest, attributes, timeout)
7189
.compose(targetingResult -> {
7290
moduleContext.setOptableTargetingExecutionTime(
@@ -81,6 +99,10 @@ public Future<InvocationResult<AuctionRequestPayload>> call(AuctionRequestPayloa
8199
});
82100
}
83101

102+
private boolean isTargetingPropertiesValid(OptableTargetingProperties properties) {
103+
return !StringUtils.isEmpty(properties.getOrigin()) && !StringUtils.isEmpty(properties.getTenant());
104+
}
105+
84106
private BidRequest applyActivityRestrictions(BidRequest bidRequest,
85107
AuctionInvocationContext auctionInvocationContext) {
86108

‎extra/modules/optable-targeting/src/main/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ public class APIClientImpl implements APIClient {
2727
private static final Logger logger = LoggerFactory.getLogger(APIClientImpl.class);
2828
private static final ConditionalLogger conditionalLogger = new ConditionalLogger(logger);
2929

30-
private static final String TENANT = "{TENANT}";
31-
private static final String ORIGIN = "{ORIGIN}";
30+
private static final String TENANT = "{{TENANT}}";
31+
private static final String ORIGIN = "{{ORIGIN}}";
3232

3333
private final String endpoint;
3434
private final HttpClient httpClient;

‎extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/BaseOptableTest.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,24 @@ protected String givenBodyFromFile(String fileName) {
224224
}
225225

226226
protected OptableTargetingProperties givenOptableTargetingProperties(boolean enableCache) {
227-
return givenOptableTargetingProperties("key", enableCache);
227+
return givenOptableTargetingProperties("key", "accountId", "origin", enableCache);
228228
}
229229

230230
protected OptableTargetingProperties givenOptableTargetingProperties(String key, boolean enableCache) {
231+
return givenOptableTargetingProperties(key, "accountId", "origin", enableCache);
232+
}
233+
234+
protected OptableTargetingProperties givenOptableTargetingProperties(String key,
235+
String tenant,
236+
String origin,
237+
boolean enableCache) {
231238
final CacheProperties cacheProperties = new CacheProperties();
232239
cacheProperties.setEnabled(enableCache);
233240

234241
final OptableTargetingProperties optableTargetingProperties = new OptableTargetingProperties();
235242
optableTargetingProperties.setApiEndpoint("endpoint");
236-
optableTargetingProperties.setTenant("accountId");
237-
optableTargetingProperties.setOrigin("origin");
243+
optableTargetingProperties.setTenant(tenant);
244+
optableTargetingProperties.setOrigin(origin);
238245
optableTargetingProperties.setApiKey(key);
239246
optableTargetingProperties.setPpidMapping(Map.of("c", "id"));
240247
optableTargetingProperties.setAdserverTargeting(true);
@@ -245,6 +252,6 @@ protected OptableTargetingProperties givenOptableTargetingProperties(String key,
245252
}
246253

247254
protected Query givenQuery() {
248-
return Query.of("que", "ry");
255+
return Query.of("?que", "ry");
249256
}
250257
}

‎extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/OptableTargetingProcessedAuctionRequestHookTest.java‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import org.prebid.server.auction.privacy.enforcement.mask.UserFpdActivityMask;
1515
import org.prebid.server.execution.timeout.Timeout;
1616
import org.prebid.server.hooks.execution.v1.auction.AuctionRequestPayloadImpl;
17+
import org.prebid.server.hooks.modules.optable.targeting.model.ModuleContext;
18+
import org.prebid.server.hooks.modules.optable.targeting.model.Status;
1719
import org.prebid.server.hooks.modules.optable.targeting.v1.core.ConfigResolver;
1820
import org.prebid.server.hooks.modules.optable.targeting.v1.core.OptableTargeting;
1921
import org.prebid.server.hooks.v1.InvocationAction;
@@ -61,7 +63,8 @@ public void setUp() {
6163
target = new OptableTargetingProcessedAuctionRequestHook(
6264
configResolver,
6365
optableTargeting,
64-
userFpdActivityMask);
66+
userFpdActivityMask,
67+
0.01);
6568

6669
when(invocationContext.accountConfig()).thenReturn(givenAccountConfig(true));
6770
when(invocationContext.auctionContext()).thenReturn(givenAuctionContext(activityInfrastructure, timeout));
@@ -128,6 +131,70 @@ public void shouldReturnResultWithUpdateActionWhenOptableTargetingReturnTargetin
128131
assertThat(bidRequest.getUser().getData().getFirst().getSegment().getFirst().getId()).isEqualTo("id");
129132
}
130133

134+
@Test
135+
public void shouldReturnFailWhenOriginIsAbsentInAccountConfiguration() {
136+
// given
137+
configResolver = new ConfigResolver(
138+
mapper,
139+
jsonMerger,
140+
givenOptableTargetingProperties("key", "tenant", null, false));
141+
target = new OptableTargetingProcessedAuctionRequestHook(
142+
configResolver,
143+
optableTargeting,
144+
userFpdActivityMask,
145+
0.01);
146+
when(invocationContext.accountConfig())
147+
.thenReturn(givenAccountConfig("key", "tenant", null, true));
148+
149+
// when
150+
final Future<InvocationResult<AuctionRequestPayload>> future = target.call(auctionRequestPayload,
151+
invocationContext);
152+
153+
// then
154+
assertThat(future).isNotNull();
155+
assertThat(future.succeeded()).isTrue();
156+
157+
final InvocationResult<AuctionRequestPayload> result = future.result();
158+
assertThat(result).isNotNull();
159+
assertThat(result.status()).isEqualTo(InvocationStatus.success);
160+
assertThat(result.action()).isEqualTo(InvocationAction.update);
161+
assertThat((ModuleContext) result.moduleContext())
162+
.extracting(it -> it.getEnrichRequestStatus().getStatus())
163+
.isEqualTo(Status.FAIL);
164+
}
165+
166+
@Test
167+
public void shouldReturnFailWhenTenantIsAbsentInAccountConfiguration() {
168+
// given
169+
configResolver = new ConfigResolver(
170+
mapper,
171+
jsonMerger,
172+
givenOptableTargetingProperties("key", null, "origin", false));
173+
target = new OptableTargetingProcessedAuctionRequestHook(
174+
configResolver,
175+
optableTargeting,
176+
userFpdActivityMask,
177+
0.01);
178+
when(invocationContext.accountConfig())
179+
.thenReturn(givenAccountConfig("key", null, null, true));
180+
181+
// when
182+
final Future<InvocationResult<AuctionRequestPayload>> future = target.call(auctionRequestPayload,
183+
invocationContext);
184+
185+
// then
186+
assertThat(future).isNotNull();
187+
assertThat(future.succeeded()).isTrue();
188+
189+
final InvocationResult<AuctionRequestPayload> result = future.result();
190+
assertThat(result).isNotNull();
191+
assertThat(result.status()).isEqualTo(InvocationStatus.success);
192+
assertThat(result.action()).isEqualTo(InvocationAction.update);
193+
assertThat((ModuleContext) result.moduleContext())
194+
.extracting(it -> it.getEnrichRequestStatus().getStatus())
195+
.isEqualTo(Status.FAIL);
196+
}
197+
131198
@Test
132199
public void shouldReturnResultWithCleanedUpUserExtOptableTag() {
133200
// given
@@ -180,6 +247,10 @@ public void shouldReturnResultWithUpdateWhenOptableTargetingDoesntReturnResult()
180247
}
181248

182249
private ObjectNode givenAccountConfig(boolean cacheEnabled) {
183-
return mapper.valueToTree(givenOptableTargetingProperties(cacheEnabled));
250+
return givenAccountConfig("key", "tenant", "origin", cacheEnabled);
251+
}
252+
253+
private ObjectNode givenAccountConfig(String key, String tenant, String origin, boolean cacheEnabled) {
254+
return mapper.valueToTree(givenOptableTargetingProperties(key, tenant, origin, cacheEnabled));
184255
}
185256
}

‎extra/modules/optable-targeting/src/test/java/org/prebid/server/hooks/modules/optable/targeting/v1/net/APIClientImplTest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,31 @@ public void shouldUseAuthorizationHeaderIfApiKeyIsPresent() {
153153
assertThat(result.result()).isNull();
154154
}
155155

156+
@Test
157+
public void shouldBuildApiUrlByReplacingTenantAndOriginMacros() {
158+
// given
159+
target = new APIClientImpl(
160+
"http://endpoint.optable.com?t={{TENANT}}&o={{ORIGIN}}",
161+
httpClient,
162+
jacksonMapper,
163+
10);
164+
165+
when(httpClient.get(any(), any(), anyLong()))
166+
.thenReturn(Future.succeededFuture(givenFailHttpResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR,
167+
"plain_text_response.json")));
168+
169+
// when
170+
final Future<TargetingResult> result = target.getTargeting(givenOptableTargetingProperties(false),
171+
givenQuery(), List.of("8.8.8.8"), timeout);
172+
173+
// then
174+
final ArgumentCaptor<String> endpointCaptor = ArgumentCaptor.forClass(String.class);
175+
verify(httpClient).get(endpointCaptor.capture(), any(), anyLong());
176+
assertThat(endpointCaptor.getValue())
177+
.isEqualTo("http://endpoint.optable.com?t=accountId&o=origin?query");
178+
assertThat(result.result()).isNull();
179+
}
180+
156181
@Test
157182
public void shouldNotUseAuthorizationHeaderIfApiKeyIsAbsent() {
158183
// given

‎sample/configs/prebid-config-with-optable.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,4 @@ hooks:
5050
enabled: true
5151
modules:
5252
optable-targeting:
53-
api-endpoint: https://na.edge.optable.co/v2/targeting?t={TENANT}&o={ORIGIN}
53+
api-endpoint: https://na.edge.optable.co/v2/targeting?t={{TENANT}}&o={{ORIGIN}}

0 commit comments

Comments
 (0)