Skip to content

Commit

Permalink
Merge pull request #581 from PubMatic-OpenWrap/DCOPS-1343-hotfix-1
Browse files Browse the repository at this point in the history 
Revert "OTT-1297: cache fix: populate cache only upon successful DB c…
  • Loading branch information
@pm-viral-vala
pm-viral-vala authored Oct 11, 2023
2 parents c8b1dfc + 68bf3ea commit 461764f
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 270 deletions.
18 changes: 8 additions & 10 deletions modules/pubmatic/openwrap/cache/gocache/gocache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 3 additions & 7 deletions modules/pubmatic/openwrap/cache/gocache/partner_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -77,15 +75,16 @@ 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 {
queryType = models.WrapperLiveVersionSlotMappings
}
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
Expand All @@ -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
}
176 changes: 20 additions & 156 deletions modules/pubmatic/openwrap/cache/gocache/partner_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -501,7 +369,6 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) {
}, nil)
},
},

{
name: "empty_partnerConfigMap_from_DB",
fields: fields{
Expand Down Expand Up @@ -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)
}
Expand Down
41 changes: 18 additions & 23 deletions modules/pubmatic/openwrap/database/mysql/adunit_config.go
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package mysql

import (
"database/sql"
"encoding/json"
"errors"
"strconv"
"strings"

Expand All @@ -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
}
Loading

0 comments on commit 461764f

Please sign in to comment.