Skip to content

Commit

Permalink
Returning only triggered offers from GetEnabledOffers
Browse files Browse the repository at this point in the history
  • Loading branch information
Andre Hahn committed May 18, 2018
1 parent 73c8ab5 commit d3e51a9
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 33 deletions.
20 changes: 10 additions & 10 deletions migrations/migrations.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 6 additions & 9 deletions models/offer.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const enabledOffers = `
WHERE
offers.game_id = $1
AND offers.enabled = true
AND (trigger->>'to')::int >= $2
AND (trigger->>'from')::int <= $2
`

var isValidString = regexp.MustCompile(`^[a-zA-Z0-9_\.]+$`).MatchString
Expand Down Expand Up @@ -86,7 +88,7 @@ func buildInefficientScope(enabledOffers string, filterAttrs map[string]string)
return query
}

func buildEffiecientScope(enabledOffers string, filterAttrs map[string]string) string {
func buildEfficientScope(enabledOffers string, filterAttrs map[string]string) string {
subQueries := []string{enabledOffers}
for k, v := range filterAttrs {
// TODO: Possible SQL injection
Expand Down Expand Up @@ -125,17 +127,14 @@ func GetOfferByID(ctx context.Context, db runner.Connection, gameID, id string,
}

//GetEnabledOffers returns all the enabled offers and matching offers
func GetEnabledOffers(ctx context.Context, db runner.Connection, gameID string, offersCache *cache.Cache, expireDuration time.Duration, filterAttrs map[string]string, allowInefficientQueries bool, mr *MixedMetricsReporter) ([]*Offer, error) {
func GetEnabledOffers(ctx context.Context, db runner.Connection, gameID string, offersCache *cache.Cache, expireDuration time.Duration, currentTime time.Time, filterAttrs map[string]string, allowInefficientQueries bool, mr *MixedMetricsReporter) ([]*Offer, error) {
var offers []*Offer
var err error

enabledOffersKey := GetEnabledOffersKey(gameID)
// TODO: See if it is possible to enable cache with filters
if len(filterAttrs) == 0 {
offersInterface, found := offersCache.Get(enabledOffersKey)

if found {
//fmt.Println("Offers Cache Hit")
offers = offersInterface.([]*Offer)
return offers, err
}
Expand All @@ -145,13 +144,11 @@ func GetEnabledOffers(ctx context.Context, db runner.Connection, gameID string,
if allowInefficientQueries {
scope = buildInefficientScope(enabledOffers, filterAttrs)
} else {
scope = buildEffiecientScope(enabledOffers, filterAttrs)
scope = buildEfficientScope(enabledOffers, filterAttrs)
}

//fmt.Println("Offers Cache Miss")
err = mr.WithDatastoreSegment("offers", SegmentSelect, func() error {
// TODO: Add a configurable limit to this query
// Explicitly not fetching filters, the player does not need to know about them

builder := db.
Select(`
Expand All @@ -161,7 +158,7 @@ func GetEnabledOffers(ctx context.Context, db runner.Connection, gameID string,
`)
builder.Execer = edat.NewExecer(builder.Execer).WithContext(ctx)
return builder.From("offers").
Scope(scope, gameID).
Scope(scope, gameID, currentTime.Unix()).
QueryStructs(&offers)
})
err = HandleNotFoundError("Offer", map[string]interface{}{"enabled": true}, err)
Expand Down
12 changes: 2 additions & 10 deletions models/offer_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ func GetAvailableOffers(
gameID,
offersCache,
expireDuration,
t,
filterAttrs,
allowInefficientQueries,
mr,
Expand All @@ -400,21 +401,12 @@ func GetAvailableOffers(
return offersByPlacement, nil
}

var trigger TimeTrigger
filteredOffers, err := filterTemplatesByTrigger(trigger, enabledOffers, t)
if err != nil {
return nil, err
}
if len(filteredOffers) == 0 {
return offersByPlacement, nil
}

offersByPlayer, err := GetOffersByPlayer(ctx, db, gameID, playerID, mr)
if err != nil {
return nil, err
}

filteredOffers, err = filterOffersByFrequencyAndPeriod(playerID, filteredOffers, offersByPlayer, t, mr)
filteredOffers, err := filterOffersByFrequencyAndPeriod(playerID, enabledOffers, offersByPlayer, t, mr)
if err != nil {
return nil, err
}
Expand Down
9 changes: 5 additions & 4 deletions models/offer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
)

var _ = Describe("Offer Models", func() {
currentTime := time.Unix(1486678000, 0)
Describe("Get offer id", func() {
It("should load an offer from existent id", func() {
id := defaultOfferID
Expand Down Expand Up @@ -308,7 +309,7 @@ var _ = Describe("Offer Models", func() {
gameID := defaultGameID

filterAttrs := make(map[string]string)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, filterAttrs, false, nil)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, currentTime, filterAttrs, false, nil)
Expect(err).NotTo(HaveOccurred())
Expect(offers).To(HaveLen(4))
for i := 0; i < len(offers); i++ {
Expand All @@ -319,7 +320,7 @@ var _ = Describe("Offer Models", func() {
It("should return an empty list if there are no enabled offers", func() {
gameID := uuid.NewV4().String()
filterAttrs := make(map[string]string)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, filterAttrs, false, nil)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, currentTime, filterAttrs, false, nil)
Expect(err).NotTo(HaveOccurred())
Expect(offers).To(HaveLen(0))
})
Expand All @@ -334,13 +335,13 @@ var _ = Describe("Offer Models", func() {
gameID := defaultGameID
start := time.Now().UnixNano()
filterAttrs := make(map[string]string)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, filterAttrs, false, nil)
offers, err := models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, currentTime, filterAttrs, false, nil)
dbElapsedTime := time.Now().UnixNano() - start
Expect(err).NotTo(HaveOccurred())
Expect(offers).To(HaveLen(4))

start = time.Now().UnixNano()
offers, err = models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, filterAttrs, false, nil)
offers, err = models.GetEnabledOffers(nil, db, gameID, offersCache, expireDuration, currentTime, filterAttrs, false, nil)
cacheElapsedTime := time.Now().UnixNano() - start
Expect(err).NotTo(HaveOccurred())
_, found := offersCache.Get(models.GetEnabledOffersKey(gameID))
Expand Down

0 comments on commit d3e51a9

Please sign in to comment.