Introduce cache drivers

[lcobucci]

Fixes #140

This provides a caching interface and implementations for the most common drivers (filesystem and APCu). Users can create their own implementation of the interface to support PSR-6/PSR-16/anything else.

[lcobucci]

@GrahamCampbell thank you for putting the time to review but, as I've tried to explain in the commit message, these implementations are naive on purpose. They're there to fulfil the needs of this lib and none of the multi-set/get/delete are actually used. Main idea was to not add any external dependency while giving people the flexibility to plug in the library of their choice.

[GrahamCampbell]

I really think it's best to provide nothing at all, than non-compliant implementations. The point of the PSR interop is not to have fewer dependencies, but to give the user a choice of what to use. Maybe just "suggest" a the psr cache implementation virtual package, and provide no implementations?

[lcobucci]

I get your point, though to not provide anything we'd have to break the previous API.

We can drop the null thing by reworking a few things but the simple file one must be kept for the bc reason.

So I defer this to @nikic

[nikic]

I don't really have a problem with breaking API for the next major version, if necessary. Some thoughts on this PR:

• As with pretty much anything that has "simple" in the library name (SOAP anyone?) the SimpleCache is not simple, and does much more than what this library actually needs. In particular, the only operations we need is a key-less fetch and store. As the SimpleCache interface (per @GrahamCampbell's comments) requires an involved implementation to satisfy all the constraints, might it make more sense to invert the relationship? That is, provide an adapter to SimpleCache, rather than the other way around.
• Generally speaking, the use of file-caching is optimal for this purpose. In conjunction with opcache, it is the most efficient way to cache this data. I am wondering what the motivation for using a different caching approach is.
• I suspect that the main motivation is cache invalidation, which is currently not handled by this library at all. It is assumed that the cache is invalidated / pre-populated on deploy, which may not always be the case. I'm not sure to what degree the migration to PSR-15 really addresses this problem.
• The other main problem I see people encounter is that closures can't be cached -- I don't think this is addressed by this change at all though.

[GrahamCampbell]

I am wondering what the motivation for using a different caching approach is.

I suppose the use case is php-fpm not having write permission to the disk?

[nikic]

@GrahamCampbell If php-fpm does not have write permissions, that implies that the application must be using pre-deploy cache population, where the FastRoute cache should also be generated. Possibly this is not really easy to integrate right now though?

[lcobucci]

I don't really have a problem with breaking API for the next major version, if necessary. Some thoughts on this PR:

• As with pretty much anything that has "simple" in the library name (SOAP anyone?) the SimpleCache is not simple, and does much more than what this library actually needs. In particular, the only operations we need is a key-less fetch and store. As the SimpleCache interface (per @GrahamCampbell's comments) requires an involved implementation to satisfy all the constraints, might it make more sense to invert the relationship? That is, provide an adapter to SimpleCache, rather than the other way around.
• Generally speaking, the use of file-caching is optimal for this purpose. In conjunction with opcache, it is the most efficient way to cache this data. I am wondering what the motivation for using a different caching approach is.

I understood from the original issue that people did want to use a different caching driver. We can definitely invert the relationship and provide an adapter for PSR-16.

I'd still provide the SimpleCache implementation (using our interface) because I do agree that it's better to rely on opcache for this. Renaming it might be good, though.

• I suspect that the main motivation is cache invalidation, which is currently not handled by this library at all. It is assumed that the cache is invalidated / pre-populated on deploy, which may not always be the case. I'm not sure to what degree the migration to PSR-15 really addresses this problem.

Invalidation is tricky, using other drivers it gets worse. Although I'd treat this kind of thing as we do for metadata cache in Doctrine ORM: let people control it outside of the application life cycle. The problem is not about how to invalidate but rather when.

• The other main problem I see people encounter is that closures can't be cached -- I don't think this is addressed by this change at all though.

This addresses #140 and not #141 indeed.

---

@nikic my main concern here is that I feel like you don't see value in supporting multiple cache drivers and am afraid that this might be a poor use of my time...

I'm wiling to rework this whole thing if you say it's something we want for v2 but if not let's just close it :+1:

[lcobucci]

@nikic I've reorganised my proposal according to the discussion. APCu has been added so we can have https://github.com/azjezz/benchmark-php-routing comparing things in a better way (and will probably be removed).

The file-based cache now does exclusive locks and atomic renames to avoid a corrupted state. The interface is also straightforward now, allowing us to add new implementations (e.g.: supporting closures).

Would you have a look, please?

[lcobucci]

@nikic I decided to get this merged for now. Please review it whenever you have the time for it and we can improve things.

[designermonkey]

While I appreciate the work that has gone into this, and fully understanding that I am not a contributor to the project, I would like to raise two concerns:

1. It's my opinion that one should not merge code contributions of their own into open source projects without agreement from other contributors, which there isn't at present.
2. None of this change is documented in the readme. Without documenting the caching abilities this adds, no one will know it is there to use.

[lcobucci]

@designermonkey thanks for sharing your thoughts.

1. It's my opinion that one should not merge code contributions of their own into open source projects without agreement from other contributors, which there isn't at present.

Indeed, not have a review isn't ideal. However, we've got extensive tests and benchmarks that give enough information not to block this PR. Nikita is extremely busy with many things and my hours to work on this particular project are limited. So, I had to make a call to move things forward.

I do invite you to review the code and share your findings here or on a PR.

2. None of this change is documented in the readme. Without documenting the caching abilities this adds, no one will know it is there to use.

I've indeed overlooked the documentation and will correct my mistake ASAP. In the meantime, feel free to send a PR - maybe you're faster than me.