Update TargetingInfo Behavior #1088

ankit-thanekar007 · 2025-01-16T15:29:07Z

Add a flag that allows devs to use this feature.
If the flag is set, the targeting info will be added to adObject regardless of bid status
If the flag is not set, it'll retain the current behavior
Add tests

@YuriyVelichkoPI : After our discussion, do we need the flag or should this be the default behavior ?

Fixes Issue : #1087

mdanylov-sigma · 2025-01-17T16:18:09Z

PrebidMobile/AdUnits/AdUnit.swift

@@ -187,6 +187,11 @@ public class AdUnit: NSObject, DispatcherDelegate {
    }

    private func setUp(_ adObject: AnyObject?, with bidResponse: BidResponse) -> ResultCode {
+
+        if Prebid.shared.forceTargetingInfo, let adObject {


Looks like the provided solution will break the existing behaviour.
In case forceTargetingInfo == false, which is default behaviour, bidResponse will never be attached for winning bid.

Yeah, thanks for catching that. I removed the flag altogether and by default it'll always add targeting params.

YuriyVelichkoPI · 2025-01-20T08:15:28Z

PrebidMobile/AdUnits/AdUnit.swift

@@ -187,6 +187,11 @@ public class AdUnit: NSObject, DispatcherDelegate {
    }

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


In order to make the solution generic and not break the current behavior, we should implement it according to the following requirements:

The current behavior should remain the same. It means that for the current users SKD should continue to add the targeting keywords only in the case when the bid response has a winning bid.

We should inform users that in the future (3.0 release), this behavior will be changed. As usual, we do it via deprecating the method or property, so the app developers see the compile time warning. But in this case, I don't see such an option. Therefore, we should at least log some error message that the current behavior will be changed soon. Also we should put a respective warning note to the public documentation.

We should add the flag to like Targeting.forceSdkChooseWinner which will allow adding targeting keywords regardless of the presence of the winning bid. The default value should be true. That was partially done in the initial commit, but it should still be adjusted according to the review comment. If forceSdkChooseWinner is true, the keywords should be sent only if the winning bid is present (the current behavior). If forceSdkChooseWinner is false, the keywords should be sent regarding the winning bid.

And the most important. If we start sending info from all bids - SDK shouldn't return .prebidDemandNoBids if there are bids in the response, regardless of the presence of the winning bid.

And, of course, we should add unit tests for the new behavior. Also, please ensure that we have unit tests that verify that the BidResposne.targetingInfo really contains targeting keywords from all bids in the response.

Hey @YuriyVelichkoPI , For 4

It's possible to not have a winning bid as it doesn't have hb_pb value. It's just targeting data.

In that case we'll be passing the targeting data to GAM, it's not a bid or a winning bid. What should we send in return ?

I think 4 should be part of the 3.x release as it's scope is much bigger.

I've made changes for 1,3 and 5.
Can you let me know the msg for 2 ? Should it use Log.warn ?

Hi @ankit-thanekar007 !

About the item 4: we already have a good ResultCode for it - .prebidDemandFetchSuccess The method that publishers use is fetchDemand. The targeting info is placed in bid.ext.prebid.targeting. Hence we can legally send the .prebidDemandFetchSuccess to the publisher because there are bids in the response. The .prebidDemandNoBids should indicate real 0 bids in the response. It's a common Prebid behavior.

About the item 2:

First, let's log the error message - Log.error. We should make it as verbose as possible to force publishers to adapt the upcoming changes.

Let's log an error if forceSdkToChooseWinner is true, regardless the response content.

The message: `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."

The warning message should be in the SDK's doc.

OlenaPostindustria · 2025-01-22T08:49:48Z

PrebidMobile/AdUnits/AdUnit.swift

+            //If Bid Present and the bid is winning
+            //OR
+            //If forceSdkChooseWinner == false : Any bid works -- winnning or loosing
+            if bidResponse.winningBid?.isWinning == true || !Targeting.shared.forceSdkToChooseWinner {


I think it would be more appropriate to verify that bidResponse.winningBid != nil to make things clearer. There's no need to verify if winningBid is already isWinning, as it inherently is if present.

OlenaPostindustria · 2025-01-22T09:06:42Z

PrebidMobileTests/AdUnitTests/AdUnitTests.swift

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


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

ankit-thanekar007 · 2025-01-22T17:56:42Z

@YuriyVelichkoPI @OlenaPostindustria Updated the PR with the fixes for comments + 1 test case fix. Please verify.

OlenaPostindustria · 2025-01-23T08:50:59Z

Example/PrebidDemo/PrebidDemoSwiftUITests/NativeAdsTest.swift

@@ -34,6 +34,6 @@ class NativeAdsTest: BaseAdsTest {
    }

    override func checkAd(testCase: String) {
-        XCTAssert(app.staticTexts["Prebid (Title)"].waitForExistence(timeout: 10),assertFailedMessage(testCase: testCase, reason: "Prebid title is not displayed"))
+        XCTAssert(app.staticTexts[testCase].waitForExistence(timeout: 10),assertFailedMessage(testCase: testCase, reason: "Prebid title is not displayed"))


Why is it necessary to modify the test conditions? testCase text is always visible on the screen since it’s not part of an ad, so it is not a valid indicator for detecting ad presence.

During this flow app.staticTexts never have "Prebid (Title)" they have the testCase strings. The tests were failing because it could never find "Prebid (Title)" inside app.staticTexts.

These started failing since Yesterday.

This occurred due to the hb_cache_id_local key missing issue causing the test case to stop working.

OlenaPostindustria · 2025-01-23T08:55:04Z

PrebidMobile/AdUnits/AdUnit.swift

            return .prebidDemandNoBids
        }
-        
+
        if let cacheId = cacheBidIfNeeded(winningBid) {


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).

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

Would this be a valid case ?

Which bids should we use in case of multiple bids ?

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.

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

I've added 2 tests that target

the new behavior + winning bid

the new behavior + losing bid

Do you mean more tests for hb_cache_id_local ?

The 2 other tests target the old behavior

Yes, I mean more tests for hb_cache_id_local

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

OlenaPostindustria · 2025-01-24T14:24:14Z

PrebidMobileTests/TargetingTests.swift

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

+    func testforceSdkToChooseWinner() {
+        //given
+        let forceSdkToChooseWinner = true


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

OlenaPostindustria · 2025-01-24T15:13:11Z

PrebidMobileTests/AdUnitTests/AdUnitTests.swift

+
+        //then
+        XCTAssertNotNil(bidResponse.targetingInfo?[PrebidLocalCacheIdKey])
+        XCTAssertTrue((adObject.allKeys as? [String])?.contains("hb_bidder") ?? false)


I believe it would be helpful to check if the hb_cache_id_local key is attached to the ad object. This applies to both native tests.

OlenaPostindustria

LGTM!

ankit-thanekar007 · 2025-01-24T15:56:41Z

@YuriyVelichkoPI Can you review and let me know in case of any changes ?

ankit-thanekar007 changed the title ~~Add Targeting option~~ Update TargetingInfo Behavior Jan 16, 2025

mdanylov-sigma reviewed Jan 17, 2025

View reviewed changes

ankit-thanekar007 requested a review from mdanylov-sigma January 17, 2025 16:54

YuriyVelichkoPI requested review from OlenaPostindustria and removed request for mdanylov-sigma January 20, 2025 07:51

YuriyVelichkoPI requested changes Jan 20, 2025

View reviewed changes

ankit-thanekar007 requested a review from YuriyVelichkoPI January 21, 2025 18:13

OlenaPostindustria requested changes Jan 22, 2025

View reviewed changes

Ankit Rajendra Thanekar added 5 commits January 22, 2025 15:58

Add Targeting option

2da2426

Remove FF usage

61cacf7

PR Comments + Tests

ac8fcb7

PR Comments

531e162

Update tests

25067ec

ankit-thanekar007 force-pushed the gh_update-targeting-info branch from af52a47 to 25067ec Compare January 22, 2025 15:59

Ankit Rajendra Thanekar added 3 commits January 22, 2025 16:13

Remove cacheId value

da3528b

Reset values for test

611d197

Fix test for checking correct testcase

8150037

ankit-thanekar007 requested a review from OlenaPostindustria January 22, 2025 17:56

OlenaPostindustria requested changes Jan 23, 2025

View reviewed changes

In case of winning bid, targeting should have local-cache-id

9e3820c

ankit-thanekar007 closed this Jan 23, 2025

ankit-thanekar007 reopened this Jan 23, 2025

Refactor teardown

2f63f0c

OlenaPostindustria requested changes Jan 24, 2025

View reviewed changes

Ankit Rajendra Thanekar added 2 commits January 24, 2025 14:47

PR Update

67b6145

Add Native format case

dcd244b

OlenaPostindustria requested changes Jan 24, 2025

View reviewed changes

Check cacheid with adObject

8b5981f

OlenaPostindustria approved these changes Jan 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update TargetingInfo Behavior #1088

Update TargetingInfo Behavior #1088

ankit-thanekar007 commented Jan 16, 2025 •

edited

Loading

mdanylov-sigma Jan 17, 2025

ankit-thanekar007 Jan 17, 2025

YuriyVelichkoPI Jan 20, 2025

ankit-thanekar007 Jan 21, 2025

ankit-thanekar007 Jan 21, 2025

YuriyVelichkoPI Jan 22, 2025

OlenaPostindustria Jan 22, 2025

OlenaPostindustria Jan 22, 2025

ankit-thanekar007 commented Jan 22, 2025

OlenaPostindustria Jan 23, 2025

ankit-thanekar007 Jan 23, 2025

ankit-thanekar007 Jan 23, 2025

OlenaPostindustria Jan 23, 2025

OlenaPostindustria Jan 23, 2025

ankit-thanekar007 Jan 23, 2025

OlenaPostindustria Jan 23, 2025 •

edited

Loading

OlenaPostindustria Jan 24, 2025

ankit-thanekar007 Jan 24, 2025

ankit-thanekar007 Jan 24, 2025

OlenaPostindustria Jan 24, 2025

ankit-thanekar007 Jan 24, 2025

OlenaPostindustria Jan 24, 2025

ankit-thanekar007 Jan 24, 2025

OlenaPostindustria Jan 24, 2025 •

edited

Loading

ankit-thanekar007 Jan 24, 2025

OlenaPostindustria left a comment

ankit-thanekar007 commented Jan 24, 2025

Update TargetingInfo Behavior #1088

Are you sure you want to change the base?

Update TargetingInfo Behavior #1088

Conversation

ankit-thanekar007 commented Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankit-thanekar007 commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlenaPostindustria Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlenaPostindustria Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlenaPostindustria left a comment

Choose a reason for hiding this comment

ankit-thanekar007 commented Jan 24, 2025

ankit-thanekar007 commented Jan 16, 2025 •

edited

Loading

OlenaPostindustria Jan 23, 2025 •

edited

Loading

OlenaPostindustria Jan 24, 2025 •

edited

Loading