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

Add modifying of VAST for video bids and add validation #1081

Merged
merged 13 commits into from
Feb 23, 2021

Conversation

DGarbar
Copy link
Contributor

@DGarbar DGarbar commented Dec 18, 2020

  • Add validation for Video bids. bid.adm or bid.nurl needs to be present
    • This case now, is not possible <VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>
  • Bid adm will be updated as cache. (see PBS video impression tracking prebid-server#1015)
  • Bid type is defined by bidder, not our (imp.video != null, etc) checks. For example Appnexus use bid.ext.appnexus.bid_ad_type to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.

  • Removed confusing maps
  • Removed confusing checks
  • Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)

  • Extract more event URL to another class
  • Use bidInfo in BidResponseCreator for BidResponse

- Add validation for Video bids. `bid.adm` or `bid.nurl` needs to be present
  - This case now, is not possible `<VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>`
- Bid adm will be updated as cache. (see prebid/prebid-server#1015)
- Bid type is defined by bidder, not our (`imp.video != null`, etc) checks. For example Appnexus use `bid.ext.appnexus.bid_ad_type` to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.
- Removed confusing maps
- Removed confusing checks
- Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)
- Extract more event URL to another class
- Use bidInfo in BidResponseCreator for BidResponse
@DGarbar DGarbar requested a review from rpanchyk December 18, 2020 16:30
…oring

# Conflicts:
#	src/main/java/org/prebid/server/validation/ResponseBidValidator.java
#	src/test/java/org/prebid/server/validation/ResponseBidValidatorTest.java
@DGarbar DGarbar requested a review from schernysh December 21, 2020 10:40
@rpanchyk rpanchyk merged commit c172af7 into master Feb 23, 2021
@rpanchyk rpanchyk deleted the bid-adm-vast-refactoring branch February 23, 2021 12:29
nickluck9 pushed a commit that referenced this pull request Aug 9, 2021
* Add modifying of VAST for video bids and add validation

- Add validation for Video bids. `bid.adm` or `bid.nurl` needs to be present
  - This case now, is not possible `<VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>`
- Bid adm will be updated as cache. (see prebid/prebid-server#1015)
- Bid type is defined by bidder, not our (`imp.video != null`, etc) checks. For example Appnexus use `bid.ext.appnexus.bid_ad_type` to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.
- Removed confusing maps
- Removed confusing checks
- Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)
- Extract more event URL to another class
- Use bidInfo in BidResponseCreator for BidResponse

* Merged master and fixed corresponding imp can't be null

* Fix after review. (without changing localhost/event)

* Use placeholders in test resources instead of concrete urls where possible (#1084)

* Fix tests and remove ordering

* Replace usage of Wiremock EqualToJsonPattern with custom implementation to prevent incorrect json comparison

* fix openx

* Use equalToBidCacheRequest for consistency and add openrtbCacheDebugComparator() when debug is used (fix flaky tests)

* Remove redundant code

Co-authored-by: Sergii Chernysh <schernysh@users.noreply.github.com>
Co-authored-by: rpanchyk <rpanchyk@rubiconproject.com>
nickluck9 pushed a commit that referenced this pull request Aug 10, 2021
* Add modifying of VAST for video bids and add validation

- Add validation for Video bids. `bid.adm` or `bid.nurl` needs to be present
  - This case now, is not possible `<VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>`
- Bid adm will be updated as cache. (see prebid/prebid-server#1015)
- Bid type is defined by bidder, not our (`imp.video != null`, etc) checks. For example Appnexus use `bid.ext.appnexus.bid_ad_type` to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.
- Removed confusing maps
- Removed confusing checks
- Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)
- Extract more event URL to another class
- Use bidInfo in BidResponseCreator for BidResponse

* Merged master and fixed corresponding imp can't be null

* Fix after review. (without changing localhost/event)

* Use placeholders in test resources instead of concrete urls where possible (#1084)

* Fix tests and remove ordering

* Replace usage of Wiremock EqualToJsonPattern with custom implementation to prevent incorrect json comparison

* fix openx

* Use equalToBidCacheRequest for consistency and add openrtbCacheDebugComparator() when debug is used (fix flaky tests)

* Remove redundant code

Co-authored-by: Sergii Chernysh <schernysh@users.noreply.github.com>
Co-authored-by: rpanchyk <rpanchyk@rubiconproject.com>
nickluck9 pushed a commit that referenced this pull request Aug 10, 2021
* Add modifying of VAST for video bids and add validation

- Add validation for Video bids. `bid.adm` or `bid.nurl` needs to be present
  - This case now, is not possible `<VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>`
- Bid adm will be updated as cache. (see prebid/prebid-server#1015)
- Bid type is defined by bidder, not our (`imp.video != null`, etc) checks. For example Appnexus use `bid.ext.appnexus.bid_ad_type` to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.
- Removed confusing maps
- Removed confusing checks
- Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)
- Extract more event URL to another class
- Use bidInfo in BidResponseCreator for BidResponse

* Merged master and fixed corresponding imp can't be null

* Fix after review. (without changing localhost/event)

* Use placeholders in test resources instead of concrete urls where possible (#1084)

* Fix tests and remove ordering

* Replace usage of Wiremock EqualToJsonPattern with custom implementation to prevent incorrect json comparison

* fix openx

* Use equalToBidCacheRequest for consistency and add openrtbCacheDebugComparator() when debug is used (fix flaky tests)

* Remove redundant code

Co-authored-by: Sergii Chernysh <schernysh@users.noreply.github.com>
Co-authored-by: rpanchyk <rpanchyk@rubiconproject.com>
SerhiiNahornyi pushed a commit that referenced this pull request Nov 30, 2023
Co-authored-by: Dubyk Danylo <pointbay2001@gmail.com>
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.

3 participants