Incomplete field masks when using empty arrays, empty strings or null values

[PierrickVoulet]

I'm running into incomplete field masks as well, in my case when using empty arrays or empty strings.

$keywordPlanCampaign = (new KeywordPlanCampaign())
	->setResourceName('') // as an example
	->setCpcBidMicrosUnwrapped(1000000)
	->setLanguageConstants([]); // legitimate use case

var_dump(FieldMasks::allSetFieldsOf($keywordPlanCampaign)->serializeToJsonString());

Expected result

"resourceName,languageConstants,cpcBidMicros"

Actual result

"cpcBidMicros"

Originally posted by @leongersen in https://github.com/googleads/google-ads-php/issues/480#issuecomment-738801974

[PierrickVoulet]

I did some extra tests:

$googleCampaign = new Campaign([
    'name' => ''
]);
var_dump(FieldMasks::allSetFieldsOf($googleCampaign)->serializeToJsonString());
$googleCampaign = new Campaign([
    'resource_name' => ''
]);
var_dump(FieldMasks::allSetFieldsOf($googleCampaign)->serializeToJsonString());

//> string(6) ""name""
//> string(2) """"

It looks like empty string works for fields that are not resource_name.

[leongersen]

I noticed the "missing" resource name when creating a reproducer for the empty array in languageConstants, so that might not be an issue.

[leongersen]

I also just ran into this when trying to clear a bid on AdGroupCriterion. setCpcBidMicros cannot be called with null, and calling clearCpcBidMicros does not result in cpc_bid_micros appearing in the field mask generated with allSetFieldsOf.

[PierrickVoulet]

I also just ran into this when trying to clear a bid on AdGroupCriterion. setCpcBidMicros cannot be called with null, and calling clearCpcBidMicros does not result in cpc_bid_micros appearing in the field mask generated with allSetFieldsOf.

Thank you for the heads up. Do you have a code snippet that illustrates the expected behavior? This would be incredibly helpful to work on the fix.

[leongersen]

Sure:

$adGroupCriterion = new AdGroupCriterion();

// This should be valid, as clearing the bid on a criterion is a valid operation
$adGroupCriterion->setCpcBidMicros(null); // Fails, "Uncaught Exception: Cannot convert '' to integer"

$adGroupCriterion->clearCpcBidMicros(); // Was not set, so does nothing

var_dump(FieldMasks::allSetFieldsOf($adGroupCriterion)->serializeToJsonString());

Expected result:

"cpc_bid_micros"

Actual result:

Either an exception, or:

""

[leongersen]

I just ran into another example of this problem.

In the AdWords API, it was possible to clear an existing bid modifier by setting its value to -1.

In the new Google Ads API, this behaviour is still available by sending an update with a null value, but the Fieldmasks utility doesn't allow for this.

As an example, the normal flow for updating a campaign criterion bid modifier would be something like this:

$criterion = new CampaignCriterion();
$criterion->setBidModifier($bidModifier); // cannot be called with null, as this checks for floats
$criterion->setResourceName(ResourceNames::forCampaignCriterion(...));

$operation = new CampaignCriterionOperation();
$operation->setUpdate($criterion);
$operation->setUpdateMask(FieldMasks::allSetFieldsOf($criterion));

To clear an existing modifier, the update mask has to be manually set to include the bid_modifier path, which otherwise will not update:

$criterion = new CampaignCriterion();
// don't call this setter...
// $criterion->setBidModifier($bidModifier);
$criterion->setResourceName(ResourceNames::forCampaignCriterion(...));

$operation = new CampaignCriterionOperation();
$operation->setUpdate($criterion);
// ...but include the field in the update
$operation->setUpdateMask((new FieldMask())->setPaths(['resource_name', 'device.type', 'bid_modifier']));

This successfully clears the device bid modifier, but manually setting the field mask is quite error prone.

I'd also suggest updating the title of this issue to include the handling of null values in FieldMasks.

[chiragvels]

Hi @PierrickVoulet ,

I also tried to clear bidModifier same as done by @leongersen

But I cannot.

The API request :

--data '{ "operations": [{ "update": { "resourceName": "customers/<customerID>/campaignCriteria/<campaignID>~<criterionID>", }, "updateMask": "bidModifier", } ] }'

Issue on Forum.

Thanks,

[JimmyBryant]

you should set bid_modifier field when you generate update_mask,then remove bid_modifier when doing update operation。

My example code run well and bid_modifier has been cleared.

$criterion = array(                
    'resource_name' => ResourceNames::forCampaignCriterion(
        $customerId,
        $campaignId,
        $locationId
    ),
    'bid_modifier'=>1, // bid modifier must set to a valid value
);

$campaignCriterion = new CampaignCriterion($criterion);
$updateMask = FieldMasks::allSetFieldsOf($campaignCriterion);  

 // remove bid_modifier field
unset($criterion['bid_modifier']);  
$campaignCriterion = new CampaignCriterion($criterion);

$operation = new CampaignCriterionOperation(['update_mask'=>$updateMask,'update' => $campaignCriterion]);
$operations = [$operation];

// Issues a mutate request to update the campaign criterion.
$campaignCriterionServiceClient = $this->googleAdsClient->getCampaignCriterionServiceClient();
$campaignCriterionServiceClient->mutateCampaignCriteria($this->customerId, $operations);

[fiboknacky]

Based on our current documentation, this works as intended. With the limitations of the language and underlying protobuf, there is no perfect way that we can do this automatically as of today.