search

Kdyby / DoctrineCache

Doctrine 2 Cache integration into Nette Framework
https://packagist.org/packages/kdyby/doctrine-cache
Other
10 stars 21 forks source link

Add new caching options #9

Closed enumag closed 8 years ago

enumag commented 8 years ago

Part of https://github.com/Kdyby/Annotations/issues/7, not ready to merge yet.

fprochazka commented 8 years ago

Looks good. I'd merge this and tag new version, what do you think?

enumag commented 8 years ago

@fprochazka I'd like to test if all the PRs together really do what I've intended. Give me a day or so. ;-)

fprochazka commented 8 years ago

@enumag works for me, I wanned to go sleep anyway :) GJ!

enumag commented 8 years ago

I'm trying the filesystem cache now and it creates really deep directory structures. For example:

/temp/cache/Doctrine.Validator/41/d3/9f/97/c4/89/2f/9d/cf/a3/1f/01/71/1c/fc/98/6b/d2/d5/2f/1f/79/a9/46/68/a3/9f/38/8f/b4/ed/b7/DoctrineNamespaceCacheKey[Kdyby_kdyby-validator-cache-validator_c4bbc638].doctrinecache.data

I really hate this behaviour. :-/

It's because of this line. What is your opinion about this behaviour? Fortunately it's not all that difficult to eliminate this problem. I can extend the FilesystemCache and replace the getFilename method with different implementation. I'm not completely sure what the implementation should look like though. Completely flat structure is not a good idea either so we need to decide on a compromise or leave it as is.

enumag commented 8 years ago

@fprochazka Aside from the issue above the PRs indeed do what I've intended. :-)

enumag commented 8 years ago

Maybe we should just wait for doctrine/cache v1.5 (see https://github.com/doctrine/cache/pull/94 for details).

enumag commented 8 years ago

@fprochazka What's your opinion?

fprochazka commented 8 years ago

@enumag I'd extend the FilesystemCache and override the method with custom impl.

enumag commented 8 years ago

@fprochazka I'd like this merged even without the fix for FilesystemCache. It won't be default anywhere unless we make it that way - which we obviously won't until the issue is solved. It can still work as an optional caching option in the meantime.

fprochazka commented 8 years ago

Agreed.