Loyal: New Adapter (#3140)

[przemkaczmarek]

No description provided.

[SerhiiNahornyi]

Java Ci failed because of checkstyle issues, please fix them

[AntoxaAntoxic]

Hi @przemkaczmarek

Could you fix the comments are left?

[przemkaczmarek]

Yesturday I pushed changes. whitch comments are left? (I think I prepared everythink)

[przemkaczmarek]

@SerhiiNahornyi Are we rdy to merge?

[AntoxaAntoxic]

My bad, I didn't pay attention to the Go version of the bidder

fyi @SerhiiNahornyi

[przemkaczmarek]

@AntoxaAntoxic U can check, I have hope that everthink is fine now.

[AntoxaAntoxic]

the type is missing, isn't the test failing?

[przemkaczmarek]

yes and this same is in Go. Its Request not expected response.

[AntoxaAntoxic]

no sure we need this check, one of those field are required by the json scheme

[przemkaczmarek]

You said:
AntoxaAntoxic 2 weeks ago
the following logic is missing

if len(adapterRequests) == 0 {
	errs = append(errs, errors.New("found no valid impressions"))
	return nil, errs
}

and this is part of this

[AntoxaAntoxic]

and you've implemented it already a few lines below

        if (httpRequests.isEmpty()) {
            return Result.withError(BidderError.badInput("found no valid impressions"));
        }

[przemkaczmarek]

when I have deleted it "invalidImp goes to the results". We have to seperate invalidImp

[AntoxaAntoxic]

so this is the Go code

if loyalExt.PlacementID != "" {
			impExt.LoyalBidderExt.PlacementID = loyalExt.PlacementID
			impExt.LoyalBidderExt.Type = "publisher"
		} else if loyalExt.EndpointID != "" {
			impExt.LoyalBidderExt.EndpointID = loyalExt.EndpointID
			impExt.LoyalBidderExt.Type = "network"
		}

as you see the imp that doesn't have the placementId and endpointId fields won't be treated as invalid, but you've added the unit test that defines invalid bid as the following:

private static Imp givenBadImp(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
        return impCustomizer.apply(Imp.builder()
                        .id("invalidImp")
                        .ext(mapper.valueToTree(ExtPrebid.of(null,
                                ExtImpLoyal.of(null, null)))))
                .build();
    }

So my conclusion the logic of your version of the bidder differs from the GO's version

[przemkaczmarek]

okey, now its clear. I hade made wrong badImp, I was doing it in wrong way.

[AntoxaAntoxic]

why do you need two different methods if there is a customizer that can be used for that matter?

[AntoxaAntoxic]

so this is the Go code

if loyalExt.PlacementID != "" {
			impExt.LoyalBidderExt.PlacementID = loyalExt.PlacementID
			impExt.LoyalBidderExt.Type = "publisher"
		} else if loyalExt.EndpointID != "" {
			impExt.LoyalBidderExt.EndpointID = loyalExt.EndpointID
			impExt.LoyalBidderExt.Type = "network"
		}

as you see the imp that doesn't have the placementId and endpointId fields won't be treated as invalid, but you've added the unit test that defines invalid bid as the following:

private static Imp givenBadImp(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
        return impCustomizer.apply(Imp.builder()
                        .id("invalidImp")
                        .ext(mapper.valueToTree(ExtPrebid.of(null,
                                ExtImpLoyal.of(null, null)))))
                .build();
    }

So my conclusion the logic of your version of the bidder differs from the GO's version