Skip to content

Commit

Permalink
feature(filestore): adds API to reliably set file modification time
Browse files Browse the repository at this point in the history
Updates PHPUnit bootstrap to allow working with file entity objects. Modifies and adds new file related tests.

Adds two new methods ElggFile::setModifiedTime and ElggFile::getModifiedTime,
which invalidate stat cache to ensure we always have latest modified time information

Refs #9571
  • Loading branch information
hypeJunction committed Mar 28, 2016
1 parent 50721e5 commit 476b6d2
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 13 deletions.
8 changes: 8 additions & 0 deletions engine/classes/Elgg/FileService/File.php
Expand Up @@ -42,6 +42,14 @@ public function setFile(\ElggFile $file) {
$this->file = $file;
}

/**
* Returns file object
* @return \ElggFile|null
*/
public function getFile() {
return $this->file;
}

/**
* Sets URL expiration
*
Expand Down
23 changes: 23 additions & 0 deletions engine/classes/ElggFile.php
Expand Up @@ -317,6 +317,29 @@ public function tell() {
return $fs->tell($this->handle);
}

/**
* Updates modification time of the file and clears stats cache for the file
* @return bool
*/
public function setModifiedTime() {
$filestorename = $this->getFilenameOnFilestore();
$modified = touch($filestorename);
if ($modified) {
clearstatcache(true, $filestorename);
} else {
elgg_log("Unable to update modified time for $filestorename", 'ERROR');
}
return $modified;
}

/**
* Returns file modification time
* @return int
*/
public function getModifiedTime() {
return filemtime($this->getFilenameOnFilestore());
}

/**
* Return the size of the file in bytes.
*
Expand Down
57 changes: 45 additions & 12 deletions engine/tests/phpunit/Elgg/Application/ServeFileHandlerTest.php
Expand Up @@ -26,13 +26,14 @@ public function setUp() {

$this->handler = _elgg_services()->serveFileHandler;

$file_mock = $this->getMockBuilder('\ElggFile')->disableOriginalConstructor()->getMock();
$file_mock->method('getFileNameOnFilestore')->willReturn("{$dataroot}file_service/foobar.txt");
$file_mock->method('exists')->willReturn(true);
$this->file = $file_mock;
$file = new \ElggFile();
$file->owner_guid = 1;
$file->setFilename("foobar.txt");

$this->file = $file;
}

function createRequest($file) {
function createRequest(\Elgg\FileService\File $file) {
$site_url = elgg_get_site_url();
$url = $file->getURL();
$path = substr($url, strlen($site_url));
Expand Down Expand Up @@ -72,7 +73,7 @@ public function testSend403OnUrlExpiration() {
/**
* @group FileService
*/
public function testSends403OnFileModificationTimeMismatch () {
public function testSends403OnFileModificationTimeMismatch() {

$file = new \Elgg\FileService\File();
$file->setFile($this->file);
Expand All @@ -82,8 +83,13 @@ public function testSends403OnFileModificationTimeMismatch () {
$response = $this->handler->getResponse($request);
$this->assertEquals(200, $response->getStatusCode());

touch($this->file->getFilenameOnFilestore());
clearstatcache(true, $this->file->getFilenameOnFilestore());
// Consecutive request will be send by the browswer with an etag header
// We need to make sure we check modified time before issuing a Not Modified header
// See issue #9571
$request->headers->set('if_none_match', '"' . $this->file->getModifiedTime() . '"');

sleep(1); // sometimes tests are too fast
$this->file->setModifiedTime();

$response = $this->handler->getResponse($request);
$this->assertEquals(403, $response->getStatusCode());
Expand Down Expand Up @@ -120,18 +126,18 @@ public function testResponseHeadersMatchFileAttributesForInlineUrls() {
$file->setFile($this->file);
$file->setDisposition('inline');
$file->bindSession(false);

$request = $this->createRequest($file);
$response = $this->handler->getResponse($request);

$this->assertEquals('text/plain', $response->headers->get('Content-Type'));

$filesize = filesize($this->file->getFilenameOnFilestore());
$this->assertEquals($filesize, $response->headers->get('Content-Length'));

$this->assertContains('inline', $response->headers->get('Content-Disposition'));

$this->assertEquals('"' . filemtime($this->file->getFilenameOnFilestore()) . '"', $response->headers->get('Etag'));
$this->assertEquals('"' . $this->file->getModifiedTime() . '"', $response->headers->get('Etag'));
}

/**
Expand All @@ -153,7 +159,7 @@ public function testResponseHeadersMatchFileAttributesForAttachmentUrls() {

$this->assertContains('attachment', $response->headers->get('Content-Disposition'));

$this->assertEquals('"' . filemtime($this->file->getFilenameOnFilestore()) . '"', $response->headers->get('Etag'));
$this->assertEquals('"' . $this->file->getModifiedTime() . '"', $response->headers->get('Etag'));
}

/**
Expand All @@ -166,4 +172,31 @@ public function testInvalidDisposition() {
$file->setDisposition('foo');
}

/**
* @group FileService
*/
public function testSends304WithIfNoneMatchHeadersIncluded() {
$file = new \Elgg\FileService\File();
$file->setFile($this->file);

$request = $this->createRequest($file);
$request->headers->set('if_none_match', '"' . $this->file->getModifiedTime() . '"');

$response = $this->handler->getResponse($request);
$this->assertEquals(304, $response->getStatusCode());
}

/**
* @group FileService
*/
public function testSends304WithIfNoneMatchHeadersIncludedAndDeflationEnabled() {
$file = new \Elgg\FileService\File();
$file->setFile($this->file);

$request = $this->createRequest($file);
$request->headers->set('if_none_match', '"' . $this->file->getModifiedTime() . '-gzip"');

$response = $this->handler->getResponse($request);
$this->assertEquals(304, $response->getStatusCode());
}
}
31 changes: 31 additions & 0 deletions engine/tests/phpunit/ElggFileTest.php
@@ -0,0 +1,31 @@
<?php

class ElggFileTest extends \PHPUnit_Framework_TestCase {

/**
* @var \ElggFile
*/
protected $file;

protected function setUp() {

$session = \ElggSession::getMock();
_elgg_services()->setValue('session', $session);
_elgg_services()->session->start();

$file = new \ElggFile();
$file->owner_guid = 1;
$file->setFilename("foobar.txt");

$this->file = $file;
}

/**
* @group FileService
*/
public function testCanSetModifiedTime() {
$time = $this->file->getModifiedTime();
$this->file->setModifiedTime();
$this->assertNotEquals($time, $this->file->getModifiedTime());
}
}
3 changes: 3 additions & 0 deletions engine/tests/phpunit/bootstrap.php
Expand Up @@ -71,5 +71,8 @@ function _elgg_testing_config(\Elgg\Config $config = null) {
// persistentLogin service needs this set to instantiate without calling DB
_elgg_services()->config->getCookieConfig();

global $GLOBALS;
$GLOBALS['DEFAULT_FILE_STORE'] = new \ElggDiskFilestore($CONFIG->dataroot);

_elgg_testing_application($app);
});
1 change: 1 addition & 0 deletions engine/tests/phpunit/test_files/dataroot/1/1/foobar.txt
@@ -0,0 +1 @@
Hello world!

This file was deleted.

0 comments on commit 476b6d2

Please sign in to comment.