diff --git a/modules/pubmatic/openwrap/cache/gocache/gocache.go b/modules/pubmatic/openwrap/cache/gocache/gocache.go index 8a65faa80a4..05f4522b5c9 100644 --- a/modules/pubmatic/openwrap/cache/gocache/gocache.go +++ b/modules/pubmatic/openwrap/cache/gocache/gocache.go @@ -31,11 +31,10 @@ func key(format string, v ...interface{}) string { // any db or cache should be injectable type cache struct { sync.Map - cache *gocache.Cache - cfg config.Cache - db database.Database - metricEngine metrics.MetricsEngine - partnerConfigExpiry int + cache *gocache.Cache + cfg config.Cache + db database.Database + metricEngine metrics.MetricsEngine } var c *cache @@ -45,11 +44,10 @@ func New(goCache *gocache.Cache, database database.Database, cfg config.Cache, m cOnce.Do( func() { c = &cache{ - cache: goCache, - db: database, - cfg: cfg, - metricEngine: metricEngine, - partnerConfigExpiry: cfg.CacheDefaultExpiry - 60, // Reduced partnerConfig expiry by 1 minute to avoid inconsistent exipry of partnerConfig, adUnitConfig and WrapperConfig + cache: goCache, + db: database, + cfg: cfg, + metricEngine: metricEngine, } }) return c diff --git a/modules/pubmatic/openwrap/cache/gocache/partner_config.go b/modules/pubmatic/openwrap/cache/gocache/partner_config.go index 28c0ce434e9..33479cbb61a 100644 --- a/modules/pubmatic/openwrap/cache/gocache/partner_config.go +++ b/modules/pubmatic/openwrap/cache/gocache/partner_config.go @@ -64,8 +64,6 @@ func (c *cache) GetPartnerConfigMap(pubID, profileID, displayVersion int, endpoi } func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileID, displayVersion int) (err error) { - var errWrapperSlotMapping error - var errAdunitConfig error cacheKey := key(PUB_HB_PARTNER, pubID, profileID, displayVersion) partnerConfigMap, err := c.db.GetActivePartnerConfigurations(pubID, profileID, displayVersion) if err != nil { @@ -77,7 +75,8 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI return fmt.Errorf("there are no active partners for pubId:%d, profileId:%d, displayVersion:%d", pubID, profileID, displayVersion) } - if errWrapperSlotMapping = c.populateCacheWithWrapperSlotMappings(pubID, partnerConfigMap, profileID, displayVersion); errWrapperSlotMapping != nil { + c.cache.Set(cacheKey, partnerConfigMap, getSeconds(c.cfg.CacheDefaultExpiry)) + if errWrapperSlotMapping := c.populateCacheWithWrapperSlotMappings(pubID, partnerConfigMap, profileID, displayVersion); errWrapperSlotMapping != nil { err = errorWrap(err, errWrapperSlotMapping) queryType := models.WrapperSlotMappingsQuery if displayVersion == 0 { @@ -85,7 +84,7 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI } c.metricEngine.RecordDBQueryFailure(queryType, strconv.Itoa(pubID), strconv.Itoa(profileID)) } - if errAdunitConfig = c.populateCacheWithAdunitConfig(pubID, profileID, displayVersion); errAdunitConfig != nil { + if errAdunitConfig := c.populateCacheWithAdunitConfig(pubID, profileID, displayVersion); errAdunitConfig != nil { queryType := models.AdunitConfigQuery if displayVersion == 0 { queryType = models.AdunitConfigForLiveVersion @@ -96,8 +95,5 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI c.metricEngine.RecordDBQueryFailure(queryType, strconv.Itoa(pubID), strconv.Itoa(profileID)) err = errorWrap(err, errAdunitConfig) } - if errWrapperSlotMapping == nil && errAdunitConfig == nil { - c.cache.Set(cacheKey, partnerConfigMap, getSeconds(c.partnerConfigExpiry)) - } return } diff --git a/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go b/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go index 0f40f2329b5..b11ee82a220 100644 --- a/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go +++ b/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go @@ -115,67 +115,6 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { wantErr: true, want: nil, }, - { - name: "db_queries_failed_getting_adunitconfig", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 1000, - VASTTagCacheExpiry: 1000, - }, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: 0, - endpoint: models.EndpointAMP, - }, - setup: func(ctrl *gomock.Controller) (*mock_database.MockDatabase, *mock_metrics.MockMetricsEngine) { - mockDatabase := mock_database.NewMockDatabase(ctrl) - mockEngine := mock_metrics.NewMockMetricsEngine(ctrl) - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, 0).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetPublisherSlotNameHash(testPubID).Return(map[string]string{"adunit@728x90": "2aa34b52a9e941c1594af7565e599c8d"}, nil) - mockDatabase.EXPECT().GetPublisherVASTTags(testPubID).Return(nil, nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, 0).Return(nil, adunitconfig.ErrAdUnitUnmarshal) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, 0).Return(nil, nil) - mockEngine.EXPECT().RecordGetProfileDataTime(models.EndpointAMP, "123", gomock.Any()).Return().Times(1) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdUnitFailUnmarshal, "5890", "123").Return().Times(1) - return mockDatabase, mockEngine - }, - wantErr: true, - want: nil, - }, - - { - name: "db_queries_failed_wrapper_slotmappings", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 1000, - VASTTagCacheExpiry: 1000, - }, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: 0, - endpoint: models.EndpointAMP, - }, - setup: func(ctrl *gomock.Controller) (*mock_database.MockDatabase, *mock_metrics.MockMetricsEngine) { - mockDatabase := mock_database.NewMockDatabase(ctrl) - mockEngine := mock_metrics.NewMockMetricsEngine(ctrl) - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, 0).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetPublisherSlotNameHash(testPubID).Return(map[string]string{"adunit@728x90": "2aa34b52a9e941c1594af7565e599c8d"}, nil) - mockDatabase.EXPECT().GetPublisherVASTTags(testPubID).Return(nil, nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, 0).Return(nil, nil) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, 0).Return(nil, fmt.Errorf("Error from the DB")) - mockEngine.EXPECT().RecordGetProfileDataTime(models.EndpointAMP, "123", gomock.Any()).Return().Times(1) - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperLiveVersionSlotMappings, "5890", "123").Return().Times(1) - return mockDatabase, mockEngine - }, - wantErr: true, - want: nil, - }, { name: "db_queries_failed_getting_adunitconfig_and_wrapper_slotmappings", fields: fields{ @@ -205,7 +144,17 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { return mockDatabase, mockEngine }, wantErr: true, - want: nil, + want: map[int]map[string]string{ + 1: { + "partnerId": "1", + "prebidPartnerName": "pubmatic", + "serverSideEnabled": "1", + "level": "multi", + "kgp": "_AU_@_W_x_H", + "timeout": "220", + "bidderCode": "pubmatic", + }, + }, }, } for _, tt := range tests { @@ -214,9 +163,8 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { defer ctrl.Finish() c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, } c.db, c.metricEngine = tt.setup(ctrl) @@ -294,9 +242,8 @@ func Test_cache_GetPartnerConfigMap_LockandLoad(t *testing.T) { defer ctrl.Finish() c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, } c.db, c.metricEngine = tt.setup(ctrl) @@ -353,85 +300,6 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { want want setup func() }{ - { - name: "error_in_SlotMapping_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: fmt.Errorf("Incorrect Slot Mappings from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, nil) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect Slot Mappings from the DB")) - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperSlotMappingsQuery, "5890", "123").Return() - }, - }, - { - name: "error_in_AdUnitConfigRead_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: fmt.Errorf("Incorrect AdUnit Config from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect AdUnit Config from the DB")) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, nil) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdunitConfigQuery, "5890", "123").Return() - }, - }, - { - name: "error_in_SlotMapping_And_AdUnitConfigRead_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: errorWrap(fmt.Errorf("Incorrect Slot Mappings from the DB"), fmt.Errorf("Incorrect AdUnit Config from the DB")), // errors.New("Incorrect AdUnit Config from the DB: Incorrect Slot Mappings from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect AdUnit Config from the DB")) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect Slot Mappings from the DB")) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdunitConfigQuery, "5890", "123").Return() - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperSlotMappingsQuery, "5890", "123").Return() - }, - }, { name: "error_returning_Active_partner_configuration_from_DB", fields: fields{ @@ -501,7 +369,6 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { }, nil) }, }, - { name: "empty_partnerConfigMap_from_DB", fields: fields{ @@ -532,22 +399,19 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { tt.setup() } c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - db: tt.fields.db, - metricEngine: mockEngine, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, + db: tt.fields.db, + metricEngine: mockEngine, } err := c.getActivePartnerConfigAndPopulateWrapperMappings(tt.args.pubID, tt.args.profileID, tt.args.displayVersion) - + assert.Equal(t, tt.want.err, err) cacheKey := key(PUB_HB_PARTNER, tt.args.pubID, tt.args.profileID, tt.args.displayVersion) partnerConfigMap, found := c.Get(cacheKey) if tt.want.cacheEntry { - assert.Equal(t, tt.want.err, err) assert.True(t, found) assert.Equal(t, tt.want.partnerConfigMap, partnerConfigMap) } else { - assert.Equal(t, tt.want.err.Error(), err.Error()) assert.False(t, found) assert.Nil(t, partnerConfigMap) } diff --git a/modules/pubmatic/openwrap/database/mysql/adunit_config.go b/modules/pubmatic/openwrap/database/mysql/adunit_config.go index 22435f21523..7d3ff03034b 100644 --- a/modules/pubmatic/openwrap/database/mysql/adunit_config.go +++ b/modules/pubmatic/openwrap/database/mysql/adunit_config.go @@ -1,9 +1,7 @@ package mysql import ( - "database/sql" "encoding/json" - "errors" "strconv" "strings" @@ -22,32 +20,29 @@ func (db *mySqlDB) GetAdunitConfig(profileID, displayVersion int) (*adunitconfig var adunitConfigJSON string err := db.conn.QueryRow(adunitConfigQuery).Scan(&adunitConfigJSON) - if err != nil && !errors.Is(err, sql.ErrNoRows) { + if err != nil { return nil, err } - if len(adunitConfigJSON) > 0 { - adunitConfig := &adunitconfig.AdUnitConfig{} - err = json.Unmarshal([]byte(adunitConfigJSON), &adunitConfig) - if err != nil { - return nil, adunitconfig.ErrAdUnitUnmarshal - } - - for k, v := range adunitConfig.Config { - adunitConfig.Config[strings.ToLower(k)] = v - // shall we delete the orignal key-val? - } + adunitConfig := &adunitconfig.AdUnitConfig{} + err = json.Unmarshal([]byte(adunitConfigJSON), &adunitConfig) + if err != nil { + return nil, adunitconfig.ErrAdUnitUnmarshal + } - if adunitConfig.ConfigPattern == "" { - //Default configPattern value is "_AU_" if not present in db config - adunitConfig.ConfigPattern = models.MACRO_AD_UNIT_ID - } + for k, v := range adunitConfig.Config { + adunitConfig.Config[strings.ToLower(k)] = v + // shall we delete the orignal key-val? + } - if _, ok := adunitConfig.Config["default"]; !ok { - adunitConfig.Config["default"] = &adunitconfig.AdConfig{} - } + if adunitConfig.ConfigPattern == "" { + //Default configPattern value is "_AU_" if not present in db config + adunitConfig.ConfigPattern = models.MACRO_AD_UNIT_ID + } - return adunitConfig, nil + if _, ok := adunitConfig.Config["default"]; !ok { + adunitConfig.Config["default"] = &adunitconfig.AdConfig{} } - return nil, nil + + return adunitConfig, err } diff --git a/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go b/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go index 48447513e5e..29396c1cb1a 100644 --- a/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go +++ b/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go @@ -2,7 +2,6 @@ package mysql import ( "database/sql" - "errors" "regexp" "testing" @@ -41,79 +40,6 @@ func Test_mySqlDB_GetAdunitConfig(t *testing.T) { return db }, }, - { - name: "No rows for given query", - fields: fields{ - cfg: config.Database{ - Queries: config.Queries{ - GetAdunitConfigForLiveVersion: "^SELECT (.+) FROM wrapper_media_config (.+) LIVE", - }, - }, - }, - args: args{ - profileID: 5890, - displayVersion: 0, - }, - want: nil, - wantErr: false, - setup: func() *sql.DB { - db, mock, err := sqlmock.New() - if err != nil { - t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) - } - mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnError(sql.ErrNoRows) - return db - }, - }, - { - name: "Error in QueryRow for given query", - fields: fields{ - cfg: config.Database{ - Queries: config.Queries{ - GetAdunitConfigForLiveVersion: "^SELECT (.+) FROM wrapper_media_config (.+) LIVE", - }, - }, - }, - args: args{ - profileID: 5890, - displayVersion: 0, - }, - want: nil, - wantErr: true, - setup: func() *sql.DB { - db, mock, err := sqlmock.New() - if err != nil { - t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) - } - mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnError(errors.New("Error in query execution")) - return db - }, - }, - { - name: "Empty content return", - fields: fields{ - cfg: config.Database{ - Queries: config.Queries{ - GetAdunitConfigForLiveVersion: "^SELECT (.+) FROM wrapper_media_config (.+) LIVE", - }, - }, - }, - args: args{ - profileID: 5890, - displayVersion: 0, - }, - want: nil, - wantErr: false, - setup: func() *sql.DB { - db, mock, err := sqlmock.New() - if err != nil { - t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) - } - rows := sqlmock.NewRows([]string{"adunitConfig"}).AddRow(``) - mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnRows(rows) - return db - }, - }, { name: "query with display version id 0", fields: fields{