Repository: prebid/prebid-server
Pull request #2390 (2022-09-26 20:36:46)

Changed FS bidder names to mixed case


Patch:
diff --git a/config/bidderinfo.go b/config/bidderinfo.go
index 74122bdf..70e1f89f 100644
--- a/config/bidderinfo.go
+++ b/config/bidderinfo.go
@@ -9,6 +9,7 @@ import (
 	"github.com/prebid/prebid-server/util/sliceutil"
 	"io/ioutil"
 	"log"
+	"path/filepath"
 	"strings"
 	"text/template"
 
@@ -170,38 +171,66 @@ func (bi BidderInfo) IsEnabled() bool {
 	return !bi.Disabled
 }
 
-// LoadBidderInfo parses all static/bidder-info/{bidder}.yaml files from the file system.
-func LoadBidderInfo(path string) (BidderInfos, error) {
-	bidderConfigs, err := ioutil.ReadDir(path)
+type InfoReader interface {
+	Read() (map[string][]byte, error)
+}
+
+type InfoReaderFromDisk struct {
+	Path string
+}
+
+func (r InfoReaderFromDisk) Read() (map[string][]byte, error) {
+	bidderConfigs, err := ioutil.ReadDir(r.Path)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	infos := BidderInfos{}
-
+	bidderInfos := make(map[string][]byte)
 	for _, bidderConfig := range bidderConfigs {
 		if bidderConfig.IsDir() {
 			continue //ignore directories
 		}
 		fileName := bidderConfig.Name()
-		filePath := fmt.Sprintf("%v/%v", path, fileName)
+		filePath := filepath.Join(r.Path, fileName)
 		data, err := ioutil.ReadFile(filePath)
 		if err != nil {
 			return nil, err
 		}
+		bidderInfos[fileName] = data
+	}
+	return bidderInfos, nil
+}
 
-		info := BidderInfo{}
-		if err := yaml.Unmarshal(data, &info); err != nil {
-			return nil, fmt.Errorf("error parsing yaml for bidder %s: %v", fileName, err)
-		}
+func LoadBidderInfoFromDisk(path string) (BidderInfos, error) {
+	bidderInfoReader := InfoReaderFromDisk{Path: path}
+	return LoadBidderInfo(bidderInfoReader)
+}
 
+func LoadBidderInfo(reader InfoReader) (BidderInfos, error) {
+	return processBidderInfos(reader, openrtb_ext.NormalizeBidderName)
+}
+
+func processBidderInfos(reader InfoReader, normalizeBidderName func(string) (openrtb_ext.BidderName, bool)) (BidderInfos, error) {
+	bidderConfigs, err := reader.Read()
+	if err != nil {
+		return nil, fmt.Errorf("error loading bidders data")
+	}
+
+	infos := BidderInfos{}
+
+	for fileName, data := range bidderConfigs {
 		bidderName := strings.Split(fileName, ".")
 		if len(bidderName) == 2 && bidderName[1] == "yaml" {
-			// all bidder names loaded from the file system should be in lowercase
-			// to match bidder names in Viper config bidder infos
-			// this will prevent from missing bidder config overrides
-			// in func applyBidderInfoConfigOverrides
-			infos[(strings.ToLower(bidderName[0]))] = info
+			normalizedBidderName, bidderNameExists := normalizeBidderName(bidderName[0])
+			if !bidderNameExists {
+				return nil, fmt.Errorf("error parsing config for bidder %s: unknown bidder", fileName)
+			}
+			info := BidderInfo{}
+			if err := yaml.Unmarshal(data, &info); err != nil {
+				return nil, fmt.Errorf("error parsing config for bidder %s: %v", fileName, err)
+			}
+
+			infos[string(normalizedBidderName)] = info
 		}
 	}
 	return infos, nil
@@ -349,9 +378,13 @@ func validateSyncer(bidderInfo BidderInfo) error {
 	return nil
 }
 
-func applyBidderInfoConfigOverrides(configBidderInfos BidderInfos, fsBidderInfos BidderInfos) (BidderInfos, error) {
+func applyBidderInfoConfigOverrides(configBidderInfos BidderInfos, fsBidderInfos BidderInfos, normalizeBidderName func(string) (openrtb_ext.BidderName, bool)) (BidderInfos, error) {
 	for bidderName, bidderInfo := range configBidderInfos {
-		if fsBidderCfg, exists := fsBidderInfos[bidderName]; exists {
+		normalizedBidderName, bidderNameExists := normalizeBidderName(bidderName)
+		if !bidderNameExists {
+			return nil, fmt.Errorf("error setting configuration for bidder %s: unknown bidder", bidderName)
+		}
+		if fsBidderCfg, exists := fsBidderInfos[string(normalizedBidderName)]; exists {
 			bidderInfo.Syncer = bidderInfo.Syncer.Override(fsBidderCfg.Syncer)
 
 			if bidderInfo.Endpoint == "" && len(fsBidderCfg.Endpoint) > 0 {
@@ -436,7 +469,9 @@ func applyBidderInfoConfigOverrides(configBidderInfos BidderInfos, fsBidderInfos
 				glog.Warningf("adapters.%s.usersync_url is deprecated and will be removed in a future version, please update to the latest user sync config values", strings.ToLower(bidderName))
 			}
 
-			fsBidderInfos[bidderName] = bidderInfo
+			fsBidderInfos[string(normalizedBidderName)] = bidderInfo
+		} else {
+			return nil, fmt.Errorf("error finding configuration for bidder %s: unknown bidder", bidderName)
 		}
 	}
 	return fsBidderInfos, nil
diff --git a/config/config.go b/config/config.go
index a424a3f1..fc57bdc7 100644
--- a/config/config.go
+++ b/config/config.go
@@ -617,7 +617,7 @@ func (cfg *TimeoutNotification) validate(errs []error) []error {
 }
 
 // New uses viper to get our server configurations.
-func New(v *viper.Viper, bidderInfos BidderInfos) (*Configuration, error) {
+func New(v *viper.Viper, bidderInfos BidderInfos, normalizeBidderName func(string) (openrtb_ext.BidderName, bool)) (*Configuration, error) {
 	var c Configuration
 	if err := v.Unmarshal(&c); err != nil {
 		return nil, fmt.Errorf("viper failed to unmarshal app config: %v", err)
@@ -700,7 +700,7 @@ func New(v *viper.Viper, bidderInfos BidderInfos) (*Configuration, error) {
 	// Migrate combo stored request config to separate stored_reqs and amp stored_reqs configs.
 	resolvedStoredRequestsConfig(&c)
 
-	mergedBidderInfos, err := applyBidderInfoConfigOverrides(c.BidderInfos, bidderInfos)
+	mergedBidderInfos, err := applyBidderInfoConfigOverrides(c.BidderInfos, bidderInfos, normalizeBidderName)
 	if err != nil {
 		return nil, err
 	}
diff --git a/main.go b/main.go
index bdd79f11..a8326666 100644
--- a/main.go
+++ b/main.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/prebid/prebid-server/config"
 	"github.com/prebid/prebid-server/currency"
+	"github.com/prebid/prebid-server/openrtb_ext"
 	"github.com/prebid/prebid-server/router"
 	"github.com/prebid/prebid-server/server"
 	"github.com/prebid/prebid-server/util/task"
@@ -30,7 +31,7 @@ func main() {
 		glog.Exitf("Unable to build configuration directory path: %v", err)
 	}
 
-	bidderInfos, err := config.LoadBidderInfo(bidderInfoPath)
+	bidderInfos, err := config.LoadBidderInfoFromDisk(bidderInfoPath)
 	if err != nil {
 		glog.Exitf("Unable to load bidder configurations: %v", err)
 	}
@@ -59,7 +60,7 @@ const infoDirectory = "./static/bidder-info"
 func loadConfig(bidderInfos config.BidderInfos) (*config.Configuration, error) {
 	v := viper.New()
 	config.SetupViper(v, configFileName, bidderInfos)
-	return config.New(v, bidderInfos)
+	return config.New(v, bidderInfos, openrtb_ext.NormalizeBidderName)
 }
 
 func serve(cfg *config.Configuration) error {


Test patch:
diff --git a/config/bidderinfo_test.go b/config/bidderinfo_test.go
index 85e15323..462e920f 100644
--- a/config/bidderinfo_test.go
+++ b/config/bidderinfo_test.go
@@ -10,7 +10,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const testInfoFilesPath = "./test/bidder-info"
+const testInfoFilesPathValid = "./test/bidder-info-valid"
+const testSimpleYAML = `
+maintainer:
+  email: "some-email@domain.com"
+gvlVendorID: 42
+`
 const fullBidderYAMLConfig = `
 maintainer:
   email: "some-email@domain.com"
@@ -36,14 +41,14 @@ endpointCompression: "GZIP"
 `
 
 func TestLoadBidderInfoFromDisk(t *testing.T) {
-	// should appear in result in lowercase
-	bidder := "somebidder"
+	// should appear in result in mixed case
+	bidder := "stroeerCore"
 	trueValue := true
 
 	adapterConfigs := make(map[string]Adapter)
 	adapterConfigs[strings.ToLower(bidder)] = Adapter{}
 
-	infos, err := LoadBidderInfo(testInfoFilesPath)
+	infos, err := LoadBidderInfoFromDisk(testInfoFilesPathValid)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -84,6 +89,80 @@ func TestLoadBidderInfoFromDisk(t *testing.T) {
 	assert.Equal(t, expected, infos)
 }
 
+func TestProcessBidderInfo(t *testing.T) {
+	testCases := []struct {
+		description         string
+		bidderInfos         map[string][]byte
+		expectedBidderInfos BidderInfos
+		expectError         string
+	}{
+		{
+			description: "Valid bidder info",
+			bidderInfos: map[string][]byte{
+				"bidderA.yaml": []byte(testSimpleYAML),
+			},
+			expectedBidderInfos: BidderInfos{
+				"bidderA": BidderInfo{
+					Maintainer: &MaintainerInfo{
+						Email: "some-email@domain.com",
+					},
+					GVLVendorID: 42,
+				},
+			},
+			expectError: "",
+		},
+		{
+			description: "Bidder doesn't exist in bidder info list",
+			bidderInfos: map[string][]byte{
+				"unknown.yaml": []byte(testSimpleYAML),
+			},
+			expectedBidderInfos: nil,
+			expectError:         "error parsing config for bidder unknown.yaml",
+		},
+		{
+			description: "Invalid bidder config",
+			bidderInfos: map[string][]byte{
+				"bidderA.yaml": []byte("invalid bidder config"),
+			},
+			expectedBidderInfos: nil,
+			expectError:         "error parsing config for bidder bidderA.yaml",
+		},
+	}
+	for _, test := range testCases {
+		reader := StubInfoReader{test.bidderInfos}
+		bidderInfos, err := processBidderInfos(reader, mockNormalizeBidderName)
+		if test.expectError != "" {
+			assert.ErrorContains(t, err, test.expectError, "")
+		} else {
+			assert.Equal(t, test.expectedBidderInfos, bidderInfos, "incorrect bidder infos for test case: %s", test.description)
+		}
+
+	}
+
+}
+
+type StubInfoReader struct {
+	mockBidderInfos map[string][]byte
+}
+
+func (r StubInfoReader) Read() (map[string][]byte, error) {
+	return r.mockBidderInfos, nil
+}
+
+var testBidderNames = map[string]openrtb_ext.BidderName{
+	"biddera": openrtb_ext.BidderName("bidderA"),
+	"bidderb": openrtb_ext.BidderName("bidderB"),
+	"bidder1": openrtb_ext.BidderName("bidder1"),
+	"bidder2": openrtb_ext.BidderName("bidder2"),
+	"a":       openrtb_ext.BidderName("a"),
+}
+
+func mockNormalizeBidderName(name string) (openrtb_ext.BidderName, bool) {
+	nameLower := strings.ToLower(name)
+	bidderName, exists := testBidderNames[nameLower]
+	return bidderName, exists
+}
+
 func TestToGVLVendorIDMap(t *testing.T) {
 	givenBidderInfos := BidderInfos{
 		"bidderA": BidderInfo{Disabled: false, GVLVendorID: 0},
@@ -105,7 +184,7 @@ const bidderInfoRelativePath = "../static/bidder-info"
 // TestBidderInfoFiles ensures each bidder has a valid static/bidder-info/bidder.yaml file. Validation is performed directly
 // against the file system with separate yaml unmarshalling from the LoadBidderInfo func.
 func TestBidderInfoFiles(t *testing.T) {
-	_, err := LoadBidderInfo(bidderInfoRelativePath)
+	_, err := LoadBidderInfoFromDisk(bidderInfoRelativePath)
 	if err != nil {
 		assert.Fail(t, err.Error(), "Errors in bidder info files")
 	}
@@ -687,7 +766,7 @@ func TestApplyBidderInfoConfigSyncerOverrides(t *testing.T) {
 	}
 
 	for _, test := range testCases {
-		bidderInfos, resultErr := applyBidderInfoConfigOverrides(test.givenConfigBidderInfos, test.givenFsBidderInfos)
+		bidderInfos, resultErr := applyBidderInfoConfigOverrides(test.givenConfigBidderInfos, test.givenFsBidderInfos, mockNormalizeBidderName)
 		if test.expectedError == "" {
 			assert.NoError(t, resultErr, test.description+":err")
 			assert.Equal(t, test.expectedBidderInfos, bidderInfos, test.description+":result")
@@ -849,17 +928,43 @@ func TestApplyBidderInfoConfigOverrides(t *testing.T) {
 		},
 	}
 	for _, test := range testCases {
-		bidderInfos, resultErr := applyBidderInfoConfigOverrides(test.givenConfigBidderInfos, test.givenFsBidderInfos)
+		bidderInfos, resultErr := applyBidderInfoConfigOverrides(test.givenConfigBidderInfos, test.givenFsBidderInfos, mockNormalizeBidderName)
 		assert.NoError(t, resultErr, test.description+":err")
 		assert.Equal(t, test.expectedBidderInfos, bidderInfos, test.description+":result")
 	}
 }
 
+func TestApplyBidderInfoConfigOverridesInvalid(t *testing.T) {
+	var testCases = []struct {
+		description            string
+		givenFsBidderInfos     BidderInfos
+		givenConfigBidderInfos BidderInfos
+		expectedError          string
+		expectedBidderInfos    BidderInfos
+	}{
+		{
+			description:            "Bidder doesn't exists in bidder list",
+			givenConfigBidderInfos: BidderInfos{"unknown": {Syncer: &Syncer{Key: "override"}}},
+			expectedError:          "error setting configuration for bidder unknown: unknown bidder",
+		},
+		{
+			description:            "Bidder doesn't exists in file system",
+			givenFsBidderInfos:     BidderInfos{"unknown": {Endpoint: "original"}},
+			givenConfigBidderInfos: BidderInfos{"bidderA": {Syncer: &Syncer{Key: "override"}}},
+			expectedError:          "error finding configuration for bidder bidderA: unknown bidder",
+		},
+	}
+	for _, test := range testCases {
+		_, err := applyBidderInfoConfigOverrides(test.givenConfigBidderInfos, test.givenFsBidderInfos, mockNormalizeBidderName)
+		assert.ErrorContains(t, err, test.expectedError, test.description+":err")
+	}
+}
+
 func TestReadFullYamlBidderConfig(t *testing.T) {
-	bidder := "someBidder"
+	bidder := "bidderA"
 	bidderInf := BidderInfo{}
 	err := yaml.Unmarshal([]byte(fullBidderYAMLConfig), &bidderInf)
-	actualBidderInfo, err := applyBidderInfoConfigOverrides(BidderInfos{bidder: bidderInf}, BidderInfos{bidder: {Syncer: &Syncer{Supports: []string{"iframe"}}}})
+	actualBidderInfo, err := applyBidderInfoConfigOverrides(BidderInfos{bidder: bidderInf}, BidderInfos{bidder: {Syncer: &Syncer{Supports: []string{"iframe"}}}}, mockNormalizeBidderName)
 
 	assert.NoError(t, err, "Error wasn't expected")
 
diff --git a/config/config_test.go b/config/config_test.go
index 27077ad7..e2a97180 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -442,7 +442,7 @@ func TestFullConfig(t *testing.T) {
 	SetupViper(v, "", bidderInfos)
 	v.SetConfigType("yaml")
 	v.ReadConfig(bytes.NewBuffer(fullConfig))
-	cfg, err := New(v, bidderInfos)
+	cfg, err := New(v, bidderInfos, mockNormalizeBidderName)
 	assert.NoError(t, err, "Setting up config should work but it doesn't")
 	cmpStrings(t, "cookie domain", cfg.HostCookie.Domain, "cookies.prebid.org")
 	cmpStrings(t, "cookie name", cfg.HostCookie.CookieName, "userid")
@@ -691,7 +691,7 @@ func TestMigrateConfig(t *testing.T) {
 	v.SetConfigType("yaml")
 	v.ReadConfig(bytes.NewBuffer(oldStoredRequestsConfig))
 	migrateConfig(v)
-	cfg, err := New(v, bidderInfos)
+	cfg, err := New(v, bidderInfos, mockNormalizeBidderName)
 	assert.NoError(t, err, "Setting up config should work but it doesn't")
 	cmpBools(t, "stored_requests.filesystem.enabled", true, cfg.StoredRequests.Files.Enabled)
 	cmpStrings(t, "stored_requests.filesystem.path", "/somepath", cfg.StoredRequests.Files.Path)
@@ -1753,7 +1753,7 @@ func TestNewCallsRequestValidation(t *testing.T) {
 			`request_validation:
     ipv4_private_networks: ` + test.privateIPNetworks)))
 
-		result, resultErr := New(v, bidderInfos)
+		result, resultErr := New(v, bidderInfos, mockNormalizeBidderName)
 
 		if test.expectedError == "" {
 			assert.NoError(t, resultErr, test.description+":err")
@@ -1788,7 +1788,7 @@ func newDefaultConfig(t *testing.T) (*Configuration, *viper.Viper) {
 	SetupViper(v, "", bidderInfos)
 	v.Set("gdpr.default_value", "0")
 	v.SetConfigType("yaml")
-	cfg, err := New(v, bidderInfos)
+	cfg, err := New(v, bidderInfos, mockNormalizeBidderName)
 	assert.NoError(t, err, "Setting up config should work but it doesn't")
 	return cfg, v
 }
@@ -1858,7 +1858,7 @@ func TestSpecialFeature1VendorExceptionMap(t *testing.T) {
 		SetupViper(v, "", bidderInfos)
 		v.SetConfigType("yaml")
 		v.ReadConfig(bytes.NewBuffer(config))
-		cfg, err := New(v, bidderInfos)
+		cfg, err := New(v, bidderInfos, mockNormalizeBidderName)
 		assert.NoError(t, err, "Setting up config error", tt.description)
 
 		assert.Equal(t, tt.wantVendorExceptions, cfg.GDPR.TCF2.SpecialFeature1.VendorExceptions, tt.description)
diff --git a/config/test/bidder-info/someBidder.yaml b/config/test/bidder-info-valid/stroeerCore.yaml
similarity index 100%
rename from config/test/bidder-info/someBidder.yaml
rename to config/test/bidder-info-valid/stroeerCore.yaml
diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go
index 866e4d77..60cc140d 100644
--- a/exchange/exchange_test.go
+++ b/exchange/exchange_test.go
@@ -56,7 +56,7 @@ func TestNewExchange(t *testing.T) {
 		},
 	}
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -80,7 +80,7 @@ func TestNewExchange(t *testing.T) {
 	e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, tcf2ConfigBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}).(*exchange)
 	for _, bidderName := range knownAdapters {
 		if _, ok := e.adapterMap[bidderName]; !ok {
-			if biddersInfo[strings.ToLower(string(bidderName))].IsEnabled() {
+			if biddersInfo[string(bidderName)].IsEnabled() {
 				t.Errorf("NewExchange produced an Exchange without bidder %s", bidderName)
 			}
 		}
@@ -1239,7 +1239,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(handlerNoBidServer))
 	defer server.Close()
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1602,7 +1602,7 @@ func TestBidResponseCurrency(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(handlerNoBidServer))
 	defer server.Close()
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1846,7 +1846,7 @@ func TestRaceIntegration(t *testing.T) {
 
 	cfg := &config.Configuration{}
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1955,7 +1955,7 @@ func TestPanicRecovery(t *testing.T) {
 		},
 	}
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -2024,7 +2024,7 @@ func TestPanicRecoveryHighLevel(t *testing.T) {
 
 	cfg := &config.Configuration{}
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -4083,7 +4083,7 @@ func TestPassExperimentConfigsToHoldAuction(t *testing.T) {
 
 	cfg := &config.Configuration{}
 
-	biddersInfo, err := config.LoadBidderInfo("../static/bidder-info")
+	biddersInfo, err := config.LoadBidderInfoFromDisk("../static/bidder-info")
 	if err != nil {
 		t.Fatal(err)
 	}