Skip to content

GMA Next-Gen SDK Support#925

Merged
mdanylov-sigma merged 36 commits intoprebid:masterfrom
postindustria-tech:next-gen-sdk-support
Mar 31, 2026
Merged

GMA Next-Gen SDK Support#925
mdanylov-sigma merged 36 commits intoprebid:masterfrom
postindustria-tech:next-gen-sdk-support

Conversation

@postindustria-code
Copy link
Copy Markdown
Collaborator

  • Created module nextEventHandlers with event handlers for banner, interstitial, rewarded ads with unit tests
  • Created a demo application for Prebid+Next-Gen SDK integration
  • Added unit and instrumented tests

- Add targeting and cache propagation to Next Gen SDK request objects
- Add banner 320x50 example to Next Gen SDK demo
- Rework check for Next Gen SDK classes
- Add Native Ad example to Next Gen SDK Demo App
- Fix Prebid Kotlin Demo app not building
- Change naming order
- Create layout for Next Gen SDK native layout
- Add event handlers for banner ads
- Add banner event handler example to demo app
- Add display interstitial event handler example to demo app
- Add display rewarded video event handler example to demo app
- Cleanup resources
- Add equals and hashCode methods to AdEvent
- Put main dispatcher in wrappers to the constructor for better testability
Comment thread PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/PrebidMobile.java Outdated
Comment thread PrebidMobile/PrebidMobile-nextEventHandlers/build.gradle Outdated
Copy link
Copy Markdown
Contributor

@YuriyVelichkoPI YuriyVelichkoPI left a comment

Choose a reason for hiding this comment

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

Please address the items from the following Claude review. All of them looks reasonable to me.

Code Review: next-gen-sdk-support branch


1. SDK Core (PrebidMobile-core)

Critical / Bugs

Util.java — Incomplete refactoring of apply() and propagateNativeParameters()

The apply() method was refactored to cache adObj.getClass() into adObjClass, but the substitution is incomplete — some branches were missed:

// apply() — second half of OR still calls adObj.getClass()
if (adObjClass == getClassFromString(AD_MANAGER_REQUEST_CLASS)
        || adObj.getClass() == getClassFromString(AD_MANAGER_REQUEST_CLASS_V20)) {
// same issue on the builder branch:
} else if (adObjClass == getClassFromString(AD_MANAGER_REQUEST_BUILDER_CLASS)
        || adObj.getClass() == getClassFromString(AD_MANAGER_REQUEST_BUILDER_CLASS_V20)) {

Same problem in propagateNativeParameters(). The double class-lookup is harmless (it's the same object) but it's noise that defeats the purpose of the refactor and will confuse readers. All four occurrences need to be updated to adObjClass.


PrebidDisplayView.javaonDetachedFromWindow() removal is a breaking lifecycle change

Removing the super.onDetachedFromWindow() → destroy() call means the view no longer self-cleans when detached from the window. This is a significant change: any caller that adds the view to a recycling container (RecyclerView, ViewPager) without explicitly calling destroy() will now leak. There is no unit test covering this new PrebidDestroyable path in DisplayView.destroy(). At minimum:

  • Add a test that verifies DisplayView.destroy() delegates to the inner view when it implements PrebidDestroyable.
  • Document in the PrebidDisplayView class why manual destroy() is now required.

Missing Unit Test Coverage

The following new code has no corresponding tests:

Code What to test
Util.isNextGenSdkRequestBuilderClass() Returns true for subclasses, false for unrelated classes, false when Next-Gen SDK not on classpath
Util.handleNextGenSdkTargeting() Calls putCustomTargeting for every entry in the bid map
Util.setCacheIdToNextGenSdk() Propagates cache ID; no-ops when cacheId is null
Util.supportedAdObject() / apply() Next-Gen builder is accepted; cache ID is set via reflection
AdViewUtils.findCacheId(null) Returns null without NPE (the null-guard added here) — trivial to add, protects from regression
DisplayView.destroy() delegating to PrebidDestroyable See lifecycle note above
AdUnitConfiguration.globalOrtbConfig getter/setter Basic round-trip test; verify it starts as null

The two BasicParameterBuilderTest tests added for globalOrtbConfig are good and thorough.


Minor

NativeAdUnit / VideoBaseAdUnit — removed getImpOrtbConfig()/setImpOrtbConfig() overrides
Fine as long as AdUnit already exposes those publicly (which it does). Just confirm the base-class methods are public, not protected, so callers on NativeAdUnit don't silently lose access.

Javadoc typo in AdUnit.java:setGlobalOrtbConfig() (copy-pasted into 4 classes):

"It takes precedence over Targeting.setGlobalOrtbConfig"

The correct class name is TargetingParams, not Targeting.

PrebidMobile.TESTED_GOOGLE_NEXT_GEN_SDK_VERSION — the constant is declared but there's no compile-time or runtime version check at SDK init (unlike the GAM SDK version check). Consider adding a version-warning log at init time following the same pattern.


2. Next-Gen Event Handlers (PrebidMobile-nextEventHandlers)

Critical

Missing Maven build and deploy support

This is the most important missing piece. The module is not included in either:

  • scripts/buildPrebidMobile.shmodules and projectPaths arrays
  • scripts/Maven/deployPrebidMobile.shmodules and extensions arrays
  • scripts/Maven/ — no PrebidMobile-nextEventHandlers-pom.xml exists

All other event handler modules (PrebidMobile-gamEventHandlers, PrebidMobile-admobAdapters, etc.) are covered. Without this, the module cannot be released to Maven Central. The pattern to follow is directly visible in the existing scripts — add the module to both arrays and create the POM by copying PrebidMobile-gamEventHandlers-pom.xml and updating the <artifactId>.


Package namespace collision with PrebidMobile-gamEventHandlers

Both modules declare namespace "org.prebid.mobile.eventhandlers" and place source in the same Java package. They share class names: AdEvent, Constants, package-info.java. If a developer ever includes both in one project, the build will fail with duplicate class errors.

At minimum, document clearly that these modules are mutually exclusive. Long-term, consider renaming the Next-Gen package to org.prebid.mobile.nextgeneventhandlers to make the distinction explicit.


Code Issues

AdViewWrapper.kt — hardcoded Dispatchers.Main in onAdLoaded

override fun onAdLoaded(ad: BannerAd) {
    ...
    CoroutineScope(Dispatchers.Main).launch { // ← should be mainDispatcher
        listener.onEvent(AdEvent.Loaded())
    }
}

All other callbacks in AdViewWrapper correctly use the injected mainDispatcher. This inconsistency breaks test isolation — the testDispatcher passed in the constructor is ignored for this specific callback.


NextRewardedEventHandler.kt — wrong error message in show()

listener?.onAdFailed(AdException(AdException.THIRD_PARTY, "GAM SDK - failed to display ad."))

Should say "Next-Gen SDK" (copy-paste from GamRewardedEventHandler).


RewardedAdWrapper.kt — wrong class KDoc

/** Internal wrapper of rewarded ad from GAM SDK. */

Should say "Next-Gen SDK".


NextInterstitialEventHandler.kt — unnecessary null check in initPublisherInterstitialAd()

if (requestInterstitial != null) {
    requestInterstitial = null
}

Can be simplified to just requestInterstitial = null.


Unscoped CoroutineScope in all three wrappers

Each callback creates CoroutineScope(mainDispatcher).launch { ... } — a new scope with no parent Job and no structured cancellation. For these short-lived callbacks it's not a practical leak, but it deviates from coroutine best practices. Consider holding a val scope = CoroutineScope(mainDispatcher + SupervisorJob()) per wrapper and cancelling it on destroy().


Test Issues

Copy-pasted test method names from GAM in NextRewardedEventHandlerTest and NextInterstitialEventHandlerTest:

  • onGamAdClosed_NotifyEventCloseListener
  • onGamAdFailedToLoad_NotifyEventErrorListener
  • onGamAdLoadedAppEventExpected_ScheduleAppEventHandler
  • etc.

These should be renamed to onNextAd* to match the banner test naming and avoid confusion.

Missing test coverage:

  • RewardedAdWrapper.metadataContainsAdEvent() — not directly tested; only the rewardedAd == null path is implicitly covered
  • Utils.handleCustomTargetingUpdate() — the empty-keywords early-return path is untested
  • InterstitialAdWrapperTest and RewardedAdWrapperTest — should follow the StandardTestDispatcher / advanceUntilIdle pattern used in AdViewWrapperTest to properly test coroutine dispatch

Minor

  • NextInterstitialEventHandler.kt and NextRewardedEventHandler.kt are missing the Apache 2.0 license header present in all other files in this module.

3. Demo App (PrebidNextGenDemo)

The app is clean and achieves its goal well. Integration code is readable and follows a consistent pattern.

General suggestions:

  • TAG in BaseAdActivity is hardcoded to "ExampleActivity" for all subclasses. Using javaClass.simpleName would make Logcat filtering per-activity useful without extra effort.

  • Error feedback — ad failures are only logged via Log.e. For a demo/integration testing app, surfacing errors in the UI (even a simple Toast or TextView) would speed up on-device debugging without ADB.

  • setOpenRtbConfig() — the "Optional" comment pattern is clear and good. Keep it consistent across all activities that add optional config.

  • Multiformat activities — a brief comment explaining which bid configuration drives which ad format would help new integrators understand the intent.

@YuriyVelichkoPI
Copy link
Copy Markdown
Contributor

@postindustria-code you should also prepare the docs PR describing the support of new SDK.
https://github.com/prebid/prebid.github.io

Copy link
Copy Markdown
Collaborator

@ValentinPostindustria ValentinPostindustria left a comment

Choose a reason for hiding this comment

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

We should create Maven XML files for releasing the SDK. As example.

Also if it's possible to target Next Gen SDK test ad, maybe we should create Rendering API examples where Next Gen SDK wins, but Prebid with no bids. It will allow to test the Next Gen SDK rendering.

@postindustria-code
Copy link
Copy Markdown
Collaborator Author

Updated with requested changes. I couldn't make noBids examples with our GAM ad unit to work, so I've used GAM test ad units.

@mdanylov-sigma
Copy link
Copy Markdown
Collaborator

Hey guys @YuriyVelichkoPI @ValentinPostindustria
Do you have capacity to finish the review? I wanna release these changes, as far as I remember your clients were interested in this integration

Copy link
Copy Markdown
Collaborator

@ValentinPostindustria ValentinPostindustria left a comment

Choose a reason for hiding this comment

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

LGTM!

@YuriyVelichkoPI
Copy link
Copy Markdown
Contributor

The last step before final approval is a PR for the docs. As soon as it is opened this one will be approved. The work on docs is in progress.

Copy link
Copy Markdown
Contributor

@YuriyVelichkoPI YuriyVelichkoPI left a comment

Choose a reason for hiding this comment

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

LGTM

@mdanylov-sigma mdanylov-sigma merged commit ca0dfd9 into prebid:master Mar 31, 2026
5 checks passed
}

private static void handleNextGenSdkTargeting(HashMap<String, String> bids, Object builder) {
Set<Map.Entry<String, String>> entries = bids.entrySet();
Copy link
Copy Markdown

@Khushboo1028 Khushboo1028 Apr 9, 2026

Choose a reason for hiding this comment

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

Just came across a NPE here. Can we add this to avoid the bug


 if(bids == null || bids.isEmpty()) {
            return;
  }

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.

5 participants