New Adapter: Compass

[CompassSSP]

doc - prebid/prebid.github.io#3441

[CompassSSP]

@guscarreon thanks for the comments, everything has been corrected

[guscarreon]

@CompassSSP thank you for adressing my feedback. Your code looks good to me and in this second pass I only let a couple of nitpicks. If you decide to leave them as is, I won't complain. Let me know your decision.

[guscarreon]

@CompassSSP thank you for addressing my feedback. This nitpick is purely aesthetic: the omitempty keyword could prove useful in avoiding the in-line declaration of the marshaled fields inside the two branches of the if/else statement. The structs that follow:

type reqBodyExt struct {
	CompassBidderExt reqBodyExtBidder `json:"bidder"`
}

type reqBodyExtBidder struct {
	Type        string `json:"type"`
	PlacementID string `json:"placementId,omitempty"`
	EndpointID  string `json:"endpointId,omitempty"`
}

would not include either PlacementID or EndpointID in the resulting JSON object if not defined. Therefore, we could simplify to only have one json.Marshal() call:

44  
45           finalyImpExt := reqCopy.Imp[0].Ext
   +         temp := reqBodyExt{CompassBidderExt: reqBodyExtBidder{}}
46           if compassExt.PlacementID != "" {
47 -             finalyImpExt, err = json.Marshal(map[string]interface{}{
48 -                 "bidder": map[string]string{
49 -                     "placementId": compassExt.PlacementID,
50 -                     "type":        "publisher",
51 -                 },
52 -             })
53 -             if err != nil {
54 -                 return nil, append(errs, err)
55 -             }
   +             temp.CompassBidderExt.PlacementID = compassExt.PlacementID
   +             temp.CompassBidderExt.Type = "publisher"
56           } else if compassExt.EndpointID != "" {
57 -             finalyImpExt, err = json.Marshal(map[string]interface{}{
58 -                 "bidder": map[string]string{
59 -                     "endpointId": compassExt.EndpointID,
60 -                     "type":       "network",
61 -                 },
62 -             })
63 -             if err != nil {
64 -                 return nil, append(errs, err)
65 -             }
   +             temp.CompassBidderExt.EndpointID = compassExt.EndpointID
   +             temp.CompassBidderExt.Type = "network"
66           }
   +         
   +         finalyImpExt, err = json.Marshal(temp)
   +         if err != nil {
   +             return nil, append(errs, err)
   +         }
67  
68           reqCopy.Imp[0].Ext = finalyImpExt
69  
adapters/compass/compass.go

[guscarreon]

If the finalyImpExt variable is going to get overwritten anyways, a small optimization is to avoid copying reqCopy.Imp[0].Ext in the first place.

45           finalyImpExt := reqCopy.Imp[0].Ext
   +         var finalyImpExt json.RawMessage
adapters/compass/compass.go

[CompassSSP]

@guscarreon Thanks! We have fixed

[guscarreon]

Looks good to me. @CompassSSP thanks for addressing my feedback

[CompassSSP]

@SyntaxNode thanks for the review. I fixed some issues and included 883 in the config file

[SyntaxNode]

Thank you for quickly addressing the feedback. The last commit you pushed opened up a small refactor which I've commented on. Otherwise looks good and I'm glad to see good test coverage using the json test framework.

[SyntaxNode]

Nitpick: This is no longer required with the last fix refactor you pushed. You can replace all return nil, append(errs, err) statements with return nil, []error{err} like you do in MakeBids. The last return of this method will become return adapterRequests, nil which is a bit more straightforward IMHO.

[CompassSSP]

thanks, refactored

[AlexBVolcy]

Thank you to responding to all the feedback, I'll approve, but there's one nitpick left that Scott commented on that I think should be fixed before merge!