Skip to content

Commit

Permalink
Merge branch 'develop'
Browse files Browse the repository at this point in the history
* develop:
  add roave bc check
  fix leaving too many files opened
  throw when persisting closed stream
  reduce number of scenarii run locally
  • Loading branch information
Baptouuuu committed Jun 16, 2020
2 parents d95a736 + db9efa4 commit 6d97ce1
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 3 deletions.
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -3,6 +3,15 @@ name: CI
on: [push]

jobs:
roave_bc_check:
name: Roave BC Check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
- name: fetch tags
run: git fetch --depth=1 origin +refs/tags/*:refs/tags/*
- name: Roave BC Check
uses: docker://nyholm/roave-bc-check-ga
phpunit:
runs-on: ${{ matrix.os }}
strategy:
Expand Down
3 changes: 3 additions & 0 deletions phpunit.xml.dist
@@ -1,6 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit colors="true" bootstrap="vendor/autoload.php" printerClass="Innmind\BlackBox\PHPUnit\ResultPrinterV8">
<php>
<env name="BLACKBOX_SET_SIZE" value="1" />
</php>
<testsuites>
<testsuite name="Filesystem test suite">
<directory>./tests</directory>
Expand Down
11 changes: 11 additions & 0 deletions src/Adapter/Filesystem.php
Expand Up @@ -14,6 +14,7 @@
Exception\PathDoesntRepresentADirectory,
Exception\PathTooLong,
Exception\RuntimeException,
Exception\CannotPersistClosedStream,
Event\FileWasRemoved,
};
use Innmind\Stream\Writable\Stream;
Expand Down Expand Up @@ -152,6 +153,11 @@ function(Set $persisted, File $file) use ($path): Set {
}

$stream = $file->content();

if ($stream->closed()) {
throw new CannotPersistClosedStream($path->toString());
}

$stream->rewind();

try {
Expand All @@ -175,6 +181,11 @@ function(Set $persisted, File $file) use ($path): Set {
$stream->read(8192)->toEncoding('ASCII'),
);
}

// Calling the rewind here helps always leave the streams in a readable
// state. It also helps avoid a fatal error when handling too many files
// (see LazyStream::rewind() for more explanations)
$stream->rewind();
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/Exception/CannotPersistClosedStream.php
@@ -0,0 +1,8 @@
<?php
declare(strict_types = 1);

namespace Innmind\Filesystem\Exception;

final class CannotPersistClosedStream extends LogicException
{
}
11 changes: 9 additions & 2 deletions src/Stream/LazyStream.php
Expand Up @@ -45,8 +45,15 @@ public function seek(Position $position, Mode $mode = null): void

public function rewind(): void
{
if ($this->stream) {
$this->stream()->rewind();
if ($this->stream && !$this->stream->closed()) {
// this trick allows to automatically close the opened files on the
// system in order to avoid a fatal error when too many files are
// opened. This is possible because of the rewind done in
// Adapter\Filesystem::createFileAt() after persisting a file.
// This does not break be behaviour of the streams as once the stream
// is manually closed we won't reopen it here
$this->stream->close();
$this->stream = null;
}
}

Expand Down
23 changes: 22 additions & 1 deletion tests/Adapter/FilesystemTest.php
Expand Up @@ -15,6 +15,7 @@
Exception\FileNotFound,
Exception\PathDoesntRepresentADirectory,
Exception\PathTooLong,
Exception\CannotPersistClosedStream,
};
use Innmind\Url\Path;
use Innmind\Stream\Readable\Stream;
Expand All @@ -25,7 +26,10 @@
PHPUnit\BlackBox,
Set as DataSet,
};
use Fixtures\Innmind\Filesystem\Name as FName;
use Fixtures\Innmind\Filesystem\{
Name as FName,
File as FFile,
};
use Properties\Innmind\Filesystem\Adapter as PAdapter;

class FilesystemTest extends TestCase
Expand Down Expand Up @@ -368,6 +372,23 @@ public function testPersistedNameCanContainOnlyOneAsciiCharacter()
});
}

public function testThrowsWhenTryingToPersistClosedFileStream()
{
$this
->forAll(FFile::any())
->then(function($file) {
$path = \sys_get_temp_dir().'/innmind/filesystem/';
(new FS)->remove($path);

$filesystem = new Filesystem(Path::of($path));

$file->content()->close();
$this->expectException(CannotPersistClosedStream::class);

$filesystem->add($file);
});
}

public function properties(): iterable
{
foreach (PAdapter::list() as $property) {
Expand Down
24 changes: 24 additions & 0 deletions tests/Stream/LazyStreamTest.php
Expand Up @@ -10,9 +10,17 @@
};
use Innmind\Url\Path;
use PHPUnit\Framework\TestCase;
use Innmind\BlackBox\{
PHPUnit\BlackBox,
Set,
};
use Fixtures\Innmind\Stream\Readable as FReadable;
use Properties\Innmind\Stream\Readable as PReadable;

class LazyStreamTest extends TestCase
{
use BlackBox;

private $stream;

public function setUp(): void
Expand Down Expand Up @@ -111,4 +119,20 @@ public function testRead()

$this->assertSame('lorem', $stream->read(5)->toString());
}

public function testHoldProperties()
{
$this
->forAll(
PReadable::properties(),
FReadable::any(),
)
->then(function($properties, $content) {
$path = \tempnam(\sys_get_temp_dir(), 'lazy_stream');
$stream = new LazyStream(Path::of($path));
\file_put_contents($path, $content->toString());

$properties->ensureHeldBy($stream);
});
}
}

0 comments on commit 6d97ce1

Please sign in to comment.