Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Remove default Graph API version fallback #643

Merged
merged 14 commits into from
Dec 23, 2016
Merged

Remove default Graph API version fallback #643

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0f3ba5b
Remove default Graph API version fallback and make default_graph_vers…
SammyK Aug 29, 2016
1db2588
Merge branch 'master' into patch-608-force-graph-version
SammyK Dec 16, 2016
883982d
Fix spacing in docblock
SammyK Dec 16, 2016
a652499
Add default_graph_version option back with null default for documenta…
SammyK Dec 16, 2016
4d8edbe
Check for emptiness with ! for consistency
SammyK Dec 16, 2016
fd40369
Allow FacebookRequest::$graphVersion to be null
SammyK Dec 16, 2016
7e222e8
Make FacebookUrlManipulator::forceSlashPrefix() always return a string
SammyK Dec 16, 2016
967246f
Update default Graph API version to v2.8 in tests
SammyK Dec 16, 2016
bc15ca9
Remove default Graph version fallback since unversioned calls to Grap…
SammyK Dec 16, 2016
bf90316
Add the default_graph_version option where required
SammyK Dec 16, 2016
1362e0e
Add test to make sure an InvalidArgumentException is thrown if no def…
SammyK Dec 16, 2016
041164b
Update request test to check for versioned request
SammyK Dec 23, 2016
4efc534
Remove unneeded namespaces
SammyK Dec 23, 2016
3f9fd16
Remove references to the default Graph API constant in tests
SammyK Dec 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Facebook/Authentication/OAuth2Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ class OAuth2Client
/**
* @param FacebookApp $app
* @param FacebookClient $client
* @param string|null $graphVersion The version of the Graph API to use.
* @param string $graphVersion The version of the Graph API to use.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indentation :)

*/
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion = null)
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion)
{
$this->app = $app;
$this->client = $client;
$this->graphVersion = $graphVersion ?: Facebook::DEFAULT_GRAPH_VERSION;
$this->graphVersion = $graphVersion;
}

/**
Expand Down
10 changes: 3 additions & 7 deletions src/Facebook/Facebook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ class Facebook
*/
const VERSION = '6.0-dev';

/**
* @const string Default Graph API version for requests.
*/
const DEFAULT_GRAPH_VERSION = 'v2.7';

/**
* @const string The name of the environment variable that contains the app ID.
*/
Expand Down Expand Up @@ -127,7 +122,6 @@ public function __construct(array $config = [])
$config = array_merge([
'app_id' => getenv(static::APP_ID_ENV_NAME),
'app_secret' => getenv(static::APP_SECRET_ENV_NAME),
'default_graph_version' => static::DEFAULT_GRAPH_VERSION,
'enable_beta_mode' => false,
'http_client_handler' => null,
'persistent_data_handler' => null,
Expand All @@ -141,6 +135,9 @@ public function __construct(array $config = [])
if (!$config['app_secret']) {
throw new FacebookSDKException('Required "app_secret" key not supplied in config and could not find fallback environment variable "' . static::APP_SECRET_ENV_NAME . '"');
}
if (empty($config['default_graph_version'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you keep the key above, then I'll check here like above, if (!$config['default_graph_version']) {, for consistency

throw new \InvalidArgumentException('Required "default_graph_version" key not supplied in config');
}

$this->app = new FacebookApp($config['app_id'], $config['app_secret']);
$this->client = new FacebookClient(
Expand All @@ -159,7 +156,6 @@ public function __construct(array $config = [])
$this->setDefaultAccessToken($config['default_access_token']);
}

// @todo v6: Throw an InvalidArgumentException if "default_graph_version" is not set
$this->defaultGraphVersion = $config['default_graph_version'];
}

Expand Down
3 changes: 2 additions & 1 deletion src/Facebook/FacebookRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public function __construct(FacebookApp $app = null, $accessToken = null, $metho
$this->setEndpoint($endpoint);
$this->setParams($params);
$this->setETag($eTag);
$this->graphVersion = $graphVersion ?: Facebook::DEFAULT_GRAPH_VERSION;
// @TODO Remove this fall-back
$this->graphVersion = $graphVersion ?: 'v2.7';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the way to handle this case is to allow null graph version in requests. This parameter is here to override the version given in the client, so it can be null, we just have to take care of this value when sending the request, falling back to the client version when this is null :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello yguedidi! I have no certainties about this case version 2.8 yet and icoguinita for me, but I resolve to be with this team for sure has brilhates minds, we try faser tests, for and studying the case to come to perfection. I trust you guys the importate and not waste time, bad weather do quick pass worth every second. with an impeccable work. ok let's see what happens reversing from 2.7 to 2.8 ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mozarliker

}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Facebook/Helpers/FacebookPageTabHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ class FacebookPageTabHelper extends FacebookCanvasHelper
*
* @param FacebookApp $app The FacebookApp entity.
* @param FacebookClient $client The client to make HTTP requests.
* @param string|null $graphVersion The version of Graph to use.
* @param string $graphVersion The version of Graph to use.
*/
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion = null)
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion)
{
parent::__construct($app, $client, $graphVersion);

Expand Down
5 changes: 2 additions & 3 deletions src/Facebook/Helpers/FacebookSignedRequestFromInputHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,11 @@ abstract class FacebookSignedRequestFromInputHelper
*
* @param FacebookApp $app The FacebookApp entity.
* @param FacebookClient $client The client to make HTTP requests.
* @param string|null $graphVersion The version of Graph to use.
* @param string $graphVersion The version of Graph to use.
*/
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion = null)
public function __construct(FacebookApp $app, FacebookClient $client, $graphVersion)
{
$this->app = $app;
$graphVersion = $graphVersion ?: Facebook::DEFAULT_GRAPH_VERSION;
$this->oAuth2Client = new OAuth2Client($this->app, $client, $graphVersion);

$this->instantiateSignedRequest();
Expand Down
8 changes: 4 additions & 4 deletions tests/FacebookBatchRequestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public function testBatchRequestEntitiesProperlyGetConvertedToAnArray($request,
public function requestsAndExpectedResponsesProvider()
{
$headers = $this->defaultHeaders();
$apiVersion = Facebook::DEFAULT_GRAPH_VERSION;
$apiVersion = FacebookTest::DEFAULT_GRAPH_VERSION;

return [
[
Expand Down Expand Up @@ -273,7 +273,7 @@ public function testBatchRequestsWithFilesGetConvertedToAnArray()
$this->assertEquals([
'headers' => $this->defaultHeaders(),
'method' => 'POST',
'relative_url' => '/' . Facebook::DEFAULT_GRAPH_VERSION . '/bar',
'relative_url' => '/' . FacebookTest::DEFAULT_GRAPH_VERSION . '/bar',
'body' => 'message=foobar&access_token=foo_token&appsecret_proof=df4256903ba4e23636cc142117aa632133d75c642bd2a68955be1443bd14deb9',
'name' => 'foo_name',
'attached_files' => $attachedFiles,
Expand All @@ -290,7 +290,7 @@ public function testPreppingABatchRequestProperlySetsThePostParams()
$params = $batchRequest->getParams();

$expectedHeaders = json_encode($this->defaultHeaders());
$version = Facebook::DEFAULT_GRAPH_VERSION;
$version = FacebookTest::DEFAULT_GRAPH_VERSION;
$expectedBatchParams = [
'batch' => '[{"headers":' . $expectedHeaders . ',"method":"GET","relative_url":"\\/' . $version . '\\/foo?access_token=bar_token&appsecret_proof=2ceec40b7b9fd7d38fff1767b766bcc6b1f9feb378febac4612c156e6a8354bd","name":"foo_name"},'
. '{"headers":' . $expectedHeaders . ',"method":"POST","relative_url":"\\/' . $version . '\\/bar","body":"foo=bar&access_token=foo_token&appsecret_proof=df4256903ba4e23636cc142117aa632133d75c642bd2a68955be1443bd14deb9"}]',
Expand All @@ -317,7 +317,7 @@ public function testPreppingABatchRequestProperlyMovesTheFiles()
$attachedFiles = implode(',', array_keys($files));

$expectedHeaders = json_encode($this->defaultHeaders());
$version = Facebook::DEFAULT_GRAPH_VERSION;
$version = FacebookTest::DEFAULT_GRAPH_VERSION;
$expectedBatchParams = [
'batch' => '[{"headers":' . $expectedHeaders . ',"method":"GET","relative_url":"\\/' . $version . '\\/foo?access_token=bar_token&appsecret_proof=2ceec40b7b9fd7d38fff1767b766bcc6b1f9feb378febac4612c156e6a8354bd","name":"foo_name"},'
. '{"headers":' . $expectedHeaders . ',"method":"POST","relative_url":"\\/' . $version . '\\/me\\/photos","body":"message=foobar&access_token=foo_token&appsecret_proof=df4256903ba4e23636cc142117aa632133d75c642bd2a68955be1443bd14deb9","attached_files":"' . $attachedFiles . '"}]',
Expand Down
2 changes: 1 addition & 1 deletion tests/FacebookClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public function testAFacebookBatchRequestWillProperlyBatchFiles()

list($url, $method, $headers, $body) = $this->fbClient->prepareRequestMessage($fbBatchRequest);

$this->assertEquals(FacebookClient::BASE_GRAPH_VIDEO_URL . '/' . Facebook::DEFAULT_GRAPH_VERSION, $url);
$this->assertEquals(FacebookClient::BASE_GRAPH_VIDEO_URL . '/' . FacebookTest::DEFAULT_GRAPH_VERSION, $url);
$this->assertEquals('POST', $method);
$this->assertContains('multipart/form-data; boundary=', $headers['Content-Type']);
$this->assertContains('Content-Disposition: form-data; name="batch"', $body);
Expand Down
6 changes: 3 additions & 3 deletions tests/FacebookRequestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,14 @@ public function testAProperUrlWillBeGenerated()

$getUrl = $getRequest->getUrl();
$expectedParams = 'foo=bar&access_token=foo_token&appsecret_proof=df4256903ba4e23636cc142117aa632133d75c642bd2a68955be1443bd14deb9';
$expectedUrl = '/' . Facebook::DEFAULT_GRAPH_VERSION . '/foo?' . $expectedParams;
$expectedUrl = '/' . FacebookTest::DEFAULT_GRAPH_VERSION . '/foo?' . $expectedParams;

$this->assertEquals($expectedUrl, $getUrl);

$postRequest = new FacebookRequest($app, 'foo_token', 'POST', '/bar', ['foo' => 'bar']);

$postUrl = $postRequest->getUrl();
$expectedUrl = '/' . Facebook::DEFAULT_GRAPH_VERSION . '/bar';
$expectedUrl = '/' . FacebookTest::DEFAULT_GRAPH_VERSION . '/bar';

$this->assertEquals($expectedUrl, $postUrl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a case for a versionned request, as it useful in the case of batched request to use a different version than the one of the master request :)

}
Expand All @@ -156,7 +156,7 @@ public function testAuthenticationParamsAreStrippedAndReapplied()
$url = $request->getUrl();

$expectedParams = 'bar=baz&access_token=foo_token&appsecret_proof=df4256903ba4e23636cc142117aa632133d75c642bd2a68955be1443bd14deb9';
$expectedUrl = '/' . Facebook::DEFAULT_GRAPH_VERSION . '/foo?' . $expectedParams;
$expectedUrl = '/' . FacebookTest::DEFAULT_GRAPH_VERSION . '/foo?' . $expectedParams;
$this->assertEquals($expectedUrl, $url);

$params = $request->getParams();
Expand Down
2 changes: 2 additions & 0 deletions tests/FacebookTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

class FacebookTest extends \PHPUnit_Framework_TestCase
{
const DEFAULT_GRAPH_VERSION = 'v2.7';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be changed to 2.8 :)


protected $config = [
'app_id' => '1337',
'app_secret' => 'foo_secret',
Expand Down
3 changes: 2 additions & 1 deletion tests/Helpers/FacebookCanvasHelperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

use Facebook\FacebookApp;
use Facebook\FacebookClient;
use Facebook\Tests\FacebookTest;
use Facebook\Helpers\FacebookCanvasHelper;

class FacebookCanvasHelperTest extends \PHPUnit_Framework_TestCase
Expand All @@ -39,7 +40,7 @@ class FacebookCanvasHelperTest extends \PHPUnit_Framework_TestCase
protected function setUp()
{
$app = new FacebookApp('123', 'foo_app_secret');
$this->helper = new FacebookCanvasHelper($app, new FacebookClient());
$this->helper = new FacebookCanvasHelper($app, new FacebookClient(), FacebookTest::DEFAULT_GRAPH_VERSION);
}

public function testSignedRequestDataCanBeRetrievedFromPostData()
Expand Down
3 changes: 2 additions & 1 deletion tests/Helpers/FacebookJavaScriptHelperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

use Facebook\FacebookApp;
use Facebook\FacebookClient;
use Facebook\Tests\FacebookTest;
use Facebook\Helpers\FacebookJavaScriptHelper;

class FacebookJavaScriptHelperTest extends \PHPUnit_Framework_TestCase
Expand All @@ -36,7 +37,7 @@ public function testARawSignedRequestCanBeRetrievedFromCookieData()
$_COOKIE['fbsr_123'] = $this->rawSignedRequestAuthorized;

$app = new FacebookApp('123', 'foo_app_secret');
$helper = new FacebookJavaScriptHelper($app, new FacebookClient());
$helper = new FacebookJavaScriptHelper($app, new FacebookClient(), FacebookTest::DEFAULT_GRAPH_VERSION);

$rawSignedRequest = $helper->getRawSignedRequest();

Expand Down
3 changes: 2 additions & 1 deletion tests/Helpers/FacebookPageTabHelperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

use Facebook\FacebookApp;
use Facebook\FacebookClient;
use Facebook\Tests\FacebookTest;
use Facebook\Helpers\FacebookPageTabHelper;

class FacebookPageTabHelperTest extends \PHPUnit_Framework_TestCase
Expand All @@ -36,7 +37,7 @@ public function testPageDataCanBeAccessed()
$_POST['signed_request'] = $this->rawSignedRequestAuthorized;

$app = new FacebookApp('123', 'foo_app_secret');
$helper = new FacebookPageTabHelper($app, new FacebookClient());
$helper = new FacebookPageTabHelper($app, new FacebookClient(), FacebookTest::DEFAULT_GRAPH_VERSION);

$this->assertFalse($helper->isAdmin());
$this->assertEquals('42', $helper->getPageId());
Expand Down
3 changes: 2 additions & 1 deletion tests/Helpers/FacebookSignedRequestFromInputHelperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace Facebook\Tests\Helpers;

use Facebook\FacebookApp;
use Facebook\Tests\FacebookTest;
use Facebook\Tests\Fixtures\FooSignedRequestHelper;
use Facebook\Tests\Fixtures\FooSignedRequestHelperFacebookClient;

Expand All @@ -41,7 +42,7 @@ class FacebookSignedRequestFromInputHelperTest extends \PHPUnit_Framework_TestCa
protected function setUp()
{
$app = new FacebookApp('123', 'foo_app_secret');
$this->helper = new FooSignedRequestHelper($app, new FooSignedRequestHelperFacebookClient());
$this->helper = new FooSignedRequestHelper($app, new FooSignedRequestHelperFacebookClient(), FacebookTest::DEFAULT_GRAPH_VERSION);
}

public function testSignedRequestDataCanBeRetrievedFromPostData()
Expand Down