Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ashishshinde-pubm committed Jul 24, 2024
1 parent f17c303 commit 6359c8c
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 62 deletions.
59 changes: 24 additions & 35 deletions adapters/ortbbidder/resolver/errortypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,78 +2,67 @@ package resolver

import "fmt"

// Severity represents the severity level of a error.
type Severity int
// severity represents the severity level of a error.
type severity int

const (
SeverityFatal Severity = iota
SeverityWarning
SeverityDebug
severityFatal severity = iota // use this to discard the entire bid response
severityWarning // use this to include errors in responseExt.warnings
severityIgnore // use this to exclude errors from responseExt.warnings
)

// Coder is used to indicate the severity of an error.
type Coder interface {
// coder is used to indicate the severity of an error.
type coder interface {
// Severity returns the severity level of the error.
Severity() Severity
Severity() severity
}

// DefaultValueError is used to flag that a default value was found.
type DefaultValueError struct {
// defaultValueError is used to flag that a default value was found.
type defaultValueError struct {
Message string
}

// Error returns the error message.
func (err *DefaultValueError) Error() string {
func (err *defaultValueError) Error() string {
return err.Message
}

// Severity returns the severity level of the error.
func (err *DefaultValueError) Severity() Severity {
return SeverityDebug
func (err *defaultValueError) Severity() severity {
return severityIgnore
}

// ValidationFailedError is used to flag that the value validation failed.
type ValidationFailedError struct {
// validationFailedError is used to flag that the value validation failed.
type validationFailedError struct {
Message string // Message contains the error message.
}

// Error returns the error message for ValidationFailedError.
func (err *ValidationFailedError) Error() string {
func (err *validationFailedError) Error() string {
return err.Message
}

// Severity returns the severity level for ValidationFailedError.
func (err *ValidationFailedError) Severity() Severity {
return SeverityWarning
func (err *validationFailedError) Severity() severity {
return severityWarning
}

// isWarning returns true if an error is labeled with a Severity of SeverityWarning.
func isWarning(err error) bool {
s, ok := err.(Coder)
return ok && s.Severity() == SeverityWarning
}

// ContainsWarning checks if the error list contains a warning.
func ContainsWarning(errors []error) bool {
for _, err := range errors {
if isWarning(err) {
return true
}
}

return false
// IsWarning returns true if an error is labeled with a Severity of SeverityWarning.
func IsWarning(err error) bool {
s, ok := err.(coder)
return ok && s.Severity() == severityWarning
}

// NewDefaultValueError creates a new DefaultValueError with a formatted message.
func NewDefaultValueError(message string, args ...any) error {
return &DefaultValueError{
return &defaultValueError{
Message: fmt.Sprintf(message, args...),
}
}

// NewValidationFailedError creates a new ValidationFailedError with a formatted message.
func NewValidationFailedError(message string, args ...any) error {
return &ValidationFailedError{
return &validationFailedError{
Message: fmt.Sprintf(message, args...),
}
}
29 changes: 12 additions & 17 deletions adapters/ortbbidder/resolver/errortypes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,53 +8,48 @@ import (

func TestValidationFailedError(t *testing.T) {
t.Run("validationFailedError", func(t *testing.T) {
err := ValidationFailedError{Message: "any validation message"}
err := validationFailedError{Message: "any validation message"}
assert.Equal(t, "any validation message", err.Error())
assert.Equal(t, SeverityWarning, err.Severity())
assert.Equal(t, severityWarning, err.Severity())
})
}

func TestDefaultValueError(t *testing.T) {
t.Run("defaultValueError", func(t *testing.T) {
err := DefaultValueError{Message: "any validation message"}
err := defaultValueError{Message: "any validation message"}
assert.Equal(t, "any validation message", err.Error())
assert.Equal(t, SeverityDebug, err.Severity())
assert.Equal(t, severityIgnore, err.Severity())
})
}

func TestContainsWarning(t *testing.T) {
func TestIsWarning(t *testing.T) {
type args struct {
errors []error
err error
}
tests := []struct {
name string
args args
want bool
}{
{
name: "error list contains warning",
name: "input err is of severity warning",
args: args{
errors: []error{
NewDefaultValueError("default value error"),
NewValidationFailedError("validation failed"),
},
err: NewValidationFailedError("error"),
},
want: true,
},
{
name: "error list not contains warning",
name: "input err is of severity ignore",
args: args{
errors: []error{
NewDefaultValueError("default value error"),
},
err: NewDefaultValueError("error"),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ContainsWarning(tt.args.errors); got != tt.want {
t.Errorf("ContainsWarning() = %v, want %v", got, tt.want)
if got := IsWarning(tt.args.err); got != tt.want {
t.Errorf("IsWarning() = %v, want %v", got, tt.want)
}
})
}
Expand Down
30 changes: 24 additions & 6 deletions adapters/ortbbidder/response_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ortbbidder
import (
"encoding/json"

"github.com/buger/jsonparser"
"github.com/prebid/openrtb/v20/openrtb2"
"github.com/prebid/prebid-server/v2/adapters"
"github.com/prebid/prebid-server/v2/adapters/ortbbidder/bidderparams"
Expand All @@ -16,12 +17,18 @@ type responseBuilder struct {
adapterRespone map[string]any // Response in the prebid format.
responseParams map[string]bidderparams.BidderParamMapper // Bidder response parameters.
request *openrtb2.BidRequest // Bid request.
isDebugEnabled bool // flag to determine if requestExt.prebid.debug is enabled.
}

func newResponseBuilder(responseParams map[string]bidderparams.BidderParamMapper, request *openrtb2.BidRequest) *responseBuilder {
var isDebugEnabled bool
if request != nil {
isDebugEnabled, _ = jsonparser.GetBoolean(request.Ext, "prebid", "debug")
}
return &responseBuilder{
responseParams: responseParams,
request: request,
isDebugEnabled: isDebugEnabled,
}
}

Expand All @@ -43,9 +50,7 @@ func (rb *responseBuilder) setPrebidBidderResponse(bidderResponseBytes json.RawM
for _, param := range resolver.ResponseParams {
bidderParam := rb.responseParams[param.String()]
resolverErrors := paramResolver.Resolve(rb.bidderResponse, adapterResponse, bidderParam.Location, param)
if resolver.ContainsWarning(resolverErrors) {
errs = append(errs, util.NewWarning("failed to set the [%s]", param.String()))
}
errs = collectWarningMessages(errs, resolverErrors, param.String(), rb.isDebugEnabled)
}
// Extract the seat bids from the bidder response.
seatBids, ok := rb.bidderResponse[seatBidKey].([]any)
Expand Down Expand Up @@ -77,9 +82,7 @@ func (rb *responseBuilder) setPrebidBidderResponse(bidderResponseBytes json.RawM
paramMapper := rb.responseParams[param.String()]
location := util.ReplaceLocationMacro(paramMapper.Location, []int{seatIndex, bidIndex})
resolverErrors := paramResolver.Resolve(bid, typedBid, location, param)
if resolver.ContainsWarning(resolverErrors) {
errs = append(errs, util.NewWarning("failed to set the [%s]", param.String()))
}
errs = collectWarningMessages(errs, resolverErrors, param.String(), rb.isDebugEnabled)
}
// Add the type bid to the list of typed bids.
typedBids = append(typedBids, typedBid)
Expand Down Expand Up @@ -107,3 +110,18 @@ func (rb *responseBuilder) buildAdapterResponse() (resp *adapters.BidderResponse
}
return
}

// collectWarningMessages appends warning messages from resolverErrors to the errs slice.
// If debugging is disabled, it appends a generic warning message and returns immediately.
func collectWarningMessages(errs, resolverErrors []error, parameter string, isDebugEnabled bool) []error {
for _, err := range resolverErrors {
if resolver.IsWarning(err) {
if !isDebugEnabled {
errs = append(errs, util.NewWarning("Potential issue encountered while setting the response parameter [%s]", parameter))
return errs
}
errs = append(errs, util.NewWarning(err.Error()))
}
}
return errs
}
82 changes: 78 additions & 4 deletions adapters/ortbbidder/response_builder_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ortbbidder

import (
"errors"
"reflect"
"testing"

Expand Down Expand Up @@ -186,7 +187,7 @@ func TestSetPrebidBidderResponse(t *testing.T) {
name: "Valid bidder respone, no bidder params",
bidderResponseBytes: []byte(`{"id":"bid-resp-id","cur":"USD","seatbid":[{"seat":"test_bidder","bid":[{"id":"123"}]}]}`),
responseParams: map[string]bidderparams.BidderParamMapper{},
expectedError: []error{util.NewWarning("failed to set the [bidType]")},
expectedError: []error{util.NewWarning("Potential issue encountered while setting the response parameter [bidType]")},
expectedResponse: map[string]any{
"Currency": "USD",
"Bids": []any{
Expand Down Expand Up @@ -255,8 +256,8 @@ func TestSetPrebidBidderResponse(t *testing.T) {
},
},
expectedError: []error{
util.NewWarning("failed to set the [fledgeAuctionConfig]"),
util.NewWarning("failed to set the [bidType]"),
util.NewWarning("Potential issue encountered while setting the response parameter [fledgeAuctionConfig]"),
util.NewWarning("Potential issue encountered while setting the response parameter [bidType]"),
},
expectedResponse: map[string]any{
"Currency": "USD",
Expand Down Expand Up @@ -285,7 +286,7 @@ func TestSetPrebidBidderResponse(t *testing.T) {
"bidMetaAdvertiserId": {Location: "seatbid.#.bid.#.ext.advertiserId"},
"bidMetaNetworkId": {Location: "seatbid.#.bid.#.ext.networkId"},
},
expectedError: []error{util.NewWarning("failed to set the [bidMetaAdvertiserId]")},
expectedError: []error{util.NewWarning("Potential issue encountered while setting the response parameter [bidMetaAdvertiserId]")},
expectedResponse: map[string]any{
"Currency": "USD",
"Bids": []any{
Expand Down Expand Up @@ -370,3 +371,76 @@ func TestBidderResponseFields(t *testing.T) {
t.Error(err)
}
}

func TestCollectWarningMessages(t *testing.T) {
type args struct {
errs []error
resolverErrors []error
parameter string
isDebugEnabled bool
}
tests := []struct {
name string
args args
want []error
}{
{
name: "No resolver errors",
args: args{
errs: []error{},
resolverErrors: []error{},
parameter: "param1",
isDebugEnabled: false,
},
want: []error{},
},
{
name: "Resolver errors with warnings and debugging enabled",
args: args{
errs: []error{},
resolverErrors: []error{
resolver.NewValidationFailedError("Warning 1"),
resolver.NewValidationFailedError("Warning 2"),
},
parameter: "param2",
isDebugEnabled: true,
},
want: []error{
util.NewWarning("Warning 1"),
util.NewWarning("Warning 2"),
},
},
{
name: "Resolver errors with warnings and debugging disabled",
args: args{
errs: []error{},
resolverErrors: []error{
resolver.NewValidationFailedError("Warning 1"),
},
parameter: "param3",
isDebugEnabled: false,
},
want: []error{
util.NewWarning("Potential issue encountered while setting the response parameter [param3]"),
},
},
{
name: "Resolver errors without warnings",
args: args{
errs: []error{},
resolverErrors: []error{
errors.New("Non-warning error"),
},
parameter: "param4",
isDebugEnabled: false,
},
want: []error{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := collectWarningMessages(tt.args.errs, tt.args.resolverErrors, tt.args.parameter, tt.args.isDebugEnabled)
assert.Equal(t, tt.want, got)
})
}
}

0 comments on commit 6359c8c

Please sign in to comment.