Skip to content

Commit

Permalink
Clean code smells (#1532)
Browse files Browse the repository at this point in the history
  • Loading branch information
rpanchyk authored Oct 13, 2021
1 parent b711364 commit b498b4a
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 19 deletions.
7 changes: 3 additions & 4 deletions docs/endpoints/openrtb2/amp.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ This endpoint supports the following query parameters:
12. `slot` - tagId parameter for Imp object.
13. `gdpr_applies` - GDPR param for Regs Ext object.
14. `consent_type` - param to define what type of consent_string passed.
15. `attl_consent` - consentedProviders param for User Ext ConsentedProvidersSettings object.
15. `addtl_consent` - consentedProviders param for User Ext ConsentedProvidersSettings object.


For information on how these get from AMP into this endpoint, see [this pull request adding the query params to the Prebid callout](https://github.com/ampproject/amphtml/pull/14155) and [this issue adding support for network-level RTC macros](https://github.com/ampproject/amphtml/issues/12374).
Expand All @@ -138,12 +138,11 @@ If present, these will override parts of your Stored Request.
6. `slot` - will be used to set `tagId` parameter to overwrite Imp object.
7. `gdpr_applies` will be used to set `request.regs.ext.gdpr`
8. `consent_type` will be used to check what should be done with consent string
9. `attl_consent` will be used to set `user.ext.ConsentedProvidersSettings.consented_providers`.
9. `addtl_consent` will be used to set `user.ext.ConsentedProvidersSettings.consented_providers`.
10. `gdpr_consent` will be used to set `request.regs.ext.us_privacy` or `user.ext.consent`
11. `consent_string` will be used to set `request.regs.ext.us_privacy` or `user.ext.consent`. This param has bigger priority then `gdpr_consent`.



### Resolving Sizes

We strive to return ads with sizes which are valid for the `amp-ad` on your page. This logic intends to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,14 @@ private Future<BidRequest> parseBidRequest(HttpRequestContext httpRequest, Aucti

final ConsentType consentType = consentTypeFromQueryStringParams(httpRequest);
final String consentString = consentStringFromQueryStringParams(httpRequest);
final String attlConsent = attlConsentFromQueryStringParams(httpRequest);
final String addtlConsent = addtlConsentFromQueryStringParams(httpRequest);
final Integer gdpr = gdprFromQueryStringParams(httpRequest);
final Integer debug = debugFromQueryStringParam(httpRequest);
final Long timeout = timeoutFromQueryString(httpRequest);

final BidRequest bidRequest = BidRequest.builder()
.site(createSite(httpRequest))
.user(createUser(consentType, consentString, attlConsent))
.user(createUser(consentType, consentString, addtlConsent))
.regs(createRegs(consentString, consentType, gdpr))
.test(debug)
.tmax(timeout)
Expand All @@ -202,18 +202,18 @@ private static Site createSite(HttpRequestContext httpRequest) {
: null;
}

private static User createUser(ConsentType consentType, String consentString, String attlConsent) {
private static User createUser(ConsentType consentType, String consentString, String addtlConsent) {
final boolean tcfV2ConsentProvided = (StringUtils.isNotBlank(consentString)
&& TcfDefinerService.isConsentStringValid(consentString))
&& (consentType == null || consentType == ConsentType.tcfV2);

if (StringUtils.isNotBlank(attlConsent) || tcfV2ConsentProvided) {
if (StringUtils.isNotBlank(addtlConsent) || tcfV2ConsentProvided) {
final ExtUser.ExtUserBuilder userExtBuilder = ExtUser.builder();
if (tcfV2ConsentProvided) {
userExtBuilder.consent(consentString);
}
if (StringUtils.isNotBlank(attlConsent)) {
userExtBuilder.consentedProvidersSettings(ConsentedProvidersSettings.of(attlConsent));
if (StringUtils.isNotBlank(addtlConsent)) {
userExtBuilder.consentedProvidersSettings(ConsentedProvidersSettings.of(addtlConsent));
}
return User.builder().ext(userExtBuilder.build()).build();
}
Expand Down Expand Up @@ -279,7 +279,7 @@ private static String consentStringFromQueryStringParams(HttpRequestContext http
return ObjectUtils.firstNonNull(requestConsentParam, requestGdprConsentParam);
}

private static String attlConsentFromQueryStringParams(HttpRequestContext httpRequest) {
private static String addtlConsentFromQueryStringParams(HttpRequestContext httpRequest) {
return httpRequest.getQueryParams().get(ADDTL_CONSENT_PARAM);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ public void shouldReturnBidRequestWithProvidersSettingsContainsAddtlConsentIfPar
}

@Test
public void shouldReturnBidRequestWithoutProvidersSettingsIfAttlConsentIsMissed() {
public void shouldReturnBidRequestWithoutProvidersSettingsIfAddtlConsentIsMissed() {
// given
givenBidRequest();

Expand All @@ -1281,10 +1281,10 @@ public void shouldReturnBidRequestWithoutProvidersSettingsIfAttlConsentIsMissed(
}

@Test
public void shouldReturnBidRequestWithoutProvidersSettingsIfAttlConsentIsBlank() {
public void shouldReturnBidRequestWithoutProvidersSettingsIfAddtlConsentIsBlank() {
// given
routingContext.queryParams()
.add("attl_consent", " ");
.add("addtl_consent", " ");

givenBidRequest();

Expand Down Expand Up @@ -1633,7 +1633,6 @@ public void shouldUseBidRequestModifiedByProcessedAuctionRequestHooks() {
final Future<AuctionContext> result = target.fromRequest(routingContext, 0L);

// then

final BidRequest resultBidRequest = result.result().getBidRequest();
assertThat(resultBidRequest.getSite()).isNull();
assertThat(resultBidRequest.getApp()).isEqualTo(App.builder().bundle("org.company.application").build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ public void shouldUseBidRequestModifiedByProcessedAuctionRequestHooks() {
final Future<AuctionContext> result = target.fromRequest(routingContext, 0L);

// then

final BidRequest resultBidRequest = result.result().getBidRequest();
assertThat(resultBidRequest.getSite()).isNull();
assertThat(resultBidRequest.getApp()).isEqualTo(App.builder().bundle("org.company.application").build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,6 @@ public void makeHttpRequestsShouldReplacePmZoneIDOldKeyNameWithNew() {
final Result<List<HttpRequest<BidRequest>>> result = pubmaticBidder.makeHttpRequests(bidRequest);

// then

final Map<String, String> expectedKeyWords = singletonMap("pmZoneId", "value1,value2");
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ public void requestBidsShouldReturnBidderSeatBidWithOneBidAndFilterOutOne() {
@Test
public void requestBidsShouldFilterBidderSeatBidForWhichImpPmpIsNull() {
// given

bidderRequester.setBidRates(Collections.singletonMap("lineItemId1", 1.00));
given(lineItemService.getLineItemById(eq("lineItemId1"))).willReturn(LineItem.of(
LineItemMetaData.builder().price(Price.of(BigDecimal.ONE, "USD")).build(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public void handleShouldReturnBadRequestWhenNoParametersInRequest() {
@Test
public void handleShouldReturnBadRequestWhenDurationWasNotDefined() {
// given

given(httpRequest.params()).willReturn(MultiMap.caseInsensitiveMultiMap().add("account", "1001"));

// when
Expand Down

0 comments on commit b498b4a

Please sign in to comment.