Hey Lev,

Picking back up on this. I've pulled the latest and the instance url stuff is working great.

If you attempt to use an instance of the SalesforceSoapPartner class with an instance of the Salesforce class that has not been authorized or whose access token has timed out, a SOAP fault occurs. In the case of not being authorized, the underlying SOAP client will thrown an INVALID_SOAP_HEADER error (because of missing session id). If the access token has timed out it will throw a INVALID_SESSION_ID error. The underlying toolkit handles neither of these errors gracefully and you'll end up with PHP fatals.

The patch is a simple workaround that makes sure we always have a good session id but first checking if the Salesforce instance is authorized. If it is, it will run an API call which will cause the access token to be refreshed if it has timed out. If the Salesforce instance is not authorized, a log entry is made and a status is set on the instance that can then be checked before running any operations.

I believe a real fix would be to either have the SalesforceSoapPartner be able to refresh it's own session header or somehow keep track of the session expiry so that an API call would only be made if that expiry has passed.

Let me know what you think.

Comments

pcave’s picture

StatusFileSize
new3.27 KB

Here is said patch.

kostajh’s picture

Status: Active » Needs review
ceardach’s picture

Version: 7.x-3.0-alpha2 » 7.x-3.x-dev
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new3.27 KB

Attached is a rerolled patch that applies to the current tip of 7.x-3.x-dev.

I've run it through my test suite which includes extensive soap contact and manipulation with Salesforce and everything passed.

kostajh’s picture

Thanks @ceardach

Would you be willing to contribute your tests? It would be useful get a test suite established for the project.

ceardach’s picture

I will be posting the tests I have specific to the Salesforce module shortly. The rest of the tests, however, are specific to my custom modules.

Attached is a new patch that can be applied after #1951674: Clean up code to match coding standards is applied.

levelos’s picture

Status: Reviewed & tested by the community » Fixed

Thanks guys, committed. Although I'm wondering what the SalesforceSoapPartner::isConnected() is for?

ceardach’s picture

It is so you can check to see if the SalesforceSoapPartner has a valid connection before trying to use it so you don't end up with a fatal error.

levelos’s picture

But the only way it will ever be false is if $this->salesforceApi->isAuthorized() returns false, and we already have that property available. Not a big deal obviously, just want to make sure we really need that additional property.

ceardach’s picture

$this->salesforceApi->isAuthorized() will return TRUE even when the session has timed out, and then SalesforceSoapPartner will try to connect and fail throwing a fatal error.

levelos’s picture

I believe with the following snippet, SalesforceSoapPartner::isConnected() will always be true and redundant.

  $sfapi = salesforce_get_api();
  if (sfapi->isAuthorized()) {
    // Since we've already tested for isAuthorized, the constructor will always set isConnected to true.
    $soap = new SalesforceSoapPartner($sfapi);
  }
ceardach’s picture

Hmmm.... Going to need to call for pcave for details on this one.

pcave’s picture

I don't have any issues with a little redundancy. Since I live in the world of the SOAP API I frequently find myself doing things directly with that API without first consulting the Salesforce class. I'm not married to it so if you feel it's not needed go ahead and remove it. I don't think we've actually coded against it anywhere.

$soap = new SalesforceSoapPartner(salesforce_get_api());
if ($soap->isConnected()) {
    $soap->query("SELECT FirstName FROM Contact LIMIT 1");
    ...
}

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 53441e6 on 7.x-3.x, 8.x-3.x, process-deleted-refactor authored by ceardach, committed by levelos:
    #1935612 by pcave, ceardach: Make an API call to refresh the access...

  • Commit 53441e6 on 7.x-3.x, 8.x-3.x, mapped-object-ui authored by ceardach, committed by levelos:
    #1935612 by pcave, ceardach: Make an API call to refresh the access...