smartystreets / smartystreets-php-sdk Goto Github PK

As far as I can tell, $result = $lookup->getResult(); returns an array. Which means $result->getMetadata(); doesn't work and returns Fatal error: Call to undefined method SmartyStreets\PhpSdk\US_Street\Lookup::getMetadata().

Obviously $addresses = $result->getAddresses(); has the same issue.

Am I doing something wrong, or is the example maybe outdated?

It would be helpful to have a php example using php curl.

Unable to make use of php sdk because it lacks crucial feature to give out sendLookup curl response json object.

SDKs should, in principle, be headless. bdc39a1, tagged 4.17.1, added an echo statement to RetrySender without any rationale provided in the changelog or commit message, perhaps an artifact left over from internal debugging?

Searching across the codebase yields the following instances of this:

Happy to provide an example explaining why this is causing issues in our codebase if it's not self-evident.

EDIT: if output is required, hooking into ClientBuilder's withDebug mode might be appropriate.

Hi,

The regular and pro US_Autocomplete classes cannot be loaded simultaneously. The issue comes from these sections conflicting:

The way to fix this is easy:

if (!defined('GEOLOCATE_TYPE_CITY')) {
  define('GEOLOCATE_TYPE_CITY', 'city', false);
}

if (!defined('GEOLOCATE_TYPE_NONE')) {
  define('GEOLOCATE_TYPE_NONE', null, false);
}

Also, the 3rd parameter of define() has been deprecated since PHP 7.3. It is strongly recommended that it be removed, as your library requires PHP 5.6 - PHP 8 in composer.json, which lets it get pulled into all current/modern versions of PHP and can automatically break them.

As a side note, if these suggested changes cause issues for your code, then you will need to rewrite it so that these external constants aren't used. Looking at the usage pattern, these constants serve absolutely no purpose to the code:

This is the only use. You could even add a public const TYPE = "city"; on the GeolocateType class itself.

But, using a global constant in place of a hard-coded string isn't an advisable pattern. This can easily break linters, static analyzers, etc. These conflicting define() calls is all it takes to break a framework like Laravel, as it needs to run certain tasks for auto-discovery of packages and features. So when this update arrives it'll break each app that requires it, preventing package autodiscovery and certain IDE helpers from running (this goes for anyone using PHPStorm and the corresponding IDE helper package(s)).

Cant seem to find the actual json response object from an api lookup; going to need it

This SDK returns a very antiquated object model reminiscent of the bad old XML/SOAP days. You have to call methods like getCities(), getCity(), etc in order to access nested objects.

Please clean this up and return plain old PHP objects (POPOs) instead. That way you can just access the cities array with myobject->Cities. Or you can access a member of that array with $myobject->Cities[0].

This is way cleaner and easier to use than what you are doing here. For compatibility reasons, you should obviously keep the option to keep doing it this way, but there should be a flag to go to this POPO model instead.

Also, why are the members of most of the returned objects private, necessitating the call to getCities, etc as discussed above? This is just annoying and unnecessary, and I can't think of a use case where this would be necessary.

I just ran into an issue where the VERSION constant collided with the project SmartyStreets was being implemented in. Looking at the source it appears VERSION was meant to be namespaced but isn't. This should be implementable two ways:

<?php
namespace SmartyStreets\PhpSdk;
const VERSION = '4.3.2';

or

<?php
namespace SmartyStreets\PhpSdk;
define(__NAMESPACE__ . '\VERSION', '4.3.2', false);

Since the minimum version for the SDK is PHP 5.6 the former might be preferable.

I am still having problems with CORS issue.
I have CORS package implemented on my Laravel app.
it works on my local but not on my staging server.

I've updated to the latest version and started receiving this notice:

Use of undefined constant STDERR - assumed 'STDERR'

1. ~/smartystreets/phpsdk/src/NativeSender.php (line 52)

It was introduced with this commit.

However, STDERR is only available in CLI environments. Additionally, curl_setopt($ch, CURLINFO_HEADER_OUT, $value) expects $value to be boolean. It would have worked in CLI since the stream returned by STDERR would evaluate to true, but it fails in non-CLI environments.

Instead, defined("STDERR") seems to be what was intended. But it could also be skipped if $this->debugMode is not true. So, I would recommend:

if ($this->debugMode && defined("STDERR"))
        curl_setopt($ch, CURLINFO_HEADER_OUT, true);

0dc5c31#diff-2258d1b1c88120c093643b76c98deca30e816a85c7e3f9788be6b8013a6c85edR85-R88 introduced the withXForwardedFor method to allow passing an IP address to Smarty's autocomplete API for geographically-adjacent suggestions (related comment/issue).

In testing, this has no effect on API results.

Changing the HTTP header results in an API response as expected. (Although the docs specify X-Forwarded-At, casing does not appear to be important.)

if ($this->ip != null) {
---   curl_setopt($ch, CURLOPT_HTTPHEADER, array("X_FORWARDED_FOR: $this->ip"));
+++   curl_setopt($ch, CURLOPT_HTTPHEADER, array("X-FORWARDED-FOR: $this->ip"));
      $smartyRequest->setHeader('X_FORWARDED_FOR', $this->ip);
}

Just like US Address; International Address could use Batch Service.

Example，
Address： 9880 Polecat, Kingston, OK, 73439

website tool Results:

SDK :
Request params:

[street] => 9880 Polecat
[street2] =>
[city] => Kingston
[state] => OK
[zipcode] => 73439
[match] => enhanced
[candidates] => 4

Response:

[inputId] => 
[inputIndex] => 0
[candidateIndex] => 0
[addressee] =>
[deliveryLine1] => 9880 Polecat
[deliveryLine2] =>
[deliveryPointBarcode] => 734395600
[lastLine] => Kingston OK 73439-5600
[smartyKey] =>
...  ...  ... ... ... ...
[analysis] => Array
    (
        [dpvMatchCode] => N
        [dpvFootnotes] => AAM3
        [cmra] =>
        [vacant] =>
        [noStat] =>
        [active] => Y
        [isEwsMatch] =>
        [footnotes] =>
        [lacsLinkCode] =>
        [lacsLinkIndicator] =>
        [isSuiteLinkMatch] =>
        [enhancedMatch] => non-postal-match
    )

The defaults within ClientBuilder can cause problems for PHP web apps. Unless the user overrides the default configuration by calling setSender(...) (or calls retryAtMost(0)) the generated clients use a RetrySender wrapped around a MyLogger instance.

This default logger is problematic for web apps in that it writes to STDOUT. In most PHP web frameworks, this causes the logging output to be written out in the generated HTML of the current web request.

Ideally, the default would be to log these messages using error_log instead of print. However, the existing behavior may be something users are relying on. As a next best option, the builder interface should allow the logger to be (easily) overridden.

I've submitted a related PR: #7

It would be nice if the $response->getStatusCode() was added to all the exceptions thrown in src/StatusCodeSender.php so we can get the code via $ex->getCode() when catching the exceptions.

Currently, there is no easy method in the SDK to validate Credentials without creating a request with address validation.
It would be nice if there was a simple way to validate credentials without creating unnecessary requests.

StatusCodeSender.php variable $errorMessage undefined, lead to error: Undefined variable $errorMessage

You can't use count() on PHP strings because they don't implement Countable, you must use strlen or it'll throw an exception.

private function fieldIsMissing($field) {
    return $field == null || count($field) == 0;
}

The following constant is defined incorrectly, and should be "none":

Per the docs:

Acceptable values [of prefer_geolocation] are: empty string (which defaults to city), none, or city.

Additionally, these constants collide with identical ones defined in the deprecated \SmartyStreets\PhpSdk\US_Autocomplete\GeolocateType class, as defined constants do not automatically inherit their parent namespace.

It's also worth noting there's no way to pass an X-Forwarded-For header containing a user's IP address, diminishing the value of the geolocation component of this SDK altogether, since the chance of a server being located in an end user's vicinity is relatively slim. I would consider such an addition a separate feature request, however.

EDIT: "collide" isn't perfectly correct terminology, since !defined is being called first, but since both files are autoloaded, there is no way to choose between constants defined in each file if/when they differ, and the value you end up with is at the mercy of Composer's autoloading strategy.

This is in StatusCodeSender.php. The default condition is to return null if the Curl status code is not matched. I recently had an issue where Curl was failing because the CaCert wasn't set in php.ini (this is a pretty common problem). But your API just returns null...thus taking me hours to climb through your code figuring out where the issue was.

Instead of trying to match the Curl status code for a select number of conditions and then returning null, please return the curl error as the fall through condition per this method:
https://stackoverflow.com/questions/8227909/curl-exec-always-returns-false

function send(Request $request) {
	
    $response = $this->inner->send($request);


    switch ($response->getStatusCode()) {
        case 200:
            return $response;
        case 400:
            throw new BadRequestException("Bad Request (Malformed Payload): A GET request lacked a street field or the request body of a POST request contained malformed JSON.");
        case 401:
            throw new BadCredentialsException("Unauthorized: The credentials were provided incorrectly or did not match any existing, active credentials.");
        case 402:
            throw new PaymentRequiredException("Payment Required: There is no active subscription for the account associated with the credentials submitted with the request.");
        case 413:
            throw new RequestEntityTooLargeException("Request Entity Too Large: The request body has exceeded the maximum size.");
        case 429:
            throw new TooManyRequestsException("When using public \"website key\" authentication, we restrict the number of requests coming from a given source over too short of a time.");
        case 500:
            throw new InternalServerErrorException("Internal Server Error.");
        case 503:
            throw new ServiceUnavailableException("Service Unavailable. Try again later.");
        default:
            return null;
    }
}

Hello,

Are you all amenable to a pull request that adds type hints to the methods?

There are a couple of PHPDoc comments that are slightly off in \SmartyStreets\PhpSdk\US_Street\Lookup. This is causing trouble with static analysis tools, particularly Phan.

Pull request #4 submitted for consideration.

PHP Fatal error: During inheritance of JsonSerializable: Uncaught Exception: Deprecated Functionality: Return type of SmartyStreets\PhpSdk\US_Street\Lookup::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

I have fixed it by forking the repo here https://github.com/vinsaj9/smartystreets-php-sdk.

Seriously? a tarbomb in 2018? Poor form.

http://www.linfo.org/tarbomb.html

-FT

Thanks for the great service. Very happy long time user.

I love that there is a php class. But having already written code to use the API, I am going to want to get back an array of results that has the same structure as your json results, and I want to create requests with arrays too... instead of using getters and setters.

Not very efficient for keeping code nice and short.

It seems like your code supports some of these things.. but your examples do not seem to. Is there any way you can highlight some of the "more than one way to do things" that your library is capable of in your default code?

Thanks,
-FT

Hey folks - we noticed after updating our SDK that the fairly-large PHPUnit executable had been added to our vendor directory. Does that file need to be checked into this repo?