overblog / GraphQLBundle

This bundle provides tools to build a complete GraphQL API server in your Symfony App.
MIT License
784 stars 221 forks source link

Paginator class depends on internal state #853

Open piotrkreft opened 3 years ago

piotrkreft commented 3 years ago
Q A
Bug report? yes
Feature request? yes
BC Break report? no
RFC? no
Version/Branch 0.13

Class Overblog\GraphQLBundle\Relay\Connection\Paginator depends on internal totalCount state, if fetched already. I find this to be sort of an antipattern in several situations, especially when dealing with DI and class being a service as it brings an additional potential place of failure being harder to find in simple tests even.

Would you consider a change? If yes I'd obviously provide a suitable code for it. If BC desired the solution could be configurable behavior via an optional constructor parameter though not sure whether it's a good practice tbh. Alternatively, an interface and cache decorator would be more suitable perhaps.

Best regards

piotrkreft commented 3 years ago

As a suggested #855 solution I decided to go with a fully compatible approach and ready out-of-the-box cache callable. Callable implements ResetInterface so it does grant the developer a possibility to reset cache state and also will be reused by Symfony HTTPKernel if used as service to be reset per request - desired if for an instance one would user DI service and long running process like Roadrunner etc.