Skip to content
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reorder the methods so that they are placed in the order they are called

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied both changes

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.prebid.server.util.BidderUtil;
import org.prebid.server.util.HttpUtil;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
Expand All @@ -45,64 +46,62 @@ public MobilefuseBidder(String endpointUrl, JacksonMapper mapper) {

@Override
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) {
final String endpoint = request.getImp().stream()
.map(this::parseImpExt)
.filter(Objects::nonNull)
.findFirst()
.map(this::makeUrl)
.orElse(null);

if (endpoint == null) {
return Result.withError(BidderError.badInput("Invalid ExtImpMobilefuse value"));
final List<Imp> modifiedImps = new ArrayList<>();
final List<BidderError> errors = new ArrayList<>();

for (Imp imp : request.getImp()) {
try {
if (!isValidImp(imp)) {
continue;
}

final ExtImpMobilefuse extImp = parseImpExt(imp);
modifiedImps.add(modifyImp(imp, extImp));
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
}
}

final List<Imp> modifiedImps = request.getImp().stream()
.map(this::modifyImp)
.filter(Objects::nonNull)
.toList();
if (!errors.isEmpty()) {
return Result.withErrors(errors);
}

if (modifiedImps.isEmpty()) {
return Result.withError(BidderError.badInput("No valid imps"));
}

final BidRequest modifiedRequest = request.toBuilder().imp(modifiedImps).build();
return Result.withValue(BidderUtil.defaultRequest(modifiedRequest, endpoint, mapper));
return Result.withValue(BidderUtil.defaultRequest(modifiedRequest, endpointUrl, mapper));
}

private Imp modifyImp(Imp imp) {
if (imp.getBanner() == null && imp.getVideo() == null && imp.getXNative() == null) {
return null;
}
private static boolean isValidImp(Imp imp) {
return imp.getBanner() != null || imp.getVideo() != null || imp.getXNative() != null;
}

final ExtImpMobilefuse impExt = parseImpExt(imp);
private Imp modifyImp(Imp imp, ExtImpMobilefuse extImp) {
final ObjectNode skadn = parseSkadn(imp.getExt());
return imp.toBuilder()
.tagid(Objects.toString(impExt != null ? impExt.getPlacementId() : null, "0"))
.tagid(Objects.toString(extImp.getPlacementId(), "0"))
.ext(skadn != null ? mapper.mapper().createObjectNode().set(SKADN_PROPERTY_NAME, skadn) : null)
.build();
}

private ExtImpMobilefuse parseImpExt(Imp imp) {
private ExtImpMobilefuse parseImpExt(Imp imp) throws PreBidException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

try {
return mapper.mapper().convertValue(imp.getExt(), MOBILEFUSE_EXT_TYPE_REFERENCE).getBidder();
} catch (IllegalArgumentException e) {
return null;
throw new PreBidException("Error parsing ExtImpMobilefuse value: %s".formatted(e.getMessage()));
}
}

private ObjectNode parseSkadn(ObjectNode impExt) {
private ObjectNode parseSkadn(ObjectNode impExt) throws PreBidException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for throws

try {
return mapper.mapper().convertValue(impExt.get(SKADN_PROPERTY_NAME), ObjectNode.class);
} catch (IllegalArgumentException e) {
return null;
throw new PreBidException(e.getMessage());
}
}

private String makeUrl(ExtImpMobilefuse extImp) {
final String baseUrl = endpointUrl + Objects.toString(extImp.getPublisherId(), "0");
return "ext".equals(extImp.getTagidSrc()) ? baseUrl + "&tagid_src=ext" : baseUrl;
}

@Override
public final Result<List<BidderBid>> makeBids(BidderCall<BidRequest> httpCall, BidRequest bidRequest) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,4 @@ public class ExtImpMobilefuse {

@JsonProperty("placement_id")
Integer placementId;

@JsonProperty("pub_id")
Integer publisherId;

@JsonProperty("tagid_src")
String tagidSrc;
}
2 changes: 1 addition & 1 deletion src/main/resources/bidder-config/mobilefuse.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
adapters:
mobilefuse:
endpoint: http://mfx.mobilefuse.com/openrtb?pub_id=
endpoint: http://mfx.mobilefuse.com/openrtb
ortb-version: "2.6"
endpoint-compression: gzip
# This bidder does not operate globally. Please consider setting "disabled: true" outside of the following regions:
Expand Down
11 changes: 1 addition & 10 deletions src/main/resources/static/bidder-params/mobilefuse.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,9 @@
"placement_id": {
"type": "integer",
"description": "An ID which identifies this specific inventory placement"
},
"pub_id": {
"type": "integer",
"description": "An ID which identifies the publisher selling the inventory."
},
"tagid_src": {
"type": "string",
"description": "ext if passing publisher's ids, empty if passing MobileFuse IDs in placement_id field. Defaults to empty"
}
},
"required": [
"placement_id",
"pub_id"
"placement_id"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@

public class MobilefuseBidderTest extends VertxTest {

private static final String ENDPOINT_URL = "https://test.endpoint.com/openrtb?pub_id=";
private static final String ENDPOINT_URL = "https://test.endpoint.com/openrtb";

private final MobilefuseBidder target = new MobilefuseBidder(ENDPOINT_URL, jacksonMapper);

private static Imp givenImp(Function<Imp.ImpBuilder, Imp.ImpBuilder> impCustomizer) {
return impCustomizer.apply(Imp.builder()
.id("imp_id")
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpMobilefuse.of(1, 2, "tagidSrc")))))
ExtImpMobilefuse.of(1)))))
.build();
}

Expand Down Expand Up @@ -84,15 +84,18 @@ public void makeHttpRequestsShouldReturnErrorIfNoValidExtFound() {
// given
final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder
.id("456")
.banner(null)
.banner(Banner.builder().build())
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode()))));

// when
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors())
.containsExactly(BidderError.badInput("Invalid ExtImpMobilefuse value"));
assertThat(result.getErrors()).hasSize(1)
.allSatisfy(error -> {
assertThat(error.getMessage()).startsWith("Error parsing ExtImpMobilefuse value:");
assertThat(error.getType()).isEqualTo(BidderError.Type.bad_input);
});
assertThat(result.getValue()).isEmpty();
}

Expand All @@ -102,44 +105,6 @@ public void creationShouldFailOnInvalidEndpointUrl() {
.isThrownBy(() -> new MobilefuseBidder("invalid_url", jacksonMapper));
}

@Test
public void makeHttpRequestsShouldCreateCorrectURLWhenTagidSrcEqualsExt() {
// given
final BidRequest bidRequest = givenBidRequest(
impBuilder -> impBuilder
.banner(Banner.builder().build())
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpMobilefuse.of(1, 2, "ext")))));

// when
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getUri)
.containsExactly("https://test.endpoint.com/openrtb?pub_id=2&tagid_src=ext");
}

@Test
public void makeHttpRequestsShouldSetPubIdToZeroIfPublisherIdNotPresentInRequest() {
// given
final BidRequest bidRequest = givenBidRequest(
impBuilder -> impBuilder
.banner(Banner.builder().build())
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpMobilefuse.of(1, null, null)))));

// when
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getUri)
.containsExactly("https://test.endpoint.com/openrtb?pub_id=0");
}

@Test
public void makeHttpRequestsShouldReturnErrorIfNoValidImpsFound() {
// given
Expand Down Expand Up @@ -193,7 +158,7 @@ public void makeHttpRequestsShouldModifyImpTagId() {
.banner(Banner.builder().build())
.tagid("some tag id")
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpMobilefuse.of(1, 2, "ext")))));
ExtImpMobilefuse.of(1)))));

// when
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
Expand All @@ -212,7 +177,7 @@ public void makeHttpRequestsShouldModifyImpWithAddingSkadnWhenSkadnIsPresent() {
// given
final ObjectNode skadn = mapper.createObjectNode().put("something", "something");
final ObjectNode impExt = mapper.createObjectNode();
impExt.set("bidder", mapper.valueToTree(ExtImpMobilefuse.of(1, 2, "ext")));
impExt.set("bidder", mapper.valueToTree(ExtImpMobilefuse.of(1)));
impExt.set("skadn", skadn);
final BidRequest bidRequest = givenBidRequest(
impBuilder -> impBuilder.banner(Banner.builder().build()).ext(impExt));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/prebid/server/it/MobilefuseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class MobilefuseTest extends IntegrationTest {
@Test
public void openrtb2AuctionShouldRespondWithBidsFromMobilefuse() throws IOException, JSONException {
// given
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/mobilefuse-exchange/1111&tagid_src=ext"))
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/mobilefuse-exchange/"))
.withRequestBody(equalToJson(jsonFrom("openrtb2/mobilefuse/test-mobilefuse-bid-request.json")))
.willReturn(aResponse().withBody(jsonFrom("openrtb2/mobilefuse/test-mobilefuse-bid-response.json"))));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
"tagid": "tag_id",
"ext": {
"mobilefuse": {
"placement_id": 999999,
"pub_id": 1111,
"tagid_src": "ext"
"placement_id": 999999
}
}
}
Expand Down
Loading