Needs complete refactoring. My two cents.

[timothymarois]

Obviously, there are no PHP devs at Microsoft. This is one terrible API SDK. I mean no offense to the author, but hire a PHP developer. There are more problems in this API than I can even list right now. The biggest thing that is incredibly irritating is the use of $GLOBALS are we coding in 1999 again? The entire sample package is utterly useless. You would have to rewrite the whole damn thing to get it integrated into any modern platform. Now you'll come back and say. "That's why it's a sample." But its worse than just looking at examples, because all of your methods require dependencies across the board, its a spider-web, a bunch of spaghetti code. This API breaks all standard coding practices.

[J7mbo]

Hey! A few minor points before the response that you may not be aware of - it’s an SDK not an API, and a tool auto-generates the code, they don’t write it themselves. So they make changes to this tool that autogenerates it.

Regardless, it doesn’t matter. Even if this library was perfect, you’d do well to protect yourself with an anti-corruption layer anyway. So do that.

As for globals, I haven’t used that once. I’m not defending the code quality, but now you know it’s written by a machine basically, and not a human ;) and it still hasn’t stopped us using it at all.

[timothymarois]

@J7mbo I wasn't aware a machine wrote this code. That might make a lot more sense now. I wonder if we all out of a job soon? haha. Well anyways, thanks for the explanation.

[madsem]

I'm having the same concerns, and a hard time integrating the SDK.

I think this should stay open, so that @eric-urban and team take notice, and maybe change some things. A SDK is made for developers, so it should be easy to use and examples should be based on real-life situations and code.

The documentation is really messy, you have to jump around like a maniac to find things, examples are infested with $GLOBAL's, it's hard to follow the code even in the examples. Look at the Google Ads API... Much better documentation and their examples are based on real-life applications.

Please remember, not everyone who is using this API is a super-duper pro programmer or has backup from an entire department of other programmers, there are many who are using this API who automate their own small business.

Here are also some examples I find especially irritating and hard to work around.

• You absolutely HAVE to use the $Authentication property in all your calls, it is actually expected everywhere in the sdk.

Code like this, makes me cringe:

elseif($authorizationData->Authentication->Type == "OAuthWebAuthCodeGrant" ||
                   $authorizationData->Authentication->Type == "OAuthDesktopMobileAuthCodeGrant" ||
                   $authorizationData->Authentication->Type == "OAuthDesktopMobileImplicitGrant") 
            {

I HAVE to use the OAuthWebAuthCodeGrant etc classes to be able to connect, because the sdk expects $Type to be set to one of those strings, which is hidden somewhere in the first level of the Authorization class.

That makes it hard to simply create one method in your package that handles authentication, what if I want the same method to handle web auth, and then later also server to server calls with just the refresh token? Then I have to pass in different initialization objects depending on what type of connection it is.

Compared to Google Ads API: webauth:

$oauth2 = new OAuth2(
            [
                'clientId' => $clientId,
                'clientSecret' => $clientSecret,
                'authorizationUri' => self::AUTHORIZATION_URI,
                'redirectUri' => $redirectUrl . self::OAUTH2_CALLBACK_PATH,
                'tokenCredentialUri' => CredentialsLoader::TOKEN_CREDENTIAL_URI,
                'scope' => self::SCOPE,
                // Create a 'state' token to prevent request forgery. See
                // https://developers.google.com/identity/protocols/OpenIDConnect#createxsrftoken
                // for details.
                'state' => sha1(openssl_random_pseudo_bytes(1024))
            ]
        );

server to server with refresh token:

$oauth2 = new OAuth2(
            [
                'authorizationUri' => self::AUTHORIZATION_URI,
                'redirectUri' => self::REDIRECT_URI,
                'tokenCredentialUri' => CredentialsLoader::TOKEN_CREDENTIAL_URI,
                'clientId' => $clientId,
                'clientSecret' => $clientSecret,
                'scope' => $scopes
            ]
        );

Same class is used, no need to instantiate and pass around different objects, only to create a s2s connection... sigh :)

And PSPS:

Method chaining on the authentication objects is another thing that complicates everything. How could I now not set the state? Doesn't seem possible, I have to duplicate code and make it conditional, would be so much easier if I could just pass in the state conditionally as array value.

protected function authenticate($type, $state = null)
    {
        if ( ! class_exists($type)) {
            throw new \InvalidArgumentException('No Bing Ads grant type of' . $type . ' exists.');
        }

        $authentication = (new $type())
            ->withEnvironment($this->getApiEnvironment())
            ->withClientId($this->clientId)
            ->withClientSecret($this->clientSecret)
            ->withRedirectUri($this->redirectUrl)
            ->withState($state);

        return (new AuthorizationData())
            ->withAuthentication($authentication)
            ->withDeveloperToken($this->apiKey);
    }

[timothymarois]

Hey @madsem I decided to write my own wrapper around this SDK for Laravel. My goal has been to make Google Ads and Bind Ads both function the same way with authentication, reports and account management. Its not entirely complete, but it does the job for the most part.