Fixed bug with the difference of key format between generate_cache_id and clean_cache!

[t-oginogin]

Hello, I found bug in clean_cache! method.

CarrierWave::Storage::File.clean_cache! always cleans all file caches before expired.

Because CarrierWave::Storage::File.clean_cache! doesn't refer TIMEINT but PID.
CarrierWave::Uploader::Cache.generate_cache_id returns key format like "TIMEINT-PID-COUNTER-RND".
It should refer 1st element(TIMEINT).
But CarrierWave::Storage::File.clean_cache! try match /(\d+)-\d+-\d+$/ .
So it matched 2nd element(PID) which shows the time when cache directory created.
And Time.at(*time) always returns like 1970/mm/dd, then cache directory is all expired.

[mshibuya]

Could you write a spec that will fail without this fix?

[t-oginogin]

@mshibuya The spec included this commit( spec/storage/file_spec.rb ) is fail without this fix.
The key format of expected data was wrong before this fix.

Expected data(File)	Result
"#{five_days_ago_int}-234-2213" (expected data is wrong)	success
"#{five_days_ago_int}-234-1234-2213" (expected data is right)	fail

But spec/storage/fog_helper.rb is not fail. Because Strage::Fog.clean_cache! matches wrong key format too.

Expected data(Fog)	Result
"#{five_days_ago_int}-234-2213" (expected data is wrong)	success
"#{five_days_ago_int}-234-1234-2213" (expected data is right)	success

• without this fix

Class	Regexp	matched
File	/(\d+)-\d+-\d+$/	(PID)-COUNTER-RND
Fog	/(\d+)-\d+-\d+/	(TIMEINT)-PID-COUNTER

• this fix

Class	Regexp	matched
File	/(\d+)-\d+-\d+-\d+/	(TIMEINT)-PID-COUNTER-RND
Fog	/(\d+)-\d+-\d+-\d+/	(TIMEINT)-PID-COUNTER-RND

Does that answer your question?

[mshibuya]

I know the purpose of this fix, but we need unit test to make sure this bug never occurs in the future.
All specs are passing right now, so it means our test isn't fully covered to detect this problem.

[t-oginogin]

I understand it means.
I try to write spec.

[t-oginogin]

@mshibuya I wrote specs to detect this problem.

[mshibuya]

Perfect, thanks!