-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add assets, and exchanges API #7
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Collection; | ||
|
||
use ScriptFUSION\Porter\Collection\CountableProviderRecords; | ||
use ScriptFUSION\Porter\Provider\Resource\ProviderResource; | ||
|
||
class AssetsRecord extends CountableProviderRecords | ||
{ | ||
public function __construct( | ||
\Iterator $providerRecords, | ||
int $count, | ||
ProviderResource $resource | ||
) { | ||
parent::__construct($providerRecords, $count, $resource); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Collection; | ||
|
||
use ScriptFUSION\Porter\Collection\CountableProviderRecords; | ||
use ScriptFUSION\Porter\Provider\Resource\ProviderResource; | ||
|
||
class ExchangesRecord extends CountableProviderRecords | ||
{ | ||
public function __construct( | ||
\Iterator $providerRecords, | ||
int $count, | ||
ProviderResource $resource | ||
) { | ||
parent::__construct($providerRecords, $count, $resource); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Resource; | ||
|
||
use PayCrypto\CryptoMonitor; | ||
use PayCrypto\Collection\AssetsRecord; | ||
use ScriptFUSION\Porter\Connector\ImportConnector; | ||
use ScriptFUSION\Porter\Provider\Patreon\Collection\PledgeRecords; | ||
use ScriptFUSION\Porter\Provider\Patreon\PatreonProvider; | ||
use ScriptFUSION\Porter\Provider\Resource\ProviderResource; | ||
|
||
class GetAssets implements ProviderResource | ||
{ | ||
private $apiKey; | ||
|
||
public function __construct(string $apiKey) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is bad design to specify the API key in the resource because it must be duplicated into every resource. Specify it in the connector instead, to eliminate this duplication. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I set the encrypted env variable in Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I also use the |
||
{ | ||
$this->apiKey = $apiKey; | ||
} | ||
|
||
public function getProviderClassName(): string | ||
{ | ||
return CryptoMonitor::class; | ||
} | ||
|
||
public function fetch(ImportConnector $connector): \Iterator | ||
{ | ||
$response = \json_decode( | ||
(string) $connector->fetch( | ||
CryptoMonitor::buildExchangeApiUrl( | ||
"v1/assets" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double quotes not needed here. Prefer single quotes where interpolation not used to clearly express intent. Applies throughout this repository. |
||
) | ||
), | ||
true | ||
); | ||
|
||
$assets = new \ArrayIterator($response); | ||
|
||
return new AssetsRecord($assets, count($assets), $this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instantiate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use the |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Resource; | ||
|
||
use PayCrypto\CryptoMonitor; | ||
use PayCrypto\Collection\ExchangesRecord; | ||
use ScriptFUSION\Porter\Connector\ImportConnector; | ||
use ScriptFUSION\Porter\Provider\Patreon\Collection\PledgeRecords; | ||
use ScriptFUSION\Porter\Provider\Patreon\PatreonProvider; | ||
use ScriptFUSION\Porter\Provider\Resource\ProviderResource; | ||
|
||
class GetExchanges implements ProviderResource | ||
{ | ||
private $apiKey; | ||
|
||
public function __construct(string $apiKey) | ||
{ | ||
$this->apiKey = $apiKey; | ||
} | ||
|
||
public function getProviderClassName(): string | ||
{ | ||
return CryptoMonitor::class; | ||
} | ||
|
||
public function fetch(ImportConnector $connector): \Iterator | ||
{ | ||
$response = \json_decode( | ||
(string) $connector->fetch( | ||
CryptoMonitor::buildExchangeApiUrl( | ||
"v1/exchanges" | ||
) | ||
), | ||
true | ||
); | ||
|
||
$exchanges = new \ArrayIterator($response); | ||
|
||
return new ExchangesRecord($exchanges, count($exchanges), $this); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Tests; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use PayCrypto\Resource\GetAssets; | ||
use PayCrypto\Collection\AssetsRecord; | ||
use ScriptFUSION\Porter\Specification\ImportSpecification; | ||
|
||
final class GetAssetsTest extends TestCase | ||
{ | ||
/** @var $apiKey This is the Coin API Key for test environment*/ | ||
private $apiKey = '4E861687-19D6-4894-87B9-E785B1EE3900'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned before, you shouldn't be committing keys to your repository. They can be used by anyone, which is a security problem. Pass keys at invocation time via environment variables instead. This also fixes the duplication issue. |
||
|
||
public function testGetAssets() | ||
{ | ||
/** @var AssetsRecord */ | ||
$assets = FixtureFactory::createPorter()->import(new ImportSpecification(new GetAssets($this->apiKey))); | ||
|
||
foreach ($assets as $assetRecord) { | ||
$this->assertArrayHasKey('asset_id', $assetRecord); | ||
$this->assertArrayHasKey('name', $assetRecord); | ||
$this->assertArrayHasKey('type_is_crypto', $assetRecord); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace PayCrypto\Tests; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use PayCrypto\Resource\GetExchanges; | ||
use PayCrypto\Collection\ExchangesRecord; | ||
use ScriptFUSION\Porter\Specification\ImportSpecification; | ||
|
||
final class GetExchangesTest extends TestCase | ||
{ | ||
/** @var $apiKey This is the Coin API Key for test environment*/ | ||
private $apiKey = '4E861687-19D6-4894-87B9-E785B1EE3900'; | ||
|
||
public function testGetExchanges() | ||
{ | ||
/** @var ExchangesRecord */ | ||
$exchanges = FixtureFactory::createPorter()->import(new ImportSpecification(new GetExchanges($this->apiKey))); | ||
|
||
foreach ($exchanges as $exchangeRecord) { | ||
$this->assertArrayHasKey('exchange_id', $exchangeRecord); | ||
$this->assertArrayHasKey('website', $exchangeRecord); | ||
$this->assertArrayHasKey('name', $exchangeRecord); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class and the
ExchangesRecord
class doesn't do anything and should be removed. Just invokenew CountableProviderRecords
directly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you show some sample about invoking
CountableProviderRecords
class directly?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that. Using the
CountableProviderRecords
directly because it has no any additional argument to be assigned.