Shopify / Shopify/shopify-api-php

Remove Context Superclass – Improve Modularity and Testability

Open
#399 5 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
473
Forks
193
Avg merge
5h 8m
Merged PRs (30d)
1

Description

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:

  1. 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)) {
    throw new MissingArgumentException('Missing access token when creating GraphQL client');
}

Proposed Fix: Inject Configuration object instead of relying on Context:

class Graphql {
    public function __construct(
        private readonly Configuration $config,
        private readonly HttpClient $httpClient,
        private readonly ?string $token = null
    ) { }
}
  1. Http Client

Problem: Uses Context::$HTTP_CLIENT_FACTORY and Context::$USER_AGENT_PREFIX to generate requests.

$client = Context::$HTTP_CLIENT_FACTORY->client();

Proposed Fix: Pass HttpClientFactory and UserAgentProvider via constructor:

class Http {
    public function __construct(
        private readonly HttpClientFactory $clientFactory,
        private readonly UserAgentProvider $userAgentProvider
    ) { }
}

Checklist

  • Refactor Context into a Configuration object that can be injected.
  • Update Graphql, Http, and other dependent classes to use dependency injection.
  • Ensure backward compatibility where possible.
  • Improve testability by allowing mock injection.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing Context usage across Graphql, Http, and the other dependent classes named in the issue, then review the existing tests around those components. Determine the dependency boundaries and the backward-compatibility expectations before proposing a configuration and injection design. Done means Context no longer provides the listed global dependencies, affected classes accept replaceable dependencies, and tests can inject mocks.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.