Skip to content

Add setWhitelist method to allow controlling which files get replaced.#7

Merged
dg merged 4 commits intodg:masterfrom
afilina:whitelist
Jul 3, 2019
Merged

Add setWhitelist method to allow controlling which files get replaced.#7
dg merged 4 commits intodg:masterfrom
afilina:whitelist

Conversation

@afilina
Copy link
Copy Markdown
Contributor

@afilina afilina commented Feb 6, 2019

Reason behind this feature:
Some frameworks stop working because of DIR and such, since the files are now served from tmp. This prompted me to add a more fine-grained control.

I added a test and documentation. The old behavior remains unaffected.

@dg
Copy link
Copy Markdown
Owner

dg commented Feb 6, 2019

Thank you for the great PR. Whitelist can be useful.

I tried to test __DIR__ and __FILE__ (see 06d98c8) and it seems that these constants works as expected. Could you describe when they did not work?

@afilina
Copy link
Copy Markdown
Contributor Author

afilina commented Feb 6, 2019

@dg That's only a guess. It was deep within Laravel's call stack. Perhaps the added layer of Docker caused the problem. We stopped digging at some point and just created the exclusion. In any case, even if the bug was somewhere else, this still covers a number of legitimate scenarios.

@dg
Copy link
Copy Markdown
Owner

dg commented Feb 8, 2019

I am thinking about method name… It is not entirely clear that this is about paths, ie allowed paths. What about setAllowedPaths()? Or simply setPaths()? setDirectories()?

@afilina
Copy link
Copy Markdown
Contributor Author

afilina commented Feb 8, 2019

Sure, whichever you prefer.

@torchello
Copy link
Copy Markdown

torchello commented Mar 23, 2019

Actually phpspec stops generating PHP classes based on specs:

warning: file_get_contents(/usr/local/var/www/myproject/src/Repository/UserOrderRepository.php): failed to open stream: No such file or directory in /usr/local/var/www/newonline/vendor/dg/bypass-finals/src/
  BypassFinals.php line 211

Was able to get it working with @afilina fork and custom phpspec bootstrap:

<?php

require __DIR__.'/../vendor/autoload.php';

DG\BypassFinals::setWhitelist(findFinalClassesToBypass());
DG\BypassFinals::enable();

function findFinalClassesToBypass(): array
{
    $finalClasses = (new Symfony\Component\Finder\Finder())
        ->in([__DIR__.'/../vendor/symfony/security-core'])
        ->name('*.php')
        ->contains('final class');

    return array_values(array_map(
        function (SplFileInfo $fileInfo) {
            return $fileInfo->getRealPath();
        },
        iterator_to_array($finalClasses)
    ));
}

@afilina Thanks! 👍

@tarampampam
Copy link
Copy Markdown

@dg PR will be merged & released? This feature is very important for me too

tarampampam pushed a commit to avto-dev/bypass-finals that referenced this pull request Jun 25, 2019
@tarampampam
Copy link
Copy Markdown

@dg dg merged commit 59a0441 into dg:master Jul 3, 2019
@sergejostir
Copy link
Copy Markdown

Isn't the name kinda misleading since you cannot just set a path, but need to set every filepath individually? Also it doesn't take into consideration the difference between operating systems (\ vs /).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants