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

Update TargetingInfo Behavior #1088

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions PrebidMobile/AdUnits/AdUnit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,28 @@ public class AdUnit: NSObject, DispatcherDelegate {
})
}

private func setUp(_ adObject: AnyObject?, with bidResponse: BidResponse) -> ResultCode {
func setUp(_ adObject: AnyObject?, with bidResponse: BidResponse) -> ResultCode {

if Targeting.shared.forceSdkToChooseWinner {
Log.error("Breaking change: set Targeting.forceSdkToChooseWinner = false and test your behavior. In the upcoming major release, the SDK will send all targeting keywords to the AdSever, so you should prepare your setup.")
}

guard let winningBid = bidResponse.winningBid else {

//When the new behavior is active
if !Targeting.shared.forceSdkToChooseWinner {

if let adObject {
Utils.shared.validateAndAttachKeywords(adObject: adObject, bidResponse: bidResponse)
}
//If there are no winning bids, but there are bids the SDK will send back prebidDemandFetchSuccess
if let bids = bidResponse.allBids, !bids.isEmpty {
return .prebidDemandFetchSuccess
}
}
return .prebidDemandNoBids
}

if let cacheId = cacheBidIfNeeded(winningBid) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are missing the addition of hb_cache_id_local (cacheId) to the adObject, which is crucial for certain types of ads (e.g., native in-app).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It currently uses winning bid, this is the case when there are no winning bids.

  1. Would this be a valid case ?
  2. Which bids should we use in case of multiple bids ?

Copy link
Collaborator

@OlenaPostindustria OlenaPostindustria Jan 23, 2025

Choose a reason for hiding this comment

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

In this case, the winningBid is the bid intended for display. The SDK should cache such a bid and attach the cacheID to the adObject as hb_cache_id_local. Otherwise, it will break the existing functionality. The SDK should maintain the default behaivior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the updates! I’d also suggest adding some unit tests to ensure this functionality doesn’t break in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added 2 tests that target

  1. the new behavior + winning bid
  2. the new behavior + losing bid

Do you mean more tests for hb_cache_id_local ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 2 other tests target the old behavior

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I mean more tests for hb_cache_id_local

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added 2 cases for native format, please can you verify those. I'm not that versed in the native format from Prebid yet

bidResponse.addTargetingInfoValue(key: PrebidLocalCacheIdKey, value: cacheId)
}
Expand Down
3 changes: 3 additions & 0 deletions PrebidMobile/ConfigurationAndTargeting/Targeting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ public class Targeting: NSObject {
UserConsentDataManager.shared.isAllowedAccessDeviceData()
}

/// This value forces SDK to choose targeting info of the winning bid
public var forceSdkToChooseWinner : Bool = true

// MARK: - External User Ids

/// Array of external user IDs.
Expand Down
95 changes: 95 additions & 0 deletions PrebidMobileTests/AdUnitTests/AdUnitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ class AdUnitTests: XCTestCase {

override func tearDown() {
Targeting.shared.clearUserKeywords()
Targeting.shared.forceSdkToChooseWinner = true

Prebid.shared.useExternalClickthroughBrowser = false
Prebid.shared.useCacheForReportingWithRenderingAPI = false
}

func testFetchDemand() {
Expand Down Expand Up @@ -114,6 +117,98 @@ class AdUnitTests: XCTestCase {
XCTAssertEqual(realBidInfo?.nativeAdCacheId, expectedCacheId)
}

//forceSdkToChooseWinner + Winner = Contains Targeting Info
func testForcedWinnerAndWinningBid() {
//given
Targeting.shared.forceSdkToChooseWinner = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please reset all global values to their default settings after the test completes (e.g. in tearDown) to prevent any unexpected behaviour.


let expected = ResultCode.prebidDemandFetchSuccess

let adUnit = AdUnit(configId: "138c4d03-0efb-4498-9dc6-cb5a9acb2ea4", size: CGSize(width: 300, height: 250), adFormats: [.banner])
//This needs to after AdUnit init as the AdUnit enables this value.
//We need to disabled to not look for cache id for winning bid
Prebid.shared.useCacheForReportingWithRenderingAPI = false
let adObject = NSMutableDictionary()
let rawWinningBid = PBMBidResponseTransformer.makeValidResponse(bidPrice: 0.75)
let jsonDict = rawWinningBid.jsonDict as? NSDictionary
let bidResponse = BidResponse(jsonDictionary: jsonDict ?? [:])

//when
let resultCode = adUnit.setUp(adObject, with: bidResponse)

//then
XCTAssertTrue((adObject.allKeys as? [String])?.contains("hb_bidder") ?? false)
XCTAssertEqual(resultCode, expected)
}

//forceSdkToChooseWinner + No Winner = Doesn't contain Targeting Info
func testForcedWinnerAndLoosingBid() {
//given
Targeting.shared.forceSdkToChooseWinner = true
let expected = ResultCode.prebidDemandNoBids
let adUnit = AdUnit(configId: "138c4d03-0efb-4498-9dc6-cb5a9acb2ea4", size: CGSize(width: 300, height: 250), adFormats: [.banner])
//This needs to after AdUnit init as the AdUnit enables this value.
//We need to disabled to not look for cache id for winning bid
Prebid.shared.useCacheForReportingWithRenderingAPI = false
let adObject = NSMutableDictionary()
let rawWinningBid = PBMBidResponseTransformer.makeValidResponseWithNonWinningTargetingInfo()
let jsonDict = rawWinningBid.jsonDict as? NSDictionary
let bidResponse = BidResponse(jsonDictionary: jsonDict ?? [:])

//when
let resultCode = adUnit.setUp(adObject, with: bidResponse)

//then
XCTAssertFalse((adObject.allKeys as? [String])?.contains("hb_bidder") ?? false)
XCTAssertEqual(resultCode, expected)
}

//Don't forceSdkToChooseWinner + Winner = Contains Targeting Info
func testNonForcedWinnerAndWinningBid() {
//given
Targeting.shared.forceSdkToChooseWinner = false
let expected = ResultCode.prebidDemandFetchSuccess
let adUnit = AdUnit(configId: "138c4d03-0efb-4498-9dc6-cb5a9acb2ea4", size: CGSize(width: 300, height: 250), adFormats: [.banner])
//This needs to after AdUnit init as the AdUnit enables this value.
//We need to disabled to not look for cache id for winning bid
Prebid.shared.useCacheForReportingWithRenderingAPI = false
let adObject = NSMutableDictionary()
let rawWinningBid = PBMBidResponseTransformer.makeValidResponse(bidPrice: 0.75)
let jsonDict = rawWinningBid.jsonDict as? NSDictionary
let bidResponse = BidResponse(jsonDictionary: jsonDict ?? [:])

//when
let resultCode = adUnit.setUp(adObject, with: bidResponse)

//then
XCTAssertTrue((adObject.allKeys as? [String])?.contains("hb_bidder") ?? false)
XCTAssertEqual(resultCode, expected)
}

//Don't forceSdkToChooseWinner + No Winner = Contains Targeting Info
func testNonForcedWinnerAndNonWinningBid() {
//given
Targeting.shared.forceSdkToChooseWinner = false

let expected = ResultCode.prebidDemandFetchSuccess
let adUnit = AdUnit(configId: "138c4d03-0efb-4498-9dc6-cb5a9acb2ea4", size: CGSize(width: 300, height: 250), adFormats: [.banner])
//This needs to after AdUnit init as the AdUnit enables this value.
//We need to disabled to not look for cache id for winning bid
Prebid.shared.useCacheForReportingWithRenderingAPI = false
let adObject = NSMutableDictionary()
let rawWinningBid = PBMBidResponseTransformer.makeValidResponseWithNonWinningTargetingInfo()
let jsonDict = rawWinningBid.jsonDict as? NSDictionary
let bidResponse = BidResponse(jsonDictionary: jsonDict ?? [:])

//when
let resultCode = adUnit.setUp(adObject, with: bidResponse)

//then
XCTAssertTrue((adObject.allKeys as? [String])?.contains("hb_bidder") ?? false)
XCTAssertEqual(adObject["hb_bidder"] as? String, "Test-Bidder-1")
XCTAssertEqual(resultCode, expected)
}

func testBidInfoCompletion() {
Prebid.shared.prebidServerAccountId = "test-account-id"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ extension PBMBidResponseTransformer {
return buildResponse("{\"id\":\"B4A2D3F4-41B6-4D37-B68B-EE8893E85C31\",\"seatbid\":[\(winningBidFragment)],\"cur\":\"USD\",\"ext\":{\"responsetimemillis\":{\"openx\":62}}}")
}

static func makeValidResponseWithNonWinningTargetingInfo() -> PrebidServerResponse {
let winningBidFragment = "{\n \"bid\": [\n {\n \"id\": \"test_id_bid\",\n \"impid\": \"test_imp\",\n \"price\": 0,\n \"ext\": {\n \"origbidcpm\": 0,\n \"prebid\": {\n \"meta\": {},\n \"targeting\": {\n \"hb_bidder\": \"Test-Bidder-1\",\n \"hb_source\": \"s2s\",\n }\n }\n }\n }\n ],\n \"seat\": \"Test-Bidder-1\"\n}"
return buildResponse("{\"id\":\"B4A2D3F4-41B6-4D37-B68B-EE8893E85C31\",\"seatbid\":[\(winningBidFragment)],\"cur\":\"USD\",\"ext\":{\"responsetimemillis\":{\"openx\":62}}}")
}

static func makeValidResponseWithCTF(bidPrice: Float, ctfBanner: Double, ctfPreRender: Double) -> PrebidServerResponse {
let winningBidFragment = "{\"bid\":[{\"id\":\"test-bid-id-1\",\"impid\":\"8BBB0D42-5A73-45AC-B275-51B299A74C32\",\"price\":\(bidPrice),\"adm\":\"<html><div>You Won! This is a test bid</div></html>\",\"adid\":\"test-ad-id-12345\",\"adomain\":[\"openx.com\"],\"crid\":\"test-creative-id-1\",\"w\":300,\"h\":250,\"ext\":{\"prebid\":{\"targeting\":{\"hb_bidder\":\"openx\",\"hb_bidder_openx\":\"openx\",\"hb_env\":\"mobile-app\",\"hb_env_openx\":\"mobile-app\",\"hb_pb\":\"0.10\",\"hb_pb_openx\":\"0.10\",\"hb_size\":\"300x250\",\"hb_size_openx\":\"300x250\"},\"type\":\"banner\"},\"bidder\":{\"ad_ox_cats\":[2],\"agency_id\":\"agency_10\",\"brand_id\":\"brand_10\",\"buyer_id\":\"buyer_10\",\"matching_ad_id\":{\"campaign_id\":1,\"creative_id\":3,\"placement_id\":2},\"next_highest_bid_price\":0.099}}}],\"seat\":\"openx\"}"
return buildResponse("{\"id\":\"B4A2D3F4-41B6-4D37-B68B-EE8893E85C31\",\"seatbid\":[\(winningBidFragment)],\"cur\":\"USD\",\"ext\":{\"responsetimemillis\":{\"openx\":62},\"prebid\":{\"passthrough\":[{\"type\":\"prebidmobilesdk\", \"sdkconfiguration\":{\"cftbanner\":\(ctfBanner),\"cftprerender\":\(ctfPreRender)}}]}}}")
Expand Down
12 changes: 12 additions & 0 deletions PrebidMobileTests/TargetingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class TargetingTests: XCTestCase {

override func tearDown() {
UtilitiesForTesting.resetTargeting(.shared)
Targeting.shared.forceSdkToChooseWinner = true
}

func testDomain() {
Expand Down Expand Up @@ -122,6 +123,17 @@ class TargetingTests: XCTestCase {
XCTAssertEqual(locationPrecision, Targeting.shared.locationPrecision)
}

func testforceSdkToChooseWinner() {
//given
let forceSdkToChooseWinner = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think false would be more suitable here, as Targeting.shared.forceSdkToChooseWinner is true by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated


//when
Targeting.shared.forceSdkToChooseWinner = forceSdkToChooseWinner

//then
XCTAssertEqual(forceSdkToChooseWinner, Targeting.shared.forceSdkToChooseWinner)
}

// MARK: - Year Of Birth
func testYearOfBirth() {
//given
Expand Down