Skip to content

Commit

Permalink
Code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
William Harris committed Feb 26, 2024
1 parent cc0cca6 commit f381f45
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 56 deletions.
22 changes: 19 additions & 3 deletions src/main/java/org/prebid/server/bidder/yieldlab/YieldlabBidder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Collectors;

public class YieldlabBidder implements Bidder<Void> {
Expand Down Expand Up @@ -273,12 +274,20 @@ private static boolean hasDsa(BidRequest request) {
&& request.getRegs().getExt().getDsa() != null;
}

private static String encodeTransparenciesAsString(List<ExtRegsDsaTransparency> transparency) {
return transparency.stream()
private static String encodeTransparenciesAsString(List<ExtRegsDsaTransparency> transparencies) {
return transparencies.stream()
.filter(transparencyIsValid())
.map(YieldlabBidder::encodeTransparency)
.collect(Collectors.joining("~~"));
}

private static Predicate<ExtRegsDsaTransparency> transparencyIsValid() {
return transparency ->
!Objects.isNull(transparency.getDomain())
&& !Objects.isNull(transparency.getDsaParams())
&& !transparency.getDsaParams().isEmpty();
}

private static String encodeTransparency(ExtRegsDsaTransparency transparency) {
return "%s~%s".formatted(transparency.getDomain(), encodeTransparencyParams(transparency.getDsaParams()));
}
Expand Down Expand Up @@ -471,8 +480,15 @@ private ObjectNode resolveExtParameter(YieldlabDigitalServicesActResponse dsa) {
return null;
}
final var ext = mapper.mapper().createObjectNode();
final var dsaNode = mapper.mapper().convertValue(dsa, JsonNode.class);
JsonNode dsaNode;
try {
dsaNode = mapper.mapper().convertValue(dsa, JsonNode.class);
} catch (IllegalArgumentException e) {
throw new PreBidException("Failed to serialize DSA object", e);
}
ext.set("dsa", dsaNode);
return ext;


}
}
82 changes: 29 additions & 53 deletions src/test/java/org/prebid/server/bidder/yieldlab/YieldlabBidderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
import com.iab.openrtb.request.User;
import com.iab.openrtb.request.Video;
import com.iab.openrtb.response.Bid;
import org.junit.Before;
import org.junit.Test;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -33,7 +33,6 @@
import org.prebid.server.proto.openrtb.ext.request.yieldlab.ExtImpYieldlab;
import org.prebid.server.proto.openrtb.ext.response.BidType;

import java.lang.reflect.InvocationTargetException;
import java.math.BigDecimal;
import java.time.Clock;
import java.time.Instant;
Expand All @@ -60,7 +59,7 @@ public class YieldlabBidderTest extends VertxTest {

private YieldlabBidder target;

@Before
@BeforeEach
public void setUp() {
clock = Clock.fixed(Instant.parse("2019-07-26T10:00:00Z"), ZoneId.systemDefault());
target = new YieldlabBidder(ENDPOINT_URL, clock, jacksonMapper);
Expand Down Expand Up @@ -318,11 +317,11 @@ public static Stream<? extends Arguments> dsaRequestCases() {
ExtRegsDsa.of(
1, 2, 3, List.of(ExtRegsDsaTransparency.of("testDomain", List.of(1, 2, 3)))
),
Map.of(
"dsarequired", "1",
"dsapubrender", "2",
"dsadatatopub", "3",
"dsatransparency", "testDomain~1_2_3"
List.of(
"&dsarequired=1",
"&dsapubrender=2",
"&dsadatatopub=3",
"&dsatransparency=testDomain%7E1_2_3"
)
),
arguments(
Expand All @@ -333,11 +332,11 @@ public static Stream<? extends Arguments> dsaRequestCases() {
ExtRegsDsaTransparency.of("testDomain2", List.of(4, 5, 6))
)
),
Map.of(
"dsarequired", "1",
"dsapubrender", "2",
"dsadatatopub", "3",
"dsatransparency", "testDomain~1_2_3~~testDomain2~4_5_6"
List.of(
"&dsarequired=1",
"&dsapubrender=2",
"&dsadatatopub=3",
"&dsatransparency=testDomain%7E1_2_3%7E%7EtestDomain2%7E4_5_6"
)

),
Expand All @@ -346,50 +345,31 @@ public static Stream<? extends Arguments> dsaRequestCases() {
ExtRegsDsa.of(
2, null, 3, null
),
Map.of(
"dsarequired", "2",
"dsadatatopub", "3"
List.of(
"dsarequired=2",
"dsadatatopub=3"
)
),
arguments(
"Incomplete transparency object is ignored",
ExtRegsDsa.of(
2, null, 3, List.of(ExtRegsDsaTransparency.of("domain", null))
),
List.of(
"dsarequired=2",
"dsadatatopub=3"
)
)
);
}

@ParameterizedTest(name = "{index} - {0}")
@MethodSource("dsaRequestCases")
public void dsaIsForwarded(String testName, ExtRegsDsa dsa, Map<String, String> expectations)
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
public void dsaParamsAreSentInRequest(String testName, ExtRegsDsa dsa, List<String> expectations) {
//given
final var regs = Regs.builder()
.ext(ExtRegs.of(null, null, null, dsa))
.build();

final var bidRequest = BidRequest.builder()
.regs(regs)
.build();

final var getDsaRequestParams =
YieldlabBidder.class.getDeclaredMethod("extractDsaRequestParams", BidRequest.class);
getDsaRequestParams.setAccessible(true);

//when
final var dsaRequestParams = (HashMap<String, String>) getDsaRequestParams.invoke(null, bidRequest);

//then
assertThat(dsaRequestParams).containsExactlyInAnyOrderEntriesOf(expectations);
}

@Test
public void dsaParamsAreSentInRequest() {
//given
final var transparencies = List.of(
ExtRegsDsaTransparency.of("testDomain", List.of(1, 2, 3)),
ExtRegsDsaTransparency.of("testDomain2", List.of(4, 5, 6))
);
final var dsa = ExtRegsDsa.of(
1, 2, 3, transparencies);
final var regs = Regs.builder()
.ext(ExtRegs.of(null, null, null, dsa))
.build();
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(Imp.builder()
.ext(mapper.valueToTree(ExtPrebid.of(null,
Expand All @@ -404,13 +384,9 @@ public void dsaParamsAreSentInRequest() {
final var result = target.makeHttpRequests(bidRequest);

//then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue().get(0).getUri())
.contains(
"&dsarequired=1",
"&dsapubrender=2",
"&dsadatatopub=3",
"&dsatransparency=testDomain%7E1_2_3%7E%7EtestDomain2%7E4_5_6"
);
.contains(expectations);
}

@Test
Expand Down

0 comments on commit f381f45

Please sign in to comment.