Skip to content

Conversation

QuentinGallard
Copy link
Contributor

🔧 Type of changes

  • bid adapter update

✨ What's the context?

SmileWanted adapter needs to support dynamic endpoint URLs based on the zoneId parameter to align with our server infrastructure requirements.

🧠 Rationale behind the change

The SmileWanted bidder endpoint needs to be constructed dynamically using the zoneId from the impression extension. This change:

  • Allows SmileWanted to identify which Prebid Server technology (Go or Java) is making the request, enabling us to provide optimized responses for each implementation
  • Enables more efficient filtering of ad calls by including the zoneId directly in the URL path, improving request routing and performance

🧪 Test plan

  • Updated unit tests to verify zoneId extraction and proper error handling
  • Updated integration tests to use the new endpoint pattern
  • Tests verify that missing or invalid zoneId returns appropriate error messages
  • Endpoint construction follows the pattern used by other adapters with dynamic parameters

🏎 Quality check

  • Are your changes following our code style guidelines?
  • Are there any breaking changes in your code?
  • Does your test coverage exceed 90%?
  • Are there any erroneous console logs, debuggers or leftover code in your changes?

@QuentinGallard QuentinGallard force-pushed the sw_java_endpoint branch 2 times, most recently from 772729c to 2ab6d26 Compare August 1, 2025 08:06
@osulzhenko
Copy link
Collaborator

@QuentinGallard Hi, is it still a draft?

@QuentinGallard QuentinGallard marked this pull request as ready for review August 19, 2025 12:23
@QuentinGallard
Copy link
Contributor Author

Hi @osulzhenko,
Sorry, I was on summer holiday.
I rebased and fixed my pull request.
It is now waiting for approval.

The same pull request exists on Prebid Server Go project : prebid/prebid-server#4468

adapters.smartyads.endpoint=http://localhost:8090/smartyads-exchange
adapters.smilewanted.enabled=true
adapters.smilewanted.endpoint=http://localhost:8090/smilewanted-exchange
adapters.smilewanted.endpoint=http://localhost:8090/smilewanted-exchange/java/
Copy link
Collaborator

Choose a reason for hiding this comment

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

this change is unnecessary

Comment on lines 96 to 93
public void makeHttpRequestsShouldReturnSingleRequest() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(givenImp("zone123")))
.build();

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would also assert the payload and impIds sent to the bidder, also I'd created a bid request with at least two imps

Comment on lines 49 to 57
@Test
public void makeHttpRequestsShouldCorrectlyAddHeaders() {
public void creationShouldFailOnNullEndpointUrl() {
assertThatNullPointerException().isThrownBy(() -> new SmileWantedBidder(null, jacksonMapper));
}

@Test
public void creationShouldFailOnNullMapper() {
assertThatNullPointerException().isThrownBy(() -> new SmileWantedBidder(ENDPOINT_URL, null));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

those are kind of redundant, can be removed

adapters:
smilewanted:
endpoint: https://prebid-server.smilewanted.com
endpoint: https://prebid-server.smilewanted.com/java/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add a {{ZoneId}} macro instead of just appending zone Id to the end of the endpoint, please consider that

Comment on lines 49 to 51
if (CollectionUtils.isEmpty(request.getImp())) {
return Result.withError(BidderError.badInput("No impressions in request"));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this check is redundant

Comment on lines 64 to 66
return Result.withValue(HttpRequest.<BidRequest>builder()
.method(HttpMethod.POST)
.uri(endpointUrl)
.uri(url)
.headers(createHeaders())
.payload(outgoingRequest)
.body(mapper.encodeToBytes(outgoingRequest))
.build());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please replace with the BidderUtil.defaultRequest()

BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").build()))
.build(),
mapper.writeValueAsString(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would hide mapper.writeValueAsString inside the givenHttpCall

Copy link
Collaborator

Choose a reason for hiding this comment

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

^^

@osulzhenko
Copy link
Collaborator

@QuentinGallard any update?

@QuentinGallard QuentinGallard force-pushed the sw_java_endpoint branch 2 times, most recently from 1f73f2f to e664dc5 Compare September 15, 2025 12:06
}

final BidRequest outgoingRequest = request.toBuilder().at(DEFAULT_AT).build();
final String url = endpointUrl.replace("{{ZoneId}}", extImpSmilewanted.getZoneId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

please extract {{ZoneId}} as a constant

Copy link
Collaborator

Choose a reason for hiding this comment

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

don't forget to add this macro to the URL in the smilewanted.json

BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").build()))
.build(),
mapper.writeValueAsString(
Copy link
Collaborator

Choose a reason for hiding this comment

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

^^

AntoxaAntoxic
AntoxaAntoxic previously approved these changes Sep 17, 2025
@QuentinGallard
Copy link
Contributor Author

Hi @AntoxaAntoxic , the test that failed does not appear to be related to my changes and seems unstable to me.

What should I do now to get this Pull Request approved?

Thanks a lot for your time :)

@AntoxaAntoxic
Copy link
Collaborator

Hi @QuentinGallard

Thank you for the great PR.

Yes, the failed test is not related to your changes.

The PR will be merged after the 2nd approval closer to the next release date.

Thanks!

Comment on lines 343 to 346
final String body = bidResponse != null
? mapper.writeValueAsString(bidResponse)
: mapper.writeValueAsString(null);
return givenHttpCall(bidRequest, body);
Copy link
Collaborator

Choose a reason for hiding this comment

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

return givenHttpCall(bidRequest, mapper.writeValueAsString(bidResponse));

}

final BidRequest outgoingRequest = request.toBuilder().at(DEFAULT_AT).build();
final String url = endpointUrl.replace(ZONE_ID_MACRO, extImpSmilewanted.getZoneId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

add url encoding for zoneId

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port PR from PBS-Go: SmileWanted: Append zoneId to endpoint path
4 participants