Skip to content

Conversation

@karimMourra
Copy link
Contributor

🔧 Type of changes

  • bid adapter update

✨ What's the context?

Addresses #3834
Ports over the following changes from the Go repo prebid/prebid-server#4219
The change consists of appending the Data Center query param to the endpoint url based on the user ID to reduce latency.

🧠 Rationale behind the change

Product requirements from JWPConnatix

🧪 Test plan

Unit tests have been added

🏎 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?

@karimMourra karimMourra changed the title Advs 1315 Port PR from PBS-Go: Connatix: Enhance endpoint with DC Mar 28, 2025
@Net-burst Net-burst requested a review from AntoxaAntoxic March 28, 2025 21:10
Comment on lines 128 to 140
private String getUserId(BidRequest request) {
final User user = request.getUser();
if (user == null) {
return null;
}

final String buyerUid = user.getBuyeruid();
if (buyerUid == null || buyerUid.isEmpty()) {
return null;
}

return buyerUid.trim();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

    private Optional<String> getUserId(BidRequest request) {
        return Optional.ofNullable(request.getUser())
                .map(User::getBuyeruid)
                .filter(StringUtils::isNotBlank)
                .map(String::trim);
    }

Comment on lines 109 to 126
private String getOptimalEndpointUrl(BidRequest request) {
final String userId = getUserId(request);
if (userId == null) {
return endpointUrl;
}

final String dataCenterCode = getDataCenterCode(userId);
if (dataCenterCode == null) {
return endpointUrl;
}

try {
final URIBuilder uriBuilder = new URIBuilder(endpointUrl);
return uriBuilder.addParameter("dc", dataCenterCode).build().toString();
} catch (URISyntaxException e) {
throw new PreBidException(e.getMessage());
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

    private String getOptimalEndpointUrl(BidRequest request) {
        try {
            final URIBuilder uriBuilder = new URIBuilder(endpointUrl);
            return getUserId(request)
                    .map(this::getDataCenterCode)
                    .map(code -> uriBuilder.addParameter("dc", code))
                    .orElse(uriBuilder)
                    .build()
                    .toString();
        } catch (URISyntaxException e) {
            throw new PreBidException(e.getMessage());
        }
    }

Comment on lines 86 to 87
errors.add(BidderError.badInput(e.getMessage()));
return Result.withErrors(errors);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Result.withError(...) and create the error list later

@karimMourra
Copy link
Contributor Author

@AntoxaAntoxic thank you for the review! I applied your suggestions

Copy link
Collaborator

@AntoxaAntoxic AntoxaAntoxic left a comment

Choose a reason for hiding this comment

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

please remove all added test JSON files

return Result.withError(BidderError.badInput("Device IP is required"));
}

final List<BidderError> errors = new ArrayList<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

please move creating an error list as closer as possible to the place it's used

Comment on lines 218 to 224
private HttpRequest<BidRequest> makeHttpRequest(BidRequest request, Imp imp, MultiMap headers,
String optimalEndpointUrl) {
final BidRequest outgoingRequest = request.toBuilder()
.imp(List.of(imp))
.build();

return BidderUtil.defaultRequest(outgoingRequest, headers, endpointUrl, mapper);
return BidderUtil.defaultRequest(outgoingRequest, headers, optimalEndpointUrl, mapper);
Copy link
Collaborator

Choose a reason for hiding this comment

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

incorrect styling

    private HttpRequest<BidRequest> makeHttpRequest(BidRequest request,
                                                    Imp imp,
                                                    MultiMap headers,
                                                    String optimalEndpointUrl) {

        final BidRequest outgoingRequest = request.toBuilder().imp(List.of(imp)).build();
        return BidderUtil.defaultRequest(outgoingRequest, headers, optimalEndpointUrl, mapper);
    }

}
}

private Optional<String> getUserId(BidRequest request) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

static

.map(String::trim);
}

private String getDataCenterCode(String usedId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

static

Comment on lines 286 to 288
httpRequests.forEach(request -> {
assertThat(request.getUri().contains("dc=us-east-2"));
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

please check other occurrences and fix them as well

assertThat(result.getValue).extracting(HttpRequest::getUri).containsOnly("dc=us-east-2");

@karimMourra
Copy link
Contributor Author

@AntoxaAntoxic thanks for the review! all feedback addressed

AntoxaAntoxic
AntoxaAntoxic previously approved these changes Apr 14, 2025
@CTMBNara CTMBNara changed the title Port PR from PBS-Go: Connatix: Enhance endpoint with DC Connatix: Enhance endpoint with DC Apr 23, 2025
Comment on lines 107 to 119
private String getOptimalEndpointUrl(BidRequest request) {
try {
final URIBuilder uriBuilder = new URIBuilder(endpointUrl);
return getUserId(request)
.map(userId -> getDataCenterCode(userId))
.map(dataCenterCode -> uriBuilder.addParameter("dc", dataCenterCode))
.orElse(uriBuilder)
.build()
.toString();
} catch (URISyntaxException e) {
throw new PreBidException(e.getMessage());
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

    private String getOptimalEndpointUrl(BidRequest request) {
        final Optional<String> dataCenterCode = getUserId(request)
                .map(ConnatixBidder::getDataCenterCode);

        if (dataCenterCode.isEmpty()) {
            return endpointUrl;
        }

        try {
            return new URIBuilder(endpointUrl)
                    .addParameter("dc", dataCenterCode.get())
                    .build()
                    .toString();
        } catch (URISyntaxException e) {
            throw new PreBidException(e.getMessage());
        }
    }

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that @AntoxaAntoxic asked you to change this piece of code. It seems a bit confusing to me (due to .map(<toBuilder>)), let's go back to the old version.

Please make this small change and we will merge the pr as soon as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CTMBNara addressed!

@osulzhenko osulzhenko requested a review from CTMBNara April 25, 2025 09:45
@CTMBNara CTMBNara merged commit c11f026 into prebid:master Apr 25, 2025
8 checks passed
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: Connatix: Enhance endpoint with DC

5 participants