Updating admixer bidder #1401

javaAndScriptDeveloper · 2021-08-04T10:39:39Z

Updated admixer due to this:
prebid/prebid-server#1872

Plz take a look at AdmixerBidder.java / line 109

fixed

fixed status habdler

rpanchyk · 2021-08-12T08:00:26Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

+        final BigDecimal customFloor = extImpAdmixer.getCustomFloor();
+        final BigDecimal impFloor = isValidBidFloor(imp.getBidfloor()) ? imp.getBidfloor() : null;
+        final BigDecimal bidfloor = isValidBidFloor(customFloor) && !isValidBidFloor(impFloor)
+                ? customFloor
+                : impFloor;


Extract to separate method.

Why you're checking imp.getBidfloor() two times?

src/test/java/org/prebid/server/bidder/admixer/AdmixerBidderTest.java

rpanchyk · 2021-08-12T08:09:14Z

src/test/java/org/prebid/server/bidder/admixer/AdmixerBidderTest.java

@@ -303,6 +267,91 @@ public void makeBidsShouldReturnBannerBidIfAudioIsPresentInRequestImp() throws J
                .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), audio, "USD"));
    }

+    @Test
+    public void shouldReturnNullIfBidFloorNullCustomFloorNull() {


Method you're testing returns Result<List<HttpRequest<BidRequest>>>. It doesn't returns null.
Rename to something like makeHttpRequestsShouldReturnBidFloorAs... to be in code style with other tests.
PS. Change all other test names below.

Maybe name it as makeHttpRequestsShouldOmitBidFloor...?

rpanchyk · 2021-08-12T08:29:22Z

src/test/java/org/prebid/server/bidder/admixer/AdmixerBidderTest.java

+    }
+
+    //method where zoneId cut from ext and passed to tagId field
+    private static Imp givenImpWithParsedTagID(UnaryOperator<Imp.ImpBuilder> impCustomizer) {


This method almost duplicates the above.
Can we call givenImp(...) here instead?

The difference is that in givenImp tagId is located as a part of json in ext field, in givenImpWithParsedTagID tagId is cropped out of json and assigned to field tagId; givenImpWithParsedTagID is quite useful to compare imps after request(during request tagId is cropped and assigned)

ok, but you can call givenImp(...) inside of givenImpWithParsedTagID(...).
So, you'll rid off the code duplication.

rpanchyk · 2021-08-13T12:06:01Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

                .ext(makeImpExt(extImpAdmixer.getCustomParams()))
                .build();
    }

+    private BigDecimal calculateBifLoor(BigDecimal customFloor, BigDecimal bidFloor) {


Suggestion:

resolveBidFloor(BigDecimal impBidFloor, BigDecimal customBidFloor)`

rpanchyk · 2021-08-13T12:06:36Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

                .ext(makeImpExt(extImpAdmixer.getCustomParams()))
                .build();
    }

+    private BigDecimal calculateBifLoor(BigDecimal customFloor, BigDecimal bidFloor) {
+        customFloor = isValidBidFloor(customFloor) ? customFloor : null;


Create new final variable resolvedCustomBidFloor. Do not override input args.

rpanchyk · 2021-08-13T12:10:19Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

+    private BigDecimal calculateBifLoor(BigDecimal customFloor, BigDecimal bidFloor) {
+        customFloor = isValidBidFloor(customFloor) ? customFloor : null;
+
+        return isValidBidFloor(bidFloor) ? bidFloor : customFloor;


Suggestion:

final BigDecimal resolvedBidFloor = isValidBidFloor(impBidFloor) ? impBidFloor : null; return ObjectUtils.defaultIfNull(resolvedBidFloor, resolvedCustomBidFloor);

have used isValidBidFloor because it checks whether decimal is zero or less than zero, not only null;

rpanchyk · 2021-08-13T12:10:37Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

+
+    }
+
+    private static boolean isValidBidFloor(BigDecimal impFloor) {


Rename impFloor to bidFloor.

rpanchyk · 2021-08-13T12:12:14Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

            throw new PreBidException("ZoneId must be UUID/GUID");
        }

        return extImpAdmixer;
    }

    private Imp processImp(Imp imp, ExtImpAdmixer extImpAdmixer) {
-        final Double extImpFloor = extImpAdmixer.getCustomFloor();
-        final BigDecimal customFloor = extImpFloor != null ? BigDecimal.valueOf(extImpFloor) : BigDecimal.ZERO;
+        final BigDecimal bidFloor = calculateBifLoor(extImpAdmixer.getCustomFloor(), imp.getBidfloor());


Inline this call. No need for separate variable since it is quite clear and doesn't required to use twice.

rpanchyk · 2021-08-13T12:12:45Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

@@ -85,23 +85,34 @@ private ExtImpAdmixer parseImpExt(Imp imp) {
        } catch (IllegalArgumentException e) {
            throw new PreBidException(String.format("Wrong Admixer bidder ext in imp with id : %s", imp.getId()));
        }
-        if (StringUtils.length(extImpAdmixer.getZone()) != 36) {
+        if (StringUtils.length(extImpAdmixer.getZone()) < 32 || StringUtils.length(extImpAdmixer.getZone()) > 36) {


Extract extImpAdmixer.getZone() to separate variable.

rpanchyk · 2021-08-13T12:13:44Z

src/test/java/org/prebid/server/bidder/admixer/AdmixerBidderTest.java

@@ -218,7 +182,7 @@ public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingExcept
        // when
        final Result<List<BidderBid>> result = admixerBidder.makeBids(httpCall,
                BidRequest.builder()
-                        .imp(singletonList(Imp.builder().id("123").build()))
+                        .imp(singletonList(givenImp(builder -> builder)))


Use indentity() instead.

And1sS · 2021-09-09T13:45:17Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

@@ -85,23 +86,34 @@ private ExtImpAdmixer parseImpExt(Imp imp) {
        } catch (IllegalArgumentException e) {
            throw new PreBidException(String.format("Wrong Admixer bidder ext in imp with id : %s", imp.getId()));
        }
-        if (StringUtils.length(extImpAdmixer.getZone()) != 36) {
+        String zoneId = extImpAdmixer.getZone();


Can be final

And1sS · 2021-09-09T13:49:23Z

src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java

                .ext(makeImpExt(extImpAdmixer.getCustomParams()))
                .build();
    }

+    private BigDecimal resolveBidFloor(BigDecimal customBidFloor, BigDecimal bidFloor) {


Can be static

javaAndScriptDeveloper added 6 commits August 2, 2021 16:38

Adding headers to vtrack and status

4a47351

This commit added headers to /vtrack and /status

375223b

Working

d65aeba

Continue working progress

1a93c52

Updated tests and moved adding headers to lambdas

3ce34b5

Updated admixer bidder and admixer.json

3236a95

javaAndScriptDeveloper requested a review from SerhiiNahornyi August 4, 2021 10:39

javaAndScriptDeveloper and others added 8 commits August 5, 2021 12:08

microfix

62a5aec

added 2 tests

deeee79

Fixed and added new tests

f02b043

Cropped unnecessary

d04a855

Merge branch 'master' into updatingAdmixerBidder

dc3c31a

Update VtrackHandlerTest.java

457a004

fixed

Update StatusHandlerTest.java

094401a

fixed status habdler

Update StatusHandlerTest.java

3f7c8a6

rpanchyk suggested changes Aug 12, 2021

View reviewed changes

Fixed

5ace28c

rpanchyk suggested changes Aug 13, 2021

View reviewed changes

javaAndScriptDeveloper added 2 commits August 13, 2021 16:41

fixed

fc4f9e3

fixed resolveBidFloor method

5d3aa28

And1sS requested changes Sep 9, 2021

View reviewed changes

minor fixes

b94cf67

And1sS approved these changes Sep 21, 2021

View reviewed changes

Merge branch 'master' into updatingAdmixerBidder

9f14274

rpanchyk approved these changes Sep 24, 2021

View reviewed changes

rpanchyk merged commit b36c977 into master Sep 24, 2021

rpanchyk deleted the updatingAdmixerBidder branch September 24, 2021 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating admixer bidder #1401

Updating admixer bidder #1401

javaAndScriptDeveloper commented Aug 4, 2021

rpanchyk Aug 12, 2021

rpanchyk Aug 12, 2021

And1sS Sep 9, 2021

rpanchyk Aug 12, 2021

javaAndScriptDeveloper Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

javaAndScriptDeveloper Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

rpanchyk Aug 13, 2021

And1sS Sep 9, 2021

And1sS Sep 9, 2021


		}

		private static boolean isValidBidFloor(BigDecimal impFloor) {

Updating admixer bidder #1401

Updating admixer bidder #1401

Conversation

javaAndScriptDeveloper commented Aug 4, 2021

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