feat: Data client #17
Conversation
danielgnh
force-pushed
the
feat/data-client
branch
from
4e3c696 to
4b7f81d
Compare
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new Data API client to the Polymarket PHP SDK, enabling developers to access market analytics, user positions, trade history, leaderboards, and other data-centric endpoints. The implementation follows the established architectural patterns in the codebase, with proper separation of concerns and consistent API design.
Changes:
- Adds a new
Dataclient with lazy instantiation and HTTP client injection support - Implements six resource classes for Data API endpoints: Positions, Trades, Activity, Analytics, Leaderboards, and Health
- Extends the
Configclass to support a configurable Data API base URL - Provides a comprehensive example script demonstrating all Data API features
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Config.php | Adds dataBaseUrl property with default value for the Data API endpoint |
| src/Client.php | Integrates Data client with constructor injection support and lazy instantiation accessor method |
| src/Data.php | Main Data API client class that manages HTTP client and provides access to all Data API resources |
| src/Resources/Data/Positions.php | Resource for fetching current positions, closed positions, and position values |
| src/Resources/Data/Trades.php | Resource for retrieving trade history and counting traded markets |
| src/Resources/Data/Activity.php | Resource for querying on-chain operations (trades, splits, merges, redemptions, etc.) |
| src/Resources/Data/Analytics.php | Resource for market analytics including holders, open interest, and live volume tracking |
| src/Resources/Data/Leaderboards.php | Resource for accessing trader rankings, builder rankings, and builder volume data |
| src/Resources/Data/Health.php | Resource for checking Data API operational status |
| examples/data-api-positions.php | Comprehensive example demonstrating all Data API functionality with proper error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Resources: | ||
| * - Health: API health check | ||
| * - Positions: Current and closed positions | ||
| * - Trades: Trade history and activity |
There was a problem hiding this comment.
The class docblock lists the available resources but omits the Activity resource. For completeness, add "Activity: On-chain operations including trades, splits, merges, redemptions" to the resources list in the docblock (around line 25), similar to how other resources are documented.
| * - Trades: Trade history and activity | |
| * - Trades: Trade history and activity | |
| * - Activity: On-chain operations including trades, splits, merges, redemptions |
|
|
||
| return $this->bridgeClient; | ||
| } | ||
|
|
There was a problem hiding this comment.
The data() method is missing a docblock comment. For consistency with other API client accessor methods in this class (gamma(), clob(), and bridge()), this method should have a docblock describing what it returns. Consider adding a docblock similar to the one used for bridge() method.
| /** | |
| * Get Data API client for market and trading data. | |
| * | |
| * @return Data | |
| */ |
| class Data | ||
| { | ||
| private HttpClientInterface $httpClient; | ||
|
|
||
| /** | ||
| * @param Config $config | ||
| * @param HttpClientInterface|null $httpClient | ||
| */ | ||
| public function __construct( | ||
| private readonly Config $config, | ||
| ?HttpClientInterface $httpClient = null | ||
| ) { | ||
| $this->httpClient = $httpClient ?? new GuzzleHttpClient($this->config->dataBaseUrl, $this->config); | ||
| } | ||
|
|
||
| public function health(): Health | ||
| { | ||
| return new Health($this->httpClient); | ||
| } | ||
|
|
||
| public function positions(): Positions | ||
| { | ||
| return new Positions($this->httpClient); | ||
| } | ||
|
|
||
| public function trades(): Trades | ||
| { | ||
| return new Trades($this->httpClient); | ||
| } | ||
|
|
||
| public function activity(): Activity | ||
| { | ||
| return new Activity($this->httpClient); | ||
| } | ||
|
|
||
| public function analytics(): Analytics | ||
| { | ||
| return new Analytics($this->httpClient); | ||
| } | ||
|
|
||
| public function leaderboards(): Leaderboards | ||
| { | ||
| return new Leaderboards($this->httpClient); | ||
| } | ||
| } |
There was a problem hiding this comment.
The Data API client and its resource classes lack test coverage. The codebase has comprehensive test coverage for other API clients (Gamma, Clob, Bridge), but no tests exist for the new Data API functionality. Consider adding test files similar to the existing pattern: tests for the Data client itself, and tests for each resource class (Positions, Trades, Activity, Analytics, Leaderboards, Health).
| try { | ||
| echo "10. Checking Data API health...\n"; | ||
| $health = $client->data()->health()->check(); | ||
| echo "Status: {$health['status']}\n"; |
There was a problem hiding this comment.
The code assumes that the API response contains a 'status' key without checking its existence first. While the try-catch block will handle exceptions, for consistency with other example files in this codebase (such as examples/bridge-deposit.php which uses isset() checks), consider adding a check before accessing the key, or use the null coalescing operator. For example: echo "Status: " . ($health['status'] ?? 'unknown') . "\n";
| echo "Status: {$health['status']}\n"; | |
| echo "Status: " . ($health['status'] ?? 'unknown') . "\n"; |
| try { | ||
| echo "3. Calculating total position value...\n"; | ||
| $value = $client->data()->positions()->value($walletAddress); | ||
| echo "Total value: \${$value['total']}\n\n"; |
There was a problem hiding this comment.
The code assumes that the API response contains a 'total' key without checking its existence first. While the try-catch block will handle exceptions, for consistency with other example files in this codebase (such as examples/bridge-deposit.php which uses isset() checks), consider adding a check before accessing the key, or use the null coalescing operator. For example: echo "Total value: $" . ($value['total'] ?? '0') . "\n\n";
| echo "Total value: \${$value['total']}\n\n"; | |
| echo "Total value: $" . ($value['total'] ?? '0') . "\n\n"; |
danielgnh
force-pushed
the
feat/data-client
branch
from
087a7b5 to
e089032
Compare
This pull request introduces a new Data API client to the codebase, enabling access to market analytics, user positions, trade history, leaderboards, and other data-centric endpoints. The changes include the addition of a dedicated
Dataclient, integration into the mainClientclass, and resource classes for each Data API feature. An example usage script is also provided to demonstrate the new functionality.New Data API client integration:
Dataclient (src/Data.php) to handle Data API operations such as positions, trades, analytics, leaderboards, and health checks.Dataclient into the mainClientclass, including support for custom HTTP clients and lazy instantiation. [1] [2] [3] [4]Configclass to support a separate Data API base URL via the newdataBaseUrlproperty. [1] [2]Data API resource classes:
Positions,Trades,Activity,Analytics,Leaderboards, andHealth, each providing typed methods for their respective API operations. [1] [2] [3] [4] [5] [6]Documentation and examples:
examples/data-api-positions.php) demonstrating how to use the Data API client to fetch user positions, analytics, trade history, leaderboard data, and API health status.