Huaweiadsadapter gdpr #2345

clho40 · 2022-08-08T12:32:08Z

Get the country value from the MCC code
Get the GAID from the Openrtb.device.ifa field by default

parse and send GDPR consent string to Huawei Adx

remove unwanted consent element

Update huaweiads.go

gofmt

adapters/huaweiads/huaweiads.go

mansinahar · 2022-08-11T15:11:40Z

adapters/huaweiads/huaweiads.go

+			return errors.New("Unmarshal: openRTBRequest.User.Ext -> extUserDataHuaweiAds failed")
+		}
+		var deviceId = extUserDataHuaweiAds.Data
+		if len(deviceId.Imei) == 0 && len(deviceId.Gaid) == 0 && len(device.Gaid) == 0 && len(deviceId.Oaid) == 0 {


Did you mean to check for the length of device.Gaid or was that added by mistake? Looks like you already have a conditional before that to check length of deviceId.Gaid

sorry for the confusion. at line#665 we tried to assign device.ifa to device.gaid field. Therefore we are checking for the existence of this value at this point, if all of them are empty we'll return an error

adapters/huaweiads/huaweiads.go

clho40 · 2022-08-15T08:37:31Z

@GenarC

bsardo · 2022-08-22T16:34:47Z

Hello @clho-huawei, @clho40,
Could you please merge with master to resolve the conflicts?

clho40 · 2022-08-22T16:36:52Z

@bsardo Hello. Thanks for reminding. We are doing some final checks and will merge them soon :)

bsardo · 2022-08-29T14:08:52Z

Hello @clho40, a couple of tests are failing due to formatting issues. Can you please run go fmt to resolve these? Thanks!

merge

adapters/huaweiads/huaweiads.go

mansinahar · 2022-09-07T12:06:21Z

adapters/huaweiads/huaweiads.go

+	if userObjExist {
+		var extUserDataHuaweiAds openrtb_ext.ExtUserDataHuaweiAds
+		if err := json.Unmarshal(openRTBRequest.User.Ext, &extUserDataHuaweiAds); err != nil {
+			return errors.New("get gaid from openrtb Device.IFA failed, and get device id failed: Unmarshal openRTBRequest.User.Ext -> extUserDataHuaweiAds.")


I'd recommend appending the actual unmarshal error to your error message so that it can be helpful in debugging. Something like:

return fmt.Errorf("get gaid from openrtb Device.IFA failed, and get device id failed: Unmarshal error: %v", err)

adapters/huaweiads/huaweiads.go

rebase huaweiadsadapter-gdpr

adapters/huaweiads/huaweiadstest/exemplary/banner4_mccmnc.json

adapters/huaweiads/huaweiadstest/exemplary/banner5_user_geo.json

VeronikaSolovei9 · 2022-09-22T06:54:29Z

adapters/huaweiads/huaweiads.go

@@ -459,10 +459,14 @@ func getReqJson(request *huaweiAdsRequest, openRTBRequest *openrtb2.BidRequest,
 func getReqAdslot30(huaweiAdsImpExt *openrtb_ext.ExtImpHuaweiAds,
 	openRTBImp *openrtb2.Imp, openRTBRequest *openrtb2.BidRequest) (adslot30, error) {


openRTBRequest *openrtb2.BidRequest parameter became unused, can you please delete it?

Parameter is deleted.

VeronikaSolovei9 · 2022-09-22T06:58:59Z

adapters/huaweiads/huaweiads.go

@@ -459,10 +459,14 @@ func getReqJson(request *huaweiAdsRequest, openRTBRequest *openrtb2.BidRequest,
 func getReqAdslot30(huaweiAdsImpExt *openrtb_ext.ExtImpHuaweiAds,
 	openRTBImp *openrtb2.Imp, openRTBRequest *openrtb2.BidRequest) (adslot30, error) {
 	adtype := convertAdtypeStringToInteger(strings.ToLower(huaweiAdsImpExt.Adtype))
+	testStatus := 0
+	if huaweiAdsImpExt != nil && huaweiAdsImpExt.IsTestAuthorization == "true" {


if huaweiAdsImpExt != nil check is not needed here, it's executed before the getReqAdslot30 function call, L281

Null check is removed

VeronikaSolovei9 · 2022-09-22T07:07:12Z

adapters/huaweiads/huaweiads.go

+		}
+	}
+}
+
 // getDeviceID include oaid gaid imei. In prebid mobile, use TargetingParams.addUserData("imei", "imei-test");
 // When ifa: gaid exists, other device id can be passed by TargetingParams.addUserData("oaid", "oaid-test");
 // When getGaidFromDeviceIFA success, getDeviceIDFromUserExt failed, no error will be reported.


Function getGaidFromDeviceIFA doesn't exist anymore, please modify comment accordingly

Because "getGaidFromDeviceIFA" function is removed, the comment is removed also.

VeronikaSolovei9 · 2022-09-22T07:28:25Z

adapters/huaweiads/huaweiads.go

+			return errors.New("get gaid from openrtb Device.IFA failed, and get device id failed: Unmarshal openRTBRequest.User.Ext -> extUserDataHuaweiAds. Error: " + err.Error())
+		}
+		var deviceId = extUserDataHuaweiAds.Data
+		if len(deviceId.Imei) == 0 && len(deviceId.Gaid) == 0 && len(device.Gaid) == 0 && len(deviceId.Oaid) == 0 {


Small optional optimization: to avoid double evaluation of len(deviceId.Imei) == 0 and others, this if condition can be removed. Instead do this:

isValidDeviceId := false if len(deviceId.Oaid) > 0 { device.Oaid = deviceId.Oaid[0] isValidDeviceId = true } if len(deviceId.Gaid) > 0 { device.Gaid = deviceId.Gaid[0] isValidDeviceId = true } if len(deviceId.Imei) > 0 { if len(deviceId.Imei) > 0{ device.Imei = deviceId.Imei[0] isValidDeviceId = true } } if !isValidDeviceId{ return errors.New("getDeviceID: Imei ,Oaid, Gaid are all empty.") }

Recommendation is applied

PR 2345

VeronikaSolovei9

Thank you for the updates, LGTM

AlexBVolcy

Just some comments related to code coverage

AlexBVolcy · 2022-09-28T01:23:32Z

adapters/huaweiads/huaweiads.go

+	intVar, err := strconv.Atoi(countryMCC)
+
+	if err != nil {
+		return defaultCountryName


Is there any way to trigger/cover this line of code

banner4_mccmnc.json file is used to cover line 783.

@GenarC I still see this line uncovered when I run the tests in the huaweiads package

adapters/huaweiads/huaweiads.go

VeronikaSolovei9 · 2022-10-04T05:59:40Z

Thank you for adding test. However it doesn't add coverage for the places Alex mentioned.

To add coverage for L822 you need to modify newly added test:
mockBidRequest.user.ext.data add "imei": ["test"] and check it in httpcalls.expectedRequest.body.device.imei: "test"
To add coverage for L786 you need to add a test where mockBidRequest.device.mccmnc is a string constant that cannot be converted to number, something like "test". Also remove mockBidRequest.device.geo and mockBidRequest.uase.geo from request.
To add coverage for L791 please follow directions in point 2 and set mockBidRequest.device.mccmnc to value that doesn't exist in MccList, something like "800-1"

This should add needed coverage

VeronikaSolovei9

Thank you for adding tests, LGTM

GenarC · 2022-10-05T06:56:45Z

Thanks for the help and the feedbacks

* origin/master: (316 commits) Fix alt bidder code test broken by imp wrapper commit (prebid#2405) ImpWrapper (prebid#2375) Update sspBC adapter to v5.7 (prebid#2394) Change errors to warnings (prebid#2385) TheMediaGrid: support native mediaType (prebid#2377) Huaweiadsadapter gdpr (prebid#2345) Add alternatebiddercodes in Prebid request ext (prebid#2339) Changed FS bidder names to mixed case (prebid#2390) Load bidders names from file system in lowercase (prebid#2388) consumable adapter: add schain and coppa support (prebid#2361) Adapter aliases: moved adapter related configs to bidder.yaml files (prebid#2353) pubmatic adapter: add param prebiddealpriority (prebid#2281) Yieldlab Adapter: Add support for ad unit sizes (prebid#2364) GDPR: add enforce_algo config flag and convert enforce_purpose to bool (prebid#2330) Resolves CVE-2022-27664 (prebid#2376) AMP Endpoint: support parameters consentType and gdprApplies (prebid#2338) Dianomi - added usync for redirect (prebid#2368) Create issue_prioritization.yml (prebid#2372) Update yaml files for gzip supported bidders (prebid#2365) Adnuntius Adapter: Set no cookies as a parameter (prebid#2352) ...

Co-authored-by: Ho Chia Leung <[email protected]> Co-authored-by: Cem Genar <[email protected]>

clho404 added 9 commits May 20, 2022 15:43

Update huaweiads.go

b497887

parse and send GDPR consent string to Huawei Adx

gofmt

55ba717

update huaweiads.go

6a16e2f

Update huaweiads.go

dfcd9ad

Update imp_huaweiads.go

6e6d368

remove unwanted consent element

Update imp_huaweiads.go

497864b

Update banner1.json

446c745

Update huaweiads.go

a2bbf46

Update huaweiads.go

8e34bed

bsardo self-requested a review August 8, 2022 18:06

bsardo self-assigned this Aug 8, 2022

bsardo requested a review from mansinahar August 8, 2022 18:06

bsardo assigned mansinahar Aug 8, 2022

GenarC and others added 3 commits August 9, 2022 15:09

Update huaweiads.go

fad3ecb

Merge pull request #1 from GenarC/patch-2

a17b298

Update huaweiads.go

Update huaweiads.go

5960f29

gofmt

mansinahar reviewed Aug 11, 2022

View reviewed changes

clho404 added 2 commits August 19, 2022 12:11

Update huaweiads.go

a88a746

Update huaweiads.go

ec9f579

clho40 added 2 commits August 29, 2022 09:43

Update banner1.json

7ec4339

Merge branch 'master' into huaweiadsadapter-gdpr

af23c87

clho404 and others added 3 commits August 29, 2022 17:58

Update huaweiads.go

145b56a

update tests

8cc6f45

Merge pull request #2 from prebid/master

f450d80

merge

mansinahar reviewed Sep 7, 2022

View reviewed changes

Fix PR_2345 comments

8fa5e53

clho40 added 5 commits September 13, 2022 17:02

fix test cases

830c140

fix test cases, and comments

9587fc6

new fixes

9e44b28

Merge branch 'prebid:master' into master

4c59355

Merge pull request #4 from clho40/master

d4868db

rebase huaweiadsadapter-gdpr

mansinahar reviewed Sep 15, 2022

View reviewed changes

adapters/huaweiads/huaweiadstest/exemplary/banner4_mccmnc.json Show resolved Hide resolved

adapters/huaweiads/huaweiadstest/exemplary/banner5_user_geo.json Show resolved Hide resolved

bsardo requested review from VeronikaSolovei9 and removed request for bsardo September 21, 2022 17:20

bsardo assigned VeronikaSolovei9 and unassigned bsardo Sep 21, 2022

VeronikaSolovei9 reviewed Sep 22, 2022

View reviewed changes

Remove unnecessary parameters

9d5e4ad

PR 2345

VeronikaSolovei9 previously approved these changes Sep 27, 2022

View reviewed changes

bsardo requested review from AlexBVolcy and mansinahar and removed request for mansinahar September 27, 2022 17:17

bsardo assigned AlexBVolcy and unassigned mansinahar Sep 27, 2022

AlexBVolcy reviewed Sep 28, 2022

View reviewed changes

Add imei case

fba24a3

GenarC dismissed VeronikaSolovei9’s stale review via fba24a3 September 30, 2022 07:13

SyntaxNode added the port to java label Oct 3, 2022

Add coverage for wrong mccmnc values & imei

64263c6

VeronikaSolovei9 approved these changes Oct 4, 2022

View reviewed changes

AlexBVolcy approved these changes Oct 5, 2022

View reviewed changes

bsardo merged commit 91336f6 into prebid:master Oct 5, 2022

shunj-nb pushed a commit to ParticleMedia/prebid-server that referenced this pull request Nov 8, 2022

Huaweiadsadapter gdpr (prebid#2345)

77e2350

Co-authored-by: Ho Chia Leung <[email protected]> Co-authored-by: Cem Genar <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Huaweiadsadapter gdpr #2345

Huaweiadsadapter gdpr #2345

clho40 commented Aug 8, 2022

mansinahar Aug 11, 2022

clho40 Aug 17, 2022

clho40 commented Aug 15, 2022

bsardo commented Aug 22, 2022

clho40 commented Aug 22, 2022

bsardo commented Aug 29, 2022

mansinahar Sep 7, 2022

VeronikaSolovei9 Sep 22, 2022

GenarC Sep 26, 2022

VeronikaSolovei9 Sep 22, 2022

GenarC Sep 26, 2022

VeronikaSolovei9 Sep 22, 2022

GenarC Sep 26, 2022

VeronikaSolovei9 Sep 22, 2022

GenarC Sep 26, 2022

VeronikaSolovei9 left a comment

AlexBVolcy left a comment

AlexBVolcy Sep 28, 2022

GenarC Sep 30, 2022

AlexBVolcy Oct 4, 2022 •

edited

Loading

VeronikaSolovei9 commented Oct 4, 2022 •

edited

Loading

VeronikaSolovei9 left a comment

GenarC commented Oct 5, 2022

		@@ -459,10 +459,14 @@ func getReqJson(request huaweiAdsRequest, openRTBRequest openrtb2.BidRequest,
		func getReqAdslot30(huaweiAdsImpExt *openrtb_ext.ExtImpHuaweiAds,
		openRTBImp openrtb2.Imp, openRTBRequest openrtb2.BidRequest) (adslot30, error) {

Huaweiadsadapter gdpr #2345

Huaweiadsadapter gdpr #2345

Conversation

clho40 commented Aug 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clho40 commented Aug 15, 2022

bsardo commented Aug 22, 2022

clho40 commented Aug 22, 2022

bsardo commented Aug 29, 2022

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

Choose a reason for hiding this comment

VeronikaSolovei9 left a comment

Choose a reason for hiding this comment

AlexBVolcy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexBVolcy Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

VeronikaSolovei9 commented Oct 4, 2022 • edited Loading

VeronikaSolovei9 left a comment

Choose a reason for hiding this comment

GenarC commented Oct 5, 2022

AlexBVolcy Oct 4, 2022 •

edited

Loading

VeronikaSolovei9 commented Oct 4, 2022 •

edited

Loading