Kobler: New adapter (#3667) #3684

przemkaczmarek · 2025-01-17T13:37:52Z

🔧 Type of changes

new bid adapter

✨ What's the context?

#3667

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic · 2025-01-24T11:32:17Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        return bidsFromResponse(bidResponse, errors);
+    }
+
+    private List<BidderBid> bidsFromResponse(BidResponse bidResponse, List<BidderError> errors) {


the errors list is redundant

the errors list is still redundant

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

src/main/resources/bidder-config/kobler.yaml

AntoxaAntoxic · 2025-01-24T11:37:24Z

src/main/java/org/prebid/server/spring/config/bidder/KoblerConfiguration.java

+    BidderDeps koblerBidderDeps(BidderConfigurationProperties koblerConfigurationProperties,
+                                 CurrencyConversionService currencyConversionService,
+                                 @NotBlank @Value("${external-url}") String externalUrl,
+                                 JacksonMapper mapper) {


indentation is bad

I used a checkstyle and it didnt show mistakes. How it should looks like?

as least the method parameter should be aligned

it's still not aligned)

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic

Ping me when you finish addressing the comments, probably this time I commented too early

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic · 2025-01-30T13:19:03Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+            return Result.withErrors(errors);
+        }
+
+        modifiedRequest = modifiedRequest.toBuilder().imp(modifiedImps).build();


please modify the request once

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic · 2025-01-30T13:21:21Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        return imp;
+    }
+
+    public boolean extractTestMode(Imp imp) {


why public?

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic · 2025-01-30T13:27:49Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        try {
+            final ExtPrebid<?, ExtImpKobler> extPrebid = mapper.mapper().convertValue(imp.getExt(),
+                    KOBLER_EXT_TYPE_REFERENCE);
+            final ExtImpKobler extImpKobler = extPrebid != null ? extPrebid.getBidder() : null;


parse the imp.ext in the same way you did before, please make it in the same style other bidders do

I changed the code. My question is:
-Can we receive an empty ext in the request? If so, the parseImpExt method will return null. shuould I handle NPE? impExtKobler field is not rerequired.

in order to reach the bidder you have to declare it as the imp.ext.prebid.BIDDER, otherwise the PBS doesn't know what to call, so this object will be always present, but it can be empty

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

AntoxaAntoxic · 2025-02-07T14:13:24Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        final List<Imp> imps = bidRequest.getImp();
+        if (!imps.isEmpty()) {
+            try {
+                testMode = parseImpExt(imps.get(0)).getTest();


imps.getFirst()

AntoxaAntoxic · 2025-02-07T14:17:52Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        try {
+            final HttpRequest<BidRequest> httpRequest = BidderUtil.defaultRequest(modifiedRequest, endpoint, mapper);
+            return Result.of(Collections.singletonList(httpRequest), errors);
+        } catch (EncodeException e) {
+            errors.add(BidderError.badInput("Failed to encode request: " + e.getMessage()));
+            return Result.withErrors(errors);
+        }


this try-catch is redundant

AntoxaAntoxic · 2025-02-07T14:23:15Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+                final Imp processedImp = processImp(bidRequest, imp);
+                modifiedImps.add(processedImp);
+            } catch (PreBidException e) {
+                errors.add(BidderError.badInput(e.getMessage()));


as I can see, in case of any imp processing error it's immediately should return the Result

any fix on that?

AntoxaAntoxic · 2025-02-07T14:24:55Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+
+        for (Imp imp : imps) {
+            try {
+                final Imp processedImp = processImp(bidRequest, imp);


let's rename the method to modifyImp and make everything inline

modifiedImps.add(modifyImp(bidRequest, imp));

AntoxaAntoxic · 2025-02-07T14:26:24Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        if (modifiedImps.isEmpty()) {
+            errors.add(BidderError.badInput("No valid impressions"));
+            return Result.withErrors(errors);
+        }


this check is redundant because the process should stop on the very first imp modification failure

AntoxaAntoxic · 2025-02-07T14:30:01Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+    private BidType getBidType(Bid bid) {
+        if (bid.getExt() == null) {
+            return BidType.banner;
+        }
+
+        final ObjectNode prebidNode = (ObjectNode) bid.getExt().get(EXT_PREBID);
+        if (prebidNode == null) {
+            return BidType.banner;
+        }
+
+        final ExtBidPrebid extBidPrebid = parseExtBidPrebid(prebidNode);
+        if (extBidPrebid == null || extBidPrebid.getType() == null) {
+            return BidType.banner;
+        }
+
+        return extBidPrebid.getType(); // jeśli udało się sparsować
+    }
+
+    private ExtBidPrebid parseExtBidPrebid(ObjectNode prebid) {
+        try {
+            return mapper.mapper().treeToValue(prebid, ExtBidPrebid.class);
+        } catch (JsonProcessingException e) {
+            return null;
+        }
+    }
+}


private BidType getBidType(Bid bid) { return Optional.ofNullable(bid.getExt()) .map(ext -> ext.get(EXT_PREBID)) .map(ObjectNode.class::cast) .map(this::parseExtBidPrebid) .map(ExtBidPrebid::getType) .orElse(BidType.banner); } private ExtBidPrebid parseExtBidPrebid(ObjectNode prebid) { try { return mapper.mapper().treeToValue(prebid, ExtBidPrebid.class); } catch (JsonProcessingException e) { return null; } }

AntoxaAntoxic · 2025-02-07T14:30:25Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+                .filter(Objects::nonNull)
+                .flatMap(Collection::stream)
+                .map(bid -> BidderBid.of(bid, getBidType(bid), bidResponse.getCur()))
+                .filter(Objects::nonNull)


this filter is redundant

AntoxaAntoxic · 2025-02-07T14:32:27Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        return bidsFromResponse(bidResponse, errors);
+    }
+
+    private List<BidderBid> bidsFromResponse(BidResponse bidResponse, List<BidderError> errors) {


the errors list is still redundant

AntoxaAntoxic · 2025-02-07T14:33:27Z

src/main/java/org/prebid/server/spring/config/bidder/KoblerConfiguration.java

+    BidderDeps koblerBidderDeps(BidderConfigurationProperties koblerConfigurationProperties,
+                                 CurrencyConversionService currencyConversionService,
+                                 @NotBlank @Value("${external-url}") String externalUrl,
+                                 JacksonMapper mapper) {


as least the method parameter should be aligned

AntoxaAntoxic · 2025-02-10T14:22:11Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+        try {
+            final List<BidderError> errors = new ArrayList<>();
+            final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class);
+            return Result.of(extractBids(bidResponse), errors);


Result.withValues(...) + get rid of redundant errors

AntoxaAntoxic · 2025-02-10T14:23:46Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

+                final Imp processedImp = processImp(bidRequest, imp);
+                modifiedImps.add(processedImp);
+            } catch (PreBidException e) {
+                errors.add(BidderError.badInput(e.getMessage()));


any fix on that?

AntoxaAntoxic · 2025-02-10T14:25:53Z

src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java

-                .filter(Objects::nonNull)
                .map(SeatBid::getBid)
-                .filter(Objects::nonNull)
                .flatMap(Collection::stream)
                .map(bid -> BidderBid.of(bid, getBidType(bid), bidResponse.getCur()))
-                .filter(Objects::nonNull)


please, return all the filters back except for the last one

AntoxaAntoxic · 2025-02-10T14:27:27Z

src/main/java/org/prebid/server/spring/config/bidder/KoblerConfiguration.java

+    BidderDeps koblerBidderDeps(BidderConfigurationProperties koblerConfigurationProperties,
+                                 CurrencyConversionService currencyConversionService,
+                                 @NotBlank @Value("${external-url}") String externalUrl,
+                                 JacksonMapper mapper) {


it's still not aligned)

AntoxaAntoxic · 2025-02-10T14:49:07Z

src/test/java/org/prebid/server/bidder/kobler/KoblerBidderTest.java

+
+    @Test
+    public void makeHttpRequestsShouldReturnErrorIfNoValidImps() {
+        // Given


please use lower-cased given-when-then, it's a nitpick, but it's worth preserving the common style

AntoxaAntoxic · 2025-02-10T14:50:21Z

src/test/java/org/prebid/server/bidder/kobler/KoblerBidderTest.java

+    @Test
+    public void makeBidsShouldReturnEmptyListWhenBidResponseIsNull() {
+        // Given
+        final BidderCall<BidRequest> httpCall = givenHttpCallWithBody("null");
+
+        // When
+        final Result<List<BidderBid>> result = target.makeBids(httpCall, BidRequest.builder().build());
+
+        // Then
+        assertThat(result.getValue()).isEmpty();
+        assertThat(result.getErrors()).isEmpty();
+    }
+
+    @Test
+    public void makeBidsShouldReturnEmptyListWhenSeatbidIsEmpty() throws JsonProcessingException {
+        // Given
+        final BidResponse bidResponse = BidResponse.builder()
+                .seatbid(Collections.emptyList())
+                .build();
+        final BidderCall<BidRequest> httpCall = givenHttpCall(bidResponse);
+
+        // When
+        final Result<List<BidderBid>> result = target.makeBids(httpCall, BidRequest.builder().build());
+
+        // Then
+        assertThat(result.getValue()).isEmpty();
+        assertThat(result.getErrors()).isEmpty();
+    }


please do not mix up makeBids and makeHttpRequests test cases

przemkaczmarek added 6 commits January 17, 2025 14:35

Kobler: New adapter (#3667)

8ea1419

fix clean code

3e4f05e

Merge branch 'master' into Kobler-New-Adapter-(#3667)

858b5df

fix bugs

c3e11ce

fix bugs

5ca450c

fix bugs

7293b44

Net-burst linked an issue Jan 21, 2025 that may be closed by this pull request

Port PR from PBS-Go: New Adapter: Kobler #3667

Open

AntoxaAntoxic requested changes Jan 24, 2025

View reviewed changes

osulzhenko added the do not port label Jan 29, 2025

przemkaczmarek added 3 commits January 30, 2025 10:51

fix comments

ac1eb72

fix checkstyle

9287f80

fix yaml

857a308

AntoxaAntoxic requested changes Jan 30, 2025

View reviewed changes

przemkaczmarek added 4 commits January 31, 2025 14:47

fix comments

c385084

fix checkstyle

3078b30

fix comments

d64f134

fix checkstyle

68770b0

osulzhenko requested a review from AntoxaAntoxic February 7, 2025 09:26

AntoxaAntoxic requested changes Feb 7, 2025

View reviewed changes

osulzhenko assigned przemkaczmarek Feb 7, 2025

fix comments

4982cf4

AntoxaAntoxic requested changes Feb 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kobler: New adapter (#3667) #3684

Kobler: New adapter (#3667) #3684

przemkaczmarek commented Jan 17, 2025

AntoxaAntoxic Jan 24, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Jan 24, 2025

przemkaczmarek Jan 24, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic left a comment

AntoxaAntoxic Jan 30, 2025

AntoxaAntoxic Jan 30, 2025

AntoxaAntoxic Jan 30, 2025

przemkaczmarek Jan 31, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 7, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 10, 2025

AntoxaAntoxic Feb 10, 2025

Kobler: New adapter (#3667) #3684

Are you sure you want to change the base?

Kobler: New adapter (#3667) #3684

Conversation

przemkaczmarek commented Jan 17, 2025

🔧 Type of changes

✨ What's the context?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AntoxaAntoxic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment