You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Context class in the Shopify PHP API library introduces global state, making the codebase harder to test, maintain, and extend. It violates multiple SOLID principles, specifically Single Responsibility Principle (SRP) and Dependency Inversion Principle (DIP). This issue proposes refactoring to remove Context and replace it with dependency injection.
Motivation
Currently, many classes depend on Context for critical API information like:
• API keys ($API_KEY, $API_SECRET_KEY)
• Session storage ($SESSION_STORAGE)
• HTTP Client Factory ($HTTP_CLIENT_FACTORY)
• API version ($API_VERSION)
• Retry logic ($RETRY_TIME_IN_SECONDS)
This leads to tightly coupled code, where:
1. Global state makes debugging difficult – Modifying Context affects all instances, leading to unpredictable behavior.
2. Unit testing is complicated – Tests must mock the static Context state rather than injecting dependencies.
3. Extensibility is restricted – Developers cannot easily replace components like HTTP clients or storage mechanisms.
Area
The Context dependency is spread across multiple classes. Some key examples:
Graphql Client
Problem: Uses Context::$IS_PRIVATE_APP and Context::$API_VERSION, making it impossible to instantiate Graphql client without relying on Context.
if (!Context::$IS_PRIVATE_APP && empty($token)) {
thrownewMissingArgumentException('Missing access token when creating GraphQL client');
}
Proposed Fix: Inject Configuration object instead of relying on Context:
Overview/summary
The
Context
class in the Shopify PHP API library introduces global state, making the codebase harder to test, maintain, and extend. It violates multiple SOLID principles, specifically Single Responsibility Principle (SRP) and Dependency Inversion Principle (DIP). This issue proposes refactoring to remove Context and replace it with dependency injection.Motivation
Currently, many classes depend on Context for critical API information like:
• API keys (
$API_KEY
,$API_SECRET_KEY
)• Session storage (
$SESSION_STORAGE
)• HTTP Client Factory (
$HTTP_CLIENT_FACTORY
)• API version (
$API_VERSION
)• Retry logic (
$RETRY_TIME_IN_SECONDS
)This leads to tightly coupled code, where:
1. Global state makes debugging difficult – Modifying
Context
affects all instances, leading to unpredictable behavior.2. Unit testing is complicated – Tests must mock the static
Context
state rather than injecting dependencies.3. Extensibility is restricted – Developers cannot easily replace components like HTTP clients or storage mechanisms.
Area
The
Context
dependency is spread across multiple classes. Some key examples:Problem: Uses
Context::$IS_PRIVATE_APP
andContext::$API_VERSION
, making it impossible to instantiateGraphql
client without relying onContext
.Proposed Fix: Inject
Configuration
object instead of relying onContext
:Problem: Uses
Context::$HTTP_CLIENT_FACTORY
andContext::$USER_AGENT_PREFIX
to generate requests.Proposed Fix: Pass
HttpClientFactory
andUserAgentProvider
via constructor:Checklist
The text was updated successfully, but these errors were encountered: