Update SchemaFactory to allow the annotation cache dir to be configurable

[MattBred]

Addresses #309

[moufmouf]

Hey @MattBred ,

Thanks a lot for this PR. If you don't mind, I'll keep it open for now.
I'm planning on working to add support for PHP 8 attributes, and I'll need some cache for those. The cache might be very different from the Doctrine cache used for Doctrine annotations, and I'm wondering if I should not mutualize both caches (meaning I would completely remove the Doctrine cache for annotations and replace it with a PSR-16 cache used elsewhere in the project...)

Not sure yet, but until I make my mind, I would like to avoid adding an additional parameter to the SchemaFactory constructor (since that parameter might disappear soon!)

[MattBred]

Sounds good @moufmouf .

I've found a few places in graphqlite + the symfony bundle that use the system temp dir instead of a configurable dir, so this PR was to address that (among some others I made)

[mdoelker]

@moufmouf Is there a good reason this does not use the cache instance provided via the constructor? This really caught me by surprise.

[moufmouf]

@mdoelker yes, there is (unfortunately!)

The cache for the annotation is a Doctrine Cache (that is not PSR-16 compatible, nor PSR-6 compatible).
It is also very fast. And we need a cache that is very fast, because we typically want very high performance for annotation fetching (we can hit this cache a big number of times per requests).

We could of course use an adapter to turn the Doctrine cache into a PSR-16 cache. The fact is that the PSR-16 cache is typically slower (if you look at the Symfony implementation, it needs to check if the key is valid according to PSR-16 rules, etc...). So it is solid, but I fear we would loose a few milliseconds here.

Moreover, I don't want users to configure a remote cache (like Redis or a DB backed cache) for annotations. That would completely destroy the performances of GraphQLite. So the only 2 viable options are APCu and PHPFile cache. So I decided to hard code those in the SchemaFactory.

[moufmouf]

Hey @MattBred ,

I'm 👍 on merging this once you move the $annotationCacheDir parameter into a setter (see comment above)

Thanks a lot for your work!

[moufmouf]

@MattBred Would you mind moving the $annotationCacheDir parameter into a setter?

The SchemaFactory class is typically following the "builder" pattern and any non compulsory parameter is configured via a setter.

[MattBred]

Sounds good, I will take care of that today

[MattBred]

@moufmouf Updated

[mdoelker]

Thanks for the explanation and for proceeding with this. Btw, does it make sense to check the apcu_enabled() func? Not too familiar with APCu.

$cache = function_exists('apcu_enabled') && apcu_enabled() ? new ApcuCache() : new PhpFileCache($cacheDir . '/graphqlite.' . crc32(__DIR__));

Returns TRUE when APCu is usable in the current environment, FALSE otherwise.

[moufmouf]

@mdoelker It does make a lot of sense, indeed!

Do you want to make add a comment with this suggestion in this PR so that @MattBred can merge it?

[MattBred]

@moufmouf Have a look now, should be good to go?

[moufmouf]

Excellent! Thanks a lot for your contribution!