Repository: prebid/prebid-server
Pull request #3619 (2024-04-09 21:52:42)

Refactor Request Alias Parsing In Request Splitter
Removes two calls to jsonparser, reusing data already available in the request wrapper.


Patch:
diff --git a/exchange/utils.go b/exchange/utils.go
index d5e94316..e64ac6aa 100644
--- a/exchange/utils.go
+++ b/exchange/utils.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/prebid/prebid-server/v2/ortb"
 
-	"github.com/buger/jsonparser"
 	"github.com/prebid/go-gdpr/vendorconsent"
 	gpplib "github.com/prebid/go-gpp"
 	gppConstants "github.com/prebid/go-gpp/constants"
@@ -32,6 +31,8 @@ import (
 	"github.com/prebid/prebid-server/v2/util/ptrutil"
 )
 
+var errInvalidRequestExt = errors.New("request.ext is invalid")
+
 var channelTypeMap = map[metrics.RequestType]config.ChannelType{
 	metrics.ReqTypeAMP:       config.ChannelAMP,
 	metrics.ReqTypeORTB2App:  config.ChannelApp,
@@ -64,7 +65,8 @@ func (rs *requestSplitter) cleanOpenRTBRequests(ctx context.Context,
 	bidAdjustmentFactors map[string]float64,
 ) (allowedBidderRequests []BidderRequest, privacyLabels metrics.PrivacyLabels, errs []error) {
 	req := auctionReq.BidRequestWrapper
-	aliases, errs := parseAliases(req.BidRequest)
+
+	requestAliases, requestAliasesGVLIDs, errs := getRequestAliases(req)
 	if len(errs) > 0 {
 		return
 	}
@@ -79,19 +81,14 @@ func (rs *requestSplitter) cleanOpenRTBRequests(ctx context.Context,
 		return
 	}
 
-	aliasesGVLIDs, errs := parseAliasesGVLIDs(req.BidRequest)
-	if len(errs) > 0 {
-		return
-	}
-
 	var allBidderRequests []BidderRequest
 	var allBidderRequestErrs []error
-	allBidderRequests, allBidderRequestErrs = getAuctionBidderRequests(auctionReq, requestExt, rs.bidderToSyncerKey, impsByBidder, aliases, rs.hostSChainNode)
+	allBidderRequests, allBidderRequestErrs = getAuctionBidderRequests(auctionReq, requestExt, rs.bidderToSyncerKey, impsByBidder, requestAliases, rs.hostSChainNode)
 	if allBidderRequestErrs != nil {
 		errs = append(errs, allBidderRequestErrs...)
 	}
 
-	bidderNameToBidderReq := buildBidResponseRequest(req.BidRequest, bidderImpWithBidResp, aliases, auctionReq.BidderImpReplaceImpID)
+	bidderNameToBidderReq := buildBidResponseRequest(req.BidRequest, bidderImpWithBidResp, requestAliases, auctionReq.BidderImpReplaceImpID)
 	//this function should be executed after getAuctionBidderRequests
 	allBidderRequests = mergeBidderRequests(allBidderRequests, bidderNameToBidderReq)
 
@@ -114,7 +111,7 @@ func (rs *requestSplitter) cleanOpenRTBRequests(ctx context.Context,
 		errs = append(errs, err)
 	}
 
-	ccpaEnforcer, err := extractCCPA(req.BidRequest, rs.privacyConfig, &auctionReq.Account, aliases, channelTypeMap[auctionReq.LegacyLabels.RType], gpp)
+	ccpaEnforcer, err := extractCCPA(req.BidRequest, rs.privacyConfig, &auctionReq.Account, requestAliases, channelTypeMap[auctionReq.LegacyLabels.RType], gpp)
 	if err != nil {
 		errs = append(errs, err)
 	}
@@ -141,7 +138,7 @@ func (rs *requestSplitter) cleanOpenRTBRequests(ctx context.Context,
 		}
 
 		gdprRequestInfo := gdpr.RequestInfo{
-			AliasGVLIDs: aliasesGVLIDs,
+			AliasGVLIDs: requestAliasesGVLIDs,
 			Consent:     consent,
 			GDPRSignal:  gdprSignal,
 			PublisherID: auctionReq.LegacyLabels.PubID,
@@ -264,14 +261,14 @@ func ccpaEnabled(account *config.Account, privacyConfig config.Privacy, requestT
 	return privacyConfig.CCPA.Enforce
 }
 
-func extractCCPA(orig *openrtb2.BidRequest, privacyConfig config.Privacy, account *config.Account, aliases map[string]string, requestType config.ChannelType, gpp gpplib.GppContainer) (privacy.PolicyEnforcer, error) {
+func extractCCPA(orig *openrtb2.BidRequest, privacyConfig config.Privacy, account *config.Account, requestAliases map[string]string, requestType config.ChannelType, gpp gpplib.GppContainer) (privacy.PolicyEnforcer, error) {
 	// Quick extra wrapper until RequestWrapper makes its way into CleanRequests
 	ccpaPolicy, err := ccpa.ReadFromRequestWrapper(&openrtb_ext.RequestWrapper{BidRequest: orig}, gpp)
 	if err != nil {
 		return privacy.NilPolicyEnforcer{}, err
 	}
 
-	validBidders := GetValidBidders(aliases)
+	validBidders := GetValidBidders(requestAliases)
 	ccpaParsedPolicy, err := ccpaPolicy.Parse(validBidders)
 	if err != nil {
 		return privacy.NilPolicyEnforcer{}, err
@@ -321,7 +318,7 @@ func getAuctionBidderRequests(auctionRequest AuctionRequest,
 	requestExt *openrtb_ext.ExtRequest,
 	bidderToSyncerKey map[string]string,
 	impsByBidder map[string][]openrtb2.Imp,
-	aliases map[string]string,
+	requestAliases map[string]string,
 	hostSChainNode *openrtb2.SupplyChainNode) ([]BidderRequest, []error) {
 
 	bidderRequests := make([]BidderRequest, 0, len(impsByBidder))
@@ -349,7 +346,7 @@ func getAuctionBidderRequests(auctionRequest AuctionRequest,
 
 	var errs []error
 	for bidder, imps := range impsByBidder {
-		coreBidder, isRequestAlias := resolveBidder(bidder, aliases)
+		coreBidder, isRequestAlias := resolveBidder(bidder, requestAliases)
 
 		reqCopy := *req.BidRequest
 		reqCopy.Imp = imps
@@ -795,36 +792,23 @@ func resolveBidder(bidder string, requestAliases map[string]string) (openrtb_ext
 	return normalisedBidderName, false
 }
 
-// parseAliases parses the aliases from the BidRequest
-func parseAliases(orig *openrtb2.BidRequest) (map[string]string, []error) {
-	var aliases map[string]string
-	if value, dataType, _, err := jsonparser.Get(orig.Ext, openrtb_ext.PrebidExtKey, "aliases"); dataType == jsonparser.Object && err == nil {
-		if err := jsonutil.Unmarshal(value, &aliases); err != nil {
-			return nil, []error{err}
-		}
-	} else if dataType != jsonparser.NotExist && err != jsonparser.KeyPathNotFoundError {
-		return nil, []error{err}
+func getRequestAliases(req *openrtb_ext.RequestWrapper) (map[string]string, map[string]uint16, []error) {
+	reqExt, err := req.GetRequestExt()
+	if err != nil {
+		return nil, nil, []error{errInvalidRequestExt}
 	}
-	return aliases, nil
-}
 
-// parseAliasesGVLIDs parses the Bidder Alias GVLIDs from the BidRequest
-func parseAliasesGVLIDs(orig *openrtb2.BidRequest) (map[string]uint16, []error) {
-	var aliasesGVLIDs map[string]uint16
-	if value, dataType, _, err := jsonparser.Get(orig.Ext, openrtb_ext.PrebidExtKey, "aliasgvlids"); dataType == jsonparser.Object && err == nil {
-		if err := jsonutil.Unmarshal(value, &aliasesGVLIDs); err != nil {
-			return nil, []error{err}
-		}
-	} else if dataType != jsonparser.NotExist && err != jsonparser.KeyPathNotFoundError {
-		return nil, []error{err}
+	if prebid := reqExt.GetPrebid(); prebid != nil {
+		return prebid.Aliases, prebid.AliasGVLIDs, nil
 	}
-	return aliasesGVLIDs, nil
+
+	return nil, nil, nil
 }
 
-func GetValidBidders(aliases map[string]string) map[string]struct{} {
+func GetValidBidders(requestAliases map[string]string) map[string]struct{} {
 	validBidders := openrtb_ext.BuildBidderNameHashSet()
 
-	for k := range aliases {
+	for k := range requestAliases {
 		validBidders[k] = struct{}{}
 	}
 
@@ -964,13 +948,13 @@ func applyFPD(fpd map[openrtb_ext.BidderName]*firstpartydata.ResolvedFirstPartyD
 
 func buildBidResponseRequest(req *openrtb2.BidRequest,
 	bidderImpResponses stored_responses.BidderImpsWithBidResponses,
-	aliases map[string]string,
+	requestAliases map[string]string,
 	bidderImpReplaceImpID stored_responses.BidderImpReplaceImpID) map[openrtb_ext.BidderName]BidderRequest {
 
 	bidderToBidderResponse := make(map[openrtb_ext.BidderName]BidderRequest)
 
 	for bidderName, impResps := range bidderImpResponses {
-		resolvedBidder, isRequestAlias := resolveBidder(string(bidderName), aliases)
+		resolvedBidder, isRequestAlias := resolveBidder(string(bidderName), requestAliases)
 		bidderToBidderResponse[bidderName] = BidderRequest{
 			BidRequest:            req,
 			BidderCoreName:        resolvedBidder,


Test patch:
diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go
index 87c3936f..88b5b28b 100644
--- a/exchange/exchange_test.go
+++ b/exchange/exchange_test.go
@@ -2106,8 +2106,8 @@ func loadFile(filename string) (*exchangeSpec, error) {
 }
 
 func runSpec(t *testing.T, filename string, spec *exchangeSpec) {
-	aliases, errs := parseAliases(&spec.IncomingRequest.OrtbRequest)
-	if len(errs) != 0 {
+	aliases, err := parseRequestAliases(spec.IncomingRequest.OrtbRequest)
+	if err != nil {
 		t.Fatalf("%s: Failed to parse aliases", filename)
 	}
 
@@ -5782,6 +5782,24 @@ func (m *mockBidder) MakeBids(internalRequest *openrtb2.BidRequest, externalRequ
 	return args.Get(0).(*adapters.BidderResponse), args.Get(1).([]error)
 }
 
+func parseRequestAliases(r openrtb2.BidRequest) (map[string]string, error) {
+	if len(r.Ext) == 0 {
+		return nil, nil
+	}
+
+	ext := struct {
+		Prebid struct {
+			Aliases map[string]string `json:"aliases"`
+		} `json:"prebid"`
+	}{}
+
+	if err := jsonutil.Unmarshal(r.Ext, &ext); err != nil {
+		return nil, err
+	}
+
+	return ext.Prebid.Aliases, nil
+}
+
 func getInfoFromImp(req *openrtb_ext.RequestWrapper) (json.RawMessage, string, error) {
 	bidRequest := req.BidRequest
 	imp := bidRequest.Imp[0]
diff --git a/exchange/utils_test.go b/exchange/utils_test.go
index eeef2e3a..69195736 100644
--- a/exchange/utils_test.go
+++ b/exchange/utils_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/prebid/prebid-server/v2/util/ptrutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
 const deviceUA = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.87 Safari/537.36"
@@ -3266,63 +3267,80 @@ func TestApplyFPD(t *testing.T) {
 	}
 }
 
-func Test_parseAliasesGVLIDs(t *testing.T) {
-	type args struct {
-		orig *openrtb2.BidRequest
-	}
+func TestGetRequestAliases(t *testing.T) {
 	tests := []struct {
-		name      string
-		args      args
-		want      map[string]uint16
-		wantError bool
+		name         string
+		givenRequest openrtb_ext.RequestWrapper
+		wantAliases  map[string]string
+		wantGVLIDs   map[string]uint16
+		wantError    string
 	}{
 		{
-			"AliasGVLID Parsed Correctly",
-			args{
-				orig: &openrtb2.BidRequest{
-					Ext: json.RawMessage(`{"prebid":{"aliases":{"somealiascode":"appnexus"}, "aliasgvlids":{"somealiascode":1}}}`),
+			name: "nil",
+			givenRequest: openrtb_ext.RequestWrapper{
+				BidRequest: &openrtb2.BidRequest{},
+			},
+			wantAliases: nil,
+			wantGVLIDs:  nil,
+			wantError:   "",
+		},
+		{
+			name: "empty",
+			givenRequest: openrtb_ext.RequestWrapper{
+				BidRequest: &openrtb2.BidRequest{
+					Ext: json.RawMessage(`{}`),
 				},
 			},
-			map[string]uint16{"somealiascode": 1},
-			false,
+			wantAliases: nil,
+			wantGVLIDs:  nil,
+			wantError:   "",
 		},
 		{
-			"AliasGVLID parsing error",
-			args{
-				orig: &openrtb2.BidRequest{
-					Ext: json.RawMessage(`{"prebid":{"aliases":{"somealiascode":"appnexus"}, "aliasgvlids": {"somealiascode":"abc"}`),
+			name: "empty-prebid",
+			givenRequest: openrtb_ext.RequestWrapper{
+				BidRequest: &openrtb2.BidRequest{
+					Ext: json.RawMessage(`{"prebid":{}}`),
 				},
 			},
-			nil,
-			true,
+			wantAliases: nil,
+			wantGVLIDs:  nil,
+			wantError:   "",
 		},
 		{
-			"Invalid AliasGVLID",
-			args{
-				orig: &openrtb2.BidRequest{
-					Ext: json.RawMessage(`{"prebid":{"aliases":{"somealiascode":"appnexus"}, "aliasgvlids":"abc"}`),
+			name: "aliases-and-gvlids",
+			givenRequest: openrtb_ext.RequestWrapper{
+				BidRequest: &openrtb2.BidRequest{
+					Ext: json.RawMessage(`{"prebid":{"aliases":{"alias1":"bidder1"}, "aliasgvlids":{"alias1":1}}}`),
 				},
 			},
-			nil,
-			true,
+			wantAliases: map[string]string{"alias1": "bidder1"},
+			wantGVLIDs:  map[string]uint16{"alias1": 1},
+			wantError:   "",
 		},
 		{
-			"Missing AliasGVLID",
-			args{
-				orig: &openrtb2.BidRequest{
-					Ext: json.RawMessage(`{"prebid":{"aliases":{"somealiascode":"appnexus"}}`),
+			name: "malformed",
+			givenRequest: openrtb_ext.RequestWrapper{
+				BidRequest: &openrtb2.BidRequest{
+					Ext: json.RawMessage(`malformed`),
 				},
 			},
-			nil,
-			false,
+			wantAliases: nil,
+			wantGVLIDs:  nil,
+			wantError:   "request.ext is invalid",
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := parseAliasesGVLIDs(tt.args.orig)
-			assert.Equal(t, tt.want, got, "parseAliasesGVLIDs() got = %v, want %v", got, tt.want)
-			if !tt.wantError && err != nil {
-				t.Errorf("parseAliasesGVLIDs() expected error got nil")
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			gotAliases, gotGVLIDs, err := getRequestAliases(&test.givenRequest)
+
+			assert.Equal(t, test.wantAliases, gotAliases, "aliases")
+			assert.Equal(t, test.wantGVLIDs, gotGVLIDs, "gvlids")
+
+			if len(test.wantError) > 0 {
+				require.Len(t, err, 1, "error-len")
+				assert.EqualError(t, err[0], test.wantError, "error")
+			} else {
+				assert.Empty(t, err, "error")
 			}
 		})
 	}