-
Notifications
You must be signed in to change notification settings - Fork 42
Description
Hey, I'm working on some changes and I found that this change is unfortunately a bit problematic:
- the interface itself is now misnamed (
ApiClientAwareContextshould be more likeApiClientInitializer(Aware?)Context) - extensions/contexts that actually want to use or inspect the api client can't do that anymore, since it is 'hidden' inside ApiContext - unless they extend ApiContext which makes the ApiClientAwareContext injector quite less useful and might not even work, if such a class needs to extend something else (of course it's possible to have a lean class extending this that cooperates with other classes, but again makes the whole concept way less useful).
- I've seen in that commit how the config can be 'extended' by making a child class call the parent method with a different config, but that also means users have only one chance of extending this (and/or can only use at most one third-party extension that needs to change the config) - at least as I understand.
In the ideal world (or at least in my opinion), it should work like so:
interface ApiClientConfigMutatorContext
{
public function mutateApiClientConfig(array $config): array;
}
interface ApiClientAwareContext
{
public function setApiClient(ClientInterface $client): void;
}
interface ApiRequestAwareContext // *
{
public function setApiRequest(RequestInterface $request): void;
}
interface ApiResponseAwareContext // *
{
public function setApiResponse(?ResponseInterface $response): void;
}
class ApiClientConfigMutator implements ContextInitializer
{
public function __construct(private array $config)
{
}
public function initializeContext(Context $context): void
{
if ($context instanceof ApiClientConfigMutatorContext) {
$this->config = $context->mutateApiClientConfig($this->config);
}
}
public function getConfig(): array
{
return $this->config;
}
}
class ApiClientAwareContextInitializer implements ContextInitializer
{
private ClientInterface $client;
public function __construct(ApiClientConfigMutator $configMutatorInitializer)
{
$this->client = new Client($configMutatorInitializer->getConfig());
}
public function initializeContext(Context $context)
{
if ($context instanceof ApiClientAwareContext) {
$context->setApiClient($this->client);
}
}
}
// more initializers for request/response or something (not sure how that should work TBH) ** It would be nice if contexts can somehow receive the request and the response, but I'm not sure how that can be achieved with context initializers. In theory, something(most probably a context) creates or requests creation of such objects and then somehow spread them around with a context initializer.
Edit: For that last point, a possibility is to share an "ApiState" object that contains the client, request and response. That state is shared across contexts with an initializer and they can all do whatever they want with the state (although it feels like the client should be readonly).
class ApiState {
public function __construct(
public readonly ClientInterface $client,
public RequestInterface $request,
public null|ResponseInterface $response,
){}
}
class ApiStateAwareContextInitializer implements ContextInitializer
{
private ApiState $state;
public function __construct(ApiClientConfigMutator $configMutatorInitializer)
{
$this->state = new ApiState(
new Client($configMutatorInitializer->getConfig())
new Request(), // the initial, empty request
null,
);
}
public function initializeContext(Context $context)
{
if ($context instanceof ApiStateAwareContext) {
$context->setApiState($this->state);
}
}
}