Page MenuHomePhabricator

Make the Files "TTL" API more structured
ClosedPublic

Authored by epriestley on Apr 4 2017, 7:28 PM.
Tags
None
Referenced Files
F14064478: D17616.diff
Mon, Nov 18, 11:39 PM
F14052873: D17616.diff
Fri, Nov 15, 11:14 AM
F14040189: D17616.diff
Mon, Nov 11, 7:42 AM
F14022529: D17616.diff
Wed, Nov 6, 6:03 PM
F14018363: D17616.id42379.diff
Tue, Nov 5, 9:54 AM
F14017065: D17616.id42379.diff
Mon, Nov 4, 2:05 PM
F14014831: D17616.diff
Sun, Nov 3, 8:45 AM
F14005583: D17616.id42379.diff
Sun, Oct 27, 4:10 PM
Subscribers

Details

Summary

Ref T11357. When creating a file, callers can currently specify a ttl. However, it isn't unambiguous what you're supposed to pass, and some callers get it wrong.

For example, to mean "this file expires in 60 minutes", you might pass either of these:

  • time() + phutil_units('60 minutes in seconds')
  • phutil_units('60 minutes in seconds')

The former means "60 minutes from now". The latter means "1 AM, January 1, 1970". In practice, because the GC normally runs only once every four hours (at least, until recently), and all the bad TTLs are cases where files are normally accessed immediately, these 1970 TTLs didn't cause any real problems.

Split ttl into ttl.relative and ttl.absolute, and make sure the values are sane. Then correct all callers, and simplify out the time() calls where possible to make switching to PhabricatorTime easier.

Test Plan
  • Generated an SSH keypair.
  • Viewed a changeset.
  • Viewed a raw diff.
  • Viewed a commit's file data.
  • Viewed a temporary file's details, saw expiration date and relative time.
  • Ran unit tests.
  • (Didn't really test Phragment.)

Diff Detail

Repository
rP Phabricator
Branch
files6
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 16293
Build 21659: Run Core Tests
Build 21658: arc lint + arc unit

Unit TestsFailed

TimeTest
2 msPhabricatorFileStorageFormatTestCase::Unknown Unit Message ("")
EXCEPTION (PhutilTypeExtraParametersException): Got unexpected parameters: format #0 /core/data/drydock/workingcopy-88/repo/phabricator/src/applications/files/storage/PhabricatorFile.php(1299): PhutilTypeSpec::checkMap(Array, Array) #1 /core/data/drydock/workingcopy-88/repo/phabricator/src/applications/files/storage/PhabricatorFile.php(398): PhabricatorFile->readPropertiesFromParameters(Array)
2 msPhabricatorFileStorageFormatTestCase::Unknown Unit Message ("")
EXCEPTION (PhutilTypeExtraParametersException): Got unexpected parameters: format #0 /core/data/drydock/workingcopy-88/repo/phabricator/src/applications/files/storage/PhabricatorFile.php(1299): PhutilTypeSpec::checkMap(Array, Array) #1 /core/data/drydock/workingcopy-88/repo/phabricator/src/applications/files/storage/PhabricatorFile.php(398): PhabricatorFile->readPropertiesFromParameters(Array)
1 msAlmanacNamesTestCase::Unknown Unit Message ("")
30 assertions passed.
0 msAlmanacServiceTypeTestCase::Unknown Unit Message ("")
1 assertion passed.
0 msAphrontHTTPSinkTestCase::Unknown Unit Message ("")
3 assertions passed.
View Full Test Results (2 Failed · 349 Passed)

Event Timeline

  • Recognize profile key in parameter dictionary.
  • Also recognize format key in parameter dictionary.
  • Also allow format to accept objects.
This revision is now accepted and ready to land.Apr 4 2017, 9:28 PM
This revision was automatically updated to reflect the committed changes.