CampaignExtensionSettingService::get() returning Types in wrong property?

[jclee100]

As you can see below, some of the variables are placed into PolicyData.Type and ExtensionFeedItem.Type instead of PolicyDataType and ExtensionFeedItemType. The var_dump is the result from executing get using CampaignExtensionSettingService. I am not sure if it affects other services.

This causes the methods like ExtensionFeedItem::getExtensionFeedItemType() to not work as those properties are null.

array(1) {
  [0]=>
  object(Google\AdsApi\AdWords\v201609\cm\CampaignExtensionSetting)#385 (3) {
    ["campaignId":protected]=>
    int(738996898)
    ["extensionType":protected]=>
    string(7) "CALLOUT"
    ["extensionSetting":protected]=>
    object(Google\AdsApi\AdWords\v201609\cm\ExtensionSetting)#382 (2) {
      ["extensions":protected]=>
      array(1) {
        [0]=>
        object(Google\AdsApi\AdWords\v201609\cm\CalloutFeedItem)#381 (17) {
          ["calloutText":protected]=>
          string(13) "Free delivery"
          ["feedId":protected]=>
          int(45250724)
          ["feedItemId":protected]=>
          int(10072062236)
          ["status":protected]=>
          string(7) "ENABLED"
          ["feedType":protected]=>
          string(7) "CALLOUT"
          ["startTime":protected]=>
          NULL
          ["endTime":protected]=>
          NULL
          ["devicePreference":protected]=>
          NULL
          ["scheduling":protected]=>
          NULL
          ["campaignTargeting":protected]=>
          NULL
          ["adGroupTargeting":protected]=>
          NULL
          ["keywordTargeting":protected]=>
          NULL
          ["geoTargeting":protected]=>
          NULL
          ["geoTargetingRestriction":protected]=>
          NULL
          ["policyData":protected]=>
          array(1) {
            [0]=>
            object(Google\AdsApi\AdWords\v201609\cm\FeedItemPolicyData)#380 (10) {
              ["placeholderType":protected]=>
              int(17)
              ["feedMappingId":protected]=>
              int(47930811)
              ["validationStatus":protected]=>
              string(5) "VALID"
              ["approvalStatus":protected]=>
              string(8) "APPROVED"
              ["validationErrors":protected]=>
              NULL
              ["qualityApprovalStatus":protected]=>
              string(7) "UNKNOWN"
              ["qualityDisapprovalReasons":protected]=>
              NULL
              ["disapprovalReasons":protected]=>
              NULL
              ["PolicyDataType":protected]=>
              NULL
              ["PolicyData.Type"]=>
              string(18) "FeedItemPolicyData"
            }
          }
          ["ExtensionFeedItemType":protected]=>
          NULL
          ["ExtensionFeedItem.Type"]=>
          string(15) "CalloutFeedItem"
        }
      }
      ["platformRestrictions":protected]=>
      string(4) "NONE"
    }
  }
}

[fiboknacky]

Hi JC,

Thanks for reporting.
Is it the result for the SOAP call or from the deserialization process by Symfony serializer?

Best,
Knack

[jclee100]

Hi @fiboknacky,

I am not sure as I haven't looked into the source code.

Here's the essence of what I did:

        /** @var CampaignExtensionSettingService $campaignExtensionSettingService */
        $campaignExtensionSettingService = $this->getService(CampaignExtensionSettingService::class);
        $selector = new Selector([
            'CampaignId', 'ExtensionType', 'Extensions', 'PlatformRestrictions'
        ]);
        $selector->setPredicates([
            new Predicate('CampaignId', PredicateOperator::EQUALS, [$campaignId])
        ]);

        $selector->setPaging(new Paging(0, static::$pageSize));

        $page = $campaignExtensionSettingService->get($selector);
        var_dump($page->getEntries());

[fiboknacky]

Hi JC,

Sorry, I wasn't clear. I should've said that "did you use batch job service?". :)
But it's clear now that you didn't.

Thanks for information.

Best,
Knack

[ddeppner]

This is a specific case of a more generic bug through the objects returned by querying services.

Here's an example of a var_dump of an object returned from the AdGroupCriterionService:

      object(Google\AdsApi\AdWords\v201609\cm\BiddableAdGroupCriterion)#11672 (25) {
        ["userStatus":protected]=>
        string(7) "ENABLED"
        ["systemServingStatus":protected]=>
        NULL
        ["approvalStatus":protected]=>
        NULL
        ["disapprovalReasons":protected]=>
        NULL
        ["destinationUrl":protected]=>
        NULL
        ["firstPageCpc":protected]=>
        NULL
        ["topOfPageCpc":protected]=>
        NULL
        ["firstPositionCpc":protected]=>
        NULL
        ["qualityInfo":protected]=>
        NULL
        ["biddingStrategyConfiguration":protected]=>
        object(Google\AdsApi\AdWords\v201609\cm\BiddingStrategyConfiguration)#11675 (6) {
          ["biddingStrategyId":protected]=>
          NULL
          ["biddingStrategyName":protected]=>
          NULL
          ["biddingStrategyType":protected]=>
          string(10) "MANUAL_CPC"
          ["biddingStrategySource":protected]=>
          NULL
          ["biddingScheme":protected]=>
          NULL
          ["bids":protected]=>
          array(1) {
            [0]=>
            object(Google\AdsApi\AdWords\v201609\cm\CpcBid)#11676 (4) {
              ["bid":protected]=>
              object(Google\AdsApi\AdWords\v201609\cm\Money)#11677 (3) {
                ["microAmount":protected]=>
                int(70000)
                ["ComparableValueType":protected]=>
                NULL
                ["ComparableValue.Type"]=>
                string(5) "Money"
              }
              ["cpcBidSource":protected]=>
              string(9) "CRITERION"
              ["BidsType":protected]=>
              NULL
              ["Bids.Type"]=>
              string(6) "CpcBid"
            }
          }
        }
        ["bidModifier":protected]=>
        NULL
        ["finalUrls":protected]=>
        NULL
        ["finalMobileUrls":protected]=>
        NULL
        ["finalAppUrls":protected]=>
        NULL
        ["trackingUrlTemplate":protected]=>
        NULL
        ["urlCustomParameters":protected]=>
        NULL
        ["adGroupId":protected]=>
        int(2877122)
        ["criterionUse":protected]=>
        string(8) "BIDDABLE"
        ["criterion":protected]=>
        object(Google\AdsApi\AdWords\v201609\cm\ProductPartition)#11673 (7) {
          ["partitionType":protected]=>
          string(4) "UNIT"
          ["parentCriterionId":protected]=>
          int(18283950120)
          ["caseValue":protected]=>
          object(Google\AdsApi\AdWords\v201609\cm\ProductOfferId)#11674 (3) {
            ["value":protected]=>
            string(13) "tr-ls14jkt0-x"
            ["ProductDimensionType":protected]=>
            NULL
            ["ProductDimension.Type"]=>
            string(14) "ProductOfferId"
          }
          ["id":protected]=>
          int(280122772)
          ["type":protected]=>
          string(17) "PRODUCT_PARTITION"
          ["CriterionType":protected]=>
          NULL
          ["Criterion.Type"]=>
          string(16) "ProductPartition"
        }
        ["labels":protected]=>
        NULL
        ["forwardCompatibilityMap":protected]=>
        NULL
        ["baseCampaignId":protected]=>
        NULL
        ["baseAdGroupId":protected]=>
        NULL
        ["AdGroupCriterionType":protected]=>
        NULL
        ["AdGroupCriterion.Type"]=>
        string(24) "BiddableAdGroupCriterion"
      }
    }
    ["totalNumEntries":protected]=>
    int(1848)
    ["PageType":protected]=>
    NULL
    ["Page.Type"]=>
    string(20) "AdGroupCriterionPage"
  }

Note that there are multiple objects that had the data put in an object property with ".Type" in the name rather than the correct property with "Type" at the end, such as "ComparableValueType" on the Money object, or the "AdGroupCriterionType".

This breaks A LOT of the new library, because there is no way to call member functions to get the data, such as on the ProductOfferId object, where you would expect to call ->getProductDimensionType() but it will only ever return a null. The "ProductDimension.Type" property that is created in error and has the correct data can't even be accessed as a property, because having that dot in the name makes it an invalid PHP variable name...

A workaround is to call something like get_object_vars($criterion->getCriterion()->getCaseValue())['ProductDimension.Type'] which will return the object's public properties as an associative array, so the data can then be accessed.

I haven't poked around a ton to figure out why this is happening, but it appears to originate in src/Google/AdsApi/Common/AdsSoapClient.php. If you var_dump the $response that's returned at line 115, you can see that the SoapClient object's __soapCall method returns the created object representing the XML data with property names that match the XML returned from the SOAP service, but that aren't correct in the context of this library.

These errors exist across multiple services and make working with this library difficult until they're fixed. At least there's a workaround, but that means I have to write some code that will break in the future when you fix this bug.

A bug fix sooner rather than later would be nice. :)

[fiboknacky]

Hello @ddeppner

Thanks so much for more information from your side. :)
As you already mentioned, this issue occurs because there are properties whose names contain "dot", e.g., ComparableValue.Type.
These aren't deserialized properly by PHP SOAP toolkit, so you see their values as null.

We're looking into this and deciding how we should fix this.
Before that, I would appreciate if you could tell me more about your use case of using such kinds of properties.

Usually such properties contain the names of their containing classes, e.g., Money's ComparableValue.Type and CallOutFeedItem's ExtensionFeedItem.Type, which can be fetched already by using ReflectionClass:getShortName() or get_class().
In what cases would you like to particularly get such properties?

Best,
Knack

[jclee100]

Hi @fiboknacky,

For me, I was using it as an identifier when serialized into JSON. During denomalization, when stored in JSON/array/stdClass. the type property can be useful as the class names are not available.

I use the other type property (the one with Enum strings) where it is present.

On the topic of JSON. All the classes in the new library use protected properties. Whenever I need to cast them to JSON, I need to pass them through a recursive function to ReflectionProperty::setAccessible() all their properties. Any better way to do this?

A normalizer/denomalizer will be helpful. I have been writing one for every class that I want to store as JSON.

[ddeppner]

@fiboknacky,

I need to crawl through the criteria in an account to determine current bids in a shopping campaign, but in this particular case I need to only work with bids in criteria that are for individual products. But the account could have other bids that are by other dimensions. Also, these ItemId bids are sometimes nested in a structure where the product feed could be split by Brand first, and then within each brand, they could be split by ItemId.

So when I SELECT the criteria, I have a problem. I need to select criteria with ProductPartitionType = "UNIT" and that isn't a filterable option. And I need to select criteria representing an actual product, and the only way I see to do that is when the CaseValue is a ProductOfferId type, and that also is not a filterable option.

So I'm including PartitionType and CaseValue in my SELECTed fields, then iterating over the list and determining which criteria to operate on. I was previously using the old library where these where represented in a nested array rather than objects with getters and setters.

Since I'm used to working with the previous libraries, I was kinda locked into thinking about this as a situation where I needed that field, so my original solution was based on trying to get at that field:

get_object_vars($criterion->getCriterion()->getCaseValue())['ProductDimension.Type']

But I take your point. Now that it's using a more modern architecture, I don't need the field to determine the type, I can look at the basename of the class:

(new \ReflectionClass($criterion->getCriterion()->getCaseValue()))->getShortName())

get_class() returns the fully qualified class name so it requires more string functions to clean it up, which works but isn't the prettiest thing in the world. instanceof could even be used, but would be problematic because of the versioning in the FQCNs...

The thing is, every one of those solutions makes me have to THINK. And they break from what I expect from the previous PHP library. This seems more natural when I'm converting legacy code or quickly typing out something I'd expect to work in new code:

$criterion->getCriterion()->getCaseValue()->getProductDimensionType()

That doesn't make me have to stop and think. It's just obvious.

I think the getSomethingType() functions should be fixed...so the PHP library conforms to what someone would expect reading the API documentation or converting code from the previous library version. But even then it's not perfect because the API docs list the field names with the "dot" in them and when PHP developers go on autopilot and type $obj->getSomething.Type() they'll get an error initially until they dig deeper and figure it out.

Why don't you just set the Type field in the constructor of the affected classes or something simple like that? Or include the ReflectionClass code in the getSomethingType() functions so it doesn't even consult the variable. Better yet, just return the name of the class without calling other code—you know it there. :) And then perhaps devote a bit of space in the PHP library documentation to calling out this issue and explaining how it works (regarding the naming of things as "ProductDimensionType" rather than "ProductDimension.Type" since we're using PHP and we need valid variable names, so in this regard the library varies a bit from the API documentation. As more people convert code from the old library to the new, it would be really helpful and save a lot of time.

David

[fiboknacky]

Hello David,

Thanks a lot for more feedback and suggestion.
We'll fix this soon.

Best,
Knack

[fiboknacky]

This is fixed in v28.0.0.