Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inherit bidder params from parent for an alias #3048

Merged
merged 7 commits into from
Aug 31, 2023
Merged

Conversation

gargcreation1992
Copy link
Contributor

Currently, we have adapter aliases that defines Bidder params explicitly using json file. However, after the adapter aliases feature, we will have the bidder params inherited from its parent adapter.
To accomplish this, we had to do some refactoring and remove the unnecessary code. We also refactored NewBidderParamsValidator function so that we can write the test cases.

We no longer require to maintain aliasBidderNames in separate slice. Instead, we will be needing aliasBidderToParentBidder map to maintain so that we can use it to get parent schema in later commit.
This refactor makes it easier to test the NewBidderParamsValidator function. Also adds the test cases.
@onkarvhanumante onkarvhanumante self-assigned this Aug 23, 2023
Comment on lines 597 to 602
var paramsValidator paramsValidatorHelper = paramsHelper{}

// NewBidderParamsValidator makes a BidderParamValidator, assuming all the necessary files exist in the filesystem.
// This will error if, for example, a Bidder gets added but no JSON schema is written for them.
func NewBidderParamsValidator(schemaDirectory string) (BidderParamValidator, error) {
fileInfos, err := os.ReadDir(schemaDirectory)
fileInfos, err := paramsValidator.readDir(schemaDirectory)
Copy link
Contributor

@onkarvhanumante onkarvhanumante Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of declaring paramsValidator as package/global var. Update NewBidderParamsValidator to take paramsValidatorHelper interface as input.

Callers of NewBidderParamsValidator can then pass paramsValidator or mockParamsHelper as implementor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in that case we would need to make paramsValidatorHelper interface and paramsHelper struct public since caller of the NewBidderParamsValidator function is outside this package. IMO, caller should not worry about the implementation details of NewBidderParamsValidator function and should be handled within the package itself.

Copy link
Contributor

@onkarvhanumante onkarvhanumante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few suggestions

Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Mostly just nitpicks.

openrtb_ext/bidders.go Show resolved Hide resolved
openrtb_ext/bidders.go Outdated Show resolved Hide resolved
openrtb_ext/bidders.go Outdated Show resolved Hide resolved
openrtb_ext/bidders.go Outdated Show resolved Hide resolved
openrtb_ext/bidders.go Outdated Show resolved Hide resolved
openrtb_ext/bidders.go Show resolved Hide resolved
openrtb_ext/bidders_test.go Outdated Show resolved Hide resolved
@Sonali-More-Xandr Sonali-More-Xandr self-assigned this Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants