Arbitrary ORTB request configuration changes #1064
Conversation
| super.init() | ||
| } | ||
|
|
||
| public static func enrich( |
There was a problem hiding this comment.
Let's change the signature of this method due to it's resposibility.
public static func merge(
sdkORTB: [String: Any],
globalORTB: String?,
impORTB: String?
) -> [String: Any] {
///
|
|
||
| if var globalORTBDict = ArbitraryGlobalORTBHelper(ortb: globalORTB).getValidatedORTBDict() { | ||
| let existingImpDict = resultORTB["imp"] as? [[String: Any]] | ||
| resultORTB["imp"] = nil |
There was a problem hiding this comment.
Can we make this function better?
In line 35, we added the "imp" object. In line 40, we are removing it.
I believe the algorithm can be more convenient.
|
|
||
| struct ProtectedFields { | ||
|
|
||
| static var deviceProps: [String] { |
There was a problem hiding this comment.
I'm just curious: why is it var?
Because of this: https://forums.swift.org/t/static-var-vs-static-let/59215 ?
There was a problem hiding this comment.
Yes, it seems like a more memory-efficient solution.
| resultORTB["imp"] = existingImps?.map { $0.deepMerging(with: impORTBDict) } ?? [impORTBDict] | ||
| } | ||
|
|
||
| if var globalORTBDict = ArbitraryGlobalORTBHelper(ortb: globalORTB).getValidatedORTBDict() { |
There was a problem hiding this comment.
Does it make sense to apply getValidatedORTBDict only once when the publisher sets the respective property?
| XCTAssertNil(result) | ||
| } | ||
|
|
||
| func testGlobalORTBHelper_ValidJSON_RemoveProtectedFields() { |
There was a problem hiding this comment.
We also need some tests that checks the correct work and combinations of setGlobalORTBConfig and setImpressionORTBConfig.
It's ok if it will be integration tests.
There was a problem hiding this comment.
Already added some tests in ArbitraryORTBServiceTests.
| /// Sets the global-level OpenRTB configuration string. | ||
| /// | ||
| /// - Parameter ortbObject: The global-level OpenRTB configuration string to set. Can be `nil` to clear the configuration. | ||
| public func setGlobalORTBConfig(_ ortbConfig: String?) { |
There was a problem hiding this comment.
Please add an example of how this method is used in the demo apps.
Let's set:
ext.myext.test = 1displaymanager= Googledisplaymanagerver= $CurrentGMASDKVersion
| /// Sets the impression-level OpenRTB configuration string for the ad unit. | ||
| /// | ||
| /// - Parameter ortbObject: The impression-level OpenRTB configuration string to set. Can be `nil` to clear the configuration. | ||
| public func setImpressionORTBConfig(_ ortbConfig: String?) { |
There was a problem hiding this comment.
Please add an example of how this method is used in the demo apps.
Let's set:
imp.bidfloor=10imp.banner.battr= [1,2,3,4]
OlenaPostindustria
force-pushed
the
1017/arbitrary-ortb-request
branch
from
a5a337f to
8a29e20
Compare
PrebidMobile/AdUnits/AdUnit.swift
Outdated
| public func setOrtbConfig(_ ortbObject: String?) { | ||
| adUnitConfig.ortbConfig = ortbObject | ||
| /// - Parameter ortbObject: The impression-level OpenRTB configuration string to set. Can be `nil` to clear the configuration. | ||
| public func setImpressionORTBConfig(_ ortbConfig: String?) { |
There was a problem hiding this comment.
Please, rename to setImpORTBConfig
OlenaPostindustria
force-pushed
the
1017/arbitrary-ortb-request
branch
from
8a29e20 to
26ed749
Compare
|
@OlenaPostindustria, please update the branch one more time to solve the conflicts. |
OlenaPostindustria
force-pushed
the
1017/arbitrary-ortb-request
branch
from
26ed749 to
182a5b4
Compare
#1017