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 @@ -99,8 +99,7 @@ private ObjectNode parseSkadn(ObjectNode impExt) {
}

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

Choose a reason for hiding this comment

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

looks like you don't the following part of code as well

        final String endpoint = request.getImp().stream()
                .map(this::parseImpExt)
                .filter(Objects::nonNull)
                .findFirst()
                .map(this::makeUrl)
                .orElse(null);

please clean up redundant code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out! I've removed the redundant code as suggested.

To ensure stability after removing the .map(this::parseImpExt).filter(Objects::nonNull) chain, I added a null check in the modifyImp(Imp imp) method. This prevents a potential NullPointerException in cases where parseImpExt could return null.

Let me know if there's anything else you'd like me to update!

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,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 @@ -102,44 +102,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 +155,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 +174,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